diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0ddc93690..09eb70211 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -170,6 +170,8 @@ When profiles exist (local `.agents/agents/` and/or enabled **`kind: "agent"`** Profiles with `orchestrator: true` may themselves call `task` (one hop only): nested dispatch installs `task` + `search_agents` with `allowOrchestrator: false` so the tree bottoms out. Unknown `agent` ids fail closed. +**Reasoning effort by role** (`src/provider/reasoning-effort.ts` → `resolveEffortForRole`): spawn-time defaults are orchestrator → `high`, leaf → `medium`, clamped to the model. Explicit profile inference pins win; parent session effort is only a fallback when the role default is unsupported. This keeps multi-agent fleets off the sol+high latency cliff — see `docs/plans/reasoning-effort-by-role.md`. + **Session records** (`src/subagent/session-store.ts`): each spawn is retained as an inspectable child session (id, profile, description, brief, status, tool activity, transcript entries). Child events land only in this store — not in the parent chat transcript. Live progress still uses the light `onProgress` channel for the status bar / Agents strip. Completed sessions are capped (`maxCompleted`) so a long chat does not grow without bound. **Enter-session TUI** (`src/tui/components/agents-strip.tsx`, `subagent-session-view.tsx`): `Ctrl+E` opens Agents-strip navigation (↑/↓ select, Enter observe, `x`/Backspace cancel selected running worker, Esc leave nav). Entering a session swaps the main log for that child's transcript (live while running, historical when done/failed/cancelled) without stealing the parent reactor. Header chrome shows which agent is focused; Esc returns to the parent; `x` cancels the focused running worker. Parent Esc/stop and `/clear` call `cancelAll` so live children close (`agent.close`) instead of continuing after the parent stops. diff --git a/docs/plans/reasoning-effort-by-role.md b/docs/plans/reasoning-effort-by-role.md new file mode 100644 index 000000000..6e43ca0ec --- /dev/null +++ b/docs/plans/reasoning-effort-by-role.md @@ -0,0 +1,32 @@ +# Reasoning effort defaults by agent role (CL-5162) + +## Why + +`gpt-5.6-sol` (and similar) at **high** reasoning effort is a **latency cliff**: thinking tokens expand wall time before useful tool calls. Multi-agent fanout multiplies that cost when every leaf inherits the primary session's high setting. + +## Product defaults + +| Role | Default effort | Notes | +|---|---|---| +| Orchestrator (`profile.orchestrator: true`) | `high` | Planning / fan-out warrants deeper reasoning | +| Task leaf (generic or non-orchestrator profile) | `medium` | Keeps fleet latency off the sol+high cliff | + +Clamped via `supportedEfforts(model)` when the model does not accept the preferred rung. + +## Precedence + +1. **Explicit pin** — profile `inference` leg `reasoningEffort`, or any future task-level pin +2. **Role default** — table above, when the model supports it +3. **Parent inheritance** — parent session effort, only when the role default is not in the model's supported set +4. **Clamp** — nearest supported rung to the role default +5. **Omit** — non-reasoning models get no effort on the wire + +Implementation: `resolveEffortForRole` / `pickEffortFromCascade` in `src/provider/reasoning-effort.ts`, applied in `src/subagent/task-tool.ts` after profile/tier provider resolution. + +## Operator UI + +**Deferred.** No settings or TUI control in this change. Operators who need a different leaf effort pin it on the agent profile's inference leg. + +## Latency eval + +PerfTrace-backed medium vs high quality/latency comparison is deferred to the CL-5174 wave (`docs/plans/core-performance-tracing.md` / package C eval). Do not block this default on that instrumentation. diff --git a/src/provider/reasoning-effort.test.ts b/src/provider/reasoning-effort.test.ts index fa896c03e..34724ee92 100644 --- a/src/provider/reasoning-effort.test.ts +++ b/src/provider/reasoning-effort.test.ts @@ -1,11 +1,15 @@ import { afterEach, describe, test, expect } from "bun:test"; import { REASONING_EFFORTS, + ROLE_DEFAULT_EFFORT, isReasoningEffort, supportedEfforts, validateEffort, setModelReasoningCapabilities, modelReasoningCapability, + clampEffort, + pickEffortFromCascade, + resolveEffortForRole, } from "./reasoning-effort.js"; describe("REASONING_EFFORTS", () => { @@ -119,3 +123,170 @@ describe("reasoning capability gate", () => { if (!result.ok) expect(result.error).toContain("does not support reasoning"); }); }); + +describe("ROLE_DEFAULT_EFFORT", () => { + test("orchestrator is higher than leaf", () => { + expect(ROLE_DEFAULT_EFFORT.orchestrator).toBe("high"); + expect(ROLE_DEFAULT_EFFORT.leaf).toBe("medium"); + expect(REASONING_EFFORTS.indexOf(ROLE_DEFAULT_EFFORT.orchestrator)).toBeGreaterThan( + REASONING_EFFORTS.indexOf(ROLE_DEFAULT_EFFORT.leaf), + ); + }); +}); + +describe("clampEffort", () => { + test("returns desired when supported", () => { + expect(clampEffort("medium", ["low", "medium", "high"])).toBe("medium"); + }); + + test("picks the nearest supported rung", () => { + // medium is between low and high; equidistant → first minimum wins (low). + expect(clampEffort("medium", ["low", "high"])).toBe("low"); + expect(clampEffort("xhigh", ["low", "medium", "high"])).toBe("high"); + expect(clampEffort("none", ["low", "medium", "high"])).toBe("low"); + }); + + test("empty supported yields undefined", () => { + expect(clampEffort("medium", [])).toBeUndefined(); + }); +}); + +describe("pickEffortFromCascade (precedence table)", () => { + test("1. pin wins over role default and parent", () => { + expect( + pickEffortFromCascade({ + pin: "low", + roleDefault: "medium", + parentEffort: "high", + supported: ["low", "medium", "high"], + }), + ).toBe("low"); + }); + + test("1b. unsupported pin is clamped onto supported", () => { + expect( + pickEffortFromCascade({ + pin: "xhigh", + roleDefault: "medium", + parentEffort: "high", + supported: ["low", "medium", "high"], + }), + ).toBe("high"); + }); + + test("2. role default when supported (ignores parent)", () => { + expect( + pickEffortFromCascade({ + roleDefault: "medium", + parentEffort: "high", + supported: ["low", "medium", "high"], + }), + ).toBe("medium"); + }); + + test("3. parent inheritance when role default is unsupported but parent is", () => { + expect( + pickEffortFromCascade({ + roleDefault: "medium", + parentEffort: "high", + supported: ["low", "high", "xhigh"], + }), + ).toBe("high"); + }); + + test("4. clamp role default when neither role default nor parent is supported", () => { + expect( + pickEffortFromCascade({ + roleDefault: "medium", + parentEffort: "xhigh", + supported: ["none", "minimal", "low"], + }), + ).toBe("low"); + }); + + test("5. empty supported yields undefined", () => { + expect( + pickEffortFromCascade({ + roleDefault: "medium", + parentEffort: "high", + supported: [], + }), + ).toBeUndefined(); + }); +}); + +describe("resolveEffortForRole", () => { + afterEach(() => setModelReasoningCapabilities({})); + + test("explicit pin wins over role default and parent", () => { + expect( + resolveEffortForRole({ + orchestrator: false, + pin: "low", + parentEffort: "high", + model: "gpt-5", + }), + ).toBe("low"); + expect( + resolveEffortForRole({ + orchestrator: true, + pin: "minimal", + parentEffort: "high", + model: "gpt-5", + }), + ).toBe("minimal"); + }); + + test("leaf role default is medium even when parent is high", () => { + expect( + resolveEffortForRole({ + orchestrator: false, + parentEffort: "high", + model: "gpt-5", + }), + ).toBe("medium"); + }); + + test("orchestrator role default is high even when parent is low", () => { + expect( + resolveEffortForRole({ + orchestrator: true, + parentEffort: "low", + model: "gpt-5", + }), + ).toBe("high"); + }); + + test("non-reasoning model yields undefined even with parent effort", () => { + setModelReasoningCapabilities({ "chat-only-model": false }); + expect( + resolveEffortForRole({ + orchestrator: false, + parentEffort: "high", + model: "chat-only-model", + }), + ).toBeUndefined(); + }); + + test("codex leaf still gets medium (not parent xhigh)", () => { + expect( + resolveEffortForRole({ + orchestrator: false, + parentEffort: "xhigh", + model: "gpt-5.6-sol", + isCodex: true, + }), + ).toBe("medium"); + }); + + test("codex orchestrator still gets high", () => { + expect( + resolveEffortForRole({ + orchestrator: true, + parentEffort: "low", + model: "gpt-5.6-sol", + isCodex: true, + }), + ).toBe("high"); + }); +}); diff --git a/src/provider/reasoning-effort.ts b/src/provider/reasoning-effort.ts index 74967e38f..38425d303 100644 --- a/src/provider/reasoning-effort.ts +++ b/src/provider/reasoning-effort.ts @@ -95,3 +95,104 @@ export function validateEffort( error: `Model "${model}" does not support reasoning effort "${effort}" (supported: ${supported.join(", ")}).`, }; } + +// --------------------------------------------------------------------------- +// Role-based product defaults (CL-5162) +// +// Orchestrators plan and fan out work — higher effort is worth the latency. +// Task leaves should stay cheaper/faster so multi-agent fleets do not multiply +// a sol+high cliff across every child. No operator UI: this is the silent +// product default until a profile/task pin says otherwise. +// --------------------------------------------------------------------------- + +/** Product default effort by agent role (before model clamping). */ +export const ROLE_DEFAULT_EFFORT = { + orchestrator: "high", + leaf: "medium", +} as const satisfies Record<"orchestrator" | "leaf", ReasoningEffort>; + +/** + * Nearest supported effort to `desired` by position on the canonical ladder. + * Returns undefined only when `supported` is empty. + */ +export function clampEffort( + desired: ReasoningEffort, + supported: readonly ReasoningEffort[], +): ReasoningEffort | undefined { + if (supported.length === 0) return undefined; + if (supported.includes(desired)) return desired; + const desiredIdx = REASONING_EFFORTS.indexOf(desired); + let best: ReasoningEffort = supported[0]!; + let bestDist = Number.POSITIVE_INFINITY; + for (const level of supported) { + const dist = Math.abs(REASONING_EFFORTS.indexOf(level) - desiredIdx); + if (dist < bestDist) { + bestDist = dist; + best = level; + } + } + return best; +} + +export type ResolveEffortForRoleOpts = { + /** True when the spawn is an orchestrator profile (may call task). */ + orchestrator: boolean; + /** Explicit profile inference leg or task-tier pin — highest precedence. */ + pin?: ReasoningEffort; + /** Parent session effort — used only when the role default is not supported. */ + parentEffort?: ReasoningEffort; + model: string; + isCodex?: boolean; +}; + +/** + * Pure cascade used by `resolveEffortForRole`. Exported for unit tests of the + * precedence table without depending on per-model supported sets. + * + * Precedence (first match wins): + * 1. Explicit pin (clamped onto supported when the pin is not in the set) + * 2. Role default when present in `supported` + * 3. Parent effort when present in `supported` + * 4. Clamp of role default onto `supported` + * 5. undefined when `supported` is empty + * + * Pins are still highest precedence, but an unsupported pin is clamped so the + * pure API owns the "never emit an unsupported effort" invariant (callers that + * want hard-fail on bad pins should validateEffort first, as task-tool does). + */ +export function pickEffortFromCascade(opts: { + pin?: ReasoningEffort; + roleDefault: ReasoningEffort; + parentEffort?: ReasoningEffort; + supported: readonly ReasoningEffort[]; +}): ReasoningEffort | undefined { + if (opts.supported.length === 0) return undefined; + if (opts.pin !== undefined) { + return opts.supported.includes(opts.pin) ? opts.pin : clampEffort(opts.pin, opts.supported); + } + if (opts.supported.includes(opts.roleDefault)) return opts.roleDefault; + if (opts.parentEffort !== undefined && opts.supported.includes(opts.parentEffort)) { + return opts.parentEffort; + } + return clampEffort(opts.roleDefault, opts.supported); +} + +/** + * Resolve reasoning effort for a sub-agent spawn. + * + * Why parent is below role default: a /agent high selection on the primary must + * not force every leaf onto high — that multiplies the sol+high latency cliff + * across the fleet. Parent still fills gaps when the role default is not in the + * model's supported set but the parent effort is. + */ +export function resolveEffortForRole(opts: ResolveEffortForRoleOpts): ReasoningEffort | undefined { + const supported = supportedEfforts(opts.model, undefined, opts.isCodex === true); + return pickEffortFromCascade({ + ...(opts.pin !== undefined ? { pin: opts.pin } : {}), + roleDefault: opts.orchestrator + ? ROLE_DEFAULT_EFFORT.orchestrator + : ROLE_DEFAULT_EFFORT.leaf, + ...(opts.parentEffort !== undefined ? { parentEffort: opts.parentEffort } : {}), + supported, + }); +} diff --git a/src/subagent/task-tool.ts b/src/subagent/task-tool.ts index 5c8ad50bf..a87b00e5a 100644 --- a/src/subagent/task-tool.ts +++ b/src/subagent/task-tool.ts @@ -21,7 +21,11 @@ import { resolveInferenceWithPolicy, validateTaskMaxTurns, } from "../config/settings.js"; -import { validateEffort } from "../provider/reasoning-effort.js"; +import { + resolveEffortForRole, + validateEffort, + type ReasoningEffort, +} from "../provider/reasoning-effort.js"; import { isCodexProviderName } from "../config/codex-providers.js"; import type { SubAgentSessionStore } from "./session-store.js"; import { @@ -112,7 +116,7 @@ export const taskToolDefinition: ToolDefinition = { agent: { type: "string", description: - "Optional agent profile id from search_agents (or .agents/agents/). Profiles specify tier, capability restrictions, and role. Omit for a generic sub-agent on the default provider.", + "Optional agent profile id from search_agents (or .agents/agents/). Profiles specify tier, capability restrictions, and role. Role drives reasoning-effort defaults (orchestrator high, leaf medium) unless the profile pins inference.reasoningEffort; parent session effort is inheritance only when the role default is unsupported on the model.", }, maxTurns: { type: "number", @@ -228,6 +232,12 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { let provider: SubAgentProvider = typeof deps.provider === "function" ? deps.provider() : deps.provider; + // Snapshot parent effort before profile/tier rebuilds so role-default + // resolution can fall back to inheritance without reading a mutated provider. + const parentEffort = provider.reasoningEffort; + // Explicit profile inference / task-tier pin (if any). Distinct from the + // parent snapshot so resolveEffortForRole can apply pin > role > parent. + let effortPin: ReasoningEffort | undefined; let capabilities: CapabilityFilter | undefined; let systemPromptRole: string | undefined; let orchestrator = false; @@ -250,7 +260,7 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { resolved: { provider: string; model: string; - reasoningEffort?: import("../provider/reasoning-effort.js").ReasoningEffort; + reasoningEffort?: ReasoningEffort; }, label: string, ): string | null => { @@ -266,12 +276,16 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { if (!verdict.ok) { return `Error: ${label} has incompatible inference: ${verdict.error}`; } + // Pin is recorded here; final effort is applied after role resolution + // so a leg without reasoningEffort still gets the role default. + effortPin = resolved.reasoningEffort; } const providerSettings = settings.providers[resolved.provider]; if (providerSettings === undefined) { return `Error: ${label} resolved to provider "${resolved.provider}" which is not configured.`; } - const effort = resolved.reasoningEffort ?? provider.reasoningEffort; + // Provider/model swap only — effort is finalized once via + // resolveEffortForRole (pin > role default > parent) below. provider = { providerName: resolved.provider, baseURL: providerSettings.baseURL, @@ -283,7 +297,6 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { ? { apiKey: providerSettings.apiKey } : {}), model: resolved.model, - ...(effort !== undefined ? { reasoningEffort: effort } : {}), }; return null; }; @@ -333,7 +346,7 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { // surfaces as a dispatch error rather than silently running on the // parent's provider. let resolved: - | { provider: string; model: string; reasoningEffort?: import("../provider/reasoning-effort.js").ReasoningEffort } + | { provider: string; model: string; reasoningEffort?: ReasoningEffort } | null = null; if (profile.inference !== undefined) { const outcome = resolveInferenceWithPolicy(profile.inference, settings); @@ -383,6 +396,25 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { tier = taskTier; } + // Role-based effort: pin > orchestrator/leaf default > parent inheritance. + // Leaves default to medium so a primary on high/sol does not multiply the + // latency cliff across every spawned worker (see resolveEffortForRole). + { + const effort = resolveEffortForRole({ + orchestrator, + ...(effortPin !== undefined ? { pin: effortPin } : {}), + ...(parentEffort !== undefined ? { parentEffort } : {}), + model: provider.model, + isCodex: isCodexProviderName(provider.providerName), + }); + if (effort !== undefined) { + provider = { ...provider, reasoningEffort: effort }; + } else { + const { reasoningEffort: _drop, ...rest } = provider; + provider = rest; + } + } + let taskMaxTurns: number | undefined; if (rawMaxTurns !== undefined) { const verdict = validateTaskMaxTurns(rawMaxTurns); diff --git a/src/subagent/types.ts b/src/subagent/types.ts index 529c00c00..0f0bc1c97 100644 --- a/src/subagent/types.ts +++ b/src/subagent/types.ts @@ -23,8 +23,9 @@ export type SubAgentProvider = { apiKey?: string; keyless?: boolean; model: string; - // Subagents inherit the parent's reasoning effort so a /agent selection - // applies to delegated work, not just the top-level loop. + // Resolved effort for this spawn (pin > role default > parent). See + // resolveEffortForRole — leaves default to medium, orchestrators to high, + // so a primary /agent high selection does not force every leaf onto high. reasoningEffort?: ReasoningEffort; // Mirrors ProviderCatalogEntry.bifrostVirtualKey. Without it the generic // (no-tier) dispatch path builds a plain openai-compatible source and the diff --git a/tests/unit/subagent.test.ts b/tests/unit/subagent.test.ts index a46ff6566..2b2b9583e 100644 --- a/tests/unit/subagent.test.ts +++ b/tests/unit/subagent.test.ts @@ -60,7 +60,9 @@ test("handler rejects empty description or prompt", async () => { expect(await callHandler(tool, { description: "label", prompt: " " })).toContain("Error:"); }); -test("handler forwards the provider's reasoning effort to the runner", async () => { +test("generic leaf gets role-default medium even when parent effort is high", async () => { + // CL-5162: leaves do not inherit primary high — that multiplies the sol+high + // latency cliff across every spawn. Role default (medium) wins over parent. let receivedEffort: RunSubAgentParams | undefined; const tool = createTaskTool({ permissionGate: testPermissionGate, cwd: "/repo", @@ -74,7 +76,7 @@ test("handler forwards the provider's reasoning effort to the runner", async () await callHandler(tool, { description: "task", prompt: "do it" }); - expect(receivedEffort?.provider.reasoningEffort).toBe("high"); + expect(receivedEffort?.provider.reasoningEffort).toBe("medium"); }); test("a provider getter is resolved at spawn time, so a live switch reaches subagents", async () => { @@ -95,7 +97,8 @@ test("a provider getter is resolved at spawn time, so a live switch reaches suba await callHandler(tool, { description: "task", prompt: "do it" }); expect(received?.provider.model).toBe("model-b"); - expect(received?.provider.reasoningEffort).toBe("high"); + // Live model switch is honored; effort still follows leaf role default. + expect(received?.provider.reasoningEffort).toBe("medium"); }); test("handler forwards trimmed args to the runner and wraps the result", async () => { @@ -646,11 +649,10 @@ describe("createTaskTool profile resolution", () => { expect(result).toContain('Error: agent "p" has incompatible inference'); }); - test("parent reasoningEffort is inherited when the resolved leg does not declare its own", async () => { - // Regression guard for the P0 fix: an agent that pins inference without a - // per-leg reasoningEffort still inherits the parent session's effort, so - // a /agent effort selection propagates uniformly across pinned and - // fall-through dispatch paths. + test("leaf role default applies when the resolved leg does not pin effort", async () => { + // CL-5162: a profile that pins provider/model without reasoningEffort gets + // the leaf role default (medium), not the parent's high — so fleet fanout + // stays off the sol+high cliff unless the profile explicitly pins effort. let received: RunSubAgentParams | undefined; const tool = createTaskTool({ permissionGate: testPermissionGate, cwd: "/repo", @@ -677,6 +679,64 @@ describe("createTaskTool profile resolution", () => { expect(received?.provider.providerName).toBe("anthropic"); expect(received?.provider.model).toBe("claude-sonnet-4"); + expect(received?.provider.reasoningEffort).toBe("medium"); + }); + + test("profile inference pin for effort wins over role default and parent", async () => { + let received: RunSubAgentParams | undefined; + const tool = createTaskTool({ permissionGate: testPermissionGate, + cwd: "/repo", + getWorkdirBase: () => "/repo/.ctx", + provider: { ...provider, reasoningEffort: "high" }, + settings: baseSettings as unknown as Parameters[0]["settings"], + profiles: [ + { + id: "p", + systemPromptRole: "You are p.", + inference: { + mode: "pin", + order: [{ provider: "anthropic", model: "claude-sonnet-4", reasoningEffort: "low" }], + }, + }, + ], + run: async (params) => { + received = params; + return "ran"; + }, + }); + + await callHandler(tool, { description: "task", prompt: "do it", agent: "p" }); + + expect(received?.provider.reasoningEffort).toBe("low"); + }); + + test("orchestrator profile gets high role default when effort is not pinned", async () => { + let received: RunSubAgentParams | undefined; + const tool = createTaskTool({ permissionGate: testPermissionGate, + cwd: "/repo", + getWorkdirBase: () => "/repo/.ctx", + provider: { ...provider, reasoningEffort: "low" }, + settings: baseSettings as unknown as Parameters[0]["settings"], + profiles: [ + { + id: "orch", + systemPromptRole: "You are orch.", + orchestrator: true, + inference: { + mode: "pin", + order: [{ provider: "anthropic", model: "claude-sonnet-4" }], + }, + }, + ], + run: async (params) => { + received = params; + return "ran"; + }, + }); + + await callHandler(tool, { description: "task", prompt: "do it", agent: "orch" }); + + expect(received?.orchestrator).toBe(true); expect(received?.provider.reasoningEffort).toBe("high"); });