feat: reviewer class - adversarial | advisory | none, the review-cost dial (2.5.54)

[apackeer]

feat: reviewer class - adversarial | advisory | none, the review-cost dial (2.5.41)

Why

Customer feedback: requirements analysis + user stories took 3 hours in a workshop, with "the reviewer agent running multiple iterations" visibly dominating the wait (30+ minutes per stage after the questions were answered). Live A/B runs against the real harness (TUI-driven, shipped settings, audit-ledger timings) reproduced and quantified it:

configuration	stage wall-clock (requirements-analysis, same seed)
v2 HEAD (adversarial 2-pass loop)	12m34s - 20m38s (verdict-dependent)
advisory single-pass	8m49s - 14m31s
no reviewer	5m53s

The adversarial refute-fix-re-review loop spends 2.4x the reviewless stage baseline on review choreography, and its first pass is NOT-READY by design (the persona hunts until it fails to refute), so the two-pass cycle is the common case even on clean artifacts. On human-gated prose stages that cost buys little: the human at the gate is already the authority, and re-review of a fixed artifact kept surfacing NEW findings (3/3 live re-reviews stayed NOT-READY) - prose has unbounded findable issues, so the loop does not converge; it exhausts its budget and presents findings to the human anyway.

The reviewer itself earns its keep: 5/5 independent review passes (adversarial AND neutral framing) caught a genuine Critical a gate-fatigued human would plausibly approve past. The fix is not removing review - it is right-sizing the loop per stage.

What

Review class - a fourth instance of the existing knob pattern (judgment tiers, depth, test strategy):

• review_class: adversarial | advisory stage frontmatter (schema-validated, requires reviewer, compile defaults adversarial). The 7 human-gated ideation/inception prose stages ship advisory; the 5 Construction design/build stages stay adversarial.
• Advisory contract: ONE review pass, both verdicts terminal, findings ranked by severity and quoted verbatim at the approval gate as decision support. No lead re-invoke, no re-review. Evidence-grounding still applies.
• Scope cap: review_cap: adversarial | advisory | none scope frontmatter; shipped bugfix, poc, and workshop (the default first-run scope) cap to advisory.
• Per-run override: /aidlc --review <class> -> config-change --review -> Review Override state field + REVIEW_CLASS_CHANGED audit event (77-event taxonomy). Low-wins resolution (resolveReviewClass in aidlc-lib.ts): stage declaration, lowered by scope cap, lowered by override - nothing can raise a class or conjure an undeclared reviewer.
• Engine-enforced iteration ceiling: aidlc-log.ts review refuses a REVIEW_REQUESTED whose --iteration exceeds the effective budget (advisory 1, adversarial reviewer_max_iterations, none 0). The refusal text teaches the terminal path instead of re-triggering a loop. REVIEW_COMPLETED receipts are never refused; resolution failures fail open.
• Swarm exemption: autonomous Bolt reviews use the declared class - inside a Bolt the reviewer is the only pre-merge verification, so caps/overrides do not apply there.
• Reviewer tier effort: the balanced tier (product-lead + architecture-reviewer, the only balanced agents) pins medium effort on Claude/Codex/opencode. Live A/B: medium review passes at ~half the xhigh wall-clock (2m00s-2m39s vs 4m42s-5m53s) with no verdict/finding-quality loss.

Evidence

11 headless reviewer runs (persona A/B + convergence) + 7 live TUI-driven end-to-end runs of the real v2 harness across 6 configurations, same seed, audit-ledger timings. Full data, per-run sandboxes, and REPORT.md retained locally; summary available on request.

Surfaces

• Engine/tools: aidlc-stage-schema, aidlc-graph (compile + FIELD_ORDER emitter), aidlc-lib (resolution + scope metadata + StageEntry), aidlc-orchestrate (directive emission run-stage + invoke-swarm, --review routing), aidlc-directive (types + field rosters), aidlc-log (ceiling), aidlc-utility (config-change/get/list, help, state template), aidlc-audit (event), aidlc-tiers (balanced effort).
• Prose: stage-protocol §12a class branch, all 5 harness SKILL.md reviewer steps, both reviewer personas (Advisory Dispatch section), 7 stage files, 3 scope files, onboarding fills (5 harnesses).
• Docs: guide 06/10/12 + glossary + introduction, reference 04/12/15 + event-count sweep (76 -> 77 across 12 files).
• Tests: NEW t265 (schema/graph/resolution/prose, 18 cases) + t266 (iteration ceiling via the real CLI, 8 cases incl. at-budget/over-budget/verdict-exempt/unit-exempt/fail-open). Pins bumped: t28, t81, t216, t220, t231, t239, coverage registry, t66 designer-export golden (verified: diff is exactly the 12 review_class rows).

Test evidence

Full 7-slice pre-merge gate ran (slice 8, Kiro IDE, is an unmet gate on the Linux CI box - needs macOS + Kiro.app). No red implicates this branch:

• smoke+unit: 189 files / 0 failed (final rebased tree, solo rerun after a 7-slice-contention flake of t17 passed alone)
• integration: 103/103 on the pre-rebase tree; in-gate contention reds (t163/t188/t49) all PASS alone; t193 is the known composer-variance watch item, PASS alone
• deterministic e2e: 29/31; t113 + t122 are the documented v2 base reds
• Claude TUI live (19 files): 18/19 - including PASS on t74 (requirements-analysis journey) and t139 (revision-loop idempotency), the two files most directly exercising the changed §12a prose. The one red (t73) reproduces byte-identically on clean v2 with zero branch changes (same assertion, same audit offset 927): it is fix: enforce pre-generation summary confirmation across harnesses #703's pre-generation summary-confirmation ordering contract failing on its own base (the [Bug]: Kiro conductor skips the mandatory Consolidated Summary Confirmation before the gate - both Kiro TUI journeys are standing reds #699 family), not this branch.
• Kiro ACP live: compose-inflight PASS alone; journey-workspace is the known codekb-validation base red
• Kiro TUI live: both reds are the documented [Bug]: Kiro conductor skips the mandatory Consolidated Summary Confirmation before the gate - both Kiro TUI journeys are standing reds #699 base signature
• Codex exec live: 5/5 PASS

Versioning

2.5.41 + README badge + CHANGELOG entry. Originally slotted 2.5.40; re-bumped after #673 merged as 2.5.40 mid-gate (branch is rebased onto that HEAD, 9608d72, absorbing #702 + #703 with union-merged §12a + schema rosters). t265/t266 free (t263/t264 in #702, t267 renamed by #673, #705 pending re-bump).

[leandrodamascena]

Deep review @ ad9bee7f

The review-class model is directionally sound, but five behavioral blockers remain. I reproduced the first four against the real packaged CLIs.

P1: --review none makes reviewer-bearing stages impossible to complete

buildRunStageDirective correctly omits the reviewer when the effective class is none (aidlc-orchestrate.ts:1589-1607). Completion still checks the stage's static reviewer declaration and unconditionally requires a fresh REVIEW_COMPLETED (aidlc-state.ts:1272-1291, called by all four completion paths).

Reproduction: a requirements-analysis directive under Review Override: none contains no reviewer, but finalize requirements-analysis refuses because no reviewer receipt exists. There is no valid route to produce that receipt because the reviewer was intentionally disabled.

The completion precondition must use the effective class while preserving the autonomous-swarm exemption.

P1: the iteration ceiling trusts the caller's label instead of enforcing a count

aidlc-log.ts:539-574 checks only whether the supplied --iteration exceeds the budget. It never counts prior REVIEW_REQUESTED rows.

These requests can therefore continue indefinitely:

--iteration 1
--iteration 1
--iteration 1

Both repeated iteration-1 requests succeeded in the packaged CLI. Omitting --iteration, or supplying zero, negative, fractional, or nonnumeric values, also bypasses enforcement; t266 explicitly preserves the no-iteration exemption.

