v1.4.1: address review follow-ups on precision-filter

action.yml

@@ -821,15 +821,41 @@ runs:
               echo "::warning::L1 postfilter failed — keeping engine findings"
             fi
             L2_OUT="${1%.json}.l2.json"
+            # Snapshot the engine's policy-block CONTENT (not just presence)
+            # before the L2 sidecar. The judge call goes through fact-proxy,
+            # whose `recordPolicyBlock()` can OVERWRITE `$POLICY_BLOCK` if the
+            # judge itself hits a guardrail — a presence-only flag can't tell
+            # "engine block preserved" from "engine block silently replaced by
+            # L2 block". After the L2 call, if we snapshotted engine content
+            # we always restore it (both success and fail paths) so the
+            # engine's guardrail record wins over an L2 sidecar overwrite.
+            POLICY_BLOCK_SNAPSHOT="${POLICY_BLOCK}.pre-l2"
+            rm -f "$POLICY_BLOCK_SNAPSHOT"
+            if [ -f "$POLICY_BLOCK" ]; then cp -f "$POLICY_BLOCK" "$POLICY_BLOCK_SNAPSHOT"; fi
             if node "$JUDGE" "$1" --model "$JUDGE_MODEL" --threshold "$JUDGE_THRESHOLD" --out "$L2_OUT" 2>&1 \
                && [ -s "$L2_OUT" ]; then
               L2_IN_COUNT=$(node -pe "(require('$1').comments||[]).length")
               L2_OUT_COUNT=$(node -pe "(require('$L2_OUT').comments||[]).length")
               echo "L2 judge ($JUDGE_MODEL, thr=$JUDGE_THRESHOLD): $L2_IN_COUNT -> $L2_OUT_COUNT"
               mv "$L2_OUT" "$1"
+              # L2 succeeded. Restore engine's block if we snapshotted one —
+              # covers the case where L2 was warn-mode guardrail-hit and
+              # overwrote engine's authoritative record mid-call.
+              if [ -f "$POLICY_BLOCK_SNAPSHOT" ]; then
+                mv -f "$POLICY_BLOCK_SNAPSHOT" "$POLICY_BLOCK"
+              fi
             else
               echo "::warning::L2 judge failed — keeping L1 findings"
+              # L2 failed. Restore engine's block if we snapshotted one; else
+              # clear any L2-authored block so a sidecar guardrail hit doesn't
+              # surface as the engine's own block on the primary review.
+              if [ -f "$POLICY_BLOCK_SNAPSHOT" ]; then
+                mv -f "$POLICY_BLOCK_SNAPSHOT" "$POLICY_BLOCK"
+              else
+                rm -f "$POLICY_BLOCK"
+              fi
             fi
+            unset POLICY_BLOCK_SNAPSHOT
             echo "::endgroup::"
           fi
           node "$CHECK" "$1" "$rc"

scripts/judge.mjs

@@ -17,11 +17,34 @@
 import fs from "node:fs";
 import os from "node:os";
 
+// Parses a strict plain decimal (e.g. "0.7", "-1", "0.5"). Returns the number
+// or NaN. Deliberately does NOT use parseFloat — parseFloat stops at the
+// first non-numeric character so "0.8oops" would silently become 0.8 and
+// "0x1" would become 0. The regex gate rejects any trailing garbage before
+// Number() coerces, so callers can trust the returned value is either a
+// clean decimal or NaN.
+function parseStrictDecimal(raw) {
+  if (typeof raw !== "string" || !/^\s*-?\d+(?:\.\d+)?\s*$/.test(raw)) return NaN;
+  return Number(raw);
+}
+
 const [file, ...rest] = process.argv.slice(2);
 let out = null, threshold = 0.7, modelOverride = null;
 for (let i = 0; i < rest.length; i += 1) {
   if (rest[i] === "--out") out = rest[++i];
-  else if (rest[i] === "--threshold") threshold = parseFloat(rest[++i]);
+  else if (rest[i] === "--threshold") {
+    const rawT = rest[++i];
+    const parsed = parseStrictDecimal(rawT);
+    if (!Number.isFinite(parsed)) {
+      console.error(`judge: --threshold must be a plain decimal, got ${JSON.stringify(rawT)}`);
+      process.exit(2);
+    }
+    if (parsed < 0 || parsed > 1) {
+      console.error(`judge: --threshold must be in [0,1], got ${JSON.stringify(rawT)}`);
+      process.exit(2);
+    }
+    threshold = parsed;
+  }
   else if (rest[i] === "--model") modelOverride = rest[++i];
 }
 if (!file) { console.error("usage: node judge.mjs <filtered.json> [--out f] [--threshold 0.7] [--model deepseek/deepseek-v4-pro]"); process.exit(2); }
