[thread store] separate thread names and repair ownership#29352
[thread store] separate thread names and repair ownership#29352anaiskillian wants to merge 11 commits into
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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 |
![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: 6cfeadd4f0
ℹ️ 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: f02d4d3cb7
ℹ️ 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".
anaiskillian
force-pushed
the
anais/thread-storage-metadata-foundation
branch
from
11cf96d to
7ddcf58
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ddcf58cd9
ℹ️ 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".
anaiskillian
force-pushed
the
anais/thread-storage-metadata-foundation
branch
from
3c3cc4b to
2d7edbc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d7edbc5b9
ℹ️ 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".
jif-oai
left a comment
jif-oai
left a comment
There was a problem hiding this comment.
The proposed name model still relies on NULL/empty-string sentinels and title-equality heuristics... so it cannot reconcile legacy writers or imported-title provenance reliably. This sounds not very strong to me
|
@jif-oai I reworked the name model in 639ee6b to address the provenance concern at the storage boundary. SQLite now stores an explicit name state plus a title snapshot: new writers represent unnamed and explicit names without NULL or empty-string sentinels, and title-only writes from older binaries are detected from snapshot drift before reconciliation, including when a prior explicit name exists. External imports now persist only actual source titles as explicit names; first-user fallback text remains preview and derived-title metadata. The affected state, rollout, thread-store, and external-session suites passed 329 tests, and the app-server session-import integration test passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 639ee6ba24
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 463f8fdc23
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3445438d98
ℹ️ 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: 24d662c41f
ℹ️ 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: 2eada6a0f2
ℹ️ 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".
| SET name = title, | ||
| name_state = 'explicit', | ||
| title_snapshot = title |
There was a problem hiding this comment.
For pre-0040 rows whose title was a user-assigned name, this migration copies it into name but leaves the same value in title/title_snapshot. The incremental rollout path starts from existing metadata and only fills title when it is empty, so the next write records that old name as the derived title; if the user later clears the explicit name, exact-title resume/search can still match the old name. In the migration, move the history-derived value into title (for example from first_user_message) or clear title/title_snapshot for rows promoted to explicit names.
AGENTS.md reference: AGENTS.md:L103-L111
Useful? React with 👍 / 👎.
| UPDATE threads | ||
| SET title_snapshot = NEW.title | ||
| WHERE id = NEW.id; |
There was a problem hiding this comment.
When an older Codex binary inserts a thread after this migration, it does not know about name, so the new columns take their defaults. This trigger only fills title_snapshot, leaving name_state='legacy_unknown'; the next new-binary upsert sees title == title_snapshot and converts the row to unnamed, dropping any user-assigned legacy title written by that older binary. The recency migration already supports this mixed-version flow, so this trigger should also classify distinct legacy titles into name/name_state.
AGENTS.md reference: AGENTS.md:L103-L111
Useful? React with 👍 / 👎.
| match name { | ||
| ThreadMetadataName::Explicit(name) => thread.name = Some(name), | ||
| ThreadMetadataName::Legacy(name) => set_thread_name_from_title(thread, name), | ||
| ThreadMetadataName::Cleared => {} |
There was a problem hiding this comment.
When a thread has an old session_index name and SQLite now records ThreadName::Cleared, read_thread_by_rollout_path first loads the legacy name from the rollout/index and then calls this helper. The Cleared arm is a no-op, so direct reads by rollout path can still return the stale name after it was explicitly cleared; this arm should reset thread.name to None to make cleared state override legacy fallback data.
Useful? React with 👍 / 👎.
2 participants
This pr separates explicit thread names from history-derived titles in SQLite and adds a lightweight list projection with canonical parent metadata. It supersedes the storage and indexing portion of [app-server] improve thread list and resume rpc paths #28801 while preserving existing RPC routing.
Rollout read-repair now updates only location-owned fields, preventing stale full-row snapshots from overwriting concurrent metadata changes. Existing state, rollout, and thread-store tests pass with the new migration and ownership contract.