fix(session-flow): make worker model tier an explicit spawn decision (0.9.1)#340
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a7c5bfb05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1a7c5bf to
622b5ef
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 622b5efc52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
622b5ef to
8a4593c
Compare
|
Claude finished @kyle-sexton's task in 1m 55s —— View job PR Review: fix(session-flow): make worker model tier an explicit spawn decisionTasks
SummaryThe changes are well-scoped and address a real silent-failure mode (premium-model fan-outs via Findings1. PR title version label is stale — cosmeticThe PR title says Not blocking, but it creates a confusing paper trail — the PR description also says "session-flow 0.8.0 → 0.8.1". Consider updating the PR title to 2. Imperative 2 doesn't surface
|
| Rule | Status |
|---|---|
| Repo-agnostic (no hardcoded paths/names) | ✅ |
| Configurable without editing the plugin | ✅ |
Plugin-form-safe (CLAUDE_PLUGIN_ROOT / CLAUDE_PLUGIN_DATA) |
✅ (instruction-only skill, no file I/O) |
| No PII / secrets | ✅ |
| Explicit semver version | ✅ (0.9.1 in plugin.json) |
Verdict
Approve with nits. Finding 2 is the only substantive gap — a single parenthetical in imperative 2 closes it. Finding 1 (PR title) is cosmetic. Finding 3 is pre-existing and out of scope.
…(0.9.1) A session armed by /orchestrate could fan out 100+ workers with no model stated on any spawn; an omitted model defaults to inherit per the subagents doc, so every worker silently ran on the parent session's premium model. SPEC EVERY SPAWN now includes a deliberately chosen model tier, CALIBRATE TO CONDITIONS adds per-worker tiering (cheap tier for high-volume mechanical work, parent tier reserved for judgment-heavy synthesis/verify, wider fan-out defaults cheaper), and sources.md cites the official model-resolution order and cost-control guidance. Rebased over session-flow 0.9.0 (reanchor skill); version target moved from 0.8.1 to 0.9.1. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
8a4593c to
7e79ac4
Compare
|
Claude finished @kyle-sexton's task in 1m 38s —— View job PR Review: fix(session-flow): make worker model tier an explicit spawn decision (0.9.1)Tasks
SummaryRebase over Finding 1 —
|
| Rule | Status |
|---|---|
| Repo-agnostic (no hardcoded paths/names) | ✅ |
| Configurable without editing the plugin | ✅ |
Plugin-form-safe (CLAUDE_PLUGIN_ROOT / CLAUDE_PLUGIN_DATA) |
✅ (instruction-only skill, no file I/O) |
| No PII / secrets | ✅ |
| Explicit semver version | ✅ (0.9.1 in plugin.json) |
Verdict
Approve with nit. The rebase is clean, the changelog and version bump are consistent, and the instruction additions are well-sourced. Finding 1 is the only substantive gap — a single parenthetical in imperative 2 closes the CLAUDE_CODE_SUBAGENT_MODEL blind spot. Finding 2 is informational. Finding 3 is pre-existing and out of scope.
… precedence Review finding: CLAUDE_CODE_SUBAGENT_MODEL resolves before per-invocation and frontmatter values, so an omitted model inherits the parent only when no consumer-level override is set — as sources.md already documents. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 48s —— View job PR Review: fix(session-flow): make worker model tier an explicit spawn decision (0.9.1)Tasks
SummaryThe latest commit ( Previous finding — now resolvedFinding 1 (P2): Both prior reviews flagged that imperative 2 implied model inheritance always applies when a spawn omits Before:
After:
The qualifier is accurate and keeps the imperative concise. The full resolution order and env-var name remain in One very minor observation: "consumer-level subagent-model override" is technically correct but abstract for a practitioner who hasn't encountered it before. Adding " Remaining items from prior reviews
CLAUDE.md design rules
Fresh-docs mandate
VerdictApprove. The one substantive gap has been closed. The four changed files (SKILL.md, sources.md, CHANGELOG.md, plugin.json) are internally consistent and the instruction additions are well-sourced. No blocking issues remain. |
) ## What Sharpens the `orchestrate` skill's per-worker tiering (imperative 7) into an explicit **volume-based default**, and broadens "tier" from model-only to model **and** reasoning effort. ## Why Carried from the re-anchor round-2 ledger (task #3), observed in production: an `/orchestrate` run fanned out ~100–200 agents, all on the top-tier model. PR #340 (0.9.1) fixed the root spawn-inherit bug — SPEC EVERY SPAWN + CALIBRATE now carry a per-worker tier — but the residual enhancement (making the tier *default* flip on volume) was never built. The prior text stated the direction as a gradient ("the wider the fan-out, the cheaper the default per-worker tier") without wiring the verify/judge carve-out as the standing exception, and never named reasoning effort as a tiering lever. ## Change `plugins/session-flow/skills/orchestrate/SKILL.md` — imperative 7: - Past a wide fan-out, the cheaper tier is now the **DEFAULT the whole fleet inherits** (volume multiplies every notch of over-provisioning). - The **standing exception** is an explicitly hard stage — verify, judge/adjudicate, judgment-heavy synthesis — which keeps the parent tier. A premium fan-out outside those stages is a per-stage decision to justify, never a default to inherit. - **Tier is model AND effort**: match reasoning depth (effort), not just the model, to the subtask. `context/sources.md` — three new verbatim citations grounding the change in current official docs: the per-worker `effort` frontmatter lever, the workflow fleet-model inherit mechanism, and the platform's own wide-run threshold (its `Large workflow` warning + `/config` size guideline). Per the skill's model-/tool-agnostic construction, these version-pinned specifics stay in `sources.md`, never the imperatives or export brief. ## Grounding (fresh docs, this session) - Subagent `model` precedence + `inherit` default, `effort` field, cost-routing guidance — <https://code.claude.com/docs/en/sub-agents> - Workflow fleet-model inheritance, per-stage routing, `Large workflow` threshold + size guideline — <https://code.claude.com/docs/en/workflows> ## Gates - `check-skill.sh orchestrate`: PASS (7/7 trigger phrases preserved, 125/500 lines, markdownlint clean) - `check-changelog-parity.sh --check` and `--check-bump origin/main`: PASS - `validate-plugin-contracts.mjs`, `generate-catalog.mjs --check`, `claude plugin validate .`: PASS - Version bumped 0.12.2 → 0.12.3 with CHANGELOG entry in the same diff. ## Related - PR #340 — the session-flow 0.9.1 spawn-inherit fix this builds on (SPEC EVERY SPAWN + CALIBRATE carrying a per-worker tier); not closed by this PR. - The re-anchor round-2 ledger task #3 production observation that motivated the residual enhancement. Closes #863. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What
The
/session-flow:orchestrateskill let an armed session fan out 100+ workers without a model stated on any spawn. Per the official subagents doc, an omittedmodeldefaults toinherit(resolution order:CLAUDE_CODE_SUBAGENT_MODELenv var → per-invocationmodelparam → agent frontmatter → main conversation's model), so every worker silently ran on the parent session's premium model. Observed: a 100–200-agent run entirely on Fable.Changes
0.9.0 → 0.9.1(rebased over the 0.9.0 reanchor-skill release), CHANGELOG entry.Why guidance, not enforcement
The skill is instruction text — it cannot set a model itself. The blunt environment-level guard (
CLAUDE_CODE_SUBAGENT_MODEL) remains available to consumers and is deliberately not prescribed: it flattens ALL subagents to one tier, including judgment-heavy verifiers.Fresh-docs mandate
Fetched this session and cited: https://code.claude.com/docs/en/sub-agents (model resolution, omitted-defaults-to-inherit, cost-control quote).
Gates
claude plugin validate ./plugins/session-flow: pass🤖 Generated with Claude Code
https://claude.ai/code/session_01VLztic1sFn2CVFJsubr7pu
Related
No linked issue.