Skip to content

fix(tui): the posture bar states how long the session has been working (#5914) - #5920

Merged
Hmbown merged 1 commit into
mainfrom
fix/session-working-timer-5914
Sep 6, 2026
Merged

fix(tui): the posture bar states how long the session has been working (#5914)#5920
Hmbown merged 1 commit into
mainfrom
fix/session-working-timer-5914

Conversation

@Hmbown

@Hmbown Hmbown commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Closes #5914

What the founder saw

"it does seem kinda weird that we don't show how long the overall thing has been working anymore? just looking at the screen" — during a multi-hour operate session with sub-agents, nothing on a fixed row said how long the session had been working, how long the current turn had been running, or whether anything was parked.

What lost it

Two commits, and the second one is the one that finished the job:

  • 146ab7f756 "refactor(tui): delete the legacy FooterWidget rendering path" removed crates/tui/src/tui/widgets/footer.rs whole, and with it footer_worked_chip — the worked Nm Ss indicator built from App::cumulative_turn_duration (PRIOR: Cumulative turn duration worked-for separator #448).
  • 329960fcbf "feat: Codewhale 0.9.12 shell, brand, fleet, and Operate (mega)" (feat: Codewhale 0.9.12 shell, brand, fleet, and Operate (mega) #5826) merged the activity and identity bands into the posture bar and deleted phase_strip::working_detail, which had painted the current turn's elapsed. Its stated rationale was "the phase word it painted now lives in the transcript's active row" — true, and true only until the transcript scrolls, which is what a long session does. (That commit also left working_detail's doc comment orphaned above the next function; this PR removes it.)

App::cumulative_turn_duration has been accruing the whole time with no reader.

Restore, not replace

The posture bar (row 1 under the composer — the line the founder's screenshot shows) now paints a two-part working clock:

▶▶ ask (Shift+Tab) · work (Tab) · working 1m 15s · 2 agents · worked 41m 12s · Esc to interrupt
  • Turn half{phase label} {elapsed}. The phase word comes from phase_marker_with_activity, the shell's existing single owner, so the reading is working 1m 15s, using tool 1m 15s, sub-agents underway 1m 15s or waiting on you 1m 15s. A bare duration cannot distinguish a session producing tokens from one parked on a tool, a sub-agent, or an unanswered prompt; waiting phases also paint in the Waiting ink.
  • Session half — the classic worked {elapsed} chip: cumulative_turn_duration (finished turns) plus the live turn, so it ticks continuously and never jumps at TurnComplete. It is model work, not wall clock since launch, and it stays silent under a minute exactly as PRIOR: Cumulative turn duration worked-for separator #448 specified.

Both durations go through crate::elapsed::format_elapsed_secs. No second formatter, no new state, no new MessageId (the session half reuses FooterWorkedChip, which survived in all 15 locale packs).

Shed ladder (narrow terminals)

Most expendable first: turn clock → session clock → hint → counts → context-cap warning → mode key → mode → permission key. The permission chip still never sheds (#5796).

Both clock halves go before the hint and the counts. The clock is what a glance wants, but at the narrowest widths the row's other facts are what a keystroke wants — the counts name work you can open, the hint names a chord you can press right now (Esc to interrupt, Enter again to send now) — and an 80-column row carrying the filesystem-scope notice cannot hold all of it. The turn half goes first: the transcript's active row and the spinner also show the turn is alive, while the session total is stated nowhere else, so the session reading is the longer-lived of the two and survives to about 100 columns.

The context-cap warning was promoted above both clocks and the counts: it is not a hint, it is the reason the next turn will not start. Previously it shed first, with the hint.

The environment-dependent chip that broke CI

one_owner_tests passed on macOS and failed deterministically on both the ubuntu and windows legs. The cause was not the clock: underwater::filesystem_scope_notice appends files: workspace (unenforced) to the permission chip whenever App::sandbox_backend is None — true on default Linux and all Windows, false on macOS, which resolves seatbelt. That is ~30 columns of chip that appears or not depending on the host, and it decides what an 80-column shed ladder can still hold.

frame_app() now pins sandbox_backend = None, and the test asserts the notice is actually present, so if the pin ever stops working the test says so instead of silently going back to host-dependent. composed_frame_paints_each_fact_in_exactly_one_row gained a 160-column row so both clock halves are asserted at a width that holds them, and the clock assertions are gated on the width that each half needs (120 for the session half, 160 for the turn half). double_tap_window_shows_the_send_now_hint is back at its original 120 columns — with the reordered ladder the steer survives there.

Two adjacent defects fixed because this code paints them

  • en.json PhaseWorking read "in the current" — a corrupted find/replace. Every other locale says "working" / "arbeitet" / "trabajando". This is the word the clock paints, so it is fixed here. (The same corruption survives as a literal string in footer_hints.rs::friendly_subagent_progress; left alone as out of scope.)
  • Ascii-safe mode painted the row's · separators raw while projecting every other glyph, which would have put a projected . inside the clock next to unprojected · separators. Separators are now projected too.

Tests

Changed behaviour, so two tests that encoded the old decision were changed with it:

  • tideline_tests::posture_bar_states_no_phase_cost_or_context_readingposture_bar_states_no_cost_or_context_reading. It still pins that the bar carries no price and no context reading; it now asserts the elapsed readings instead of forbidding them.
  • one_owner_tests::composed_frame_paints_each_fact_in_exactly_one_row asserted the posture bar contains no underway and no 1m 15s. It now asserts the clock is there and paints in exactly one row — the one-owner contract is kept, the owner moved.

New:

  • live_turn_clock_states_the_turn_and_the_session — both readings and both inks.
  • session_reading_carries_finished_turns_plus_the_live_one — continuity across TurnComplete, and the stopped clock's quiet ink.
  • clock_distinguishes_working_from_waiting_on_something — working vs sub-agents underway vs waiting on you, driven end-to-end through tideline_footer_from_app with real App state.
  • a_session_that_has_not_worked_states_no_clock — the sub-minute floor, and that a live turn still times itself from the first second.
  • narrow_widths_shed_the_clocks_before_the_hint_and_counts — over every width 8..=160: the turn clock never survives without the session clock, the session clock never without the hint, the hint never without the counts, and neither reading is ever painted half-truncated.
  • cap_warning_outranks_the_clock_when_only_one_fits.
  • posture_bar_sheds_the_clocks_then_the_hint_counts_and_posture_chips — the full ladder, by narrowest width at which each fact appears.

Of the four footer_{w}x{h} goldens, only footer_80x24 shows a shed change (it keeps the hint and the counts and drops the turn clock); 100/120/160 carry the full band. The settings-stage goldens are untouched (the preview's footer area is too narrow for a clock, so it does not get one).

Verified

Command Result
cargo fmt -p codewhale-tui clean
cargo check -p codewhale-tui clean
cargo clippy -p codewhale-tui --all-targets clean
RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --lib 11690 passed, 0 failed, 12 ignored
... --lib -- tui::ui::frame tui::phase_strip tui::footer 45 passed, 0 failed
... --lib -- tui::phase_strip 30 passed, 0 failed
... --lib -- tui::ui::frame 12 passed, 0 failed
... --lib -- tui::footer 3 passed, 0 failed
... --lib -- localization 49 passed, 0 failed
... --lib -- elapsed 20 passed, 0 failed

No stack-overflow aborts locally. Three flakes seen across full-suite runs, each passing in isolation and on the next run, none touching this change: session_control_acceptance::tests::tui_and_api_listings_agree, client::tests::deepseek_anthropic_translate_uses_messages_endpoint, tui::clipboard::tests::tmux_load_buffer_w_reaches_attached_client_with_default_passthrough_disabled.

Not verified

  • No run against a real provider, a real multi-hour session, or a real terminal. All evidence here is buffer-level: rendered TestBackend buffers and unit tests. Nobody has watched the clock tick in a live operate session.
  • The 14 non-English locale packs are unchanged. The session half reuses their existing FooterWorkedChip translation; the turn half reuses their existing phase words. The PhaseWorking fix is English-only because English was the only pack that was wrong.
  • #5906 (sub-agents parked as "waiting for input" with no cleanup path), reported in the same session, is not addressed here. This PR only makes the parked time visible.

🤖 Generated with Claude Code

https://claude.ai/code/session_0188XYyJaw9Mh9uSrqQBoqhm

Copilot AI lite review requested due to automatic review settings September 6, 2026 05:46
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-06T05:52:26.842466Z c8edf73 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cursor

cursor Bot commented Sep 6, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_23a8abfd-6d4e-4b02-b51a-be3bc7c643b4)

@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 2m 59s —— View job


Review: fix(tui): posture bar states how long the session has been working (#5914)

Todo list

  • Gather context (diff against the PR's single commit vs. its parent — origin/main was locally stale/diverged, so I diffed HEAD~1..HEAD, which matches the PR's listed changed files exactly)
  • Review phase_strip.rs logic changes (turn/session clock, shed ladder, ascii-safe separators)
  • Review test changes (tideline_tests.rs, one_owner_tests.rs)
  • Check locale fix (en.json) and golden file updates
  • Check for provider/model safety issues (none applicable — no provider/model routing touched)
  • Post final review

Summary

This is a clean, well-scoped restoration. I read the full diff of all 8 files plus the surrounding module (App::turn_started_at, App::cumulative_turn_duration, ShellPhase, ChromeInk, ascii_fallback) to verify the claims in the PR description against the actual code.

Correctness

  • working_clock (crates/tui/src/tui/phase_strip.rs:1035) correctly derives the turn half from app.turn_started_at.map(|s| s.elapsed()) and the session half from cumulative_turn_duration.saturating_add(turn.unwrap_or_default()). This matches how cumulative_turn_duration is actually accrued in event_loop.rs:2129 at TurnComplete (turn elapsed folded in, turn_started_at reset to None), so the "never jumps at TurnComplete" claim holds up.
  • The rung constants (SHED_HINT..SHED_PERMISSION_KEY, MAX_SHED) are a good replacement for the old magic numbers (shed < 5, shed < 4, etc.) — this alone is a readability win independent of the feature.
  • The ascii-safe separator fix (footer.sym(separator_before(item)) at phase_strip.rs:910) is correct: ascii_fallback only exact-matches bare glyphs like "·", not " · " with surrounding spaces, but sym()'s char-by-char fallback path handles the padded separator correctly by mapping each character individually. Verified both ITEM_SEPARATOR (" · ") and COUNT_SEPARATOR (", ") project correctly.
  • PhaseWorking in en.json was indeed uniquely corrupted ("in the current") versus all 14 other locales already saying the equivalent of "working" — confirmed by grepping every locale file. Good, narrowly-scoped fix.
  • FooterWorkedChip reuse confirmed present in all 15 locale packs, so no missing-translation risk from reusing that message id for the session clock.

One documentation nit (not a code bug): the PR body says "At 80 columns the turn half sheds and the session half stays," but the actual footer_80x24.txt golden shows the opposite — the hint sheds first and both clock halves plus counts remain visible at 80 columns (▶▶ ask (Shift+Tab) · work (Tab) · working 1m 15s · 2 agents · worked 41m 12s fits in 80 with room to spare). This doesn't affect the code's correctness — the shed order in posture_items is right and well-tested — it's just a stale/imprecise line in the write-up.

Tests: thorough and well-targeted. narrow_widths_shed_the_hint_and_counts_before_the_clock and cap_warning_outranks_the_clock_when_only_one_fits sweep the full width range (8..=160) rather than spot-checking, which is the right way to pin a shed ladder. The two rewritten tests (posture_bar_states_no_cost_or_context_reading, composed_frame_paints_each_fact_in_exactly_one_row) correctly flip their assertions to match the restored behavior rather than just weakening them.

Reuse: no new formatter, no new MessageId, no new state beyond the two ClockReading fields threaded through the existing builder pattern — follows the "restore, not replace" framing well. No dead code introduced; the orphaned doc comment above posture_items was correctly cleaned up.

Not verified: I could not run cargo check/cargo fmt/cargo test in this sandboxed environment — commands requiring build/network access were blocked from proceeding without interactive approval. My review is based on static reading of the diff and cross-referencing call sites (App fields, ShellPhase variants, ChromeInk variants, locale files), not a compiled/executed build. The PR's own "Verified" table (11690 passed, clippy/fmt clean) is the actual build evidence for this change.

No correctness bugs, no provider/model-routing concerns (none of this touches routing), and no security issues found.

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.

🟡 Changes recommended

Several new/updated tests assert exact elapsed-time strings derived from Instant::elapsed(), making them non-deterministic and potentially flaky under slow CI conditions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Restores a continuously visible two-part “working clock” to the TUI posture bar so long-running sessions show (a) what the current turn is doing and for how long, and (b) how long the session has “worked” in total, addressing the regression reported in #5914.

Changes:

  • Adds turn + session clock facts to the posture bar rendering (including ascii-safe projection for separators) and defines a new shed ladder that keeps the session clock late under width pressure.
  • Updates/extends posture-bar tests and goldens to assert the restored clock behavior and revised shedding priorities.
  • Fixes the English PhaseWorking localization string.
File summaries
File Description
crates/tui/src/tui/ui/frame/one_owner_tests.rs Updates one-owner frame assertions to include the working clock and adjusts hint expectations under width pressure.
crates/tui/src/tui/phase_strip/tideline_tests.rs Updates posture-bar expectations and adds a working-clock test suite (wording/ink/continuity/shedding).
crates/tui/src/tui/phase_strip.rs Implements working clock facts, shedding priorities, and ascii-safe projection for separators; wires clock into footer facts.
crates/tui/src/tui/goldens/footer_80x24.txt Re-blesses footer golden for restored clock content at 80 columns.
crates/tui/src/tui/goldens/footer_100x30.txt Re-blesses footer golden for restored clock content at 100 columns.
crates/tui/src/tui/goldens/footer_120x32.txt Re-blesses footer golden for restored clock content at 120 columns.
crates/tui/src/tui/goldens/footer_160x40.txt Re-blesses footer golden for restored clock content at 160 columns.
crates/tui/locales/en.json Fixes PhaseWorking translation (“in the current” → “working”).
Review details

Suppressed comments (5)

crates/tui/src/tui/phase_strip/tideline_tests.rs:475

  • This test pins an exact "worked 60m 30s" string even though the live reading is derived from Instant::elapsed(). That makes the test timing-sensitive; you can preserve the continuity invariant by asserting the idle reading equals whatever the live reading was.
    // Turn in flight: the finished total plus this turn.
    app.is_loading = true;
    app.turn_started_at = Some(Instant::now() - Duration::from_secs(30));
    let (_, live) = working_clock(&app, ShellPhase::Working, "working");
    assert_eq!(live.expect("live session clock").0, "worked 60m 30s");

crates/tui/src/tui/phase_strip/tideline_tests.rs:506

  • These asserts hard-code exact elapsed strings derived from Instant::elapsed(). The important behavior here is the phase word/ink changing; asserting exact seconds makes the test timing-sensitive.
    let working = tideline_footer_from_app(&mut app, 160)
        .turn_clock
        .expect("working clock");
    assert_eq!(working.0, "working 1m 15s");
    assert_eq!(working.1, ChromeInk::Active);

crates/tui/src/tui/phase_strip/tideline_tests.rs:529

  • This assertion depends on an exact "waiting on you 1m 15s" string derived from Instant::elapsed(). Since the timing is inherently non-deterministic, assert the stable prefix instead and keep the ink assertion.
    let waiting = tideline_footer_from_app(&mut app, 160)
        .turn_clock
        .expect("waiting clock");
    assert_eq!(waiting.0, "waiting on you 1m 15s");
    assert_eq!(waiting.1, ChromeInk::Waiting);
    assert_ne!(waiting.1, working.1, "waiting must not read as working");

crates/tui/src/tui/phase_strip/tideline_tests.rs:563

  • This asserts an exact "working 3s" string derived from Instant::elapsed(). If the test runs slowly enough to tick another second, it can fail even though behavior is correct; assert the stable prefix and that the clock isn't at 0s instead.
    let facts = tideline_footer_from_app(&mut app, 160);
    assert_eq!(
        facts.turn_clock.expect("a live turn always times itself").0,
        "working 3s"
    );
    assert!(
        facts.session_clock.is_none(),
        "the sub-minute total is still quiet"
    );

crates/tui/src/tui/ui/frame/one_owner_tests.rs:201

  • Like the session clock assertion above, this turn-clock assertion matches an exact duration string derived from Instant::elapsed(), which can tick during a slow test run. Using a stable prefix/needle avoids time-based flakes while still asserting the turn clock only appears in one row at wider widths.
        if width >= 120 {
            assert!(
                rows[posture].contains("sub-agents underway 1m 15s"),
                "{}",
                rows[posture]
            );
            assert_eq!(
                count_rows_containing(&rows, "sub-agents underway 1m 15s"),
                1,
                "{width}x{height}: the turn clock paints in exactly one row:\n{}",
                rows.join("\n")
            );
  • Files reviewed: 8/8 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +452 to +460
let facts = tideline_footer_from_app(&mut app, 160);
let (turn, turn_ink) = facts.turn_clock.expect("a live turn states its elapsed");
assert_eq!(turn, "working 1m 15s");
assert_eq!(turn_ink, ChromeInk::Active);
let (session, session_ink) = facts
.session_clock
.expect("a working session states its total");
assert_eq!(session, "worked 41m 15s");
assert_eq!(session_ink, ChromeInk::Active);
Comment on lines +183 to +189
assert!(rows[posture].contains("worked 1m 15s"), "{}", rows[posture]);
assert_eq!(
count_rows_containing(&rows, "worked 1m 15s"),
1,
"{width}x{height}: the session clock paints in exactly one row:\n{}",
rows.join("\n")
);

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8edf739d8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1051 to +1053
let worked = app
.cumulative_turn_duration
.saturating_add(turn.unwrap_or_default());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve elapsed time when an active turn is interrupted

When a user cancels with Esc/Ctrl+C, or when stall/disconnect recovery runs, the new total includes the live turn_started_at duration on one frame, but those paths clear turn_started_at without adding it to cumulative_turn_duration (mark_active_turn_cancelled_locally and recover_stalled_runtime_turn). The next frame therefore makes worked … jump backward by the entire interrupted turn, and the later completion event cannot recover that time because the start timestamp is already gone. Fold the elapsed duration into the cumulative value before every active-turn reset.

Useful? React with 👍 / 👎.

Comment on lines +1044 to +1046
format!(
"{phase_label} {}",
crate::elapsed::format_elapsed_secs(turn.as_secs())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Localize the complete turn-clock phrase

The new user-visible clock concatenates a translated activity label with a duration in renderer code, so translators cannot control word order, punctuation, or inflection; this already yields copy such as Compacting context… 1m 15s because the existing activity string includes an ellipsis. Add a typed message with named phase/duration placeholders and translate the complete phrase rather than composing prose here.

AGENTS.md reference: crates/tui/AGENTS.md:L25-L26

Useful? React with 👍 / 👎.

Comment on lines +1051 to +1053
let worked = app
.cumulative_turn_duration
.saturating_add(turn.unwrap_or_default());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Stop adding parked time to the worked total

When a turn is parked on a user-input or approval prompt, turn_started_at remains set while ShellPhase becomes Waiting/Approval, so this unconditional addition keeps increasing the worked … total for the entire unattended wait. After a prompt sits unanswered for hours, the bar therefore claims those hours as actual session work even while its adjacent turn clock says waiting on you; pause or exclude the live contribution while the typed phase is parked.

Useful? React with 👍 / 👎.

phase: ShellPhase,
phase_label: &str,
) -> (ClockReading, ClockReading) {
let turn = app.turn_started_at.map(|started| started.elapsed());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Schedule clock refreshes when animations are disabled

For users with fancy_animations = false, MotionMode::Still makes should_tick_status_animation return false and the event loop redraws only on state changes. Because this clock merely samples Instant::elapsed() during rendering and adds no semantic refresh request, it can remain frozen throughout a silent model request—the exact stuck-turn scenario this feature is intended to expose—until another engine or input event happens to redraw the frame. Schedule a low-frequency clock redraw independently of decorative animation.

Useful? React with 👍 / 👎.

Comment on lines +1037 to +1040
let ink = match phase {
ShellPhase::Waiting | ShellPhase::Approval => ChromeInk::Waiting,
ShellPhase::Failed => ChromeInk::Attention,
_ => ChromeInk::Active,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Render failed clock readings with failure ink

When a non-recoverable engine error marks runtime_turn_status as failed, turn_started_at remains populated until TurnComplete, so the new turn and session readings render during a real failed phase. Mapping that phase to ChromeInk::Attention paints both readings with the waiting/approval cognition color instead of the status-bar grammar's ChromeInk::Failure, making an actual failure look like an approval state. Use the failure ink for ShellPhase::Failed.

AGENTS.md reference: crates/tui/AGENTS.md:L14-L15

Useful? React with 👍 / 👎.

@codewhale-agent codewhale-agent 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.

Codewhale review

Restores a two-part working clock to the posture bar, fixes the English PhaseWorking string, and projects separators in ascii-safe mode. The change reuses existing timing fields and locale strings, with extensive buffer-level tests.

Findings

  • [INFO] PR description contradicts the 80-column golden (crates/tui/src/tui/goldens/footer_80x24.txt:24)
    The description states 'At 80 columns the turn half sheds and the session half stays', but the updated footer_80x24.txt golden shows both 'working 1m 15s' (turn half) and 'worked 41m 12s' (session half) present. This is likely a documentation error, not a code bug, because the golden is generated from the implementation, but it could confuse future maintainers about the intended shedding behavior at narrow widths.
  • [INFO] Missing test for ascii-safe separator projection (crates/tui/src/tui/phase_strip.rs)
    The diff changes the separator rendering to call footer.sym() on the separator string, but no new test verifies that ascii-safe mode actually projects the '·' separators. Existing golden tests may not cover ascii-safe mode, leaving this fix unguarded against regressions.
  • [INFO] Session clock floor boundary not explicitly tested (crates/tui/src/tui/phase_strip/tideline_tests.rs)
    The session clock is suppressed below 60 seconds of model work (CLOCK_SESSION_FLOOR_SECS). Tests cover 42s and 45s as sub-minute, but there is no test for exactly 59s vs 60s to pin the boundary condition. This is a minor gap given the thoroughness of the rest of the suite.

Suggestions

  • crates/tui/src/tui/phase_strip.rs:912 — Add a test in tideline_tests.rs that renders with ascii-safe mode enabled and asserts the posture bar separators are projected (e.g., '.') rather than raw '·'. This would cover the new separator projection behavior.

Assessment

The PR restores the lost working clock with a clear design, good reuse of existing state, and extensive tests. The implementation is sound and the changes are low risk. Remaining concerns are documentation consistency and a couple of minor test coverage gaps.


Advisory review by Codewhale (codewhale review --pr 5920 --post, head c8edf739d8caa0a2bb6958a16718f92cc7276a62). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.



▶▶ ask (Shift+Tab) · work (Tab) · 2 agents · Esc to interrupt
▶▶ ask (Shift+Tab) · work (Tab) · working 1m 15s · 2 agents · worked 41m 12s

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[INFO] PR description contradicts the 80-column golden

The description states 'At 80 columns the turn half sheds and the session half stays', but the updated footer_80x24.txt golden shows both 'working 1m 15s' (turn half) and 'worked 41m 12s' (session half) present. This is likely a documentation error, not a code bug, because the golden is generated from the implementation, but it could confuse future maintainers about the intended shedding behavior at narrow widths.

// Projected like every other glyph on the row: an ascii-safe
// terminal that cannot draw `·` must not get one here either,
// least of all next to a clock reading whose own separator was
// projected.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add a test in tideline_tests.rs that renders with ascii-safe mode enabled and asserts the posture bar separators are projected (e.g., '.') rather than raw '·'. This would cover the new separator projection behavior.

#5914)

A multi-hour operate session had no working-time indicator anywhere a
glancing user looks. Two commits took it away:

  146ab7f  deleted the legacy FooterWidget path, and with it the
              `worked Nm Ss` chip built from `App::cumulative_turn_duration`
              (#448). `crates/tui/src/tui/widgets/footer.rs` went whole.
  329960f  the 0.9.12 mega shell merged the activity and identity bands
              into the posture bar and deleted `phase_strip::working_detail`,
              which had painted the current turn's elapsed. Its rationale —
              "the transcript's active row owns the pulse" — is true until
              the transcript scrolls, which is exactly what a long session
              does.

`App::cumulative_turn_duration` kept accruing the whole time with no reader.
Restore rather than replace: the posture bar now paints a two-part working
clock in the row under the composer.

  ▶▶ ask (Shift+Tab) · work (Tab) · working 1m 15s · 2 agents · worked 41m 12s · Esc to interrupt

- Turn half: `{phase label} {elapsed}`, using the phase word the transcript
  already owns, so `working`, `using tool`, `sub-agents underway` and
  `waiting on you` distinguish producing tokens from parked on something.
  Waiting phases paint in the Waiting ink.
- Session half: the classic `worked {elapsed}` chip — finished turns plus the
  live one, so it ticks continuously and does not jump at TurnComplete. Model
  work, not wall clock since launch. Silent under a minute, as in #448.
- Both durations go through `crate::elapsed::format_elapsed_secs`; no second
  formatter, and no new MessageId.

Shed ladder, most expendable first: turn clock, session clock, hint, counts,
context-cap warning, mode key, mode, permission key. The permission chip
still never sheds (#5796). Both clock halves go before the hint and the
counts: the clock is what a glance wants, but at the narrowest widths the
counts name work you can open and the hint names a chord you can press right
now, and an 80-column row carrying the filesystem-scope notice cannot hold
all of it. The turn half goes first because the transcript's active row and
the spinner also show the turn is alive; the session total is stated nowhere
else, so it is the longer-lived of the two and survives to ~100 columns. The
cap warning was promoted above both clocks and the counts, since a full
context is the one thing that stops the next turn.

The frame fixtures now pin `sandbox_backend = None`. The permission chip
carries `files: workspace (unenforced)` only where no sandbox backend can
enforce the policy — true on Linux and Windows, false on macOS seatbelt —
which is 30 columns of chip that decides what an 80-column ladder holds.
`one_owner_tests` passed locally and failed on both CI legs on exactly that
difference; the fixture now asserts the notice is present so the widths mean
the same thing everywhere.

Also fixed: en.json `PhaseWorking` read "in the current" — a corrupted
find/replace. Every other locale says "working". This is the word the clock
paints. (The same corruption survives as a literal in `footer_hints.rs`; left
alone, out of scope.)

Ascii-safe mode now projects the row's `·` separators like every other glyph;
they were being painted raw next to projected text.

Tests: `cargo test -p codewhale-tui --lib` 11690 passed, 0 failed, 12
ignored. `tui::phase_strip` 30 passed, `tui::ui::frame` 12 passed,
`tui::footer` 3 passed, `localization` 49 passed, `elapsed` 20 passed.
`cargo fmt`, `cargo check -p codewhale-tui` and `cargo clippy -p
codewhale-tui --all-targets` clean. Of the four `footer_{w}x{h}` goldens only
80x24 shows a shed change (it keeps the hint and drops the turn clock); the
settings-stage goldens are untouched.

Not verified: no run against a real provider or a real multi-hour session —
this is buffer-level evidence only.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0188XYyJaw9Mh9uSrqQBoqhm
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
@Hmbown
Hmbown force-pushed the fix/session-working-timer-5914 branch from c8edf73 to b8bd12c Compare September 6, 2026 06:26
@cursor

cursor Bot commented Sep 6, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_fda4a3ea-7892-46a4-a10c-b03039691032)

@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 3m 59s —— View job


Review: fix(tui): posture bar states how long the session has been working (#5914)

Todo list

  • Gather context (diff of the single commit HEAD~1..HEAD, matches the PR's 8 listed files)
  • Verify the correctness concerns already flagged by Codex, by tracing the actual call sites
  • Check provider/model-routing safety
  • Check reuse/simplification and test coverage
  • Post inline comments + summary

Summary

I traced Codex's four P2 findings against the actual state-management code (event_loop.rs, session_state.rs, ui.rs, underwater.rs) rather than just the diff, since two of them are about interactions with code this PR doesn't touch. Two are confirmed real bugs, one is a confirmed behavior mismatch worth a decision, one I'd downgrade. Posted as inline comments on phase_strip.rs.

Confirmed — correctness bugs (inline comments posted):

  1. Session clock jumps backward and permanently loses time on cancel/stall (phase_strip.rs:1055). working_clock derives worked from cumulative_turn_duration + turn_started_at.elapsed(). The only place that folds the live turn into cumulative_turn_duration is the TurnComplete handler at event_loop.rs:2117-2130. But mark_active_turn_cancelled_locally (ui.rs:989-999, used by Esc/Ctrl+C/approval-abort) and two paths in session_state.rs (dispatch-watchdog timeout, stall recovery) all clear turn_started_at = None without folding it in first. Once that happens, even a later TurnComplete computes turn_elapsed as zero (the start timestamp is already gone), so the time isn't deferred — it's lost. Net effect: pressing Esc mid-turn drops the worked … chip backward immediately and it never recovers that time. This directly contradicts the PR's own claim that the chip "never jumps at TurnComplete" — it still jumps, just at cancellation.

  2. ShellPhase::Failed uses the wrong ink (phase_strip.rs:1043). The rest of the codebase (underwater.rs:595, history/tideline_stream.rs, history/automation.rs, work_surface/panels.rs) maps Failed → ChromeInk::Failure, a dedicated variant that exists precisely for this. This new code maps it to ChromeInk::Attention instead — the same ink used for Waiting/Approval — so a real failure renders in the "waiting on you" color rather than the failure color.

Confirmed — behavior worth a decision (inline comment posted):

  1. Waiting/approval time counts toward "worked" (phase_strip.rs:1056). turn_started_at stays Some while parked in Waiting/Approval, and that elapsed time is unconditionally folded into worked. That contradicts the adjacent doc comment ("model work, not wall clock… an idle TUI does not claim to have been working") — a 2-hour unanswered approval prompt would show worked 2h 00s even though nothing ran. This is the same failure mode the turn-clock half is designed to disambiguate, leaking into the session half.

Looked at, not flagging:

  • Redraw scheduling with fancy_animations = false: confirmed should_tick_status_animation (ui/motion.rs:225) returns false whenever MotionMode::Still, independent of app.is_loading, so the periodic animation-driven redraw this loop uses is skipped in that mode. However this is pre-existing scheduling shared by the transcript's own live phase word, not something this PR introduces or changes — any freeze during a silent model turn under Still mode already affects other footer/transcript liveness cues today, not just the new clock. Not something I'd hold this PR on.
  • Composing {phase_label} {elapsed} without a dedicated localized template: real i18n quality concern (raises the "Compacting context… 1m 15s" double-punctuation case Codex noted), but it's consistent with how the surrounding file already composes chips (chip() combining chip text + key hint), and the PR is explicit that non-English packs are unchanged/out of scope for this pass. Minor, not blocking.
  • Exact-elapsed-string test assertions (Copilot's flakiness callout): agree these are theoretically timing-sensitive, but each captures Instant::now() - Duration::from_secs(N) and formats to whole seconds — a flake requires the test to itself run for a full second between setup and assertion, which is a real but low-probability risk given the existing suite's flake report was elsewhere (session_control_acceptance). Not urgent.

Reuse / design: the shed-ladder constants (SHED_HINT..MAX_SHED) are a clear improvement over the old magic numbers, and reusing FooterWorkedChip / format_elapsed_secs / the existing phase-label helper avoids new state or formatters, matching the "restore, not replace" framing. The en.json PhaseWorking fix and the ascii-safe separator fix are both narrowly scoped and correct as described.

Provider/model routing: not applicable — no routing, model selection, or prefix-parsing code touched.

Tests: the new suite (live_turn_clock_states_the_turn_and_the_session, session_reading_carries_finished_turns_plus_the_live_one, narrow_widths_shed_the_hint_and_counts_before_the_clock, etc.) is thorough for the intended behavior, but none of it exercises the cancel/stall paths in finding #1, so the backward-jump-and-loss bug has no regression coverage. Worth a test once #1 is fixed (assert cumulative_turn_duration after a simulated Esc mid-turn).

Not verified: I did not run cargo build/test in this environment (no build/network approval here); this review is based on static tracing of the diff against its call sites in event_loop.rs, session_state.rs, ui.rs, underwater.rs, and palette/grammar.rs. The PR's own verification table (11690 passed) is the actual build evidence.
· branch fix/session-working-timer-5914

let turn = app.turn_started_at.map(|started| started.elapsed());
let ink = match phase {
ShellPhase::Waiting | ShellPhase::Approval => ChromeInk::Waiting,
ShellPhase::Failed => ChromeInk::Attention,

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.

Correctness: ShellPhase::Failed should use ChromeInk::Failure, not Attention.

ChromeInk has a dedicated Failure variant (crates/tui/src/palette/grammar.rs:66) that the rest of the codebase uses for exactly this phase — e.g. crates/tui/src/tui/underwater.rs:595: ShellPhase::Failed => ChromeInk::Failure, and the same mapping in history/tideline_stream.rs, history/automation.rs, work_surface/panels.rs. This new working_clock instead maps Failed to Attention, the same ink used elsewhere for Waiting/Approval. A real turn failure will render in the cognitively-loaded "waiting on you" color instead of the failure color, undermining the "ink reflects waiting vs active work" claim in the PR description.

Suggested change
ShellPhase::Failed => ChromeInk::Attention,
ShellPhase::Failed => ChromeInk::Failure,

ink,
)
});
let worked = app

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.

Correctness: the session clock jumps backward — and permanently loses time — on every local cancel/stall recovery, not just at normal TurnComplete.

This computes worked = cumulative_turn_duration + turn_started_at.elapsed(). The only place that folds the live turn into cumulative_turn_duration is the TurnComplete handler at crates/tui/src/tui/ui/event_loop.rs:2117-2130. But three other paths clear turn_started_at = None without folding it in first:

  • mark_active_turn_cancelled_locally (crates/tui/src/tui/ui.rs:989-999) — Esc/Ctrl+C/approval-abort
  • the dispatch-watchdog and stall-sync branches in crates/tui/src/tui/ui/session_state.rs:103 and :128
  • recover_stalled_runtime_turn (crates/tui/src/tui/ui/session_state.rs:250-271)

Once one of these runs, turn_started_at is already None, so even if a later TurnComplete event does arrive, turn_elapsed there computes to Duration::ZERO (app.turn_started_at.map(|t| t.elapsed()).unwrap_or_default() at event_loop.rs:2120) — the time is not deferred, it's gone for good.

Net effect: pressing Esc 5 minutes into a turn makes the worked … chip visibly drop by 5 minutes on the very next frame, and that 5 minutes never comes back. This directly contradicts the PR's own framing ("ticks continuously and never jumps at TurnComplete") — it still jumps, just at cancellation instead. Consider folding the live turn into cumulative_turn_duration at the top of each of these three reset paths, the same way the TurnComplete handler does.

)
});
let worked = app
.cumulative_turn_duration

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.

Behavior mismatch: time spent parked on Waiting/Approval is counted as "worked".

turn_started_at stays Some while the phase is Waiting (parked on a sub-agent/tool) or Approval (parked on the user), so turn.unwrap_or_default() keeps growing during those phases too, and it's added into worked unconditionally here. That contradicts the doc comment three lines up ("It is model work, not wall clock… an idle TUI does not claim to have been working") and the PR description's own framing that the turn clock exists precisely to distinguish "producing tokens" from "parked... on an unanswered prompt". Concretely: a turn that sits on waiting on you for 2 hours will show worked 2h 00s even though the model did nothing — the same failure mode the PR is trying to fix for the turn half leaks into the session half. Worth excluding elapsed time accrued while phase is Waiting/Approval from the worked total (or clamping it at the moment the phase enters those states).

@codewhale-agent codewhale-agent 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.

Codewhale review

PR #5920 restores the working clock in the posture bar by adding turn and session clock halves to TidelineFooter/TidelineFooterFacts, computing them from App state, and updating the shed ladder so clocks shed before hints and counts. It fixes the corrupted en PhaseWorking string, projects separators in ASCII-safe mode, pins the filesystem-scope notice in frame tests, and adds substantial unit coverage.

Assessment

The change is well-tested and mechanically sound. The shed ladder constants and owner tests are coherent, the clock logic correctly separates turn and cumulative session time, and existing goldens/tests were updated consistently. No correctness issues found.


Advisory review by Codewhale (codewhale review --pr 5920 --post, head b8bd12c66a23d0f3cdc58ed78e5b76cf0cb8a5e1). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.

@Hmbown
Hmbown merged commit 2b490a4 into main Sep 6, 2026
35 checks passed
@Hmbown
Hmbown deleted the fix/session-working-timer-5914 branch September 6, 2026 09:42
Hmbown pushed a commit that referenced this pull request Sep 6, 2026
…#5914)

The #5950 feature commit references #5914, so the release-range audit wants a
receipt for it too. #5920 shipped the fix as a fix() commit without one.

Verified: check-feature-release-notes.sh v0.9.12 HEAD exit 0;
sync-changelog.sh --check clean.

No-Issue: release-note receipt for already-merged work.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SJrzNAmppg4vt3LNJbaeri
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
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.

TUI: overall working-time indicator no longer visible during long sessions

2 participants