Skip to content

fix(sdlc): the review now holds a change to the contract's acceptance list - #3465

Merged
Wirasm merged 2 commits into
devfrom
fix/3449-review-acceptance-coverage
Sep 24, 2026
Merged

Wirasm merged 2 commits into
devfrom
fix/3449-review-acceptance-coverage

Conversation

@Wirasm

@Wirasm Wirasm commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Problem and outcome

The delivery pack's review could call a change ready when it missed the accepted contract's acceptance list. The lenses look for defects in what changed, and no step asked whether what changed delivers what was required. On the #2325 delivery (PR #3446), the review printed the contract, fixed four diff-level findings, and declared ready while a required real-process test, the operator-visible recorded facts, and part of the solution steering were missing.

  • Outcome: every review round judges each acceptance item, stated invariant, and steering constraint against the whole change. Each is met with cited evidence, unmet as an Important (or Critical) finding with sources: [contract], or deferred to a named later step such as CI. An unmet item blocks ready like any Important finding. The report gains a "Contract coverage" table.
  • Invariant: the agent judges whether evidence meets an item; nothing matches contract text with regexes or keywords. Findings keep the existing findings.json shape; there is no second verdict channel.
  • Scope boundary: no new node, no YAML field, no change to the lenses or the verdict schema. A contract that states no acceptance, invariants, or steering reviews as before.
  • Root cause: two gaps. The contract lost its acceptance list on the way to review: triage wrote "five criteria" without the items, and review scope copied that summary and the plan's "out of scope" list (which dropped required behavior) as the accepted contract. Then synthesis had no instruction to check coverage at all.

Review guidance

  • Feedback requested: the exact prompt wording (listed below), and whether contract as a new sources value is the right attribution.
  • Start here: .archon/workflows/sdlc/review/commands/review-synthesize.md, section "Judge contract coverage".
  • Review order: review-synthesize.md, review-scope.md ("Resolve the accepted contract"), triage.md (one sentence in "Contract"), archon-review.yaml description, the test.
  • Lower-attention areas: bundled-defaults.generated.ts is regenerated by bun run generate:bundled; check:bundled passes.
  • Known risk or uncertainty: judgment varies by sample. See Validation for the rates measured.

Solution

Synthesis owns the check because it is the only node that runs on every round. Continuation rounds skip the lenses, and both rounds of the #2325 review missed the gaps, so a lens extension would not cover round 2.

Wording changes:

  • review-scope.md, "Resolve the accepted contract": resolve the contract in every mode, including light mode, from the originating source (issue, request, document) the work order names, not from triage summaries, plans, or a prior report. List each acceptance, invariant, and steering item quoted. A document that only counts or paraphrases acceptance has not given the items. A derived plan that declares a required item out of scope is recorded as a conflict, not an accepted non-goal. The scope file gets Acceptance, Invariants, and Steering subsections; each says "none" or "could not be read" when that applies. Light mode must not copy the prior report's contract.
  • review-synthesize.md, new "Judge contract coverage" section: the met / unmet / deferred rules above. When an item names its proof (a real process, a test per surface, output the operator sees), only that proof counts; code reading, a mocked test, or a log line does not. Continuation rounds carry coverage rows forward and re-judge unmet or touched items; when the prior report has no coverage, every item is judged now as a missed earlier finding. Items scope could not read force ready: false. Also: an extra sentence in the verdict, a new report section 4, contract-coverage findings carry sources: [contract], and the verify step checks one coverage row per item.
  • triage.md, "Contract" section: for a READY verdict, quote the source's invariants, acceptance items, and steering.
  • archon-review.yaml: the description says every round holds the change to the accepted contract.

Validation

  • bun run validate passed on the final head (includes workflow-fixtures, 55 passed).
  • bun run check:bundled: up to date. bundled-defaults.test.ts: 35 pass, including a new test that pins the triage, scope, and synthesis instructions.
  • Replay of the fix(core): abandon stops a reachable owner before releasing the run, and the in-use refusal names working commands #2325 case. I ran the review-scope and review-synthesize prompts with claude -p --model sonnet against a checkout at 45e0f8080, using the saved run's artifacts (triage.md, plan.md, implementation.md, round-1 report and lens files) as round 2 started. The review target was pinned to that commit. The script and results are in my scratchpad, not the repository.
    • Old prompts: ready, 2 of 2 samples. This reproduces the bug.
    • New review prompts with the saved triage.md (10 samples across four prompt revisions): not ready in 5. In the three ready samples I inspected, scope had taken the contract from triage's summary, so acceptance never reached synthesis. That is why triage changed too.
    • End to end, re-running the new triage prompt at 7258169c5 first: triage quoted all five acceptance lines in 3 of 3 samples. The review was not ready in 3 of 3. All 3 raised the missing real-process test as a contract finding. One raised "recorded facts printed" as unmet. Another raised the related Invariant 5 line (host not stated plainly) but judged the logger.info diagnostic as "printed". One sample also caught that the refusal never offers abandon.
    • Positive control: a small synthetic change (slugify with maxLength) with 3 acceptance items, 1 invariant and 1 steering item, all evidenced by tests, and clean lens reports. It converged to ready in 8 of 8 samples across the revisions, 2 of 2 on the final prompts, with every item met and cited.
  • Not verified: a full archon-ship run through the engine with DeepSeek (the model the original run used). The replay calls the prompts directly with Sonnet. The "recorded facts printed" finding is inconsistent (1 of 3 end to end), because the model sometimes accepts a structured log line as operator output.

Links

Summary by CodeRabbit

  • Workflow Improvements
    • Reviews now assess every change against the accepted contract, including acceptance criteria, invariants, and steering constraints, and require cited evidence for each.
    • Review reports now show whether each contract item is met, unmet, or deferred; unmet items are recorded as blocking findings.
    • Triage assessments preserve the source requirements verbatim for use in later planning, implementation, and review.
    • Light-mode reviews now require a prior report and identify its path when it is missing.

The delivery review looked only for defects in what changed. Nothing asked
whether the change delivered the accepted contract, so a PR could converge to
ready while missing acceptance items, an invariant, or the solution steering.

review-scope now resolves the contract from its originating source rather
than a derived summary, plan, or prior report, and records each acceptance,
invariant, and steering item. A plan that drops an originating requirement is
recorded as a conflict, not an accepted non-goal. review-synthesize judges
every item against the whole change on every round (met with cited evidence,
unmet as a blocking finding with sources [contract], or deferred to a named
later step) and writes a contract coverage table into the report.
… check it

A replay of the saved #2325 run showed the acceptance list was lost at triage: triage.md recorded "five criteria" without the items, and review scope resolved the contract from that summary in about half the samples. For a READY verdict, triage now quotes the source's invariants, acceptance items, and steering in its Contract section.
@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 04507782-2780-439a-bebb-30bb1bb5f9d6

📥 Commits

Reviewing files that changed from the base of the PR and between eb9635c and f497b3e.

⛔ Files ignored due to path filters (1)
  • packages/workflows/src/defaults/bundled-defaults.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (5)
  • .archon/workflows/sdlc/review/archon-review.yaml
  • .archon/workflows/sdlc/review/commands/review-scope.md
  • .archon/workflows/sdlc/review/commands/review-synthesize.md
  • .archon/workflows/sdlc/triage/commands/triage.md
  • packages/workflows/src/defaults/bundled-defaults.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The review workflow now resolves acceptance items, invariants, and steering constraints from their source. It records these items in scope.md and requires review synthesis to judge each item, cite evidence, and report unmet items as findings.

Changes

Accepted Contract Review

Layer / File(s) Summary
Capture and resolve the accepted contract
.archon/workflows/sdlc/triage/commands/triage.md, .archon/workflows/sdlc/review/commands/review-scope.md, .archon/workflows/sdlc/review/archon-review.yaml
Triage instructions require verbatim contract items for READY verdicts. Review-scope instructions resolve the originating contract, require its items in scope.md, and verify the Acceptance, Invariants, and Steering subsections. The workflow description states that every review round checks the accepted contract.
Judge contract coverage
.archon/workflows/sdlc/review/commands/review-synthesize.md, packages/workflows/src/defaults/bundled-defaults.test.ts
Review synthesis defines met, unmet, and deferred coverage verdicts, requires findings for unmet items, and checks report coverage completeness. A bundled-defaults test checks the related triage and review instructions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f497b

Unmet contract items still block review readiness, while evidence assigned to a later delivery step remains tracked for that step. No actionable issue remains in the reviewed changes; they are ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: reviews now enforce the contract's acceptance list.
Description check ✅ Passed The description includes the required Problem and outcome, Review guidance, Solution, and Validation sections. It explains the motivation, scope, implementation, evidence, risks, and remaining verific…
Linked Issues check ✅ Passed The changes satisfy the coding requirements in [#3449]. Review scope now resolves and records every acceptance item, invariant, and steering constraint from the originating contract. Synthesis assigns…
Out of Scope Changes check ✅ Passed The changed workflow prompts and the bundled-workflow test stay within [#3449]. They implement contract resolution, contract coverage, blocking verdict behavior, source quoting, and validation for tho…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Wirasm
Wirasm merged commit e822169 into dev Sep 24, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(sdlc): the delivery review can pass a change that misses the contract's acceptance list

1 participant