[codex] Inject agent graph store into ThreadManager - #29736
Conversation
bdd89e8 to
cd3e0fc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16a3d504ab
ℹ️ 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".
rasmusrygaard
left a comment
There was a problem hiding this comment.
Comments about interop with in memory and ephemeral threads seem plausible to me but the rest looks good
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a73b2493e9
ℹ️ 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".
| return Ok(resumed_thread_id); | ||
| }; | ||
| let Some(state_db_ctx) = resumed_thread.state_db() else { | ||
| let Some(agent_graph_store) = state.agent_graph_store() else { |
There was a problem hiding this comment.
We don't store nicknames role etc. Find by me but this means this is only MAv2
There was a problem hiding this comment.
Discussed live but: the nicknames / role / etc are stored in thread metadata rather than in the agent graph store. This PR doesn't change that. I did add a change to load those values through ThreadStore interface rather than directly from Sqlite, however.
56a537b to
e38f36b
Compare
Pick up the AgentGraphStore migration.
ThreadManagerLocalAgentGraphStorewhen SQLite is availableThis required some changes to the interface to deal with futures:
ThreadStore's object-safe pattern by returning a boxedAgentGraphStoreFuturedirectly, allowingThreadManagerto holdArc<dyn AgentGraphStore>Slight behavior change! Unfiltered subtree enumeration now performs a single all-status breadth-first traversal, so a closed grandchild beneath an open edge is included; the previous Open-then-Closed traversals could not cross mixed-status paths and silently omitted it.