mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat(coderd/x/chatd/chatadvisor): add advisor runtime and tool wrapper (#24620)
## Summary Introduce the `coderd/x/chatd/chatadvisor` package: the self-contained runtime that performs a nested, **tool-less, single-step** model call to return strategic guidance to the parent agent. Also ships the thin `AgentTool` wrapper that the agent exposes as the `advisor` built-in. ## Motivation The advisor is a "consult before you act" planning step. Keeping its runtime in its own package (rather than inside `chatd.go` or `chattool/`) makes the behavior easy to unit-test, keeps `chattool/` thin, and avoids import cycles with `chatprompt`. ## Changes - `chatadvisor/types.go`: `AdvisorArgs` and `AdvisorResult` (`advice` / `limit_reached` / `error` variants) plus usage metadata. Stable JSON shape for the UI. - `chatadvisor/guidance.go`: the nested advisor system prompt and `ParentGuidanceBlock` injected into the outer agent (tagged `<advisor-guidance>`). The nested prompt explicitly tells the advisor it is advising the parent agent, must not address the user, and must not claim actions happened. - `chatadvisor/handoff.go`: builds the advisor input from the already-prepared outer prompt tail (not a fresh DB reload) with hard truncation budgets and a recent-context bias so the advisor sees the exact context the outer model saw. - `chatadvisor/runner.go`: wraps `chatloop.Run()` in strict one-step mode (`Tools: nil`, `ProviderTools: nil`, `MaxSteps: 1`) so the nested call is structurally incapable of calling tools. - `chatadvisor/tool.go`: the `AgentTool` wrapper. Validates the question (non-empty, bounded to 2000 runes), invokes the runtime, and maps the result to a JSON tool response. - Defensive assertions throughout (question non-empty after trim, positive limits, zero tools on nested call, known result variants, non-negative remaining uses). - Unit tests cover question validation, tool-less execution, and each result variant. ## Stack context This is **PR 2 of 6** in the advisor feature stack. It depends on PR 1's `ExclusiveToolNames` hook; no consumer of this package exists yet (that lands in PR 4). ## Scope / non-goals - No wiring into `chatd`. - No HTTP/API surface. - No UI. - No separate provider; the runtime reuses the outer chat's resolved model/provider/keys. ## Validation - `go test ./coderd/x/chatd/chatadvisor/...` - `make lint` --- <details> <summary>📋 Implementation Plan (shared across the advisor stack)</summary> # Plan: Add a Mux-style advisor tool to coder agents/chatd ## Outcome Add a first-class `advisor` tool to agent chats in `coderd/x/chatd` that feels native to Coder: - it is a built-in server-side tool, not an MCP/dynamic-tool workaround; - it performs a nested **tool-less** model call for strategic advice; - it is exposed only when eligible, and the prompt mentions it only when it is actually available; - it is treated as a **planning-only** tool so it does not run alongside action tools in the same batch; - it tracks usage/cost separately enough for operators to reason about it; - it has a minimally polished UI in the Agents page; - and it ships with explicit dogfooding evidence, including screenshots and repro videos. ## Design decisions to lock before coding 1. **Primary architecture:** native built-in tool in `chattool/`, backed by a small `chatadvisor` package. 2. **Nested model execution:** reuse chatd's existing model/provider stack for a one-step, tool-less advisor call rather than inventing a new provider pathway. 3. **Execution policy:** treat `advisor` as an exclusive/planning-only tool; mixed batches must return structured policy errors and force the model to retry cleanly. 4. **Availability:** initial rollout is for root agent chats only; disable for child/sub-agent chats until recursion/cost policy is proven. 5. **Prompt sync:** use one eligibility boolean to drive both tool registration and advisor guidance injection. 6. **Persistence/cost split:** MVP should keep advisor usage visible in result metadata and server metrics; only add DB schema if product/billing explicitly needs queryable advisor-specific cost. 7. **UI scope:** generic tool rendering is an acceptable temporary milestone during backend bring-up, but the release candidate should include a dedicated lightweight advisor renderer. ## Delivery model The work should be executed as coordinated workstreams with one integration owner and parallel contributors for low-conflict areas. The integration owner should own `coderd/x/chatd/chatd.go` because prompt assembly, tool registration, and model resolution all converge there. ## Detailed workstreams ### Repo evidence used for this plan <details> <summary>Mux reference and current chatd seams</summary> **Mux reference implementation** - `src/node/services/tools/advisor.ts` — native advisor tool implementation. - `src/common/constants/advisor.ts` — advisor prompt/constants and truncation policy. - `src/common/utils/tools/tools.ts` — conditional tool registration. - `src/node/services/streamContextBuilder.ts` — injects advisor guidance only when the tool is available. **Current chatd seams** - `coderd/x/chatd/chatd.go` - `processChat()` — tool assembly, prompt assembly, and chatloop invocation. - `resolveChatModel()` — current model/provider/key resolution seam. - `type Config struct` — server-level chatd configuration surface. - `coderd/x/chatd/chatloop/chatloop.go` - `Run()` — main streaming/model loop. - `executeTools()` — built-in tool execution/batching seam. - `coderd/x/chatd/chattool/` — built-in tool implementations. - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` — tool renderer dispatch. - `site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts` and `ConversationTimeline.tsx` — tool/result merge and rendering flow. </details> ### Workstream map and ownership | Workstream | Primary owner | Main files | Can run in parallel? | Done when | |---|---|---|---|---| | 0. Integration + gating | Integration lead | `coderd/x/chatd/chatd.go` | No; central merge lane | Tool registration, prompt sync, and model selection are wired together | | 1. Advisor runtime + tool | Backend agent | new `coderd/x/chatd/chatadvisor/`, new `coderd/x/chatd/chattool/advisor.go` | Yes | Tool can perform a tool-less advisor call in memory and return structured results | | 2. Planning-only execution policy | Chatloop agent | `coderd/x/chatd/chatloop/chatloop.go`, related tests | Yes | Mixed `advisor` + action-tool batches are rejected cleanly and deterministically | | 3. Metrics/usage/config | Backend/telemetry agent | `chatd.go`, `chatloop/metrics.go`, optional config plumbing | Partially; coordinate with integration lead | Advisor usage is separately visible in metadata/metrics and limits are enforced | | 4. Frontend rendering | Frontend agent | `site/.../tools/Tool.tsx`, new `AdvisorTool.tsx`, stories | Yes after result schema stabilizes | Advisor renders as a readable card and story tests pass | | 5. Dogfood + QA evidence | QA agent | dev server, Storybook, dogfood output | After backend + UI are usable | Repro videos, screenshots, and a concise QA report exist | ### Parallelization rules - **Do not split `coderd/x/chatd/chatd.go` across multiple execution agents without an integration lead.** That file owns prompt building, tool registration, model resolution, and cost persistence. - Workstreams 1 and 2 can be developed in parallel and then stacked onto the integration branch. - Workstream 4 should begin once the backend result schema is agreed on, even if the backend is still behind a feature flag. - Any agent that needs to re-check Mux behavior should clone `coder/mux` into a temporary directory (for example, `$(mktemp -d)/mux`) and inspect it read-only; do not vendor or copy code from Mux directly. ## Phase 0 — Preflight and guardrails ### Goals - Align the team on the smallest shippable architecture. - Prevent scope creep into MCP/dynamic-tool/sub-agent variants. - Decide upfront what is MVP vs. follow-up. ### Tasks 1. **Confirm the MVP boundary.** - Ship a built-in advisor tool first. - Do **not** make MCP, dynamic tools, or sub-agents the primary implementation. - Do **not** add transient streaming phases in the first backend PR unless they fall out almost for free. 2. **Confirm local workflow hygiene before coding.** - Ensure the repo is using the project git hooks from `scripts/githooks`. - Do not bypass hooks with `--no-verify`. - Use `./scripts/develop.sh` for the full dev server rather than manual build/run commands. 3. **Lock the model-selection policy.** - **Recommended MVP:** advisor uses the same resolved provider/model/cost config as the current chat, with advisor-specific max-output and usage caps. - **Follow-up only if required:** add a separate `AdvisorModelConfigID`-style override that resolves through the existing `configCache`/model-config path. Do not invent a new free-form `provider:model` parser if chatd already stores provider/model separately. 4. **Lock the persistence policy.** - **Recommended MVP:** no DB migration. Persist advisor-visible metadata in the tool result and record separate metrics in memory/Prometheus. - **Only if product/billing explicitly asks for queryable advisor cost:** add a later DB migration or usage table, following the normal `queries/*.sql` + `make gen` workflow. 5. **Create an execution ADR note in the work item or tracking doc.** - Capture: built-in tool, tool-less nested call, root-chat-only rollout, exclusive execution policy, MVP no-DB-migration default. ### Quality gate - Everyone on the team can state the same answers to these questions: - Is advisor a built-in tool? **Yes.** - Can advisor run with action tools in the same batch? **No.** - Does advisor get tools of its own? **No.** - Is a DB migration required for MVP? **No, unless billing insists.** ## Phase 1 — Build the advisor runtime and tool wrapper ### Goals Create the core advisor implementation in a way that is easy to test and keeps `chattool/` thin. ### Files to add - `coderd/x/chatd/chatadvisor/types.go` - `coderd/x/chatd/chatadvisor/guidance.go` - `coderd/x/chatd/chatadvisor/handoff.go` - `coderd/x/chatd/chatadvisor/runtime.go` - `coderd/x/chatd/chatadvisor/runner.go` - `coderd/x/chatd/chattool/advisor.go` ### Responsibilities by file 1. **`types.go`** - Define the input/result schema used by the tool and UI. - Keep the result shape close to Mux so the UI and model both have predictable cases. - Recommended result variants: - `advice` - `limit_reached` - `error` Recommended shape: ```go type AdvisorArgs struct { Question string `json:"question"` } type AdvisorResult struct { Type string `json:"type"` Advice string `json:"advice,omitempty"` Error string `json:"error,omitempty"` AdvisorModel string `json:"advisor_model,omitempty"` RemainingUses int `json:"remaining_uses,omitempty"` Usage *AdvisorUsageResult `json:"usage,omitempty"` } ``` 2. **`guidance.go`** - Hold two strings: - the nested advisor system prompt; - the parent-agent guidance block to inject into the outer system prompt. - The nested advisor prompt must say, in plain language: - you are advising the parent agent; - you do not address the end user directly; - you do not claim actions happened; - you return concise strategic guidance and tradeoffs. 3. **`runtime.go`** - Define the per-run runtime state. - Recommended fields: - resolved model + model config; - provider keys/options reused from the outer chat; - `MaxUsesPerRun`; - `MaxOutputTokens`; - atomic/current call counter; - callback(s) to obtain the current prompt snapshot and current-step snapshot; - optional metrics/usage hook. - Add fail-fast validation for impossible config: nil model, non-positive limits, empty prompt builders, etc. 4. **`handoff.go`** - Build the advisor handoff message from: - the explicit question; - the exact prompt/messages the parent model just used; - the current step's text/reasoning snapshot, if available; - the most recent relevant tool outputs, if they are already in the prompt snapshot. - **Important:** use the already-prepared outer prompt tail, not a fresh DB reload. That keeps the advisor aligned with compaction and the exact context the outer model saw. - Apply hard truncation budgets with recent-context bias. 5. **`runner.go`** - Execute the nested advisor call. - **Recommended implementation:** call `chatloop.Run()` in an in-memory, one-step mode: - `Tools: nil` - `ProviderTools: nil` - `MaxSteps: 1` - `PersistStep`: capture the assistant output in memory instead of writing DB rows - Reuse the existing provider/model/cost path instead of building a second provider runner. - Assert that no tool definitions are passed to the nested call. 6. **`chattool/advisor.go`** - Keep this file thin and consistent with other built-ins. - Responsibilities: - decode `AdvisorArgs`; - validate `Question` is non-empty and bounded; - call the `chatadvisor` runner; - return a structured tool response. ### Defensive programming requirements - Assert `Question` is non-empty after trimming. - Assert runtime limits are positive. - Assert the nested advisor call runs with zero tools/provider tools. - Assert `AdvisorResult.Type` is one of the known variants before returning. - Assert remaining uses never goes negative. ### Acceptance criteria - A unit test can call the advisor tool with a fake model and receive a stable `advice` result. - The nested advisor call is impossible to run with tools accidentally attached. - The core logic lives in `chatadvisor/`, not embedded inside `chatd.go`. ## Phase 2 — Wire advisor into chatd and keep prompt/tool availability in sync ### Goals Register the tool in the right place, expose it only when eligible, and inject system guidance only when the tool is present. ### Files to modify - `coderd/x/chatd/chatd.go` - optionally a small helper file if `chatd.go` becomes too crowded ### Tasks 1. **Compute one eligibility boolean in `processChat()`.** Recommended inputs: - server-level advisor enabled flag; - root chat only (`chat.ParentChatID == uuid.Nil` or equivalent existing root/child check); - a usable resolved model/provider exists; - optional experiment/workspace/org gate if product wants staged rollout. 2. **Create the runtime once per outer chat run.** - Use the model/config/keys resolved by `resolveChatModel()`. - Reuse provider options from the current chat's `ChatModelCallConfig`. - Set `MaxUsesPerRun` and `MaxOutputTokens` from advisor config defaults. 3. **Register the tool in the built-in tool block.** - Insert after the skill tools and before MCP tools in `processChat()`. - Record `builtinToolNames["advisor"] = true` so metrics stay bounded. 4. **Inject advisor guidance into the outer system prompt using the same boolean.** - Use `chatprompt.InsertSystem()` in the same prompt assembly path that already injects user/system instructions. - Place the block near the existing instruction insertion, before plan-path/skill context blocks. - Wrap the guidance in an explicit tag like `<advisor-guidance>` so it is easy to spot in tests and future refactors. 5. **Keep advisor out of child chats for the first release.** - That avoids recursion/cost blowups with `spawn_agent` / `wait_agent` flows. - Document this explicitly in the rollout notes and tests. ### Acceptance criteria - If advisor is disabled, neither the tool nor the prompt guidance appears. - If advisor is enabled, both the tool and the prompt guidance appear. - Root chats can use advisor; child chats cannot. - Built-in tool names include `advisor` so metrics do not collapse it into the generic `mcp` label. ## Phase 3 — Enforce planning-only execution policy in `chatloop` ### Goals Prevent the model from calling `advisor` and action tools in the same execution batch. ### Files to modify - `coderd/x/chatd/chatloop/chatloop.go` - related chatloop tests ### Recommended implementation Keep the MVP small; do **not** build a general policy engine yet. 1. Add a minimal field to `chatloop.RunOptions`, for example: ```go ExclusiveToolName *string ``` 2. In `Run()` / `executeTools()`, detect the case where the exclusive tool appears in the same local-tool batch as any other locally executed tool. 3. When that happens, synthesize structured tool-result errors for the affected calls instead of executing anything in the batch. - `advisor` should receive a clear error like: _advisor must be called by itself before action tools_. - The sibling action tools should receive a paired policy error like: _this tool was skipped because advisor must run alone_. 4. Let the outer model see those tool errors and retry cleanly. - This is simpler and safer than partial execution or hidden deferral. - It preserves deterministic transcript history for debugging. 5. Pass the just-finished step snapshot into the tool execution context. - The advisor runtime should be able to see the current step's text/reasoning content, because that is often the best hint about what the outer model is trying to decide. ### Why this is the right fit - It matches the intended semantics: advisor is consulted **before** taking action. - It avoids subtle race conditions caused by concurrent built-in tool execution. - It keeps the behavior easy to test with fake models. ### Acceptance criteria - A model-emitted batch containing only `advisor` succeeds. - A model-emitted batch containing `advisor` plus any other locally executed tool returns deterministic policy errors and executes nothing. - Non-advisor tool execution stays unchanged for normal chats. ## Phase 4 — Usage limits, metrics, and configuration ### Goals Make advisor safe to operate without over-designing billing/storage in the first release. ### Files to modify - `coderd/x/chatd/chatd.go` - `coderd/x/chatd/chatloop/metrics.go` as needed - `coderd/x/chatd/chatd.go` `Config` struct and constructor path - optional follow-up config/db files only if a separate advisor model or persistent billing is required ### Tasks 1. **Add explicit server config knobs for MVP.** Recommended fields on `chatd.Config` or a nested advisor config struct: - `AdvisorEnabled bool` - `AdvisorMaxUsesPerRun int` - `AdvisorMaxOutputTokens int64` 2. **Track usage per outer run.** - Reset the counter for each `processChat()` invocation. - Return `remaining_uses` in the tool result. - Return `limit_reached` when the cap is exhausted. 3. **Expose advisor usage metadata in the tool result.** - Include model name and token/cost summary if available. - Use the same `callConfig.Cost` calculation path as the outer chat for MVP if advisor reuses the same model. 4. **Record server-side metrics.** - Count advisor invocations, failures, and latency. - Ensure they show up under the built-in tool label `advisor`. 5. **Optional decision gate: separate advisor model.** - If product insists on a stronger/different advisor model, add a follow-up config hook that resolves another existing chat model config through the same `configCache` path. - Keep that out of the first landing PR unless it is required for acceptance. 6. **Optional decision gate: queryable advisor cost.** - If this becomes required, spin a follow-up DB task: - update `coderd/database/queries/*.sql`; - add migration files; - run `make gen`; - update audit mappings if a new auditable type/field is introduced. ### Acceptance criteria - Advisor calls are capped per outer run. - Limit exhaustion is user-visible in the tool result. - Metrics distinguish advisor calls from other built-in tools. - MVP does not require a schema migration unless explicitly approved. ## Phase 5 — Frontend rendering and Storybook coverage ### Goals Make advisor feel intentional in the Agents UI without blocking the backend on fancy streaming UI. ### Files to modify - `site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx` - new `site/src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.tsx` - Storybook story file(s) in the same tools directory ### Delivery strategy 1. **Intermediate milestone during backend bring-up:** rely on the existing generic tool renderer if needed. - This is acceptable only as a short-lived integration checkpoint. 2. **Release milestone:** add a dedicated lightweight `AdvisorTool` renderer. - Reuse existing primitives: - `ToolCollapsible` - `ToolIcon` - `Response` for markdown/prose rendering - `ScrollArea` if the advice can be long - Keep styling light and consistent with the Agents page. - Do not add unnecessary React memoization in `site/src/pages/AgentsPage/`; that area is already React-Compiler aware. 3. **Render the structured result states cleanly.** - `advice` — readable prose/markdown with optional metadata footer. - `limit_reached` — warning-style message. - `error` — error state with visible fallback text. - `running` — existing tool loading state/spinner is enough for MVP. 4. **Add Storybook coverage instead of ad-hoc component tests.** Recommended stories: - successful advice; - running/loading; - limit reached; - error. 5. **Keep the UI contract narrow.** - Prefer one text field like `advice` plus small metadata rather than a deeply nested schema. - That keeps the UI resilient to prompt iteration. ### Acceptance criteria - The advisor tool card renders readable content rather than raw quoted JSON in the final release branch. - Running, limit, and error states are visibly distinct. - Storybook stories and play assertions cover the new states. - Existing tool rendering flows remain unchanged. ## Phase 6 — Automated tests and validation gates ### Backend tests to add 1. **Advisor runtime/tool tests** - question validation; - tool-less nested execution assertion; - success result shaping; - limit-reached result shaping; - error result shaping. 2. **Prompt/gating tests in chatd** - advisor disabled ⇒ no tool, no guidance; - advisor enabled/root chat ⇒ tool + guidance; - child chat ⇒ advisor absent. 3. **Chatloop policy tests** - advisor alone runs; - advisor + action tool mixed batch returns deterministic policy errors; - non-advisor tools still execute normally. 4. **Usage/metrics tests** - per-run cap resets correctly; - builtin tool labeling includes `advisor`; - returned metadata includes model/usage summary when available. ### Frontend tests to add - Storybook `play()` assertions for the advisor renderer states. - Verify expand/collapse behavior and visible fallback text. - Verify the message timeline still renders adjacent tools correctly. ### Recommended command sequence Run these as the implementation matures, not only at the end: 1. Backend-focused gate after phases 1–4: - `make test RUN=TestAdvisor` - `make test RUN=TestChatloopAdvisor` - `make lint` 2. Frontend-focused gate after phase 5: - `pnpm test:storybook src/pages/AgentsPage/components/ChatElements/tools/AdvisorTool.stories.tsx` - `pnpm lint` - `pnpm format` 3. Final repo gate before handoff: - `make pre-commit` - run any additional targeted `make test RUN=...` selections covering touched chatd paths > Use the exact new test names the implementing agents create; the names above are recommended anchors, not existing tests. ## Dogfooding plan ### Principle Dogfood the change as a real agent feature, not just a unit-tested backend. Per the dogfood and `agent-browser` skills, the reviewer should get **watchable repro videos** plus screenshots that make the behavior obvious without reading logs. ### Required setup 1. Start the full dev environment with: - `./scripts/develop.sh` 2. If the frontend renderer changes, also start Storybook from `site/` with: - `pnpm storybook --no-open` 3. Use `agent-browser` directly — **never `npx agent-browser`**. 4. Use named browser sessions and an output folder such as: - `./dogfood-output/advisor/` - with subfolders `screenshots/` and `videos/` ### Evidence protocol For every interactive scenario below: 1. Start video recording **before** the action. 2. Capture step-by-step screenshots at human pace. 3. Capture one annotated screenshot of the final state. 4. Stop the recording. 5. Note the exact pass/fail observation in the QA report. For static UI states (for example Storybook error/limit cards), an annotated screenshot is sufficient; video is optional but still encouraged by this project’s review preference. ### Dogfood scenarios #### Scenario A — Happy path in the real Agents UI **Goal:** prove that a root agent chat can invoke advisor and produce a readable recommendation before taking further action. Steps: 1. Open the Agents page with an advisor-enabled root chat. 2. Start a repro video. 3. Send a prompt that should reasonably trigger strategic planning, such as an architecture or multi-tradeoff question. 4. Capture screenshots of: - the prompt before send; - the running advisor state; - the completed advisor card and the assistant’s follow-up response. 5. Stop recording. Pass criteria: - advisor appears in the timeline; - the rendered result is readable; - the assistant can continue after consuming the advisor output. #### Scenario B — Advisor unavailable path **Goal:** prove the feature is truly gated. Suggested variants (at least one is required, both are better): - feature flag/config off; - child/sub-agent chat. Evidence: - annotated screenshot of the chat/tool state showing advisor is absent; - short video if toggling the gate live is part of the repro. Pass criteria: - no advisor tool is available; - no advisor-specific prompt behavior leaks through. #### Scenario C — UI states in Storybook **Goal:** prove the renderer handles non-happy states cleanly. Required story states: - success/advice; - running; - limit reached; - error. Evidence: - one screenshot per state; - at least one short video showing collapse/expand behavior. Pass criteria: - success renders readable advice; - limit/error have visible fallback text; - the component behaves like the other tool cards. #### Scenario D — Regression sweep of nearby tools **Goal:** ensure advisor does not break the surrounding chat timeline. Check at minimum: - another existing built-in tool still renders correctly near advisor; - sub-agent/tool cards still expand/collapse normally; - no obvious console errors appear in the Agents page during the advisor flow. Evidence: - screenshots of adjacent tool cards; - console/error capture if anything suspicious appears. ### `agent-browser` usage notes for the QA agent - Prefer `agent-browser batch` for 2+ sequential commands when no intermediate parsing is needed. - Use `snapshot -i` to discover interactive refs. - Re-snapshot after navigation or major DOM changes. - Avoid `wait --load networkidle` unless the page is known to go idle; prefer explicit element/text waits or short fixed waits. - Record videos at human pace and include pauses that a reviewer can follow. ## Rollout plan ### Initial rollout - Gate behind a server-side advisor-enabled flag. - Enable only for selected internal/root agent chats first. - Watch metrics for: - invocation count; - failure rate; - latency; - obvious retry loops. ### Expansion conditions Expand beyond the initial rollout only after the following are true: - mixed-batch policy behavior is stable; - cost impact is understood; - frontend UX is readable in production-like dogfood; - no recursion surprises have appeared with sub-agent flows. ### Explicit non-goals for the first release - advisor inside child/sub-agent chats; - provider-agnostic streaming phase UI; - MCP-based external advisor implementation; - mandatory DB-backed advisor cost reporting. ## Final acceptance checklist - [ ] `advisor` is a built-in chatd tool, not an MCP/dynamic-tool substitute. - [ ] The nested advisor call is tool-less and bounded to one in-memory step. - [ ] One eligibility boolean controls both tool registration and prompt guidance injection. - [ ] Root chats can use advisor; child chats cannot in the initial rollout. - [ ] Mixed advisor/action batches produce deterministic policy errors instead of partial execution. - [ ] Per-run usage caps and limit-reached behavior work. - [ ] Advisor usage is visible in metadata/metrics without forcing a DB migration for MVP. - [ ] The Agents UI has a readable advisor card and Storybook coverage. - [ ] Dogfooding produced screenshots and repro videos for the required scenarios. - [ ] Validation commands (`make lint`, targeted `make test`, Storybook tests, `make pre-commit`) passed before handoff. ## Suggested PR split 1. **PR 1 — Backend foundation** - `chatadvisor/` package - `chattool/advisor.go` - `chatloop` exclusive policy - chatd gating/prompt sync - backend tests 2. **PR 2 — Frontend + QA** - advisor renderer - stories/play assertions - dogfood artifacts and QA notes 3. **PR 3 — Optional follow-ups only if demanded by stakeholders** - separate advisor model override - persistent advisor billing/queryability - transient phase-stream UX </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-opus-4-7` • Thinking: `max`_
This commit is contained in:
Thomas Kosiewski
@@ -0,0 +1,24 @@
|
||||
package chatadvisor
|
||||
|
||||
const (
|
||||
// AdvisorSystemPrompt steers the nested advisor model to help the parent
|
||||
// agent rather than speaking directly to the end user.
|
||||
AdvisorSystemPrompt = `You are an internal advisor for another AI coding agent.
|
||||
You are advising the parent agent, not the end user.
|
||||
Give concise strategic guidance that helps the parent decide what to do next.
|
||||
Focus on planning ambiguity, architecture tradeoffs, debugging strategy,
|
||||
and risk reduction.
|
||||
Do not address the user directly.
|
||||
Do not suggest using tools yourself because this nested run has no tools.
|
||||
Respond with practical guidance only.`
|
||||
|
||||
// ParentGuidanceBlock is a reusable prompt block for teaching parent agents
|
||||
// when to invoke the built-in advisor tool.
|
||||
ParentGuidanceBlock = `<advisor-guidance>
|
||||
Use the built-in advisor tool when you need strategic guidance on planning
|
||||
ambiguity, architectural tradeoffs, debugging strategy, or repeated failures.
|
||||
The advisor sees recent conversation context, runs as a single-step nested model
|
||||
call with no tools, and returns concise guidance for the parent agent rather
|
||||
than the end user.
|
||||
</advisor-guidance>`
|
||||
)
|
||||
@@ -0,0 +1,208 @@
|
||||
package chatadvisor
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"maps"
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
"charm.land/fantasy"
|
||||
)
|
||||
|
||||
const (
|
||||
// advisorRecentMessageLimit caps how many recent non-system messages
|
||||
// from the parent conversation are forwarded to the advisor. The
|
||||
// advisor only needs enough tail to ground its guidance, not the full
|
||||
// history.
|
||||
advisorRecentMessageLimit = 20
|
||||
// advisorConversationJSONByteBudget caps the combined size of the
|
||||
// forwarded recent messages, measured as JSON-serialized bytes (not
|
||||
// raw text runes). The JSON wrapping inflates the count relative to
|
||||
// user-visible text, so the effective text budget is smaller than the
|
||||
// number suggests. The walk stops at the first message that would
|
||||
// overflow, trading breadth for contiguity.
|
||||
advisorConversationJSONByteBudget = 12000
|
||||
// advisorSystemJSONByteBudget caps the combined size of inherited
|
||||
// system messages forwarded to the advisor. Without a cap, a large
|
||||
// parent system prompt (long injected instructions, accumulated
|
||||
// context) could push the advisor call past the model's context
|
||||
// window on top of the advisor contract, the recent tail, and the
|
||||
// question, surfacing as a provider error instead of advice.
|
||||
advisorSystemJSONByteBudget = 12000
|
||||
defaultAdvisorQuestion = "Provide concise strategic guidance for the parent agent."
|
||||
)
|
||||
|
||||
// BuildAdvisorMessages prepares a nested advisor prompt using the recent chat
|
||||
// context plus the explicit advisor question.
|
||||
func BuildAdvisorMessages(
|
||||
question string,
|
||||
conversationSnapshot []fantasy.Message,
|
||||
) []fantasy.Message {
|
||||
trimmedQuestion := strings.TrimSpace(question)
|
||||
if trimmedQuestion == "" {
|
||||
trimmedQuestion = defaultAdvisorQuestion
|
||||
}
|
||||
|
||||
messages := make([]fantasy.Message, 0, len(conversationSnapshot)+2)
|
||||
|
||||
// Place inherited system messages before AdvisorSystemPrompt so the
|
||||
// advisor contract is the final system instruction the model sees.
|
||||
// Later system directives win when they conflict, and the parent's
|
||||
// prompt may tell the model to address the end user directly or use
|
||||
// tools. The advisor must override those behaviors, not be overridden
|
||||
// by them.
|
||||
//
|
||||
// Walk system messages newest-to-oldest when consuming the byte
|
||||
// budget so that truncation preserves the most recent directives.
|
||||
// The parent may have injected recent safety or user-instruction
|
||||
// blocks that should win over older foundational prompts, and later
|
||||
// directives override earlier ones anyway. After selection, restore
|
||||
// the original order before appending so the advisor still sees the
|
||||
// parent's intended directive sequence.
|
||||
inheritedSystem := make([]fantasy.Message, 0)
|
||||
remainingSystemBudget := advisorSystemJSONByteBudget
|
||||
for i := len(conversationSnapshot) - 1; i >= 0; i-- {
|
||||
msg := conversationSnapshot[i]
|
||||
if msg.Role != fantasy.MessageRoleSystem {
|
||||
continue
|
||||
}
|
||||
messageBytes := messageJSONByteCount(msg)
|
||||
if messageBytes > remainingSystemBudget {
|
||||
// Skip oversized inherited system messages rather
|
||||
// than forwarding them wholesale. A single massive
|
||||
// parent system prompt could otherwise push the
|
||||
// advisor prompt past the model's context window,
|
||||
// returning a provider error instead of advice.
|
||||
// Continue walking so smaller older directives can
|
||||
// still contribute; stopping here would drop them
|
||||
// solely because a newer sibling was oversized.
|
||||
continue
|
||||
}
|
||||
inheritedSystem = append(inheritedSystem, cloneMessage(msg))
|
||||
remainingSystemBudget -= messageBytes
|
||||
}
|
||||
slices.Reverse(inheritedSystem)
|
||||
messages = append(messages, inheritedSystem...)
|
||||
messages = append(messages, textMessage(fantasy.MessageRoleSystem, AdvisorSystemPrompt))
|
||||
|
||||
recent := make([]fantasy.Message, 0, min(len(conversationSnapshot), advisorRecentMessageLimit))
|
||||
remainingBudget := advisorConversationJSONByteBudget
|
||||
for i := len(conversationSnapshot) - 1; i >= 0; i-- {
|
||||
msg := conversationSnapshot[i]
|
||||
if msg.Role == fantasy.MessageRoleSystem {
|
||||
continue
|
||||
}
|
||||
if len(recent) >= advisorRecentMessageLimit {
|
||||
break
|
||||
}
|
||||
|
||||
messageBytes := messageJSONByteCount(msg)
|
||||
if messageBytes > remainingBudget {
|
||||
// Stop at the first message that doesn't fit so the
|
||||
// advisor window stays contiguous from most recent
|
||||
// backward. Skipping an oversized message would leave
|
||||
// the advisor with an invisible hole in the history,
|
||||
// where later messages reference context that is no
|
||||
// longer present.
|
||||
break
|
||||
}
|
||||
|
||||
recent = append(recent, cloneMessage(msg))
|
||||
remainingBudget -= messageBytes
|
||||
}
|
||||
slices.Reverse(recent)
|
||||
recent = dropOrphanToolMessages(recent)
|
||||
messages = append(messages, recent...)
|
||||
messages = append(messages, textMessage(fantasy.MessageRoleUser, trimmedQuestion))
|
||||
return messages
|
||||
}
|
||||
|
||||
// dropOrphanToolMessages removes tool-role messages whose tool-call references
|
||||
// have been truncated out of the recent window. Providers reject prompts with
|
||||
// tool_result blocks that do not have a matching tool_use, so a truncation cut
|
||||
// that lands between an assistant tool-call message and its tool-result message
|
||||
// would otherwise produce a provider error rather than advice. The backward
|
||||
// walk always picks up tool results before their originating assistant
|
||||
// message, so orphan results can only appear at the leading edge of the
|
||||
// recent window. A single forward pass tracking known tool-call IDs is
|
||||
// sufficient to drop them.
|
||||
func dropOrphanToolMessages(recent []fantasy.Message) []fantasy.Message {
|
||||
if len(recent) == 0 {
|
||||
return recent
|
||||
}
|
||||
known := make(map[string]struct{})
|
||||
result := make([]fantasy.Message, 0, len(recent))
|
||||
for _, msg := range recent {
|
||||
if msg.Role == fantasy.MessageRoleAssistant {
|
||||
for _, part := range msg.Content {
|
||||
call, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](part)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
known[call.ToolCallID] = struct{}{}
|
||||
}
|
||||
result = append(result, msg)
|
||||
continue
|
||||
}
|
||||
if msg.Role != fantasy.MessageRoleTool {
|
||||
result = append(result, msg)
|
||||
continue
|
||||
}
|
||||
|
||||
kept := make([]fantasy.MessagePart, 0, len(msg.Content))
|
||||
for _, part := range msg.Content {
|
||||
tr, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](part)
|
||||
if !ok {
|
||||
kept = append(kept, part)
|
||||
continue
|
||||
}
|
||||
if _, matched := known[tr.ToolCallID]; matched {
|
||||
kept = append(kept, part)
|
||||
}
|
||||
}
|
||||
if len(kept) == 0 {
|
||||
continue
|
||||
}
|
||||
trimmed := msg
|
||||
trimmed.Content = kept
|
||||
result = append(result, trimmed)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
func textMessage(role fantasy.MessageRole, text string) fantasy.Message {
|
||||
return fantasy.Message{
|
||||
Role: role,
|
||||
Content: []fantasy.MessagePart{
|
||||
fantasy.TextPart{Text: text},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func cloneMessage(msg fantasy.Message) fantasy.Message {
|
||||
cloned := msg
|
||||
cloned.Content = append([]fantasy.MessagePart(nil), msg.Content...)
|
||||
cloned.ProviderOptions = maps.Clone(msg.ProviderOptions)
|
||||
return cloned
|
||||
}
|
||||
|
||||
// messageJSONByteCount approximates the message's contribution to the
|
||||
// advisor prompt using the length of its JSON serialization. The JSON
|
||||
// wrapping ({"role":"...","content":[{"type":"text","text":"..."}]}) is
|
||||
// counted alongside the user-visible text; the measurement is intended
|
||||
// for budget accounting, not for reporting visible character counts.
|
||||
func messageJSONByteCount(msg fantasy.Message) int {
|
||||
data, err := json.Marshal(msg)
|
||||
if err == nil {
|
||||
return len(data)
|
||||
}
|
||||
|
||||
total := 0
|
||||
for _, part := range msg.Content {
|
||||
partData, partErr := json.Marshal(part)
|
||||
if partErr == nil {
|
||||
total += len(partData)
|
||||
}
|
||||
}
|
||||
return total
|
||||
}
|
||||
@@ -0,0 +1,98 @@
|
||||
package chatadvisor
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
|
||||
"charm.land/fantasy"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chatloop"
|
||||
)
|
||||
|
||||
// RunAdvisor executes a single, tool-less nested advisor call.
|
||||
func (rt *Runtime) RunAdvisor(
|
||||
ctx context.Context,
|
||||
question string,
|
||||
conversationSnapshot []fantasy.Message,
|
||||
) (AdvisorResult, error) {
|
||||
// Model, MaxUsesPerRun, and MaxOutputTokens are validated by NewRuntime.
|
||||
// Runtime fields are unexported so callers cannot bypass that.
|
||||
if strings.TrimSpace(question) == "" {
|
||||
return AdvisorResult{}, xerrors.New("advisor question is required")
|
||||
}
|
||||
|
||||
if !rt.tryAcquire() {
|
||||
return AdvisorResult{
|
||||
Type: ResultTypeLimitReached,
|
||||
RemainingUses: 0,
|
||||
}, nil
|
||||
}
|
||||
|
||||
// Clone per invocation and reset inherited state so chatloop cannot
|
||||
// mutate the Runtime's stored options across calls, and so the nested
|
||||
// call never runs as a chain-mode continuation against stale parent
|
||||
// state or persists an orphan stored response on the provider side.
|
||||
nestedProviderOptions := cloneProviderOptions(rt.cfg.ProviderOptions)
|
||||
resetProviderOptionsForNestedCall(nestedProviderOptions)
|
||||
|
||||
var persistedStep chatloop.PersistedStep
|
||||
runOpts := chatloop.RunOptions{
|
||||
Model: rt.cfg.Model,
|
||||
Messages: BuildAdvisorMessages(question, conversationSnapshot),
|
||||
MaxSteps: 1,
|
||||
ModelConfig: rt.cfg.ModelConfig,
|
||||
ProviderOptions: nestedProviderOptions,
|
||||
PersistStep: func(_ context.Context, step chatloop.PersistedStep) error {
|
||||
persistedStep = step
|
||||
return nil
|
||||
},
|
||||
}
|
||||
|
||||
if err := chatloop.Run(ctx, runOpts); err != nil {
|
||||
// Refund the use so a transient provider failure does not
|
||||
// permanently exhaust the per-run advisor budget.
|
||||
rt.release()
|
||||
return AdvisorResult{
|
||||
Type: ResultTypeError,
|
||||
Error: err.Error(),
|
||||
RemainingUses: rt.RemainingUses(),
|
||||
}, nil
|
||||
}
|
||||
|
||||
advice := extractAdvisorText(persistedStep)
|
||||
if advice == "" {
|
||||
// Refund: the run did not produce advice, so the contract
|
||||
// "increments on every successful advisor call" treats this
|
||||
// as not consuming a use.
|
||||
rt.release()
|
||||
return AdvisorResult{
|
||||
Type: ResultTypeError,
|
||||
Error: "advisor produced no text output",
|
||||
RemainingUses: rt.RemainingUses(),
|
||||
}, nil
|
||||
}
|
||||
|
||||
return AdvisorResult{
|
||||
Type: ResultTypeAdvice,
|
||||
Advice: advice,
|
||||
AdvisorModel: rt.cfg.Model.Provider() + "/" + rt.cfg.Model.Model(),
|
||||
RemainingUses: rt.RemainingUses(),
|
||||
}, nil
|
||||
}
|
||||
|
||||
func extractAdvisorText(step chatloop.PersistedStep) string {
|
||||
parts := make([]string, 0, len(step.Content))
|
||||
for _, content := range step.Content {
|
||||
text, ok := fantasy.AsContentType[fantasy.TextContent](content)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
trimmed := strings.TrimSpace(text.Text)
|
||||
if trimmed == "" {
|
||||
continue
|
||||
}
|
||||
parts = append(parts, trimmed)
|
||||
}
|
||||
return strings.TrimSpace(strings.Join(parts, "\n\n"))
|
||||
}
|
||||
@@ -0,0 +1,585 @@
|
||||
package chatadvisor_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"iter"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"charm.land/fantasy"
|
||||
fantasyopenai "charm.land/fantasy/providers/openai"
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chatadvisor"
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chattest"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
func TestAdvisorRunAdvice(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const (
|
||||
question = "What is the smallest safe change?"
|
||||
maxOutputTokens = int64(321)
|
||||
)
|
||||
|
||||
var capturedCall fantasy.Call
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, call fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
capturedCall = call
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "Take the smallest safe change."},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop},
|
||||
}), nil
|
||||
},
|
||||
},
|
||||
MaxUsesPerRun: 2,
|
||||
MaxOutputTokens: maxOutputTokens,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
result, err := runtime.RunAdvisor(t.Context(), question, []fantasy.Message{
|
||||
textMessage(fantasy.MessageRoleSystem, "existing system"),
|
||||
textMessage(fantasy.MessageRoleUser, "hello"),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, chatadvisor.ResultTypeAdvice, result.Type)
|
||||
require.Equal(t, "Take the smallest safe change.", result.Advice)
|
||||
require.Equal(t, "test-provider/test-model", result.AdvisorModel)
|
||||
require.Equal(t, 1, result.RemainingUses)
|
||||
|
||||
require.Empty(t, capturedCall.Tools)
|
||||
require.NotNil(t, capturedCall.MaxOutputTokens)
|
||||
require.Equal(t, maxOutputTokens, *capturedCall.MaxOutputTokens)
|
||||
require.NotEmpty(t, capturedCall.Prompt)
|
||||
require.Equal(t, fantasy.MessageRoleUser, capturedCall.Prompt[len(capturedCall.Prompt)-1].Role)
|
||||
require.Equal(t, question, singleText(t, capturedCall.Prompt[len(capturedCall.Prompt)-1]))
|
||||
}
|
||||
|
||||
func TestAdvisorRunLimitReached(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var calls int
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
calls++
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "first answer"},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop},
|
||||
}), nil
|
||||
},
|
||||
},
|
||||
MaxUsesPerRun: 1,
|
||||
MaxOutputTokens: 64,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
first, err := runtime.RunAdvisor(t.Context(), "first?", nil)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, chatadvisor.ResultTypeAdvice, first.Type)
|
||||
require.Equal(t, 0, first.RemainingUses)
|
||||
|
||||
second, err := runtime.RunAdvisor(t.Context(), "second?", nil)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, chatadvisor.ResultTypeLimitReached, second.Type)
|
||||
require.Equal(t, 0, second.RemainingUses)
|
||||
require.Equal(t, 1, calls)
|
||||
}
|
||||
|
||||
func TestAdvisorRunError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
return nil, xerrors.New("boom")
|
||||
},
|
||||
},
|
||||
MaxUsesPerRun: 1,
|
||||
MaxOutputTokens: 64,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
result, err := runtime.RunAdvisor(t.Context(), "what failed?", nil)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, chatadvisor.ResultTypeError, result.Type)
|
||||
require.Contains(t, result.Error, "boom")
|
||||
// A transient nested run failure must not consume quota: callers
|
||||
// can retry up to MaxUsesPerRun times despite the failure.
|
||||
require.Equal(t, 1, result.RemainingUses)
|
||||
|
||||
// Confirm the refund left the runtime in a usable state by issuing
|
||||
// a successful call after the failure, even though MaxUsesPerRun=1.
|
||||
runtime2, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func() func(context.Context, fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
var calls int
|
||||
return func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
calls++
|
||||
if calls == 1 {
|
||||
return nil, xerrors.New("boom")
|
||||
}
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "recovered"},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop},
|
||||
}), nil
|
||||
}
|
||||
}(),
|
||||
},
|
||||
MaxUsesPerRun: 1,
|
||||
MaxOutputTokens: 64,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
failed, err := runtime2.RunAdvisor(t.Context(), "first?", nil)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, chatadvisor.ResultTypeError, failed.Type)
|
||||
require.Equal(t, 1, failed.RemainingUses)
|
||||
|
||||
retried, err := runtime2.RunAdvisor(t.Context(), "retry?", nil)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, chatadvisor.ResultTypeAdvice, retried.Type)
|
||||
require.Equal(t, "recovered", retried.Advice)
|
||||
require.Equal(t, 0, retried.RemainingUses)
|
||||
}
|
||||
|
||||
func TestNewRuntimeValidation(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
matchingTokens := int64(64)
|
||||
mismatchedTokens := int64(32)
|
||||
model := &chattest.FakeModel{ProviderName: "test-provider", ModelName: "test-model"}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
cfg chatadvisor.RuntimeConfig
|
||||
errText string
|
||||
}{
|
||||
{
|
||||
name: "NilModel",
|
||||
cfg: chatadvisor.RuntimeConfig{MaxUsesPerRun: 1, MaxOutputTokens: 64},
|
||||
errText: "advisor model is required",
|
||||
},
|
||||
{
|
||||
name: "NonPositiveMaxUses",
|
||||
cfg: chatadvisor.RuntimeConfig{
|
||||
Model: model,
|
||||
MaxUsesPerRun: 0,
|
||||
MaxOutputTokens: 64,
|
||||
},
|
||||
errText: "advisor max uses per run must be positive",
|
||||
},
|
||||
{
|
||||
name: "NonPositiveMaxOutputTokens",
|
||||
cfg: chatadvisor.RuntimeConfig{
|
||||
Model: model,
|
||||
MaxUsesPerRun: 1,
|
||||
MaxOutputTokens: 0,
|
||||
},
|
||||
errText: "advisor max output tokens must be positive",
|
||||
},
|
||||
{
|
||||
name: "MismatchedModelConfigMaxOutputTokens",
|
||||
cfg: chatadvisor.RuntimeConfig{
|
||||
Model: model,
|
||||
MaxUsesPerRun: 1,
|
||||
MaxOutputTokens: matchingTokens,
|
||||
ModelConfig: codersdk.ChatModelCallConfig{
|
||||
MaxOutputTokens: &mismatchedTokens,
|
||||
},
|
||||
},
|
||||
errText: "must match runtime max output tokens",
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range tests {
|
||||
t.Run(testCase.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
_, err := chatadvisor.NewRuntime(testCase.cfg)
|
||||
require.Error(t, err)
|
||||
require.ErrorContains(t, err, testCase.errText)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewRuntimeDeepClonesOpenAIResponsesProviderOptions(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
parentPrevID := "resp_parent_abc123"
|
||||
parentOpts := &fantasyopenai.ResponsesProviderOptions{
|
||||
PreviousResponseID: &parentPrevID,
|
||||
}
|
||||
parentProviderOpts := fantasy.ProviderOptions{
|
||||
fantasyopenai.Name: parentOpts,
|
||||
}
|
||||
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "advice"},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop},
|
||||
}), nil
|
||||
},
|
||||
},
|
||||
ProviderOptions: parentProviderOpts,
|
||||
MaxUsesPerRun: 1,
|
||||
MaxOutputTokens: 64,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
result, err := runtime.RunAdvisor(t.Context(), "anything?", nil)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, chatadvisor.ResultTypeAdvice, result.Type)
|
||||
|
||||
// Parent's OpenAI Responses entry must still carry its PreviousResponseID;
|
||||
// the advisor's nested chatloop run must not have mutated the shared pointer.
|
||||
require.NotNil(t, parentOpts.PreviousResponseID)
|
||||
require.Equal(t, parentPrevID, *parentOpts.PreviousResponseID)
|
||||
}
|
||||
|
||||
func TestAdvisorRunStripsChainStateAndIsConsistentAcrossCalls(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
parentPrevID := "resp_parent_xyz"
|
||||
parentOpts := &fantasyopenai.ResponsesProviderOptions{
|
||||
PreviousResponseID: &parentPrevID,
|
||||
}
|
||||
parentProviderOpts := fantasy.ProviderOptions{
|
||||
fantasyopenai.Name: parentOpts,
|
||||
}
|
||||
|
||||
// Snapshot PreviousResponseID and Store at stream time, before chatloop
|
||||
// has any chance to clear them on the shared map. Comparing across calls
|
||||
// proves the advisor observes consistent (non-chained, non-persisted)
|
||||
// options each invocation.
|
||||
type observedOpts struct {
|
||||
prevID *string
|
||||
store *bool
|
||||
}
|
||||
var observed []observedOpts
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, call fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
openaiOpts, ok := call.ProviderOptions[fantasyopenai.Name].(*fantasyopenai.ResponsesProviderOptions)
|
||||
if !ok {
|
||||
observed = append(observed, observedOpts{})
|
||||
} else {
|
||||
snap := observedOpts{}
|
||||
if openaiOpts.PreviousResponseID != nil {
|
||||
copied := *openaiOpts.PreviousResponseID
|
||||
snap.prevID = &copied
|
||||
}
|
||||
if openaiOpts.Store != nil {
|
||||
copied := *openaiOpts.Store
|
||||
snap.store = &copied
|
||||
}
|
||||
observed = append(observed, snap)
|
||||
}
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "advice"},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop},
|
||||
}), nil
|
||||
},
|
||||
},
|
||||
ProviderOptions: parentProviderOpts,
|
||||
MaxUsesPerRun: 2,
|
||||
MaxOutputTokens: 64,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
for i := range 2 {
|
||||
result, err := runtime.RunAdvisor(t.Context(), fmt.Sprintf("q%d", i), nil)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, chatadvisor.ResultTypeAdvice, result.Type)
|
||||
}
|
||||
|
||||
require.Len(t, observed, 2)
|
||||
for i, snap := range observed {
|
||||
// Each nested call must run without chain mode so prompts built
|
||||
// from full history by BuildAdvisorMessages are accepted.
|
||||
require.Nil(t, snap.prevID, "call %d unexpectedly ran in chain mode", i)
|
||||
// Store must be explicitly disabled so the provider does not
|
||||
// persist an orphan response that later chain-mode calls would
|
||||
// fail to resume.
|
||||
require.NotNil(t, snap.store, "call %d did not disable Store", i)
|
||||
require.False(t, *snap.store, "call %d ran with Store enabled", i)
|
||||
}
|
||||
|
||||
// The parent's pointer must be untouched across repeated advisor runs.
|
||||
require.NotNil(t, parentOpts.PreviousResponseID)
|
||||
require.Equal(t, parentPrevID, *parentOpts.PreviousResponseID)
|
||||
}
|
||||
|
||||
func TestBuildAdvisorMessagesTruncatesToRecentMessageLimit(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
snapshot := []fantasy.Message{textMessage(fantasy.MessageRoleSystem, "existing system")}
|
||||
for i := range 25 {
|
||||
snapshot = append(snapshot, textMessage(fantasy.MessageRoleUser, fmt.Sprintf("msg-%02d", i)))
|
||||
}
|
||||
|
||||
messages := chatadvisor.BuildAdvisorMessages("Need advice", snapshot)
|
||||
// cloned existing system + advisor system + 20 most recent user messages + question.
|
||||
require.Len(t, messages, 23)
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[0].Role)
|
||||
require.Equal(t, "existing system", singleText(t, messages[0]))
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[1].Role)
|
||||
require.Contains(t, singleText(t, messages[1]), "parent agent")
|
||||
require.Equal(t, "msg-05", singleText(t, messages[2]))
|
||||
require.Equal(t, "msg-24", singleText(t, messages[len(messages)-2]))
|
||||
require.Equal(t, "Need advice", singleText(t, messages[len(messages)-1]))
|
||||
}
|
||||
|
||||
func TestBuildAdvisorMessagesStopsAtOversizedMessage(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// The walk is backward from the end of the snapshot. user-late fits,
|
||||
// the oversized assistant message breaks the walk, and user-early is
|
||||
// never reached. This preserves contiguity: the advisor never sees a
|
||||
// message that references missing context.
|
||||
snapshot := []fantasy.Message{
|
||||
textMessage(fantasy.MessageRoleSystem, "existing system"),
|
||||
textMessage(fantasy.MessageRoleUser, "user-early"),
|
||||
textMessage(fantasy.MessageRoleAssistant, strings.Repeat("x", 20000)),
|
||||
textMessage(fantasy.MessageRoleUser, "user-late"),
|
||||
}
|
||||
|
||||
messages := chatadvisor.BuildAdvisorMessages("Need advice", snapshot)
|
||||
require.Len(t, messages, 4)
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[0].Role)
|
||||
require.Equal(t, "existing system", singleText(t, messages[0]))
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[1].Role)
|
||||
require.Contains(t, singleText(t, messages[1]), "parent agent")
|
||||
require.Equal(t, "user-late", singleText(t, messages[2]))
|
||||
require.Equal(t, "Need advice", singleText(t, messages[3]))
|
||||
|
||||
for _, msg := range messages {
|
||||
require.NotContains(t, singleText(t, msg), strings.Repeat("x", 100))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildAdvisorMessagesPlacesAdvisorPromptAfterInheritedSystem(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
snapshot := []fantasy.Message{
|
||||
textMessage(fantasy.MessageRoleSystem, "parent-first"),
|
||||
textMessage(fantasy.MessageRoleSystem, "parent-second"),
|
||||
textMessage(fantasy.MessageRoleUser, "hello"),
|
||||
}
|
||||
|
||||
messages := chatadvisor.BuildAdvisorMessages("Need advice", snapshot)
|
||||
|
||||
// Inherited system messages come first in their original order, then
|
||||
// the advisor contract, then the recent tail, then the question.
|
||||
// This ordering makes the advisor prompt the last system directive
|
||||
// so it wins over conflicting parent instructions.
|
||||
require.Len(t, messages, 5)
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[0].Role)
|
||||
require.Equal(t, "parent-first", singleText(t, messages[0]))
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[1].Role)
|
||||
require.Equal(t, "parent-second", singleText(t, messages[1]))
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[2].Role)
|
||||
require.Contains(t, singleText(t, messages[2]), "parent agent")
|
||||
require.Equal(t, fantasy.MessageRoleUser, messages[3].Role)
|
||||
require.Equal(t, "hello", singleText(t, messages[3]))
|
||||
require.Equal(t, fantasy.MessageRoleUser, messages[4].Role)
|
||||
require.Equal(t, "Need advice", singleText(t, messages[4]))
|
||||
}
|
||||
|
||||
func TestBuildAdvisorMessagesDropsOversizedInheritedSystem(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// A single oversized parent system message is skipped so it cannot
|
||||
// push the advisor prompt past the model's context window. Smaller
|
||||
// system messages that fit the budget survive, as do later non-system
|
||||
// messages.
|
||||
snapshot := []fantasy.Message{
|
||||
textMessage(fantasy.MessageRoleSystem, "small-system"),
|
||||
textMessage(fantasy.MessageRoleSystem, strings.Repeat("x", 20000)),
|
||||
textMessage(fantasy.MessageRoleUser, "hello"),
|
||||
}
|
||||
|
||||
messages := chatadvisor.BuildAdvisorMessages("Need advice", snapshot)
|
||||
|
||||
// small-system + advisor system + recent user + question. The
|
||||
// oversized inherited system message must not appear.
|
||||
require.Len(t, messages, 4)
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[0].Role)
|
||||
require.Equal(t, "small-system", singleText(t, messages[0]))
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[1].Role)
|
||||
require.Contains(t, singleText(t, messages[1]), "parent agent")
|
||||
require.Equal(t, fantasy.MessageRoleUser, messages[2].Role)
|
||||
require.Equal(t, "hello", singleText(t, messages[2]))
|
||||
require.Equal(t, fantasy.MessageRoleUser, messages[3].Role)
|
||||
require.Equal(t, "Need advice", singleText(t, messages[3]))
|
||||
|
||||
for _, msg := range messages {
|
||||
require.NotContains(t, singleText(t, msg), strings.Repeat("x", 100))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildAdvisorMessagesPrefersNewestSystemDirectivesUnderBudget(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Two parent system messages together exceed the advisor system byte
|
||||
// budget, so one must be dropped. Later directives override earlier
|
||||
// ones when they conflict, so the advisor must receive the newest
|
||||
// directive and drop the older one. Preserve original order among
|
||||
// messages that survive so the parent's intended directive sequence
|
||||
// is unchanged.
|
||||
const payload = 9000
|
||||
snapshot := []fantasy.Message{
|
||||
textMessage(fantasy.MessageRoleSystem, "older-"+strings.Repeat("a", payload)),
|
||||
textMessage(fantasy.MessageRoleSystem, "newer-"+strings.Repeat("b", payload)),
|
||||
textMessage(fantasy.MessageRoleUser, "hello"),
|
||||
}
|
||||
|
||||
messages := chatadvisor.BuildAdvisorMessages("Need advice", snapshot)
|
||||
|
||||
// newer parent system + advisor system + recent user + question. The
|
||||
// older system message must be dropped because the newer directive
|
||||
// consumed the remaining budget.
|
||||
require.Len(t, messages, 4)
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[0].Role)
|
||||
require.Contains(t, singleText(t, messages[0]), "newer-")
|
||||
require.NotContains(t, singleText(t, messages[0]), "older-")
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[1].Role)
|
||||
require.Contains(t, singleText(t, messages[1]), "parent agent")
|
||||
require.Equal(t, fantasy.MessageRoleUser, messages[2].Role)
|
||||
require.Equal(t, "hello", singleText(t, messages[2]))
|
||||
require.Equal(t, fantasy.MessageRoleUser, messages[3].Role)
|
||||
require.Equal(t, "Need advice", singleText(t, messages[3]))
|
||||
}
|
||||
|
||||
func TestBuildAdvisorMessagesDropsOrphanToolResults(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Simulate a truncation cut that lands between the assistant tool-call
|
||||
// message and its tool-result. The resulting recent window should not
|
||||
// contain an orphan tool_result referencing a missing tool_use block.
|
||||
// Building the window with only [tool_result, assistant_reply] mimics
|
||||
// the state produced by the backward walk hitting its byte budget right
|
||||
// before the tool-call assistant message.
|
||||
snapshot := []fantasy.Message{
|
||||
toolResultMessage("call-1", "ok"),
|
||||
textMessage(fantasy.MessageRoleAssistant, "final reply"),
|
||||
}
|
||||
|
||||
messages := chatadvisor.BuildAdvisorMessages("Need advice", snapshot)
|
||||
|
||||
// Advisor system + assistant reply + question. The orphan tool result
|
||||
// must not appear in the advisor prompt.
|
||||
require.Len(t, messages, 3)
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[0].Role)
|
||||
require.Contains(t, singleText(t, messages[0]), "parent agent")
|
||||
require.Equal(t, fantasy.MessageRoleAssistant, messages[1].Role)
|
||||
require.Equal(t, "final reply", singleText(t, messages[1]))
|
||||
require.Equal(t, fantasy.MessageRoleUser, messages[2].Role)
|
||||
require.Equal(t, "Need advice", singleText(t, messages[2]))
|
||||
|
||||
for _, msg := range messages {
|
||||
require.NotEqual(t, fantasy.MessageRoleTool, msg.Role)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildAdvisorMessagesKeepsPairedToolCallAndResult(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
snapshot := []fantasy.Message{
|
||||
toolCallAssistantMessage("call-1", "search", `{"q":"x"}`),
|
||||
toolResultMessage("call-1", "ok"),
|
||||
textMessage(fantasy.MessageRoleAssistant, "done"),
|
||||
}
|
||||
|
||||
messages := chatadvisor.BuildAdvisorMessages("Need advice", snapshot)
|
||||
|
||||
// Advisor system + assistant tool call + tool result + assistant reply
|
||||
// + question. The matched pair must survive.
|
||||
require.Len(t, messages, 5)
|
||||
require.Equal(t, fantasy.MessageRoleSystem, messages[0].Role)
|
||||
require.Equal(t, fantasy.MessageRoleAssistant, messages[1].Role)
|
||||
require.Equal(t, fantasy.MessageRoleTool, messages[2].Role)
|
||||
require.Equal(t, fantasy.MessageRoleAssistant, messages[3].Role)
|
||||
require.Equal(t, "done", singleText(t, messages[3]))
|
||||
require.Equal(t, fantasy.MessageRoleUser, messages[4].Role)
|
||||
}
|
||||
|
||||
func streamFromParts(parts []fantasy.StreamPart) fantasy.StreamResponse {
|
||||
return iter.Seq[fantasy.StreamPart](func(yield func(fantasy.StreamPart) bool) {
|
||||
for _, part := range parts {
|
||||
if !yield(part) {
|
||||
return
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func textMessage(role fantasy.MessageRole, text string) fantasy.Message {
|
||||
return fantasy.Message{
|
||||
Role: role,
|
||||
Content: []fantasy.MessagePart{
|
||||
fantasy.TextPart{Text: text},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func toolCallAssistantMessage(callID, name, input string) fantasy.Message {
|
||||
return fantasy.Message{
|
||||
Role: fantasy.MessageRoleAssistant,
|
||||
Content: []fantasy.MessagePart{
|
||||
fantasy.ToolCallPart{
|
||||
ToolCallID: callID,
|
||||
ToolName: name,
|
||||
Input: input,
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func toolResultMessage(callID, text string) fantasy.Message {
|
||||
return fantasy.Message{
|
||||
Role: fantasy.MessageRoleTool,
|
||||
Content: []fantasy.MessagePart{
|
||||
fantasy.ToolResultPart{
|
||||
ToolCallID: callID,
|
||||
Output: fantasy.ToolResultOutputContentText{Text: text},
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func singleText(t *testing.T, msg fantasy.Message) string {
|
||||
t.Helper()
|
||||
require.NotEmpty(t, msg.Content)
|
||||
text, ok := fantasy.AsMessagePart[fantasy.TextPart](msg.Content[0])
|
||||
require.True(t, ok)
|
||||
return text.Text
|
||||
}
|
||||
@@ -0,0 +1,142 @@
|
||||
package chatadvisor
|
||||
|
||||
import (
|
||||
"sync/atomic"
|
||||
|
||||
"charm.land/fantasy"
|
||||
fantasyopenai "charm.land/fantasy/providers/openai"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
// RuntimeConfig configures a single advisor runtime instance.
|
||||
type RuntimeConfig struct {
|
||||
Model fantasy.LanguageModel
|
||||
ModelConfig codersdk.ChatModelCallConfig
|
||||
ProviderOptions fantasy.ProviderOptions
|
||||
MaxUsesPerRun int
|
||||
MaxOutputTokens int64
|
||||
}
|
||||
|
||||
// Runtime executes nested, tool-less advisor runs against the configured
|
||||
// language model.
|
||||
//
|
||||
// Each Runtime instance is scoped to a single outer chat run. The
|
||||
// MaxUsesPerRun counter increments on every successful advisor call and
|
||||
// is never reset, so callers must construct a fresh Runtime (via
|
||||
// NewRuntime) for each outer run. There is intentionally no Reset method:
|
||||
// the per-run quota is a safety bound on a single run, not a rolling
|
||||
// window.
|
||||
type Runtime struct {
|
||||
cfg RuntimeConfig
|
||||
used atomic.Int64
|
||||
}
|
||||
|
||||
// NewRuntime validates and normalizes advisor runtime configuration.
|
||||
func NewRuntime(cfg RuntimeConfig) (*Runtime, error) {
|
||||
if cfg.Model == nil {
|
||||
return nil, xerrors.New("advisor model is required")
|
||||
}
|
||||
if cfg.MaxUsesPerRun <= 0 {
|
||||
return nil, xerrors.New("advisor max uses per run must be positive")
|
||||
}
|
||||
if cfg.MaxOutputTokens <= 0 {
|
||||
return nil, xerrors.New("advisor max output tokens must be positive")
|
||||
}
|
||||
if cfg.ModelConfig.MaxOutputTokens != nil &&
|
||||
*cfg.ModelConfig.MaxOutputTokens != cfg.MaxOutputTokens {
|
||||
return nil, xerrors.Errorf(
|
||||
"advisor model_config.max_output_tokens (%d) must match runtime max output tokens (%d)",
|
||||
*cfg.ModelConfig.MaxOutputTokens,
|
||||
cfg.MaxOutputTokens,
|
||||
)
|
||||
}
|
||||
|
||||
normalized := cfg
|
||||
normalized.ProviderOptions = cloneProviderOptions(cfg.ProviderOptions)
|
||||
maxOutputTokens := cfg.MaxOutputTokens
|
||||
normalized.ModelConfig.MaxOutputTokens = &maxOutputTokens
|
||||
|
||||
return &Runtime{cfg: normalized}, nil
|
||||
}
|
||||
|
||||
// cloneProviderOptions returns a copy of opts with pointer entries for known,
|
||||
// in-place mutated provider option types replaced by a shallow struct copy.
|
||||
// chatloop mutates the OpenAI Responses entry (PreviousResponseID) on
|
||||
// chain-mode exit, so sharing the pointer with the parent run would let an
|
||||
// advisor call corrupt the parent's chain state. Value fields such as
|
||||
// Metadata and Include are still shared with the parent; nothing in this
|
||||
// package mutates them, but callers that need true deep-copy semantics must
|
||||
// handle those fields explicitly.
|
||||
func cloneProviderOptions(opts fantasy.ProviderOptions) fantasy.ProviderOptions {
|
||||
if opts == nil {
|
||||
return nil
|
||||
}
|
||||
cloned := make(fantasy.ProviderOptions, len(opts))
|
||||
for key, value := range opts {
|
||||
switch typed := value.(type) {
|
||||
case *fantasyopenai.ResponsesProviderOptions:
|
||||
if typed == nil {
|
||||
cloned[key] = value
|
||||
continue
|
||||
}
|
||||
copied := *typed
|
||||
cloned[key] = &copied
|
||||
default:
|
||||
cloned[key] = value
|
||||
}
|
||||
}
|
||||
return cloned
|
||||
}
|
||||
|
||||
// resetProviderOptionsForNestedCall strips inherited state from opts that
|
||||
// does not apply to an ephemeral advisor call. PreviousResponseID is
|
||||
// cleared so the nested call is not sent as a chain-mode continuation
|
||||
// (BuildAdvisorMessages sends the full history, not an incremental turn).
|
||||
// Store is forced off so the advisor call does not persist an orphan
|
||||
// response on the provider side. Must be called on a cloned map to avoid
|
||||
// mutating shared parent state.
|
||||
func resetProviderOptionsForNestedCall(opts fantasy.ProviderOptions) {
|
||||
for _, value := range opts {
|
||||
if typed, ok := value.(*fantasyopenai.ResponsesProviderOptions); ok && typed != nil {
|
||||
storeDisabled := false
|
||||
typed.PreviousResponseID = nil
|
||||
typed.Store = &storeDisabled
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// RemainingUses reports how many advisor calls are still available for the
|
||||
// current runtime.
|
||||
func (rt *Runtime) RemainingUses() int {
|
||||
if rt == nil || rt.cfg.MaxUsesPerRun <= 0 {
|
||||
return 0
|
||||
}
|
||||
|
||||
remaining := int64(rt.cfg.MaxUsesPerRun) - rt.used.Load()
|
||||
if remaining < 0 {
|
||||
return 0
|
||||
}
|
||||
return int(remaining)
|
||||
}
|
||||
|
||||
func (rt *Runtime) tryAcquire() bool {
|
||||
for {
|
||||
used := rt.used.Load()
|
||||
if used >= int64(rt.cfg.MaxUsesPerRun) {
|
||||
return false
|
||||
}
|
||||
if rt.used.CompareAndSwap(used, used+1) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// release returns a previously acquired use to the pool. Callers must
|
||||
// invoke this at most once per successful tryAcquire when the advisor
|
||||
// call did not complete successfully, so a transient provider failure
|
||||
// does not permanently consume quota for the run.
|
||||
func (rt *Runtime) release() {
|
||||
rt.used.Add(-1)
|
||||
}
|
||||
@@ -0,0 +1,60 @@
|
||||
package chatadvisor
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"strings"
|
||||
"unicode/utf8"
|
||||
|
||||
"charm.land/fantasy"
|
||||
)
|
||||
|
||||
// advisorQuestionMaxRunes caps the parent agent's question at a length
|
||||
// that leaves room in the advisor prompt for system preamble and recent
|
||||
// conversation context.
|
||||
const advisorQuestionMaxRunes = 2000
|
||||
|
||||
// ToolOptions configures the built-in advisor tool.
|
||||
type ToolOptions struct {
|
||||
Runtime *Runtime
|
||||
GetConversationSnapshot func() []fantasy.Message
|
||||
}
|
||||
|
||||
// Tool returns a fantasy.AgentTool that asks a nested model for concise
|
||||
// strategic guidance. The nested advisor sees recent conversation
|
||||
// context, runs without tools, and is limited to a single model step.
|
||||
func Tool(opts ToolOptions) fantasy.AgentTool {
|
||||
return fantasy.NewAgentTool(
|
||||
"advisor",
|
||||
"Ask a separate advisor pass for strategic guidance about planning, architecture, tradeoffs, or debugging strategy. Provide a brief question. The advisor sees recent conversation context, runs without tools for a single step, and responds to the parent agent rather than the end user.",
|
||||
func(ctx context.Context, args AdvisorArgs, _ fantasy.ToolCall) (fantasy.ToolResponse, error) {
|
||||
if opts.Runtime == nil {
|
||||
return fantasy.NewTextErrorResponse("advisor runtime is not configured"), nil
|
||||
}
|
||||
if opts.GetConversationSnapshot == nil {
|
||||
return fantasy.NewTextErrorResponse("conversation snapshot provider is not configured"), nil
|
||||
}
|
||||
|
||||
question := strings.TrimSpace(args.Question)
|
||||
if question == "" {
|
||||
return fantasy.NewTextErrorResponse("question is required"), nil
|
||||
}
|
||||
if utf8.RuneCountInString(question) > advisorQuestionMaxRunes {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
fmt.Sprintf("question must be %d runes or fewer", advisorQuestionMaxRunes),
|
||||
), nil
|
||||
}
|
||||
|
||||
result, err := opts.Runtime.RunAdvisor(ctx, question, opts.GetConversationSnapshot())
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse(err.Error()), nil
|
||||
}
|
||||
data, err := json.Marshal(result)
|
||||
if err != nil {
|
||||
return fantasy.NewTextResponse("{}"), nil
|
||||
}
|
||||
return fantasy.NewTextResponse(string(data)), nil
|
||||
},
|
||||
)
|
||||
}
|
||||
@@ -0,0 +1,266 @@
|
||||
package chatadvisor_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"charm.land/fantasy"
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chatadvisor"
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chattest"
|
||||
)
|
||||
|
||||
func TestAdvisorToolSuccess(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "Use the smaller diff."},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop},
|
||||
}), nil
|
||||
},
|
||||
},
|
||||
MaxUsesPerRun: 2,
|
||||
MaxOutputTokens: 128,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
tool := chatadvisor.Tool(chatadvisor.ToolOptions{
|
||||
Runtime: runtime,
|
||||
GetConversationSnapshot: func() []fantasy.Message {
|
||||
return []fantasy.Message{{
|
||||
Role: fantasy.MessageRoleUser,
|
||||
Content: []fantasy.MessagePart{
|
||||
fantasy.TextPart{Text: "We need a safe fix."},
|
||||
},
|
||||
}}
|
||||
},
|
||||
})
|
||||
|
||||
resp := runAdvisorTool(t, tool, chatadvisor.AdvisorArgs{Question: "What's the safest next step?"})
|
||||
require.False(t, resp.IsError)
|
||||
|
||||
var result chatadvisor.AdvisorResult
|
||||
require.NoError(t, json.Unmarshal([]byte(resp.Content), &result))
|
||||
require.Equal(t, chatadvisor.ResultTypeAdvice, result.Type)
|
||||
require.Equal(t, "Use the smaller diff.", result.Advice)
|
||||
require.Equal(t, "test-provider/test-model", result.AdvisorModel)
|
||||
require.Equal(t, 1, result.RemainingUses)
|
||||
}
|
||||
|
||||
func TestAdvisorToolRejectsEmptyQuestion(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tool := chatadvisor.Tool(chatadvisor.ToolOptions{
|
||||
Runtime: mustAdvisorRuntime(t),
|
||||
GetConversationSnapshot: func() []fantasy.Message {
|
||||
return nil
|
||||
},
|
||||
})
|
||||
|
||||
resp := runAdvisorTool(t, tool, chatadvisor.AdvisorArgs{Question: " \t\n "})
|
||||
require.True(t, resp.IsError)
|
||||
require.Contains(t, resp.Content, "question is required")
|
||||
}
|
||||
|
||||
func TestAdvisorToolRejectsLongQuestion(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tool := chatadvisor.Tool(chatadvisor.ToolOptions{
|
||||
Runtime: mustAdvisorRuntime(t),
|
||||
GetConversationSnapshot: func() []fantasy.Message {
|
||||
return nil
|
||||
},
|
||||
})
|
||||
|
||||
resp := runAdvisorTool(t, tool, chatadvisor.AdvisorArgs{Question: strings.Repeat("x", 2001)})
|
||||
require.True(t, resp.IsError)
|
||||
require.Contains(t, resp.Content, "2000 runes or fewer")
|
||||
}
|
||||
|
||||
func TestAdvisorToolRejectsMissingRuntime(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tool := chatadvisor.Tool(chatadvisor.ToolOptions{
|
||||
GetConversationSnapshot: func() []fantasy.Message {
|
||||
return nil
|
||||
},
|
||||
})
|
||||
|
||||
resp := runAdvisorTool(t, tool, chatadvisor.AdvisorArgs{Question: "Need advice"})
|
||||
require.True(t, resp.IsError)
|
||||
require.Contains(t, resp.Content, "advisor runtime is not configured")
|
||||
}
|
||||
|
||||
func TestAdvisorToolRejectsMissingSnapshotFunc(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tool := chatadvisor.Tool(chatadvisor.ToolOptions{Runtime: mustAdvisorRuntime(t)})
|
||||
|
||||
resp := runAdvisorTool(t, tool, chatadvisor.AdvisorArgs{Question: "Need advice"})
|
||||
require.True(t, resp.IsError)
|
||||
require.Contains(t, resp.Content, "conversation snapshot provider is not configured")
|
||||
}
|
||||
|
||||
func TestAdvisorToolReportsNestedError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
return nil, xerrors.New("boom")
|
||||
},
|
||||
},
|
||||
MaxUsesPerRun: 1,
|
||||
MaxOutputTokens: 64,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
tool := chatadvisor.Tool(chatadvisor.ToolOptions{
|
||||
Runtime: runtime,
|
||||
GetConversationSnapshot: func() []fantasy.Message { return nil },
|
||||
})
|
||||
|
||||
resp := runAdvisorTool(t, tool, chatadvisor.AdvisorArgs{Question: "why?"})
|
||||
require.False(t, resp.IsError)
|
||||
|
||||
var result chatadvisor.AdvisorResult
|
||||
require.NoError(t, json.Unmarshal([]byte(resp.Content), &result))
|
||||
require.Equal(t, chatadvisor.ResultTypeError, result.Type)
|
||||
require.Contains(t, result.Error, "boom")
|
||||
require.Empty(t, result.Advice)
|
||||
require.Empty(t, result.AdvisorModel)
|
||||
// A failed nested run does not consume the per-run quota.
|
||||
require.Equal(t, 1, result.RemainingUses)
|
||||
}
|
||||
|
||||
func TestAdvisorToolReportsLimitReached(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "first"},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop},
|
||||
}), nil
|
||||
},
|
||||
},
|
||||
MaxUsesPerRun: 1,
|
||||
MaxOutputTokens: 64,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
tool := chatadvisor.Tool(chatadvisor.ToolOptions{
|
||||
Runtime: runtime,
|
||||
GetConversationSnapshot: func() []fantasy.Message { return nil },
|
||||
})
|
||||
|
||||
first := runAdvisorTool(t, tool, chatadvisor.AdvisorArgs{Question: "first?"})
|
||||
require.False(t, first.IsError)
|
||||
|
||||
second := runAdvisorTool(t, tool, chatadvisor.AdvisorArgs{Question: "second?"})
|
||||
require.False(t, second.IsError)
|
||||
|
||||
var result chatadvisor.AdvisorResult
|
||||
require.NoError(t, json.Unmarshal([]byte(second.Content), &result))
|
||||
require.Equal(t, chatadvisor.ResultTypeLimitReached, result.Type)
|
||||
require.Equal(t, 0, result.RemainingUses)
|
||||
require.Empty(t, result.Advice)
|
||||
require.Empty(t, result.Error)
|
||||
require.Empty(t, result.AdvisorModel)
|
||||
}
|
||||
|
||||
func TestAdvisorToolReportsEmptyModelOutput(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop},
|
||||
}), nil
|
||||
},
|
||||
},
|
||||
MaxUsesPerRun: 1,
|
||||
MaxOutputTokens: 64,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
tool := chatadvisor.Tool(chatadvisor.ToolOptions{
|
||||
Runtime: runtime,
|
||||
GetConversationSnapshot: func() []fantasy.Message { return nil },
|
||||
})
|
||||
|
||||
resp := runAdvisorTool(t, tool, chatadvisor.AdvisorArgs{Question: "anything?"})
|
||||
require.False(t, resp.IsError)
|
||||
|
||||
var result chatadvisor.AdvisorResult
|
||||
require.NoError(t, json.Unmarshal([]byte(resp.Content), &result))
|
||||
require.Equal(t, chatadvisor.ResultTypeError, result.Type)
|
||||
require.Contains(t, result.Error, "no text output")
|
||||
require.Empty(t, result.Advice)
|
||||
// An advisor call that produces no advice does not count as a
|
||||
// successful use, so the quota must still be available.
|
||||
require.Equal(t, 1, result.RemainingUses)
|
||||
}
|
||||
|
||||
func mustAdvisorRuntime(t *testing.T) *chatadvisor.Runtime {
|
||||
t.Helper()
|
||||
|
||||
runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{
|
||||
Model: &chattest.FakeModel{
|
||||
ProviderName: "test-provider",
|
||||
ModelName: "test-model",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
return streamFromParts([]fantasy.StreamPart{
|
||||
{Type: fantasy.StreamPartTypeTextStart, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "fallback advice"},
|
||||
{Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"},
|
||||
{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop},
|
||||
}), nil
|
||||
},
|
||||
},
|
||||
MaxUsesPerRun: 2,
|
||||
MaxOutputTokens: 64,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
return runtime
|
||||
}
|
||||
|
||||
func runAdvisorTool(
|
||||
t *testing.T,
|
||||
tool fantasy.AgentTool,
|
||||
args chatadvisor.AdvisorArgs,
|
||||
) fantasy.ToolResponse {
|
||||
t.Helper()
|
||||
|
||||
data, err := json.Marshal(args)
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err := tool.Run(t.Context(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "advisor",
|
||||
Input: string(data),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
return resp
|
||||
}
|
||||
@@ -0,0 +1,28 @@
|
||||
package chatadvisor
|
||||
|
||||
// ResultType is the tagged variant of AdvisorResult. Callers should
|
||||
// compare against the exported constants rather than string literals.
|
||||
type ResultType string
|
||||
|
||||
const (
|
||||
// ResultTypeAdvice indicates the advisor returned guidance.
|
||||
ResultTypeAdvice ResultType = "advice"
|
||||
// ResultTypeLimitReached indicates the per-run advisor budget is exhausted.
|
||||
ResultTypeLimitReached ResultType = "limit_reached"
|
||||
// ResultTypeError indicates the nested advisor run failed.
|
||||
ResultTypeError ResultType = "error"
|
||||
)
|
||||
|
||||
// AdvisorArgs contains the tool-visible advisor question.
|
||||
type AdvisorArgs struct {
|
||||
Question string `json:"question"`
|
||||
}
|
||||
|
||||
// AdvisorResult is the structured result returned by the advisor runtime.
|
||||
type AdvisorResult struct {
|
||||
Type ResultType `json:"type"`
|
||||
Advice string `json:"advice,omitempty"`
|
||||
Error string `json:"error,omitempty"`
|
||||
AdvisorModel string `json:"advisor_model,omitempty"`
|
||||
RemainingUses int `json:"remaining_uses"`
|
||||
}
|
||||
Reference in New Issue
Block a user