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 {