Feature/agentic buffered #4231
Feature/agentic buffered #4231
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
The PR introduces significant changes to enhance agent search functionality and improve code quality across the Onyx backend. Here's a summary of the key changes:
- Added new models and methods in
chat/models.pyfor organizing sub-questions hierarchically, includingSubQuestionKeyclass andmake_dict_by_levelfunctionality - Implemented buffered agent responses in
chat_backend.pywith support for sub-questions, refined answers, and document tracking - Fixed widespread typo by renaming
current_chat_accesssible_usertocurrent_chat_accessible_useracross multiple files - Added database connection cleanup with
SqlEngine.reset_engine()in FastAPI application lifecycle - Introduced comprehensive agent test script with timing metrics and comparison between basic and agent-based search flows
The changes appear well-structured but the test script could benefit from improved resource management and validation.
12 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| level: int | None = None | ||
| level_question_num: int | None = None | ||
|
|
||
| @staticmethod | ||
| def make_dict_by_level( | ||
| original_dict: Mapping[tuple[int | None, int | None], "SubQuestionIdentifier"] |
There was a problem hiding this comment.
I'm in favor of removing all the | None's and asserting that values aren't None for typing; probably better to find out that way if something breaks
There was a problem hiding this comment.
It might be out of scope for this PR, but I'm now leaning that we should have made the fields for SubQuestionIdentifier non-Nullable and added it as an attribute instead of subclassing... oh well
| def make_dict_by_level( | ||
| original_dict: Mapping[tuple[int | None, int | None], "SubQuestionIdentifier"] | ||
| ) -> dict[int | None, list["SubQuestionIdentifier"]]: | ||
| """returns a dict of level to object list (sorted by level_question_num) |
There was a problem hiding this comment.
Maybe we can make this return a list once we purge the Nones?
There was a problem hiding this comment.
there's still levels and questions per level, so i think it's better the way it is.
| value2, key=lambda o: o.query_id | ||
| ) | ||
| # sort the question_index dict of level | ||
| level_question_dict[key1] = OrderedDict( |
There was a problem hiding this comment.
In favor of getting rid of None's explicitly via assert and using lists
Description
Fixes DAN-1499.
https://linear.app/danswer/issue/DAN-1499/agent-search-non-streaming-endpoints
How Has This Been Tested?
[Describe the tests you ran to verify your changes]
Backporting (check the box to trigger backport action)
Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.