-
Notifications
You must be signed in to change notification settings - Fork 332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/add conversation logger merged #1385
Conversation
* cli tests * add sdk tests * typo fix * change workflow ordering * add collection integration tests (#1352) * bump pkg * remove workflows * fix sdk test port * fix delete collection return check * Fix document info serialization (#1353) * Update integration-test-workflow-debian.yml * pre-commit * slightly modify * up * up * smaller file * up * typo, change order * up * up * change order --------- Co-authored-by: emrgnt-cmplxty <68796651+emrgnt-cmplxty@users.noreply.github.com> Co-authored-by: emrgnt-cmplxty <owen@algofi.org> Co-authored-by: Nolan Tremelling <34580718+NolanTrem@users.noreply.github.com>
* add documentation * up * Update js/sdk/src/models.tsx Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> * pre-commit --------- Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: Simeon <simeon@theobald.nz>
* cleanups * add back overzealous edits * extend workflows * fix full setup * simplify cli * add ymls * rename to light * try again * start light * add cli tests * fix * fix * testing.. * trying complete matrix testflow * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * up * up * up * All actions * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * try offic pgvec formula * sudo make * sudo make * push and pray * push and pray * add new actions * add new actions * docker push & pray * inspect manifests during launch * inspect manifests during launch * inspect manifests during launch * inspect manifests during launch * setup docker * setup docker * fix default * fix default
* cleanups * add back overzealous edits * extend workflows * fix full setup * simplify cli * add ymls * rename to light * try again * start light * add cli tests * fix * fix * testing.. * trying complete matrix testflow * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * cleanup matrix logic * up * up * up * All actions * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * rename to runner * try offic pgvec formula * sudo make * sudo make * push and pray * push and pray * add new actions * add new actions * docker push & pray * inspect manifests during launch * inspect manifests during launch * inspect manifests during launch * inspect manifests during launch * setup docker * setup docker * fix default * fix default * make changes
* up * up * modify assertion * up * up * increase entity limit * changing aristotle back to v2 * pre-commit * typos * add test_ingest_sample_file_2_sdk
* add docs and refine code * add python SDK documentation * up
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 81eef69 in 1 minute and 21 seconds
More details
- Looked at
2662
lines of code in41
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. py/core/agent/base.py:54
- Draft comment:
Thearun
method has a TODO comment indicating it should return a list of messages. Ensure this is implemented as intended. - Reason this comment was not posted:
Confidence changes required:50%
Thearun
method inpy/core/agent/base.py
has a TODO comment indicating that it should return a list of messages. This is a clear indication that the current implementation might not be complete or correct as per the intended design.
2. py/core/base/logging/run_logger.py:132
- Draft comment:
Theedit_message
method creates a new branch and message. Ensure this doesn't lead to unnecessary data duplication. - Reason this comment was not posted:
Confidence changes required:50%
Theedit_message
method inpy/core/base/logging/run_logger.py
creates a new branch and a new message when editing. This might lead to unnecessary data duplication if not handled properly. Consider optimizing this process.
3. py/core/base/logging/run_logger.py:303
- Draft comment:
Theget_conversation
method returns an empty list if no branches are found. Consider raising an exception or returning a more informative response. - Reason this comment was not posted:
Confidence changes required:50%
Theget_conversation
method inpy/core/base/logging/run_logger.py
returns an empty list if no branches are found. This might not be the best way to handle this case, as it could lead to confusion. Consider raising an exception or returning a more informative response.
4. py/core/main/api/retrieval_router.py:216
- Draft comment:
Theagent_app
function has bothmessage
andmessages
parameters, withmessages
marked as deprecated. Consider removing the deprecated parameter to avoid confusion. - Reason this comment was not posted:
Confidence changes required:50%
Inpy/core/main/api/retrieval_router.py
, theagent_app
function has bothmessage
andmessages
parameters, withmessages
marked as deprecated. This could lead to confusion. Consider removing the deprecated parameter.
5. py/core/main/services/retrieval_service.py:370
- Draft comment:
Remove or replace the print statement with proper logging. - Reason this comment was not posted:
Confidence changes required:50%
Inpy/core/main/services/retrieval_service.py
, theagent
method has a print statement for debugging purposes. This should be removed or replaced with proper logging before merging.
Workflow ID: wflow_pqTDm2WhIpjlSpEK
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Important
This PR adds a conversation logging feature with history and branching support, updates API endpoints, models, and tests to manage conversations effectively.
core/base/logging/run_logger.py
./v2/conversations/{conversation_id}
inmanagement_router.py
to retrieve conversation history.agent_app
inretrieval_router.py
to handle conversation IDs and branching.WrappedConversationResponse
inmanagement/responses.py
for conversation data.RAGAgentResponse
inretrieval/responses.py
to includeconversation_id
.test_chat_logging_provider.py
.test_conversation_history_sdk
inrunner_sdk.py
.score_completion
functionality frommanagement_router.py
and related files.Message
class inllm.py
to support new conversation features.This description was created by for 81eef69. It will automatically update as commits are pushed.