Skip to content

[roadmap] review-plan's anchor grammar silently drops cross-unit findings #69

Description

@azevedo

Evidence: a live /ba:review-plan run on docs/plans/2026-07-30-refactor-ba-commands-to-skills-plan.md (2026-07-30, v0.38.0).

The gap

review-plan's Plan-Anchor & Confidence Grammar allows exactly one key per finding: a section heading, a U<n>, or an AC<n>. Step 4b then drops any record whose anchor does not resolve and tallies it in dropped_off_plan.

Cross-unit findings have no representation in that grammar. "U15's static sweep and U17's new CI needle contradict each other" is a relation between two units — no single key expresses it. A reviewer has three bad options: pick one key arbitrarily and lose half the statement, write a multi-key anchor and get dropped, or not report it.

What actually happened

Five reviewers produced 29 findings. Four carried multi-key anchors:

  • **U15 / U17 / AC7** (confidence: 90) — the two units are mutually exclusive and AC7 was unachievable as written
  • **U11 / U14** (confidence: 75) — both Verify lines foreclosed documenting the migration itself
  • **U6 / Phase 2→Phase 3 boundary** (confidence: 50) — an automated phase gate would let five skills ship broken
  • **U12 / U13 / U14 / U15** (confidence: 40) — BRE alternation is a GNU extension, the dev machine is darwin

By the letter of the grammar all four drop into a counter. The first was the highest-value finding in the review and led to a plan-level fix (AC7 now carries a documented exception). The orchestrator overrode the drop rule to keep them — which is the tell: the rule had to be broken for the command to do its job.

Two further records were dropped legitimately (**Overview / Proposed Solution prose**, **Naming scheme ba-**), both self-described as speculative, so the drop path itself works. The problem is what it catches alongside them.

Why this matters more here than in /ba:review

review-plan is the third mirror site of the never-hide-ledger convention. The ledger guarantees no reviewer is silently dropped; nothing guarantees the same for a finding. A silent-drop path for cross-unit findings is a hole in exactly the property the command advertises — and cross-unit findings skew architectural, so the class being lost is the expensive one.

Anchor-only fingerprinting has a second-order effect worth folding into the same brainstorm: Step 4c groups by normalized anchor, so two genuinely different defects that share a unit key get merged into one entry. This run merged two distinct U11 findings (a false invariant, and a forward reference) and two distinct U2 findings (a missing basename check, and ls-output parsing) — with the corroboration math then treating them as agreement.

Fix directions (open — hence needs-brainstorm)

  1. Multi-key anchor form — a primary key plus related keys, e.g. - **U15** *(also: U17, AC7)* (confidence: N) — …. Preserves dedup on the primary; makes the relation first-class. Requires a grammar change on both the emitter and parser sides, and it is a machine-boundary contract, so it earns literal specification.
  2. Visible bucket instead of a counter — non-resolving anchors render in a ## Unanchored section rather than a tally. Cheaper, no grammar change, and consistent with never-hide; loses dedup and gate participation for those findings.
  3. Fingerprint on anchor + body, not anchor alone — fixes the false-merge half independently of the anchor form.

Option 2 is a small edit and would have preserved every finding in this run. Option 1 is the fuller fix. They compose.

Relationship to #61

#61 (declared output contract for reviewer subagents) is the near neighbour and cites the same family of salvage counters (dropped_no_fileline, off_diff) as evidence the prose contract drifts in practice. This item is not drift: the grammar is being followed and is simply not expressive enough. If #61 resolves toward a declared schema, that schema must include a multi-key anchor form, so #61 could subsume this. If #61 resolves toward "just harden the existing grammar", this stays separate. Sequence after #61's brainstorm, or fold in.

Note #41 (commands→skills) renames the file this grammar lives in — commands/ba/review-plan.mdskills/ba-review-plan/SKILL.md. Bodies move byte-unchanged, so line references survive with only a path-prefix shift.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions