Skip to content

feat(review): live token counts for claude and codex reviewers - #1666

Merged
peyton-alt merged 5 commits into
mainfrom
feat/review-live-tokens
Jul 13, 2026
Merged

feat(review): live token counts for claude and codex reviewers#1666
peyton-alt merged 5 commits into
mainfrom
feat/review-live-tokens

Conversation

@peyton-alt

@peyton-alt peyton-alt commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

https://entire.io/gh/entireio/cli/trails/788

Salvages the live-token slices from closed #1370 (per its disposition comment: "live-token tailing … wanted separately"), rebuilt against current main's parsers rather than cherry-picked — #1370 was based on the old redesign stack.

What

Reviewer token totals previously appeared only at run completion. This PR makes them move during the run:

  • Claude: assistant stream-json envelopes carry a usage snapshot taken at the start of each turn — input/cache counts are real, but output_tokens is a 1–8 token "initial decision" stub that never updates as text streams. The parser now emits input-only Tokens{In, Out: 0} per usage-carrying assistant envelope (context growth is honest to show; the misleading output stub is not), with the terminal result envelope still delivering the final {In, Out} aggregate.
  • Codex: exec --json stdout only carries usage on turn.completed, and a review is usually a single turn — so a new rollout-transcript tailer (review_tokens.go) locates the session's rollout file by thread.started's thread_id (the same source codex's interactive UI reads for its live counter) and emits cumulative Tokens per token_count event, deduped on movement. The parser stops the tailer and waits for it before closing the event channel, so a tailer send can never race the close. turn.completed now also emits per-turn (multi-turn runs update at each boundary), keeping the old post-loop emission as a defensive backstop.

All consumers already treat Tokens as cumulative-overwrite ("overwrite, don't sum"), which is exactly the contract both sources emit.

Verification

  • TDD throughout — every behavior change had a failing test first; -race clean on the codex package.
  • New wiring test (TestParseCodexOutput_StartsRolloutTailerOnThreadStarted) pins that the parser launches the tailer, receives its Tokens through the real event channel, and shuts it down cleanly on stdout EOF.
  • End-to-end shim smoke: fake claude/codex binaries drove entire review — claude streaming usage-carrying envelopes, codex writing real rollout token_count lines picked up by the tailer — both to clean completion (codex wall time == shim sleep budget, i.e. no shutdown hang).
  • mise run fmt + lint clean, full unit suite green (7625 tests).

File adjacency (benign)

Touches claudecode/reviewer.go and codex/reviewer.go, which open PRs #1647 (child context + fan-out) and #1669 (codex skills — absorbed #1655) also modify — but different functions (they touch buildCmd/prompt composition; this touches the parse*Output paths and adds a new file). Merges are expected to be clean in any order; all three were verified together in the live integration runs.

🤖 Generated with Claude Code


Note

Cursor Bugbot is generating a summary for commit 5860d3a. Configure here.

Salvages the live-token slices from closed #1370 onto current main.
Reviewer token totals previously appeared only at run completion:
claude's parser emitted one Tokens event at the terminal result
envelope, and codex's exec --json stdout only carries usage on
turn.completed (a review is usually a single turn).

Claude: assistant envelopes carry a usage snapshot taken at the start
of each turn — input/cache counts are real but output_tokens is a 1-8
token 'initial decision' stub. Emit input-only Tokens{In, Out: 0} per
usage-carrying assistant envelope so consumers see context growth
during the run; the result envelope still delivers the final {In, Out}
aggregate. Emitting the misleading early output snapshot was rejected
after capturing real claude stream-json.

Codex: tail the rollout transcript (located by thread.started's
thread_id, same source codex's interactive UI reads) and emit
cumulative Tokens per token_count event, deduped on movement. The
parser stops the tailer and waits for it before closing the event
channel. turn.completed now also emits Tokens per turn (multi-turn
runs update at each boundary) with the previous post-loop emission
kept as a defensive backstop.

Verified end-to-end via agent shims driving entire review: claude
streaming envelopes with usage snapshots, and a codex shim writing
rollout token_count lines picked up by the tailer, both to clean
completion. Race-detector clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 01KWYC8H544S7DV9G0YSEQNV9H
Copilot AI review requested due to automatic review settings July 7, 2026 13:29
@peyton-alt
peyton-alt requested a review from a team as a code owner July 7, 2026 13:29

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 5860d3a. Configure here.

Comment thread cmd/entire/cli/agent/codex/review_tokens.go
Comment thread cmd/entire/cli/agent/codex/reviewer_test.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves entire review live token reporting by emitting incremental token updates during reviewer runs for both Claude Code and Codex, instead of only reporting totals at completion.

Changes:

  • Claude Code: emit input-only Tokens{In, Out:0} snapshots from assistant stream-json envelopes, plus final {In, Out} from the terminal result.
  • Codex: start a rollout-transcript tailer keyed by thread_id to emit live cumulative Tokens between turn boundaries; additionally emit Tokens on every turn.completed.
  • Add targeted unit tests and fixtures to lock these live-token semantics.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cmd/entire/cli/agent/codex/reviewer.go Starts/stops a rollout token tailer and emits Tokens per turn.completed.
cmd/entire/cli/agent/codex/reviewer_test.go Adds coverage asserting per-turn token emissions.
cmd/entire/cli/agent/codex/review_tokens.go New rollout JSONL tailer that emits cumulative Tokens on token_count events.
cmd/entire/cli/agent/codex/review_tokens_test.go Adds tests for tailing behavior and parser↔tailer wiring.
cmd/entire/cli/agent/claudecode/testdata/stream_with_deltas.jsonl Adds a realistic Claude stream-json fixture with per-assistant usage snapshots and a final result.
cmd/entire/cli/agent/claudecode/reviewer.go Emits input-only Tokens for assistant envelopes; final aggregate Tokens from result.
cmd/entire/cli/agent/claudecode/reviewer_test.go Updates existing expectations and adds a new test for input-only during-run token behavior.
Comments suppressed due to low confidence (1)

cmd/entire/cli/agent/codex/reviewer.go:236

  • Finished should be the last event on the channel. With the rollout tailer running concurrently, emit Finished only after stopping and waiting for the tailer; otherwise, a final token_count write could arrive after Finished and reorder the stream.
			// Success is hard-coded true here because codex's `turn.completed`
			// envelope has no turn-level error field in 0.130.0. If a future
			// codex version adds one (e.g., an `error` or `is_error` field on
			// the envelope), capture it into a local during the switch case and
			// thread it through here as `!turnErr` — mirroring claude's
			// `!resultErr` pattern.
			out <- reviewtypes.Finished{Success: true}
			return

Comment thread cmd/entire/cli/agent/codex/reviewer.go Outdated
Comment thread cmd/entire/cli/agent/codex/reviewer_test.go
peyton-alt and others added 2 commits July 7, 2026 18:03
…ngs)

A live full-crew dogfood review of this PR (claude x2 + codex, all
request-changes) converged on the same contract violations; every
finding is reproduced by a failing test first.

Claude: mid-run emissions were per-API-call context snapshots, but the
Tokens contract is cumulative running totals. The parser now sums input
once per unique message id (the same usage block repeats on every
content-block envelope of one call), so mid-run values are true running
totals converging exactly to the result aggregate — the fixture pins
56277 -> 112903 -> 169637 == result. Duplicate per-envelope emissions
are gone; the false 'consumers render input-only Tokens differently'
comment is corrected.

Codex: the rollout tailer was stopped in a defer that ran after the
terminal Tokens/Finished sends, so a lagging tailer tick could emit
after Finished and make the final recorded totals nondeterministic
(reproduced 13/20 before the fix). The tailer is now stopped and
awaited before every terminal emission. Single-source authority: rollout
token_count totals are session-cumulative while turn.completed usage is
per-turn scale, so per-turn stdout emissions are suppressed once the
tailer has emitted (it remains the fallback when no rollout is found).
The unconditional post-loop backstop could only ever emit Tokens{0,0}
and erase the tailer's genuine totals; it is now gated on non-zero
usage and no tailer values. The tailer also does a final catch-up drain
on stop, stops (with a debug log) on persistent non-EOF read errors
instead of silently re-polling, and logs rollout-discovery exhaustion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 01KWYN394Q3PX5FRV70DGM53ZW
…OU re-emit, honest scale caveat

The post-fix verification crew run (approve with nits) left four lows:

- Claude's terminal result emission was unconditional, so a usage-less
  result envelope emitted Tokens{0,0} and erased the mid-run cumulative
  total — the same clobber class fixed on the codex side; now gated on
  non-zero usage.
- The rollout tailer re-emits its last totals on stop (bypassing dedup),
  closing the nanosecond TOCTOU where a per-turn stdout emission races
  past the parser's tailerEmitted check and would otherwise stand as the
  final recorded value.
- The parser comment claimed turn.completed usage is per-turn scale
  while the test fixture comment called it running usage; both now state
  the honest position — unverified for real multi-turn output, identical
  either way for single-turn exec reviews, tailer wins whenever present.
- Two parser tests fed thread.started without the codex session-dir
  override, sending the tailer globbing the real ~/.codex/sessions and
  exposing exact-count assertions to nondeterministic injection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 01KWYP9YXJ19MGHWQ2RWFA4EDR
peyton-alt and others added 2 commits July 8, 2026 15:32
…ot finding)

