fix(history): keep every Pi session in Earlier sessions when label scans hit the deadline by devswha · Pull Request #2410 · devswha/ovis

devswha · 2026-10-29T23:57:54Z

Found by the flake-hunt lane while it fixed the #2377 timing flakes (#2402); Dori decided to fix it in its own PR. On a slow PC, Earlier sessions for Pi could become an error that never cleared.

The bug (MEDIUM, user-visible)

@IdrisGit's streaming reader (#2377) reads each Pi transcript in full, so native names and first prompts show even when they sit deep in the file. A listing has one shared two-second deadline. By design, a scan that cannot finish returned a retryable error, never incomplete labels:

On a deadline: the API answered 503 session_history_busy. The client retried up to six times, then emptied the list and showed "Pi sessions could not be read right now. Try again." with a Retry button.
Finished files: their labels were cached, so retries usually got further.
Partial scans: these were discarded. One transcript whose scan alone takes more than two seconds on a slow disk never finished, so Earlier sessions showed the error on every open.

Why "never incomplete labels" is relaxed

#2377's rule kept a half-read file from showing a wrong label. What it actually cost was the whole list. This PR keeps the part that matters and drops the part that hurt:

A label from a torn or unfinished read is still never shown.
A session whose label is not known yet is listed with its session ID, the same fallback a session with no name has always had.
Its label fills in on a later open, because the scan resumes where it stopped.

So the list can be incomplete in its labels, but never wrong, and never empty because one file is slow.

Design

Credit to @IdrisGit: #2377's bounded streaming reader is kept whole. Its 64 KiB chunks, checked descriptors, stat-keyed cache, EOF validation and queue limits all stay. This PR changes what a deadline does and adds resumable progress:

Rows before labels: a header pass establishes the newest rows (up to 50) before any label budget is spent. Completed cached labels show at once; unfinished ones use the session-ID fallback. The header pass has its own 10-second deadline and reads at most one 64 KiB chunk per candidate per open. If it expires, the rows already checked are kept. Only when no header at all could be checked does it still return the retryable 503, rather than a false empty list.
Resumable progress: parser progress for long headers and transcripts is kept per file, keyed by its stat identity. Any change, append, truncation, rewrite or replacement discards it. A huge transcript therefore gets its label after a few opens. Headers from other folders are refused before caching, so they cannot evict that progress.
No whole-list 503 for one file: a label-pass error keeps the rows and withholds only that file's label. The errors are a file changing mid-scan (a running Pi agent appending), a read failure, and deletion or replacement. 503 remains only for a full queue and a stopped reader.
Windows: folder eligibility is checked once, in the header pass.
Small files are finished during the header pass, at no extra I/O.

Proof

Fail-first: the original five deadline regressions failed on origin/main (0 passed, 5 failed, PiMetadataBusy). Each review finding was reproduced as a test before its fix:

Case	Before	After
Expiry before any header / older rows hidden (round 1)	failed	passed
Long header, malformed or incomplete preamble	3 failed	3 passed
Foreign-header cache churn evicting progress	1 failed	1 passed
Windows identity checked twice	1 failed	1 passed
Label append, read error, deletion, replacement	4 failed	4 passed
Active or queued header past its deadline	2 failed	2 passed
257 headers over 64 KiB churning the cache (round 3)	1 failed	1 passed

Tests: the Pi reader suite passed 65, the reader and history suites together 159, and the isolated history API cases 7. bun run check fast passed (8,810 unit tests).
Browser, before and after: a fictional bridge drove the real components, with one transcript whose scan exceeds an injected deadline, at desktop (1440) and phone (390) widths.
- Before (origin/main): only "Pi sessions could not be read right now. Try again." and Retry.
- After: every row, with the slow one shown by its ID.
- After a reopen: every row labelled.

