Skip to content
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

feat(agents-api): Adaptive context (via recursive summarization) #306

Merged
merged 45 commits into from
Jun 11, 2024

Conversation

creatorrr
Copy link
Contributor

@creatorrr creatorrr commented May 1, 2024

Closes #328


🚀 This description was created by Ellipsis for commit 0ddc271

Summary:

This PR enhances the agents-api by improving session management, message summarization, introducing new features, and updating the OpenAPI model.

Key points:

  • Enhances 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.
  • Refactored summarize_messages in summarization.py for efficiency.
  • Introduced token_budget and context_overflow for dynamic session management.
  • Updated OpenAPI model and session handling logic.
  • Fixed various issues across modules for improved functionality.

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 the entire pull request up to 11ac607
  • Looked at 14 lines of code in 1 files
  • Took 51 seconds to review
More info
  • Skipped 2 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.
1. agents-api/pyproject.toml:42:
  • Assessed confidence : 66%
  • Grade: 0%
  • Comment:
    The addition of jupyterlab and ipywidgets to the dev dependencies 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 of jupyterlab and ipywidgets to the dev dependencies section of the pyproject.toml file 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.

Copy link
Contributor

ellipsis-dev bot commented May 1, 2024

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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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!

  • Performed an incremental review on e716d90
  • Looked at 9 lines of code in 1 files
  • Took 53 seconds to review
More info
  • Skipped 1 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.
1. /agents-api/notebooks/test-chat.json:1:
  • Assessed confidence : 66%
  • Grade: 0%
  • Comment:
    Ensure that the JSON structure in test-chat.json is 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.

Copy link
Contributor

ellipsis-dev bot commented May 3, 2024

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

Copy link
Contributor

ellipsis-dev bot commented May 3, 2024

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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 83803dc in 2 minutes and 19 seconds

More details
  • Looked at 848 lines of code in 13 files
  • Skipped 1 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.
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:
    The generate function in generate.py uses the AsyncClient from the openai library to create chat completions. The function is decorated with a retry mechanism from the tenacity library, 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.

Copy link
Contributor

ellipsis-dev bot commented May 21, 2024

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

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on c05113f in 3 minutes and 46 seconds

More details
  • Looked at 60 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted 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.

agents-api/agents_api/activities/summarization.py Outdated Show resolved Hide resolved
@whiterabbit1983 whiterabbit1983 force-pushed the f/rec-sum-experiments branch from c05113f to a41a85b Compare May 22, 2024 19:28
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on a41a85b in 3 minutes and 45 seconds

More details
  • Looked at 550 lines of code in 8 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted 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.

agents-api/agents_api/activities/summarization.py Outdated Show resolved Hide resolved
@whiterabbit1983 whiterabbit1983 force-pushed the f/rec-sum-experiments branch from a41a85b to 4925666 Compare May 24, 2024 11:49
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 4925666 in 4 minutes and 36 seconds

More details
  • Looked at 640 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted 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 whiterabbit1983 force-pushed the f/rec-sum-experiments branch from 4925666 to 88aa9b2 Compare May 24, 2024 11:54
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on 88aa9b2 in 4 minutes and 35 seconds

More details
  • Looked at 642 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted 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.

agents-api/agents_api/activities/logger.py Show resolved Hide resolved
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 0003f29 in 3 minutes and 49 seconds

More details
  • Looked at 24 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/activities/summarization.py:211
  • Draft comment:
    Ensure that summarization_model_name is 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 142cffe in 4 minutes and 38 seconds

More details
  • Looked at 41 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on 91a9e78 in 4 minutes and 12 seconds

More details
  • Looked at 41 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted 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.

agents-api/agents_api/rec_sum/generate.py Show resolved Hide resolved
@hbrooks
Copy link

hbrooks commented May 25, 2024

