Skip to content

fix(tui): freeze the completion duration at Stop - #1438

Merged
dennisonbertram merged 1 commit into
mainfrom
fix/1434-freeze-completion-duration
Sep 9, 2026
Merged

fix(tui): freeze the completion duration at Stop#1438
dennisonbertram merged 1 commit into
mainfrom
fix/1434-freeze-completion-duration

Conversation

@dennisonbertram

Copy link
Copy Markdown
Owner

Closes #1434

Process note first: this fix was written, committed and pushed hours ago, and I never opened the PR — I was interrupted mid-flow and did not come back to it. The branch sat stranded on the remote while main kept the bug. It surfaced again only because the untrusted reviewer independently re-found the same defect on a later pass over this package. Rebased onto current main and opened now.

The bug

Worked for <d> kept climbing while it was on screen:

tick 0: "· Worked for 5.0s"
tick 1: "· Worked for 5.3s"
tick 2: "· Worked for 5.6s"
tick 3: "· Worked for 5.9s"

The run is already finished. View re-rendered ElapsedSeconds() — a live wall-clock read — on every tick of the completion window, so the figure a user reads was inflated by up to the window's length (~1.2s at 10 ticks x 120ms). For a short run that is a large relative error: a 0.4s run reports as 1.6s.

Fix

Stop freezes the elapsed duration into stoppedAfter, and the completion branch renders that. ElapsedSeconds() keeps its meaning for a live spinner, so nothing else changes.

Two cleanups from the same original review ride along, both in this file:

  • shortenLabel carried an unused full parameter — removed.
  • Tick indexes holds[m.step] while advancing modulo len(pulse), so an init now enforces that the two slices stay the same length rather than leaving a future edit to panic at runtime.

Verification

The test asserts successive renders are identical rather than asserting a number — a number would be timing-dependent and flaky, and identity is the property that actually matters. A control keeps the frozen value reflecting the real elapsed time, so freezing at zero would fail.

tick 0: "· Worked for 5.0s"
tick 1: "· Worked for 5.0s"
tick 2: "· Worked for 5.0s"
tick 3: "· Worked for 5.0s"

Spinner package green after rebase onto current main.

Provenance

Originally found by openai-gpt-oss-120b ($0.07/M) during a cheap-model evaluation, on a file gpt-6-astra ($10/M list) had already reviewed and passed — Astra had looked at the same code and drawn the wrong conclusion, filing it as snapshot-test nondeterminism, which is a false positive since those tests set startTime explicitly.

Then re-found by Astra on a later pass, which is what recovered this stranded branch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5

"Worked for <d>" kept climbing while it was displayed: View re-rendered
ElapsedSeconds(), a live wall-clock read, on every tick of the completion
window. The figure a user reads was inflated by up to ~1.2s, which for a
short run is a large relative error — a 0.4s run reported as 1.6s.

Stop now freezes the elapsed duration and the completion branch renders
that. ElapsedSeconds() keeps its meaning for a live spinner.

The test asserts successive renders are identical rather than asserting a
number: a number would be timing-dependent, and identity is the property
that actually matters. A control keeps the frozen value reflecting the
real elapsed time, so freezing at zero would fail.

Two cleanups from the same review: shortenLabel carried an unused `full`
parameter, and Tick indexes holds[m.step] while advancing modulo
len(pulse), so an init now enforces the two slices stay in step rather
than leaving a future edit to panic at runtime.

Both findings came from cheap models ($0.05-$0.07/M) reviewing a file
gpt-6-astra ($10/M) had already passed — and which it had looked at and
drawn the wrong conclusion about.

Closes #1434

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@dennisonbertram
dennisonbertram merged commit e008d64 into main Sep 9, 2026
2 checks passed
@dennisonbertram
dennisonbertram deleted the fix/1434-freeze-completion-duration branch September 9, 2026 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: completion line keeps counting after the run finishes

1 participant