quality-locked

Quality-Locked Loop

When the workflow has quality_locked_mode: true (set by --quality-locked flag, see flag-parser/SKILL.md), every review checkpoint runs as a deterministic loop instead of a single review-then-user-decides pass.

Decision logic is in code, not prose. The agent calls a Python checker that parses review output, applies the clean bar, and returns the next action. This eliminates "I think this is clean enough" miscounts and silent iteration drift.

Activation Points

The loop runs at these checkpoints (the same ones where auto-review normally fires):

Per-Iteration Loop (what the agent does)

For each iteration (1..10):

1. Run the review sub-agent (Task tool, fresh context).
   Capture the raw text output.

2. Call the quality-locked checker:

   python3 sage/runtime/tools/sage_flags.py check \
     --review-output "<sub-agent output text>" \
     --iteration <current iteration number> \
     --history-json '<JSON array of prior iteration records>'

   Returns JSON with: counts, is_clean, cap_reached, stuck, action,
   iteration_record.

3. Append iteration_record to manifest.md under quality_locked_history
   (the agent writes; the checker provides the structured record).

4. Dispatch on `action`:
   - PASS:         exit loop, continue the workflow
   - REVISE:       apply fixes to the artifact/code, increment, loop
   - CAP_REACHED:  present F/R/E/A prompt to the user
   - ESCALATE:     present escalation prompt (3 iterations no improvement)

The agent never decides "is this clean enough" — the checker does. The agent only runs the sub-agent and applies the revisions.

Fallback Chain

The check command is the primary path. If Python is unavailable:

1. Python primary: python3 sage/runtime/tools/sage_flags.py check ...
2. Prose fallback: read this SKILL.md's decision rules below and reason manually. Announce the fallback to the user so the reliability degradation is visible.

(Unlike flag-parser, there is no Bash fallback layer here. The parsing and state logic are non-trivial enough that a Bash implementation would be its own reliability risk.)

JSON Contract

The checker emits this shape:

{
  "counts": {
    "critical": 0,
    "major": 0,
    "substantive": 0,
    "cosmetic": 1
  },
  "is_clean": true,
  "cap_reached": false,
  "stuck": false,
  "action": "PASS",
  "iteration_record": {
    "iteration": 3,
    "counts": { ... },
    "result": "PASS"
  }
}

action is one of: PASS, REVISE, CAP_REACHED, ESCALATE.

Clean Bar Definition (used by the checker)

A review is "clean" when ALL of:

• critical == 0
• major == 0
• substantive == 0

cosmetic count is ignored for the clean bar. Cosmetic findings never trigger another iteration.

The classifier maps both review formats to this unified schema:

• auto-review/auto-qa: CRITICAL/MAJOR/MINOR-substantive/MINOR-cosmetic
• quality-review: CRITICAL/WARNING/SUGGESTION-substantive/SUGGESTION-cosmetic
• Mapped: WARNING → major, SUGGESTION-substantive → substantive, etc.

Action Behavior

PASS

• Exit the loop
• Append the final iteration record to manifest
• Continue the workflow to the next step
• Briefly announce: "Auto-review PASS (n iterations)."

REVISE

• Announce: "Auto-revising (iteration N/10, --quality-locked active)..."
• Read the findings list from the sub-agent output
• Apply fixes for ALL Critical, Major, and substantive Minor findings
• Stay within scope (writable files only)
• Re-loop with iteration + 1

CAP_REACHED

Sage: --quality-locked cap reached (10 iterations).
  Remaining at {checkpoint_name}:
  - Critical: {n}
  - Major: {n}
  - Minor (substantive): {n}

The same findings keep returning. This suggests:
- The artifact has a structural issue that revision can't fix
- Consider escalating to /architect for a design rethink
- Or accept the findings and proceed manually

[F] Force-proceed — accept the remaining findings, continue
[R] Revise manually — drop --quality-locked, let me edit
[E] Escalate — type /architect to rethink the design
[A] Abort — cancel this workflow

Pick F/R/E/A, or describe what to do.

Interaction with --autonomous

When --autonomous is also active, the [A] Review checkpoint that triggers this loop is auto-picked by the agent. The cap-reached and stuck-escalation prompts below still require user input — see sage/core/capabilities/orchestration/autonomous/SKILL.md section "Auto-Pick at Checkpoints" for the full rules.

ESCALATE (stuck — 3 iterations with same critical+major count)

Sage: 3 iterations with no improvement in findings count.
  Iteration {n-2}: {c} critical, {m} major
  Iteration {n-1}: {c} critical, {m} major
  Iteration {n}:   {c} critical, {m} major

This suggests architectural-level issues that spec revision can't fix.

[E] Escalate to /architect (recommended)
[C] Continue iterating (up to cap of 10)
[R] Revise manually — drop --quality-locked

Pick E/C/R, or describe what to do.

Always log the chosen action to manifest with the user's selection.

Manifest Update Format

After each iteration, agent appends to manifest.md:

quality_locked_history:
  - checkpoint: spec
    iteration: 1
    counts: { critical: 2, major: 1, substantive: 0, cosmetic: 1 }
    result: REVISE
  - checkpoint: spec
    iteration: 2
    counts: { critical: 0, major: 0, substantive: 0, cosmetic: 1 }
    result: PASS

Pass the existing array (or [] for first iteration) as --history-json so the checker can detect "stuck" patterns.

Failure Modes

Rules

• Iteration cap is non-negotiable at 10.
• MINOR-cosmetic findings NEVER trigger another iteration.
• Every iteration logs to manifest BEFORE the next sub-agent call.
• Stuck detection requires ≥3 prior iterations and matching counts >0.
• The agent does not interpret findings — the classifier does. The agent's job is to RUN the sub-agent and APPLY the fixes.

Review Loop v2 (ledger mode)

The ledger loop is the DEFAULT: it REPLACES the v1 loop above unless .sage/config.yaml carries review_loop: with mode: v1 (the explicit pin sage update writes into pre-flip projects). Everything else in this skill (activation points, fallback announcements, autonomous interaction) still applies on both paths.

What changes and why: the reviewer loses the verdict — findings are structured evidence, the decision is computed by sage/runtime/tools/review.py + sage_flags.py from ledger facts. A finding that cites nothing (or whose citation resolves against no spec/plan/constitution source — intake checks) and demonstrates nothing is capped at substantive on intake and never blocks. Findings, once recorded, cannot be forgotten or silently re-raised — including disputed ones: a Phase-A-disputed entry (cannot-reproduce, or DISPUTED-STANDS) never drives another round, but it must receive a disposition before any STOP records (review_loop.disputed_disposition: false restores the old vanish).

Per iteration (1..cap, default cap 5 — review_loop.iteration_cap):

0. FIRST round of each checkpoint only: open the instance —
     python3 sage/runtime/tools/review.py open-instance \
       .sage/work/<slug>/review-ledger.json --checkpoint <spec|plan|code|qa>
   One ledger carries every checkpoint's loop; cap, stall, and the
   round number count WITHIN the instance (field ledgers hit exit
   records at iter=13 against a cap of 5 before this existed).

1. Assemble the input packet (template in the review capability's
   "Input packet (v2)" section) and dispatch the review sub-agent.

2. Rounds >1 — Phase A first: for each open/not-fixed ledger entry the
   sub-agent returns FIXED | NOT-FIXED | DISPUTED-STANDS with evidence.
   Record it:
     python3 sage/runtime/tools/review.py verify \
       .sage/work/<slug>/review-ledger.json \
       --iteration N --results-json '<Phase A JSON>'

3. Phase B findings (the JSON array from the sub-agent output's fenced
   block — prose outside it is not parsed) go through intake:
     python3 sage/runtime/tools/review.py intake \
       .sage/work/<slug>/review-ledger.json \
       --iteration N --artifact <spec|plan|code|adr> \
       --findings-file <saved findings JSON>
   intake computes fingerprints, caps severities, dedups, and guards
   re-litigation. Its normalization is not yours to re-litigate.

4. Close the round — the verdict is computed, never reported:
     python3 sage/runtime/tools/review.py close-round \
       .sage/work/<slug>/review-ledger.json --iteration N

5. Dispatch on `action`:
   - CONTINUE:      run the fix round (fixer protocol in the review
                    capability), increment, loop.
   - STOP_CLEAN:    exit loop, continue the workflow.
   - STOP_ADVISORY / STOP_CAP: close-round REFUSES to record until every
                    open AND every Phase-A-disputed entry has a
                    disposition. Present the remaining entries
                    (`review.py report`) with the disposition menu
                    below, record the choices — `review.py disposition`
                    per entry, or ONE `review.py disposition-batch` for
                    several sharing a decision — then close-round again.
   - ESCALATE:      render `review.py report`, present the escalation
                    prompt. The controller does not spend past a stall.

Disposition menu (per remaining open or disputed entry, RR-7):

[F] Fix now — one more fix round for this finding (converts the stop
    back into CONTINUE for it). BLOCKING or DISPUTED entries only —
    the tool refuses it on a substantive/cosmetic (a non-blocking
    finding buying rounds was the field's advisory-stop-never-stops
    pathology; human override: review_loop.fix_now_blocking_only: false)
[D] Defer — ticketed; requires a ticket/issue ref — `cleanup.md` is the
    canonical cycle ticket (close-round writes the file itself); witness
    test stays red-marked in the suite
[X] Reject — requires a reason, recorded; re-raising it later needs
    the anchor to have actually changed
[B] Batch — one decision across several entries:
    `review.py disposition-batch <ledger> [F-ids...] [--severity
    substantive|cosmetic] --action defer --ticket <one cleanup ticket>`
    (or `--action reject --reason ...`). Every entry still gets its own
    ledger record; only the round-trips collapse.

Rounds >1 non-blocking discoveries never reach this menu: intake auto-defers them to cleanup.md on arrival (review_loop.late_finding_disposition: open restores the old flow) — the fix→new-nit→fix treadmill ends at the tool. Round-1 substantives/cosmetics DO reach the menu; settle them in ONE command: review.py disposition-batch <ledger> --severity substantive --action defer --ticket cleanup.md (same for cosmetic, or --action reject --reason ...). close-round writes cleanup.md itself at the sealing stop; deferred findings are fixed AFTER the loop exits, as ordinary work with ordinary commits.

