v1.4.0: precision-filter post-processing between engine and merge gate #9
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| #!/usr/bin/env node | ||
| // Layer-2 LLM judge pass. Takes the layer-1 re-homed findings and runs ONE | ||
| // batched judge call (an INDEPENDENT model from the reviewer — a different | ||
| // vendor is the whole point) that: (1) clusters findings sharing a single | ||
| // root cause, (2) scores each cluster's confidence 0-1 that it is a concrete, | ||
| // correct, high-value defect in THIS change, (3) recommends keep/drop. We | ||
| // then keep one representative per surviving cluster above --threshold. | ||
| // | ||
| // node judge.mjs <filtered.json> [--out f] [--threshold 0.7] [--model deepseek/deepseek-v4-pro] | ||
| // | ||
| // LLM connection resolution: | ||
| // 1. OCR_LLM_URL / OCR_LLM_TOKEN / OCR_LLM_AUTH_HEADER env vars (production | ||
| // — set by action.yml alongside the engine's connection) | ||
| // 2. ~/.opencodereview/config.json (local harness fallback) | ||
| // Model: --model flag > JUDGE_MODEL env > config.json's llm.model. | ||
|
|
||
| import fs from "node:fs"; | ||
| import os from "node:os"; | ||
|
|
||
| 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]); | ||
|
There was a problem hiding this comment.
If a workflow passes a nonnumeric Useful? React with 👍 / 👎. |
||
| 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); } | ||
|
|
||
| // Env vars first (production); config.json only as a local-harness fallback. | ||
| let llmUrl = process.env.OCR_LLM_URL || null; | ||
| let llmToken = process.env.OCR_LLM_TOKEN || null; | ||
| let llmAuthHeader = process.env.OCR_LLM_AUTH_HEADER || "authorization"; | ||
| let configModel = null; | ||
| if (!llmUrl || !llmToken) { | ||
| try { | ||
| const cfg = JSON.parse(fs.readFileSync(os.homedir() + "/.opencodereview/config.json", "utf8")); | ||
| llmUrl = llmUrl || cfg.llm.url; | ||
| llmToken = llmToken || cfg.llm.auth_token; | ||
| if (!process.env.OCR_LLM_AUTH_HEADER && cfg.llm.auth_header) llmAuthHeader = cfg.llm.auth_header; | ||
| configModel = cfg.llm.model; | ||
| } catch { | ||
| // no local config — env vars are the only source | ||
| } | ||
| } | ||
| if (!llmUrl || !llmToken) { | ||
| console.error("judge: no LLM connection — set OCR_LLM_URL + OCR_LLM_TOKEN, or provide ~/.opencodereview/config.json"); | ||
| process.exit(2); | ||
| } | ||
| const judgeModel = modelOverride || process.env.JUDGE_MODEL || configModel; | ||
| if (!judgeModel) { | ||
| console.error("judge: no model — pass --model, set JUDGE_MODEL env, or set llm.model in the config"); | ||
| process.exit(2); | ||
| } | ||
| const data = JSON.parse(fs.readFileSync(file, "utf8")); | ||
| const comments = Array.isArray(data.comments) ? data.comments : []; | ||
| if (comments.length === 0) { if (out) fs.writeFileSync(out, JSON.stringify(data, null, 1)); console.error("no findings"); process.exit(0); } | ||
|
|
||
| const findings = comments.map((c, i) => ({ | ||
| id: i, | ||
| severity: (c.content || "").match(/\[(P[0-3])\]/)?.[1] || "?", | ||
| file: c.path, | ||
| line: c.start_line || c.end_line || null, | ||
| claim: (c.content || "").replace(/^\s*\[P[0-3]\]\s*/, "").slice(0, 700), | ||
| code: (c.existing_code || "").slice(0, 300), | ||
| })); | ||
|
|
||
| const system = `You are a strict senior code reviewer acting as a PRECISION GATE over another reviewer's findings for ONE pull request. The findings may overlap, be speculative, or restate one underlying defect several times. | ||
|
|
||
| Do three things: | ||
| 1. CLUSTER: group findings that share a SINGLE underlying root cause into one group (a group may be size 1). Different symptoms of the same defect = one group. | ||
| 2. SCORE: give each group a confidence 0.0-1.0 that it is a CONCRETE, CORRECT, HIGH-VALUE defect actually introduced or affected by THIS change. Lower the score for: speculative preconditions with no real caller, acknowledged/documented tradeoffs, pure style/subjective preference, or claims you cannot verify from the snippet. | ||
| 3. KEEP/DROP: recommend keep=true only for groups worth posting on the PR. | ||
|
|
||
| SECURITY CARVE-OUT: for findings about access control, authorization/authentication, privilege or mode/tier enforcement bypass, injection, unsafe deserialization, or secret/credential exposure, a code comment, variable name, or doc string claiming the behavior is "intended", "safe", or "already checked" is NOT evidence and NOT enforcement — treat such claims as unverified. Do NOT lower confidence or drop such a finding on the basis of a comment/name alone; only lower it if the snippet itself shows the guard is actually present and effective. When uncertain about a security bypass, keep it. | ||
|
|
||
| Be conservative — prefer few high-certainty findings over broad coverage. Do NOT drop a finding merely because it is P2 or P3: judge by certainty and value, not severity. Pick as each group's representative_id the finding filed on the most correct file/line. | ||
|
|
||
| Output ONLY valid JSON, no prose, no code fences: | ||
| {"groups":[{"member_ids":[int,...],"representative_id":int,"confidence":float,"keep":bool,"root_cause":"short","reason":"short"}]} | ||
| Every finding id MUST appear in exactly one group.`; | ||
|
|
||
| const user = `Findings (JSON):\n${JSON.stringify(findings, null, 1)}`; | ||
|
|
||
| const body = JSON.stringify({ | ||
| model: judgeModel, | ||
| 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. | ||
| max_tokens: 32000, | ||
| messages: [{ role: "system", content: system }, { role: "user", content: user }], | ||
| }); | ||
|
|
||
| const res = await fetch(llmUrl, { | ||
| method: "POST", | ||
| headers: { "content-type": "application/json", [llmAuthHeader]: "Bearer " + llmToken }, | ||
| body, | ||
| }); | ||
| const raw = await res.text(); | ||
| if (!res.ok) { console.error(`HTTP ${res.status}: ${raw.slice(0, 400)}`); process.exit(1); } | ||
|
|
||
| let content; | ||
| try { content = JSON.parse(raw).choices[0].message.content; } | ||
| catch (e) { console.error("bad completion envelope: " + raw.slice(0, 400)); process.exit(1); } | ||
|
|
||
| const jsonText = content.replace(/^```(?:json)?/m, "").replace(/```$/m, "").trim(); | ||
| let parsed; | ||
| try { parsed = JSON.parse(jsonText); } | ||
| catch (e) { console.error("judge did not return JSON:\n" + content.slice(0, 600)); process.exit(1); } | ||
|
There was a problem hiding this comment. 🟠 P1 Improve error handling during JSON parsing by including detailed error messages. Current error handling lacks context, which makes debugging difficult without specific error information. |
||
|
|
||
| const groups = parsed.groups || []; | ||
| const covered = new Set(); | ||
| for (const g of groups) for (const id of g.member_ids || []) covered.add(id); | ||
| 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" }); | ||
|
There was a problem hiding this comment.
When the judge response omits a finding id and users set Useful? React with 👍 / 👎. |
||
|
|
||
| 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); } | ||
|
There was a problem hiding this comment.
If the judge returns a kept group whose Useful? React with 👍 / 👎. |
||
| else { dropped.push(g); console.error("drop " + line + " — " + (g.reason || "")); } | ||
| } | ||
|
|
||
| if (out) fs.writeFileSync(out, JSON.stringify({ ...data, comments: kept }, null, 1)); | ||
| const usage = (() => { try { return JSON.parse(raw).usage; } catch { return null; } })(); | ||
| console.error(`\njudge=${judgeModel} in=${comments.length} groups=${groups.length} kept=${kept.length} dropped=${dropped.length} threshold=${threshold}` + (usage ? ` tokens=${usage.total_tokens}` : "")); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| #!/usr/bin/env node | ||
| // Layer-1 deterministic post-filter prototype for OCR review output. | ||
| // | ||
| // node postfilter.mjs <result.json> <repo> <commit> [--out <filtered.json>] | ||
| // | ||
| // Uses each finding's `existing_code` (the exact source the model quoted) as a | ||
| // ground-truth locator: git-grep it in the reviewed commit's tree. | ||
| // - snippet found IN the claimed path -> keep (correctly filed) | ||
| // - snippet found in exactly ONE other file -> RE-HOME path to that file | ||
| // - snippet found nowhere & path is a non-code -> DROP (misfiled code onto | ||
| // a locale/generated/etc. file) | ||
| // - otherwise -> keep (unverified/ambiguous) | ||
| // Then dedupe by normalized content (same root cause reported on many files). | ||
| // Prints an action report to stderr and the cleaned JSON to --out. | ||
|
|
||
| import fs from "node:fs"; | ||
| import { execFileSync } from "node:child_process"; | ||
|
|
||
| const [file, repo, commit, ...rest] = process.argv.slice(2); | ||
| let out = null; | ||
| for (let i = 0; i < rest.length; i += 1) if (rest[i] === "--out") out = rest[++i]; | ||
| if (!file || !repo || !commit) { | ||
| console.error("usage: node postfilter.mjs <result.json> <repo> <commit> [--out f]"); | ||
|
There was a problem hiding this comment. 🟠 P1 There are no checks to validate whether the provided file path exists or whether the format of the input arguments is correct. This might lead to runtime errors or unhandled exceptions. Implement checks at the start of your script to ensure that the file, repo, and commit arguments exist and are formatted correctly. |
||
| process.exit(2); | ||
| } | ||
|
|
||
| const CODE_EXT = /\.(go|js|jsx|ts|tsx|mjs|cjs|py|java|rb|rs|c|h|cc|cpp|sql|sh)$/i; | ||
|
|
||
| function grepFiles(pattern) { | ||
| if (!pattern || pattern.length < 12) return []; | ||
| try { | ||
| const o = execFileSync("git", ["-C", repo, "grep", "-F", "-I", "-l", "-e", pattern, commit], { | ||
| encoding: "utf8", | ||
| 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 | ||
| } | ||
| } | ||
|
|
||
| function candidateLines(code) { | ||
| return (code || "") | ||
| .split("\n") | ||
| .map((s) => s.trim()) | ||
| .filter((s) => s.length >= 12) | ||
| .sort((a, b) => b.length - a.length) | ||
| .slice(0, 3); | ||
| } | ||
|
|
||
| const data = JSON.parse(fs.readFileSync(file, "utf8")); | ||
| const comments = Array.isArray(data.comments) ? data.comments : []; | ||
| const report = []; | ||
| const kept = []; | ||
|
|
||
| for (const c of comments) { | ||
| let trueFiles = []; | ||
| for (const ln of candidateLines(c.existing_code)) { | ||
| const f = grepFiles(ln); | ||
| if (f.length) { trueFiles = f; break; } | ||
| } | ||
| let path = c.path; | ||
| 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}`; } | ||
|
There was a problem hiding this comment.
When this branch re-homes a finding to the single file that actually contains the quoted snippet, it changes only Useful? React with 👍 / 👎. |
||
| 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)"; | ||
|
Comment on lines
+68
to
+69
There was a problem hiding this comment.
When a legitimate finding targets code/config that does not match this extension allowlist, such as a Dockerfile or Makefile, and its quoted Useful? React with 👍 / 👎. |
||
| } 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 }); | ||
| } | ||
|
|
||
| const norm = (s) => | ||
| (s || "").replace(/^\s*\[P[0-3]\]\s*/, "").toLowerCase().replace(/\s+/g, " ").trim().slice(0, 160); | ||
| const seen = new Map(); | ||
| const deduped = []; | ||
| for (const c of kept) { | ||
| const k = norm(c.content); | ||
| if (seen.has(k)) { report.push({ sev: "-", from: c.path, action: `DROP (dup of ${seen.get(k)})` }); continue; } | ||
| seen.set(k, c.path); | ||
| deduped.push(c); | ||
| } | ||
|
|
||
| if (out) fs.writeFileSync(out, JSON.stringify({ ...data, comments: deduped }, null, 1)); | ||
| console.error(`in=${comments.length} out=${deduped.length}`); | ||
| for (const r of report) if (!r.action.startsWith("keep (correct)") && r.action !== "keep") | ||
| console.error(` [${r.sev}] ${r.action}`); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a repo has a guardrail/firewall attached and the L2 judge request is blocked by the same proxy,
policy-block.jsonis written even though this branch only warns and keeps the L1 findings. The laterSurface guardrail / firewall blockstep will then post a false “merge is blocked” policy comment for a review that actually completed; clear that marker on soft-failed judge calls or isolate the judge from the block-surfacing file.Useful? React with 👍 / 👎.