From 00b4d8b3cf10f6744867b1b1f9c0f716b0b61fbd Mon Sep 17 00:00:00 2001 From: Manas Raghuwanshi Date: Tue, 4 Aug 2026 17:53:43 +0530 Subject: [PATCH] feat(tui): stop the interface and the step budget fighting the user Two things made the agent unpleasant to sit in front of, and both were measured before being changed rather than argued about. The interface fought the transcript. Appending reset the scroll offset, so reading a tool result mid-turn lasted until the next streamed token. Four uncoordinated animation timers repainted the whole frame 45-60 times a second while the turn merely waited on the provider. The timeline was unwindowed, so 200 streamed tokens took 364ms at 50 items and 2018ms at 2000. The diff panel and the transcript both shrank freely, so at a 1000-item transcript the edit under review rendered zero body rows and no footer, while Enter still applied it. The budget failed work it should have paused. Twenty steps was documented as a quota guard, but the provider guards quota itself and exactly - a 429 carries a RetryInfo, and the client turns it into a message naming the quota page. A constant here cannot know what is left of anyone's budget, so as a spending limit it is always wrong in one direction; at twenty it sat inside the range of ordinary tasks and fired almost only on legitimate work, which then read as `failed`. What a ceiling honestly catches is a stuck loop with nobody watching, and that wants a number high enough to be reached rarely and a checkpoint rather than an exception when it is. The alternative for the budget was simply raising the number. That moves the cliff instead of removing it: a long task still ends in `failed` with its explanation in a status that clears after three seconds. Asking puts the decision where the information is, and is what makes a generous ceiling safe - a runaway still stops and waits. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 2 +- commands/agent.tsx | 48 ++++- commands/agentController.ts | 1 + config/types.ts | 13 ++ docs/guides/working-in-a-repository.md | 5 +- docs/introduction/why.md | 6 +- docs/reference/configuration.md | 2 +- .../tests/runtime/agentController.test.ts | 5 +- packages/tests/runtime/agentLoop.test.ts | 6 +- .../tests/runtime/iterationBudget.test.ts | 114 ++++++++-- ...cyWarning.test.ts => turnCounters.test.ts} | 94 ++++----- packages/tests/shared/mocks.ts | 19 +- runtime/loop.ts | 109 ++++++---- runtime/turnState.ts | 3 - tui/src/app.overlay.test.tsx | 56 ++++- tui/src/app.tsx | 128 +++++++---- tui/src/components/CommandBlock.tsx | 6 +- tui/src/components/ContinueTurn.tsx | 77 +++++++ tui/src/components/DiffPreview.tsx | 65 ++++-- tui/src/components/RunningGlyph.tsx | 23 ++ tui/src/components/Scrollbar.tsx | 60 ++++++ tui/src/components/StatusSpinner.tsx | 19 +- tui/src/components/ToolStatus.tsx | 12 +- tui/src/components/TurnFooter.tsx | 83 +++++--- tui/src/history.test.ts | 68 ++++++ tui/src/history.ts | 87 ++++++++ tui/src/hooks/useClock.tsx | 47 +++++ tui/src/layout.test.ts | 13 ++ tui/src/layout.ts | 9 + tui/src/prompt.tsx | 31 ++- tui/src/scrollbar.test.ts | 45 ++++ tui/src/scrollbar.ts | 52 +++++ tui/src/statusBar.test.ts | 40 ++++ tui/src/statusBar.tsx | 75 +++++-- tui/src/store/ui-store.test.ts | 126 +++++++++++ tui/src/store/ui-store.ts | 199 +++++++++++++++--- tui/src/timeline.tsx | 50 ++++- tui/src/types.ts | 18 ++ 38 files changed, 1527 insertions(+), 289 deletions(-) rename packages/tests/runtime/{efficiencyWarning.test.ts => turnCounters.test.ts} (54%) create mode 100644 tui/src/components/ContinueTurn.tsx create mode 100644 tui/src/components/RunningGlyph.tsx create mode 100644 tui/src/components/Scrollbar.tsx create mode 100644 tui/src/history.test.ts create mode 100644 tui/src/history.ts create mode 100644 tui/src/hooks/useClock.tsx create mode 100644 tui/src/scrollbar.test.ts create mode 100644 tui/src/scrollbar.ts create mode 100644 tui/src/statusBar.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 640e350..5c8518a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,7 +56,7 @@ One structural fact to know before editing: - **Approval is split in two.** `runtime/approval/classifier.ts` decides how risky a shell command is; `runtime/approval/policy.ts` decides whether that risk needs asking. Adding an approval mode is one entry in a table. -A turn: `cli.ts` → `AgentController` (owns client, model, cancellation) → `buildRepositoryContext` in `config/config.ts` (package metadata, README, agent instruction files, structure — each capped, the whole capped again) → `agentLoop` in `runtime/loop.ts` (stream, collect tool calls, execute, feed results back; 20 iterations by default) → tools resolved via `toolRegistry` in `tools/index.ts`. +A turn: `cli.ts` → `AgentController` (owns client, model, cancellation) → `buildRepositoryContext` in `config/config.ts` (package metadata, README, agent instruction files, structure — each capped, the whole capped again) → `agentLoop` in `runtime/loop.ts` (stream, collect tool calls, execute, feed results back; 40 iterations per stretch, then it asks via `onBudgetExhausted` — absent handler means nobody to ask, and exhaustion throws as before) → tools resolved via `toolRegistry` in `tools/index.ts`. Providers implement `ProviderClient` in `providers/client.ts`, whose `stream()` yields `StreamEvent`s (`text`, `tool_call`, `done`). Google, OpenAI and Anthropic are all enabled in `providers/providerRegistry.ts`. The Gemini client lives in `providers/client.ts`, the Anthropic one in `providers/anthropicClient.ts`, the OpenAI one in `providers/openaiClient.ts`; `createProviderClient` picks between them. diff --git a/commands/agent.tsx b/commands/agent.tsx index 5b3a4d4..ab8bd40 100644 --- a/commands/agent.tsx +++ b/commands/agent.tsx @@ -242,6 +242,27 @@ async function runInteractive(modelOverride?: string) { store.setStatus(status); }, + // The loop reports this every iteration and the TUI used to drop it, so + // nothing on screen said how much of the window the session was spending. + // Each provider client normalises `promptTokens` to the whole prompt — + // Anthropic's sums its three counts — so the number means the same thing + // whichever one is answering. + onUsage(iteration) { + store.setUsage(iteration.usage?.promptTokens); + }, + + // Implemented here and deliberately not in `runHeadless`: the loop treats + // an absent handler as "nobody is there to ask" and raises the budget error + // instead, which is what the headless exit codes are built on. + async onBudgetExhausted({ steps }) { + const shouldContinue = await store.setPendingContinuation({ + id: crypto.randomUUID(), + steps, + }); + + return shouldContinue ? "continue" : "stop"; + }, + onToolStart(tool) { store.finishAssistantMessage(); store.startTool(tool); @@ -363,9 +384,26 @@ async function runInteractive(modelOverride?: string) { } }; + // The app paints a full-height frame, so it belongs on the alternate screen: + // in the normal buffer it overwrites the scrollback of whatever the user was + // doing and leaves its last frame stranded there on exit. + // + // Restoring is not optional, and `handleExit` is not enough on its own — an + // uncaught throw leaves the terminal in the alternate buffer with mouse + // reporting still on, which reads as a hung shell. `restoreTerminal` is + // idempotent so the exit hook and the normal path can both call it. + let terminalRestored = false; + const restoreTerminal = () => { + if (terminalRestored || !process.stdin.isTTY) return; + terminalRestored = true; + process.stdin.off("data", onData); + process.stdout.write("\x1b[?1006l\x1b[?1000l\x1b[?1049l"); + }; + if (process.stdin.isTTY) { process.stdin.on("data", onData); - process.stdout.write("\x1b[?1000h\x1b[?1006h"); + process.stdout.write("\x1b[?1049h\x1b[?1000h\x1b[?1006h"); + process.once("exit", restoreTerminal); } const { unmount } = render( @@ -379,18 +417,18 @@ async function runInteractive(modelOverride?: string) { if (exiting) return; exiting = true; - if (process.stdin.isTTY) { - process.stdin.off("data", onData); - process.stdout.write("\x1b[?1006l\x1b[?1000l"); - } if (scrollFrame) clearTimeout(scrollFrame); store.clearPendingEdit(); store.clearPendingCommand(); store.cancelPendingQuestion(); + store.clearPendingContinuation(); controller.cancel(); await controller.dispose(); unmount(); + // After unmount, so Ink's final frame lands in the alternate buffer rather + // than in the scrollback the user is about to get back. + restoreTerminal(); process.exit(0); } diff --git a/commands/agentController.ts b/commands/agentController.ts index a269e54..6474fb8 100644 --- a/commands/agentController.ts +++ b/commands/agentController.ts @@ -346,6 +346,7 @@ export class AgentController { store.clearPendingEdit(); store.clearPendingCommand(); store.cancelPendingQuestion(); + store.clearPendingContinuation(); this.abortController?.abort(); } diff --git a/config/types.ts b/config/types.ts index 0733bd7..67db1ec 100644 --- a/config/types.ts +++ b/config/types.ts @@ -241,8 +241,21 @@ export interface ToolFailure extends ToolCall { error: string; } +/** What to do when a turn reaches its step ceiling. */ +export type BudgetDecision = "continue" | "stop"; + export interface AgentCallbacks { onStatus?(status: string): void; + /** + * The turn has used its whole budget and is not finished. Answering + * `continue` grants another budget and carries the same turn on. + * + * Optional, and the absence is meaningful rather than a default: it means + * nobody is there to ask, so the loop raises `IterationBudgetExhaustedError` + * as it always has. Headless runs deliberately do not implement it, which is + * what keeps their exit-code contract. + */ + onBudgetExhausted?(info: { steps: number }): Promise; /** Reported once per completed iteration, before the next one starts. */ onUsage?(usage: IterationUsage): void; /** Reported exactly once when the turn ends, however it ends. */ diff --git a/docs/guides/working-in-a-repository.md b/docs/guides/working-in-a-repository.md index e3b5c9d..82305c5 100644 --- a/docs/guides/working-in-a-repository.md +++ b/docs/guides/working-in-a-repository.md @@ -73,8 +73,9 @@ edit than describing the outcome. **Ask before you change.** A question costs one turn and no writes, and it tells you whether the agent has understood the project before you let it edit. -**Keep the scope to one thing.** A turn has a budget of 20 iterations. Two -unrelated changes in one prompt tends to produce a partial result for both. +**Keep the scope to one thing.** A turn works in stretches of 40 steps and asks +before taking another. Two unrelated changes in one prompt tends to mean +answering that question with neither of them finished. ## Git diff --git a/docs/introduction/why.md b/docs/introduction/why.md index 0c20c15..4ee8aac 100644 --- a/docs/introduction/why.md +++ b/docs/introduction/why.md @@ -28,9 +28,9 @@ the review. ## What it is bad at -**Large refactors across many files.** A turn is capped at 20 iterations. Broad -sweeping changes will run out of budget partway through, and you will get a -partial result rather than a clean stop. +**Large refactors across many files.** A turn works in stretches of 40 steps and +stops to ask before taking another, so a broad sweeping change means answering +that question repeatedly rather than handing the work over once. **Long autonomous runs.** There is no plan-then-execute mode and no background work. If you want to hand over a task and come back in an hour, this is the diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 7fea19b..088edb8 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -138,7 +138,7 @@ the run. | Variable | Default | Effect | | --- | --- | --- | | `WOOPCODE_PROVIDER` | `google` | Pairs with `WOOPCODE_API_KEY` | -| `WOOPCODE_MAX_ITERATIONS` | `20` | Steps the agent may take in one turn. The interactive default is deliberately low, because a human is waiting and a runaway loop spends their quota; an automated caller working one hard task wants far more | +| `WOOPCODE_MAX_ITERATIONS` | `40` | Steps a turn may take before it stops to ask whether to keep going. Interactively the ceiling is a checkpoint, so it is set to catch a stuck loop rather than to ration requests — the provider rations those itself, and answering the checkpoint grants another `40`. A headless run has nobody to ask, so this is the whole budget and exhausting it exits `2` | | `WOOPCODE_MAX_ATTEMPTS` | `3` | Tries per provider request before the error surfaces | | `WOOPCODE_TOOL_HISTORY_BUDGET` | unset (off) | Characters of tool history to keep before older results are compacted. Off by default — see the measurements in `runtime/compaction.ts` | | `WOOPCODE_THINKING_BUDGET` | `-1` | Reasoning depth; see below | diff --git a/packages/tests/runtime/agentController.test.ts b/packages/tests/runtime/agentController.test.ts index 1da0ff7..636e64b 100644 --- a/packages/tests/runtime/agentController.test.ts +++ b/packages/tests/runtime/agentController.test.ts @@ -79,7 +79,9 @@ mock.module("../../../providers/client", () => ({ createProviderClient, })); -// Mock the UI store +// Mock the UI store. Every store method the controller calls needs an entry — +// this stub replaces the module for the whole run, so a missing one fails here +// and in any other file that touches the controller. const mockStore = { addUserMessage: mock(() => {}), startTurn: mock(() => {}), @@ -94,6 +96,7 @@ const mockStore = { clearPendingEdit: mock(() => {}), clearPendingCommand: mock(() => {}), cancelPendingQuestion: mock(() => {}), + clearPendingContinuation: mock(() => {}), }; mock.module("../../../tui/src", () => ({ diff --git a/packages/tests/runtime/agentLoop.test.ts b/packages/tests/runtime/agentLoop.test.ts index 5e0b818..90f5b2f 100644 --- a/packages/tests/runtime/agentLoop.test.ts +++ b/packages/tests/runtime/agentLoop.test.ts @@ -437,10 +437,10 @@ describe("agentLoop - Iteration Limits", () => { const promise = agentLoop(dynamicClient, messages, "", callbackSpy); await expect(promise).rejects.toThrow( - "Agent exceeded the maximum number of iterations (20)", + "Agent exceeded the maximum number of iterations (40)", ); - - expect(mockTool.executionCount).toBe(20); + + expect(mockTool.executionCount).toBe(40); }); test("completes successfully within iteration limit", async () => { diff --git a/packages/tests/runtime/iterationBudget.test.ts b/packages/tests/runtime/iterationBudget.test.ts index 17d437c..a7f1f53 100644 --- a/packages/tests/runtime/iterationBudget.test.ts +++ b/packages/tests/runtime/iterationBudget.test.ts @@ -3,7 +3,11 @@ import { agentLoop, IterationBudgetExhaustedError, } from "../../../runtime/loop"; -import type { ProviderClient, StreamEvent } from "../../../config/types"; +import type { + AgentCallbacks, + ProviderClient, + StreamEvent, +} from "../../../config/types"; import { MockTool, MockToolRegistry } from "../shared/mocks"; import { createRuntimeTest } from "../shared/testHelpers"; @@ -86,21 +90,21 @@ describe("iteration budget", () => { delete process.env.WOOPCODE_MAX_ITERATIONS; const error = await runToExhaustion(); - expect(error?.message).toContain("(20)"); + expect(error?.message).toContain("(40)"); }); test("a non-numeric budget falls back to the default", async () => { process.env.WOOPCODE_MAX_ITERATIONS = "many"; const error = await runToExhaustion(); - expect(error?.message).toContain("(20)"); + expect(error?.message).toContain("(40)"); }); test("a non-positive budget falls back to the default", async () => { process.env.WOOPCODE_MAX_ITERATIONS = "0"; const error = await runToExhaustion(); - expect(error?.message).toContain("(20)"); + expect(error?.message).toContain("(40)"); }); }); @@ -111,6 +115,14 @@ describe("iteration budget", () => { * it was still starting new work at the wall, because the only notice went to * stderr through onStatus. The model cannot act on something it was never sent. */ +/** The nudge pushed into the conversation as the ceiling comes into view. */ +const budgetNotices = (messages: Array<{ role: string; content?: string }>) => + messages.filter( + (message) => + message.role === "user" && + (message.content ?? "").includes("before this turn is stopped"), + ); + describe("running out of budget", () => { /** Runs to exhaustion, keeping the transcript and the statuses. */ async function runKeepingMessages() { @@ -125,26 +137,21 @@ describe("running out of budget", () => { return { messages, callbacks }; } - const budgetNotices = (messages: Array<{ role: string; content?: string }>) => - messages.filter( - (message) => - message.role === "user" && - (message.content ?? "").includes("before this turn is stopped"), - ); - - test("the model is told, not just the terminal", async () => { + test("the model is told, and only the model", async () => { process.env.WOOPCODE_MAX_ITERATIONS = "8"; const { messages, callbacks } = await runKeepingMessages(); expect(budgetNotices(messages)).toHaveLength(1); - // The status still fires: the TUI shows it, and removing it would trade one - // audience for the other. + // The status used to fire too, back when reaching the ceiling ended the + // turn as a failure and this row was the user's only warning. The ceiling + // asks them directly now, so a transcript row saying the turn is nearly + // over is a worse version of the question they are about to be asked. const statuses = callbacks .getCallsByName("onStatus") .map((call: { args: any[] }) => String(call.args[0])); expect(statuses.some((text) => text.includes("iterations remaining"))).toBe( - true, + false, ); }); @@ -167,3 +174,80 @@ describe("running out of budget", () => { expect(budgetNotices(messages)).toHaveLength(0); }); }); + +/** + * Reaching the ceiling asks rather than fails. + * + * The distinction that matters here is between "the user said stop" and "there + * was nobody to ask". They look the same from inside the loop and mean opposite + * things: one is a turn a human ended, the other is a headless run whose exit + * code a harness reads. An absent callback is the second, and must keep + * throwing however the first behaves. + */ +describe("the budget checkpoint", () => { + /** Runs to exhaustion with a handler, capturing what the loop did. */ + async function runWithHandler( + onBudgetExhausted: AgentCallbacks["onBudgetExhausted"], + ) { + const { callbacks, messages } = createRuntimeTest(); + callbacks.onError = () => {}; + callbacks.onBudgetExhausted = onBudgetExhausted; + + let threw: unknown; + try { + await agentLoop(neverFinishingProvider(), messages, "", callbacks); + } catch (error) { + threw = error; + } + return { threw, callbacks, messages }; + } + + test("with nobody to ask, exhaustion is still an error", async () => { + process.env.WOOPCODE_MAX_ITERATIONS = "2"; + + // No handler at all — the headless case, whose exit code depends on this. + const error = await runToExhaustion(); + expect(error).toBeInstanceOf(IterationBudgetExhaustedError); + }); + + test("continuing carries the same turn past the original ceiling", async () => { + process.env.WOOPCODE_MAX_ITERATIONS = "8"; + + let asked = 0; + const { messages } = await runWithHandler(async () => { + asked += 1; + // Continue once, then stop, so the test ends rather than looping forever. + return asked === 1 ? "continue" : "stop"; + }); + + expect(asked).toBe(2); + // Once for the first eight steps, once for the eight the checkpoint added. + // The warning tracking the extension is how the model learns the second + // stretch is also finite. + expect(budgetNotices(messages)).toHaveLength(2); + }); + + test("the handler is told how many steps have been taken", async () => { + process.env.WOOPCODE_MAX_ITERATIONS = "3"; + + const seen: number[] = []; + await runWithHandler(async ({ steps }) => { + seen.push(steps); + return seen.length === 1 ? "continue" : "stop"; + }); + + expect(seen).toEqual([3, 6]); + }); + + test("stopping ends the turn as a cancellation, not a failure", async () => { + process.env.WOOPCODE_MAX_ITERATIONS = "2"; + + const { threw, callbacks } = await runWithHandler(async () => "stop"); + + // Nothing thrown is the point: the controller marks a turn `error` from a + // raised exception, and this turn did not fail — it was halted. + expect(threw).toBeUndefined(); + expect(callbacks.getCallsByName("onCancel")).toHaveLength(1); + expect(callbacks.getCallsByName("onError")).toHaveLength(0); + }); +}); diff --git a/packages/tests/runtime/efficiencyWarning.test.ts b/packages/tests/runtime/turnCounters.test.ts similarity index 54% rename from packages/tests/runtime/efficiencyWarning.test.ts rename to packages/tests/runtime/turnCounters.test.ts index 5e815e6..9a625b1 100644 --- a/packages/tests/runtime/efficiencyWarning.test.ts +++ b/packages/tests/runtime/turnCounters.test.ts @@ -1,5 +1,10 @@ import { describe, test, expect, beforeEach, mock } from "bun:test"; -import type { Message, ProviderClient, StreamEvent } from "../../../config/types"; +import type { + Message, + ProviderClient, + StreamEvent, + TurnSummary, +} from "../../../config/types"; import { MockTool, MockToolRegistry, CallbackSpy } from "../shared/mocks"; import { createUserMessage, @@ -53,7 +58,19 @@ function warnings(spy: CallbackSpy): string[] { .filter((status) => status.startsWith("⚠️")); } -describe("efficiency warning counts tools, not iterations", () => { +function summary(spy: CallbackSpy) { + const calls = spy.getCallsByName("onTurnSummary"); + return calls.at(-1)?.args[0] as TurnSummary | undefined; +} + +/** + * These used to assert a notice that fired at the sixth tool call. It was + * removed: unlike the iteration-budget warning it sat beside, it pushed nothing + * into the conversation, so the advice it gave — start implementing — reached + * only the user, who is not the one deciding what to call next. What remains + * are the counters underneath it, which the turn summary still reports. + */ +describe("a turn counts the tools it actually ran", () => { let callbackSpy: CallbackSpy; let messages: Message[]; @@ -64,35 +81,17 @@ describe("efficiency warning counts tools, not iterations", () => { mockToolRegistry.register(new MockTool("read_file", "contents")); }); - test("fires on the sixth tool even when they arrive in one response", async () => { - // The old counter would report "1 tool used" here. + test("counts every call in a response, not the response", async () => { + // Six calls arriving together are six tools run. A counter reading the + // iteration would call this one. const client = new ScriptedClient([toolCalls(6)]); await agentLoop(client, messages, "", callbackSpy); - const notices = warnings(callbackSpy).filter((text) => text.includes("tools used")); - expect(notices).toHaveLength(1); - expect(notices[0]).toContain("6 tools used"); - }); - - test("does not fire when the sixth response ran no tool", async () => { - // Five tool calls, then the model answers. The old counter warned "6 tools - // used" at the start of the sixth iteration, before any sixth tool existed. - const client = new ScriptedClient([ - toolCalls(1, 0), - toolCalls(1, 1), - toolCalls(1, 2), - toolCalls(1, 3), - toolCalls(1, 4), - [createTextEvent("finished"), createDoneEvent()], - ]); - - await agentLoop(client, messages, "", callbackSpy); - - expect(warnings(callbackSpy).filter((text) => text.includes("tools used"))).toHaveLength(0); + expect(summary(callbackSpy)?.toolCalls).toBe(6); }); - test("counts tools across iterations and reports the running total", async () => { + test("accumulates across iterations", async () => { const client = new ScriptedClient([ toolCalls(4, 0), toolCalls(3, 4), @@ -101,26 +100,12 @@ describe("efficiency warning counts tools, not iterations", () => { await agentLoop(client, messages, "", callbackSpy); - const notices = warnings(callbackSpy).filter((text) => text.includes("tools used")); - expect(notices).toHaveLength(1); - expect(notices[0]).toContain("7 tools used"); - }); - - test("reports at most once per turn", async () => { - const client = new ScriptedClient([ - toolCalls(6, 0), - toolCalls(6, 6), - [createTextEvent("finished"), createDoneEvent()], - ]); - - await agentLoop(client, messages, "", callbackSpy); - - expect(warnings(callbackSpy).filter((text) => text.includes("tools used"))).toHaveLength(1); + expect(summary(callbackSpy)?.toolCalls).toBe(7); }); test("skipped duplicate calls do not count as tools used", async () => { - // Eight identical calls: four run, the rest are skipped by the duplicate - // guard, so the total stays below the warning threshold. + // Eight identical calls. SAME_TOOL_THRESHOLD lets two through and the + // duplicate guard skips the rest. const repeated: StreamEvent[] = [ ...Array.from({ length: 8 }, (_, index) => createToolCallEvent("read_file", { path: "same.ts" }, `call-${index}`), @@ -131,19 +116,30 @@ describe("efficiency warning counts tools, not iterations", () => { await agentLoop(client, messages, "", callbackSpy); - expect(warnings(callbackSpy).filter((text) => text.includes("tools used"))).toHaveLength(0); + // Two ran; the rest never reached a tool, so counting them would report + // work the turn did not do. + expect(summary(callbackSpy)?.toolCalls).toBe(2); }); - test("the iteration-budget notice still tracks iterations", async () => { - // Fifteen tool-calling responses reach the iteration milestone. + test("the iteration-budget notice reaches the model, not the terminal", async () => { + // Thirty-five tool-calling responses reach the milestone five from the end + // of the default budget. const client = new ScriptedClient( - Array.from({ length: 15 }, (_, index) => toolCalls(1, index)), + Array.from({ length: 35 }, (_, index) => toolCalls(1, index)), ); await agentLoop(client, messages, "", callbackSpy); - const notices = warnings(callbackSpy).filter((text) => text.includes("iterations remaining")); - expect(notices).toHaveLength(1); - expect(notices[0]).toContain("5 iterations remaining"); + // It is a nudge for the model — stop starting new work — and the user is + // asked about the ceiling directly rather than warned about it here. See + // packages/tests/runtime/iterationBudget.test.ts for both halves. + expect(warnings(callbackSpy).filter((t) => t.includes("iterations remaining"))).toHaveLength(0); + expect( + messages.filter( + (message) => + message.role === "user" && + (message.content ?? "").includes("before this turn is stopped"), + ), + ).toHaveLength(1); }); }); diff --git a/packages/tests/shared/mocks.ts b/packages/tests/shared/mocks.ts index 7893be8..6bc0ce7 100644 --- a/packages/tests/shared/mocks.ts +++ b/packages/tests/shared/mocks.ts @@ -1,4 +1,9 @@ -import type { ProviderClient, StreamEvent, Tool } from "../../../config/types"; +import type { + AgentCallbacks, + ProviderClient, + StreamEvent, + Tool, +} from "../../../config/types"; /** * Mock Provider Client for testing @@ -164,6 +169,18 @@ export class CallbackSpy { this.calls.push({ name: "onCancel", args: [] }); }; + /** + * Declared, and deliberately left undefined. + * + * Unlike every other callback here, this one's absence carries meaning: the + * loop reads a missing handler as "nobody is there to ask" and raises the + * budget error, which is the behaviour headless runs and most of these tests + * depend on. Giving it a default implementation would quietly turn every + * exhaustion test into a continuation test. Tests that want the checkpoint + * assign it themselves. + */ + onBudgetExhausted?: AgentCallbacks["onBudgetExhausted"]; + getCallsByName(name: string) { return this.calls.filter((call) => call.name === name); } diff --git a/runtime/loop.ts b/runtime/loop.ts index b0e4498..f0e4098 100644 --- a/runtime/loop.ts +++ b/runtime/loop.ts @@ -142,8 +142,23 @@ export function renderContext(context: TurnContext): string { return [instructions, repository, executionLog].filter(Boolean).join("\n\n"); } -/** Loop budget when nothing overrides it — tuned for interactive use. */ -const DEFAULT_MAX_ITERATIONS = 20; +/** + * Steps a turn may take before it stops to ask whether to keep going. + * + * Not a quota guard, though it was written as one. The provider enforces quota + * itself and exactly: a 429 carries a `RetryInfo` saying when to come back, + * `providerRetryDelayMs` honours it, and the client turns it into a message + * naming the quota page. A constant here cannot know what is left of anyone's + * budget, so as a spending limit it is always either too tight or too loose. + * + * What it does guard is a pathological loop with nobody watching — an agent + * re-reading the same three files until the day's requests are gone. That wants + * a ceiling high enough that ordinary work never reaches it. Twenty was not: + * turns were dying mid-edit, having done nothing wrong, and the number fired + * almost only on the false positive. Reaching this one asks rather than fails + * (see `onBudgetExhausted`), which is what makes it safe to be generous. + */ +const DEFAULT_MAX_ITERATIONS = 40; /** Conversation turns kept in the window sent to the provider. */ const MAX_TURNS = 6; @@ -162,9 +177,6 @@ const SAME_TOOL_THRESHOLD = 2; /** Iterations left when the model is told the budget is running out. */ const REMAINING_ITERATIONS_WARNING = 5; -/** Tools actually run before the turn is nudged toward implementing. */ -const TOOLS_BEFORE_EFFICIENCY_WARNING = 6; - /** * Asked once, never twice. The model may have a good reason not to verify — * the change may be unverifiable, or the tests may not exist — and a loop that @@ -194,12 +206,12 @@ export class IterationBudgetExhaustedError extends Error { } /** - * Resolves the loop budget, allowing `WOOPCODE_MAX_ITERATIONS` to raise it. + * Resolves the loop budget, allowing `WOOPCODE_MAX_ITERATIONS` to set it. * - * The interactive default is deliberately small: a human is watching, and a - * runaway loop spends their quota. Automated callers working a single hard - * task have the opposite tradeoff and need a far larger budget, so the limit - * has to be settable from outside rather than compiled in. + * An interactive session can afford a checkpoint at the ceiling, because + * somebody is there to answer it. An automated caller cannot — there is nobody + * to ask, so the number it starts with is the number it gets — which is why the + * limit has to be settable from outside rather than compiled in. */ function maxIterations(env: Record = process.env): number { const raw = env.WOOPCODE_MAX_ITERATIONS?.trim(); @@ -585,7 +597,12 @@ export async function agentLoop( useTools = true, options: AgentLoopOptions = {}, ) { - const MAX_ITERATIONS = maxIterations(); + // The budget for one stretch of work, and the amount each checkpoint grants. + // Mutable because a turn the user chooses to continue is the same turn: the + // history, the model's reasoning and the footer's clock all carry on, and + // nothing has to be re-established. + const BUDGET_STEP = maxIterations(); + let budget = BUDGET_STEP; const planMode = options.planMode === true; // Withholding the writing tools is the first of plan mode's two gates. The // second is the refusal below, which is what covers a write reaching the disk @@ -601,21 +618,20 @@ export async function agentLoop( const state = new TurnState(); try { - while (state.iterations < MAX_ITERATIONS) { + while (state.iterations < budget) { state.iterations++; - // This one is about the loop budget, so the iteration counter is the - // right measure. + // Said to the model and to nobody else. A benchmark trial that exhausted + // its 200 iterations was still writing at its 198th tool call, because + // this warning only ever reached stderr — a status callback cannot change + // what the model does next, and a message in the conversation can. // - // Said to the model as well as to the terminal. A benchmark trial that - // exhausted its 200 iterations was still writing at its 198th tool call, - // because this warning only ever reached stderr. A status callback cannot - // change what the model does next; a message in the conversation can. - if (state.iterations === MAX_ITERATIONS - REMAINING_ITERATIONS_WARNING) { - const remaining = MAX_ITERATIONS - state.iterations; - callbacks.onStatus?.( - `⚠️ ${remaining} iterations remaining - prioritize completion`, - ); + // It used to be shown to the user as well, back when reaching the ceiling + // ended the turn as a failure and a warning was the only notice they got. + // Now the ceiling asks them directly, so a row saying the turn is nearly + // over is a worse version of a question they are about to be asked. + if (state.iterations === budget - REMAINING_ITERATIONS_WARNING) { + const remaining = budget - state.iterations; messages.push({ role: "user", content: @@ -687,7 +703,7 @@ export async function agentLoop( callbacks, state, assistantText, - MAX_ITERATIONS, + budget, truncated, ); @@ -727,24 +743,39 @@ export async function agentLoop( } } - // Reported once, after the tools of this iteration have run, so the - // count is what was actually used rather than how many times the model - // has been asked to respond. - if ( - !state.efficiencyWarningSent && - state.toolCallsExecuted >= TOOLS_BEFORE_EFFICIENCY_WARNING - ) { - state.efficiencyWarningSent = true; - // The ⚠️ prefix is load-bearing: it is how the UI tells an informational - // notice from a terminal status, so it shows in the transcript and the - // activity indicator keeps saying the turn is still running. - callbacks.onStatus?.( - `⚠️ ${state.toolCallsExecuted} tools used - start implementing now to conserve quota`, - ); + // The budget is spent and the turn is still working. Ask before ending + // it: the work so far is on disk either way, and whether to spend more is + // the user's call rather than this constant's. + // + // Inside the loop rather than after it, so answering `continue` re-enters + // the same `while` with a raised ceiling instead of restarting anything. + if (state.iterations >= budget) { + // No handler means nobody is there to answer, which is not the same as + // an answer of `stop`. Headless runs rely on this: they never implement + // it, so exhaustion stays the error their exit code is built on. + if (!callbacks.onBudgetExhausted) break; + + const decision = await callbacks.onBudgetExhausted({ + steps: state.iterations, + }); + + // No separate abort check: cancelling resolves an open checkpoint as + // `stop`, so Ctrl+C arrives here as the answer below. + if (decision === "stop") { + // Reported as a cancellation because that is what it is: the user + // stopped a turn that was still going. It also means the turn footer + // reads `cancelled` rather than `failed` — the controller sets that + // from this callback — which is the honest word for work that was + // halted rather than broken. + callbacks.onCancel?.(); + return ""; + } + + budget += BUDGET_STEP; } } - throw new IterationBudgetExhaustedError(MAX_ITERATIONS); + throw new IterationBudgetExhaustedError(budget); } catch (error) { if (signal?.aborted) { callbacks.onCancel?.(); diff --git a/runtime/turnState.ts b/runtime/turnState.ts index 1e2548b..f7c2d1a 100644 --- a/runtime/turnState.ts +++ b/runtime/turnState.ts @@ -25,9 +25,6 @@ export class TurnState { */ toolCallsExecuted = 0; - /** Set once the efficiency notice has been sent, so it is sent only once. */ - efficiencyWarningSent = false; - /** Provider requests retried. Reported for the turn so a slow run reads differently from a flaky one. */ retries = 0; diff --git a/tui/src/app.overlay.test.tsx b/tui/src/app.overlay.test.tsx index 111d8a0..818d11c 100644 --- a/tui/src/app.overlay.test.tsx +++ b/tui/src/app.overlay.test.tsx @@ -209,9 +209,8 @@ function pendingEdit(id: string) { describe("a chord never answers the diff", () => { beforeEach(() => { store.clearTimeline(); - // The diff shares the screen with the transcript, and an empty timeline - // renders the home screen instead — where there is no DiffPreview to press - // a key at. + // An empty timeline renders the home screen, where there is no DiffPreview + // to press a key at. store.addUserMessage(TRANSCRIPT); }); @@ -276,3 +275,54 @@ describe("a chord never answers the diff", () => { app.unmount(); }); }); + +/** + * The diff used to split the screen with the transcript, both halves free to + * shrink. Measured at 80x30 with a 200-line diff pending, a 200-item transcript + * left the diff two rows and a 1000-item one left it none: no diff body, and no + * footer either, while Enter still applied the edit. Approving a write with + * nothing on screen to judge is the failure that layout allowed, and only a + * rendered frame can catch it — every unit underneath was behaving correctly. + */ +describe("the diff owns the screen while it is being judged", () => { + beforeEach(() => { + store.clearTimeline(); + }); + + function longEdit() { + const body = Array.from({ length: 200 }, (_, line) => `+added line ${line}`); + return { + id: "long", + filePath: "runtime/loop.ts", + oldContent: "before", + newContent: "after", + diff: ["--- runtime/loop.ts", "+++ runtime/loop.ts", "@@ -1 +1 @@", ...body].join("\n"), + toolCallId: "call-long", + }; + } + + test("shows the change and how to answer it, whatever is behind it", async () => { + for (const transcriptLength of [1, 200, 1000]) { + store.clearTimeline(); + for (let index = 0; index < transcriptLength; index += 1) { + store.addUserMessage(`message ${index}`); + } + + const app = mount(); + const decision = store.setPendingEdit(longEdit()); + await settle(); + + const frame = app.stdout.text(); + expect(frame).toContain("runtime/loop.ts"); + // The body, not just the header that names the file. + expect(frame).toContain("added line 0"); + // And the way out, which went missing at the same time the body did. + expect(frame).toContain("reject"); + expect(frame).toContain("apply"); + + store.rejectPendingEdit(); + await expect(decision).resolves.toBe(false); + app.unmount(); + } + }); +}); diff --git a/tui/src/app.tsx b/tui/src/app.tsx index e040477..03643ad 100644 --- a/tui/src/app.tsx +++ b/tui/src/app.tsx @@ -1,4 +1,4 @@ -import { Box, measureElement, type DOMElement } from "ink"; +import { Box, Text, measureElement, type DOMElement } from "ink"; import { Header } from "./header"; import { Timeline } from "./timeline"; import { ConnectedStatusBar } from "./statusBar"; @@ -10,14 +10,17 @@ import { DiffPreview } from "./components/DiffPreview"; import { ModelPicker } from "./components/ModelPicker"; import { ApprovalPicker } from "./components/ApprovalPicker"; import { CommandApproval } from "./components/CommandApproval"; +import { ContinueTurn } from "./components/ContinueTurn"; import { QuestionDialog } from "./components/QuestionDialog"; import type { AgentController } from "../../commands/agentController"; import type { ActiveTurn, TimeLineItem } from "./types"; -import { useEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState, type ReactNode } from "react"; import { useTerminalSize } from "./hooks/useTerminalSize"; import { useCancelKey } from "./hooks/useCancelKey"; import { planLayout } from "./layout"; -import { PaletteProvider } from "./styles/palette"; +import { PaletteProvider, usePalette } from "./styles/palette"; +import { ClockProvider } from "./hooks/useClock"; +import { Scrollbar } from "./components/Scrollbar"; import { getModelDisplayName } from "../../providers/client"; import { matchCommands } from "../../commands/slash/match"; @@ -39,13 +42,15 @@ export function App({ controller, onExit, homeScreen }: AppProps) { const hasPendingEdit = state.pendingEdit !== null; const hasPendingCommand = state.pendingCommand !== null; const hasPendingQuestion = state.pendingQuestion !== null; + const hasPendingContinuation = state.pendingContinuation !== null; // These float over the app rather than replacing it, so the work behind them - // stays readable. The diff preview is not one of them: it splits the screen. + // stays readable. The diff preview is not one of them: it takes the screen. const dialogOpen = state.modelPickerOpen || state.approvalPickerOpen || hasPendingCommand || - hasPendingQuestion; + hasPendingQuestion || + hasPendingContinuation; // Registered here because App is the only component that is always mounted. // Every modal below replaces the composer, which is where this used to live. @@ -71,6 +76,9 @@ export function App({ controller, onExit, homeScreen }: AppProps) { height={height} backgroundColor="#000000" > + {/* One interval drives every animation below. See useClock for what the + four independent timers this replaced were costing. */} + {/* Everything behind a dialog renders in the faded palette, so the panel above reads as the foreground instead of the app going black. */} @@ -90,28 +98,20 @@ export function App({ controller, onExit, homeScreen }: AppProps) { )} /> ) : hasPendingEdit ? ( - /* Split layout: Timeline on top, Diff below */ - - - - {/* Diff preview - takes remaining space */} - - - + /* The whole region, not a share of it. This used to split the screen + with the transcript, and both halves were free to shrink: measured + at 80x30 with a 200-line diff, a 200-item transcript left the diff + two rows and a 1000-item one left it none at all — no diff body and + no Esc/Enter footer, while the keys still worked. Approving a write + with nothing on screen to judge is the failure that layout allowed. */ + + ) : ( + {/* Costs a row only while it has something to say, which is why it + can sit above a composer whose height is budgeted to the row. */} + {state.scrollOffset > 0 && ( + + )} {layout.showStatusBar && } @@ -162,12 +167,34 @@ export function App({ controller, onExit, homeScreen }: AppProps) { ) : hasPendingCommand ? ( + ) : hasPendingContinuation ? ( + ) : ( )} )} + + + ); +} + +/** + * Says the transcript is not showing its latest line. + * + * Necessary because the view now stays where the user put it: without this, a + * transcript that has stopped following looks identical to one that has stopped + * receiving. + */ +function ScrolledAwayHint() { + const colors = usePalette(); + + return ( + + ↑ scrolled · + End + to jump to latest ); } @@ -199,6 +226,9 @@ function ConversationViewport({ }: ConversationViewportProps) { const viewportRef = useRef(null); const contentRef = useRef(null); + // Kept for the scrollbar, which needs both heights to size a thumb. The + // measurement was already being taken; only the second half was thrown away. + const [measured, setMeasured] = useState({ content: 0, viewport: 0 }); const measurementTimer = useRef | undefined>(undefined); @@ -214,6 +244,11 @@ function ConversationViewport({ const viewportHeight = measureElement(viewportRef.current).height; const contentHeight = measureElement(contentRef.current).height; store.setScrollLimit(contentHeight - viewportHeight); + setMeasured((current) => + current.content === contentHeight && current.viewport === viewportHeight + ? current + : { content: contentHeight, viewport: viewportHeight }, + ); }, 75); }, [updateKey, layoutKey]); @@ -225,23 +260,34 @@ function ConversationViewport({ ); return ( - - + - + + + + + {/* This viewport is bottom-anchored — its offset counts up from the last + line — so it is converted here to the distance from the top the + scrollbar wants. */} + 0} + /> ); } diff --git a/tui/src/components/CommandBlock.tsx b/tui/src/components/CommandBlock.tsx index 3d611e3..5b3b393 100644 --- a/tui/src/components/CommandBlock.tsx +++ b/tui/src/components/CommandBlock.tsx @@ -1,6 +1,6 @@ import { Box, Text } from "ink"; -import Spinner from "ink-spinner"; import { usePalette } from "../styles/palette"; +import { RunningGlyph } from "./RunningGlyph"; /** * How many lines of output a block shows before it stops. Long enough for a @@ -55,9 +55,7 @@ export function CommandBlock({ command, output, status }: CommandBlockProps) { > {status === "running" ? ( - - - + ) : ( // A refused command never ran, so it does not get the `$` that says // one did. diff --git a/tui/src/components/ContinueTurn.tsx b/tui/src/components/ContinueTurn.tsx new file mode 100644 index 0000000..2fcdc83 --- /dev/null +++ b/tui/src/components/ContinueTurn.tsx @@ -0,0 +1,77 @@ +import { Box, Text, useInput } from "ink"; +import type { PendingContinuation } from "../types"; +import { store } from "../store/ui-store"; +import { usePalette } from "../styles/palette"; +import { planLayout } from "../layout"; +import { useTerminalSize } from "../hooks/useTerminalSize"; + +/** + * The checkpoint a turn reaches when it has spent its step budget. + * + * This used to be an error: the loop threw, the footer said `failed`, and the + * explanation went to a status that cleared after three seconds — for a turn + * that had not broken and whose edits were already on disk. Asking instead puts + * the decision where the information is, and is what lets the ceiling be + * generous without letting a stuck loop run unwatched. + */ +export function ContinueTurn({ continuation }: { continuation: PendingContinuation }) { + const colors = usePalette(); + const { width, height } = useTerminalSize(); + const layout = planLayout(width, height); + + useInput((input, key) => { + // Ink reports a chord's letter as plain input with a modifier flag, so + // Ctrl+C would otherwise read as "continue" here — and Ctrl+C means stop + // the agent, which useCancelKey owns. Only an unmodified letter answers. + const letter = key.ctrl || key.meta ? "" : input.toLowerCase(); + + if (key.return || letter === "c") { + store.continuePendingTurn(); + } else if (key.escape || letter === "s") { + store.stopPendingTurn(); + } + }); + + return ( + + {/* Shaped like the command approval: title row, the thing being decided, + then the keys. */} + + + + Still working + + esc + + + + + {`${continuation.steps} steps used, and the turn is not finished.`} + + {/* Said plainly because the old failure implied otherwise: nothing is + lost by stopping here. */} + + Work done so far is already saved either way. + + + + {layout.showDialogHints && ( + + + Esc stop here + + + Enter keep going + + + )} + + + ); +} diff --git a/tui/src/components/DiffPreview.tsx b/tui/src/components/DiffPreview.tsx index 7cd90a7..39cca0c 100644 --- a/tui/src/components/DiffPreview.tsx +++ b/tui/src/components/DiffPreview.tsx @@ -1,11 +1,13 @@ import { Box, measureElement, Text, type DOMElement, useInput } from "ink"; -import { useEffect, useRef } from "react"; +import { useEffect, useRef, useState } from "react"; import type { PendingEdit } from "../types"; import { DiffViewer } from "./DiffViewer"; +import { Scrollbar } from "./Scrollbar"; import { store } from "../store/ui-store"; import { useUIStore } from "../store/useUIStore"; import { usePalette } from "../styles/palette"; import { useTerminalSize } from "../hooks/useTerminalSize"; +import { planLayout } from "../layout"; interface DiffPreviewProps { pendingEdit: PendingEdit; @@ -16,20 +18,31 @@ export function DiffPreview({ pendingEdit }: DiffPreviewProps) { const { pendingEditScrollOffset } = useUIStore(); const { width, height } = useTerminalSize(); + const layout = planLayout(width, height); const viewportRef = useRef(null); const contentRef = useRef(null); + const [measured, setMeasured] = useState({ content: 0, viewport: 0 }); useEffect(() => { + // Coalesced the way the conversation viewport's measurement is, rather than + // taken once on a bare timeout: the panel is re-laid out by anything that + // changes the rows around it, and a limit measured before that settles + // leaves the diff either unscrollable or scrollable into blank space. const timer = setTimeout(() => { if (!viewportRef.current || !contentRef.current) return; const viewportHeight = measureElement(viewportRef.current).height; const contentHeight = measureElement(contentRef.current).height; store.setPendingEditScrollLimit(contentHeight - viewportHeight); - }, 0); + setMeasured((current) => + current.content === contentHeight && current.viewport === viewportHeight + ? current + : { content: contentHeight, viewport: viewportHeight }, + ); + }, 75); return () => clearTimeout(timer); - }, [pendingEdit.id, pendingEdit.diff, width, height]); + }, [pendingEdit.id, pendingEdit.diff, pendingEdit.filePath, width, height]); useInput((input, key) => { if (key.upArrow) { @@ -122,27 +135,47 @@ export function DiffPreview({ pendingEdit }: DiffPreviewProps) { −{deletions} - + - + + + + {/* This viewport scrolls top-down, so its offset is already the + distance from the top — unlike the transcript's. */} + {/* Always painted here, unlike the transcript's. A diff is a thing + being judged rather than a stream being followed, and how much of + it is still below the fold is part of the judgement. */} + - Esc reject · ↑↓ scroll + Esc reject + {layout.showDialogHints && ( + <> + {" · "} + ↑↓ PgUp/PgDn scroll + + )} Enter apply diff --git a/tui/src/components/RunningGlyph.tsx b/tui/src/components/RunningGlyph.tsx new file mode 100644 index 0000000..d5ae5fa --- /dev/null +++ b/tui/src/components/RunningGlyph.tsx @@ -0,0 +1,23 @@ +import { Text } from "ink"; +import { usePalette } from "../styles/palette"; +import { useClock } from "../hooks/useClock"; + +/** + * The braille cycle `ink-spinner` drew, on the app's shared clock. + * + * Same frames as `cli-spinners`' "dots", one third of a turn slower — the + * shared clock ticks at 100ms where that spinner ran its own 80ms timer. What + * it buys is that N running tools cost one commit between them instead of N + * timers each repainting the whole frame on its own schedule. + * + * Isolated in its own component so only the animated glyph re-renders on a + * tick, not the row that contains it. + */ +const FRAMES = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"] as const; + +export function RunningGlyph() { + const colors = usePalette(); + const frame = useClock(); + + return {FRAMES[frame % FRAMES.length]}; +} diff --git a/tui/src/components/Scrollbar.tsx b/tui/src/components/Scrollbar.tsx new file mode 100644 index 0000000..aed6523 --- /dev/null +++ b/tui/src/components/Scrollbar.tsx @@ -0,0 +1,60 @@ +import { Box, Text } from "ink"; +import { usePalette } from "../styles/palette"; +import { scrollbarThumb } from "../scrollbar"; + +interface ScrollbarProps { + contentHeight: number; + viewportHeight: number; + /** Rows between the top of the content and the first visible row. */ + offsetFromTop: number; + /** + * False while the viewport is pinned to the end of its content. + * + * A painted glyph on every row of the frame is not free: the gutter is + * redrawn with each frame of a running turn, and measured, always painting it + * took the app from 16KB of terminal output per three seconds to 50KB — + * three times, while the agent works and the frame is changing anyway. Where + * it earns that is when the reader has moved away from the end and needs to + * know where they are; pinned to the latest line, the answer is "at the + * bottom", which the transcript is already showing them. + * + * The column is reserved either way, so appearing costs no reflow. + */ + active: boolean; +} + +/** A one-column gutter saying where you are in something taller than the window. */ +export function Scrollbar({ + contentHeight, + viewportHeight, + offsetFromTop, + active, +}: ScrollbarProps) { + const colors = usePalette(); + const thumb = scrollbarThumb(contentHeight, viewportHeight, offsetFromTop); + + // The column is held open even with nothing to draw in it. Trailing blanks + // cost nothing to write, and the transcript keeps its width when the bar + // arrives rather than reflowing under the reader mid-scroll. + if (!thumb || !active) { + return ; + } + + const below = viewportHeight - thumb.start - thumb.size; + + // The thumb has to be the brighter of the two, so `borderMuted` is the track + // and not the thumb — `borderStrong` is darker than `textFaint` and would + // draw the bar inside out. + return ( + + {thumb.start > 0 && {stack("│", thumb.start)}} + {stack("▐", thumb.size)} + {below > 0 && {stack("│", below)}} + + ); +} + +/** `count` rows of one glyph, as a single string. */ +function stack(glyph: string, count: number) { + return Array.from({ length: count }, () => glyph).join("\n"); +} diff --git a/tui/src/components/StatusSpinner.tsx b/tui/src/components/StatusSpinner.tsx index 1e51160..30e61f2 100644 --- a/tui/src/components/StatusSpinner.tsx +++ b/tui/src/components/StatusSpinner.tsx @@ -1,11 +1,14 @@ import { Box, Text } from "ink"; -import { useEffect, useState } from "react"; import { useDimmed } from "../styles/palette"; import { dimHex } from "../styles/theme"; +import { useClock } from "../hooks/useClock"; const TRACK_LENGTH = 8; -const HOLD_AT_START = 30; -const HOLD_AT_END = 9; +// Scaled to the shared 100ms clock from the 60ms interval this used to own, so +// the sweep still takes about the same three seconds end to end. The holds are +// where the rounding goes; the travel is fixed by the track. +const HOLD_AT_START = 18; +const HOLD_AT_END = 5; const TOTAL_FRAMES = TRACK_LENGTH + HOLD_AT_END + (TRACK_LENGTH - 1) + HOLD_AT_START; @@ -88,19 +91,11 @@ function inactiveColor( } export function StatusSpinner() { - const [frame, setFrame] = useState(0); + const frame = useClock() % TOTAL_FRAMES; // The spinner carries its own ramp, so it fades itself. const dimmed = useDimmed(); const shade = (color: string) => (dimmed ? dimHex(color) : color); - useEffect(() => { - const interval = setInterval(() => { - setFrame((current) => (current + 1) % TOTAL_FRAMES); - }, 60); - - return () => clearInterval(interval); - }, []); - return ( diff --git a/tui/src/components/ToolStatus.tsx b/tui/src/components/ToolStatus.tsx index a747e9b..c022686 100644 --- a/tui/src/components/ToolStatus.tsx +++ b/tui/src/components/ToolStatus.tsx @@ -1,6 +1,6 @@ -import { Box, Text } from "ink"; -import Spinner from "ink-spinner"; +import { Text } from "ink"; import { usePalette } from "../styles/palette"; +import { RunningGlyph } from "./RunningGlyph"; interface ToolStatusProps { status: "running" | "completed" | "failed" | "blocked"; @@ -11,13 +11,7 @@ interface ToolStatusProps { export function ToolStatus({ status, glyph }: ToolStatusProps) { const colors = usePalette(); - if (status === "running") { - return ( - - - - ); - } + if (status === "running") return ; // A completed call is a record, not an outcome to celebrate: the glyph says // what kind of work it was and stays out of the way. Only failure stands out. diff --git a/tui/src/components/TurnFooter.tsx b/tui/src/components/TurnFooter.tsx index 1a293b7..101f3e9 100644 --- a/tui/src/components/TurnFooter.tsx +++ b/tui/src/components/TurnFooter.tsx @@ -1,6 +1,6 @@ import { Box, Text } from "ink"; -import { useEffect, useState } from "react"; import { usePalette } from "../styles/palette"; +import { useClock } from "../hooks/useClock"; import type { Palette } from "../styles/theme"; import { getModelDisplayName } from "../../../providers/client"; import type { TurnIdentity, TurnOutcome } from "../types"; @@ -9,11 +9,13 @@ import { sessionModeLabel } from "../../../runtime/planMode"; import { useTerminalSize } from "../hooks/useTerminalSize"; /** - * Fast enough that the tenths digit reads as a running clock, slow enough that - * it costs a fraction of what streaming tokens already cost per second. + * Frames of the shared 100ms clock between pulse steps. + * + * Two rather than the 240ms this used to run at: the clock cannot be divided + * into 240, and the elapsed time has to update every frame anyway to keep the + * tenths digit honest, so the pulse costs nothing extra at 200ms. */ -const TICK_INTERVAL_MS = 100; -const PULSE_INTERVAL_MS = 240; +const PULSE_EVERY_FRAMES = 2; /** Breathes the marker while the turn is in flight. */ const pulseColors = ["#453B82", "#7263CE", "#8F83E0", "#ACA3EC", "#8F83E0", "#7263CE"] as const; @@ -53,40 +55,59 @@ interface TurnFooterProps extends TurnIdentity { outcome: TurnOutcome | null; } -export function TurnFooter({ +/** + * Splits on whether the turn is still running, and the split is load-bearing. + * + * Only a running footer may subscribe to the clock. Context does not respect + * `memo`, so a finished footer that read the clock would re-render on every + * tick — and a long session holds hundreds of them, which is the cost the + * shared clock exists to remove. + */ +export function TurnFooter(props: TurnFooterProps) { + if (props.endedAt === null) return ; + + return ( + + ); +} + +function RunningTurnFooter(props: TurnFooterProps) { + const frame = useClock(); + + // Read at render rather than held in state: the clock already re-renders this + // component, so a second copy of "what time is it" would only be one more + // thing to keep in step. + return ( + + ); +} + +function TurnFooterRow({ agent, model, - startedAt, endedAt, outcome, -}: TurnFooterProps) { + elapsed, + pulseFrame, +}: TurnFooterProps & { elapsed: number; pulseFrame: number | null }) { const colors = usePalette(); - - const running = endedAt === null; - const [now, setNow] = useState(() => Date.now()); - const [pulse, setPulse] = useState(0); const { width, height } = useTerminalSize(); const layout = planLayout(width, height); - useEffect(() => { - if (!running) return; - - const clock = setInterval(() => setNow(Date.now()), TICK_INTERVAL_MS); - const breathing = setInterval( - () => setPulse((frame) => (frame + 1) % pulseColors.length), - PULSE_INTERVAL_MS, - ); - - return () => { - clearInterval(clock); - clearInterval(breathing); - }; - }, [running]); - - const elapsed = (endedAt ?? now) - startedAt; - const markerColor = running - ? pulseColors[pulse] - : outcomeColor(outcome ?? "completed", colors, agent); + const markerColor = + pulseFrame === null + ? outcomeColor(outcome ?? "completed", colors, agent) + : pulseColors[ + Math.floor(pulseFrame / PULSE_EVERY_FRAMES) % pulseColors.length + ]; // One row, always. Wrapping this split "Build · Gemini 2.5 Flash Lite · 3.5s" // across three lines in a narrow terminal; the model name gives up columns and diff --git a/tui/src/history.test.ts b/tui/src/history.test.ts new file mode 100644 index 0000000..8a6c2fd --- /dev/null +++ b/tui/src/history.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, test } from "bun:test"; +import { PromptHistory } from "./history"; + +describe("prompt history", () => { + test("walks back through what was submitted, newest first", () => { + const history = new PromptHistory(); + history.push("first"); + history.push("second"); + + expect(history.previous("")).toBe("second"); + expect(history.previous("")).toBe("first"); + }); + + test("stops at the oldest entry rather than wrapping round", () => { + const history = new PromptHistory(); + history.push("only"); + + expect(history.previous("")).toBe("only"); + expect(history.previous("")).toBeNull(); + }); + + test("gives back the half-typed line at the bottom of the walk", () => { + const history = new PromptHistory(); + history.push("run the tests"); + + expect(history.previous("what I was typ")).toBe("run the tests"); + expect(history.next()).toBe("what I was typ"); + expect(history.isWalking()).toBe(false); + }); + + test("↓ does nothing when the composer is showing the user's own text", () => { + const history = new PromptHistory(); + history.push("something"); + + expect(history.next()).toBeNull(); + }); + + test("submitting ends the walk, so ↑ starts from the newest again", () => { + const history = new PromptHistory(); + history.push("first"); + history.push("second"); + history.previous(""); + history.previous(""); + + history.push("third"); + expect(history.isWalking()).toBe(false); + expect(history.previous("")).toBe("third"); + }); + + test("ignores blank submissions and immediate repeats", () => { + const history = new PromptHistory(); + history.push("same"); + history.push(" "); + history.push("same"); + + expect(history.previous("")).toBe("same"); + expect(history.previous("")).toBeNull(); + }); + + test("is empty until something is submitted, so ↑ can fall back to scrolling", () => { + const history = new PromptHistory(); + expect(history.isEmpty()).toBe(true); + expect(history.previous("")).toBeNull(); + + history.push("now it is not"); + expect(history.isEmpty()).toBe(false); + }); +}); diff --git a/tui/src/history.ts b/tui/src/history.ts new file mode 100644 index 0000000..a1e9654 --- /dev/null +++ b/tui/src/history.ts @@ -0,0 +1,87 @@ +/** + * The prompts submitted this session, and where ↑ has walked to in them. + * + * Session-scoped and never written to disk. Prompts routinely contain the + * contents of whatever the user was looking at, so persisting them would put + * fragments of one project's code in front of the next; the shells this borrows + * its keys from make that choice too and get it wrong often enough to be a + * known hazard. + * + * Pure, with no React in it, because the interesting part is the walk: an index + * that runs off either end, or forgets where it was when the user types, is a + * bug you can only see by pressing keys in an order nobody tests by hand. + */ +export class PromptHistory { + private entries: string[] = []; + /** + * How far back ↑ has walked. -1 means "not walking" — the composer is showing + * what the user typed rather than a recalled entry. + */ + private cursor = -1; + /** What was in the composer when the walk started, restored by walking back off the end. */ + private draft = ""; + + /** Records a submitted prompt and ends any walk in progress. */ + push(prompt: string) { + const value = prompt.trim(); + this.reset(); + + if (value === "") return; + // A prompt repeated immediately is one entry: pressing ↑ twice to reach the + // one before it is the behaviour every shell has. + if (this.entries.at(-1) === value) return; + + this.entries.push(value); + } + + /** + * The previous prompt, or null when there is nothing older to show. + * + * `current` is kept so that walking back down past the newest entry returns + * the half-typed line the user abandoned rather than an empty composer. + */ + previous(current: string): string | null { + if (this.entries.length === 0) return null; + + if (this.cursor === -1) { + this.draft = current; + this.cursor = this.entries.length - 1; + return this.entries[this.cursor] ?? null; + } + + if (this.cursor === 0) return null; + + this.cursor -= 1; + return this.entries[this.cursor] ?? null; + } + + /** The next prompt down, or the abandoned draft at the bottom of the walk. */ + next(): string | null { + if (this.cursor === -1) return null; + + if (this.cursor >= this.entries.length - 1) { + const draft = this.draft; + this.reset(); + return draft; + } + + this.cursor += 1; + return this.entries[this.cursor] ?? null; + } + + /** True while ↑ is showing a recalled entry rather than the user's own text. */ + isWalking() { + return this.cursor !== -1; + } + + /** Whether ↑ has anything to offer at all. Empty history falls back to scrolling. */ + isEmpty() { + return this.entries.length === 0; + } + + /** Ends the walk, leaving whatever is in the composer alone. */ + reset() { + this.cursor = -1; + this.draft = ""; + } +} diff --git a/tui/src/hooks/useClock.tsx b/tui/src/hooks/useClock.tsx new file mode 100644 index 0000000..168d1dc --- /dev/null +++ b/tui/src/hooks/useClock.tsx @@ -0,0 +1,47 @@ +import { createContext, useContext, useEffect, useState, type ReactNode } from "react"; + +/** + * One interval for every animation in the app. + * + * Four used to run independently — the status spinner at 60ms, the turn + * footer's clock at 100ms and its pulse at 240ms, and one `ink-spinner` per + * running tool at 80ms. Each drove its own `setState`, and Ink repaints the + * whole frame per commit, so a turn merely waiting on the provider measured + * 45-60 full repaints a second and 32KB of terminal writes every three. That is + * the flicker, and none of it was buying anything: three glyphs were moving. + * + * Sharing the tick means one commit animates all of them. + */ +const TICK_MS = 100; + +const ClockContext = createContext(0); + +export function ClockProvider({ children }: { children: ReactNode }) { + const [frame, setFrame] = useState(0); + + useEffect(() => { + const interval = setInterval(() => { + // Wraps at a large multiple of every consumer's cycle length, so no + // consumer sees its sequence jump at the wrap. + setFrame((current) => (current + 1) % 3600); + }, TICK_MS); + + return () => clearInterval(interval); + }, []); + + return {children}; +} + +/** + * The current frame, advancing every 100ms. + * + * Components that want a slower cycle divide it rather than starting a timer: + * `Math.floor(frame / 2)` is the old 200ms, and so on. A component that wants a + * *faster* one cannot have it, which is the point. + */ +export function useClock() { + return useContext(ClockContext); +} + +/** Milliseconds between frames, for consumers converting a duration to frames. */ +export const CLOCK_TICK_MS = TICK_MS; diff --git a/tui/src/layout.test.ts b/tui/src/layout.test.ts index 550c8ce..8967b98 100644 --- a/tui/src/layout.test.ts +++ b/tui/src/layout.test.ts @@ -14,6 +14,19 @@ describe("layout planning", () => { expect(layout.rhythm).toBe(3); }); + test("drops the context meter before the keys that say what a press does", () => { + // The meter reports; the hints instruct. On a status bar too narrow for + // both, the one that cannot change what happens next is the one to lose. + expect(planLayout(72, 30).showContextMeter).toBe(true); + expect(planLayout(71, 30).showContextMeter).toBe(false); + expect(planLayout(71, 30).showKeyHints).toBe(true); + + for (let width = 20; width <= 120; width++) { + const layout = planLayout(width, 30); + if (layout.showContextMeter) expect(layout.showKeyHints).toBe(true); + } + }); + test("swaps the wordmark for the compact mark before it can clip", () => { // The figlet is 73 columns; anything narrower must not render it. expect(planLayout(FULL_WORDMARK_COLUMNS + 4, 30).wordmark).toBe("full"); diff --git a/tui/src/layout.ts b/tui/src/layout.ts index fa33c0b..00eeed4 100644 --- a/tui/src/layout.ts +++ b/tui/src/layout.ts @@ -31,6 +31,12 @@ const CAPABILITIES_MIN_ROWS = 20; const HOME_FOOTER_MIN_ROWS = 12; const STATUS_BAR_MIN_ROWS = 12; const KEY_HINTS_MIN_COLUMNS = 56; +/** + * The status bar carries the path, the meter and three hints. Below this the + * meter is what goes: it is the only one of the three that says nothing about + * what a keystroke will do. + */ +const CONTEXT_METER_MIN_COLUMNS = 72; const COMPOSER_PROVIDER_MIN_COLUMNS = 60; const HEADER_TAGLINE_MIN_COLUMNS = 48; const HEADER_META_MIN_COLUMNS = 30; @@ -60,6 +66,8 @@ export interface LayoutPlan { showHomeFooter: boolean; showStatusBar: boolean; showKeyHints: boolean; + /** The status bar's prompt-size meter; dropped before the key hints are. */ + showContextMeter: boolean; /** The composer's "Build · model · provider" line drops the provider first. */ showComposerProvider: boolean; /** Header's " / coding agent" tagline. */ @@ -166,6 +174,7 @@ export function planLayout(width: number, height: number): LayoutPlan { showHomeFooter: height >= HOME_FOOTER_MIN_ROWS, showStatusBar, showKeyHints: width >= KEY_HINTS_MIN_COLUMNS, + showContextMeter: width >= CONTEXT_METER_MIN_COLUMNS, showComposerProvider: width >= COMPOSER_PROVIDER_MIN_COLUMNS, showHeaderTagline: width >= HEADER_TAGLINE_MIN_COLUMNS, showHeaderMeta: width >= HEADER_META_MIN_COLUMNS, diff --git a/tui/src/prompt.tsx b/tui/src/prompt.tsx index f78fb21..ebc07ae 100644 --- a/tui/src/prompt.tsx +++ b/tui/src/prompt.tsx @@ -10,6 +10,7 @@ import { useUIStore } from "./store/useUIStore"; import { sessionModeColor, sessionModeLabel } from "../../runtime/planMode"; import { usePalette } from "./styles/palette"; import { CommandPreview } from "./components/CommandPreview"; +import { PromptHistory } from "./history"; import { planLayout } from "./layout"; import { useTerminalSize } from "./hooks/useTerminalSize"; @@ -53,6 +54,10 @@ export function Prompt({ const { width, height } = useTerminalSize(); const layout = planLayout(width, height); const lastActivityTime = useRef(Date.now()); + // A ref rather than state: the composer re-mounts when the layout swaps the + // block variant for the inline one, and history that reset on a resize would + // be worse than none. + const history = useRef(new PromptHistory()); useEffect(() => { const BLINK_INTERVAL = 530; @@ -107,12 +112,29 @@ export function Prompt({ return; } } else { + // ↑/↓ recall prompts, which is what they do in every shell the user + // reached this terminal through. Transcript scrolling keeps PgUp/PgDn, + // Home/End and the wheel. + // + // With nothing submitted yet there is nothing to recall, so they fall + // through to scrolling rather than doing nothing at all — the first + // keystroke of a session should not be a no-op. if (key.upArrow) { - store.scrollUp(); + const recalled = history.current.previous(value); + if (recalled === null) { + if (history.current.isEmpty()) store.scrollUp(); + return; + } + handleValueChange(recalled); return; } if (key.downArrow) { - store.scrollDown(); + if (!history.current.isWalking()) { + if (history.current.isEmpty()) store.scrollDown(); + return; + } + const recalled = history.current.next(); + if (recalled !== null) handleValueChange(recalled); return; } if (key.pageUp) { @@ -171,6 +193,11 @@ export function Prompt({ } } + // Recorded here rather than on entry: everything above either opens a + // picker or completes a half-typed command back into the composer, and + // neither is a prompt the user would want ↑ to bring back. + history.current.push(prompt); + // 🔥 Slash command interception const context = { controller, diff --git a/tui/src/scrollbar.test.ts b/tui/src/scrollbar.test.ts new file mode 100644 index 0000000..a2fefcb --- /dev/null +++ b/tui/src/scrollbar.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test } from "bun:test"; +import { scrollbarThumb } from "./scrollbar"; + +describe("scrollbar thumb", () => { + test("says nothing when the content already fits", () => { + expect(scrollbarThumb(10, 10, 0)).toBeNull(); + expect(scrollbarThumb(4, 10, 0)).toBeNull(); + }); + + test("says nothing when there is no viewport to draw in", () => { + expect(scrollbarThumb(100, 0, 0)).toBeNull(); + }); + + test("touches the top when nothing is scrolled and the bottom when all of it is", () => { + const top = scrollbarThumb(100, 10, 0); + expect(top?.start).toBe(0); + + const bottom = scrollbarThumb(100, 10, 90); + expect(bottom).not.toBeNull(); + expect(bottom!.start + bottom!.size).toBe(10); + }); + + test("sits proportionally between the two", () => { + const half = scrollbarThumb(100, 10, 45); + expect(half).not.toBeNull(); + expect(half!.start).toBeGreaterThan(0); + expect(half!.start + half!.size).toBeLessThan(10); + }); + + test("keeps a thumb visible when the proportion rounds to nothing", () => { + // 4000 rows of diff in a 12-row window: the exact proportion is 0.036 of a + // row. Rounded down that is no thumb at all, which reads as "this fits". + const thumb = scrollbarThumb(4000, 12, 0); + expect(thumb?.size).toBe(1); + }); + + test("never runs off the end of the track", () => { + for (const offset of [-50, 0, 33, 90, 500]) { + const thumb = scrollbarThumb(100, 10, offset); + expect(thumb).not.toBeNull(); + expect(thumb!.start).toBeGreaterThanOrEqual(0); + expect(thumb!.start + thumb!.size).toBeLessThanOrEqual(10); + } + }); +}); diff --git a/tui/src/scrollbar.ts b/tui/src/scrollbar.ts new file mode 100644 index 0000000..3c67cca --- /dev/null +++ b/tui/src/scrollbar.ts @@ -0,0 +1,52 @@ +/** + * Where the thumb goes, given what is scrolled and by how much. + * + * Pure and separate from the component because it is the part that can be + * wrong in a way nobody notices: a thumb that renders is not a thumb that + * points at the right rows, and the arithmetic is the same for the transcript + * and for the diff even though the two scroll in opposite directions. + */ + +export interface Thumb { + /** Rows between the top of the track and the top of the thumb. */ + start: number; + /** Rows the thumb covers. At least one, so it is never invisible. */ + size: number; +} + +/** + * The thumb for a viewport, or null when everything already fits. + * + * `offsetFromTop` is how far the first visible row is below the first row of + * the content. The transcript stores its offset the other way round — it is + * bottom-anchored, so its offset counts up from the *last* line — and converts + * at the call site rather than here, because the conversion is a fact about + * that viewport rather than about scrollbars. + */ +export function scrollbarThumb( + contentHeight: number, + viewportHeight: number, + offsetFromTop: number, +): Thumb | null { + if (viewportHeight <= 0) return null; + if (contentHeight <= viewportHeight) return null; + + // Proportional, then floored to one row: on a long diff in a short window the + // exact proportion rounds to zero, and a scrollbar with no thumb says the + // content fits — the opposite of what is true. + const size = Math.max( + 1, + Math.round((viewportHeight * viewportHeight) / contentHeight), + ); + + const scrollable = contentHeight - viewportHeight; + const travel = viewportHeight - size; + const progress = Math.min(1, Math.max(0, offsetFromTop / scrollable)); + + // Rounding the ends rather than the middle: the thumb must touch the top when + // nothing is scrolled and the bottom when everything is, or the bar implies + // there is more to reach when there is not. + const start = Math.round(progress * travel); + + return { start: Math.min(start, travel), size }; +} diff --git a/tui/src/statusBar.test.ts b/tui/src/statusBar.test.ts new file mode 100644 index 0000000..07e972f --- /dev/null +++ b/tui/src/statusBar.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, test } from "bun:test"; +import { formatContextMeter } from "./statusBar"; +import { DEFAULT_MODEL_ID, findModel } from "../../providers/modelCatalog"; + +/** + * The meter reads a model out of the catalog, so it is tested against the + * catalog rather than an invented window — a percentage is only meaningful + * against the real denominator, and that number lives in models.json. + */ +describe("the context meter", () => { + const contextWindow = findModel(DEFAULT_MODEL_ID)!.contextWindow; + + test("reports the prompt against the window it has to fit in", () => { + const meter = formatContextMeter(contextWindow / 2, DEFAULT_MODEL_ID); + + expect(meter).toContain("50%"); + expect(meter).toContain(`/${formatted(contextWindow)}`); + }); + + test("rounds an empty context to nothing rather than hiding it", () => { + expect(formatContextMeter(0, DEFAULT_MODEL_ID)).toContain("0%"); + }); + + test("says nothing at all for a model the catalog does not know", () => { + // The alternative is a confident percentage against a guessed window, which + // is worse than no meter: it reads exactly like a measurement. + expect(formatContextMeter(50_000, "some-unreleased-model")).toBeUndefined(); + expect(formatContextMeter(50_000, null)).toBeUndefined(); + }); + + test("never claims more than a full window", () => { + expect(formatContextMeter(contextWindow * 3, DEFAULT_MODEL_ID)).toContain("100%"); + }); + + function formatted(tokens: number) { + return tokens >= 1_000_000 + ? `${Number((tokens / 1_000_000).toFixed(1))}M` + : `${Number((tokens / 1_000).toFixed(1))}K`; + } +}); diff --git a/tui/src/statusBar.tsx b/tui/src/statusBar.tsx index 6d9b0b0..9e3f6ed 100644 --- a/tui/src/statusBar.tsx +++ b/tui/src/statusBar.tsx @@ -5,6 +5,7 @@ import { StatusSpinner } from "./components/StatusSpinner"; import { useTerminalSize } from "./hooks/useTerminalSize"; import { planLayout, truncateStart } from "./layout"; import { sessionModeColor } from "../../runtime/planMode"; +import { findModel, formatContextWindow } from "../../providers/modelCatalog"; const workspacePath = process.cwd().replace(process.env.HOME ?? "", "~"); @@ -21,6 +22,8 @@ interface StatusBarProps { labelWidth?: number; /** Plan mode: the Tab hint points the other way and turns amber. */ planning?: boolean; + /** Prompt size against the model's window, e.g. "48k/1M · 5%". Empty until measured. */ + context?: string; } export function StatusBar({ @@ -29,6 +32,7 @@ export function StatusBar({ showKeyHints = true, labelWidth, planning = false, + context, }: StatusBarProps) { const colors = usePalette(); @@ -38,19 +42,24 @@ export function StatusBar({ - {showKeyHints && ( - - {/* Named after what the key leads to, not the state it leaves: "tab - plan" while building, "tab build" while planning. Amber only while - planning — in Build this is one hint among three and should not - outrank them. */} - - tab {planning ? "build" : "plan"} - - ↑↓ scroll - ctrl+c - - )} + + {context && ( + {context} + )} + {showKeyHints && ( + <> + {/* Named after what the key leads to, not the state it leaves: "tab + plan" while building, "tab build" while planning. Amber only while + planning — in Build this is one hint among three and should not + outrank them. */} + + tab {planning ? "build" : "plan"} + + ↑↓ history + ctrl+c + + )} + ); } @@ -115,23 +124,57 @@ function StatusLabel({ * and "tab plan", two for its gap. Left short, the workspace path would run into * the hints on a narrow terminal instead of being truncated before them. */ -const HINTS_AND_ICON_COLUMNS = 38; +const HINTS_AND_ICON_COLUMNS = 39; + +/** Columns "48k/1M · 5%" and its gap need before the path has to give way. */ +const CONTEXT_METER_COLUMNS = 16; + +/** + * The prompt against the window it has to fit in. + * + * The percentage is the point rather than the raw count: 48k means nothing + * without knowing whether the model holds 200k or a million, and the number a + * user acts on — clear the conversation, or carry on — is the fraction. + * + * Exported for its own test. It reads a model that may not be in the catalog, + * which is the case it has to get right: an unknown model has no window, and + * inventing one would put a confident percentage on a guess. + */ +export function formatContextMeter( + promptTokens: number, + modelId: string | null, +): string | undefined { + const contextWindow = modelId ? findModel(modelId)?.contextWindow : undefined; + if (!contextWindow) return undefined; + + const percent = Math.min(100, Math.round((promptTokens / contextWindow) * 100)); + + return `${formatContextWindow(promptTokens)}/${formatContextWindow(contextWindow)} · ${percent}%`; +} export function ConnectedStatusBar() { - const { status, sessionMode } = useUIStore(); + const { status, sessionMode, usage, selectedModel } = useUIStore(); const { width, height } = useTerminalSize(); const layout = planLayout(width, height); const { state, message } = parseStatus(status); + const context = + usage && layout.showContextMeter + ? formatContextMeter(usage.promptTokens, selectedModel) + : undefined; + return ( ); diff --git a/tui/src/store/ui-store.test.ts b/tui/src/store/ui-store.test.ts index 2c90db7..0aaee13 100644 --- a/tui/src/store/ui-store.test.ts +++ b/tui/src/store/ui-store.test.ts @@ -35,6 +35,72 @@ describe("UIStore conversation scrolling", () => { store.setScrollLimit(4); expect(store.getState().scrollOffset).toBe(4); }); + + /** + * The transcript used to reset to the bottom whenever anything was appended, + * so reading a tool result mid-turn lasted until the next streamed token. The + * offset is measured from the last line, so holding a position means moving + * the number as the content grows — these cover both directions of that. + */ + test("holds the reader's place while the turn keeps appending", () => { + const store = new UIStore(); + store.setScrollLimit(20); + store.pageUp(); + expect(store.getState().scrollOffset).toBe(8); + + // Six rows of tool output land below what is being read. + store.setScrollLimit(26); + expect(store.getState().scrollOffset).toBe(14); + + store.startTool({ id: "t1", name: "read_file", arguments: {} }); + store.appendAssistantText("more"); + expect(store.getState().scrollOffset).toBe(14); + }); + + test("follows the latest line while the reader is at the bottom", () => { + const store = new UIStore(); + store.setScrollLimit(20); + expect(store.getState().scrollOffset).toBe(0); + + store.setScrollLimit(40); + expect(store.getState().scrollOffset).toBe(0); + expect(store.isFollowing()).toBe(true); + }); + + test("scrolling back to the bottom starts following again", () => { + const store = new UIStore(); + store.setScrollLimit(10); + store.pageUp(); + expect(store.isFollowing()).toBe(false); + + store.pageDown(); + expect(store.isFollowing()).toBe(true); + + store.setScrollLimit(30); + expect(store.getState().scrollOffset).toBe(0); + }); + + test("submitting a prompt returns to the latest line", () => { + const store = new UIStore(); + store.setScrollLimit(20); + store.scrollToTop(); + expect(store.getState().scrollOffset).toBe(20); + + store.addUserMessage("what changed?"); + expect(store.getState().scrollOffset).toBe(0); + + store.setScrollLimit(28); + expect(store.getState().scrollOffset).toBe(0); + }); + + test("reports how far the transcript can scroll, for the scrollbar", () => { + const store = new UIStore(); + store.setScrollLimit(17); + expect(store.getState().maxScrollOffset).toBe(17); + + store.setScrollLimit(-3); + expect(store.getState().maxScrollOffset).toBe(0); + }); }); describe("UIStore edit approvals", () => { @@ -141,6 +207,66 @@ describe("UIStore headless mode", () => { await expect(store.setPendingEdit(edit)).resolves.toBe(false); await expect(store.setPendingCommand(command)).resolves.toBe(false); }); + + test("never continues a turn on an absent user's behalf", async () => { + const store = new UIStore(); + // Even here, where every other approval is granted: continuing is the one + // answer that lets a stuck loop run unattended, which is the situation the + // step ceiling exists for. + store.setNonInteractive({ autoApprove: true }); + + await expect( + store.setPendingContinuation({ id: "cont-1", steps: 40 }), + ).resolves.toBe(false); + expect(store.getState().pendingContinuation).toBeNull(); + }); +}); + +describe("UIStore turn continuation", () => { + const continuation = { id: "cont-1", steps: 40 }; + + test("resolves true when the user chooses to keep going", async () => { + const store = new UIStore(); + const decision = store.setPendingContinuation(continuation); + + expect(store.getState().pendingContinuation).toEqual(continuation); + + store.continuePendingTurn(); + await expect(decision).resolves.toBe(true); + expect(store.getState().pendingContinuation).toBeNull(); + }); + + test("resolves false when the user stops the turn", async () => { + const store = new UIStore(); + const decision = store.setPendingContinuation(continuation); + + store.stopPendingTurn(); + await expect(decision).resolves.toBe(false); + }); + + /** + * Dismissing has to resolve, not just close. The loop is awaiting this + * promise, so a dialog that vanished without answering would leave the turn + * suspended with no way back to it. + */ + test("dismissing the dialog stops the turn rather than hanging it", async () => { + const store = new UIStore(); + const decision = store.setPendingContinuation(continuation); + + expect(store.hasOpenModal()).toBe(true); + expect(store.dismissTopModal()).toBe(true); + + await expect(decision).resolves.toBe(false); + expect(store.hasOpenModal()).toBe(false); + }); + + test("cancelling the session resolves a checkpoint left open", async () => { + const store = new UIStore(); + const decision = store.setPendingContinuation(continuation); + + store.clearPendingContinuation(); + await expect(decision).resolves.toBe(false); + }); }); describe("UIStore turn footer", () => { diff --git a/tui/src/store/ui-store.ts b/tui/src/store/ui-store.ts index ee5bd87..ce27bfd 100644 --- a/tui/src/store/ui-store.ts +++ b/tui/src/store/ui-store.ts @@ -1,4 +1,4 @@ -import type { Listener, PendingCommand, PendingEdit, PendingQuestion, TimeLineItem, TurnIdentity, TurnOutcome, UIState } from "../types"; +import type { Listener, PendingCommand, PendingContinuation, PendingEdit, PendingQuestion, TimeLineItem, TurnIdentity, TurnOutcome, UIState } from "../types"; import type { TodoItem, ToolCall } from "../../../config/types"; import { DEFAULT_APPROVAL_MODE, type ApprovalMode } from "../../../runtime/approval"; import { nextSessionMode, type SessionMode } from "../../../runtime/planMode"; @@ -23,20 +23,34 @@ export class UIStore { pendingEdit: null, pendingCommand: null, pendingQuestion: null, + pendingContinuation: null, pendingEditScrollOffset: 0, scrollOffset: 0, + maxScrollOffset: 0, + maxPendingEditScrollOffset: 0, + usage: null, }; private listeners: Set = new Set(); private activeAssistantId: string | null = null; private pendingEmit = false; - private maxScrollOffset = 0; - private maxPendingEditScrollOffset = 0; + /** + * Whether the transcript is pinned to its latest line. + * + * Appending used to reset `scrollOffset` to 0 outright, so scrolling up to + * read a tool result mid-turn survived exactly until the next streamed token + * yanked the view back down. The offset is now left alone while the user is + * reading, and only `setScrollLimit` — the one place that learns how much the + * content grew — moves it, to hold the same lines on screen. + */ + private follow = true; private editResolvers: Map< string, { resolve: (approved: boolean) => void; reject: (error: Error) => void } > = new Map(); private commandResolvers: Map void }> = new Map(); private questionResolvers: Map void }> = new Map(); + private continuationResolvers: Map void }> = + new Map(); private nonInteractive = false; private nonInteractiveApproval = false; private statusResetTimer: ReturnType | null = null; @@ -84,9 +98,12 @@ export class UIStore { } addUserMessage(content: string) { + // The one append that does jump to the bottom: submitting a prompt is a + // statement that you want to watch what it does. + this.follow = true; this.state = { ...this.state, - scrollOffset: 0, // Reset scroll on new message + scrollOffset: 0, timeline: [ ...this.state.timeline, { @@ -109,11 +126,30 @@ export class UIStore { this.state = { ...this.state, activeTurn: { id: crypto.randomUUID(), ...turn }, + // Cleared rather than carried over: the meter reports the prompt the + // *current* turn is sending, and a stale number beside a new turn reads + // as a measurement of it. + usage: null, }; this.emit(); } + /** + * Records what the last provider request cost. + * + * Only the prompt side is kept. That is the number the context window is + * spent on and the one a user can act on by clearing the conversation; + * completion tokens are gone the moment they are rendered. + */ + setUsage(promptTokens: number | undefined) { + if (promptTokens === undefined) return; + if (this.state.usage?.promptTokens === promptTokens) return; + + this.state = { ...this.state, usage: { promptTokens } }; + this.emit(); + } + /** * Freezes the footer into the timeline at the position it reached, so the * final elapsed time stays readable in scrollback while the next turn starts @@ -162,7 +198,6 @@ export class UIStore { startTool(tool: ToolCall) { this.state = { ...this.state, - scrollOffset: 0, // Reset scroll timeline: [ ...this.state.timeline, { @@ -275,7 +310,6 @@ export class UIStore { this.state = { ...this.state, - scrollOffset: 0, timeline: [ ...withoutPrevious, { id: crypto.randomUUID(), type: "todo", items }, @@ -291,7 +325,6 @@ export class UIStore { this.state = { ...this.state, - scrollOffset: 0, // Reset scroll timeline: [ ...this.state.timeline, { @@ -418,11 +451,11 @@ export class UIStore { return Promise.resolve(this.nonInteractiveApproval); } - this.maxPendingEditScrollOffset = 0; this.state = { ...this.state, pendingEdit: edit, pendingEditScrollOffset: 0, + maxPendingEditScrollOffset: 0, }; this.emit(); @@ -553,15 +586,66 @@ export class UIStore { this.emit(); } + /** + * Asks whether a turn that has spent its budget should carry on. + * + * Declines rather than blocking when nobody is at the keyboard. Answering + * "continue" for an absent user is the one wrong answer available: it is the + * case where a stuck loop runs unattended, which is the whole reason a + * ceiling exists. + */ + setPendingContinuation(continuation: PendingContinuation): Promise { + if (this.nonInteractive) { + return Promise.resolve(false); + } + + this.state = { ...this.state, pendingContinuation: continuation }; + this.emit(); + + return new Promise((resolve) => { + this.continuationResolvers.set(continuation.id, { resolve }); + }); + } + + continuePendingTurn() { + this.resolvePendingContinuation(true); + } + + stopPendingTurn() { + this.resolvePendingContinuation(false); + } + + /** Dismissed without an answer — the turn stops, as with any other modal. */ + clearPendingContinuation() { + this.resolvePendingContinuation(false); + } + + private resolvePendingContinuation(shouldContinue: boolean) { + const continuation = this.state.pendingContinuation; + if (!continuation) return; + + this.continuationResolvers.get(continuation.id)?.resolve(shouldContinue); + this.continuationResolvers.delete(continuation.id); + this.state = { ...this.state, pendingContinuation: null }; + this.emit(); + } + /** True when a modal owns the screen, so global keys can tell. */ hasOpenModal() { - const { modelPickerOpen, approvalPickerOpen, pendingCommand, pendingQuestion, pendingEdit } = - this.state; + const { + modelPickerOpen, + approvalPickerOpen, + pendingCommand, + pendingQuestion, + pendingContinuation, + pendingEdit, + } = this.state; return ( modelPickerOpen || approvalPickerOpen || pendingCommand !== null || pendingQuestion !== null || + pendingContinuation !== null || pendingEdit !== null ); } @@ -589,6 +673,10 @@ export class UIStore { this.cancelPendingQuestion(); return true; } + if (this.state.pendingContinuation) { + this.clearPendingContinuation(); + return true; + } if (this.state.pendingEdit) { this.rejectPendingEdit(); return true; @@ -600,11 +688,11 @@ export class UIStore { this.clearPendingEdit(); this.clearPendingCommand(); this.cancelPendingQuestion(); + this.clearPendingContinuation(); // This sets the status directly, so a pending reset would be redundant at // best and would fire over a later status at worst. this.clearStatusReset(); - this.maxScrollOffset = 0; - this.maxPendingEditScrollOffset = 0; + this.follow = true; this.state = { ...this.state, timeline: [], @@ -615,7 +703,11 @@ export class UIStore { pendingEditScrollOffset: 0, pendingCommand: null, pendingQuestion: null, + pendingContinuation: null, scrollOffset: 0, + maxScrollOffset: 0, + maxPendingEditScrollOffset: 0, + usage: null, }; this.activeAssistantId = null; this.emit(); @@ -634,9 +726,14 @@ export class UIStore { const scrollOffset = Math.max( 0, - Math.min(this.state.scrollOffset + lines, this.maxScrollOffset), + Math.min(this.state.scrollOffset + lines, this.state.maxScrollOffset), ); + // Reaching the bottom re-arms the follow, leaving it disarms it. Set even + // when the offset did not move, so pressing ↓ at the bottom of a transcript + // that has grown while the user was reading puts them back in follow. + this.follow = scrollOffset === 0; + if (scrollOffset === this.state.scrollOffset) return; this.state = { @@ -655,24 +752,49 @@ export class UIStore { } scrollToTop() { + this.follow = false; this.state = { ...this.state, - scrollOffset: this.maxScrollOffset, + scrollOffset: this.state.maxScrollOffset, }; this.emit(); } + /** + * Records how far the transcript can scroll, and holds the reader's place. + * + * This is the only place that learns the content grew, so it is where a view + * scrolled away from the bottom is kept still: the offset is measured from + * the last line, so rows landing below would otherwise push what the user is + * reading off the top by exactly the amount the content grew. Adding that + * amount back keeps the same lines on screen, without the store needing to + * know the height of anything it appended. + */ setScrollLimit(maxScrollOffset: number) { - this.maxScrollOffset = Math.max(0, maxScrollOffset); - const scrollOffset = Math.min(this.state.scrollOffset, this.maxScrollOffset); - - if (scrollOffset === this.state.scrollOffset) return; + const limit = Math.max(0, maxScrollOffset); + const previousLimit = this.state.maxScrollOffset; + const grew = Math.max(0, limit - previousLimit); + + const held = this.follow ? 0 : this.state.scrollOffset + grew; + const scrollOffset = Math.max(0, Math.min(held, limit)); + + if ( + scrollOffset === this.state.scrollOffset && + limit === this.state.maxScrollOffset + ) { + return; + } - this.state = { ...this.state, scrollOffset }; + this.state = { ...this.state, scrollOffset, maxScrollOffset: limit }; this.emit(); } - resetScroll() { + /** Back to the latest line, and following it again. */ + scrollToBottom() { + this.follow = true; + + if (this.state.scrollOffset === 0) return; + this.state = { ...this.state, scrollOffset: 0, @@ -680,6 +802,16 @@ export class UIStore { this.emit(); } + /** The name `End` and the slash commands already use. */ + resetScroll() { + this.scrollToBottom(); + } + + /** Whether the transcript is pinned to its latest line. */ + isFollowing() { + return this.follow; + } + scrollPendingEditBy(lines: number) { if (lines === 0) return; @@ -687,7 +819,7 @@ export class UIStore { 0, Math.min( this.state.pendingEditScrollOffset + lines, - this.maxPendingEditScrollOffset, + this.state.maxPendingEditScrollOffset, ), ); @@ -698,15 +830,24 @@ export class UIStore { } setPendingEditScrollLimit(maxScrollOffset: number) { - this.maxPendingEditScrollOffset = Math.max(0, maxScrollOffset); + const limit = Math.max(0, maxScrollOffset); const pendingEditScrollOffset = Math.min( this.state.pendingEditScrollOffset, - this.maxPendingEditScrollOffset, + limit, ); - if (pendingEditScrollOffset === this.state.pendingEditScrollOffset) return; + if ( + pendingEditScrollOffset === this.state.pendingEditScrollOffset && + limit === this.state.maxPendingEditScrollOffset + ) { + return; + } - this.state = { ...this.state, pendingEditScrollOffset }; + this.state = { + ...this.state, + pendingEditScrollOffset, + maxPendingEditScrollOffset: limit, + }; this.emit(); } @@ -717,10 +858,14 @@ export class UIStore { } scrollPendingEditToEnd() { - if (this.state.pendingEditScrollOffset === this.maxPendingEditScrollOffset) return; + if ( + this.state.pendingEditScrollOffset === this.state.maxPendingEditScrollOffset + ) { + return; + } this.state = { ...this.state, - pendingEditScrollOffset: this.maxPendingEditScrollOffset, + pendingEditScrollOffset: this.state.maxPendingEditScrollOffset, }; this.emit(); } diff --git a/tui/src/timeline.tsx b/tui/src/timeline.tsx index e41ddcf..8a2f30e 100644 --- a/tui/src/timeline.tsx +++ b/tui/src/timeline.tsx @@ -21,14 +21,22 @@ interface TimelineProps { activeTurn: ActiveTurn | null; } -export function Timeline({ items, isThinking, activeTurn }: TimelineProps) { - const colors = usePalette(); +/** + * Timeline items laid out per frame. + * + * Ink re-lays out every mounted node on every commit, so an unbounded + * transcript makes streaming progressively slower: measured, 200 streamed + * tokens took 364ms at 50 items, 526ms at 500, and 2018ms at 2000 — about ten + * milliseconds of layout per token by then. Capping bounds that cost whatever + * the session length. The price is that rows past the cap leave in-app + * scrollback; the conversation itself is untouched, and the count says so. + */ +const MAX_RENDERED_ITEMS = 300; +export function Timeline({ items, isThinking, activeTurn }: TimelineProps) { return ( - {items.map((item) => ( - - ))} + {/* Rendered after the items rather than as one of them: the running turn's footer has to stay below whatever the turn appends next, which is what walks it down the transcript toward the composer. */} @@ -46,6 +54,38 @@ export function Timeline({ items, isThinking, activeTurn }: TimelineProps) { ); } +/** + * The settled transcript, memoized on the items array. + * + * The array identity only changes when the store emits, so a clock tick + * re-renders the running turn's footer and the spinners inside it and leaves + * every finished row alone. + */ +const TimelineHistory = memo(function TimelineHistory({ + items, +}: { + items: TimeLineItem[]; +}) { + const colors = usePalette(); + const hidden = Math.max(0, items.length - MAX_RENDERED_ITEMS); + const rendered = hidden === 0 ? items : items.slice(hidden); + + return ( + <> + {hidden > 0 && ( + + + {`… ${hidden} earlier item${hidden === 1 ? "" : "s"}`} + + + )} + {rendered.map((item) => ( + + ))} + + ); +}); + const TimelineItem = memo(function TimelineItem({ item }: { item: TimeLineItem }) { const colors = usePalette(); diff --git a/tui/src/types.ts b/tui/src/types.ts index 9174879..fe66b9f 100644 --- a/tui/src/types.ts +++ b/tui/src/types.ts @@ -90,6 +90,13 @@ export interface PendingQuestion { questions: string[]; } +/** A turn that has spent its step budget and is asking whether to keep going. */ +export interface PendingContinuation { + id: string; + /** Steps taken so far, which is what the user is being asked to extend. */ + steps: number; +} + export interface UIState { timeline: TimeLineItem[]; activeTurn: ActiveTurn | null; @@ -107,8 +114,19 @@ export interface UIState { pendingEdit: PendingEdit | null; pendingCommand: PendingCommand | null; pendingQuestion: PendingQuestion | null; + pendingContinuation: PendingContinuation | null; pendingEditScrollOffset: number; scrollOffset: number; + /** + * How far each viewport *can* be scrolled, measured from its rendered + * content. In the state rather than private to the store because the + * scrollbar is drawn from the pair: an offset alone cannot say how much is + * left below it. + */ + maxScrollOffset: number; + maxPendingEditScrollOffset: number; + /** Prompt tokens the last provider request carried; null before the first. */ + usage: { promptTokens: number } | null; } export interface TimelineProps {