waitForRollout returned '' after ~30s of polling even with the review
still running, permanently exiting the tailer — a rollout materialising
late lost live tokens for the rest of a minutes-long run (the stdout
per-turn fallback still delivered final counts, but no live movement).
The poll now continues until the file appears or stop fires, debug-
logging once past the expected-quickly window (the signature of a codex
release changing the rollout layout) and backing off to a slower
cadence so a missing file costs one cheap glob every ~2.4s.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 01KX0YVCTWCESSW3AW8H673NGE
@peyton-alt
peyton-alt merged commit 380c265 into main Jul 13, 2026
11 checks passed
@peyton-alt
peyton-alt deleted the feat/review-live-tokens branch July 13, 2026 17:30
peyton-alt added a commit that referenced this pull request Jul 14, 2026
Follow-on to the one-row-per-agent collapse: #1666 (merged) streams
cumulative token counts per worker, and the TUI row overwrites on each
Tokens event (CU2). With two skill-workers folded into one agent row,
their independent cumulative counts overwrote each other — the live
number bounced between skills instead of showing the agent total. (The
final summary was already summed; only the live display was wrong.)

The sink now tracks each worker's latest cumulative Tokens and forwards
the per-agent SUM on every Tokens event, so the row's overwrite shows a
correct running total. Single-worker rows pass through unchanged.
Runs on the serial dispatch goroutine, so no locking; race-clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 01KXH2J8TTE8GJYBQE47W4BHKX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants