feat: build profile review rules

MAP.md

@@ -32,7 +32,7 @@ Status: ✅ done
 | Skill | Status | Scope | Does NOT cover (owner) |
 |---|---|---|---|
 | `rust-testing` | ✅ | unit/integration/doc, async, rstest, proptest, cargo-fuzz, cargo-mutants, mockall, insta, testcontainers, coverage, runner, CI | benchmarks → `rust-performance` |
-| `rust-review` | ✅ | cargo gate, dependency-context step (review against pinned versions), severity checklist + **ID-tagged rule catalog** (`rules.md`), verdict; **public-API design pass** (Rust API Guidelines checklist → `api-design.md`); requesting a craft review (agent dispatch + crafted brief); the Rust "what proves what" verification table | *how* to fix → topic skills; *how* to test → `rust-testing` |
+| `rust-review` | ✅ | cargo gate, dependency-context step (review against pinned versions), severity checklist + **ID-tagged rule catalog** (`rules.md`), verdict; **exclusion catalog** — false-positive precedents + the `KEEP-*` non-reasons, each demanding a trace (`fp-rules.md`); premise grounding (`whereChecked`); the mirror walk (enforcement asymmetry); measured severity magnitude; **public-API design pass** (Rust API Guidelines checklist → `api-design.md`); requesting a craft review (agent dispatch + crafted brief); the Rust "what proves what" verification table | *how* to fix → topic skills; *how* to test → `rust-testing` |
 | `rust-errors` | ✅ | `Result`/`Option`, `?`, domain failures vs defects (ZIO model), thiserror vs anyhow, library-vs-app design, recovery/retry/circuit-breaker | panics as control flow → `rust-idioms` |
 | `rust-ownership` | ✅ | borrowing, lifetimes, `Cow`, smart pointers (`Box`/`Rc`/`Arc`), interior mutability (`Cell`/`RefCell`); fixes for E0382/E0597/E0499/E0502 | cross-thread sharing/`Send`+`Sync` → `rust-concurrency` |
 | `rust-concurrency` | ✅ | threads vs async, `Send`/`Sync`, `Arc<Mutex>`, channels, tokio, deadlocks, lock-across-await | single-thread `Rc`/`RefCell` → `rust-ownership` |
@@ -104,6 +104,7 @@ them by `agentType` — internal to the plugin, no external dependency).
 
 ## Documentation
 
+- `docs/LESSONS.md` — operational lessons not derivable from the code: stable-numbered evidence entries folded into principles. Add one when something was learned at a cost.
 - `docs/observability.md` — run-record store (`~/.craft/runs/`) emitted by the `rust-review` / `rust-audit` / `triage-findings` workflows and the review agents.
 
 ## Cross-cutting skills (language-agnostic)

agents/nix-reviewer.md

@@ -34,6 +34,13 @@ lens, review the whole Nix diff against the full rubric.
    `allowUnfree`-not-in-`nix develop` gotcha (`DEV`), secrets in the world-readable store (`MOD`),
    and dead/anti-idiomatic code (`MNT`).
 
