From e5ee72bf001060025de586f99f18da2fef0494d2 Mon Sep 17 00:00:00 2001 From: Denver2828 <215248980+Denver2828@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:28:08 -0300 Subject: [PATCH 1/3] perf(shell): reuse session-derived footer values until the session changes Fullscreen rail digests rebuild the footer model every frame, and each build walked the whole session through getContextUsage() and sessionCost(). Memoize both per session manager behind (leafId, entryCount, model provider/id, contextWindow); when the host session manager lacks getEntryCount or getLeafId (outside the supported Pi range, which has both since 0.99.0) every build recomputes as before. Refs #1681 --- extensions/gentle-shell.ts | 37 +++++- odd/tasks/fix-1681-shared-footer-digest.md | 76 +++++++++++++ tests/shell-footer-model-cache.test.ts | 125 +++++++++++++++++++++ 3 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 odd/tasks/fix-1681-shared-footer-digest.md create mode 100644 tests/shell-footer-model-cache.test.ts diff --git a/extensions/gentle-shell.ts b/extensions/gentle-shell.ts index 2744b5033..36438c533 100644 --- a/extensions/gentle-shell.ts +++ b/extensions/gentle-shell.ts @@ -252,6 +252,39 @@ function sessionCost(ctx: ExtensionContext): number { return total; } +// Fullscreen rail digests rebuild the footer model every frame, and both +// session-derived values walk the whole session (gentle-shell#1681). Pi +// sessions are append-only, so the leaf id and entry count identify a session +// revision; context usage also follows the model's window. `getEntryCount` is +// newer than the peer range and absent from ReadonlySessionManager's type, so +// it is feature-detected and older Pi recomputes every build. +interface SessionRevision { + getLeafId?: () => string | null; + getEntryCount?: () => number; +} + +interface SessionDerived { + key: string; + usage: ReturnType; + cost: number; +} + +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) }; + } + 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) }; + sessionDerivedCache.set(ctx.sessionManager, fresh); + return fresh; +} + export function buildShellBarModel( pi: ExtensionAPI, ctx: ExtensionContext, @@ -259,7 +292,7 @@ export function buildShellBarModel( options: BuildOptions = {}, ): ShellBarModel { const home = options.home ?? os.homedir(); - const usage = ctx.getContextUsage(); + const { usage, cost } = sessionDerived(ctx); const model = ctx.model; const statuses = Array.from(footerData.getExtensionStatuses().entries()) .sort(([a], [b]) => a.localeCompare(b)) @@ -274,7 +307,7 @@ export function buildShellBarModel( 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..d79dffa84 --- /dev/null +++ b/odd/tasks/fix-1681-shared-footer-digest.md @@ -0,0 +1,76 @@ +# 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. +- [ ] 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. + +## Next step + +T3 bench numbers go back to the parent for the record; push, fork and PR stay user decisions. diff --git a/tests/shell-footer-model-cache.test.ts b/tests/shell-footer-model-cache.test.ts new file mode 100644 index 000000000..82d19090c --- /dev/null +++ b/tests/shell-footer-model-cache.test.ts @@ -0,0 +1,125 @@ +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 }; + const entries: FakeEntry[] = [assistant("a1", 0.5), assistant("a2", 0.25)]; + let leafId: string | null = "a2"; + const model = { provider: "openai", id: "gpt-5.5", reasoning: true, contextWindow: 200_000 }; + const sessionManager: Record = { + getCwd: () => "/repo", + getSessionName: () => "Release notes", + 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; + }, + 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(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); +}); From 5cefde5a702a042ec543820b6c5c2f0e98b967ca Mon Sep 17 00:00:00 2001 From: Denver2828 <215248980+Denver2828@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:59:19 -0300 Subject: [PATCH 2/3] docs(odd): record before and after measurements for the footer memo --- odd/tasks/fix-1681-shared-footer-digest.md | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/odd/tasks/fix-1681-shared-footer-digest.md b/odd/tasks/fix-1681-shared-footer-digest.md index d79dffa84..42e99f1f7 100644 --- a/odd/tasks/fix-1681-shared-footer-digest.md +++ b/odd/tasks/fix-1681-shared-footer-digest.md @@ -45,7 +45,7 @@ count and branch switches change the leaf. 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. -- [ ] T3 — Re-measure with the gentle-aporte bench (Bench A and B) against the patched tree, +- [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 @@ -71,6 +71,21 @@ count and branch switches change the leaf. 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). + ## Next step -T3 bench numbers go back to the parent for the record; push, fork and PR stay user decisions. +User decides: fork, push, claim comment on #1681 and PR. From 34f0d615908ee7114aac6616cb659c9d18360422 Mon Sep 17 00:00:00 2001 From: Denver2828 <215248980+Denver2828@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:26:59 -0300 Subject: [PATCH 3/3] perf(shell): cache the session name with the session revision getSessionName() scans every entry when the session has no session_info entry, so an unnamed session still paid a full-history walk per fullscreen digest. A rename appends a session_info entry, so the existing (leafId, entryCount, model) key already invalidates it. --- extensions/gentle-shell.ts | 23 ++++++++++++---------- odd/tasks/fix-1681-shared-footer-digest.md | 6 ++++++ tests/shell-footer-model-cache.test.ts | 22 +++++++++++++++++++-- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/extensions/gentle-shell.ts b/extensions/gentle-shell.ts index 36438c533..d15c4794d 100644 --- a/extensions/gentle-shell.ts +++ b/extensions/gentle-shell.ts @@ -252,12 +252,14 @@ function sessionCost(ctx: ExtensionContext): number { return total; } -// Fullscreen rail digests rebuild the footer model every frame, and both -// session-derived values walk the whole session (gentle-shell#1681). Pi -// sessions are append-only, so the leaf id and entry count identify a session -// revision; context usage also follows the model's window. `getEntryCount` is -// newer than the peer range and absent from ReadonlySessionManager's type, so -// it is feature-detected and older Pi recomputes every build. +// 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; @@ -267,6 +269,7 @@ interface SessionDerived { key: string; usage: ReturnType; cost: number; + sessionName: string | undefined; } const sessionDerivedCache = new WeakMap(); @@ -274,13 +277,13 @@ 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) }; + 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) }; + const fresh = { key, usage: ctx.getContextUsage(), cost: sessionCost(ctx), sessionName: ctx.sessionManager.getSessionName() }; sessionDerivedCache.set(ctx.sessionManager, fresh); return fresh; } @@ -292,7 +295,7 @@ export function buildShellBarModel( options: BuildOptions = {}, ): ShellBarModel { const home = options.home ?? os.homedir(); - const { usage, cost } = sessionDerived(ctx); + const { usage, cost, sessionName } = sessionDerived(ctx); const model = ctx.model; const statuses = Array.from(footerData.getExtensionStatuses().entries()) .sort(([a], [b]) => a.localeCompare(b)) @@ -302,7 +305,7 @@ 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, diff --git a/odd/tasks/fix-1681-shared-footer-digest.md b/odd/tasks/fix-1681-shared-footer-digest.md index 42e99f1f7..00141e6a2 100644 --- a/odd/tasks/fix-1681-shared-footer-digest.md +++ b/odd/tasks/fix-1681-shared-footer-digest.md @@ -85,6 +85,12 @@ count and branch switches change the leaf. - 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 diff --git a/tests/shell-footer-model-cache.test.ts b/tests/shell-footer-model-cache.test.ts index 82d19090c..34a59adb4 100644 --- a/tests/shell-footer-model-cache.test.ts +++ b/tests/shell-footer-model-cache.test.ts @@ -19,13 +19,17 @@ function assistant(id: string, cost: number): FakeEntry { } function fakeSession({ entryCount = true }: { entryCount?: boolean } = {}) { - const calls = { usage: 0, entries: 0 }; + 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: () => "Release notes", + getSessionName: () => { + calls.name += 1; + return sessionName; + }, getLeafId: () => leafId, getEntries: () => { calls.entries += 1; @@ -60,6 +64,11 @@ function fakeSession({ entryCount = true }: { entryCount?: boolean } = {}) { 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; }, @@ -75,6 +84,7 @@ test("an unchanged session and model walk the session once across repeated 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); }); @@ -123,3 +133,11 @@ test("without getEntryCount every build recomputes", () => { 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); +});