feat: add graph representation of agent network#15056
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e24a0f8c4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3b9f1b72c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
charley-oai
left a comment
charley-oai
left a comment
There was a problem hiding this comment.
Looks graphical to me
| let mut resume_queue = VecDeque::from([(thread_id, root_depth)]); | ||
| while let Some((parent_thread_id, parent_depth)) = resume_queue.pop_front() { | ||
| let child_ids = match state_db_ctx | ||
| .list_thread_spawn_children_with_status( |
There was a problem hiding this comment.
Could maybe just call list_thread_spawn_descendants_matching and then share code with live_thread_spawn_children to traverse the tree in level order except a separate sqlite query per level. However I imagine the num levels will normally be small so that's prob not necessary.
Maybe vague benefit would be to share more code btwn the in-memory traversals and the sqlite traversals
The other slightly confusing thing about reviewing this diff is how the names of functions for traversing sqlite-stored graph vs in-memory graph are similar, so it's a bit unclear when we're querying sqlite vs when we're pulling from in-memory state (maybe a footgun for future editors?)
There was a problem hiding this comment.
Not really a simplification because we need the depth during resume while the other is just a BFS
I will try to unify in a follow-up
2 participants
Add a representation of the agent graph. This is now used for:
Later, this will also be used for post-compaction stuffing of the context
Direct fix for: #14458