feat(agents-api): Adaptive context (via recursive summarization) #306
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me!
- Reviewed the entire pull request up to 11ac607
- Looked at
14lines of code in1files - Took 51 seconds to review
More info
- Skipped
2files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. agents-api/pyproject.toml:42:
- Assessed confidence :
66% - Grade:
0% - Comment:
The addition ofjupyterlabandipywidgetsto thedevdependencies is not mentioned in the PR description. Please clarify the need for these dependencies to ensure they are necessary for the project's development environment. - Reasoning:
The addition ofjupyterlabandipywidgetsto thedevdependencies section of thepyproject.tomlfile suggests that these packages are intended for development purposes, likely for creating or testing Jupyter notebooks. However, the PR title and description do not mention any specific use of Jupyter notebooks or a need for these dependencies. It's important to ensure that every dependency added is justified to avoid bloating the project with unnecessary packages, which can lead to longer build times and potential conflicts.
Workflow ID: wflow_LminO3ZJJVwl3Jkk
You can customize Ellipsis with review rules, user-specific overrides, quiet mode, and more. See docs.
|
Skipped PR review on b14b0f7 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away. Generated with ❤️ by ellipsis.dev |
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on e716d90
- Looked at
9lines of code in1files - Took 53 seconds to review
More info
- Skipped
1files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. /agents-api/notebooks/test-chat.json:1:
- Assessed confidence :
66% - Grade:
0% - Comment:
Ensure that the JSON structure intest-chat.jsonis correctly parsed and utilized within the application. Verify that the file's integration points (like loading and parsing mechanisms) are robust against potential JSON format errors. - Reasoning:
The JSON file added in this PR contains a conversation script for a chat simulation. It's important to ensure that the JSON structure is correct and that there are no syntax errors, as these could cause issues when the file is parsed by the application. The JSON appears to be well-formed with appropriate key-value pairs for each message role and content. However, it's crucial to check if the file is being used correctly in the context of the application, such as being properly loaded and parsed in the chat simulation environment.
Workflow ID: wflow_EPvmUyKYikz8lTUi
You can customize Ellipsis with review rules, user-specific overrides, quiet mode, and more. See docs.
|
Skipped PR review on 5fb33ad because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away. Generated with ❤️ by ellipsis.dev |
|
Skipped PR review on f40c35f because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away. Generated with ❤️ by ellipsis.dev |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 83803dc in 2 minutes and 19 seconds
More details
- Looked at
848lines of code in13files - Skipped
1files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. /agents-api/notebooks/rec_sum/generate.py:8:
- Assessed confidence :
66% - Grade:
0% - Comment:
Consider handling specific exceptions in the retry decorator to make retries more meaningful and efficient. For example, you might want to retry on specific exceptions like network errors or API rate limits. - Reasoning:
Thegeneratefunction ingenerate.pyuses theAsyncClientfrom theopenailibrary to create chat completions. The function is decorated with a retry mechanism from thetenacitylibrary, which is set to retry up to 5 times with a fixed wait of 2 seconds between attempts. This is a common pattern to handle potential transient issues in network calls or service availability. However, the function could be improved by handling specific exceptions that might occur during the API call, such as network errors or API rate limits, to make retries more meaningful and efficient.
Workflow ID: wflow_LDOX3DZ5icz4XLAR
You can customize Ellipsis with review rules, user-specific overrides, quiet mode, and more. See docs.
|
Skipped PR review on e77381e because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away. Generated with ❤️ by ellipsis.dev |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on c05113f in 3 minutes and 46 seconds
More details
- Looked at
60lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_9oPGJL9pgUhJZPRs
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
whiterabbit1983
force-pushed
the
f/rec-sum-experiments
branch
from
c05113f
to
a41a85b
Compare
There was a problem hiding this comment.
❌ Changes requested. Incremental review on a41a85b in 3 minutes and 45 seconds
More details
- Looked at
550lines of code in8files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_0mRqc8Qby7Cvd3qN
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| name="entities", | ||
| content=entities["content"], | ||
| timestamp=entries[0]["timestamp"] + ts_delta, | ||
| ) | ||
| ) | ||
|
|
There was a problem hiding this comment.
(See comment below about the entries query)
So this produces {"role": "system", "name": "entities", "content": ...} messages.
But this part needs some logic for handling a pre-existing entities message. If one exists then the new one will replace the old one. Maybe this needs to happen at the part where the resulting messages are saved with a relation.
whiterabbit1983
force-pushed
the
f/rec-sum-experiments
branch
from
a41a85b
to
4925666
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 4925666 in 4 minutes and 36 seconds
More details
- Looked at
640lines of code in10files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:65
- Draft comment:
The formatting of UUIDs into the query string is incorrect. The UUIDs are being converted to strings, which may not be correctly interpreted by the database expecting UUIDs. Consider passing the UUIDs as parameters to the query without converting them to strings.
entry_ids = [id for id in entry_ids]
- Reason this comment was not posted:
Confidence of 30% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_RuFLMQus3enNLbut
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
whiterabbit1983
force-pushed
the
f/rec-sum-experiments
branch
from
4925666
to
88aa9b2
Compare
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 88aa9b2 in 4 minutes and 35 seconds
More details
- Looked at
642lines of code in10files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_rbgtRnlNrtHboCnV
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 0003f29 in 3 minutes and 49 seconds
More details
- Looked at
24lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/activities/summarization.py:211
- Draft comment:
Ensure thatsummarization_model_nameis validated before use to confirm it corresponds to a valid model capable of performing the required tasks. This is important to prevent runtime errors due to invalid or unset model names. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_hkhTGLWRhNJ4iYxw
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 142cffe in 4 minutes and 38 seconds
More details
- Looked at
41lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/activities/summarization.py:165
- Draft comment:
Consider removing this large commented-out block of code if it is no longer in use to keep the codebase clean and maintainable. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_geefcEwGU2rgpWMv
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 91a9e78 in 4 minutes and 12 seconds
More details
- Looked at
41lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_PgwXatFt6stiLeV7
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Hey @creatorrr, founder @ellipsis-dev here. You can disable code reviews from certain branch names, such as into |
|
Sorry, I don't know how to help with that. If you tag me (@ellipsis-dev) in a comment, I can do one of these things (but not multiple at once):
|
There was a problem hiding this comment.
❌ Changes requested. Incremental review on e08dbc4 in 1 minute and 47 seconds
More details
- Looked at
65lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_ockFwLqrlzEmPTgG
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
whiterabbit1983
force-pushed
the
f/rec-sum-experiments
branch
from
e08dbc4
to
bb75af4
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on bb75af4 in 3 minutes and 48 seconds
More details
- Looked at
65lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/activities/summarization.py:194
- Draft comment:
Ensure thatentries_summarization_querycorrectly handles the deletion of old entries as intended, sincedelete_entries_by_ids_querywas removed. This is crucial for maintaining data integrity. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_QA8XlJbZ4mNSDH6M
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 7f286dd in 2 minutes and 4 seconds
More details
- Looked at
12lines of code in1files - Skipped
1files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/pyproject.toml:33
- Draft comment:
The addition of thetenacitypackage is noted. Please ensure that this package is actually used in the project as there is no mention of its usage in the PR details. Adding unused dependencies can lead to increased maintenance and potential conflicts. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_MYaaUARDeGOPV9Wd
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on e2eec7f in 1 minute and 51 seconds
More details
- Looked at
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/rec_sum/trim.py:62
- Draft comment:
The PR description mentions significant changes insummarization.pyand removal of unused imports, but the diff provided only shows changes intrim.py. Please ensure that the PR description accurately reflects the changes made or update the PR to include all intended changes. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_fTyPji3ml0fWfdyh
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 2dfbe42 in 1 minute and 46 seconds
More details
- Looked at
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/rec_sum/summarize.py:77
- Draft comment:
The PR description mentions significant refactoring of thesummarizationfunction, but the diff provided does not reflect these changes. Please ensure that the correct files and changes are included in the PR. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_ATteQakeBuEjcyZe
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
whiterabbit1983
force-pushed
the
f/rec-sum-experiments
branch
from
2dfbe42
to
cee49d7
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on cee49d7 in 2 minutes and 36 seconds
More details
- Looked at
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/rec_sum/summarize.py:77
- Draft comment:
The retry strategy here stops after only 2 attempts. Consider using a more robust retry strategy, possibly with a combination ofstop_after_attemptandwait_fixedorwait_random_exponentialto handle transient errors more effectively. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_2Hsm7G0goJky6624
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 9bb40bb in 2 minutes and 20 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/activities/summarization.py:190
- Draft comment:
The PR description mentions the addition of retry capabilities usingtenacity, but there is no implementation of this in the provided code. Please ensure that the retry logic is implemented as described or update the PR description if it was included by mistake. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_KxCwe2sz6XVPV770
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
whiterabbit1983
force-pushed
the
f/rec-sum-experiments
branch
from
e93ed4b
to
2e20a4c
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on e93ed4b in 1 minute and 40 seconds
More details
- Looked at
36lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/worker/__main__.py:77
- Draft comment:
Ensure that the newTruncationWorkflowand its associated activitytruncationare thoroughly tested, especially since they are part of the critical path of session management and message summarization enhancements. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_lgwiT2q2xmjf3Oet
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 2e20a4c in 1 minute and 59 seconds
More details
- Looked at
36lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/worker/__main__.py:77
- Draft comment:
Ensure that theTruncationWorkflowis thoroughly tested since it's a new addition to the worker's capabilities. This is crucial for maintaining stability and performance of the worker. - Reason this comment was not posted:
Confidence changes required:33%
The PR description mentions the introduction of a newTruncationWorkflowand its associated activitytruncation. The code in__main__.pyhas been updated to include these in the worker's configuration. This is necessary for the worker to be able to handle tasks related to the new truncation functionality. The import statements and the worker setup both reflect these changes, which aligns with the PR's intent to enhance session management and message summarization.
Workflow ID: wflow_4tBppMIasJdouzjS
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on bb41d92 in 2 minutes and 26 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_z0dHJI8WY3SvzKfX
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
|
||
| @cozo_query | ||
| def delete_entries_by_ids_query(entry_ids: list[UUID]) -> tuple[str, dict]: | ||
| entry_ids = [[f'to_uuid("{id}")' for id in entry_ids]] |
There was a problem hiding this comment.
The list comprehension here is incorrectly wrapped in an additional list, which will result in a list of lists rather than a flat list of UUID strings. This should be corrected to match the expected format for the query.
| entry_ids = [[f'to_uuid("{id}")' for id in entry_ids]] | |
| entry_ids = [f'to_uuid("{id}")' for id in entry_ids] |
whiterabbit1983
force-pushed
the
f/rec-sum-experiments
branch
from
bb41d92
to
c824516
Compare
There was a problem hiding this comment.
❌ Changes requested. Incremental review on c824516 in 2 minutes and 29 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_iUEQHzHdZE6x52Ei
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
|
||
| @cozo_query | ||
| def delete_entries_by_ids_query(entry_ids: list[UUID]) -> tuple[str, dict]: | ||
| entry_ids = [[f'to_uuid("{id}")'] for id in entry_ids] |
There was a problem hiding this comment.
The list comprehension incorrectly wraps each UUID in a list. This should be a flat list of string representations of UUIDs.
| entry_ids = [[f'to_uuid("{id}")'] for id in entry_ids] | |
| entry_ids = [f'to_uuid("{id}")' for id in entry_ids] |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 4e7798a in 3 minutes and 2 seconds
More details
- Looked at
205lines of code in3files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_YWzXx7roEC2VikFb
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
|
||
| for m in reversed(messages[offset:]): | ||
| token_cnt += m.token_count | ||
| if token_cnt < token_count_threshold: |
There was a problem hiding this comment.
The logic in get_extra_entries might incorrectly include messages that do not individually exceed the token count threshold. Consider revising the logic to ensure that only messages that contribute to exceeding the threshold are included.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on e696951 in 2 minutes and 41 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:74
- Draft comment:
The conditional check fore.role.valueensures compatibility with both enum types and direct string values for roles. This is a good practice to maintain flexibility in the data model. - Reason this comment was not posted:
Confidence changes required:0%
The code in delete_entries.py is handling the deletion of entries based on a list of Entry objects. The code constructs a query to delete entries from a database. The line in question checks if the 'role' attribute of an Entry object has a 'value' attribute and uses it if present; otherwise, it uses the 'role' directly. This is a common pattern when dealing with enums in Python, where the enum member has a 'value' attribute that holds the actual value to be used in operations like database queries. This approach ensures that the correct value is used in the query, whether 'role' is an enum or a plain value.
Workflow ID: wflow_VUfmx4bfXlcEZKs7
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on f38bf3e in 2 minutes and 27 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:90
- Draft comment:
The query operation:rm entriesis not recognized in standard SQL. Please ensure that this is the correct syntax for the database query language being used. If this is intended for an SQL database, it should be replaced with the correct SQL DELETE operation. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_HvqehyirJsrkPWdN
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on d313315 in 2 minutes and 42 seconds
More details
- Looked at
50lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:83
- Draft comment:
The usage ofto_uuidfunction here seems incorrect as it is typically used to convert a string to a UUID, not the other way around. If the intent is to ensure the UUIDs are in string format for the query, you should directly convert them to strings withoutto_uuid.
f'"{str(e.id)}"',
f'"{str(e.session_id)}"',
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_o6BCWzvQDvpIZM25
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 9b58614 in 4 minutes and 14 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:76
- Draft comment:
Ensure that the database schema and operations can handleNonevalues for thenamefield appropriately to avoid potential issues. - Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_YYR50Hkudir3RJ7s
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 08f6295 in 1 minute and 21 seconds
More details
- Looked at
124lines of code in3files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_TZZ72Ux8Buf72cMn
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
|
||
| def get_extra_entries(messages: list[Entry], token_count_threshold: int) -> list[UUID]: | ||
| if not len(messages): | ||
| return messages |
There was a problem hiding this comment.
Returning messages directly when it's empty leads to a type inconsistency since the function should return a list of UUIDs. Consider returning an empty list of UUIDs instead.
| return messages | |
| return [] |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 8f8b604 in 1 minute and 58 seconds
More details
- Looked at
10lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:115
- Draft comment:
The query uses$entry_keyswhich is not defined or replaced in the function. This will cause the query to fail as the actualentry_idsare not being passed correctly. Consider replacing$entry_keyswith the correct placeholder and ensure it is properly formatted in the query parameters.
return (query, {'entry_keys': [str(id) for id in entry_ids]})
- Reason this comment was not posted:
Confidence of 20% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_dycNRCsSyCJSbcZf
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 9f74d34 in 1 minute and 33 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:70
- Draft comment:
The change from$entry_keysto$entry_idsin the query is correct and aligns with the dictionary key used in the return statement. This ensures that the variable names are consistent and correctly reference the passed UUIDs for deletion. - Reason this comment was not posted:
Confidence changes required:0%
The PR description mentions fixing list handling indelete_entriesto properly handle UUIDs and robust role handling. However, the change in the diff seems incorrect. The variable name in the query should match the key in the dictionary passed to the query. The original code usedentry_keysin the query, but the dictionary key was not shown. The change toentry_idsin the query should correspond to the dictionary key, which is correctly shown in the return statement asentry_ids. This suggests that the change in the query is correct and aligns with the dictionary key used in the return statement.
Workflow ID: wflow_1w4lcqkN9TYHwuKi
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 97d265d in 1 minute and 38 seconds
More details
- Looked at
10lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_gviMm647r7Q6RvzU
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| :returning | ||
| }""" | ||
|
|
||
| return (query, {"entry_ids": [[str(id)] for id in entry_ids]}) |
There was a problem hiding this comment.
The change to wrap str(id) in an additional list seems incorrect and could potentially break the query. The expected format is likely a flat list of string UUIDs. Please revert to the original format:
| return (query, {"entry_ids": [[str(id)] for id in entry_ids]}) | |
| return (query, {"entry_ids": [str(id) for id in entry_ids]}) |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 2c4ceb6 in 2 minutes and 44 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. agents-api/agents_api/activities/summarization.py:212
- Draft comment:
The change in indexing forold_entry_idsmight introduce an off-by-one error. If the indices inmsg["summarizes"]are 0-based, the original indexing method should be used to avoid accessing incorrect or non-existent entries. Please verify the expected index base formsg["summarizes"]and adjust accordingly.
UUID(entries[idx]["entry_id"], version=4)
- Reason this comment was not posted:
Confidence of 40% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_p7GGvJsDNU2TJyBg
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on ddbf7d0 in 3 minutes and 17 seconds
More details
- Looked at
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_aH8DZKOCRK4Betav
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| summarized = await summarize_messages( | ||
| trimmed_messages, model=summarization_model_name | ||
| ) | ||
| ts_delta = (entries[1]["timestamp"] - entries[0]["timestamp"]) / 2 |
There was a problem hiding this comment.
The calculation of ts_delta assumes there are at least two entries and that they are ordered by timestamp. This could lead to errors if there's only one entry or if the entries are not in the expected order. Consider adding checks to ensure there are sufficient entries and handle cases where these assumptions might not hold.
Closes #328
Summary:
This PR enhances the agents-api by improving session management, message summarization, and introducing new features like
TruncationWorkflow, along with updates to the OpenAPI model and session handling logic.Key points:
summarize_messagesfor better efficiency and clarity.summarize_messagesusingtenacity.summarize_messages.stringify_contentin/agents-api/agents_api/common/utils/messages.py.stringify_contentacross various modules for consistent message handling.delete_entriesto properly handle UUIDs and robust role handling.get_entitiesin/agents-api/agents_api/rec_sum/entities.py.token_budgetandcontext_overflowfor dynamic session context management.Entryclass for different content types.token_budgetandcontext_overflowin session-related models and schemas./agents-api/tests/test_sessions_context.pyto ensure functionality of truncation logic and new tests for truncation logic in sessions.TruncationWorkflowin/agents-api/agents_api/worker/__main__.py.Generated with ❤️ by ellipsis.dev