From c6fd50b8e68e5bec43a17755eda61d9d6823cc5d Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 15 Jun 2026 16:49:21 +0200 Subject: [PATCH] feat(coding-agent): added advisor context auto-maintenance with safer replay and compaction - Added advisor context maintenance hook and token estimation before prompting for auto-upkeep. - Added re-prime replay handling to reset advisor context and recover deferred prompts. - Implemented session-level context compaction with model promotion and snapcompact-first fallback summarization. - Surfaced advisor settings in the model tab and updated advisor system guidance text. --- packages/coding-agent/CHANGELOG.md | 3 +- .../src/advisor/__tests__/advisor.test.ts | 34 +++ packages/coding-agent/src/advisor/runtime.ts | 30 ++- .../src/config/settings-schema.ts | 2 +- .../src/prompts/advisor/system.md | 32 ++- .../coding-agent/src/session/agent-session.ts | 215 +++++++++++++++++- .../assistant-message-error.test.ts | 4 +- 7 files changed, 302 insertions(+), 18 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2942718da..500e04943 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,7 +1,6 @@ # Changelog ## [Unreleased] - ### Breaking Changes - Renamed the SDK tool format type and resolver from `ToolCallFormat`/`resolveToolCallSyntax` to `DialectFormat`/`resolveDialect`, and the agent option from `toolCallSyntax` to `dialect`. @@ -24,7 +23,9 @@ ### Fixed +- Fixed advisor context handling to automatically maintain its token budget by promoting the advisor model or compacting/restarting advisor context when needed, preventing advice from degrading on long sessions - Fixed `startup.quiet` leaving MCP and LSP startup status events visible during launch ([#2639](https://github.com/can1357/oh-my-pi/issues/2639)). +- Registered the `Advisor` group in the `model` settings tab so advisor settings render correctly in the settings panel. ## [15.13.3] - 2026-06-15 diff --git a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts index 67040b217..f6499825a 100644 --- a/packages/coding-agent/src/advisor/__tests__/advisor.test.ts +++ b/packages/coding-agent/src/advisor/__tests__/advisor.test.ts @@ -246,6 +246,40 @@ describe("advisor", () => { expect(promptInputs).toHaveLength(2); expect(promptInputs[1]).toContain("summary-bbb"); }); + + it("triggers a re-prime and full replay when maintainContext returns true", async () => { + const promptInputs: string[] = []; + const agent = makeAgent(promptInputs); + const messages: AgentMessage[] = [{ role: "user", content: "aaa", timestamp: 1 } as AgentMessage]; + let shouldRePrime = false; + const host: AdvisorRuntimeHost = { + snapshotMessages: () => messages, + enqueueAdvice: () => {}, + maintainContext: async tokens => { + expect(tokens).toBeGreaterThan(0); + return shouldRePrime; + }, + }; + const runtime = new AdvisorRuntime(agent, host); + + // First turn: normal incremental prompt + runtime.onTurnEnd(); + await Promise.resolve(); + expect(promptInputs).toHaveLength(1); + expect(promptInputs[0]).toContain("aaa"); + + // Second turn: maintainContext resolves true, triggering a re-prime + shouldRePrime = true; + messages.push({ role: "user", content: "bbb", timestamp: 2 } as AgentMessage); + runtime.onTurnEnd(); + await Promise.resolve(); + await Promise.resolve(); + + // The reset cleared history and prompted a full replay (so the batch contains both aaa and bbb) + expect(promptInputs).toHaveLength(2); + expect(promptInputs[1]).toContain("aaa"); + expect(promptInputs[1]).toContain("bbb"); + }); }); describe("read-only tool allowlist", () => { diff --git a/packages/coding-agent/src/advisor/runtime.ts b/packages/coding-agent/src/advisor/runtime.ts index 193d69a03..fc4f0530f 100644 --- a/packages/coding-agent/src/advisor/runtime.ts +++ b/packages/coding-agent/src/advisor/runtime.ts @@ -1,4 +1,5 @@ import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; +import { estimateTokens } from "@oh-my-pi/pi-agent-core/compaction"; import { logger } from "@oh-my-pi/pi-utils"; import { formatSessionHistoryMarkdown } from "../session/session-history-format"; @@ -15,6 +16,16 @@ export interface AdvisorRuntimeHost { snapshotMessages(): AgentMessage[]; /** Surface one advice note to the primary (enqueues into the session YieldQueue). */ enqueueAdvice(note: string, severity?: "nit" | "concern" | "blocker"): void; + /** + * Pre-prompt context maintenance for the advisor's own append-only context. + * Promotes the advisor model to a larger sibling when its context nears the + * window (mirroring the primary's promote-first policy) and resolves `true` + * when the advisor should re-prime — reset and replay the current + * primary-bounded transcript — because promotion did not free enough room. + * Optional: hosts that omit it get no maintenance (context only shrinks when + * the primary's next compaction triggers {@link AdvisorRuntime.reset}). + */ + maintainContext?(incomingTokens: number): Promise; } export class AdvisorRuntime { @@ -93,7 +104,24 @@ export class AdvisorRuntime { this.#busy = true; try { while (!this.#disposed && this.#pending.length) { - const batch = this.#pending.splice(0).join("\n\n---\n\n"); + const candidateBatch = this.#pending.join("\n\n---\n\n"); + const incomingTokens = estimateTokens({ + role: "user", + content: candidateBatch, + timestamp: Date.now(), + }); + + let batch: string | null; + if (this.host.maintainContext && (await this.host.maintainContext(incomingTokens))) { + // Promotion could not fit the advisor's context — re-prime: drop the + // accumulated review history and replay the current (primary-bounded) + // transcript so the next turn resumes from a fresh, in-window context. + this.reset(); + batch = this.#renderDelta(); + } else { + batch = this.#pending.splice(0).join("\n\n---\n\n"); + } + if (this.#disposed || batch === null) continue; try { await this.agent.prompt(batch); } catch (err) { diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 0e2ce28e1..daad3960d 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -106,7 +106,7 @@ export const TAB_METADATA: Record = { appearance: ["Theme", "Status Line", "Display", "Images"], - model: ["Thinking", "Sampling", "Prompt", "Retry & Fallback"], + model: ["Thinking", "Sampling", "Prompt", "Retry & Fallback", "Advisor"], interaction: [ "Input", "Approvals", diff --git a/packages/coding-agent/src/prompts/advisor/system.md b/packages/coding-agent/src/prompts/advisor/system.md index df6a68179..4dfbc6095 100644 --- a/packages/coding-agent/src/prompts/advisor/system.md +++ b/packages/coding-agent/src/prompts/advisor/system.md @@ -1,9 +1,31 @@ -You are a senior engineer silently watching another agent work. You receive that agent's transcript incrementally, including its private thinking, rendered as concise markdown. + +RFC 2119 applies: MUST, SHOULD, AVOID, NEVER (= NEVER). You are a pair programmer with independent perspective — your only output is the `advise` tool. +You can explore the workspace; budget is 2–3 tool calls per advise (exception: critical bugs warrant deeper verification before raising a blocker). + -You cannot change anything or run commands. You have read-only access to the workspace through `read`, `search`, and `find` — use them sparingly to verify a suspicion (confirm an API exists, check a callsite, read the function under edit) before weighing in. The only way you can speak to the agent is by calling the `advise` tool. +You bring a different angle. +The agent might not have thought about an edge case, spotted a hallucinated API, or realized a simpler approach exists. +Your job is to offer that view before they sink work into the wrong direction. -Call `advise` only for things that materially matter: a wrong approach, a missed edge case or failure mode, a hallucinated fact or API, scope creep beyond the task, going in circles, or a likely bug about to be written. Prefer silence. If the agent is on track, do not call any tool. + +You receive the agent's transcript incrementally, including private thinking. +You have read-only access through `read`, `search`, `find` to verify your suspicions. +Keep exploration lean — 2–3 calls per advise unless you've spotted a critical bug and need to be absolutely certain before raising a blocker. + -At most one `advise` per update. Keep each note to one or two sentences. Address the agent in second person. Never restate what it already knows. Never give meta-instructions about how to use the advisor. + +One `advise` per update. Address the agent directly. Offer alternatives, not lectures. Never restate what they know; never explain how to use the advisor. + -Severity controls delivery: a `nit` is folded in non-interruptingly at the next step boundary, while a `concern` or `blocker` interrupts the agent mid-work to reach it immediately. Reserve `concern`/`blocker` for advice worth stopping the agent for; default to `nit` for anything that can wait. Use `blocker` only when continuing will clearly waste the turn. + +You SHOULD call `advise` when: agent might be heading the wrong way, missed an edge case, about to call a hallucinated API, going in circles, picking brittle approach over better one. Low confidence bar — "this might be wrong" is worth noting if they didn't think about it. +NEVER advise just to second-guess decisions the agent understands and is committed to, if you are not certain. + + + +**`nit`** — Non-urgent cleanup, refactor, style, missed opportunity. Folded at next step boundary; agent keeps working. Examples: edge cases that don't break correctness, simplifications, better approach the agent can consider. +**`concern`** — Agent might be heading wrong or missed something material. Offers your view; agent decides. Use when: exploring wrong code path, picking fragile approach when better exists, missing constraint, hallucinated API, going in circles, edge case about to be baked in. +**`blocker`** — Stop and reconsider. Use ONLY when: continuing will clearly waste the turn, produce broken output, or the path is fundamentally unsound. Verify thoroughly before raising. + + +You MAY suggest an approach or fix if you've explored enough to be confident. Your job is pair programming, not just bugs — offer the better designs, not just the warning. diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 5a7e206db..c01d0bac1 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -30,6 +30,7 @@ import { type AgentTool, AppendOnlyContextManager, type AsideMessage, + type CompactionSummaryMessage, resolveTelemetry, ThinkingLevel, } from "@oh-my-pi/pi-agent-core"; @@ -54,6 +55,8 @@ import { generateHandoff, prepareCompaction, resolveThresholdTokens, + type SessionEntry, + type SessionMessageEntry, type ShakeConfig, type ShakeRegion, type SummaryOptions, @@ -1516,6 +1519,7 @@ export class AgentSession { this.#advisorRuntime = new AdvisorRuntime(advisorAgentFacade, { snapshotMessages: () => this.agent.state.messages, enqueueAdvice, + maintainContext: incomingTokens => this.#maintainAdvisorContext(incomingTokens), }); if (seedToCurrent) { this.#advisorRuntime.seedTo(this.agent.state.messages.length); @@ -1553,6 +1557,203 @@ export class AgentSession { this.#advisorYieldQueueUnsubscribe = undefined; } + async #promoteAdvisorContextModel(currentModel: Model): Promise { + const promotionSettings = this.settings.getGroup("contextPromotion"); + if (!promotionSettings.enabled) return false; + const contextWindow = currentModel.contextWindow ?? 0; + if (contextWindow <= 0) return false; + const targetModel = await this.#resolveContextPromotionTarget(currentModel, contextWindow); + if (!targetModel) return false; + + const advisorSel = resolveRoleSelection( + ["advisor"], + this.settings, + this.#modelRegistry.getAvailable(), + this.#modelRegistry, + ); + const advisorThinkingLevel = advisorSel?.thinkingLevel ?? ThinkingLevel.Medium; + + try { + this.#advisorAgent?.setModel(targetModel); + this.#advisorAgent?.setThinkingLevel(toReasoningEffort(advisorThinkingLevel)); + this.#advisorAgent?.setDisableReasoning(shouldDisableReasoning(advisorThinkingLevel)); + this.#advisorAgent?.appendOnlyContext?.invalidateForModelChange(); + logger.debug("Advisor context promotion switched model on overflow", { + from: `${currentModel.provider}/${currentModel.id}`, + to: `${targetModel.provider}/${targetModel.id}`, + }); + return true; + } catch (error) { + logger.warn("Advisor context promotion failed", { + from: `${currentModel.provider}/${currentModel.id}`, + to: `${targetModel.provider}/${targetModel.id}`, + error: String(error), + }); + return false; + } + } + + async #maintainAdvisorContext(incomingTokens: number): Promise { + const advisor = this.#advisorAgent; + if (!advisor) return false; + + const compactionSettings = this.settings.getGroup("compaction"); + if (compactionSettings.strategy === "off") return false; + if (!compactionSettings.enabled) return false; + + const advisorModel = advisor.state.model; + const contextWindow = advisorModel.contextWindow ?? 0; + if (contextWindow <= 0) return false; + + const messages = advisor.state.messages; + let contextTokens = incomingTokens; + for (const message of messages) { + contextTokens += estimateTokens(message); + } + + if (!shouldCompact(contextTokens, contextWindow, compactionSettings)) { + return false; + } + + // 1. Try promotion first + if (await this.#promoteAdvisorContextModel(advisorModel)) { + // Promotion succeeded, check if new model has enough space + const newModel = advisor.state.model; + const newWindow = newModel.contextWindow ?? 0; + if (newWindow > 0) { + const stillNeedsCompaction = shouldCompact(contextTokens, newWindow, compactionSettings); + if (!stillNeedsCompaction) return false; + } + } + + // 2. Run compaction on advisor messages + const pathEntries: SessionEntry[] = messages.map((message, i) => { + const id = `msg-${i}`; + const parentId = i > 0 ? `msg-${i - 1}` : null; + const timestamp = String(message.timestamp || Date.now()); + + if (message.role === "compactionSummary") { + return { + type: "compaction", + id, + parentId, + timestamp, + summary: message.summary, + shortSummary: message.shortSummary, + firstKeptEntryId: (message as any).firstKeptEntryId || `msg-${i + 1}`, + tokensBefore: message.tokensBefore, + } satisfies CompactionEntry; + } + + return { + type: "message", + id, + parentId, + timestamp, + message, + } satisfies SessionMessageEntry; + }); + + const preparation = prepareCompaction(pathEntries, compactionSettings); + if (!preparation) { + // Cannot prepare compaction, fallback to re-prime + return true; + } + + let action: "context-full" | "snapcompact" = + compactionSettings.strategy === "snapcompact" && advisorModel.input.includes("image") + ? "snapcompact" + : "context-full"; + + let summary: string; + let shortSummary: string | undefined; + let firstKeptEntryId: string; + let tokensBefore: number; + + // Try snapcompact first if vision model + let snapcompactResult: snapcompact.CompactionResult | undefined; + if (action === "snapcompact") { + try { + snapcompactResult = await snapcompact.compact(preparation, { + convertToLlm: messages => this.#convertToLlmForSideRequest(messages), + model: advisorModel, + maxFrames: snapcompact.providerFrameBudget(advisorModel.provider), + }); + const budget = contextWindow - effectiveReserveTokens(contextWindow, compactionSettings); + const projected = this.#projectSnapcompactContextTokens(preparation, snapcompactResult); + if (projected > budget) { + action = "context-full"; + snapcompactResult = undefined; + } + } catch (err) { + logger.warn("Advisor snapcompact failed, falling back to LLM summary", { error: String(err) }); + action = "context-full"; + } + } + + if (snapcompactResult) { + summary = snapcompactResult.summary; + shortSummary = snapcompactResult.shortSummary; + firstKeptEntryId = snapcompactResult.firstKeptEntryId; + tokensBefore = snapcompactResult.tokensBefore; + } else { + // Run LLM-summary compaction + const availableModels = this.#modelRegistry.getAvailable(); + const candidates = this.#resolveCompactionModelCandidates(advisorModel, availableModels); + if (candidates.length === 0) { + // No compaction candidates, fallback to re-prime + return true; + } + + let compactResult: CompactionResult | undefined; + let lastError: unknown; + + for (const candidate of candidates) { + const apiKey = await this.#modelRegistry.getApiKey( + candidate, + this.sessionId ? `${this.sessionId}-advisor` : undefined, + ); + if (!apiKey) continue; + + try { + compactResult = await compact( + preparation, + candidate, + this.#modelRegistry.resolver(candidate, this.sessionId ? `${this.sessionId}-advisor` : undefined), + undefined, + undefined, + { + thinkingLevel: toReasoningEffort(advisor.state.thinkingLevel), + convertToLlm: messages => this.#convertToLlmForSideRequest(messages), + }, + ); + break; + } catch (error) { + lastError = error; + } + } + + if (!compactResult) { + logger.warn("Advisor compaction failed, falling back to re-prime", { error: String(lastError) }); + return true; + } + + summary = compactResult.summary; + shortSummary = compactResult.shortSummary; + firstKeptEntryId = compactResult.firstKeptEntryId; + tokensBefore = compactResult.tokensBefore; + } + + // Rebuild messages with the compaction summary + const summaryMessage = { + ...createCompactionSummaryMessage(summary, tokensBefore, new Date().toISOString(), shortSummary), + firstKeptEntryId, + } as CompactionSummaryMessage & { firstKeptEntryId?: string }; + + advisor.replaceMessages([summaryMessage, ...preparation.recentMessages]); + return false; + } + /** Model registry for API key resolution and model discovery */ get modelRegistry(): ModelRegistry { return this.#modelRegistry; @@ -8025,6 +8226,10 @@ export class AgentSession { } #getCompactionModelCandidates(availableModels: Model[]): Model[] { + return this.#resolveCompactionModelCandidates(this.model, availableModels); + } + + #resolveCompactionModelCandidates(preferredModel: Model | null | undefined, availableModels: Model[]): Model[] { const candidates: Model[] = []; const seen = new Set(); @@ -8036,15 +8241,9 @@ export class AgentSession { candidates.push(model); }; - const currentModel = this.model; - // Prefer the active session's model: it's what the user is actively using, - // and routing compaction to a different provider (e.g. an OpenAI default - // model while the chat is on Anthropic) changes provider-specific behavior - // like remote compaction endpoints. Role-based candidates only kick in - // as auth fallbacks when the current model has no usable credentials. - addCandidate(currentModel); + addCandidate(preferredModel ?? undefined); for (const role of MODEL_ROLE_IDS) { - addCandidate(this.#resolveRoleModelFull(role, availableModels, currentModel).model); + addCandidate(this.#resolveRoleModelFull(role, availableModels, preferredModel ?? undefined).model); } const sortedByContext = [...availableModels].sort((a, b) => (b.contextWindow ?? 0) - (a.contextWindow ?? 0)); diff --git a/packages/coding-agent/test/modes/components/assistant-message-error.test.ts b/packages/coding-agent/test/modes/components/assistant-message-error.test.ts index 1050b8eae..73e4716c0 100644 --- a/packages/coding-agent/test/modes/components/assistant-message-error.test.ts +++ b/packages/coding-agent/test/modes/components/assistant-message-error.test.ts @@ -173,8 +173,8 @@ describe("AssistantMessageComponent streaming thinking pulse", () => { return lines; } - // First frame of the breathing ·‥…‥ pulse; deterministic right after updateContent. - const PULSE = "·"; + // First frame of the breathing ▁▃▄▃ pulse; deterministic right after updateContent. + const PULSE = "▁"; it("shows the pulse in place of hidden reasoning while thinking streams", () => { const lines = liveLines(streaming([{ type: "thinking", thinking: "private reasoning" }]));