This does not close the stated failure mode of a conductor losing count. The tool should own the sequence/count, require a valid positive integer, and reject duplicates or out-of-order requests.

P1: rebase, re-bump, and reconcile the event taxonomy

Current v2 already owns 2.5.41 through #677. GitHub reports this PR as CONFLICTING/DIRTY.

Both branches independently add the 77th event. Their combination must become 78 events, retain the current 20 categories, resolve the taxonomy/count-test conflicts, select a new version, regenerate distributions, and rerun checks.

P2: --unit is treated as proof of an autonomous swarm

The ceiling ignores scope caps and run overrides whenever flags.unit is present (aidlc-log.ts:533-558). However, ordinary gated inline Construction stages also review per unit (aidlc-orchestrate.ts:3008-3022).

I reproduced iteration 2 being accepted for inline functional-design --unit unit-alpha under a configuration whose effective class is advisory/none. An arbitrary fake --unit also restores the declared budget on a non-unit stage.

The exemption must be based on actual autonomous Bolt/swarm execution, not user-controlled --unit presence. The current t266 case codifies the over-broad behavior.

P2: scope and birth routes silently discard --review

During an in-flight scope change, the emitted command forwards --depth and --test-strategy, but not --review (aidlc-orchestrate.ts:2367-2387).

Reproduction:

next --scope feature --review none

emits only:

scope-change --scope feature

When the supplied scope is already active, neither mutation branch applies the review override. Fresh-workflow birth similarly forwards depth and test strategy but omits review (aidlc-orchestrate.ts:494-510).

Please thread --review through scope-change and intent-birth, or reject unsupported combinations rather than silently ignoring user input.

P3: documentation counts are stale

• CHANGELOG.md:11 says "75-event taxonomy," while this branch declares 77.
• audit-format.md:65 says "Navigation Events (6 events)" but lists seven.
• After rebasing #677, the combined registry should be 78 events and 20 categories.

Verification

• New focused suites: 18/18 passed.
• Independent CLI probes reproduced the four behavioral findings above.
• git diff --check: passed.
• Current remote CI is green, but it ran on the pre-rebase conflicting head.

Verdict: CHANGES_REQUESTED.

[apackeer]

Addressed the requested changes and force-pushed the rebased branch at abe3c738.

Resolved:

• Rebased onto current v2; rebumped to 2.5.43, reconciled the 78-event taxonomy, and moved the new suites to non-colliding t266 / t271 IDs.
• Effective --review none now removes both dispatch and the completion receipt requirement, while real autonomous Bolt stages retain their declared reviewer requirement.
• aidlc-log review now requires a positive, audit-derived sequential iteration under the active intent lock; reviewer identity must match the graph; REVIEW_COMPLETED must pair with an unmatched request.
• The autonomous exemption now requires the actual swarm topology plus a live matching Bolt attempt; BOLT_FAILED / BOLT_COMPLETED close it, and inline per-unit stages retain scope/run caps.
• Birth, scope-change, same-scope, parked-workflow, dispatcher config set review, and config read/list routes now preserve the override. Unsupported compound modes reject --review instead of silently dropping it.
• Added runtime directive enum/coupling validation, audit provenance, documentation/count fixes, and expanded completion/routing/locking regressions.

Verification:

• bun run check passed: package parity, all TypeScript projects, and Biome.
• Coverage registry freshness + ratchet passed.
• Post-rebase focused unit slice: 7 files, 136 assertions, 0 failed.
• Affected integration slice: 8 files, 104 assertions, 0 failed.
• Additional reviewer-precondition/backstop/unit-pruning reds passed alone after correction.

[apackeer]

Fixed at 8aedd100.

Root cause: t236-ensemble-evidence-gate synthesized REVIEW_COMPLETED rows without the paired REVIEW_REQUESTED + positive Iteration now required by freshReviewReceipts, so six approval-path assertions were rejected as stale/missing reviews.

Changes:

