diff --git a/extensions/gentle-shell.ts b/extensions/gentle-shell.ts index 2744b5033..d15c4794d 100644 --- a/extensions/gentle-shell.ts +++ b/extensions/gentle-shell.ts @@ -252,6 +252,42 @@ function sessionCost(ctx: ExtensionContext): number { return total; } +// Fullscreen rail digests rebuild the footer model every frame, and the +// session-derived values walk the whole session (gentle-shell#1681): context +// usage and cost always, and the session name when no session_info entry +// exists. Pi sessions are append-only, so the leaf id and entry count identify +// a session revision (a rename appends a session_info entry); context usage +// also follows the model's window. `getEntryCount` is absent from +// ReadonlySessionManager's type, so it is feature-detected and a session +// manager without it recomputes every build. +interface SessionRevision { + getLeafId?: () => string | null; + getEntryCount?: () => number; +} + +interface SessionDerived { + key: string; + usage: ReturnType; + cost: number; + sessionName: string | undefined; +} + +const sessionDerivedCache = new WeakMap(); + +function sessionDerived(ctx: ExtensionContext): Omit { + const session = ctx.sessionManager as SessionRevision; + if (typeof session.getEntryCount !== "function" || typeof session.getLeafId !== "function") { + return { usage: ctx.getContextUsage(), cost: sessionCost(ctx), sessionName: ctx.sessionManager.getSessionName() }; + } + const model = ctx.model; + const key = JSON.stringify([session.getLeafId(), session.getEntryCount(), model?.provider ?? null, model?.id ?? null, model?.contextWindow ?? null]); + const cached = sessionDerivedCache.get(ctx.sessionManager); + if (cached?.key === key) return cached; + const fresh = { key, usage: ctx.getContextUsage(), cost: sessionCost(ctx), sessionName: ctx.sessionManager.getSessionName() }; + sessionDerivedCache.set(ctx.sessionManager, fresh); + return fresh; +} + export function buildShellBarModel( pi: ExtensionAPI, ctx: ExtensionContext, @@ -259,7 +295,7 @@ export function buildShellBarModel( options: BuildOptions = {}, ): ShellBarModel { const home = options.home ?? os.homedir(); - const usage = ctx.getContextUsage(); + const { usage, cost, sessionName } = sessionDerived(ctx); const model = ctx.model; const statuses = Array.from(footerData.getExtensionStatuses().entries()) .sort(([a], [b]) => a.localeCompare(b)) @@ -269,12 +305,12 @@ export function buildShellBarModel( profile: options.profile, branch: footerData.getGitBranch(), dirty: options.dirty, - sessionName: ctx.sessionManager.getSessionName(), + sessionName, modelId: model?.id ?? "no-model", effort: model?.reasoning ? pi.getThinkingLevel() : undefined, contextPercent: usage?.percent ?? null, contextWindow: usage?.contextWindow ?? model?.contextWindow ?? 0, - costTotal: sessionCost(ctx), + costTotal: cost, subscription: model ? ctx.modelRegistry.isUsingOAuth(model) : false, usage: options.usage, statuses, diff --git a/odd/tasks/fix-1681-shared-footer-digest.md b/odd/tasks/fix-1681-shared-footer-digest.md new file mode 100644 index 000000000..00141e6a2 --- /dev/null +++ b/odd/tasks/fix-1681-shared-footer-digest.md @@ -0,0 +1,97 @@ +# fix #1681: stop rebuilding the footer model on every fullscreen frame + +## Objective + +In fullscreen with the sidebar on, a settled frame must not walk the whole session to +compute the footer digest. Session-derived values are recomputed only when the session or +model actually changes. + +## Problem + +Measured on `ac671593` with Pi 1.0.0 (gentle-aporte commit `b967db8b`, +`docs/measurements/gentle-shell-fullscreen-perf.md`): + +- `lib/shell-sidebar-layout.ts:216` evaluates every rail digest on every frame. +- The footer rail (`extensions/gentle-shell.ts:1829`) and the header rail (`:1838`) both call + `footerModel()`, which calls `buildShellBarModel()` (`:255`). +- That runs `sessionCost()` (`:246`, copies every entry through `getEntries()`) and + `ctx.getContextUsage()` (`:262`, which projects the session in Pi). +- At 5000 messages: the digest takes about 2.0 ms of a 3.2 ms settled frame; about 90% of it is + `getContextUsage`. + +## Approach + +Memoize the two session walks behind a cheap revision key: +`sessionManager.getLeafId()`, `sessionManager.getEntryCount()`, and the model identity and +context window. Pi sessions are append-only (`getEntries` doc comment), so appends change the +count and branch switches change the leaf. + +- `getEntryCount()` landed in Pi on 2026-09-29, and the peer range still admits `>=0.99.1`. + Feature-detect it; when it is missing, fall back to today's uncached behavior. +- Keep `buildShellBarModel`'s exported signature and output identical. + +## Non-goals + +- No change to Pi. +- No change to the sidebar layout's flat cost or to the digest/memo mechanism in + `lib/shell-sidebar-layout.ts`. +- No behavior or visual change to the footer, header, or bottom bar. + +## Tasks + +- [x] T1 — RED: a test proving that, for an unchanged session and model, repeated footer + model builds call `getContextUsage` and walk entries only once. It must also prove the + values refresh after an append, a leaf change, and a model change, and that the fallback + works without `getEntryCount`. Route: delegated writer. +- [x] T2 — GREEN: implement the memo in `extensions/gentle-shell.ts`. Run the focused tests, + then the full `pnpm test`. Route: same delegated writer. +- [x] T3 — Re-measure with the gentle-aporte bench (Bench A and B) against the patched tree, + and record before/after. Route: same delegated writer. + +## Acceptance criteria + +- RED observed before the implementation, GREEN after. +- Full `pnpm test` result reported verbatim, including failures and skips. +- Settled-frame digest cost no longer grows with N for an unchanged session. + +## Delivery + +- Branch `fix/1681-shared-footer-digest` from `ac671593`. +- Forecast: under 200 authored lines. +- Push, fork, claim comment, and PR are user decisions. + +## Progress + +- 2026-10-02: document created; clone at `C:\A_Desarrollos\Proyectos\gentle-shell-1681`. +- 2026-10-02: T1 RED observed (`tests/shell-footer-model-cache.test.ts`: the unchanged-session + test failed with 6 `getContextUsage` calls instead of 1). T2 GREEN: `sessionDerived()` in + `extensions/gentle-shell.ts` memoizes usage and cost per session manager behind + `(leafId, entryCount, provider, id, contextWindow)`; 5/5 focused tests pass. Full `pnpm test`: + unit-tests FAIL (4645 tests, 230 fail, 1 cancelled, 87 skipped; every failure is in a file that + does not import the patched module, or is identical on the unpatched base), provider-contract + PASS, runtime-harness PASS. + +- 2026-10-02: T3 re-measured with the gentle-aporte bench (unchanged; it already calls the exported + `buildShellBarModel` over a real Pi 1.0.0 `SessionManager`). Settled cache-hit frames, medians: + + | N | footer digest µs (before → after) | sidebar-on frame ms (before → after) | + |---|---|---| + | 100 | 20.2 → 4.9 | 0.648 → 0.596 | + | 1000 | 164.6 → 6.3 | 0.967 → 0.680 | + | 5000 | 1275.7 → 13.6 | 3.190 → 1.258 | + + After the patch the real-digest frame matches the constant-digest control at every N. + Cache-miss frames (the session just changed) keep the old cost. +- 2026-10-02: parent verified `getEntryCount` ships in Pi 0.99.0, 0.99.1 and 0.99.2, so the whole + supported peer range has it; the fallback only covers hosts outside that range. The perf commit body + was reworded to say so (originally `a8c1d49c`, now `e5ee72bf`; local only, never pushed). +- 2026-10-03: CodeRabbit review on PR #1697 (minor, outside the diff): `getSessionName()` scans + every entry when no session_info entry exists. Cached the name with the same revision key (a rename + appends a session_info entry). RED: unchanged-session test failed with 6 name lookups instead of 1; + GREEN: focused + shell-bar 61/61; the 11 test files importing the module show the same 5 base + failures; typecheck no regressions. Also corrected the source comment that called + `getEntryCount` newer than the peer range. + +## Next step + +User decides: fork, push, claim comment on #1681 and PR. diff --git a/tests/shell-footer-model-cache.test.ts b/tests/shell-footer-model-cache.test.ts new file mode 100644 index 000000000..34a59adb4 --- /dev/null +++ b/tests/shell-footer-model-cache.test.ts @@ -0,0 +1,143 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; +import { buildShellBarModel } from "../extensions/gentle-shell.ts"; + +// gentle-shell#1681: in fullscreen the footer and header rail digests rebuild +// the footer model every frame. The session-derived values (context usage and +// cost) walk the whole session, so they are reused until the session or the +// model changes, keyed on Pi's cheap leaf id and entry count. + +interface FakeEntry { + type: string; + id: string; + message?: { role: string; usage?: { cost?: { total?: number } } }; +} + +function assistant(id: string, cost: number): FakeEntry { + return { type: "message", id, message: { role: "assistant", usage: { cost: { total: cost } } } }; +} + +function fakeSession({ entryCount = true }: { entryCount?: boolean } = {}) { + const calls = { usage: 0, entries: 0, name: 0 }; + const entries: FakeEntry[] = [assistant("a1", 0.5), assistant("a2", 0.25)]; + let leafId: string | null = "a2"; + let sessionName: string | undefined = "Release notes"; + const model = { provider: "openai", id: "gpt-5.5", reasoning: true, contextWindow: 200_000 }; + const sessionManager: Record = { + getCwd: () => "/repo", + getSessionName: () => { + calls.name += 1; + return sessionName; + }, + getLeafId: () => leafId, + getEntries: () => { + calls.entries += 1; + return entries.slice(); + }, + }; + if (entryCount) sessionManager.getEntryCount = () => entries.length; + const state = { model }; + const ctx = { + sessionManager, + get model() { + return state.model; + }, + modelRegistry: { isUsingOAuth: () => false }, + getContextUsage: () => { + calls.usage += 1; + const tokens = entries.length * 1000; + return { tokens, contextWindow: state.model.contextWindow, percent: (tokens / state.model.contextWindow) * 100 }; + }, + } as unknown as ExtensionContext; + const pi = { getThinkingLevel: () => "medium" } as unknown as ExtensionAPI; + const footerData = { + getGitBranch: () => "main", + getExtensionStatuses: () => new Map(), + getAvailableProviderCount: () => 1, + onBranchChange: () => () => {}, + }; + return { + calls, + build: () => buildShellBarModel(pi, ctx, footerData, { home: "/home/alan" }), + append(entry: FakeEntry) { + entries.push(entry); + leafId = entry.id; + }, + rename(name: string) { + // Pi records a rename as an appended session_info entry. + entries.push({ type: "session_info", id: `info-${entries.length}` }); + sessionName = name; + }, + setLeaf(id: string | null) { + leafId = id; + }, + setModel(next: Partial) { + state.model = { ...state.model, ...next }; + }, + }; +} + +test("an unchanged session and model walk the session once across repeated builds", () => { + const session = fakeSession(); + const first = session.build(); + for (let frame = 0; frame < 5; frame += 1) assert.deepEqual(session.build(), first); + assert.equal(session.calls.usage, 1); + assert.equal(session.calls.entries, 1); + assert.equal(session.calls.name, 1); + assert.equal(first.costTotal, 0.75); + assert.equal(first.contextPercent, 1); +}); + +test("an append refreshes context usage and cost", () => { + const session = fakeSession(); + session.build(); + session.append(assistant("a3", 1)); + const built = session.build(); + assert.equal(built.costTotal, 1.75); + assert.equal(built.contextPercent, 1.5); + assert.equal(session.calls.usage, 2); + assert.equal(session.calls.entries, 2); +}); + +test("a leaf change refreshes the session-derived values", () => { + const session = fakeSession(); + session.build(); + session.setLeaf("a1"); + session.build(); + assert.equal(session.calls.usage, 2); + assert.equal(session.calls.entries, 2); +}); + +test("a model or context-window change refreshes context usage", () => { + const session = fakeSession(); + session.build(); + session.setModel({ id: "gpt-5.5-mini" }); + assert.equal(session.build().modelId, "gpt-5.5-mini"); + assert.equal(session.calls.usage, 2); + session.setModel({ provider: "anthropic" }); + session.build(); + assert.equal(session.calls.usage, 3); + session.setModel({ contextWindow: 100_000 }); + const built = session.build(); + assert.equal(session.calls.usage, 4); + assert.equal(built.contextWindow, 100_000); + assert.equal(built.contextPercent, 2); +}); + +test("without getEntryCount every build recomputes", () => { + const session = fakeSession({ entryCount: false }); + session.build(); + session.build(); + session.build(); + assert.equal(session.calls.usage, 3); + assert.equal(session.calls.entries, 3); +}); + +test("a rename refreshes the cached session name", () => { + const session = fakeSession(); + assert.equal(session.build().sessionName, "Release notes"); + session.rename("Hotfix"); + assert.equal(session.build().sessionName, "Hotfix"); + assert.equal(session.calls.name, 2); +});