Hey @creatorrr, founder @ellipsis-dev here. You can disable code reviews from certain branch names, such as into dev or from f/* by adding a config file: https://docs.ellipsis.dev/config#ignore-certain-basehead-branches.

Copy link
Contributor

ellipsis-dev bot commented May 25, 2024

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):

  • review and summarize this pull request
  • make changes to this pull request
  • answer questions about this PR, or about the codebase in general
  • hide my old reviews on this PR (to reduce clutter)

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on e08dbc4 in 1 minute and 47 seconds

More details
  • Looked at 65 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted 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.

agents-api/agents_api/activities/summarization.py Outdated Show resolved Hide resolved
@whiterabbit1983 whiterabbit1983 force-pushed the f/rec-sum-experiments branch from e08dbc4 to bb75af4 Compare May 28, 2024 06:37
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on bb75af4 in 3 minutes and 48 seconds

More details
  • Looked at 65 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/activities/summarization.py:194
  • Draft comment:
    Ensure that entries_summarization_query correctly handles the deletion of old entries as intended, since delete_entries_by_ids_query was 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 7f286dd in 2 minutes and 4 seconds

More details
  • Looked at 12 lines of code in 1 files
  • Skipped 1 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/pyproject.toml:33
  • Draft comment:
    The addition of the tenacity package 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on e2eec7f in 1 minute and 51 seconds

More details
  • Looked at 15 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/rec_sum/trim.py:62
  • Draft comment:
    The PR description mentions significant changes in summarization.py and removal of unused imports, but the diff provided only shows changes in trim.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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 2dfbe42 in 1 minute and 46 seconds

More details
  • Looked at 16 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/rec_sum/summarize.py:77
  • Draft comment:
    The PR description mentions significant refactoring of the summarization function, 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 whiterabbit1983 force-pushed the f/rec-sum-experiments branch from 2dfbe42 to cee49d7 Compare May 28, 2024 08:48
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on cee49d7 in 2 minutes and 36 seconds

More details
  • Looked at 16 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted 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 of stop_after_attempt and wait_fixed or wait_random_exponential to 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 9bb40bb in 2 minutes and 20 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted 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 using tenacity, 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on f38bf3e in 2 minutes and 27 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:90
  • Draft comment:
    The query operation :rm entries is 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on d313315 in 2 minutes and 42 seconds

More details
  • Looked at 50 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:83
  • Draft comment:
    The usage of to_uuid function 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 without to_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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 9b58614 in 4 minutes and 14 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted 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 handle None values for the name field 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on 08f6295 in 1 minute and 21 seconds

More details
  • Looked at 124 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 8f8b604 in 1 minute and 58 seconds

More details
  • Looked at 10 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:115
  • Draft comment:
    The query uses $entry_keys which is not defined or replaced in the function. This will cause the query to fail as the actual entry_ids are not being passed correctly. Consider replacing $entry_keys with 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 9f74d34 in 1 minute and 33 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/models/entry/delete_entries.py:70
  • Draft comment:
    The change from $entry_keys to $entry_ids in 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 in delete_entries to 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 used entry_keys in the query, but the dictionary key was not shown. The change to entry_ids in the query should correspond to the dictionary key, which is correctly shown in the return statement as entry_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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on 97d265d in 1 minute and 38 seconds

More details
  • Looked at 10 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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! Incremental review on 2c4ceb6 in 2 minutes and 44 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/activities/summarization.py:212
  • Draft comment:
    The change in indexing for old_entry_ids might introduce an off-by-one error. If the indices in msg["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 for msg["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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on ddbf7d0 in 3 minutes and 17 seconds

More details
  • Looked at 15 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted 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.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on cde5055 in 1 minute and 41 seconds

More details
  • Looked at 42 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_fSVzh65rSZQIgppm


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.

agents-api/agents_api/activities/summarization.py Outdated Show resolved Hide resolved
@whiterabbit1983 whiterabbit1983 force-pushed the f/rec-sum-experiments branch from cde5055 to 0ddc271 Compare June 8, 2024 15:23
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Incremental review on 0ddc271 in 1 minute and 58 seconds

More details
  • Looked at 30 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_A3NlWLkOVX0gwmhe


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 whiterabbit1983 force-pushed the f/rec-sum-experiments branch from 0ddc271 to 8fc6ad6 Compare June 8, 2024 18:22
@whiterabbit1983 whiterabbit1983 force-pushed the f/rec-sum-experiments branch from 8fc6ad6 to 9a4935a Compare June 10, 2024 07:26
@creatorrr creatorrr merged commit 5b06179 into dev Jun 11, 2024
4 of 8 checks passed
@creatorrr creatorrr deleted the f/rec-sum-experiments branch June 11, 2024 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat: Implement new recursive summarization logic
4 participants