Fix round (v2) — witness-first, one finding one commit

On CONTINUE, fix the BLOCKING findings — criticals, and majors beyond major_budget — in severity order; the witness-first ceremony below is theirs. Substantive and cosmetic entries are NOT fixed inside the loop by default: they stay open and are settled at STOP with dispositions (the [B] batch path collapses N of them into one command — defers share one cleanup ticket). Fixing a substantive anyway is allowed when it shares an anchor or commit with a blocking fix (cluster rule, step 3); ceremony must scale with severity, not with count. Per blocking finding:

1. Materialize the witness before touching code. witness.kind: test — run it, confirm red at HEAD. repro/trace — write the test that encodes it at tests/review/<F-id>.*, run it red, then review.py attach-witness <F-id> --ref <path>. A trace-kind finding from an empty matrix cell — the witness IS the missing test: write it (red or green as the code warrants; an empty test cell over green code is still a fix). If the witness cannot be reproduced at HEAD: review.py verify <ledger> --iteration N --cannot-reproduce <F-id> --evidence "<run output>" — bounced to the controller as a Phase-A dispute that must be dispositioned before any STOP records, never silently skipped and never a way out: a disputed finding does not vanish from the verdict. (The tdd-gate already blocks a source edit without a test in scope; witness-first is that rule's loop-shaped application.)

2. Collateral safety (advisory): when the packet's blast radius shows a neighbor of the fix with no covering test, pin current behavior with 1–3 asserts first (tests/review/<F-id>-sentinel.*). You cannot avoid breaking what nothing observes; sentinels are the cheapest observer.

3. Fix, then commit — ONE commit per finding (cluster findings only when one change genuinely closes them together — then Sage-Fix names ALL of them, e.g. Sage-Fix: F-003 F-007, and check-diff widens scope to the union of their anchors ∪ witnesses), with trailers:

   Sage-Fix: F-003
   Sage-Cause: <why the defect existed — one line>
   Sage-Change: <what the fix does — one line>
   Sage-Risk: <what could regress — one line>
   Sage-Collateral: src/session.ts:88-95 (why)   # if any
   Sage-License: spec §4.2                        # if behavior changes
   

   Cause/Change/Risk is the three-line fix plan — the misunderstood-finding tripwire — recorded where bisect finds it. Because commits map 1:1 to findings, a regression later bisects to a single Sage-Fix trailer.

4. Scope check (controller step, per fix commit): review.py check-diff <ledger> --finding <F-id> --commit <sha>. Out-of-scope hunks exit 1 and land in the ledger as a machine finding witnessed by the hunk itself. A modified non-witness test without Sage-License also exits 1 — correctness is amended through the spec with approval, never redefined in the diff; if the spec is silent, raise a spec finding and pause the code fix behind its disposition. review_loop.scope_check: false restores v1.

5. Per commit: run the finding's witness + targeted tests for touched files. Per round close: the closing proof runs THROUGH the bounded gate script — bash .sage/gates/scripts/sage-verify.sh --quiet plus the other deterministic gates on the fixed HEAD, NEVER the raw runner (a raw make test at round close is exactly the unbounded hang the watchdog exists to catch; the script's summary line is the evidence) — and record it: review.py close-round ... --suite-evidence "<the script's summary line>" --gates-evidence "<exits>", so the next Phase A verifies against facts already on file. Rounds whose fixes touched ONLY spec/plan/adr artifacts run document checks instead of the code suite — record --suite-evidence "doc-only round: <files>" (free text; a code suite proves nothing about a prose edit, and the field ran four full suites for one spec review).

Witness tests are permanent: they land with the fix, run in the suite thereafter, and are never deleted on STOP — a deferred finding's witness stays red-marked (xfail/todo per runner idiom) as the ticket's executable form.

The exit record in decisions.md is written by review.py close-round itself — do not write it by hand. If any review.py command exits 1, STOP and surface the error verbatim: the ledger fails closed, and a broken ledger stops the loop loudly. Do not reconstruct ledger state by hand or continue the loop around it.

The v1 sections above ("cap of 10", quality_locked_history in the manifest, sage_flags.py check) do not apply on the v2 path — history lives in the ledger's history[], written by close-round.

Model routing (review_loop.review_model: cheap, optional): the checklist perspective passes and Phase A verification MAY run on a cheap model, reserving the default model for one adjudication pass over the assembled findings — checklist-shaped work transfers down-model; open judgment does not. The cost delta is UNCLAIMED until measured: the knob exists, the number waits.

Quality Criteria

• Checker is deterministic (same input → same output, verified by tests)
• Both review formats produce identical unified counts
• Stuck detection requires findings >0 (three clean reviews aren't "stuck")
• Cap-reached prompt offers actionable choices
• Manifest history is complete and machine-readable
• Fallback announcement is mandatory when degrading to prose