@@ -86,8 +109,8 @@ const body = JSON.stringify({
   temperature: 0,
   // Scales with the finding count (~250 tokens per cluster JSON entry —
   // member_ids + representative_id + confidence + keep + root_cause + reason —
-  // plus a bit of overhead). 8k tokens truncated the response at 48 findings
-  // (observed on minimax runs on 80bffaa72); 32k gives headroom well past 100.
+  // plus a bit of overhead). 32k gives headroom well past 100 findings; 8k
+  // truncated the response on large batches in earlier testing.
   max_tokens: 32000,
   messages: [{ role: "system", content: system }, { role: "user", content: user }],
 });
@@ -112,18 +135,65 @@ catch (e) { console.error("judge did not return JSON:\n" + content.slice(0, 600)
 const groups = parsed.groups || [];
 const covered = new Set();
 for (const g of groups) for (const id of g.member_ids || []) covered.add(id);
+// Fail-open for findings the judge did not classify into any group: mark
+// them keep with a synthetic confidence that survives ANY user-set
+// threshold, so raising the threshold does not silently drop the judge's
+// blind spots (the previous 0.5 constant fails closed at threshold > 0.5,
+// contradicting the "fail-open" comment).
 for (let i = 0; i < findings.length; i += 1) if (!covered.has(i))
-  groups.push({ member_ids: [i], representative_id: i, confidence: 0.5, keep: true, root_cause: "(uncovered)", reason: "not classified by judge; kept fail-open" });
+  groups.push({ member_ids: [i], representative_id: i, confidence: 1.0, keep: true, root_cause: "(uncovered)", reason: "not classified by judge; kept fail-open above any threshold" });
 
 const kept = [];
 const dropped = [];
 for (const g of groups) {
-  const surv = g.keep && g.confidence >= threshold;
-  const rep = comments[g.representative_id] ?? comments[g.member_ids[0]];
-  const others = (g.member_ids || []).filter((id) => id !== (g.representative_id ?? g.member_ids[0]));
-  const line = `[conf ${g.confidence?.toFixed(2)}] ${(rep?.content || "").match(/\[(P[0-3])\]/)?.[0] || ""} ${rep?.path} :: ${g.root_cause} ${others.length ? "(merged " + others.length + ")" : ""}`;
-  if (surv) { kept.push(rep); console.error("KEEP  " + line); }
-  else { dropped.push(g); console.error("drop  " + line + " — " + (g.reason || "")); }
+  // Coerce confidence — the judge sometimes serializes it as a JSON string
+  // (e.g. "0.95") on schema drift. Type-guard first: `Number()` will happily
+  // return 1 for `true` and 0 for `false`/`null`/`[]`, and 1 for `[1]`, any of
+  // which would slip past ANY threshold and bypass the whole gate. Accept
+  // only real numbers or numeric strings; anything else becomes NaN and the
+  // group is dropped. Then reject values outside the documented [0,1] range
+  // outright rather than clamping — a stray `2` is a schema violation, and
+  // silently clamping it to `1` (max confidence) lets an invalid response
+  // survive every threshold. Match the invalid-type policy: violation → drop.
+  const rawConfInput = g.confidence;
+  const rawConf =
+    typeof rawConfInput === "number"
+      ? rawConfInput
+      : typeof rawConfInput === "string"
+        ? parseStrictDecimal(rawConfInput)
+        : NaN;
+  const boundedConf =
+    Number.isFinite(rawConf) && rawConf >= 0 && rawConf <= 1 ? rawConf : NaN;
+  // Coerce `keep` strictly — same schema-drift class as `confidence` above.
+  // Truthy JS treats the STRING `"false"` as true, so a raw `&& g.keep` would
+  // retain a group the judge intended to drop when the LLM stringifies the
+  // boolean. Accept only real `true` or the literal string "true" (case-
+  // insensitive); anything else drops the group as a safe default.
+  const keepInput = g.keep;
+  const groupKeep =
+    keepInput === true ||
+    (typeof keepInput === "string" && keepInput.trim().toLowerCase() === "true");
+  const surv = groupKeep && Number.isFinite(boundedConf) && boundedConf >= threshold;
+  // Resolve the representative comment by trying the primary id then every
+  // member_id — a malformed group whose representative_id is out of range
+  // must still surface a valid member, otherwise the coverage pass has
+  // already marked those members as "handled" and the whole group would
+  // vanish silently along with real findings.
+  const candidateIds = [g.representative_id, ...(Array.isArray(g.member_ids) ? g.member_ids : [])];
+  let repId = null;
+  for (const id of candidateIds) {
+    if (Number.isInteger(id) && id >= 0 && id < comments.length && comments[id]) { repId = id; break; }
+  }
+  const rep = repId != null ? comments[repId] : null;
+  const others = Array.isArray(g.member_ids)
+    ? g.member_ids.filter((id) => id !== repId)
+    : [];
+  const line = `[conf ${Number.isFinite(boundedConf) ? boundedConf.toFixed(2) : "?"}] ${(rep?.content || "").match(/\[(P[0-3])\]/)?.[0] || ""} ${rep?.path ?? "?"} :: ${g.root_cause} ${others.length ? "(merged " + others.length + ")" : ""}`;
+  if (surv) {
+    if (!rep) { console.error("skip malformed judge group (no valid rep): " + JSON.stringify(g).slice(0, 120)); continue; }
+    kept.push(rep);
+    console.error("KEEP  " + line);
+  } else { dropped.push(g); console.error("drop  " + line + " — " + (g.reason || "")); }
 }
 
 if (out) fs.writeFileSync(out, JSON.stringify({ ...data, comments: kept }, null, 1));

scripts/postfilter.mjs

@@ -24,8 +24,34 @@ if (!file || !repo || !commit) {
   process.exit(2);
 }
 
-const CODE_EXT = /\.(go|js|jsx|ts|tsx|mjs|cjs|py|java|rb|rs|c|h|cc|cpp|sql|sh)$/i;
+// Only drop findings whose snippet did NOT match anywhere in the tree when
+// the claimed path is a KNOWN non-code file (locale JSON / doc markdown /
+// changelogs / lockfiles / images / binaries). Extensionless code files
+// (Dockerfile, Makefile, scripts with no extension) previously fell through
+// the "not code-ext" branch and got dropped as if they were locale files —
+// reverse the polarity so only clearly-non-code paths incur the drop.
+//
+// Extensions that are ALWAYS non-code (docs, media, binaries, generated).
+const DEFINITELY_NON_CODE_EXT = /\.(md|txt|lock|log|csv|tsv|po|pot|properties|map|png|jpe?g|gif|svg|ico|webp|pdf|woff2?|ttf|eot|otf|zip|tar|gz|tgz|bin|exe|dll|so|dylib|class|jar|wasm|mp3|mp4|mov|wav)$/i;
+// JSON / YAML are ambiguous: action.yml, GitHub workflows, package.json,
+// tsconfig.json, k8s / IaC manifests are all reviewable configuration. Only
+// treat a .json / .yml / .yaml path as non-code when the path itself
+// signals "lockfile", "locale/translation bundle", or "generated build
+// output" — anything else keeps the finding for L2 to judge.
+const CONVENTIONAL_NON_CODE_JSON_YAML_PATH =
+  /(?:(?:^|\/)(?:package-lock|pnpm-lock)\.(?:json|ya?ml))|(?:[-.]lock\.(?:json|ya?ml)$)|(?:(?:^|\/)(?:locales?|i18n|translations|messages|dist|build|generated|out|coverage)\/)/i;
+function isKnownNonCode(p) {
+  if (!p) return false;
+  if (DEFINITELY_NON_CODE_EXT.test(p)) return true;
+  if (!/\.(?:json|ya?ml)$/i.test(p)) return false;
+  return CONVENTIONAL_NON_CODE_JSON_YAML_PATH.test(p);
+}
 
+// Returns a de-duped list of files containing `pattern` at the reviewed
+// commit. Uses `-l` (file-list only) so we sidestep the colon-in-filename
+// ambiguity of `-n`'s "<commit>:<file>:<line>:<text>" format — with `-l`
+// the output is just "<commit>:<file>" and stripping the commit prefix is
+// unambiguous.
 function grepFiles(pattern) {
   if (!pattern || pattern.length < 12) return [];
   try {
@@ -34,9 +60,34 @@ function grepFiles(pattern) {
       maxBuffer: 1 << 24,
     });
     return [...new Set(o.split("\n").filter(Boolean).map((l) => l.slice(l.indexOf(":") + 1)))];
-  } catch {
-    return []; // grep exit 1 = no match
-  }
+  } catch { return []; }
+}
+
+// Returns line numbers where `pattern` matches inside a specific `file` at
+// the commit. We pass `file` as an explicit git pathspec after `--`, so
+// git-grep's output has an exact `<commit>:<file>:` prefix we can strip
+// unambiguously — the colon-in-filename concern from raw `-n` output does
+// not apply here because we already know the file.
+function grepLinesIn(pattern, file) {
+  if (!pattern || pattern.length < 12) return [];
+  try {
+    const o = execFileSync(
+      "git",
+      ["-C", repo, "grep", "-F", "-I", "-n", "-e", pattern, commit, "--", file],
+      { encoding: "utf8", maxBuffer: 1 << 24 },
+    );
+    const prefix = `${commit}:${file}:`;
+    const lines = [];
+    for (const raw of o.split("\n")) {
+      if (!raw || !raw.startsWith(prefix)) continue;
+      const rest = raw.slice(prefix.length);
+      const colon = rest.indexOf(":");
+      if (colon < 0) continue;
+      const n = Number(rest.slice(0, colon));
+      if (Number.isFinite(n)) lines.push(n);
+    }
+    return lines;
+  } catch { return []; }
 }
 
 function candidateLines(code) {
@@ -54,25 +105,66 @@ const report = [];
 const kept = [];
 
 for (const c of comments) {
-  let trueFiles = [];
+  // Try each candidate line from `existing_code` until one has hits.
+  // Remember which candidate matched so we can look up its line numbers
+  // in the rehome target with a second targeted grep.
+  let files = [];
+  let matchedPattern = null;
   for (const ln of candidateLines(c.existing_code)) {
     const f = grepFiles(ln);
-    if (f.length) { trueFiles = f; break; }
+    if (f.length) { files = f; matchedPattern = ln; break; }
   }
   let path = c.path;
+  let start_line = c.start_line;
+  let end_line = c.end_line;
   let action = "keep";
-  if (trueFiles.length) {
-    if (trueFiles.includes(c.path)) action = "keep (correct)";
-    else if (trueFiles.length === 1) { path = trueFiles[0]; action = `REHOME ${c.path} -> ${path}`; }
-    else action = `keep (ambiguous: ${trueFiles.length} files)`;
-  } else if (!CODE_EXT.test(c.path)) {
-    action = "DROP (code snippet, filed on non-code file, not found)";
+  if (files.length) {
+    if (files.includes(c.path)) action = "keep (correct)";
+    else if (files.length === 1) {
+      // Single file elsewhere. Fetch the actual line NUMBERS in that file
+      // to update `start_line`/`end_line` on rehome — but only if the hit
+      // is unambiguous (exactly one line). Multiple hits in the same file
+      // mean the snippet appears more than once, so we cannot pick a
+      // single line to post on — treat as ambiguous and leave the finding
+      // on its original path.
+      const target = files[0];
+      const targetLines = matchedPattern ? grepLinesIn(matchedPattern, target) : [];
+      if (targetLines.length === 1) {
+        path = target;
+        start_line = targetLines[0];
+        end_line = targetLines[0];
+        action = `REHOME ${c.path} -> ${path}:${targetLines[0]}`;
+      } else if (targetLines.length > 1) {
+        // The snippet appears more than once in the target file so we cannot
+        // pick a single line to post on — but L1 has still proven the code
+        // lives in `target`, NOT in `c.path`. Rehome the path (dropping the
+        // now-inapplicable line) rather than leaving the finding on a
+        // known-wrong file just because the line is ambiguous.
+        path = target;
+        start_line = null;
+        end_line = null;
+        action = `REHOME ${c.path} -> ${path} (line ambiguous: ${targetLines.length} hits)`;
+      } else {
+        // Line lookup returned nothing (rare — `-l` said match exists).
+        // Rehome the path but clear the line: the stale line came from the
+        // wrong file (`c.path`) and would either post the comment on an
+        // unrelated line in `target`, or trip GitHub's inline-comment range
+        // validation and reject the whole review.
+        path = target;
+        start_line = null;
+        end_line = null;
+        action = `REHOME ${c.path} -> ${path} (line unresolved)`;
+      }
+    }
+    else action = `keep (ambiguous: ${files.length} files)`;
+  } else if (isKnownNonCode(c.path)) {
+    action = "DROP (code snippet, filed on known-non-code file, not found)";
   } else {
     action = "keep (snippet unverified)";
   }
   report.push({ sev: (c.content || "").match(/\[(P[0-3])\]/)?.[1] || "?", from: c.path, action });
   if (action.startsWith("DROP")) continue;
-  kept.push({ ...c, path });
+  kept.push({ ...c, path, start_line, end_line });
 }
 
 const norm = (s) =>