+   **Ground every off-site premise.** A finding usually rests on a claim not visible at the line it
+   cites — "that flake input provides this", "the module default is X", "no other module sets it".
+   Open that code (the locked input's own source included) and report the `file:line` in
+   `whereChecked`; a premise you did not open is not admissible — open it or drop the claim. This
+   binds rejection too: dismissing a finding on an unopened "the module already sets this" discards
+   a real bug silently.
+
 5. **Report everything you suspect — do not self-censor.** Borderline findings are surfaced, not
    dropped; downstream verification decides Confirmed vs Suspected. Each finding cites
    `severity · file:line · [ruleId] · what · why · fix` (ruleId from `nix-review/rules.md` when it

agents/rust-reviewer.md

@@ -27,6 +27,13 @@ gives no lens, review the whole diff against the full rubric.
 
 4. **Apply the rubric** for your slice, walking CRITICAL → HIGH → MEDIUM tiers.
 
+   **Ground every off-site premise.** A finding usually rests on a claim that is not visible at the
+   line it cites — "the dependency rejects this", "reachable from untrusted input", "no caller
+   guards it". Open that code (dependency sources included) and report the `file:line` in
+   `whereChecked`; a premise you did not open is not admissible — open it or drop the claim. This
+   binds rejection too: dismissing a finding on an unopened "a caller already validates this"
+   discards a real bug silently. See the rust-review skill → *Premise grounding*.
+
 5. **Report everything you suspect — do not self-censor.** Borderline findings are surfaced, not
    dropped; downstream verification decides Confirmed vs Suspected. Each finding cites
    `severity · file:line · what · why · fix`. Use an empty location only when truly not locatable.

docs/LESSONS.md

@@ -0,0 +1,73 @@
+# craft — lessons
+
+Operational lessons about building and running craft that are **not derivable from the code**: why
+a rule is shaped the way it is, what an adopted practice cost, what a change broke. Not a changelog
+(`CHANGELOG.md`), not a plan (`docs/superpowers/plans/`), not architecture (`MAP.md`).
+
+**Structure.** The **principles** are the working set — what belongs in your head. Each folds one or
+more numbered entries in the **evidence appendix**, where the L-numbers are **stable**: never
+renumber, never reuse a retired one, so commits and notes can cite `L2` and still mean it. When a
+principle and its evidence seem to disagree, the evidence is the record of what happened; the
+principle is the compression.
+
+Add an entry when something was **learned at a cost** — a practice that didn't transfer, a design
+that had to be reworked, a failure mode that surfaced in use. Routine work does not qualify.
+
+---
+
+## Principles
+
+### P1 — Adopt the reasoning, not the artifact: check that the failure mode's preconditions exist here — folds L1, L2
+
+Practices imported from another harness or repo arrive shaped by *its* architecture. The lesson
+underneath is usually sound; the mechanism on top often solves a problem craft does not have, or
+collides with a distinction craft already draws. Before adopting, ask what specific conditions
+produced the original failure and whether those conditions hold here — then re-derive the mechanism
+in craft's own idiom instead of transplanting it.
+
+- **Do:** name the precondition, check it against craft's actual agent shapes and verdict model, and
+  re-express the rule in craft's vocabulary. A rule that has to be explained by reference to the
+  source repo has not been adopted, only copied.
+
+---
+
+## Evidence appendix
+
+### L1 — A borrowed safety rule can be inert because craft's agents have a different shape · 2026-08-01
+
+While mining `scadastrangelove/rust-in-peace` for review practices, one candidate was a prohibition
+on agents repairing shared toolchain state. Its origin: six parallel **fix**-agents, each with full
+Bash and a mandate to make the build pass, sharing one `$HOME`; one decided on its own initiative to
+repair `rustup`, caught a network reset mid-download, and left `~/.rustup` half-uninstalled — killing
+`cargo` for every other concurrent agent and the orchestrator. Git worktrees isolate sources, not
+toolchains.
+
+It was queued for adoption and dropped on inspection. craft's fanned-out agents are **read-only**
+reviewers and scanners — `triage-findings` explicitly makes no edits — and every one already carries
+"tool absent → note it and continue, never fail" (`agents/rust-security-scanner.md`,
+`agents/rust-miri.md`, the build-matrix prompt in `workflows/rust-audit.js`). An agent told not to
+fail has no motive to repair anything: the pressure that produced the incident is absent. The rule
+would have added prohibition text that never fires.
+
+- **Change:** none — deliberately. Revisit if the `addressing-findings` fix loop ever fans out into
+  parallel *editing* agents; that is the shape the rule guards, and then it earns its place.
+
+### L2 — An imported rule set has to be re-routed through craft's own verdict model · 2026-08-01
+
+Adopting the same repo's false-positive catalog as `skills/rust-review/fp-rules.md` looked like a
+straight port: seven exclusion precedents, each demanding a trace. Two did not fit. The source
+treats "the input is operator-controlled" and "the panic is only reachable from CLI/config" as
+FALSE_POSITIVE verdicts, because its pipeline ranks live vulnerabilities and latent hardening on one
+axis. craft's verifier separates them: `refuted` means *the technical claim is false*, and its
+refutation rule already states that context — test-only, low impact, intentional — never justifies
+it. Importing those two as written would have contradicted that rule and taught verifiers to delete
+findings whose claims hold.
+
+They ship as **severity downgrades with `refuted=false`** instead. The same round produced a second
+instance: an unsupported premise also had to be routed to Suspected rather than refuted, because
+`adversarial-review` feeds its refuted list forward as "adversarially disproven — do not re-report",
+so a missing citation would have buried a possibly real defect for the rest of the run.
+
+- **Change:** when importing a rubric, map every verdict it produces onto craft's existing verdict
+  vocabulary **before** writing it down, and check what each downstream stage does with that verdict.
+  A bucket name that matches is not a meaning that matches.

docs/observability.md

@@ -18,6 +18,18 @@ can be studied later.
 Common: `ts`, `runtime` (`"claude-code"` | `"opencode"`), `kind` (`workflow`|`agent`), `name`, `project`, `commit`, `dirty`, `verdict`,
 `findings: {total, bySeverity:{Critical,High,Medium,Low,Info}}`, `nested`, `via`.
 
+**Engine identity** — `craftVersion` (the plugin release, stamped from a `CRAFT_VERSION` const that
+`lib/check-workflows.mjs` keeps in sync with `.claude-plugin/plugin.json`) and `craftCommit`
+(craft's own git HEAD, best-effort via `$CLAUDE_PLUGIN_ROOT`). Distinct from `commit`, which is the
+**reviewed project's** HEAD, and from `schemaVersion`, which versions this record format.
+
+Both ride in `index.jsonl` as well as the detail file, because filtering an aggregate to one engine
+version is done by scanning the index. Without them, findings-per-run and refute rates average
+across every rubric change the store has ever seen, so "did tightening that lens help?" cannot be
+answered. `node lib/analyze-runs.mjs --version latest` (or `--version 0.13.1`) applies the filter;
+with no flag, a store holding more than one version says so in the report. Records written before
+these fields carry `null` and are simply outside any version filter.
+
 Workflows add: `scout`, `dimensions[]`, `verification {candidates, confirmed, refuteRate}`,
 `notRun[]`, `outputTokens` (approximate — `budget.spent()`, shared per-turn pool). The `scout`
 shape is workflow-specific — rust-review records `{size, lenses, model, maxRounds, verifyVotes}`,

lib/analyze-runs.mjs

@@ -26,6 +26,12 @@ const isIncomplete = v => /INCOMPLETE/i.test(String(v || ''))
 // A per-lens refute rate needs a minimum candidate pool before it means anything — one refuted
 // finding out of one is not "an over-firing lens". Below this, a lens is omitted from the NOISE rank.
 const MIN_REFUTE_CANDIDATES = 4
+// ...and a lens is only NOISY if it actually over-refutes. Without a floor the section listed EVERY
+// lens with enough candidates — including ones at refute 0.00 — each under the header "over-refuting"
+// and the advice "tighten this lens's rubric". That advice is backwards for a precise lens: tightening
+// it suppresses findings that were all being confirmed. Observed on a 60-run store, where 13 of 13
+// ranked lenses were listed and the bottom two sat at 0/9 and 0/5.
+const MIN_REFUTE_RATE = 0.25
 
 // Pure: array of parsed run records → structured summary. Tolerant of malformed / partial records.
 export function aggregate(records) {
@@ -63,7 +69,12 @@ export function aggregate(records) {
     for (const dim of (Array.isArray(r.dimensions) ? r.dimensions : [])) {
       if (!dim || typeof dim !== 'object') continue
       const k = String(dim.dimension || '(unnamed)')
-      const agg = byDimension[k] || (byDimension[k] = { runs: 0, findings: 0, confirmed: 0, suspected: 0, refuted: 0, bySeverity: { Critical: 0, High: 0, Medium: 0, Low: 0, Info: 0 } })
+      const agg = byDimension[k] || (byDimension[k] = { runs: 0, dead: 0, findings: 0, confirmed: 0, suspected: 0, refuted: 0, bySeverity: { Critical: 0, High: 0, Medium: 0, Low: 0, Info: 0 } })
+      // A dimension row exists for every PLANNED lens, so a lens that never returned looks identical
+      // to one that ran and found nothing — and yield-per-run silently divides by the dead runs too.
+      // `ran: false` marks the dead ones; records predating the flag have no way to tell, so they
+      // count as having run (the previous behaviour) rather than being guessed at.
+      if (dim.ran === false) { agg.dead++; continue }
       agg.runs++
       agg.findings += Number(dim.findingCount) || 0
       // Per-lens survival — present only on records written after the per-lens telemetry landed.
@@ -93,7 +104,7 @@ export function aggregate(records) {
   const dimensions = Object.entries(byDimension).map(([dimension, d]) => {
     const candidates = d.confirmed + d.suspected + d.refuted
     return {
-      dimension, runs: d.runs, findings: d.findings, bySeverity: d.bySeverity,
+      dimension, runs: d.runs, dead: d.dead, findings: d.findings, bySeverity: d.bySeverity,
       findingsPerRun: d.runs ? round2(d.findings / d.runs) : 0,
       confirmed: d.confirmed, suspected: d.suspected, refuted: d.refuted, candidates,
       // null (not 0) when there is no per-lens verification data, so old runs don't read as "0% refute".
@@ -105,15 +116,25 @@ export function aggregate(records) {
 }
 
 // ---- CLI ----
+// Returns {records, unreadable} — or null when the store directory does not exist.
+// `unreadable` is load-bearing, not a diagnostic nicety: a record that fails to parse is a run whose
+// telemetry is GONE, and silently dropping it makes the store look smaller rather than damaged. A
+// 60-run store was found holding a 0-byte record (a write that died mid-flight); the count mismatch
+// against the file listing was the only trace, and nothing in the report mentioned it.
 export function loadRecords(dir) {
   let files
   try { files = fs.readdirSync(dir) } catch { return null }
-  const out = []
+  const records = []
+  const unreadable = []
   for (const f of files) {
     if (!f.endsWith('.json')) continue   // skips index.jsonl and README.md
-    try { out.push(JSON.parse(fs.readFileSync(path.join(dir, f), 'utf8'))) } catch { /* skip malformed */ }
+    try {
+      records.push(JSON.parse(fs.readFileSync(path.join(dir, f), 'utf8')))
+    } catch (e) {
+      unreadable.push({ file: f, reason: String((e && e.message) || e).slice(0, 80) })
+    }
   }
-  return out
+  return { records, unreadable }
 }
 
 export function renderReport(a) {
@@ -132,25 +153,67 @@ export function renderReport(a) {
   if (a.dimensions.length) for (const d of a.dimensions) {
     const sev = SEVERITIES.filter(s => d.bySeverity[s]).map(s => `${s[0]}${d.bySeverity[s]}`).join(' ') || '—'
     L.push(`- ${d.dimension}: ${d.findings} finding(s) / ${d.runs} run(s) (${d.findingsPerRun}/run) · ${sev}`
-      + `${d.refuteRate != null ? ` · refute ${d.refuteRate} (${d.refuted}/${d.candidates})` : ''}`)
+      + `${d.refuteRate != null ? ` · refute ${d.refuteRate} (${d.refuted}/${d.candidates})` : ''}`
+      + `${d.dead ? ` · ⚠️ ${d.dead} run(s) it never returned` : ''}`)
   } else L.push('- none')
-  L.push('', `## NOISE — lenses over-refuting (per-lens refute rate, ≥${MIN_REFUTE_CANDIDATES} candidates)`)
-  const noisy = a.dimensions
-    .filter(d => d.refuteRate != null && d.candidates >= MIN_REFUTE_CANDIDATES)
+  L.push('', `## NOISE — lenses over-refuting (refute ≥ ${MIN_REFUTE_RATE}, ≥${MIN_REFUTE_CANDIDATES} candidates)`)
+  const rated = a.dimensions.filter(d => d.refuteRate != null && d.candidates >= MIN_REFUTE_CANDIDATES)
+  const noisy = rated
+    .filter(d => d.refuteRate >= MIN_REFUTE_RATE)
     .sort((x, y) => y.refuteRate - x.refuteRate || y.candidates - x.candidates)
   if (noisy.length) for (const d of noisy) {
     L.push(`- ${d.dimension}: refute ${d.refuteRate} (${d.refuted}/${d.candidates}) · ${d.confirmed} confirmed — tighten this lens's rubric`)
+  } else if (rated.length) {
+    L.push(`- none — all ${rated.length} lens(es) with enough candidates refute below ${MIN_REFUTE_RATE}`)
   } else L.push('- no per-lens refute data yet (needs runs recorded after the per-lens telemetry landed)')
   return L.join('\n')
 }
 
 const invokedDirectly = process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)
 if (invokedDirectly) {
-  const dir = process.argv[2] || path.join(os.homedir(), '.craft', 'runs')
-  const records = loadRecords(dir)
-  if (records === null) {
+  // `--version <v>` / `--version latest` narrows the aggregate to one engine version. Mixing
+  // versions is the default only because old records predate the field; any before/after question
+  // ("did tightening that lens help?") needs this filter or the answer blends both rubrics.
+  let wantVersion = null
+  const positional = []
+  const rawArgs = process.argv.slice(2)
+  for (let i = 0; i < rawArgs.length; i++) {
+    const a = rawArgs[i]
+    if (a.startsWith('--version=')) { wantVersion = a.slice('--version='.length); continue }
+    // Consume the VALUE too, or it is mistaken for the store directory.
+    if (a === '--version') { wantVersion = rawArgs[++i] ?? 'latest'; continue }
+    if (!a.startsWith('-')) positional.push(a)
+  }
+  const dir = positional[0] || path.join(os.homedir(), '.craft', 'runs')
+  const loaded = loadRecords(dir)
+  if (loaded === null) {
     console.log(`No run store at ${dir} — nothing to analyze yet. Run some reviews first.`)
     process.exit(0)
   }
-  console.log(renderReport(aggregate(records.filter(r => r && r.schemaVersion))))
+  const { records, unreadable } = loaded
+  // Records that parse but carry no schemaVersion are pre-telemetry runs — excluded from the
+  // aggregate on purpose, but counted out loud so the report's run total is explainable.
+  let usable = records.filter(r => r && r.schemaVersion)
+  const versions = [...new Set(usable.map(r => r.craftVersion).filter(Boolean))].sort()
+  let versionNote = ''
+  if (wantVersion) {
+    const target = wantVersion === 'latest' ? versions[versions.length - 1] : wantVersion
+    if (!target) {
+      console.log('No run carries a craftVersion yet — nothing to filter on. Showing everything.')
+    } else {
+      const before = usable.length
+      usable = usable.filter(r => r.craftVersion === target)
+      versionNote = `\n_Filtered to craft ${target}: ${usable.length} of ${before} run(s)._`
+    }
+  } else if (versions.length > 1) {
+    // Silence here would be the trap: the numbers look like one engine and are actually several.
+    versionNote = `\n_⚠️ This store mixes ${versions.length} craft versions (${versions.join(', ')}) — findings-per-run and refute rates are averaged ACROSS rubric changes. Use \`--version latest\` to compare like with like._`
+  }
+  console.log(renderReport(aggregate(usable)) + versionNote)
+  const legacy = records.length - usable.length
+  if (legacy) console.log(`\n_${legacy} record(s) skipped: no schemaVersion (pre-telemetry runs)._`)
+  if (unreadable.length) {
+    console.log(`\n## ⚠️ Unreadable records — ${unreadable.length} run(s) of telemetry lost`)
+    for (const u of unreadable) console.log(`- ${u.file} — ${u.reason}`)
+  }
 }