feat(triage): sift findings by project memory before validation

MAP.md

@@ -107,7 +107,7 @@ committed.
 | `review` | **auto-detects language(s)** → per-profile (rust/nix) scout + gate + scout-scaled lens fan-out → loop-until-dry → adversarial + self-verification → merge | one Confirmed/Suspected report + verdict across all detected languages (the generic engine; `PROFILES.{rust,nix}` inline) |
 | `rust-review` / `nix-review` | thin pins → `workflow('review', {languages:['rust'\|'nix']})` | single-language review via the engine above |
 | `rust-audit` | per-crate review + inter-crate contracts + architecture + crate-decomposition + security + miri + semver + build-matrix + deps + unused-crates (verified) + tests-cov (parallel) | one synthesized severity-ranked report |
-| `triage-findings` | gather → validate (parallel, per finding) → plan (barrier) | one ordered fix plan (writing-plans format) + triage ledger; no edits |
+| `triage-findings` | gather → sift by project memory → validate (parallel, per finding) → plan (barrier) | one ordered fix plan (writing-plans format) + triage ledger; no edits |
 | `adversarial-review` | scout + codebase-memory warm-up → throttled finder lenses → 1 combined verifier per finding (3-lens panel for critical/high) → completeness critic with verified gaps | language-agnostic adversarial diff review with bounded fan-out and steady request rate (subscription-friendly); Confirmed/Suspected report + Block/Warning/Approve verdict. Distinct from `review --strict` (maintainability-block mode of the generic engine) |
 
 ## Documentation

README.md

@@ -56,7 +56,7 @@ cross-linking is the collection's cohesion.
 | workflow | `review` | **Default diff-review path.** Auto-detects the language(s) in the diff (Rust/Nix), runs each language's gate + scout-scaled lens fan-out (loop-until-dry, adversarial + self-verification), and merges into one Confirmed/Suspected report + verdict |
 | workflow | `rust-review` / `nix-review` | Thin pins over `review` that force a single language (`workflow('review', {languages:['rust'\|'nix']})`) |
 | workflow | `rust-audit` | Full crate audit — per-crate review, inter-crate contracts, architecture, crate-decomposition, security, Miri, semver, build-matrix, deps, unused-crate detection (verified), and test/doc health, run in parallel and synthesized into one report |
-| workflow | `triage-findings` | Validates review findings (craft agents + GitHub PR comments) in parallel, dedups/conflict-checks, and renders one ordered fix plan (checkbox-task format) + a triage ledger — no edits |
+| workflow | `triage-findings` | Sifts review findings (craft agents + GitHub PR comments) by project memory, validates the rest in parallel, dedups/conflict-checks, and renders one ordered fix plan (checkbox-task format) + a triage ledger — no edits |
 
 ## Layout
 
@@ -135,8 +135,10 @@ review, inter-crate contracts, architecture, crate-decomposition, security, Miri
 semver, build-matrix, deps, unused-crate detection (with verification), and test/doc health — and
 a final step synthesizes one severity-ranked report.
 
-`workflows/triage-findings.js` validates findings (craft agents + GitHub PR comments), dedups and
-conflict-checks them, and renders one ordered fix plan + a triage ledger — no edits.
+`workflows/triage-findings.js` gathers findings (craft agents + GitHub PR comments), sifts out what
+the project's memory already settled, validates the rest, dedups and conflict-checks them, and
+renders one ordered fix plan + a triage ledger (and the issues to file for findings the PR did not
+introduce) — no edits.
 
 Both review workflows and the standalone agents emit a structured run record after each run; see
 `docs/observability.md`.

docs/observability.md

