mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Fix filtered thread-list resume regression in TUI (#19591)
## Why `codex resume` regressed after [#18502](https://github.com/openai/codex/pull/18502) changed the default `thread/list` scan-and-repair path for metadata-filtered listings. The TUI resume picker uses `thread/list` with source/provider/cwd filters and `useStateDbOnly: false`, which is the intended correctness-preserving mode: it should still consult the filesystem so healthy, missing, or stale SQLite state can be repaired. The regression was that #18502 made that filtered, filesystem-backed path call `reconcile_rollout` for every filesystem hit, and then call it again for each SQLite hit. When `reconcile_rollout` does not already have extracted rollout items, it falls back to loading the full JSONL rollout. That changed the resume picker’s first page from a cheap rollout-head scan plus SQLite read-repair into full-file reads for large sessions, so a few long threads could dominate TUI startup/resume latency. This change addresses the regression by keeping `useStateDbOnly: false` on the correctness-preserving path while avoiding unnecessary full JSONL reads for rows the filesystem scan has already validated. Source/provider/cwd filters can be decided from rollout-head metadata, so non-search resume listings only need the lightweight read-repair path for filesystem hits. Full reconciliation is still used for DB-only filtered rows because those can be stale false positives, and for search listings because search can depend on title metadata that may require scanning the full rollout. This fixes #19483. ## What changed - For non-search filtered listings, repair filesystem hits with the lightweight `read_repair_rollout_path` path instead of full `reconcile_rollout`. - Track thread IDs proven by the filesystem scan and only fully reconcile SQLite-filtered hits that the filesystem scan did not return, preserving stale-DB false-positive cleanup without full-reading every healthy rollout. - Leave search listings on full reconciliation, since search depends on full title metadata rather than only source/provider/cwd metadata from the rollout head. ## Verification - `cargo test -p codex-rollout list_threads` - `cargo test -p codex-app-server thread_list`
This commit is contained in:
committed by
GitHub
Unverified
parent
277186ec85
commit
850f035b8c
@@ -444,9 +444,19 @@ impl RolloutRecorder {
|
||||
));
|
||||
}
|
||||
|
||||
// Warm the DB by repairing every filesystem hit before querying SQLite.
|
||||
// For metadata-filtered listings the filesystem page is the page we return. Track those
|
||||
// IDs so the later DB page only triggers full reconciliation for DB-only hits.
|
||||
let fs_page_thread_ids = fs_page
|
||||
.items
|
||||
.iter()
|
||||
.filter_map(|item| item.thread_id)
|
||||
.collect::<HashSet<_>>();
|
||||
|
||||
// Warm the DB by repairing every filesystem hit before querying SQLite. Source/provider/cwd
|
||||
// filters are already validated from rollout head metadata, so lightweight read-repair is
|
||||
// enough there. Search can depend on full title metadata, so keep full reconciliation.
|
||||
for item in &fs_page.items {
|
||||
if listing_has_metadata_filters {
|
||||
if search_term.is_some() {
|
||||
state_db::reconcile_rollout(
|
||||
state_db_ctx.as_deref(),
|
||||
item.path.as_path(),
|
||||
@@ -517,6 +527,12 @@ impl RolloutRecorder {
|
||||
}
|
||||
if listing_has_metadata_filters {
|
||||
for item in &db_page.items {
|
||||
// Rows that also appeared in the filesystem page were just validated from the
|
||||
// rollout head. Rows only found by SQLite may be stale filter matches, so fully
|
||||
// reconcile those before returning the filesystem-backed page.
|
||||
if fs_page_thread_ids.contains(&item.id) {
|
||||
continue;
|
||||
}
|
||||
state_db::reconcile_rollout(
|
||||
state_db_ctx.as_deref(),
|
||||
item.rollout_path.as_path(),
|
||||
|
||||
Reference in New Issue
Block a user