Skip to content

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

Merged
gurinderu merged 10 commits into
mainfrom
feat/triage-sift
Oct 23, 2026
Merged

gurinderu merged 10 commits into
mainfrom
feat/triage-sift

Conversation

@gurinderu

Copy link
Copy Markdown
Owner

The findings-handling workflow now sifts every finding — craft's, another reviewer's, a reviewer's own tool — before validation (realm @nick/craft, #305, case №37): repeats in the batch collapse; project memory is recalled (merged with priorDecisions, which the engine used to drop — #306) and each finding is matched with the review engine's matcher and judge; a finding a live decision answers is set aside as rejected-before (Critical/High and scope-changed decisions are raised again, #309); one the judge sees going against a recorded decision is a conflict for the human. Sifted rows are never carried forward or re-recorded. The validator judges noise classes (#303): an out-of-PR finding, checked at the base, becomes a deferral plus an issue the skill files itself (search open issues first). Rejections in other reviewers' PR threads are recorded as decisions without asking (#302), except an outsider dismissing their own remark.

Dropped Low findings:

  • The "checked at the base" test reads the premise's form; git show HEAD:path also passes.
  • An outsider can still dismiss their own remark from a second account (limited by scope, judge, and Critical/High re-raise).
  • PR-comment severity is estimated by the gather agent, so a guessed Medium can be set aside without a validator.
  • The skill's stated reason for batch-repeat rows ("settled by the record of the finding it repeats") does not always hold; the rule itself is right.
  • lib/triage-sift-engine.test.mjs is 152 lines, over the 150-line guideline.

Follow-up (a Low that can hurt a consumer): a sifted batch-repeat row is written to the review ledger as rejected, so if its kept twin's fix misses the repeat's line, the next re-review carries it as rejected; it should take its twin's disposition.

Open beyond this PR: the OpenCode triage copy now diverges (#311, the owner's call).

triage-findings now recalls the project memory itself (the review engine's
recall agent, merged with the passed priorDecisions) and, before any
validator runs, collapses repeats in the batch and sets aside what an active
decision, deferred question or project rule already answers, with the
record's who/when/link in the ledger. Critical/High matches and records whose
scope changed since their commit are raised again; a finding the match judge
shows going against a record is a conflict for a human.

The validator now names the noise class of each finding: "not this change's"
is checked at the base and becomes a deferral carrying an issue action (the
plan returns the issues to file), "not decidable from code" a needs-decision,
inflated severity is lowered, optional test/comment nits are planned apart.
The PR-thread rejection reading now also reads threads whose root is not a
craft finding — a human reviewer's remark or their own tool's — when the last
word is a reply opening with a rejection, from the PR author or an
OWNER/MEMBER/COLLABORATOR. Such a thread becomes a decision record titled by
the remark's first line, so the same remark in the next PR is set aside by
the triage sift. A silently resolved thread is still no rejection, and an
outside reviewer rejecting their own remark is named, not recorded.
…ings as issues

The fix loop shows the memory sift as its own step before triage (what a
decision, deferred question or project rule already answers is settled from
memory, Critical/High and changed scopes go on, a finding against a record is
a conflict), names the validator's noise classes, and sends a finding the PR
did not introduce to an issue on the forge without asking: an open issue on
the same concern gets a comment, else one is created, and the memory deferral
links it. Thread replies still wait for the human's explicit OK. The PR
rejection reference now covers other reviewers' threads; the description,
README, MAP and the run-record doc follow.
A row the sift set aside (a repeat, a record's answer) is marked with a
sifted field; a re-run with priorLedger decides such a finding afresh
from current memory instead of carrying it, so a finding the sift raises
again (scope changed, decision superseded, copy gone) reaches a validator.
…aside

A ledger row with sifted: true is already decided by an existing record;
re-recording it at HEAD would renew a stale record's commit and defeat
the sift's scope-changed check. The skill tells such rows apart by the
field, and the triage-ledger schema names it.
… and a reject files no issue

A class-1 verdict is read as checked at the base only when premise_checked
names a merge-base or a git show <ref>:<path>; any other premise leaves it
for a human. A validator reject with class 1 stays a reject: not a real
problem, so no issue is filed.
… own function

Keeps validateOne under the complexity bound after the sifted-row rule.
@gurinderu
gurinderu merged commit 1aa44f2 into main Oct 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant