Skip to content

feat(cli): live activity while busy (elapsed heartbeat + tool labels) - #55

Draft
atoomic wants to merge 1 commit into
DenizOkcu:mainfrom
atoomic:status-msg
Draft

atoomic wants to merge 1 commit into
DenizOkcu:mainfrom
atoomic:status-msg

Conversation

@atoomic

@atoomic atoomic commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Problem

The `⠧ Haze is thinking` busy indicator was a static line that never changed. During the gap between a tool finishing and the next model step emitting output — especially with slow / reasoning models, or a long blocked tool run — the UI showed no rolling activity at all, so it looked frozen even though work was in progress. Combined with the generous 5-minute idle timeout, this made it hard to tell whether Haze was stuck or still running.

Changes

  • Turn-elapsed heartbeat (`chat.tsx`): a 1s interval ticks while busy, so the busy line always shows rolling elapsed time (e.g. `⠧ Haze is thinking · 12s`) even when there is no streamed output and no tool running. All `setBusy` call sites route through a heartbeat-aware setter that stamps/clears the turn start time.
  • Tool-specific busy labels (`formatters.ts` + `streaming.ts`): a new `busyToolLabel(toolName, input)` produces a friendly phrase (e.g. `Reading src/foo.ts`, `Searching`, `Running command`). The indicator updates the instant a tool starts (`tool-input-start` / `tool-call`) and resets to `Haze is thinking` when the model resumes streaming text.
  • Tests (`tests/cli/formatters.test.ts`): added coverage for `busyToolLabel`.

Files

```
src/cli/commands/chat.tsx | 39 +++++++++++++++++++++++++++++++++------
src/cli/commands/formatters.ts | 36 ++++++++++++++++++++++++++++++++++++
src/cli/commands/streaming.ts | 5 ++++-
tests/cli/formatters.test.ts | 40 ++++++++++++++++++++++++++++++++--
```

Validation

  • `npm run typecheck` ✅
  • `npm test` ✅ (811 tests, 86 files)
  • `npm run lint` ✅

…ool labels

The busy indicator was a static 'Haze is thinking' line that never changed,
so during the gap between a tool finishing and the next model step (slow or
reasoning models, long blocked tool runs) the UI looked frozen even though it
was working. With a 5m idle timeout this was ambiguous.

- Add turn-elapsed heartbeat: a 1s interval ticks while busy so the busy line
  always shows rolling elapsed time (e.g. 'Haze is thinking · 12s') even when
  there is no streamed output and no tool running.
- Add tool-specific busy labels via busyToolLabel() so the indicator says what
  is happening (e.g. 'Reading src/foo.ts', 'Searching', 'Running command') the
  instant a tool starts, resetting to 'Haze is thinking' when the model resumes.
@atoomic
atoomic marked this pull request as draft June 30, 2026 03:20
@BabyKoan

Copy link
Copy Markdown
Contributor

PR Review — feat(cli): live activity while busy (elapsed heartbeat + tool labels)

Good UX improvement with a clean separation between formatter logic and UI heartbeat, but two real correctness issues should be fixed before merge.

  • The heartbeat wrapper setBusyWithHeartbeat correctly routes all busy state changes and the 1s interval keeps the indicator alive during stalled output.

  • busyToolLabel is a focused pure function with sensible fallbacks, placed appropriately next to toolCallSummary.

  • Tests exercise the main tool-name branches and confirm no regressions in existing formatters.

  • The busy closure captured by setBusyWithHeartbeat is stale for the lifetime of a runAgentTurn invocation, so retries reset the elapsed timer.

  • busyElapsedLabel can render · 0s on the first busy render because it accepts any positive elapsed time.

  • The mcp prefix branch in busyToolLabel appears to be dead code given how MCP tools are loaded.

  • setBusyLabel('Haze is thinking') fires on every text delta, and a few busyToolLabel branches lack tests.


🟡 Important

1. Stale `busy` closure can reset elapsed timer
src/cli/commands/chat.tsx:143-162

setBusyWithHeartbeat closes over the React state busy captured when submit() builds the callbacks object. runAgentTurn holds that same callback for the entire turn, including recursive retries, so the wrapper always sees the original busy value (usually false).

Impact: on a model retry the heartbeat resets turnStartedAtRef.current to Date.now(), so the elapsed time jumps back to zero even though the user's turn is still ongoing. It also makes the wrapper sensitive to any future caller that reuses an old callback.

Fix: track the busy flag in a ref that all closures read at call time, or compute the heartbeat start time inside runAgentTurn and pass it back through a callback instead of inferring it from state transitions.

const setBusyWithHeartbeat = (nextBusy: boolean) => {
  if (nextBusy && !busy) turnStartedAtRef.current = Date.now();
  if (!nextBusy) turnStartedAtRef.current = undefined;
  setBusy(nextBusy);
};
2. Elapsed label can flash `0s` immediately
src/cli/commands/chat.tsx:71-78

busyElapsedLabel returns a non-empty string for any elapsed > 0. Because formatElapsedTimeWhole(500) returns '0s', the first render after setBusy(true) (before the 1s heartbeat fires) will show · 0s.

Impact: the user briefly sees a confusing 0s counter right after the turn starts, contradicting the PR goal of showing meaningful rolling elapsed time.

Fix: only render once at least one whole second has elapsed, e.g. return '' when elapsed < 1000, or reuse the tick count so the label is derived from whole seconds only.

function busyElapsedLabel(startedAt: number | undefined) {
  if (startedAt == null) return '';
  const elapsed = Date.now() - startedAt;
  return elapsed > 0 ? formatElapsedTimeWhole(elapsed) : '';
}

🟢 Suggestions

1. Speculative `mcp` prefix branch may be dead code
src/cli/commands/formatters.ts:105-140

busyToolLabel handles toolName.startsWith('mcp'), but MCP tool names in this codebase are loaded verbatim from the server in src/llm/mcp.ts with no mcp prefix. Unless a downstream branch or config adds such a prefix, this branch never matches and is misleading documentation.

Impact: dead code that suggests a naming convention the codebase does not enforce.

Fix: verify whether MCP tools are ever prefixed; if not, drop the branch or map labels by tool category from toolCategories instead of string prefixes.

default:
  if (toolName.startsWith('lsp')) return 'Querying LSP';
  if (toolName.startsWith('mcp')) return 'Running MCP tool';
  return `Running ${toolName}`;
2. Busy label reset on every text delta is redundant
src/cli/commands/streaming.ts:256-272

Setting callbacks.setBusyLabel?.('Haze is thinking') inside the text-delta case runs on every streamed token. The label only needs to reset once when transitioning from a tool back to model text.

Impact: negligible performance, but unnecessary React setState churn and coupling of streaming frequency to UI label updates.

Fix: track a small lastLabelWasTool flag and reset the label only on the first text-delta after a tool event.

case 'text-delta': {
  callbacks.setBusyLabel?.('Haze is thinking');
  toolDisplay.startFreshToolGroup();
3. Incomplete `busyToolLabel` test coverage
tests/cli/formatters.test.ts:90-127

The new tests cover the common cases well, but miss several branches that are easy to regress:

  • skill and writeTasks cases are implemented but not tested.
  • readToolOutput is not handled (it falls through to Running readToolOutput); consider if it deserves a friendly label.
  • Boundary inputs (null, undefined, malformed non-object input) are not tested, even though the implementation casts input as Record<string, unknown>.

Impact: future refactors could break edge-case labels silently.

Fix: add tests for skill, writeTasks, readToolOutput, and null/undefined/malformed inputs.

describe('busyToolLabel', () => {
  it('labels bash as running a command', () => { ... });
  // ... existing cases ...
});
4. Elapsed label computed twice per render
src/cli/commands/chat.tsx:1175

The JSX calls busyElapsedLabel(turnStartedAtRef.current) once for the truthy check and again for the rendered value. The function is cheap, but duplicating the call is unnecessary and slightly noisy.

Fix: compute the label once in render scope and reuse the variable in the conditional and the JSX.

{busyElapsedLabel(turnStartedAtRef.current) ? <Text color={theme.muted} dimColor> · {busyElapsedLabel(turnStartedAtRef.current)}</Text> : null}

Checklist

  • No hardcoded secrets
  • Input validation at boundaries
  • Error handling covers failure paths
  • No unbounded collections or repeated I/O in hot paths
  • New code branches are tested — suggestion #3
  • No backward-incompatible public interface changes

To rebase specific severity levels, mention me: @BabyKoan rebase critical (fixes 🔴 only), @BabyKoan rebase important (fixes 🔴 + 🟡), or just @BabyKoan rebase for all.


Automated review by Kōan (Claude) HEAD=db16bbd 23 min 37s

@BabyKoan BabyKoan 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.

Blocking issues found.

  • Stale busy closure can reset elapsed timer
  • Elapsed label can flash 0s immediately

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.

2 participants