Add thread recencyAt for sidebar ordering#27910
Conversation
…-recency-at # Conflicts: # codex-rs/rollout/src/state_db.rs
jif-oai
left a comment
jif-oai
left a comment
There was a problem hiding this comment.
Pre-approving but the backfill issue needs to be fixed before merging
![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: 336a28faaf
ℹ️ 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".
| ThreadSortKey::RecencyAt => item | ||
| .item | ||
| .recency_at | ||
| .as_deref() | ||
| .or(item.item.updated_at.as_deref()) |
There was a problem hiding this comment.
When search results are sorted by recency and the page boundary falls inside a group of threads sharing the same recency_at, this cursor only encodes the timestamp. The next keyset query cannot resume after the specific thread, so it advances past the rest of that timestamp bucket and silently drops matches from later pages.
Useful? React with 👍 / 👎.
| let ThreadResumeResponse { thread, .. } = to_response::<ThreadResumeResponse>(resume_resp)?; | ||
|
|
||
| assert_eq!(thread.updated_at, before_resume.updated_at); | ||
| assert_eq!(thread.recency_at, before_resume.recency_at); |
There was a problem hiding this comment.
This asserts resume leaves recency_at unchanged but never that send_turn_start_request advances it. For this agent-visible recency change, add integration coverage for TurnStarted so that path cannot regress while this resume-only assertion passes. guidance
Useful? React with 👍 / 👎.
| Ok(result.rows_affected() > 0) | ||
| } | ||
|
|
||
| pub async fn touch_thread_recency_at( |
There was a problem hiding this comment.
This recencyAt rollout is non-mechanical but spans 66 files and 1,177 changed lines across migration, state runtime, thread-store, app-server protocol/schemas/tests, and TUI. Please split it into a smaller coherent stage so each behavior can be reviewed and tested. guidance
Useful? React with 👍 / 👎.
| #[ts(type = "number")] | ||
| pub updated_at: i64, | ||
| /// Unix timestamp (in seconds) used for thread recency ordering. | ||
| #[ts(optional, type = "number")] |
There was a problem hiding this comment.
When recency_at is None, serde emits "recencyAt": null, but #[ts(optional, type = "number")] makes clients expect omission/number instead of required nullable. Strict consumers can reject valid Thread payloads; make it required nullable or align artifacts per v2 guidance
Useful? React with 👍 / 👎.
| .or_else(|| thread_id_from_rollout_path(item.path.as_path()))?; | ||
| let created_at = parse_rfc3339(item.created_at.as_deref()).unwrap_or_else(Utc::now); | ||
| let updated_at = parse_rfc3339(item.updated_at.as_deref()).unwrap_or(created_at); | ||
| let recency_at = parse_rfc3339(item.recency_at.as_deref()).unwrap_or(updated_at); |
There was a problem hiding this comment.
When a thread is read by rollout path, ThreadItem.recency_at comes from the rollout/file mtime fallback, and read_thread_by_rollout_path only overlays Git fields from SQLite. After a turn start followed by background output, path-based resume/read responses report recencyAt as the later updatedAt, so the new recency ordering signal is wrong for those callers.
Useful? React with 👍 / 👎.
2 participants
Summary
Add a server-owned
recencyAttimestamp andrecency_atthread-list sort key for product recency ordering while preserving the existing meaning ofupdatedAtas the latest persisted thread mutation.This is the server-side alternative to #27697. Rather than narrowing
updatedAt, clients can sort the sidebar byrecency_atand continue treatingupdatedAtas mutation time.Paired Codex Apps PR: openai/openai#1024599
Contract
recencyAtinitializes when a thread is created.recencyAtmonotonically.updatedAtretains its existing behavior and continues to advance for persisted thread mutations.recencyAt; the response field is optional in generated TypeScript so clients connected to older servers can fall back toupdatedAt.Persistence and compatibility
Migration 0038 adds second- and millisecond-precision recency columns, backfills them from the existing updated timestamp, creates list indexes, and includes an insert trigger so older binaries writing to a migrated database seed recency without causing later mutations to advance it.
Generic metadata upserts preserve existing recency values. Turn-start updates use a dedicated monotonic touch, and process-local allocation keeps millisecond cursor values unique. State DB list, search, read, filtered-list repair, rollout fallback propagation, and app-server conversions all carry the new field.
API
Threadresponses include:recencyAt?: numberthread/listandthread/searchaccept:{ "sortKey": "recency_at" }Generated TypeScript and JSON schemas are included.
Validation
just test -p codex-state— 146 passedjust test -p codex-rollout— 69 passedjust test -p codex-thread-store— 81 passedjust test -p codex-app-server-protocol— 231 passedjust fixfor state, rollout, thread-store, app-server-protocol, and app-serverjust fmtgit diff --check