fix(gateway): fill bounded transcript windows across short reads#108242
fix(gateway): fill bounded transcript windows across short reads#108242sunlit-deng wants to merge 1 commit into
Conversation
d1af51a to
fdfb2d8
Compare
|
Codex review: needs maintainer review before merge. Reviewed July 25, 2026, 1:24 PM ET / 17:24 UTC. ClawSweeper reviewWhat this changesThis PR completes bounded gateway transcript reads after positive short reads and adds regression coverage for session titles, previews, and usage snapshots. Merge readinessThe supplied PR evidence describes a focused short-read regression fix with real terminal proof and green checks, but the read-only sandbox failed before I could inspect the exact current-main code, complete PR diff, scoped policies, maintainer notes, or git history. Keep this PR open for a source-backed maintainer review rather than making a cleanup or merge decision from context alone. Priority: P2 Review scores
Verification
How this fits togetherGateway session utilities read bounded JSONL transcript windows to produce session titles, recent-message previews, and usage data. Those summaries feed session-list and diagnostic-facing gateway behavior after the transcript bytes are parsed. flowchart LR
A[Session transcript JSONL] --> B[Bounded positional read]
B --> C[Complete positive short reads]
C --> D[Parse transcript window]
D --> E[Session title and preview]
D --> F[Usage snapshot]
Before merge
Agent review detailsSecurityNone. PR surfaceSource +3, Tests +145. Total +148 across 2 files. View PR surface stats
Review metricsNone. Stored data modelPersistent data-model change detected: Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Review the exact head against current main, confirm every bounded gateway transcript-read path uses the established completion helper and that the new regressions still distinguish the branch from main, then merge only if the focused behavior remains unique. Do we have a high-confidence way to reproduce the issue? Unclear: the PR provides a concrete real-file short-read reproduction and claimed baseline failure, but this review environment could not inspect or execute the current-main reader path. Is this the best way to solve the issue? Unclear: completing positive short reads is the appropriate narrow pattern indicated by the related merged repairs, but the exact gateway callers and current-main implementation still need direct comparison. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 1b2dc1819012. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (5 earlier review cycles)
|
fdfb2d8 to
34ef500
Compare
34ef500 to
24d1316
Compare
a64048c to
293f3c8
Compare
293f3c8 to
e7b0e99
Compare
What Problem This Solves
Fixes an issue where gateway transcript readers silently corrupt or drop data when positional reads return short (a documented POSIX behavior, common on network filesystems). Several bounded window reads in
src/gateway/session-utils.fs.tseither ignore the returned byte count and decode the full buffer — turning the unwritten remainder into NUL garbage inside the JSONL parser — or accept a single partial read and lose the missing bytes: session title previews (readSessionTitleFieldsFromTranscripthead/tail reads), session preview items (readRecentMessagesFromTranscript), the recent usage/cost snapshot (readRecentSessionUsageFromTranscript), and the recent transcript tail lines reader. Users see wrong or empty last-message previews in the session list, missing preview items, and missing cost/usage data.Why This Change Was Made
The file adds two small local helpers,
readFdRangeFullyandreadHandleRangeFully, that loop until the bounded window is filled or EOF (the same idiom asreadLogWindowFullymerged in #105066), and the seven single-shot positional reads in this file now use them and decode onlybytesRead. No exported signature changes; the bounded window sizes are unchanged.User Impact
Session list previews, session preview items, and session cost/usage snapshots stay intact when transcript reads return short, instead of showing empty or corrupted values.
Evidence
node scripts/run-vitest.mjs run src/gateway/session-utils.short-read.test.ts— main: 4 failed; this branch: 4 passed. Sibling suitesession-utils.fs.test.ts: 64 passed.fs.readSyncboundary (each capped call still performs a real kernel read).Baseline (main @ bef86c8):
After (this branch, same script and input):
live-proof.mjs (save in repo root, run with `node --import tsx live-proof.mjs`)
AI-assisted: built with Codex