review: per-trigger effort is dead on the claude-subagent path (config says medium, cad-reviewer pins high) #64

New issue

Closed

opened 2026-07-28 17:41:21 +00:00 by crenshawdev · 2 comments

crenshawdev commented

2026-07-28 17:41:21 +00:00

(Migrated from github.com)

What happened

On the shipped default configuration, the per-trigger effort value is read from config, passed nowhere, and silently discarded. Every review trigger runs at high regardless of what the config says.

fire(trigger) resolves { gate, tier, effort } (cadence-core/references/review-triggers.md:20). effort is then consumed in exactly one place, step 4's cross-model branch, where it becomes a seam argument:

--provider <name> --model <model> --effort <effort>

The claude-subagent branch immediately above it dispatches cad-reviewer through the spawn-agent seam and passes no effort at all. That agent's frontmatter pins it:

# agents/cad-reviewer.md:7
effort: high

The shipped default is review.reviewers: ["claude-subagent"] with no provider keys configured, so on a stock install every trigger takes the branch that ignores effort. That includes diff, which cadence-core/templates/config.json sets to medium and which fires at every plan completion.

What you expected

Either the configured effort reaches the reviewer, or the config key does not exist. A key that is read, resolved, and then dropped with no warning is the worst of the three.

DESIGN.md's "Per-trigger cognitive fit" documents the intent the code drops, so the design is on record and only the wiring is missing.

Reproduction

Stock install, no review provider keys set, so review.reviewers is the default ["claude-subagent"].

Confirm the config value the default template ships:

node cadence-core/bin/config.mjs get review.triggers.diff.effort

Expect medium.

Complete a phase plan so the diff trigger fires.
The dispatched cad-reviewer runs at high, from its own frontmatter. Nothing reports the discrepancy.

Static confirmation, no run required:

grep -n 'effort' cadence-core/references/review-triggers.md

Every hit is inside the cross-model provider arm.

sed -n '1,8p' agents/cad-reviewer.md

effort: high, pinned.

Environment

Claude Code version: 2.1.220
node --version: v26.4.0
OS: Linux (CachyOS, 7.1.5-1-cachyos)
Cadence: v1.4.0 at fba5879
Relevant config: review.reviewers: ["claude-subagent"] (default), review.triggers.diff.effort: medium (template default)

Notes

The fix folds into the reviewer row of the effort-as-rigor-lever proposal, which makes reviewer effort per-trigger by construction. Filed separately so the defect stays on the record if that proposal stalls or is rejected. Cross-link both ways.

Worth deciding as part of the fix: whether a resolved-but-undeliverable effort should warn rather than pass silently. Cadence already treats a silently-degraded path as a bug class of its own, and this is one.

## What happened On the shipped default configuration, the per-trigger `effort` value is read from config, passed nowhere, and silently discarded. Every review trigger runs at `high` regardless of what the config says. `fire(trigger)` resolves `{ gate, tier, effort }` (`cadence-core/references/review-triggers.md:20`). `effort` is then consumed in exactly one place, step 4's cross-model branch, where it becomes a seam argument: ``` --provider <name> --model <model> --effort <effort> ``` The `claude-subagent` branch immediately above it dispatches `cad-reviewer` through the spawn-agent seam and passes no effort at all. That agent's frontmatter pins it: ```yaml # agents/cad-reviewer.md:7 effort: high ``` The shipped default is `review.reviewers: ["claude-subagent"]` with no provider keys configured, so on a stock install every trigger takes the branch that ignores `effort`. That includes `diff`, which `cadence-core/templates/config.json` sets to `medium` and which fires at every plan completion. ## What you expected Either the configured effort reaches the reviewer, or the config key does not exist. A key that is read, resolved, and then dropped with no warning is the worst of the three. `DESIGN.md`'s "Per-trigger cognitive fit" documents the intent the code drops, so the design is on record and only the wiring is missing. ## Reproduction 1. Stock install, no review provider keys set, so `review.reviewers` is the default `["claude-subagent"]`. 2. Confirm the config value the default template ships: ``` node cadence-core/bin/config.mjs get review.triggers.diff.effort ``` Expect `medium`. 3. Complete a phase plan so the `diff` trigger fires. 4. The dispatched `cad-reviewer` runs at `high`, from its own frontmatter. Nothing reports the discrepancy. Static confirmation, no run required: ``` grep -n 'effort' cadence-core/references/review-triggers.md ``` Every hit is inside the cross-model provider arm. ``` sed -n '1,8p' agents/cad-reviewer.md ``` `effort: high`, pinned. ## Environment - Claude Code version: 2.1.220 - `node --version`: v26.4.0 - OS: Linux (CachyOS, 7.1.5-1-cachyos) - Cadence: v1.4.0 at `fba5879` - Relevant config: `review.reviewers: ["claude-subagent"]` (default), `review.triggers.diff.effort: medium` (template default) ## Notes The fix folds into the reviewer row of the effort-as-rigor-lever proposal, which makes reviewer effort per-trigger by construction. Filed separately so the defect stays on the record if that proposal stalls or is rejected. Cross-link both ways. Worth deciding as part of the fix: whether a resolved-but-undeliverable `effort` should warn rather than pass silently. Cadence already treats a silently-degraded path as a bug class of its own, and this is one.

crenshawdev commented

2026-07-28 17:41:49 +00:00

(Migrated from github.com)

The fix folds into the cad-reviewer row of #63, which makes reviewer effort per-trigger by construction. Filed separately so the defect stays on the record if #63 is rejected.

The fix folds into the `cad-reviewer` row of #63, which makes reviewer effort per-trigger by construction. Filed separately so the defect stays on the record if #63 is rejected.

crenshawdev commented

2026-07-28 17:56:37 +00:00

(Migrated from github.com)

Scheduled into v2.0.0 rather than a patch, because #63 rewrites the exact dispatch path this fix lands on and doing it twice is waste.

Standing contingency, written down so it does not have to be reconstructed: if v2.0.0 slips or #63 is rejected, pull this forward into its own patch immediately. It is a live defect on the shipped default path, and that is why it was filed separately from #63 in the first place.

Scheduled into v2.0.0 rather than a patch, because #63 rewrites the exact dispatch path this fix lands on and doing it twice is waste. Standing contingency, written down so it does not have to be reconstructed: if v2.0.0 slips or #63 is rejected, pull this forward into its own patch immediately. It is a live defect on the shipped default path, and that is why it was filed separately from #63 in the first place.