feat(sdk): thread reasoningEffort through agent spawning pipeline#1148
feat(sdk): thread reasoningEffort through agent spawning pipeline#1148ahhlun wants to merge 7 commits into
Conversation
Add support for configuring reasoning effort level per-agent through: - Charter markdown: **Reasoning Effort:** field in ## Model section - Config persistence: defaultReasoningEffort and agentReasoningEffortOverrides in .squad/config.json - SDK builders: reasoningEffort on defineAgent() and defineDefaults() - Layered resolution: resolveReasoningEffort() with priority hierarchy matching model resolution - Lifecycle manager: SpawnAgentOptions.reasoningEffortOverride passes through to SquadSessionConfig - Fan-out pipeline: AgentSpawnConfig.reasoningEffortOverride passes through to createSession() The value 'auto' is treated as a sentinel meaning 'let the SDK/API decide' and resolves to undefined. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds end-to-end “reasoning effort” support across charter parsing, persistent config, spawn/fan-out session creation, and the public builder API.
Changes:
- Parse
**Reasoning Effort:**in charters and thread it through charter compilation + session creation. - Add persistent config support (
defaultReasoningEffort,agentReasoningEffortOverrides) plus a layeredresolveReasoningEffort()resolver. - Add tests covering config round-trips, charter parsing/compilation, and fan-out session parameter propagation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test/model-preference.test.ts | Adds unit tests for reasoning-effort config read/write + layered resolution. |
| test/fan-out.test.ts | Verifies reasoningEffort propagation (override/charter/auto) into createSession. |
| test/charter-compiler.test.ts | Adds charter markdown parsing + compile-time resolution tests for reasoning effort. |
| packages/squad-sdk/templates/charter.md | Updates charter template to include “Reasoning Effort: auto”. |
| packages/squad-sdk/src/coordinator/fan-out.ts | Threads reasoningEffortOverride into spawn/session creation (with optional resolver). |
| packages/squad-sdk/src/config/schema.ts | Extends config schema types with reasoning-effort fields. |
| packages/squad-sdk/src/config/models.ts | Implements read/write + layered resolution for reasoning effort. |
| packages/squad-sdk/src/builders/types.ts | Exposes reasoningEffort in builder types. |
| packages/squad-sdk/src/builders/index.ts | Validates reasoningEffort in defineAgent/defineDefaults. |
| packages/squad-sdk/src/agents/lifecycle.ts | Threads reasoning effort through agent lifecycle session creation. |
| packages/squad-sdk/src/agents/index.ts | Adds reasoningEffort to AgentCharter. |
| packages/squad-sdk/src/agents/charter-compiler.ts | Parses and compiles a resolved reasoning effort from charter/config overrides. |
| .changeset/reasoning-effort.md | Declares a minor release and documents the feature. |
Add clampReasoningEffort() that caps the requested effort to the highest level the model supports. This prevents API errors when a user requests e.g. 'xhigh' on a model that only supports up to 'high' (like GPT-5.5). resolveReasoningEffort() now accepts an optional supportedEfforts array (from listModels()) and auto-clamps the result. Behavior: - xhigh on GPT-5.5 [low,medium,high] -> high (clamped down) - xhigh on GPT-5.2-codex [low,medium,high,xhigh] -> xhigh (unchanged) - xhigh on gpt-4.1 (no effort support) -> undefined (stripped) - max and xhigh treated as equivalent rank Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Validate reasoning effort at parse time: charter compiler now rejects invalid values and normalizes 'auto' to undefined early, preventing invalid strings from reaching SquadSessionConfig - Validate readReasoningEffort() against allowed set (low/medium/high/xhigh/auto), matching the per-agent override validation for consistency - Add isValid() guard in resolveReasoningEffort() so invalid values at any layer (spawn override, charter, config) fall through gracefully - Remove duplicate extractReasoningEffort() regex in lifecycle.ts; use compileCharterFull() output instead (single source of truth) - Add plain-object check after JSON.parse in write functions to prevent corruption when config.json contains non-object JSON (e.g. array/number) - Fix changeset docs to match actual priority order: per-agent config > global config > spawn override > charter > undefined - Add tests for invalid inputs at all resolver layers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
All 10 review comments addressed in commit 423a7b0:
190 tests pass (up from 183). |
Add Per-Agent Reasoning Effort section to squad.agent.md so the coordinator knows to use reasoningEffort as a session parameter instead of switching to different model variants (e.g. using claude-opus-4.7-1m-internal with xhigh effort instead of switching to claude-opus-4.7-xhigh). Includes: - Layered resolution instructions matching the SDK implementation - Config persistence commands (always use xhigh, per-agent, clear) - Explicit rule: same model + different effort, NOT different model - Updated spawn output format showing effort annotation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Centralize VALID_REASONING_EFFORTS as exported const from config/models.ts; charter-compiler and builders import it instead of maintaining duplicates - Validate reasoning effort in lifecycle before setting on SquadSessionConfig: reject auto and invalid values so they never reach the SDK - Validate in fan-out fallback path: invalid reasoningEffortOverride values are filtered out instead of passed through to createSession - Fix clampReasoningEffort to clamp UP to model's minimum when requested effort is below the supported range (e.g. low on a model that only supports [medium] returns medium instead of undefined) - Validate write APIs: writeReasoningEffort and writeAgentReasoningEffortOverrides now reject invalid values instead of persisting them to config.json - Add test for clamp-up behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
All review feedback addressed across commits 423a7b0, b437d8a, and 046138a. Round 1 (10 comments) — 423a7b0:
Round 2 (7 new comments) — 046138a:
Coordinator update — b437d8a:
237 tests pass across 6 test files. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
👋 Friendly nudge — this PR has had no activity for 16 days. What needs attention:
If this PR is abandoned, please close it. If it's blocked on something external, leave a comment so the team knows. |
|
@bradygaster @tamirdresher can you help with review? |
|
@ahhlun nice work — the thing this PR sets out to do is done. On the integrator-injection shape
One testing question The live integration test in the PR description confirms The other two paths this PR ships — charter Smaller nits — not blockers
Address nits here or in a follow-up — your call. Looking forward to the integrator wiring PR. |
Follow-up to @tamirdresher's review on PR bradygaster#1148: - clampReasoningEffort: document the deliberate upward clamp (JSDoc and inline comment) so a request below a model's minimum isn't mistaken for a bug. - writeReasoningEffort: warn and preserve the existing preference on invalid input instead of silently clearing it (only null clears). - writeAgentReasoningEffortOverrides: warn about dropped invalid entries instead of discarding them silently. - charter parsing: warn on an invalid Reasoning Effort value so charter authors get feedback instead of a silent drop. - Consolidate the reasoning-effort union on the canonical SquadReasoningEffort type: builders/types.ts references it and VALID_REASONING_EFFORTS is tied to it via satisfies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🛫 PR Readiness Check
PR Scope: 📦🔧 Mixed (product + infrastructure)
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 7 commits — consider squashing before review |
| ✅ | Not in draft | Ready for review |
| ❌ | Branch up to date | dev is 164 commit(s) ahead — rebase recommended |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | Changeset file found |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ✅ | Copilot threads resolved | 12 active Copilot thread(s) resolved (5 outdated skipped) |
| ❌ | CI passing | 6 check(s) still running |
Files Changed (19 files, +1022 −6)
| File | +/− |
|---|---|
.changeset/reasoning-effort.md |
+17 −0 |
.github/agents/squad.agent.md |
+27 −0 |
.squad-templates/squad.agent.md |
+27 −0 |
packages/squad-cli/templates/squad.agent.md.template |
+27 −0 |
packages/squad-sdk/src/adapter/types.ts |
+4 −0 |
packages/squad-sdk/src/agents/charter-compiler.ts |
+32 −0 |
packages/squad-sdk/src/agents/index.ts |
+5 −0 |
packages/squad-sdk/src/agents/lifecycle.ts |
+20 −6 |
packages/squad-sdk/src/builders/index.ts |
+8 −0 |
packages/squad-sdk/src/builders/types.ts |
+8 −0 |
packages/squad-sdk/src/config/models.ts |
+333 −0 |
packages/squad-sdk/src/config/schema.ts |
+3 −0 |
packages/squad-sdk/src/coordinator/fan-out.ts |
+16 −0 |
packages/squad-sdk/templates/charter.md |
+1 −0 |
packages/squad-sdk/templates/squad.agent.md.template |
+27 −0 |
templates/squad.agent.md.template |
+27 −0 |
test/charter-compiler.test.ts |
+66 −0 |
test/fan-out.test.ts |
+72 −0 |
test/model-preference.test.ts |
+302 −0 |
Total: +1022 −6
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
🟠 Impact Analysis — PR #1148Risk tier: 🟠 HIGH 📊 Summary
🎯 Risk Factors
📦 Modules Affectedci-workflows (1 file)
root (2 files)
squad-cli (1 file)
squad-sdk (11 files)
templates (1 file)
tests (3 files)
|
|
@tamirdresher thanks for the thorough review! Addressed all four nits in ff77480:
|
Summary
Threads
reasoningEffortthrough the full agent spawning pipeline, enabling per-agent configuration of reasoning effort levels (low,medium,high,xhigh).Problem
SquadSessionConfigalready has areasoningEffort?: SquadReasoningEffortfield that passes through to the Copilot SDK, but Squad's spawning pipeline never sets it. Users cannot configure reasoning effort per-agent through charter files, config, or SDK builders.Solution
Thread
reasoningEffortthrough all 3 configuration layers and both spawning paths, mirroring the existing pattern formodel:Configuration Layers
**Reasoning Effort:**from## Modelsection incharter.mddefaultReasoningEffortandagentReasoningEffortOverridesin.squad/config.jsonreasoningEffortondefineAgent()anddefineDefaults()Spawning Paths
SpawnAgentOptions.reasoningEffortOverride→SquadSessionConfig.reasoningEffortAgentSpawnConfig.reasoningEffortOverride→createSession({reasoningEffort})Resolution
New
resolveReasoningEffort()with layered priority hierarchy matching model resolution:The value
autois a sentinel meaning "let the downstream SDK/API decide" and resolves toundefined.Effort Clamping
New
clampReasoningEffort()caps requested effort to the model's maximum supported level. This prevents API errors when a user requests e.g.xhighon a model that only supports up tohigh.Behavior:
xhighon a model with[low,medium,high]→high(clamped)xhighon a model with no effort support →undefined(stripped)maxandxhightreated as equivalent rankresolveReasoningEffort()accepts optionalsupportedEffortsfromlistModels()and auto-clampsUsage Example
In charter.md:
In .squad/config.json:
{ "defaultReasoningEffort": "high", "agentReasoningEffortOverrides": { "fenster": "xhigh" } }In squad.config.ts:
Files Changed (13)
packages/squad-sdk/src/agents/charter-compiler.ts**Reasoning Effort:**, add toParsedCharter,CharterConfigOverrides,CompiledCharterpackages/squad-sdk/src/agents/index.tsreasoningEfforttoAgentCharterinterface, wire into compile methodspackages/squad-sdk/src/agents/lifecycle.tsreasoningEffortOverridetoSpawnAgentOptions, pass toSquadSessionConfigpackages/squad-sdk/src/coordinator/fan-out.tsreasoningEffortOverridetoAgentSpawnConfig, optionalresolveReasoningEffortonFanOutDependenciespackages/squad-sdk/src/config/models.tsread/writeReasoningEffort(),resolveReasoningEffort(),clampReasoningEffort()packages/squad-sdk/src/config/schema.tsreasoningEfforttoAgentConfig,defaultReasoningEfforttoModelConfigpackages/squad-sdk/src/builders/types.tsreasoningEfforttoAgentDefinitionandDefaultsDefinitionpackages/squad-sdk/src/builders/index.tsreasoningEffortindefineAgent()anddefineDefaults()packages/squad-sdk/templates/charter.md**Reasoning Effort:** autoto## Modelsectiontest/charter-compiler.test.tstest/fan-out.test.tstest/model-preference.test.ts.changeset/reasoning-effort.mdTesting
Unit Tests
npm run build)resolveReasoningEffortonFanOutDependenciesis optionalLive Integration Test (claude-opus-4.7-1m-internal)
Ran a live test creating real Copilot sessions to verify reasoning effort is actually applied at the API level:
xhighassistant.reasoningeventKey findings:
reasoningEffort: "xhigh"onclaude-opus-4.7-1m-internalproduced 8 reasoning/thinking delta events with actual thinking content. The default (medium) produced zero.SquadSessionConfig.reasoningEffort→ Copilot SDK → Copilot CLI → API → model thinking tokens.Live Model Capabilities (from
listModels()API)[none,low,medium,high][none,low,medium,high][none,medium][none,low,medium,high,xhigh][none,low,medium,high,xhigh][none,low,medium,high,xhigh][none,low,medium,high,xhigh][none,low,medium,high][none,low,medium,high]Clamping Validation
xhightoclaude-sonnet-4.6(max:high) — API silently accepted (no crash), confirming clamping is a nice-to-have for transparency rather than crash prevention.