• Updated the fixture helper to emit a protocol-valid request/completion pair, preserving optional per-unit identity.
• Rebased onto current v2; resolved the release collision by moving this PR to 2.5.44 and regenerating all harness distributions.

Verification:

• Local smoke + unit: 197 files, 4,849 assertions, 0 failures.
• Package parity, all TypeScript projects, and repository lint pass.
• GitHub CI run 31074015620 is green; documentation build also passes.

[leandrodamascena]

The previous findings are substantially addressed, but one authority bypass and three recovery/edge-path regressions still block approval.

[P1] Reserve review receipts from the public audit CLI
CLI_RESERVED_EVENT_TYPES does not include REVIEW_REQUESTED or REVIEW_COMPLETED. I reproduced aidlc-audit append REVIEW_REQUESTED ... --iteration 99 succeeding and writing the row, bypassing reviewer identity, budget, sequencing, and locking enforced by aidlc-log. A forged pending request can then be paired with a completion without a legitimate reviewer dispatch.

[P1] Allow recovery when reviewer dispatch fails
REVIEW_REQUESTED is recorded before dispatch. If the reviewer invocation fails or the session terminates, iteration 1 remains consumed. Retrying iteration 1 is rejected as a duplicate, while iteration 2 exceeds the advisory budget, leaving no documented path to obtain the required terminal receipt.

[P2] Preserve --review during flat-layout migration
The migration branch in aidlc-utility.ts returns before applying the parsed review override. intent-birth --review none can therefore succeed during migration while silently leaving the migrated workflow review behavior unchanged.

[P2] Honor the declared advisory class inside autonomous swarms
The swarm directive preserves review_class: advisory, but the review logger uses reviewer_max_iterations ?? 2 for autonomous attempts. A plugin-defined advisory swarm stage therefore receives two passes, violating the advisory single-pass contract. The exemption should ignore scope/run caps while retaining the stage declaration.

The five earlier findings are otherwise corrected. Focused baseline validation passed: 111 tests and 514 assertions. Remote CI and git diff --check are green; the added public-CLI authority probe failed as described.

[apackeer]

Fixed at b804d4d0.

Resolved:

• Reserved REVIEW_REQUESTED and REVIEW_COMPLETED from public aidlc-audit append / append-batch, blocked reserved event smuggling through append-raw and caller-supplied Event fields, while retaining trusted library emitters.
• Added documented aidlc-log review --retry-pending recovery. It is accepted only for the same unmatched request, writes an audit-marked retry row, and does not consume another logical iteration.
• Applied and audited --review before the flat-layout migration branch returns, including legacy state files without a Test Strategy field.
• Kept autonomous swarms exempt from scope/run caps while enforcing the declared advisory single-pass limit in both the review logger and emitted invoke-swarm directive.

Verification:

• bun run check passes package parity for all harnesses, all TypeScript projects, and Biome.
• Focused unit slice: t115, t201, t271 — 75 tests, 0 failures.
• Migration integration slice: t165 — 32 tests, 0 failures.
• Broad smoke+unit run completed all 198 files: 196 passed immediately; it exposed two park/unpark regressions from over-reserving Timestamp. That validation was narrowed to the actual Event smuggling vector, then t114 passed 38/38 and t17 passed 83/83 alone.
• git diff --check and the pre-push sensitive-content scan are clean.

GitHub has not registered new check runs for this head yet.

[leandrodamascena]

The latest corrective commit resolves the previously blocking review-authority, retry-recovery, migration, and autonomous-swarm issues. Focused unit/integration suites, package parity, typecheck, lint, and audit count/version checks pass.

Approving with non-blocking follow-ups:

• Reject a bare --review flag instead of silently continuing under the prior review policy.
• Add review_class to the authoritative stage-definition contract and review_cap to the scope-authoring guides.
• Update t111’s stale 77-event commentary and mirrored list to include REVIEW_CLASS_CHANGED.

The branch still needs rebasing onto current v2 2.5.53, resolving the version/changelog conflicts, regenerating distributions, and rerunning CI before merge.