From 5076052ab7712d8dc29e54840c9852a974ed0fab Mon Sep 17 00:00:00 2001 From: tomolom <37050939+tomolom@users.noreply.github.com> Date: Tue, 8 Sep 2026 04:13:10 +0100 Subject: [PATCH] fix(pi): derive context entries from the branch both hosts expose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `turn_start` called `SessionManager.buildContextEntries()`, which exists on Pi but not on Oh My Pi 18.x — on a live 18.1.14 SessionManager instance the property is `undefined`. The handler threw at its first statement on every turn, so it surfaced a per-turn extension error and collected no Fable/Mythos 5.1 mid-conversation effort markers at all. `getBranch()` is present on both hosts and returns the same root-to-leaf path `buildContextEntries()` walks, so `deriveContextEntries()` applies the compaction trim locally: the compaction entry, the tail retained from `firstKeptEntryId`, then everything appended after it. The handler now also degrades to no transitions instead of throwing. It only annotates requests with effort markers, so an unreadable session shape must cost the session its transitions and nothing else. Also run the pi package's tests in `bun run test`, which covered core and opencode only. Fixes #200 --- package.json | 2 +- packages/pi/src/effort-history.ts | 40 +++++ packages/pi/src/index.ts | 26 +++- packages/pi/src/tests/effort-history.test.ts | 65 +++++++- packages/pi/src/tests/index.test.ts | 156 ++++++++++++++++++- 5 files changed, 280 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index 349af1bb..9fa1e2a9 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "extract": "bun scripts/extract-system-prompt.ts", "check:claustrum-golden": "bun scripts/check-claustrum-golden.ts", "analyze:cache": "node scripts/analyze-cache-usage.mjs", - "test": "bun run build && cd packages/core && bun test src/tests && cd ../opencode && bun run test", + "test": "bun run build && cd packages/core && bun test src/tests && cd ../opencode && bun run test && cd ../pi && bun run test", "test:e2e": "cd packages/core && bun run build && cd ../.. && bun run --cwd packages/e2e-tests test", "typecheck": "cd packages/core && bun run build && cd ../opencode && bun run typecheck && cd ../pi && bun run typecheck && cd ../.. && tsc -p tsconfig.scripts.json", "types": "bun run typecheck", diff --git a/packages/pi/src/effort-history.ts b/packages/pi/src/effort-history.ts index 5fe70775..a3f828ff 100644 --- a/packages/pi/src/effort-history.ts +++ b/packages/pi/src/effort-history.ts @@ -8,9 +8,49 @@ type SessionEntryLike = { id?: unknown type?: unknown thinkingLevel?: unknown + firstKeptEntryId?: unknown message?: { role?: unknown } } +/** + * Reproduce the host's context-entry view — the entries the next request will + * actually carry — from the branch path, ordered root to leaf. + * + * `SessionManager.buildContextEntries()` computes this on Pi hosts but does not + * exist on Oh My Pi 18.x, where calling it threw on every turn (issue #200). + * `getBranch()` is present on both hosts and is the same path that method walks, + * so the compaction trim is applied here instead of asking the host for it. + * + * A branch without compaction is already the context. After a compaction the + * context is the compaction entry, then the tail retained from + * `firstKeptEntryId`, then everything appended after the compaction. + */ +export function deriveContextEntries( + branch: readonly Entry[], +): Entry[] { + let compactionIndex = -1 + for (let index = 0; index < branch.length; index++) { + if (branch[index]?.type === 'compaction') compactionIndex = index + } + + const compaction = branch[compactionIndex] + if (!compaction) return branch.slice() + + const contextEntries: Entry[] = [compaction] + let retaining = false + for (let index = 0; index < compactionIndex; index++) { + const entry = branch[index] + if (!entry) continue + if (entry.id === compaction.firstKeptEntryId) retaining = true + if (retaining) contextEntries.push(entry) + } + for (let index = compactionIndex + 1; index < branch.length; index++) { + const entry = branch[index] + if (entry) contextEntries.push(entry) + } + return contextEntries +} + function entryRole(entry: SessionEntryLike): unknown { if (entry.type === 'compaction' || entry.type === 'branch_summary') { return 'user' diff --git a/packages/pi/src/index.ts b/packages/pi/src/index.ts index 669a2640..a2da6914 100644 --- a/packages/pi/src/index.ts +++ b/packages/pi/src/index.ts @@ -17,7 +17,10 @@ import type { import type { ExtensionAPI } from '@earendil-works/pi-coding-agent' import { registerCommands } from './commands.ts' -import { collectPiEffortHistory } from './effort-history.ts' +import { + collectPiEffortHistory, + deriveContextEntries, +} from './effort-history.ts' import { streamCortexKitAnthropic } from './stream.ts' async function loginAnthropic( @@ -71,10 +74,23 @@ export default function cortexKitPiAnthropicAuth(pi: ExtensionAPI) { pi.on('turn_start', async (_event, ctx) => { const sessionId = ctx.sessionManager.getSessionId() if (!sessionId) return - const transitions = collectPiEffortHistory( - ctx.sessionManager.buildContextEntries(), - ctx.sessionManager.getBranch(), - ) + // This hook only adds mid-conversation effort markers to Fable/Mythos 5.1 + // requests. A host whose session entries do not match what this reads must + // cost the session its transitions and nothing else: the handler runs + // before every turn, and an exception here surfaced as a per-turn extension + // error while collecting no effort history at all (issue #200). + // + // The catch stays quiet: `ExtensionAPI` carries no log surface on either + // host, and writing to stdout from a per-turn hook corrupts the host's + // rendering — which is the same per-turn noise this fix removes. The + // degraded state is observable in the request: no effort markers. + let transitions: MidConversationEffortTransition[] + try { + const branch = ctx.sessionManager.getBranch() + transitions = collectPiEffortHistory(deriveContextEntries(branch), branch) + } catch { + transitions = [] + } effortHistoryBySession.delete(sessionId) effortHistoryBySession.set(sessionId, transitions) while (effortHistoryBySession.size > 128) { diff --git a/packages/pi/src/tests/effort-history.test.ts b/packages/pi/src/tests/effort-history.test.ts index 236bdc21..73a961aa 100644 --- a/packages/pi/src/tests/effort-history.test.ts +++ b/packages/pi/src/tests/effort-history.test.ts @@ -1,5 +1,8 @@ import { describe, expect, test } from 'bun:test' -import { collectPiEffortHistory } from '../effort-history.ts' +import { + collectPiEffortHistory, + deriveContextEntries, +} from '../effort-history.ts' const entry = ( id: string, @@ -34,7 +37,7 @@ describe('Pi Fable 5.1 effort history', () => { entry('t1', 'thinking_level_change', { thinkingLevel: 'high' }), entry('u2', 'message', { message: { role: 'user' } }), entry('a2', 'message', { message: { role: 'assistant' } }), - entry('compact', 'compaction'), + entry('compact', 'compaction', { firstKeptEntryId: 't1' }), entry('t2', 'thinking_level_change', { thinkingLevel: 'xhigh' }), entry('u3', 'message', { message: { role: 'user' } }), ] @@ -46,9 +49,67 @@ describe('Pi Fable 5.1 effort history', () => { fullBranch[7]!, fullBranch[8]!, ] + expect(deriveContextEntries(fullBranch)).toEqual(contextEntries) expect(collectPiEffortHistory(contextEntries, fullBranch)).toEqual([ { afterAssistantMessages: 0, effort: 'high' }, { afterAssistantMessages: 1, effort: 'xhigh' }, ]) }) }) + +// The host accessor these entries came from — buildContextEntries() — exists on +// Pi but not on Oh My Pi 18.x, so the compaction trim is derived here from the +// branch path both hosts expose (issue #200). +describe('Pi context entries', () => { + test('carries an uncompacted branch through unchanged', () => { + const branch = [ + entry('u1', 'message', { message: { role: 'user' } }), + entry('a1', 'message', { message: { role: 'assistant' } }), + ] + const derived = deriveContextEntries(branch) + expect(derived).toEqual(branch) + expect(derived).not.toBe(branch) + }) + + test('drops entries older than the compaction it retains from', () => { + const branch = [ + entry('u1', 'message', { message: { role: 'user' } }), + entry('a1', 'message', { message: { role: 'assistant' } }), + entry('u2', 'message', { message: { role: 'user' } }), + entry('compact', 'compaction', { firstKeptEntryId: 'u2' }), + entry('a2', 'message', { message: { role: 'assistant' } }), + ] + expect(deriveContextEntries(branch).map((item) => item.id)).toEqual([ + 'compact', + 'u2', + 'a2', + ]) + }) + + test('retains nothing before a compaction with an unreachable first kept id', () => { + const branch = [ + entry('u1', 'message', { message: { role: 'user' } }), + entry('compact', 'compaction', { firstKeptEntryId: 'pruned' }), + entry('u2', 'message', { message: { role: 'user' } }), + ] + expect(deriveContextEntries(branch).map((item) => item.id)).toEqual([ + 'compact', + 'u2', + ]) + }) + + test('anchors on the last compaction when a branch has several', () => { + const branch = [ + entry('u1', 'message', { message: { role: 'user' } }), + entry('c1', 'compaction', { firstKeptEntryId: 'u1' }), + entry('u2', 'message', { message: { role: 'user' } }), + entry('c2', 'compaction', { firstKeptEntryId: 'u2' }), + entry('u3', 'message', { message: { role: 'user' } }), + ] + expect(deriveContextEntries(branch).map((item) => item.id)).toEqual([ + 'c2', + 'u2', + 'u3', + ]) + }) +}) diff --git a/packages/pi/src/tests/index.test.ts b/packages/pi/src/tests/index.test.ts index 412f9cca..c48ebb27 100644 --- a/packages/pi/src/tests/index.test.ts +++ b/packages/pi/src/tests/index.test.ts @@ -1,8 +1,38 @@ -import { describe, expect, test } from 'bun:test' +import { afterEach, describe, expect, mock, test } from 'bun:test' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { saveAccounts } from '@cortexkit/anthropic-auth-core' import type { ExtensionAPI } from '@earendil-works/pi-coding-agent' import cortexKitPiAnthropicAuth from '../index' +let tempDir: string | undefined +const originalFetch = globalThis.fetch + +// Fable 5.1 is the family that carries mid-conversation effort markers, so it +// is the model that can observe what turn_start collected. +const fableModel = { + id: 'claude-fable-5-1', + name: 'Claude Fable 5.1', + api: 'cortexkit-anthropic-messages', + provider: 'anthropic', + baseUrl: 'https://api.anthropic.com', + reasoning: true, + input: ['text'], + cost: { input: 1, output: 1, cacheRead: 1, cacheWrite: 1 }, + contextWindow: 1_000_000, + maxTokens: 128_000, +} +const messagesUrl = `${fableModel.baseUrl}/v1/messages` + +afterEach(async () => { + globalThis.fetch = originalFetch + delete process.env.PI_ANTHROPIC_AUTH_FILE + if (tempDir) await rm(tempDir, { recursive: true, force: true }) + tempDir = undefined +}) + function mockPi() { const providers = new Map< string, @@ -102,3 +132,127 @@ describe('cortexKitPiAnthropicAuth provider registration', () => { }) }) }) + +// Oh My Pi 18.x dropped SessionManager.buildContextEntries(); calling it threw +// on every turn, so no effort history was ever collected (issue #200). Only +// getSessionId/getBranch are assumed here — the accessors both hosts expose. +describe('cortexKitPiAnthropicAuth turn_start effort history', () => { + test('carries transitions from a getBranch-only host into the request', async () => { + tempDir = await mkdtemp(join(tmpdir(), 'pi-turn-start-effort-')) + const storagePath = join(tempDir, 'anthropic-auth.json') + process.env.PI_ANTHROPIC_AUTH_FILE = storagePath + await saveAccounts( + { + version: 1, + main: { type: 'opencode', provider: 'anthropic' }, + accounts: [], + }, + storagePath, + ) + + const { pi, providers, events } = mockPi() + cortexKitPiAnthropicAuth(pi) + + // minimal -> low, then xhigh, with one assistant message between them. + const branch = [ + { id: 't0', type: 'thinking_level_change', thinkingLevel: 'minimal' }, + { id: 'u1', type: 'message', message: { role: 'user' } }, + { id: 'a1', type: 'message', message: { role: 'assistant' } }, + { id: 't1', type: 'thinking_level_change', thinkingLevel: 'xhigh' }, + { id: 'u2', type: 'message', message: { role: 'user' } }, + ] + const handler = events.get('turn_start') + expect(handler).toBeDefined() + await handler?.( + { type: 'turn_start' }, + { + sessionManager: { + getSessionId: () => 'session-omp', + getBranch: () => branch, + getEntries: () => branch, + }, + }, + ) + + // Only the messages POST may be captured: if the stream path ever adds + // another request (relay, quota, retry), this must fail loudly rather than + // let the assertions below inspect that body instead. + let requestBody: Record | undefined + globalThis.fetch = mock( + async (input: string | URL | Request, init?: RequestInit) => { + const url = input.toString() + if (url.includes('/api/claude_cli/bootstrap')) { + return new Response( + JSON.stringify({ + oauth_account: { account_uuid: 'pi-turn-start-account' }, + }), + ) + } + const method = (init?.method ?? 'GET').toUpperCase() + if (method !== 'POST' || !url.startsWith(messagesUrl)) { + throw new Error(`unexpected request: ${method} ${url}`) + } + expect(requestBody).toBeUndefined() + requestBody = JSON.parse(String(init?.body)) + return new Response( + [ + 'event: message_start\ndata: {"type":"message_start","message":{"usage":{"input_tokens":1,"output_tokens":0}}}\n\n', + 'event: message_delta\ndata: {"type":"message_delta","delta":{"stop_reason":"end_turn"},"usage":{"output_tokens":1}}\n\n', + 'event: message_stop\ndata: {"type":"message_stop"}\n\n', + ].join(''), + { status: 200 }, + ) + }, + ) as unknown as typeof fetch + + const stream = providers.get('anthropic')?.streamSimple?.( + fableModel, + { + systemPrompt: 'test', + tools: [], + messages: [ + { role: 'user', content: 'first', timestamp: 0 }, + { + role: 'assistant', + content: [{ type: 'text', text: 'answer' }], + timestamp: 0, + }, + { role: 'user', content: 'second', timestamp: 0 }, + ], + }, + { apiKey: 'sk-ant-oat-turn-start', sessionId: 'session-omp' }, + ) + for await (const _event of stream as AsyncIterable) { + // Drain the provider stream. + } + + // The transitions the handler collected, as the request carries them: the + // opening effort on the body and the later change as its own marker turn. + expect(requestBody).toBeDefined() + const sent = requestBody as { output_config: unknown; messages: unknown[] } + expect(sent.output_config).toEqual({ effort: 'low' }) + expect(sent.messages[2]).toEqual({ + role: 'system', + content: [], + output_config: { effort: 'xhigh' }, + }) + }) + + test('degrades to no transitions when the host session shape is unreadable', async () => { + const { pi, events } = mockPi() + cortexKitPiAnthropicAuth(pi) + + const handler = events.get('turn_start') + expect(handler).toBeDefined() + const ctx = { + sessionManager: { + getSessionId: () => 'session-broken', + getBranch: () => { + throw new TypeError('getBranch is not a function') + }, + }, + } + + expect(await handler?.({ type: 'turn_start' }, ctx)).toBeUndefined() + }) +})