@@ -95,10 +95,13 @@ its "Reviewer agent unavailable" section. Records written before this by `review
 `lib/analyze-runs.mjs` reads both shapes and ranks the result in its AGENT UNAVAILABLE section.
 `triage-findings` is not a review: it carries `verdict: ""` and `findings` summarizing the findings
 it *triaged* (total + severity mix of the gathered raw findings, not findings it produced). It adds
-`sources[]` (`{source, count}` per gathered source) and `triage {gathered, validated, accept,
-reject, defer, needs-decision, conflict}` (disposition tally from the plan ledger, or the solo
-validations when the plan phase produced nothing); `notRun[]` lists requested sources that failed to
-gather. It has no `scout`/`dimensions`/`verification`/`outputTokens`.
+`sources[]` (`{source, count}` per gathered source), `triage {gathered, validated, accept,
+reject, defer, needs-decision, conflict, sifted, issues}` (disposition tally from the plan ledger, or
+the solo validations and the sifted rows when the plan phase produced nothing; `sifted {repeats,
+rejectedBefore, deferred, covered, conflict, reraised}` counts what the memory sift settled or raised
+again before validation; `issues` the findings deferred to an issue as not the PR's) and `memory`
+(where the sift's records came from, as `review` records it); `notRun[]` lists requested sources that
+failed to gather. It has no `scout`/`dimensions`/`verification`/`outputTokens`.
 The `rust-security-scanner` agent additionally records `toolsRun[]` (which cargo tools actually ran).
 rust-review records also carry `gate {status, provenance}` (always) and `failedChecks[]`
 (gate-fail path only); these summary-only extras are NOT carried into the `index.jsonl` projection.

lib/pr-rejections.mjs

@@ -1,10 +1,12 @@
-// The author's rejections of craft findings, read from a PR's review threads, as decision records in
+// The author's rejections of review findings, read from a PR's review threads, as decision records in
 // the memory skill's shape (skills/memory) — what addressing-findings records before it works the
 // findings, so the next review can set those findings aside (lib/prior-decisions.mjs). Run by the
 // skill as `node <plugin>/lib/pr-rejections.mjs <threads.json>`, over the `gh api graphql` answer
-// skills/addressing-findings/pr-rejections.md names. The per-thread rule is lib/pr-thread-rule.mjs.
+// skills/addressing-findings/pr-rejections.md names. The per-thread rule is lib/pr-thread-rule.mjs for a
+// craft finding's thread, lib/pr-reviewer-thread.mjs for any other reviewer's (realm @nick/craft, node #302).
 import fs from 'node:fs'
 import { threadDecision, NOT_AUTHORIZED, NOT_CRAFT_ROOT } from './pr-thread-rule.mjs'
+import { reviewerThreadDecision, SELF_DISMISSED } from './pr-reviewer-thread.mjs'
 import { outputLines, runIfMain } from './script-main.mjs'
 
 /** @typedef {import('./pr-thread-rule.mjs').ReviewThread} ReviewThread @typedef {import('./pr-thread-rule.mjs').DecisionRecord} DecisionRecord */
@@ -60,14 +62,16 @@ export function rejectionsFromThreads(answer) {
   const skipped = []
   const read = threads.nodes.slice(0, PR_THREADS_MAX)
   for (const t of read) {
-    const d = threadDecision(t, threads.author)
+    const d = threadDecision(t, threads.author) ?? reviewerThreadDecision(t, threads.author)
     if (typeof d === 'string') skipped.push(d)
     else if (d) decisions.push(d)
   }
   const forged = skipped.filter(s => s.endsWith(NOT_CRAFT_ROOT)).length
   if (forged) skipped.push(`${forged} thread(s) not read as craft findings: the marked root comment's author is not OWNER/MEMBER/COLLABORATOR`)
   const unauthorized = skipped.filter(s => s.endsWith(NOT_AUTHORIZED)).length
   if (unauthorized) skipped.push(`${unauthorized} rejection(s) not recorded: the replier is neither the PR author nor OWNER/MEMBER/COLLABORATOR`)
+  const selfDismissed = skipped.filter(s => s.endsWith(SELF_DISMISSED)).length
+  if (selfDismissed) skipped.push(`${selfDismissed} rejection(s) not recorded: an outside reviewer rejected their own remark`)
   const unread = Math.max(threads.total, threads.nodes.length) - read.length
   if (unread > 0) skipped.push(`${unread} thread(s) past the ${PR_THREADS_MAX} read were not examined — nothing recorded from them`)
   return { decisions, skipped }

lib/pr-rejections.test.mjs

@@ -41,12 +41,12 @@ test('rule: a reply opening with a rejection, as the last word, records a decisi
   }
 })
 
-test('rule: conservative — mid-sentence, not the last word, a reply from no authority, a non-craft root: nothing recorded', () => {
+test('rule: conservative — mid-sentence, not the last word, a reply from no authority: nothing recorded', () => {
   const none = (/** @type {any[]} */ cs, over = {}) => /** @type {any} */ (rejectionsFromThreads(answer([thread(cs, over)]))).decisions.length
   assert.equal(none([ROOT, reply('Fixed in abc123 — this was not a bug before the refactor though')]), 0, 'does not open with a rejection')
   assert.equal(none([ROOT, reply('Not a bug.'), reply('Actually it is, fixing', 'bob')]), 0, 'the rejection is not the last word')
   assert.equal(none([ROOT, reply('Not a bug.', 'gurinderu', 'NONE')]), 0, 'the poster with no authority cannot reject its own finding')
-  assert.equal(none([{ ...ROOT, body: '[Medium] unwrap in parser may panic' }, reply('Not a bug.')]), 0, 'a root without the marker cannot be tied to a finding')
+  assert.equal(none([{ ...ROOT, body: '[Medium] unwrap in parser may panic' }, reply('Not a bug.')]), 1, 'a root without the marker is another reviewer\'s remark: its rejection is recorded too (pr-reviewer-thread.test.mjs)')
   const r = /** @type {any} */ (rejectionsFromThreads(answer([thread([ROOT, reply('Fixed, thanks')])])))
   assert.match(r.skipped[0], /src\/parse\.rs: unwrap in parser may panic — the last reply does not open with a rejection/)
 })

lib/pr-reviewer-thread.mjs

@@ -0,0 +1,89 @@
+// A rejection in the thread of a reviewer that is not craft (realm @nick/craft, node #302): a person or
+// their own tool comments on the PR, the comment carries no craft-finding marker, and the last word is a
+// reply that rejects it, from the PR author or an OWNER/MEMBER/COLLABORATOR — the same reply rule as a
+// craft thread (lib/pr-thread-rule.mjs, nodes #182, #197). It becomes a decision record without asking
+// (node #223), so the same remark in the next PR is set aside by triage's sift (node #305). The root may
+// come from anyone; one guard more than a craft thread needs: an outsider who rejects their own remark
+// has settled nothing, or a contributor could post a remark and its rejection to silence a finding for
+// the whole project (the reason a forged craft root is refused, node #186). No reply is no rejection
+// (node #185). Read across a whole PR by lib/pr-rejections.mjs, after lib/pr-thread-rule.mjs says the
+// thread is not craft's.
+import { REJECTING_ASSOCIATIONS, THREAD_COMMENTS_MAX, decisionId, recordProblem, threadAnchor, threadRejection } from './pr-thread-rule.mjs'
+
+/**
+ * @typedef {import('./pr-thread-rule.mjs').ReviewThread} ReviewThread
+ * @typedef {import('./pr-thread-rule.mjs').DecisionRecord} DecisionRecord
+ */
+
+// A reviewer's remark has no title line of its own: its first line, cut at a word, stands in for one.
+export const REMARK_TITLE_MAX = 120
+
+// The tail of the skip line for an outsider rejecting their own remark; lib/pr-rejections.mjs counts by it.
+export const SELF_DISMISSED = 'who also wrote the remark and is not an owner, member or collaborator: not recorded'
+
+/** @param {unknown} v */
+const str = v => String(v ?? '').trim()
+
+/**
+ * The title a remark stands under: its first line with any heading, quote or emphasis marks around it
+ * dropped, cut at a word past REMARK_TITLE_MAX; '' when it has no line of text.
+ * @param {unknown} body @returns {string}
+ */
+export function remarkTitle(body) {
+  const first = String(body ?? '').split('\n').map(l => l.replace(/^[\s#>*_-]+|[\s*_]+$/g, '').replace(/\s+/g, ' ')).find(Boolean) ?? ''
+  if (first.length <= REMARK_TITLE_MAX) return first
+  const cut = first.slice(0, REMARK_TITLE_MAX)
+  const at = cut.lastIndexOf(' ')
+  return `${(at > REMARK_TITLE_MAX / 2 ? cut.slice(0, at) : cut).trimEnd()}…`
+}
+
+/**
+ * A thread whose root is not a craft finding, read: its comments, the root, the remark's title, the
+ * path, its reviewer, how many comments it has in all, and how a skip line names it. null with no reply.
+ * @param {ReviewThread} t
+ */
+export function remarkThread(t) {
+  const cs = Array.isArray(t.comments?.nodes) ? t.comments.nodes : []
+  const root = cs[0]
+  if (!root || cs.length < 2) return null
+  const title = remarkTitle(root.body)
+  const scope = str(t.path)
+  const reviewer = str(root.author?.login)
+  return { cs, root, title, scope, reviewer, total: Number(t.comments?.totalCount ?? cs.length), where: remarkName(scope, title, reviewer) }
+}
+
+/** How a skip line names a remark. @param {string} scope @param {string} title @param {string} reviewer */
+const remarkName = (scope, title, reviewer) => `${scope || '?'}: ${title || '(remark without text)'} (reviewer ${reviewer || '?'})`
+
+/**
+ * Why a rejection cannot settle the remark, '' when it can: an outsider rejecting their own remark, or a
+ * remark with no text to title the decision.
+ * @param {NonNullable<ReturnType<typeof remarkThread>>} c @param {string} by @returns {string}
+ */
+function unsettled(c, by) {
+  const assoc = str(c.cs[c.cs.length - 1]?.authorAssociation).toUpperCase()
+  if (c.reviewer && c.reviewer === by && !REJECTING_ASSOCIATIONS.includes(assoc)) return `rejected by ${by}, ${SELF_DISMISSED}`
+  return c.title ? '' : 'the remark has no text to title the decision: record it by hand'
+}
+
+/**
+ * One thread whose root is not a craft finding, as a decision record, or why its rejection is not
+ * recorded; null when the thread holds no rejection at all (most remarks: fixed, answered, open).
+ * @param {ReviewThread} t @param {string} [prAuthor]  the PR author's login, '' when unknown
+ * @returns {DecisionRecord | string | null}
+ */
+export function reviewerThreadDecision(t, prAuthor = '') {
+  const c = remarkThread(t)
+  if (!c) return null
+  if (c.total > c.cs.length) return `${c.where} — ${c.total} comments, past the ${THREAD_COMMENTS_MAX} read: the last word is unknown`
+  const r = threadRejection(c.cs, prAuthor)
+  if (typeof r === 'string') return r.startsWith('rejected by') ? `${c.where} — ${r}` : null
+  const commit = str(c.root.originalCommit?.oid)
+  const problem = unsettled(c, r.by) || recordProblem(c.title, c.scope, commit, r.reason)
+  if (problem) return `${c.where} — ${problem}`
+  return {
+    id: decisionId(c.title, c.scope), kind: 'decision', title: c.title, body: r.reason, scope: c.scope, status: 'active',
+    date: str(c.cs[c.cs.length - 1]?.createdAt).slice(0, 10), author: r.by, commit, links: [r.url, str(c.root.url)].filter(Boolean),
+    ...threadAnchor(t, {}),
+  }
+}