Skip to content

feat(ci): mechanize the surface/clause coverage matrix for the review-disposition contract #1659

Description

@kyle-sexton

🤖 Agent-authored (autonomous babysit lane). Adopted recommendation from the #1614 implementation (#1633); filing per orchestrator direction.

Problem

The review-disposition-and-resolution contract is distributed across eight surfaces:

Surface Role
plugins/source-control/reference/review-discipline.md canonical — D4.6, D7.5, authorization
plugins/source-control/skills/pull-request/SKILL.md restates (D-sequence checklist + gotcha)
plugins/source-control/skills/pull-request/reference/monitor.md restates
plugins/source-control/skills/babysit-prs/reference/loop.md restates
plugins/source-control/skills/babysit-prs/SKILL.md references by range
plugins/source-control/skills/babysit-prs/reference/safety.md never-do list, non-convergence taxonomy
plugins/source-control/skills/babysit-prs/reference/orchestration.md Worker Contract
babysit_resolve_thread.py::classify, babysit_classify.py::thread_is_open mechanical enforcers

Nothing checks that they agree. Editing the canonical text and missing a restatement produces a surface that instructs a lane to do something another surface or a Python guard forbids — and the failure is silent, because every surface is individually well-formed.

Evidence: five consecutive rounds on one PR

#1633 is a controlled demonstration. Seven findings across five automated review rounds, every one class (c) — a defect in text that PR itself authored — and every one an agreement failure between surfaces:

Round Defect Shape
1 Authorization named an unreachable dispatch canonical vs pre-escalation-dispatch.md
1 3 of 4 restatements still gated on a pushed fix canonical vs restatements
2 Eligibility per-finding vs per-thread canonical vs thread_is_open
2 Provenance scoped by file rather than base behavior internal self-contradiction
2 Worker told to resolve what classify refuses canonical vs Worker Contract
3 D4.6 absent from the one enumerated D-sequence canonical vs restatement
4 Authorization absent from ALL THREE merge-capable paths canonical vs restatements
5 Routing rule mandated an unreachable dispatch internal self-contradiction

The escalation is the useful part. A clause patch failed. A deliberate whole-section rewrite failed — round 3 found the surface it missed. Only a mechanical cross-surface sweep converged, and it caught two gaps the reviewer had not yet reported.

Then the hand-run matrix itself produced a false "no remaining gaps": it grepped for the carve-out phrase but never asked whether a surface restates the eligibility enumeration, so a file could pass on "has the carve-out somewhere" while carrying an uncarved copy elsewhere. An independent resolver caught it at pull-request/SKILL.md:272. A hand-run check is not a gate — that is precisely the argument for mechanizing it.

Proposal

A CI gate asserting, for each clause of the contract, that every surface either owns it, points at it, or restates it completely (with every qualifier the canonical carries).

Working prototype from #1633, corrected after the false negative — keys on restatement rather than phrase presence:

enum_re  = re.compile(r"(grounded deferral|grounded `VALID \(defer\)`|`VALID \(defer\)` grounded)", re.I)
carve_re = re.compile(r"(never clears the gate|adjudicating context|authorizes a resolution|independent resolution dispatch)", re.I)
# verdict per surface:
#   canonical            -> owns it
#   enumerates == 0      -> OK (points, no restatement)
#   enumerates && carve  -> OK (restates + carve-out)
#   enumerates && !carve -> GAP

Reports zero gaps across all eight surfaces at #1633 head.

Design notes for whoever takes this

  • Regex over prose is a starting point, not the destination. It is checking natural language and will drift. Consider tagged spans (an HTML comment marking a restatement region and the clause id it restates) so the gate keys on declared intent rather than phrasing. That also makes the gate teach: an untagged restatement is itself a finding.
  • The surface list must not be hand-maintained — that reintroduces the same defect one level up. Derive it: any file citing D7.5, D4.6, or the disposition vocabulary is in scope.
  • Prefer fewer surfaces over a better gate. The strongest fix is pointer-not-copy: cut restatements down to pointers so there is less to keep in agreement. pull-request/SKILL.md:272 was fixed exactly this way. The gate should make a new restatement visible enough to argue about.
  • Cover the Python enforcers too. Round 2's per-finding/per-thread defect was prose contradicting thread_is_open; a prose-only gate would have missed it.

Acceptance criteria

  • A gate fails when a surface restates a contract clause without its qualifiers, and passes at feat(source-control): ground deferred findings and name self-inflicted rounds #1633 head.
  • The in-scope surface list is derived, not hardcoded.
  • The gate names the surface, the clause, and what is missing — not just a diff.
  • A self-test exists, so a broken detector cannot mask a real gap behind a green gate (the convention every other gate in this repo follows).
  • The prose/Python agreement direction is covered or explicitly deferred with a reason.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

automatedOpened by automation.priority: highSignificant impact, or blocks an imminent release; staff this cycle.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions