Skip to content

Verify review-lens coverage instead of self-reporting it (per-lens receipts) #32

Description

@topij

Follow-up from #31, and the honest version of what that PR tried and failed to build.

What #31 shipped, and what it does not do

--record-review --lenses adversarial,correctness records which lenses ran, and
the poll render shows that at merge time. It is an audit trail: the operator
writes both the source and the lens names in one invocation, and nothing binds
either to a review that actually happened.

Why #31 does not verify it (four attempts, four defeats)

Each round moved the check and each was broken by the adversarial lens:

  1. Match the source string. fallback:panel (adversarial + correctness),
    fallback-panel, fallback: panel — all accepted with zero lenses and no
    warning
    , while the honest fallback:codex --lenses correctness produced the
    ⚠. The quietest receipt was the dishonest one.
  2. Report the recorded lens count instead. A newline in source split the
    render line and left the first half reading as a completed panel; , is both
    the lens separator and ordinary punctuation, so "correctness, i.e. does it do what it says" counted as two.
  3. Require names from the configured roster. Refused a genuine third ad-hoc
    lens, making the doc's own "two disjoint lenses is the floor, not the ceiling"
    false, and told the operator to record a three-lens review under a
    single-lens source.
  4. Count roster names, allow extras. The record gate matched source with
    == while the render matched it with startswith, so
    --record-review "fallback:panel (2 lenses)" --lenses "correctness, i.e. …"
    defeated the gate and the render then affirmed 2 lenses. Additionally an
    empty roster — reachable from four plausible config shapes, silently — failed
    open.

safety-critical-changes.md rule 1: "Treat 'we tightened the matcher' as a
stopgap, not a fix."
Four tightenings is the signal to stop. The verification
machinery was removed in #31 rather than shipped as theatre.

The shape that would actually work

Rule 1 asks for a deterministic artifact written at decision time and verified
at act time
. The artifact has to come from the lens, not from the operator
summarising it afterwards:

uv run <engine-dir>/pr_watch.py <PR#> --record-lens adversarial --head <sha>
uv run <engine-dir>/pr_watch.py <PR#> --record-lens correctness --head <sha>
  • each isolated lens records its own entry, from its own context, as its
    final act
  • the panel is derived — count distinct lens entries bound to the current head
  • --record-review "fallback:panel" then validates against entries that exist
    rather than against a string the caller just typed

An operator can still fake it by running the command themselves, but that is a
deliberate act with its own audit entry, rather than the path of least effort.

Open questions

  • Where does a lens write? The per-PR state file is the obvious place, but
    a subagent may not share the cockpit's filesystem view. Worth checking against
    the isolated-worktree requirement (fallback-review-panel.md contract item 7).
  • Does anything gate on it, or is it still reported? Resolve the queued-vs-unavailable ambiguity on both surfaces (#19 + #23) #25/Surface which commit each bot's last review actually saw (#27) #29 established
    report, never gate for signal / bot_signal / coverage, because gating
    risks wedging a repo whose bot is permanently unavailable. The same argument
    probably applies here.
  • Is it worth the machinery? The audit trail may be enough. This issue exists
    so the decision is made deliberately rather than by four more rounds of
    matcher-tightening.

Evidence

The four defeats are reproducible from #31's review thread and commit messages.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions