feat(subagents,workflows): suffix-first reasoning levels for model fallbacks

[lavaman131]

Summary

Introduces `model:reasoning_level` suffix notation (e.g. `openai/gpt-5:high`) as the canonical, per-candidate way to configure reasoning levels in both subagents and workflows. Adds a `fallbackThinkingLevels` compatibility shim for gradual migration and deprecates the shared `thinking`/`thinkingLevel` options.

Key Changes

New `:level` suffix syntax

• Model strings and `fallbackModels` entries now accept an optional `:` suffix — valid values: `:off`, `:minimal`, `:low`, `:medium`, `:high`, `:xhigh`
• `splitReasoningSuffix` / `splitKnownThinkingSuffix` validate the suffix against the known level set before extracting; unrecognised suffixes (e.g. Ollama `:latest`, OpenRouter `:free`) are left intact as part of the model ID — no silent truncation
• Each fallback candidate is self-contained — its suffix takes precedence over any legacy shared `thinking`/`thinkingLevel` options

Compatibility shim (`fallbackThinkingLevels`)

• Added `fallbackThinkingLevels?: string[]` to both subagent and workflow configs — entries in `fallbackModels` without a suffix inherit a level from the parallel array; entries that already carry a suffix ignore the shim
• Invalid shim entries throw `WorkflowModelValidationError` at candidate-build time
• Parsed from frontmatter (comma-separated) in `.md` agent definitions alongside `fallbackModels`

Deduplication

• `candidateKey` hashes on `id::reasoningLevel` instead of `id` alone, so the same model at different reasoning levels is treated as a distinct fallback entry

Metadata & observability

• `WorkflowModelAttempt.reasoningLevel` and `ModelAttempt.reasoningLevel` surface the effective reasoning level in attempt/status metadata
• Fallback warning messages include the reasoning suffix (e.g. `claude-3-5-sonnet:high failed … retrying with claude-3-opus:medium`)
• Fixed foreground subagent attempt metadata to report the per-candidate reasoning level from the model suffix even when the legacy `thinking` option is unset, matching the background run path

Stage runner wiring

• `stageOptionsForCandidate` injects `thinkingLevel` from `candidate.reasoningLevel` when present, so each attempt receives the correct reasoning configuration
• `applyCandidateThinking` resolves effective reasoning per-candidate (suffix > stage-level fallback)

Deprecations

• `thinkingLevel` stage option in `@bastani/workflows` — prefer `:` suffix on `model`/`fallbackModels`
• `thinking` field on subagent config — prefer `:` suffix on `model`/`fallbackModels`

Builtins migrated

• All bundled subagent `.md` definitions and workflow builtins (`goal`, `ralph`, `deep-research-codebase`, `open-claude-design`) encode reasoning levels directly in their model/fallback strings

Tests

• `test/unit/model-fallback.test.ts` — suffix parsing, deduplication, `fallbackThinkingLevels` compat logic, and `WorkflowModelValidationError` on invalid shim values
• `test/unit/stage-runner.test.ts` — end-to-end reasoning propagation in stage execution
• `test/unit/subagents-reasoning-suffix.test.ts` — subagent-side suffix parsing and candidate building

Migration

Before:
```ts
defineWorkflow("example")
.stage("step", {
model: "openai/gpt-5",
fallbackModels: ["anthropic/claude-opus-4"],
thinkingLevel: "high",
})
```

After:
```ts
defineWorkflow("example")
.stage("step", {
model: "openai/gpt-5:high",
fallbackModels: ["anthropic/claude-opus-4:medium"],
})
```

The `thinkingLevel`/`thinking` options continue to work as shared fallback defaults for any candidate that lacks a suffix — no immediate migration required.

Closes #1199

[claude]

Code Review

Nice, focused change. The suffix-first design is clear, retry semantics now flow consistently through foreground/background/async, and test coverage hits the important interaction paths (suffix precedence, legacy thinking fallback, dedup, positional fallbackThinkingLevels).

A few items worth addressing before merge:

Bugs / correctness

1. Stray indentation in packages/workflows/builtin/open-claude-design.ts:235
Removing thinkingLevel: \"high\" as const, left the closing brace mis-indented:

    const designModelConfig = {
      model: \"anthropic/claude-opus-4-8:high\",
      fallbackModels: [ ... ],
      };   // <-- 6 spaces; should be 4

Minor, but the linter/format style elsewhere in the file uses 4 spaces.

2. Inconsistent handling of invalid reasoning suffixes between subagents and workflows

• packages/workflows/src/runs/shared/model-fallback.ts:splitReasoningSuffix throws WorkflowModelValidationError when a colon-suffix looks like a level (matches /^[A-Za-z][A-Za-z0-9_-]*$/) but isn't canonical. Likewise buildModelCandidates throws on invalid fallbackThinkingLevels[i].
• packages/subagents/src/runs/shared/model-fallback.ts:splitThinkingSuffix and applyFallbackThinkingLevel silently treat an unknown suffix as no suffix (or drop an invalid fallbackThinkingLevels[i] entry).

A user typo like gpt-5:hihg or fallbackThinkingLevels: [\"mediun\"] will be loudly rejected for workflow configs but silently misapplied for subagent configs (the legacy thinking value then leaks through). Since the two packages document the same suffix vocabulary and target the same audience, the failure mode should be the same — ideally both validate at parse/load time and surface a clear error.

3. Duplicated split helper inside the subagents package
splitThinkingSuffix in src/runs/shared/model-fallback.ts:13 and splitKnownThinkingSuffix in src/shared/model-info.ts:40 are functionally identical. Pick one and delete the other (or have one re-export the other) — easy to let them drift later.

4. WorkflowModelAttempt.model shape differs between packages

• Workflows (stage-runner.ts:743/747): model = candidate.id (no suffix), reasoningLevel is separate.
• Subagents (subagent-runner.ts:707 and execution.ts:867): model = attemptModel which already includes the :level suffix, and reasoningLevel is set. So the suffix is encoded twice.

Anything consuming modelAttempts downstream (status output, telemetry, doctor, UI) now has to know that the workflows side will only show suffix-as-reasoningLevel but the subagents side will show suffix-as-both. Recommend stripping the suffix from attempt.model in the subagents path so the field semantics match — reasoningLevel should be the canonical place.

Smaller polish

• workflow-schema.ts: fallbackThinkingLevels is Type.Array(Type.String()) with no enum. Runtime validation catches bad values, but a tighter TypeBox schema would surface bad config in TUI/editor sooner and matches the README's enumeration.
• The fallback warning string (stage-runner.ts:752) only annotates the suffix when candidate.reasoningLevel is set. If the legacy stageOptions.thinkingLevel is the effective level, the warning omits it, so users using the deprecated path get a less informative message. Cheap to surface effectiveCandidateReasoning(candidate) instead.
• splitThinkingSuffix test asserts splitThinkingSuffix(\"provider:model:ultra\") → { baseModel: \"provider:model:ultra\", thinkingSuffix: \"\" }, which means applyThinkingSuffix(\"provider:model:ultra\", \"high\") → \"provider:model:ultra:high\". That seems intentionally permissive, but if any provider id ever ships a model whose colon-suffix happens to collide with a canonical level (e.g. an experimental :low build), the parser will silently steal it. Probably fine in practice; worth a note in the deprecation comment.

Things I liked

• Per-candidate self-containment via stageOptionsForCandidate + applyCandidateThinking cleanly threads the level through session creation and attempt metadata.
• Dedup key is (id, reasoningLevel) in both packages, which correctly preserves "same model, different reasoning" entries in a fallback chain.
• Deprecation strategy (keep thinking / thinkingLevel as the no-suffix default; defer removal) is a good migration path, and the README/SKILL.md updates explain the precedence explicitly.
• Test for legacy thinkingLevel still applying when no candidate has a suffix is exactly the right regression check.

Not concerns

• No security or performance impact — pure config plumbing.
• Test coverage of the new code paths looks proportionate.

Suggest at minimum fixing items 1 and 4 (and ideally aligning 2) before merging. The rest can land as follow-ups.

[claude]

Review — feat(subagents,workflows): suffix-first reasoning levels

Nice migration overall. The suffix-first model is clearly more expressive than the parallel thinkingLevel option, and the lenient parser (only stripping canonical levels) correctly accommodates legitimate colon-tagged ids like ollama/llama3:latest, openrouter/.../...:free, etc. Documentation, CHANGELOGs, and the migration shim all look intentional. Below are a few things worth tightening before merge.

Correctness

1. Inconsistent validation for invalid fallbackThinkingLevels between the two packages

   • packages/workflows/src/runs/shared/model-fallback.ts:166-178 — throws WorkflowModelValidationError if a fallbackThinkingLevels[i] is not canonical.
   • packages/subagents/src/runs/shared/model-fallback.ts:27-31 — applyFallbackThinkingLevel silently returns the model unchanged on an invalid level.

   A subagent author who types fallbackThinkingLevels: meidum, low in markdown frontmatter gets zero feedback and silently runs without their intended level, while the workflow author for the same typo gets a hard error. Pick one policy and apply it in both packages (I'd lean on the workflow path — fail loud).

2. packages/workflows/builtin/open-claude-design.ts:235 — the closing }; is over-indented (6 spaces instead of 4) after thinkingLevel: \"high\" as const, was deleted. Lint won't catch it; just visual noise.

3. packages/workflows/src/runs/shared/model-fallback.ts:114 — the synthesized candidate uses ${fallback}:${compatLevel} with the raw, un-trimmed fallback. If the user wrote \" openai/gpt-5 \" in their fallback list, the appended suffix becomes \" openai/gpt-5 :low\". The outer input.trim() in resolveStringModel strips the leading/trailing space but not the inner one. Trivial fix: rawValues.push(\${fallback.trim()}:${compatLevel}`)`.

Design / Maintainability

4. Duplicated source-of-truth for canonical thinking levels. packages/workflows/src/runs/shared/model-fallback.ts:17 declares its own WORKFLOW_THINKING_LEVELS, while subagents pull from packages/subagents/src/shared/model-info.ts:1. These two will drift the next time a level is added (e.g. when xxhigh lands somewhere). Either share via a small common module or at minimum leave a // keep in sync with … comment on both.

5. Two near-identical parsers — splitThinkingSuffix (subagents) and splitReasoningSuffix (workflows) — same intent, different return shapes ({thinkingSuffix: string} vs {level?: WorkflowThinkingLevel}). Worth converging on a single helper for long-term maintenance; otherwise a fix on one side will be easy to forget on the other.

Test Coverage

6. Validation-error path is uncovered — buildModelCandidates throwing for invalid fallbackThinkingLevels[i] has no direct test in test/unit/model-fallback.test.ts. Worth a one-liner assert.throws to lock the contract.

7. Length-mismatch semantics aren't asserted. fallbackThinkingLevels shorter than fallbackModels is exercised in the new tests; the reverse (longer than fallbackModels) silently drops extras. Add a test or a doc note so future readers don't have to grep.

Deprecations

8. The @deprecated Prefer suffixing... removal is deferred JSDoc tags on WorkflowScopedModel.thinkingLevel and StageOptions.thinkingLevel don't name a target removal window. Even "next major" is enough to help consumers plan migration.

Security / Performance

No concerns. Suffix parsing is lastIndexOf + 6-element scan — bounded and operates on author-owned config.

Nice things to call out

• The deduplication key change (\${candidate.id}::${candidate.reasoningLevel ?? ""}`) plus the per-attempt reasoningLevel metadata is the right shape — distinct levels for the same base model are now treated as distinct retry slots, and downstream observers (WorkflowModelAttempt.reasoningLevel, ModelAttempt.reasoningLevel`) get the effective level in one place.
• candidateLabel(...) ensures fallback warnings include the suffix in user-facing logs (anthropic/primary:high failed … retrying with openai/fallback:low) — small but very helpful debugging UX.
• Lenient handling of :free, :beta, :latest, :online, :exacto etc. is well-covered by the new lenient-suffix tests (Add fallbackThinkingLevels support for subagents and workflows #1199) and prevents the regression mentioned in the issue.

Overall: ship-worthy after the validation symmetry and minor cleanups.

[lavaman131]

Implementation Notes

Task: Implement GitHub issue #1199 (#1199) in this repo, the Bun workspace atomic-monorepo. Title: 'Add fallbackThinkingLevels support for subagents and workflows'. The accepted plan is suffix-first reasoning levels.

Decision / migration plan

Do NOT remove existing thinking / thinkingLevel fields. Migrate subagents and workflows to a suffix-first model where every primary and fallback model candidate can carry its own reasoning level as model_name:reasoning_level.

1. Support reasoning suffixes EVERYWHERE models are accepted:
   • Subagents: model, fallbackModels, builtin/user/project overrides, and frontmatter accept entries like claude-sonnet-4:medium or gpt-5:low.
   • Workflows: stage options, task/direct/chain/parallel parameters, workflow parameters, builtin workflow definitions, and any catalog/schema path that accepts model/fallbackModels accept the same suffix format.
   • Each retry candidate is self-contained: the selected model string determines BOTH provider/model and reasoning level when a suffix is present.
2. Deprecate separate thinking fields but keep compatibility:
   • Keep accepting thinking (subagents) and thinkingLevel (workflows) during the migration window.
   • Treat them as legacy/default reasoning-level settings used ONLY when the selected model entry has no reasoning suffix.
   • Document them as deprecated; prefer explicit suffixes.
3. Migrate builtins and docs:
   • Update builtin subagents/workflows to use model_name:reasoning_level suffixes instead of separate thinking fields where reasoning level is needed.
   • Update README/skill/workflow docs to teach the suffix-first format and mark separate thinking fields deprecated. Show fallback-chain examples, e.g. model: claude-sonnet-4:high with fallbackModels [claude-sonnet-4:medium, gpt-5:low, claude-haiku-4:off].
4. Removal of separate fields is deferred to a later breaking release (do NOT remove now).

fallbackThinkingLevels is retained ONLY as an optional compatibility/helper path for legacy users, NOT the preferred long-term API. Preferred user-facing API is explicit reasoning suffixes on each model/fallbackModels entry.

Expected behavior

• model and fallbackModels entries accept optional :reasoning_level suffixes everywhere subagents and workflows accept model names.
• Primary attempt uses configured model incl. suffix; retries use fallbackModels in order incl. each entry's suffix.
• If an entry has no suffix, legacy thinking/thinkingLevel may supply the reasoning level during the deprecation window.
• If an entry HAS a suffix, the suffix takes precedence over legacy thinking fields for that candidate.
• Existing fallback behavior remains intact when no suffixes are used.
• Attempt/result/status metadata clearly reports BOTH the resolved model and effective reasoning level used for each attempt.

Likely implementation areas (from issue's repo search)

Subagents

• packages/subagents/src/runs/shared/model-fallback.ts: buildModelCandidates(), splitThinkingSuffix(), retryable failure detection.
• packages/subagents/src/runs/shared/pi-args.ts: applyThinkingSuffix(), buildPiArgs() passes suffixed model via --model.
• packages/subagents/src/runs/foreground/execution.ts: builds candidates, loops modelsToTry, runSingleAttempt(), retries on isRetryableModelFailure().
• packages/subagents/src/runs/background/async-execution.ts: builds candidates, maps each through applyThinkingSuffix(..., agentConfig.thinking).
• packages/subagents/src/runs/background/subagent-runner.ts: starts attempts with model + effective thinking metadata, records status, runs fallback attempts.
• packages/subagents/src/agents/agents.ts: AgentConfig, BuiltinAgentOverrideBase, BuiltinAgentOverrideConfig define model/fallbackModels/thinking; frontmatter parsing; override build/apply.
• packages/subagents/README.md and packages/subagents/skills/subagent/SKILL.md: document override + frontmatter fields.

Workflows

• packages/workflows/src/extension/workflow-schema.ts: StageSessionOptionProperties exposes model/thinkingLevel/fallbackModels for direct task, parallel chain step, workflow params, stage options schema.
• packages/workflows/src/shared/types.ts: WorkflowModelFallbackFields, WorkflowModelAttempt, StageOptions.
• packages/workflows/src/runs/foreground/stage-runner.ts: createStageContext() builds candidates, strips workflow-only options, per-candidate sessions, applies pending setThinkingLevel(), retries in promptWithFallback(), reports model attempt metadata.
• packages/workflows/src/runs/shared/model-fallback.ts: buildModelCandidates()/buildModelCandidatesFromCatalog() generate ordered attempts and validate fallback models.
• packages/workflows/builtin/*.ts: builtin workflow stage definitions may need migration from separate thinkingLevel to suffix-based model entries.

Conventions (from AGENTS.md — follow strictly)

• Use Bun ONLY: bun, bunx, bun run. Never node/npm/npx/yarn/pnpm.
• packages/workflows ships raw .ts with NO build step — do NOT add dist/, tsconfig.build.json, outDir, or bundling.
• Avoid any/unknown; use specific types. Source files use .js import extensions (TS ESM convention).
• Add CHANGELOG entries under each affected package's ## [Unreleased] section (### Added / ### Changed / ### Deprecated as appropriate), referencing issue Add fallbackThinkingLevels support for subagents and workflows #1199. Read existing [Unreleased] first; append to existing subsections; never modify released sections.
• Tests use bun:test + node:assert/strict.

Validation (all must pass before declaring done)

• bun run typecheck
• bun run lint
• bun run test:unit (and relevant integration tests for touched areas).
• New unit tests must cover: suffix parsing/application; suffix precedence over legacy thinking fields; ordered + deterministic candidate generation with de-duping of model+reasoning combinations; foreground subagent retry; chain/parallel and async/background subagent retry; foreground workflow stage retry (direct task, chain, parallel); legacy compatibility; and attempt/result/status metadata reporting model + effective reasoning level.

Acceptance criteria (must all hold)

• Suffixes model_name:reasoning_level accepted for model and fallbackModels everywhere subagents and workflows accept model config.
• Existing thinking/thinkingLevel configs continue to work during the deprecation window.
• When both suffix and legacy thinking field are present, the suffix defines the effective reasoning level for that candidate.
• Builtin subagents/workflows and docs migrated to suffix-first examples; separate fields documented as deprecated with removal deferred to a later breaking release.
• fallbackThinkingLevels retained only as optional compatibility/helper path.
• Retry candidate generation remains ordered and deterministic with clear de-duping rules.
• Foreground, chain/parallel, and async/background subagent runs use the resolved per-candidate reasoning level on retryable failures.
• Foreground workflow stage runs (direct task, chain, parallel) use the resolved per-candidate reasoning level on retryable failures.
• Attempt/result/status metadata clearly reports the model and reasoning level used for each attempt.
• typecheck, lint, and tests pass.
• At the end, since this is a detached-HEAD worktree, create and push a branch from HEAD and open a PR if credentials/remote are available.

Running Notes

• Record implementation decisions, deviations from the spec, tradeoffs, blockers, validation notes, and anything else the user should know.

Iteration 1/10 — Outcomes (2026-06-02)

Setup

• Fresh worktree had NO node_modules; ran bun install (353 packages). Baseline bun run typecheck passed before changes.

Delegated work (subagent tool)

• Stream A (subagents pkg) + Stream B (workflows pkg) implemented in parallel (disjoint files).
• Stream C added behavioral retry/metadata tests for spec §8.3 gaps.

Decisions / deviations

• splitThinkingSuffix HARDENED (subagents) to only treat trailing :<token> as a reasoning suffix when <token> is canonical (off|minimal|low|medium|high|xhigh). Behavior change: provider:model:ultra now treated as base model, not split. Resolves spec open question add agent instructions #1 (no current model id relies on unvalidated split).
• Builtin migration (open question update readme and mcp servers #3): replicated the prior uniform thinking/thinkingLevel onto the primary model AND every fallback entry to preserve existing effective behavior, then removed separate thinking: lines (subagents) / used model:level suffixes (workflows).
• fallbackThinkingLevels implemented as OPTIONAL compat helper only: positionally aligned to fallbackModels, applied only when that entry has no suffix; suffix always wins. Not the preferred API.
• Workflows codec is a small LOCAL copy (no cross-package import) per AGENTS.md raw-.ts/no-coupling rule.
• Open question Flora131/feat/add skills #4 (setThinkingLevel post-attach vs create-time): workflows now places candidate level into per-candidate create options AND keeps the post-attach setThinkingLevel seam. Executor fork path uses SessionManager.forkFrom in stripWorkflowOnlyOptions; replay stub rejects mutations but replay creates no live fallback attempts. Considered safe.
• Foreground subagent attempt.model now records the EFFECTIVE resolved model string (after applyThinkingSuffix), so legacy no-suffix configs surface the appended suffix in metadata. Intentional for G7 metadata clarity.
• Test seam tradeoff: subagent foreground/async retry tests assert at the candidate->attempt mapping seam (buildModelCandidates + applyThinkingSuffix + resolveEffectiveThinking) rather than full pi-process orchestration (no new process-mock framework). Workflow stage-runner tests are full behavioral retries (direct/chain/parallel) via existing createStageContext harness incl. setThinkingLevel + __modelFallbackMeta assertions.

Bug found & fixed during testing

• workflows stage-runner: legacy no-suffix fallback attempts applied thinkingLevel to the session but recorded reasoningLevel: undefined in WorkflowModelAttempt. Fixed to record candidate.reasoningLevel ?? stageOptions.thinkingLevel.

Validation

• bun run typecheck: PASS (== lint). bun test test/unit: 1975 pass / 0 fail.
• bun test test/integration: 217 pass; the 1 "failure" is a known flake — workflow-package-typing test runs a real tsc (~5.7s) and exceeds bun's default 5s per-test timeout. Re-run with --timeout 30000: PASS.

New/changed test files

• test/unit/subagents-reasoning-suffix.test.ts (new), test/unit/model-fallback.test.ts (extended, workflows), test/unit/stage-runner.test.ts (extended).

Branch / PR

• Branch: feature/issue-1199-suffix-reasoning (from detached HEAD 56aaa82).
• Commit: 1dde9b8 — 32 files changed, +576/-103 (spec file under specs/ intentionally NOT committed).
• Pre-commit/prek hooks ran on commit AND push (lint + test:unit) — all Passed, no hook edits.
• PR: feat(subagents,workflows): suffix-first reasoning levels for model fallbacks #1203 (base main), state OPEN, "Closes Add fallbackThinkingLevels support for subagents and workflows #1199".

Iteration 2/10 — Outcomes (2026-06-02)

Scope

Iteration 1 claimed the two review-round-1 fixes were done, but the COMMITTED code still had both defects. Iteration 2 actually lands them.

Fixes applied (via typescript-expert subagent)

1. Workflows splitReasoningSuffix (model-fallback.ts) made LENIENT: removed the throw branch on non-canonical alphabetic colon suffixes. Unknown suffix now keeps the WHOLE string as base model (matches subagents' splitThinkingSuffix + upstream model-resolver.ts). resolveStringModel try/catch simplified to a direct call. WorkflowModelValidationError class KEPT (still used for catalog-miss/ambiguous + fallbackThinkingLevels invalid-level check). gpt-5:ultra now surfaces the generic 'not available' catalog-miss error, not a suffix-specific throw.
2. Subagents foreground execution.ts:866 reordered to applyThinkingSuffix(candidate, agent.thinking) ?? result.model ?? agent.model ?? 'default' so attempt metadata derives the per-candidate reasoning level from the suffixed candidate (not the SDK's suffix-stripped result.model echo). undefined-candidate case still falls back to result.model.

Pre-existing uncommitted helpers

Working tree already had makeCandidate (model-fallback.ts) and candidateLabel (stage-runner.ts) refactors from an earlier attempt; kept and built upon. stage-runner.ts change is only that cosmetic candidateLabel helper.

Decisions / tradeoffs

• Test B (foreground metadata) uses SEAM-level assertions (applyThinkingSuffix + resolveEffectiveThinking + the line-866 precedence expression) rather than mocking the real runSync child-process path — consistent with the existing sibling-test convention; full process mocking does not exist in the unit suite.
• Updated (not just added) the old splitReasoningSuffix assert.throws test since its strict contract now contradicts the lenient behavior.

CHANGELOG

Added ### Fixed entries under ## [Unreleased] referencing #1199 in both packages/workflows/CHANGELOG.md and packages/subagents/CHANGELOG.md.

Validation

• bun run typecheck: PASS. bun run lint (== tsc --noEmit): PASS.
• bun test test/unit: 1982 pass / 0 fail across 111 files.

Files touched (excl. spec)

packages/workflows/src/runs/shared/model-fallback.ts, packages/workflows/src/runs/foreground/stage-runner.ts, packages/subagents/src/runs/foreground/execution.ts, packages/workflows/CHANGELOG.md, packages/subagents/CHANGELOG.md, test/unit/model-fallback.test.ts, test/unit/subagents-reasoning-suffix.test.ts.

[claude]

Review: suffix-first reasoning levels

Nice work — this is a clean, well-tested feature. The lenient suffix parsing (only stripping canonical off/minimal/low/medium/high/xhigh) is the right call and correctly unblocks legitimate colon-tagged ids like OpenRouter :free / :exacto and Ollama llama3:latest. Suffix-over-legacy precedence is applied consistently across the foreground and background subagent paths and the workflow stage runner, and the changelog/docs discipline is solid. A few things worth addressing:

🟡 Inconsistent invalid-level handling between the two packages

The workflows path throws WorkflowModelValidationError on an invalid fallbackThinkingLevels entry (packages/workflows/src/runs/shared/model-fallback.ts), but the subagents path silently ignores it. In packages/subagents/src/runs/shared/model-fallback.ts:27, applyFallbackThinkingLevel returns the model unchanged when thinkingLevel is not in THINKING_LEVELS — a typo'd level is silently dropped.

So a fallbackThinkingLevels value of hgih errors in a workflow but is silently no-op'd in a subagent. The PR description says invalid shim values throw, which only holds for workflows. Consider making subagents validate too (or at least surface a warning) so a typo isn't silently swallowed.

🟡 Workflows: invalid compat level only validated when the paired fallback lacks a suffix

In buildModelCandidates, the validation branch is guarded by split.level === undefined. If a fallbackModels[i] already carries a suffix, an invalid fallbackThinkingLevels[i] at that index is silently ignored rather than rejected. Minor, but it means the same invalid input is sometimes an error and sometimes not, depending on the paired entry. Validating the whole fallbackThinkingLevels array up front would be more predictable.

🟡 Duplicated suffix-parsing logic in the subagents package

After this change, splitThinkingSuffix (runs/shared/model-fallback.ts:14) is behaviorally identical to the pre-existing splitKnownThinkingSuffix (shared/model-info.ts:40) — same signature, same validation, same return shape. There are also now three copies of the level list: THINKING_LEVELS in shared/model-info.ts:1, a separate local THINKING_LEVELS in runs/shared/pi-args.ts:15, and WORKFLOW_THINKING_LEVELS in workflows. Cross-package duplication is understandable (workflows ships raw TS), but the two intra-package split* helpers and the duplicated THINKING_LEVELS const are worth consolidating to a single source of truth to avoid future drift.

🟢 Nit: stray indentation in open-claude-design.ts

The closing brace of designModelConfig lost its alignment when the thinkingLevel line was removed — it is now indented 6 spaces instead of 4. Harmless, but the formatter/lint hook should catch it; please re-run bun run lint / the prek hooks.

Minor: silent length mismatch

In both packages, a fallbackThinkingLevels array longer than fallbackModels silently ignores the extra entries. A one-line warning when lengths diverge would help authors catch a misaligned array.

Tests

Coverage is genuinely good — suffix parsing, dedup by id::level, positional compat mapping, retry metadata propagation, and the lenient-parsing regression cases for OpenRouter/Ollama ids are all exercised. I couldn't run the suite in this review sandbox; please confirm bun run test:unit and bun run typecheck are green on the three touched test files plus the builtins.

Overall: approve-with-nits. The two validation-asymmetry points are the most worth resolving before merge since they affect author-facing error behavior.

[claude]

Review: suffix-first reasoning levels (#1203)

Reviewed the full diff against CLAUDE.md conventions. This is a well-structured, well-documented change — the suffix-first model is clean, the legacy thinking/thinkingLevel defaults are preserved with suffix-wins precedence, and the per-attempt reasoningLevel metadata is a nice observability win. The leniency fix for colon-tagged ids (OpenRouter :free/:nitro/:exacto, Ollama :latest) is a genuine correctness improvement and is the best-tested part of the PR. Dedup by id::level and the positional fallbackThinkingLevels compat mapping both look correct, and the two suffix mechanisms (workflows split the level off into thinkingLevel; subagents keep the suffix in the model string and let pi args interpret it) are each internally consistent.

A few things worth addressing before merge — none are blockers:

Test coverage gap (highest priority)

The PR description says model-fallback.test.ts covers "WorkflowModelValidationError on invalid shim values", but there is no test that exercises the new throw path for an invalid fallbackThinkingLevels entry (packages/workflows/src/runs/shared/model-fallback.ts:169-171). The existing fallbackThinkingLevels test (test/unit/model-fallback.test.ts:116) only uses valid ["low", "off"]. Please add a case that passes e.g. fallbackThinkingLevels: ["bogus"] against a suffix-less fallback and asserts it throws WorkflowModelValidationError with the invalid fallbackThinkingLevels[0] message.

Minor bug — trim ordering in the compat branch

packages/workflows/src/runs/shared/model-fallback.ts:166-172: split is computed from the trimmed fallback, but the compat branch builds the concatenated string from the untrimmed fallback. A fallback entry with trailing whitespace ("gpt-5 ") becomes "gpt-5 :low"; after resolveStringModel trims the ends, the space is now interior, so the base id resolves to "gpt-5 " and catalog lookup fails with "not available". Trim once up front and reuse the trimmed value for both the split and the concatenation.

Formatting nit (lint will not catch this)

packages/workflows/builtin/open-claude-design.ts:235: the closing brace line is indented 6 spaces instead of 4. Since repo lint is tsc --noEmit, indentation drift like this slips through CI — worth fixing manually.

DRY — two now-identical split helpers

splitThinkingSuffix (packages/subagents/src/runs/shared/model-fallback.ts:14) is, after this change, identical in logic to splitKnownThinkingSuffix (packages/subagents/src/shared/model-info.ts:40). Consider consolidating to a single exported helper so the two cannot drift apart later.

Unmentioned (but good) side effect

Tightening splitThinkingSuffix to canonical-only levels also changes chain-clarify.ts TUI behavior (e.g. ollama/llama3:latest is no longer mis-stripped at lines 552/693/925). This is an improvement and consistent with the leniency fix, but it is a behavior change in the clarification UI that is not called out in the CHANGELOG — worth a line, plus a quick manual check that the TUI level-edit flow still extracts canonical suffixes correctly.

Lower priority

• fallbackThinkingLevels longer than fallbackModels silently drops trailing entries, and an invalid level on an already-suffixed fallback entry is silently ignored. Both are acceptable per the documented design, but a recordWarning on a typo-d/orphaned level would save users a confused debugging session.
• Inherent ambiguity (documented): a real model id ending in a canonical token (:high/:low/:off) would be misparsed as a reasoning level. Fine to accept given the docs — just flagging.
• README.md has a double blank line before the new ## Model reasoning levels heading.

Nice work overall — the design and docs are thorough, and backward compatibility is handled carefully.

🤖 Generated with Claude Code

[claude]

PR Review — suffix-first reasoning levels for model fallbacks

Reviewed the implementation, builtins migration, and tests. This is a well-structured change: the :level suffix is parsed leniently (so ollama/llama3:latest, OpenRouter :free/:exacto survive), suffix-wins-over-legacy precedence is consistent, dedup correctly keys on id::level, and the test coverage is genuinely thorough (suffix parsing, dedup, compat-shim positional mapping, whitespace trim, invalid-level throw, end-to-end stage retry propagation). The migration of every builtin is complete and faithful to the prior thinkingLevel values.

A few things worth addressing before merge:

1. (Medium) The fallback-whitespace bug fixed in workflows is NOT mirrored in subagents

Commit ab11fd9 fixed buildModelCandidates in packages/workflows by trimming the fallback before concatenating the compat level, because resolveStringModel can only trim the outer ends — interior whitespace in an id :level string is left intact and breaks catalog lookup.

The subagents path (packages/subagents/src/runs/shared/model-fallback.ts) has the identical pattern but was not fixed. applyFallbackThinkingLevel appends the :level to the untrimmed model, and the subsequent raw.trim() only strips the outer ends. With fallbackModels: ["gpt-5 "] + fallbackThinkingLevels: ["low"], you get "gpt-5 :low"; the interior space survives .trim(), so the base id resolves to "gpt-5 ", misses the catalog, and a malformed candidate is handed to the SDK. The .md frontmatter parser trims entries so this is masked there, but programmatic/builtin-override fallbackModels can hit it. Recommend trimming inside applyFallbackThinkingLevel (or before the .map) for parity with the workflows fix.

2. (Low) Inconsistent validation strictness for invalid fallbackThinkingLevels

• workflows: invalid compat level throws WorkflowModelValidationError at candidate-build time (good, and tested).
• subagents: applyFallbackThinkingLevel silently returns the model unchanged when the level is not canonical — an invalid level is silently dropped.

Same option name, two different behaviors. The silent path masks a user typo (fallbackThinkingLevels: ["hgih"] just does nothing). Consider aligning subagents to surface the bad value, or at least documenting the divergence.

3. (Low / edge) splitReasoningSuffix ambiguity for a model id that legitimately ends in a canonical word

lastIndexOf(":") + canonical-set membership means a real model id ending in :high/:low/:off would be mis-split (suffix stripped as a reasoning level, base lookup then fails to "not available"). No such id exists today and the lenient design is the right tradeoff, but the inherent collision is worth a one-line doc note so future provider ids adopting those tags do not silently break.

4. (Nit) Parallel level constants

packages/workflows keeps its own WORKFLOW_THINKING_LEVELS while this PR routed subagents onto the single splitKnownThinkingSuffix/THINKING_LEVELS in shared/model-info.ts. The packages are independent so duplication is fine, but the two lists must stay in lockstep manually — a brief cross-reference comment would prevent drift.

Verification

I could not execute bun test in this sandboxed review environment, so correctness was reviewed statically. Please confirm bun run test:unit and bun run typecheck are green in CI (the new subagents-reasoning-suffix.test.ts, stage-runner.test.ts, and model-fallback.test.ts cases look correct by inspection).

Overall: solid, well-tested, backward-compatible. Item 1 is the one I would want addressed (or consciously waived); the rest are polish.