Browser, before and after (one transcript's scan exceeds an injected deadline)

	Desktop (1440)	Phone (390)
Before (origin/main)
After: every row, the slow one shown by its ID
After a reopen: labels filled in

Not run locally: the full Integration and native Windows lanes (CI runs them). The Windows alias case is covered deterministically.

Reviews

This PR changes server code and the history API's behaviour, so both reviewers ran, over three rounds.

omo-native-code-reviewer: APPROVE (WATCH) in round 3.

Round 1 BLOCKed on an expiry before any header was checked returning an empty 200 (HIGH), and on rows older than the expired file being dropped (MEDIUM). Both were fixed by the header pass.
Round 2 BLOCKed on non-deadline label-pass errors (an append, a read failure, a deleted file) still turning the list into a 503. Fixed: those rows keep null or cached labels.
Round 3 confirmed every earlier finding fixed, and that the late safety correction is right: a swapped symlink or a changed identity is "no row", not an unchecked header. LOWs, kept:
- If every file in a store fails to read, each open answers 503 rather than a false empty list.
- An append during the one-chunk header read can hide that row for one open.
- A promise-ordering race in one test is deterministic but order-sensitive.
- A file whose first line is a multi-MiB malformed record takes several opens to recover.
Round 4, the delta that fixes the cache churn: APPROVE (WATCH). LOWs, kept:
- The 256-entry cache is shared across folders, so three or more folders, each with 50+ headers over 64 KiB, opened in rotation could evict each other's progress. Pi does not write headers that large.
- When the unfinished-header cap stops a pass, older complete sessions are missing from that one response; they fill in on later opens.

Codex (Sol 6.1 in round 1, Terra 6.1 from round 2, per the owner's switch).

Round 1 reproduced the hidden-older-rows gap. Fixed.
Round 2 raised three HIGHs and a MEDIUM; each HIGH was reproduced as a failing test first, then fixed:
- a long header causing a 503 loop;
- foreign-header cache churn evicting progress;
- the Windows identity being checked twice.
- MEDIUM: the header pass had no time bound. It now has a 10 s deadline.
Round 3 confirmed those fixed and raised three more HIGHs:
- Cache churn across 257 files that each have a header over 64 KiB (a permanent 503). Reproduced (0 passed, 1 failed on 98df12ca) and fixed in 606cf7b2: each header traversal now stops after 50 unfinished headers. That is independent of the 50-row limit and well below the 256-entry cache, so one traversal never evicts the newest checkpoints before the next open advances them. After the fix the test passes (1/1), the reader suite passes 61, and check fast passes (8,806 unit tests).
- A header over 64 KiB is missing from the first response. Kept, on purpose. Real Pi headers are a few hundred bytes. Such a file is listed on a later open, and older rows show meanwhile.
- 503 when no header at all could be checked (for example EIO on the only file). Kept, on purpose. Without one checked header there is no trustworthy session identity to list. A 200 with no rows would claim "no earlier sessions", which is the false answer round 1 removed. The 503 makes the client retry.
Round 4, on the delta, found the same eviction class through headerless files: empty .jsonl files cached as null but not counted toward the cap. Fixed in 5405fbbc: one traversal touches at most 200 files that can take a cache entry, below the 256 slots. A 344-file repro failed before and passes after.
- Its MEDIUM is kept as a known limit: 50 transcripts that are all being appended to, each with a header over 64 KiB, ahead of an older session keep that session out until they settle. Real Pi headers are a few hundred bytes.
Round 5, on 5405fbbc: both reviewers found that 200 empty .jsonl files newer than a real session made every listing return an empty array. Fixed in 90da040d: completed headerless results are not cached and do not count. The cap is derived from the exported PI_CACHE_ENTRIES (3/4 = 192). A cutoff with no rows gives the retryable 503, never an empty 200. Repros: 200 and 201 empty files, 0/2 before, 2/2 after. Cache hits still count, because counting only insertions would let a hit promote an old entry past new checkpoints; omo agreed.
Round 6, on 90da040d: omo APPROVE. Terra's last MEDIUM, the same case as omo's L13: a headerless file over 64 KiB restarted from byte 0 on every open, delaying older sessions. Fixed in 69415701: a completed empty result is cached only for files larger than one chunk, keyed by stat identity, and counts like any cached entry. Terra's 150-file repro failed before (0/1) and passes after (1/1); later unchanged opens read zero bytes.

Follow-up, outside this PR: Terra noted that origin/main's existing 400-check discovery bound can still produce an empty list, if enough newer empty files fill the budget before a real session is discovered. That limit predates this PR.

Round 7, on 69415701: omo APPROVE (WATCH). Both reviewers named the same boundary, Terra as a MEDIUM and omo as L14: 192 or more files over 64 KiB with no session header, newer than every real session in one folder, keep the list on a retryable 503. This is shipped as a documented limit, the same kind as the cache and discovery bounds.
- Any fixed per-open budget can be filled by enough junk files. Each review round moved this threshold further out, and none removed it.
- Pi writes no such files.
- origin/main's existing 400-check discovery bound has the same shape.
- The list never shows a false "no sessions" from this PR's paths; a cutoff with no rows answers the retryable 503.

Known limits (documented in docs/development.md)

Labels for files beyond the per-open budgets fill in over later opens.
192 or more headerless files over 64 KiB ahead of every real session keep a retryable 503, as above.
50 transcripts that are all being appended to, each with a header over 64 KiB, delay older sessions until they settle.
Several folders, each with many oversized headers, opened in rotation can evict each other's progress (the cache is shared).

The shared listing deadline discarded unfinished scans and rejected the entire list. A slow transcript could never finish, and stopping at that file also hid older sessions, even when their labels were already cached. Establish up to 50 newest-first rows with stat-checked header reads bounded to one 64 KiB chunk per candidate and a shared ten-second header deadline. Keep long-header progress instead of permanently refusing it. Reject ineligible headers before caching and reuse checked Windows eligibility. Spend the remaining shared two-second label deadline on labels, retaining bounded parser checkpoints under the existing stat key. Keep completed cached labels and return null unfinished labels; later opens resume stable files. Preserve queue limits and EOF torn-read validation. Relax #2377's "never incomplete labels" policy because optional labels must not imply that sessions are absent. Established rows survive label read errors, changes and deletion without publishing torn labels. Queue saturation, shutdown and unavailable headers with no checked rows retain the compatible retryable 503 response. Keep #2402's reader/server deadline injection for the long-store tests. Fail-first: five regressions against rebased origin/main 7a6f8b0 produced 0 pass, 5 fail with PiMetadataBusy. The earlier staged implementation also failed queued/first-chunk/all-older-row cases (0 pass, 3 fail), and the cached older-row case (0 pass, 1 fail). Round-two HIGH reproductions failed 0/9 before the fixes; both header timeout cases also failed. All 11 now pass. The complete fixed reader suite passes all 60 tests. The combined reader/history suites pass 159 tests, including explicit no-row refusal for swapped symlinks and mismatched descriptor identities. Browser QA uses the real components and an isolated fictional bridge with injected read time: origin/main shows an error after seven requests; the fix shows every row with fallback IDs and fills labels on reopening. Desktop and phone captures are retained locally under .omo/run/. Final validation: isolated history API cases 7 pass; check fast exit 0, source unchanged and verified, including typecheck, build and 8805 passing unit tests with 35 platform skips across 557 files.

A newest-first traversal of 257 headers larger than 64 KiB churned the 256-entry cache. Each open evicted the next checkpoint before reaching it, so even the second open returned busy instead of completing a header. Stop the header traversal after 50 unfinished headers, independently of the 50-row limit. This keeps one traversal well below cache capacity and lets the next open complete the newest headers. Completed ineligible headers do not consume this progress budget. Preserve the accepted behavior for an unfinished header on the first response and for busy when no header could be checked. Fail-first: a frozen-clock fixture with 257 valid 70 KiB headers failed on 98df12ca (0 pass, 1 fail: PiMetadataBusy on the second open). With this fix, the same test passes and returns the newest 50 labeled rows on that open, without rereading their first chunks. The full reader suite passes 61 tests. Validation: isolated history API cases 7 pass. Required check fast exits 0 with unchanged-source verification: typecheck, build and 8806 unit tests pass, with 35 skips across 557 files.

Limiting unfinished headers alone missed completed headerless files cached as null. With six empty files between each of 50 long headers, a traversal touched 344 paths and churned the 256-entry cache before any header could complete on reopening. Bound a traversal to 200 potentially cached file touches, including null and future cached results. Label scans revisit only that set, so one traversal cannot evict its own checkpoints. Only explicitly rejected headers are exempt because they never enter the cache; discovery still bounds their I/O and the 260-foreign-header regression keeps working. Document the invariant and the accepted low-likelihood starvation limit for actively growing oversized headers. Fail-first on 606cf7b2: the deterministic 344-file reproduction gave 0 pass, 1 fail (PiMetadataBusy on the second open). After the fix, the same test gives 1 pass, 0 fail and completes the 29 long headers among the first 200 cache-bearing files without rereading their prefixes. The full Pi reader suite passes 62 tests. Validation: isolated history API cases 7 pass. Required check fast exits 0 with unchanged-source verification: 8807 unit tests pass, 0 fail, 35 skip across 557 files; typecheck, build and generated-file checks also pass.

Caching completed null results made 200 static empty JSONL files consume every cache-bearing touch on each open, returning a false empty list even though a real older session remained in discovery. Do not retain completed headerless results, and make their null returns free in the cache-touch budget. Keep counting all retained entry touches, including hits: insertion-only counting is unsafe because hits can promote old entries past newly inserted checkpoints before the next eviction. If a cutoff leaves discovery unfinished and no row, return busy rather than an empty success. Export PI_CACHE_ENTRIES and derive the touch cap as three quarters of it, so the no-self-eviction bound cannot silently exceed the cache capacity. Fail-first on 5405fbbc: both 200- and 201-empty-file regressions returned empty arrays (0 pass, 2 fail). Both now list the older real session on the first and second opens (2 pass, 0 fail), caching only that real session. The interleaved-empty-file regression now recovers all 50 long headers on the second open. The full Pi reader suite passes 64 tests. Validation: isolated history API cases 7 pass. Required check fast exits 0 with unchanged-source verification: 8809 unit tests pass, 0 fail, 35 skip across 557 files; typecheck, build and generated-file checks also pass.

Discarding every completed headerless result restarted unchanged files larger than 64 KiB on later opens. They repeatedly consumed unfinished header slots and delayed an older valid Pi session. Cache the stat-validated empty result only when the file needed more than one chunk. Notify the listing for these retained null results, including cache hits, so they consume the same traversal budget as other entries. Single-chunk headerless files remain uncached and free. Fail-first on 90da040d: 150 unchanged 65537-byte whitespace files ahead of a valid session still returned PiMetadataBusy on the fourth open (0 pass, 1 fail). The same regression now passes: the fourth open lists the real session, subsequent opens read zero transcript bytes, and appending a valid session invalidates the completed-empty cache entry correctly. The full reader suite passes 65 tests, including the 200- and 201-empty single-chunk cases. The pre-existing 400-check discovery bound is unchanged. Validation: isolated history API cases 7 pass. Required check fast exits 0 with unchanged-source verification: 8810 unit tests pass, 0 fail, 35 skip across 557 files; typecheck, build and generated-file checks also pass.

coderabbitai · 2026-10-29T23:58:34Z

📝 Summary

Summary by CodeRabbit

Bug Fixes
- Pi session history now shows available sessions even when older transcript details are slow to load. Sessions without completed labels display their session ID, and later listings can fill in the labels as scanning continues.
- If a listing cannot check any sessions before timing out or the metadata queue is busy, it returns a retryable busy response instead of partial results.

Pi session history now uses bounded, resumable header and label scans. Listings can retain established rows when label reads are incomplete, and later listings can resume scans for unchanged files. The changes add reader and API tests and update the development documentation.

Changes: Pi session history

Layer / File(s)	Summary
Bounded resumable metadata scans `server/pi-session-label.ts`, `server/pi-session-metadata.ts`, `server/pi-session-metadata.test.ts`, `docs/development.md`	The reader caches bounded parser progress and validates it against file identity. Header and label reads use separate deadlines. Tests cover resumption, cache limits, deadlines, and file changes.
Header selection and partial listings `server/session-history.ts`, `server/api.contract.test.ts`, `shared/session-history.ts`, `docs/development.md`, `changelog.d/pi-history-partial-labels.md`	Pi history selects rows in a bounded header pass before reading labels. Established rows remain in the response when label reads fail or expire; if no rows are established and the header pass is incomplete, the API returns a busy response.

Priority: Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Possibly related PRs

devswha/ovis#2377: Adds the Pi session-label reader and the earlier all-or-error listing behavior that this change extends.
devswha/ovis#2402: Adds the configurable deadline for the Pi listing path updated here.

Suggested reviewers: idrisgit

Merge Risk: Minimal · up to c9c90

Earlier sessions can retain available Pi sessions when scans are incomplete. No identified issue prevents merging after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the main change: preserving Pi session rows when label scans exceed the deadline.
Description check		The description explains the bug, resulting behavior, design, validation results, contract-test coverage, browser checks, skipped test lanes, and known limits. It is substantially complete despite not…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the bug, resulting behavior, design, validation results, contract-test coverage, browser checks, skipped test lanes, and known limits. It is substantially complete despite not using the exact template headings.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 6 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

devswha · 2026-10-30T00:11:45Z

Merging c9c90d8. Pi 'Earlier sessions' is no longer emptied by the 2 s listing deadline: every session gets a row (its ID until its label is read), labels fill in on later opens and the scan resumes where it stopped; 503 session_history_busy is kept for a full queue, a stopped reader or nothing checkable, which the client already retries. Builds on @IdrisGit's reader from #2377. Reviewed by the merge pass after the lane's seven rounds: no HIGH; the client's retry and the ID fallback stay compatible with older clients; 170 reader/history tests, 7 API contract tests and typecheck pass. One MEDIUM, reproduced with an injected 2.1 s header check, goes to follow-up #2411: when checking file headers alone uses up every listing's deadline, label reading never advances (rows stay, labels stay as IDs) (server/session-history.ts:509-510,553). Known limit, accepted: 192+ header-less files over 64 KiB newer than every session keep a retryable 503 (Pi never writes such files; main has the same bounded-discovery shape).

devswha added 5 commits October 30, 2026 08:55

devswha mentioned this pull request Oct 30, 2026

Pi Earlier sessions: labels never fill when header checks use up every listing deadline #2411

Open

devswha merged commit 393d135 into main Oct 30, 2026
19 checks passed

devswha deleted the fix/pi-history-partial-labels branch October 30, 2026 00:11

Navigation Menu

Repository navigation

fix(history): keep every Pi session in Earlier sessions when label scans hit the deadline - #2410

fix(history): keep every Pi session in Earlier sessions when label scans hit the deadline#2410
devswha merged 5 commits into
mainfrom
fix/pi-history-partial-labels

devswha commented Oct 29, 2026

Uh oh!

coderabbitai Bot commented Oct 29, 2026 •

edited

Loading

Summary by CodeRabbit

Uh oh!

devswha commented Oct 30, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

devswha commented Oct 29, 2026

The bug (MEDIUM, user-visible)

Why "never incomplete labels" is relaxed

Design

Proof

Browser, before and after (one transcript's scan exceeds an injected deadline)

Reviews

Known limits (documented in docs/development.md)

Uh oh!

coderabbitai Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

devswha commented Oct 30, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Oct 29, 2026 •

edited

Loading