store and expose parent_thread_id on Threads#25113
Conversation
79f0199 to
5019c4d
Compare
5e9ee64 to
21583d8
Compare
jif-oai
left a comment
There was a problem hiding this comment.
lgtm if you're sure it does not break anything in the app UX for multi-agent (I guess not but double check please)
| })?; | ||
| if let Ok(meta_line) = read_session_meta_line(path.as_path()).await { | ||
| thread.forked_from_id = meta_line.meta.forked_from_id; | ||
| thread.parent_thread_id = meta_line.meta.parent_thread_id; |
There was a problem hiding this comment.
Just for the record: this is not backward compatible
But it's good for now
bcad0d2 to
903dcac
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 768000f484
ℹ️ 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".
| @@ -1690,6 +1699,7 @@ fn thread_item_from_state_metadata(item: codex_state::ThreadMetadata) -> ThreadI | |||
| .or_else(|_| serde_json::from_value(Value::String(item.source))) | |||
| .unwrap_or(SessionSource::Unknown), | |||
| ), | |||
| parent_thread_id: None, | |||
There was a problem hiding this comment.
Preserve parent IDs when listing from SQLite
When SQLite is available, the normal unfiltered thread list path returns db_page.into(), which maps every codex_state::ThreadMetadata through this newly added field. Because state metadata is not extended to store parent_thread_id, this hard-coded None makes app-server thread/list report parentThreadId: null for subagent threads even though the rollout SessionMeta has the parent ID; this defeats the new API field for the common DB-backed listing path.
Useful? React with 👍 / 👎.
f42275f to
b90f37f
Compare
Why
This PR #24161 (comment) revealed a subagent data modeling issue, where we overloaded
forked_from_idto also meanparent_thread_id. That's incorrect since guardian and review subagents can be a subagent and NOT fork the main thread's history.The solution here is to explicitly store a new
parent_thread_idonSessionMeta, alongsideforked_from_idwhich already exists. While we're at it, also expose it in the app-server protocol on theThreadobject.A thread->subagent relationship and a fork of thread history are orthogonal concepts.
What Changed
parent_thread_idpersistence onSessionMetaand runtime/session plumbing throughSessionConfiguredEvent,CodexSpawnArgs,SessionConfiguration,ThreadConfigSnapshot,TurnContext, andModelClient.SessionSourceorforked_from_thread_id.InitialHistory.Thread.parentThreadId.parentThreadIdresponse field.