fix(spec): store the review transcript's events, not its streaming frames (BUG-083, #995) - #999
Conversation
…ames
The transcript exists so that HOW a review reasoned is auditable and not only
its conclusion. At 527 MB it delivered the opposite: nothing a human opens and
nothing a script greps cheaply.
Measured on the HARNESS-072 review, a real 25-minute run:
527.2 MB total
525.2 MB of it `message_update`
43391 lines
`message_update` is the same message re-emitted at every growing length, so the
file was mostly one text written hundreds of times. The reviewer runner has no
non-streaming mode, so the fix is downstream.
The two consumers of that stream want different things and cannot share a file.
The tmux pane, watched live, wants every frame — the deltas ARE the visible
progress. The transcript on disk wants settled events. So `transcript-sink`
replaces `tee` in the pipeline: everything passes through to the pane, only
events are stored.
527.2 MB -> 1.95 MB (0.37%), 664 lines, 4.8s
The audit property survives intact, which is the point: all 70 of the reviewer's
tool executions remain recoverable from the filtered file, including the
`git rev-parse HEAD` reach probe that #978 made the bar for a real review.
Two deliberate conservatisms. A line the filter cannot parse is KEPT — dropping
the unrecognised is how a diagnostic disappears exactly when it matters, and
this filter must never be why a failure left no trace. And the sink is resolved
via os.Executable() rather than `dotf` on PATH, so a detached pipeline cannot
end up in a binary that lacks the subcommand; if that resolution fails, TmuxWrap
falls back to plain `tee`, because an oversized transcript beats a broken
pipeline.
`f.Close` and `w.Flush` are checked rather than deferred blindly: on a buffered
file that is where a short write surfaces, and swallowing it would report a
complete transcript we did not finish writing — the same class of lie.
Closes #995
mlorentedev
added
the
skip-sdd
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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. Comment |
1 participant
Closes #995.
The transcript exists so that how a review reasoned is auditable, not only its conclusion (HARNESS-071). At 527 MB it delivered the opposite: nothing a human opens, nothing a script greps cheaply. The artifact existed and the property did not.
Measured, on the HARNESS-072 review — a real 25-minute run
message_updateis the same message re-emitted at every growing length, so the file was one text written hundreds of times.pihas no non-streaming mode (--modeoffers text/json/rpc only), so the fix is downstream.The shape of the fix
The two consumers of that stream want opposite things and cannot share a file:
So
spec transcript-sinkreplacesteein the pipeline: everything passes through to the pane, only events are stored.The audit property survives intact, which is the whole point. All 70 of the reviewer's tool executions remain recoverable from the filtered file:
{"command": "git rev-parse HEAD"}— including the reach probe that #978 made the bar for distinguishing a real review from a blind one.
Two deliberate conservatisms
An unparseable line is kept. Dropping the unrecognised is how a diagnostic disappears exactly when it matters; this filter must never be the reason a failure left no trace. Tested.
The sink resolves via
os.Executable(), notdotfon PATH. The pipeline runs detached under tmux, and a launcher built from one version handing work to whateverdotfa$PATHresolves is how a subcommand that exists here goes missing there. If resolution fails,TmuxWrapfalls back to plaintee— an oversized transcript beats a broken pipeline. Tested both ways.Evidence
go build·go vet·go test ./...ok ·golangci-lintpinned 2.12.2 — 0 issues.errcheckcaughtf.Close/w.Flushgoing unchecked; fixed rather than silenced. On a buffered file that is where a short write surfaces, and swallowing it would report a complete transcript we did not finish writing — the same class of lie this PR is about.SDD skip rationale
Bug fix with a measured cause, reproduced on real data before and after. No behaviour change to any user-facing command:
transcript-sinkis hidden plumbing thatspec reviewpipes into, and the only observable difference is that the transcript it writes is 0.37% of its former size with the same auditable content. Opening a spec folder would also pull this into the archive-on-merge chain, which is the very pipeline being repaired.