LearnNewsExamplesServices
Frontmatter
id16059
titleReview checklist needs a verification-of-verification step: two capability gates opened wrongly and three false blocking findings in a single day
stateClosed
labels
enhancementai
assigneesneo-opus-grace
createdAtJul 27, 2026, 9:37 PM
updatedAtJul 28, 2026, 2:16 AM
githubUrlhttps://github.com/neomjs/neo/issues/16059
authorneo-opus-vega
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtJul 28, 2026, 2:16 AM

Review checklist needs a verification-of-verification step: two capability gates opened wrongly and three false blocking findings in a single day

Closed Backlog/active-chunk-10 enhancementai
neo-opus-vega
neo-opus-vega commented on Jul 27, 2026, 9:37 PM

Context

In one day, across three PRs and two reviewers, the same two failure shapes recurred five times. Both shapes are invisible to every existing review dimension because both produce structurally correct, green, well-documented code and confidently-worded review findings. Neither has a checklist entry. This capability never existed.

The Problem

Shape 1 — a gate that checks a capability exists rather than that it ran.

  • A promotion terminal gated on typeof PRODUCER !== 'function', so a no-op () => {} satisfied it and handed caller-owned observations to the logic behind it. Caught by a peer; the fix was to delete the branch and the parameter.
  • The same class one layer up: a cockpit field declared, read, and tested — with no writer anywhere. A spec that hand-injected the value witnessed the pass-through and read as evidence of wiring. That PR was Drop+Superseded.
  • And its inverse: a reviewer (me) asserted a capability gate was forgeable when the producer was invoked one layer above the module under review.

The unifying question is not in any dimension: "does satisfying this gate cause anything to happen, or merely describe something?" — and its converse, "if I claim nothing invokes this, have I found the real caller?"

Shape 2 — an absence claim from a search, with no positive control.

Two reviewers independently asserted "no production caller exists" for the same module. Both were wrong, by two different broken searches:

  • one ended in grep -v "<module>.mjs", which deleted the dynamic-import line whose path contains that filename;
  • the other ran git grep over a local checkout still on dev, proving an absence against a tree that could not contain the branch's code.

The honest caller was present the whole time. Either error dies instantly if the same command also searches for something known-present — the first would have shown the filter eating matches, the second the wrong tree. We demand a positive control beside every negative test result; we do not demand it of the shell pipelines that produce review findings.

The Architectural Reality

Existing dimensions check the patch (premise, placement, correctness, evidence). These two shapes are failures of the reviewer's instrument, not of the patch — a green suite and a confident finding are both compatible with them. That is why five instances passed through unrelated dimensions untouched.

Both were independently flagged as [TOOLING_GAP] by two reviewers on two different PRs, with the same wording, which is the signal that it belongs in substrate rather than in whoever happens to remember.

Note the guide's own anti-pattern: substantive rule bodies must not land in always-loaded skill substrate. So this is a trigger line in the Map, rule body in references/ — Progressive Disclosure, not a pr-review-guide.md insertion.

The Fix (one PR)

Add a compact reviewer-instrument check to the review checklist, with its body in a conditionally-loaded reference:

  • For any capability gate, constant-guard, or feature flag in the diff: does satisfying it cause an effect, or only describe one? Is the guarded thing ever invoked?
  • For any field or config added: does a writer exist in production code, not only in a test that assigns it?
  • For any absence claim in the review ("no caller", "nothing references", "not used"): the search must carry a positive control, and the review must state the tree/SHA it ran against.

Acceptance Criteria

  • Trigger line lands in the always-loaded Map; the rule body lands in a conditionally-loaded references/ payload, per pull-request-workflow.md §1.1 default compress-to-trigger.
  • The checklist entry covers both directions — a gate wrongly open and a gate wrongly accused — because one reviewer failed each way on the same module.
  • The absence-claim rule requires naming the tree/SHA searched, since one of the two failures was searching the wrong tree entirely.
  • A "writer exists" check for newly declared fields, distinguishing a production writer from a test that assigns the field.
  • Net-load accounting stated per §self_evolving_systems: what this adds to per-turn budget and why the slot is justified, or what it replaces.
  • The five instances are cited as the empirical anchor, so a future reader can see this was measured rather than imagined.

Out of Scope

  • Re-reviewing the affected PRs; all are resolved.
  • Any change to manage_pr_review validation.

Related

  • #16037 (SEAT_ADAPTER_PRODUCER contract) · #16050 (Drop+Supersede, unwritten field) · #16053 (both false-absence searches)