From 9331158d572aa2552d5e8725c4d6e74c02ef9587 Mon Sep 17 00:00:00 2001
From: Aashil <bijuksheashil@gmail.com>
Date: Fri, 9 Oct 2026 14:35:34 +0545
Subject: [PATCH 1/7] tune(qa-review): scale findings to change size, tighten
 prompt, allowlist wpeverest/user-registration for public dry runs

---
 config/qa-review-repos.json    |  4 +++-
 prompts/qa-review.md           | 10 ++++++----
 scripts/qa-review-render.mjs   | 19 ++++++++++++++----
 scripts/qa-review-run.mjs      |  8 +++++---
 scripts/qa-review-run.test.mjs | 36 ++++++++++++++++++++++++++++++++++
 5 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/config/qa-review-repos.json b/config/qa-review-repos.json
index 9c39917..40825f4 100644
--- a/config/qa-review-repos.json
+++ b/config/qa-review-repos.json
@@ -1,5 +1,7 @@
 {
-  "wpeverest": [],
+  "wpeverest": [
+    "user-registration"
+  ],
   "themegrill": [
     "user-registration-pro"
   ]
diff --git a/prompts/qa-review.md b/prompts/qa-review.md
index b85321b..5073594 100644
--- a/prompts/qa-review.md
+++ b/prompts/qa-review.md
@@ -14,11 +14,13 @@ PHPCS/coding-standards, formatting, and the end-to-end suite run separately. Do
 - Security-relevant changes: missing nonce/capability checks, unescaped output, unprepared SQL, in code the PR adds or changes.
 - Behaviour a user can see that has no existing test case.
 
-# Evidence rules (strict, enforced by a script after you answer)
+# Evidence rules (strict, partly enforced by a script after you answer)
 
 - A "risk" MUST cite a `file` from the diff and a `line` number that is an ADDED line, i.e. one marked `L<number>+` in the diff. Use that exact number. Findings that cite anything else are discarded.
-- A "manual_test" MUST be copied VERBATIM from `<existing_test_cases>`. Titles not in that list are discarded. Only choose cases genuinely affected by the change (maximum 8).
-- Say what you can see in the diff. Do not claim a bug unless the changed lines show it. If it depends on code you cannot see, say "depends on code outside this diff" in `evidence`, or leave it out. Prefer fewer, solid findings over many weak ones. Empty lists are a good answer for a clean change.
+- A risk must describe a CONCRETE failure that the changed lines themselves show: what input or situation breaks, and why. Do NOT write "if X relies on Y" or "could potentially" about code you cannot see. Do NOT raise anything the PR description already explains or answers. If you would have to guess about code outside the diff, leave it out. An empty `risks` list is the correct answer for most small, clean fixes.
+- A "manual_test" MUST be copied VERBATIM from `<existing_test_cases>`, and must test the SAME feature the PR changes. A test for a different feature that merely shares a page, a shortcode or a screen is wrong: leave it out. No test is better than a loosely related one.
+- A "new_scenario" is for user-visible behaviour with NO existing test case. Do NOT repeat anything already listed in the PR description's own testing steps; the author has covered those. Do not pad: if you have nothing the PR author has not already covered, return none.
+- Match the size of your answer to the size of the change. A one-to-ten line change normally deserves a short summary and few or no findings. More findings is not better.
 - Never state a confidence number.
 
 # Output
@@ -38,4 +40,4 @@ Return ONLY a JSON object, no markdown fences, exactly this shape:
   ]
 }
 
-Limits: at most 8 risks, 8 manual_tests, 5 new_scenarios. Keep every string under 300 characters.
+Hard maximums are 8 risks, 8 manual_tests, 5 new_scenarios, and a script trims further for small changes. Keep every string under 300 characters.
diff --git a/scripts/qa-review-render.mjs b/scripts/qa-review-render.mjs
index f2c8d89..10a9def 100644
--- a/scripts/qa-review-render.mjs
+++ b/scripts/qa-review-render.mjs
@@ -27,9 +27,20 @@ export function sanitize(value, max = 300) {
 
 const asArray = (v) => (Array.isArray(v) ? v : []);
 
-// ctx: { addedByFile: {file: Set<line>}, titles: [{title,file}] }
+// How many findings a change of this size can plausibly justify. Enforced here
+// in code because a prompt-only limit was ignored: a ONE-line PR got 1 risk,
+// 1 test and 3 scenarios (user-registration-pro#1610 dry run).
+export function limitsFor(changedLines) {
+  if (changedLines <= 10) return { risks: 1, manual: 2, scenarios: 1 };
+  if (changedLines <= 60) return { risks: 3, manual: 4, scenarios: 2 };
+  if (changedLines <= 300) return { risks: 5, manual: 6, scenarios: 3 };
+  return { risks: 8, manual: 8, scenarios: 5 };
+}
+
+// ctx: { addedByFile: {file: Set<line>}, titles: [{title,file}], limits?: {risks,manual,scenarios} }
 export function validateReview(raw, ctx) {
   const data = raw && typeof raw === "object" ? raw : {};
+  const limits = ctx.limits ?? { risks: 8, manual: 8, scenarios: 5 };
   const known = new Map(ctx.titles.map((t) => [t.title.trim(), t]));
   const dropped = { risks: 0, manual_tests: 0, new_scenarios: 0 };
 
@@ -41,7 +52,7 @@ export function validateReview(raw, ctx) {
       dropped.risks++;
       continue;
     }
-    if (risks.length < 8) risks.push({ file: r.file, line, claim: sanitize(r.claim), evidence: sanitize(r.evidence) });
+    if (risks.length < limits.risks) risks.push({ file: r.file, line, claim: sanitize(r.claim), evidence: sanitize(r.evidence) });
   }
 
   const manual = [];
@@ -53,7 +64,7 @@ export function validateReview(raw, ctx) {
       continue;
     }
     seen.add(key);
-    if (manual.length < 8) manual.push({ title: key, file: known.get(key).file, why: sanitize(t.why) });
+    if (manual.length < limits.manual) manual.push({ title: key, file: known.get(key).file, why: sanitize(t.why) });
   }
 
   const scenarios = [];
@@ -62,7 +73,7 @@ export function validateReview(raw, ctx) {
       dropped.new_scenarios++;
       continue;
     }
-    if (scenarios.length < 5) scenarios.push({ scenario: sanitize(s.scenario), why: sanitize(s.why) });
+    if (scenarios.length < limits.scenarios) scenarios.push({ scenario: sanitize(s.scenario), why: sanitize(s.why) });
   }
 
   return { summary: sanitize(data.summary, 700), risks, manual, scenarios, dropped };
diff --git a/scripts/qa-review-run.mjs b/scripts/qa-review-run.mjs
index 3b443be..e8d34be 100644
--- a/scripts/qa-review-run.mjs
+++ b/scripts/qa-review-run.mjs
@@ -6,7 +6,7 @@ import { fileURLToPath } from "node:url";
 import { ghTokenForRepo } from "./github-client.mjs";
 import { chatJSONWithUsage } from "./openai-client.mjs";
 import { buildDiff, isTestFile, loadQaData, makeReader, mapAreas } from "./qa-review-context.mjs";
-import { renderComment, validateReview } from "./qa-review-render.mjs";
+import { limitsFor, renderComment, validateReview } from "./qa-review-render.mjs";
 
 const TAGS = "pr_title|pr_body|diff|knowledge|existing_test_cases|other_checks|areas_touched";
 
@@ -58,7 +58,9 @@ export async function runReview({ repo, prNumber, expectedSha, reader, chat, sys
 
   const { data, usage } = await chat(systemPrompt, buildUserMessage({ pr, areas, checks, qa, diff }), null);
   if (!data || typeof data !== "object") throw new Error("Model returned unparseable output; nothing to post.");
-  const review = validateReview(data, { addedByFile: diff.addedByFile, titles: qa.titles });
+  // Size = what the model was actually shown, so a huge PR with a tiny shown part isn't over-credited.
+  const changedLines = files.filter((f) => diff.shown.includes(f.filename)).reduce((n, f) => n + (f.additions ?? 0) + (f.deletions ?? 0), 0);
+  const review = validateReview(data, { addedByFile: diff.addedByFile, titles: qa.titles, limits: limitsFor(changedLines) });
   // An empty summary means the model refused or derailed. Posting "no risks found"
   // from that would read as a clean bill of health, which is the wrong failure mode.
   if (!review.summary) throw new Error("Model returned no usable summary; refusing to post an empty review.");
@@ -83,7 +85,7 @@ export async function runReview({ repo, prNumber, expectedSha, reader, chat, sys
   // Fail safe: only an explicit `false` counts as public. The review is analysis
   // of the repo's code, and this workflow's own run summary/logs are public.
   const isPrivate = pr.base?.repo?.private !== false;
-  return { body, review, usage, isPrivate };
+  return { body, review, usage, isPrivate, changedLines };
 }
 
 async function main() {
diff --git a/scripts/qa-review-run.test.mjs b/scripts/qa-review-run.test.mjs
index bbf03e1..4c97cbb 100644
--- a/scripts/qa-review-run.test.mjs
+++ b/scripts/qa-review-run.test.mjs
@@ -197,3 +197,39 @@ test("renderComment output never contains the sticky marker (only the poster add
   });
   assert.ok(!body.includes(MARKER));
 });
+
+test("limitsFor scales with change size; validateReview enforces it", async () => {
+  const { limitsFor } = await import("./qa-review-render.mjs");
+  assert.deepEqual(limitsFor(1), { risks: 1, manual: 2, scenarios: 1 });
+  assert.deepEqual(limitsFor(10), { risks: 1, manual: 2, scenarios: 1 });
+  assert.deepEqual(limitsFor(11), { risks: 3, manual: 4, scenarios: 2 });
+  assert.deepEqual(limitsFor(61), { risks: 5, manual: 6, scenarios: 3 });
+  assert.deepEqual(limitsFor(5000), { risks: 8, manual: 8, scenarios: 5 });
+
+  const many = {
+    summary: "s",
+    risks: [11, 12].map((line) => ({ file: "includes/a.php", line, claim: `c${line}` })),
+    manual_tests: [{ title: "Verify login" }, { title: "Verify login" }],
+    new_scenarios: [{ scenario: "a" }, { scenario: "b" }, { scenario: "c" }],
+  };
+  const tiny = validateReview(many, { ...CTX, limits: limitsFor(1) });
+  assert.deepEqual([tiny.risks.length, tiny.manual.length, tiny.scenarios.length], [1, 1, 1]);
+  const none = validateReview(many, CTX); // no limits given => generous defaults
+  assert.deepEqual([none.risks.length, none.scenarios.length], [2, 3]);
+});
+
+test("runReview applies the size limit from the shown files' changed lines", async () => {
+  const chat = async () => ({
+    data: {
+      summary: "s",
+      risks: [11, 12].map((line) => ({ file: "includes/a.php", line, claim: `c${line}` })),
+      manual_tests: [],
+      new_scenarios: [{ scenario: "a" }, { scenario: "b" }, { scenario: "c" }],
+    },
+    usage: {},
+  });
+  const r = await runReview(args({ chat, reader: fakeReader({ listFiles: async () => [{ filename: "includes/a.php", status: "modified", patch: PATCH, additions: 2, deletions: 1 }] }) }));
+  assert.equal(r.changedLines, 3);
+  assert.equal(r.review.risks.length, 1);
+  assert.equal(r.review.scenarios.length, 1);
+});

From c0a3b488abb0c7956af673d880fa1fae86ef0351 Mon Sep 17 00:00:00 2001
From: Aashil <bijuksheashil@gmail.com>
Date: Fri, 9 Oct 2026 14:37:09 +0545
Subject: [PATCH 2/7] qa-review: enroll themegrill/user-registration (public;
 repo moved from wpeverest) instead of the wpeverest name

---
 config/qa-review-repos.json | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/config/qa-review-repos.json b/config/qa-review-repos.json
index 40825f4..831f5aa 100644
--- a/config/qa-review-repos.json
+++ b/config/qa-review-repos.json
@@ -1,8 +1,7 @@
 {
-  "wpeverest": [
-    "user-registration"
-  ],
+  "wpeverest": [],
   "themegrill": [
+    "user-registration",
     "user-registration-pro"
   ]
 }

From 856ad1033aaa3a9274fbd9a8ee7ef3c253c1992c Mon Sep 17 00:00:00 2001
From: Aashil <bijuksheashil@gmail.com>
Date: Fri, 9 Oct 2026 14:38:25 +0545
Subject: [PATCH 3/7] qa-review: also print the review to the job log for
 public target repos

---
 .github/workflows/qa-review.yml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/qa-review.yml b/.github/workflows/qa-review.yml
index 8242f23..6339389 100644
--- a/.github/workflows/qa-review.yml
+++ b/.github/workflows/qa-review.yml
@@ -151,7 +151,8 @@ jobs:
       # to the PR itself (post_comment). Only an explicit "false" counts as public.
       - name: Show result in the run summary (public target repos only)
         if: steps.review.outcome == 'success' && steps.review.outputs.repo_private == 'false'
-        run: cat "$QA_BODY_OUT" >> "$GITHUB_STEP_SUMMARY"
+        # tee: also in the job log, which (unlike the summary) can be read via the API/CLI.
+        run: tee -a "$GITHUB_STEP_SUMMARY" < "$QA_BODY_OUT"
 
       - name: Note that the result is withheld from the run summary
         if: steps.review.outcome == 'success' && steps.review.outputs.repo_private != 'false'

From 90f702f17783ce809644c729a288fa09b2cd7959 Mon Sep 17 00:00:00 2001
From: Aashil <bijuksheashil@gmail.com>
Date: Fri, 9 Oct 2026 14:41:46 +0545
Subject: [PATCH 4/7] qa-review: log why suggestions were discarded (public
 repos only); don't report all-dropped risks as clean

---
 scripts/qa-review-render.mjs   | 15 +++++++++++--
 scripts/qa-review-run.mjs      |  2 ++
 scripts/qa-review-run.test.mjs | 39 ++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/scripts/qa-review-render.mjs b/scripts/qa-review-render.mjs
index 10a9def..90e9fe5 100644
--- a/scripts/qa-review-render.mjs
+++ b/scripts/qa-review-render.mjs
@@ -43,13 +43,19 @@ export function validateReview(raw, ctx) {
   const limits = ctx.limits ?? { risks: 8, manual: 8, scenarios: 5 };
   const known = new Map(ctx.titles.map((t) => [t.title.trim(), t]));
   const dropped = { risks: 0, manual_tests: 0, new_scenarios: 0 };
+  // Why each item was discarded, for debugging. Contains model/PR-derived text,
+  // so the caller must only ever print it for explicitly public repos.
+  const detail = [];
 
   const risks = [];
   for (const r of asArray(data.risks)) {
     const line = Number(r?.line);
     const lines = ctx.addedByFile[r?.file];
-    if (!lines || !Number.isInteger(line) || !lines.has(line) || !sanitize(r.claim)) {
+    const why = !lines ? "file-not-in-shown-diff" : !Number.isInteger(line) ? "line-not-an-integer" : !lines.has(line) ? "line-not-an-added-line" : !sanitize(r.claim) ? "empty-claim" : null;
+    if (why) {
       dropped.risks++;
+      const added = lines ? [...lines].sort((a, b) => a - b) : [];
+      detail.push({ kind: "risk", why, file: String(r?.file).slice(0, 120), line: r?.line, addedRange: added.length ? [added[0], added[added.length - 1], added.length] : null });
       continue;
     }
     if (risks.length < limits.risks) risks.push({ file: r.file, line, claim: sanitize(r.claim), evidence: sanitize(r.evidence) });
@@ -61,6 +67,7 @@ export function validateReview(raw, ctx) {
     const key = typeof t?.title === "string" ? t.title.trim() : "";
     if (!known.has(key) || seen.has(key)) {
       dropped.manual_tests++;
+      detail.push({ kind: "manual_test", why: seen.has(key) ? "duplicate-title" : "title-not-in-index", title: key.slice(0, 120) });
       continue;
     }
     seen.add(key);
@@ -71,12 +78,13 @@ export function validateReview(raw, ctx) {
   for (const s of asArray(data.new_scenarios)) {
     if (!sanitize(s?.scenario)) {
       dropped.new_scenarios++;
+      detail.push({ kind: "scenario", why: "empty-scenario" });
       continue;
     }
     if (scenarios.length < limits.scenarios) scenarios.push({ scenario: sanitize(s.scenario), why: sanitize(s.why) });
   }
 
-  return { summary: sanitize(data.summary, 700), risks, manual, scenarios, dropped };
+  return { summary: sanitize(data.summary, 700), risks, manual, scenarios, dropped, detail };
 }
 
 const STATE_ICON = { success: "✅", failure: "❌", cancelled: "⚪", skipped: "⚪", neutral: "⚪", timed_out: "❌", action_required: "⚠️" };
@@ -117,6 +125,9 @@ export function renderComment({ review, facts }) {
   L.push("### Possible risks in the changed lines");
   if (review.risks.length) {
     for (const r of review.risks) L.push(`- \`${sanitize(r.file, 200)}:${r.line}\`: ${r.claim}${r.evidence ? ` _(${r.evidence})_` : ""}`);
+  } else if (review.dropped.risks > 0) {
+    // Not "none found": the model raised something and it failed verification.
+    L.push(`_The model raised ${review.dropped.risks} possible issue(s) that could not be tied to a changed line in the diff, so none are shown. This is not a clean result; review the change by hand._`);
   } else {
     L.push("_None found that could be tied to a specific changed line._");
   }
diff --git a/scripts/qa-review-run.mjs b/scripts/qa-review-run.mjs
index e8d34be..1d678ae 100644
--- a/scripts/qa-review-run.mjs
+++ b/scripts/qa-review-run.mjs
@@ -85,6 +85,8 @@ export async function runReview({ repo, prNumber, expectedSha, reader, chat, sys
   // Fail safe: only an explicit `false` counts as public. The review is analysis
   // of the repo's code, and this workflow's own run summary/logs are public.
   const isPrivate = pr.base?.repo?.private !== false;
+  // Discard reasons quote model/PR-derived text: public repos only, never private.
+  if (!isPrivate && review.detail.length) console.log(`Discarded suggestions: ${JSON.stringify(review.detail)}`);
   return { body, review, usage, isPrivate, changedLines };
 }
 
diff --git a/scripts/qa-review-run.test.mjs b/scripts/qa-review-run.test.mjs
index 4c97cbb..5ef421b 100644
--- a/scripts/qa-review-run.test.mjs
+++ b/scripts/qa-review-run.test.mjs
@@ -233,3 +233,42 @@ test("runReview applies the size limit from the shown files' changed lines", asy
   assert.equal(r.review.risks.length, 1);
   assert.equal(r.review.scenarios.length, 1);
 });
+
+test("discard reasons are recorded, and an all-dropped risk list is not reported as clean", () => {
+  const r = validateReview(
+    {
+      summary: "s",
+      risks: [
+        { file: "nope.php", line: 1, claim: "c" },
+        { file: "includes/a.php", line: 10, claim: "context line" },
+        { file: "includes/a.php", line: "x", claim: "c" },
+      ],
+      manual_tests: [{ title: "Invented" }],
+    },
+    CTX
+  );
+  assert.deepEqual(r.detail.map((d) => d.why), ["file-not-in-shown-diff", "line-not-an-added-line", "line-not-an-integer", "title-not-in-index"]);
+  assert.deepEqual(r.detail[1].addedRange, [11, 12, 2]); // lets us see the model was near the real lines
+  const body = renderComment({
+    review: r,
+    facts: { headSha: SHA, areas: {}, unmapped: [], checks: [], shown: [], omitted: [], noPatch: [], testsTouched: [], qaDataPresent: true, usage: null, model: "m" },
+  });
+  assert.match(body, /raised 3 possible issue\(s\) that could not be tied/);
+  assert.ok(!body.includes("None found"));
+});
+
+test("runReview prints discard details only for explicitly public repos", async () => {
+  const bad = async () => ({ data: { summary: "s", risks: [{ file: "includes/a.php", line: 999, claim: "c" }] }, usage: {} });
+  const withRepo = (repo) => fakeReader({ getPr: async () => ({ title: "T", body: "", head: { sha: SHA }, base: { ref: "develop", repo } }) });
+  const logs = [];
+  const orig = console.log;
+  console.log = (m) => logs.push(String(m));
+  try {
+    await runReview(args({ chat: bad, reader: withRepo({ private: true }) }));
+    assert.equal(logs.filter((l) => l.startsWith("Discarded")).length, 0);
+    await runReview(args({ chat: bad, reader: withRepo({ private: false }) }));
+    assert.equal(logs.filter((l) => l.startsWith("Discarded")).length, 1);
+  } finally {
+    console.log = orig;
+  }
+});

From 1059fdd65ebce21f8e466c0f1c4d5fe93230ec4d Mon Sep 17 00:00:00 2001
From: Aashil <bijuksheashil@gmail.com>
Date: Fri, 9 Oct 2026 14:47:15 +0545
Subject: [PATCH 5/7] qa-review: enforce the output schema via structured
 outputs (model sometimes dropped the test title key)

---
 scripts/openai-client.mjs      |  7 +++++--
 scripts/qa-review-run.mjs      | 13 ++++++++++++-
 scripts/qa-review-run.test.mjs | 19 ++++++++++++++++++-
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/scripts/openai-client.mjs b/scripts/openai-client.mjs
index 202cd38..f3f6142 100644
--- a/scripts/openai-client.mjs
+++ b/scripts/openai-client.mjs
@@ -8,7 +8,10 @@ export async function chatJSON(systemPrompt, userContent, fallback) {
 }
 
 // Same call, plus the token usage OpenAI reports -- feeds the event log.
-export async function chatJSONWithUsage(systemPrompt, userContent, fallback) {
+// responseFormat defaults to loose JSON mode (existing callers unchanged). Pass
+// {type:"json_schema", json_schema:{name, strict:true, schema}} to make the API
+// enforce the shape -- loose mode lets the model rename keys (seen in QA review).
+export async function chatJSONWithUsage(systemPrompt, userContent, fallback, responseFormat = { type: "json_object" }) {
   if (!OPENAI_API_KEY || !CLASSIFY_MODEL) {
     throw new Error("Missing required env var: OPENAI_API_KEY or CLASSIFY_MODEL");
   }
@@ -20,7 +23,7 @@ export async function chatJSONWithUsage(systemPrompt, userContent, fallback) {
     },
     body: JSON.stringify({
       model: CLASSIFY_MODEL,
-      response_format: { type: "json_object" },
+      response_format: responseFormat,
       messages: [
         { role: "system", content: systemPrompt },
         { role: "user", content: userContent },
diff --git a/scripts/qa-review-run.mjs b/scripts/qa-review-run.mjs
index 1d678ae..e3f6b12 100644
--- a/scripts/qa-review-run.mjs
+++ b/scripts/qa-review-run.mjs
@@ -8,6 +8,17 @@ import { chatJSONWithUsage } from "./openai-client.mjs";
 import { buildDiff, isTestFile, loadQaData, makeReader, mapAreas } from "./qa-review-context.mjs";
 import { limitsFor, renderComment, validateReview } from "./qa-review-render.mjs";
 
+const str = { type: "string" };
+const obj = (properties) => ({ type: "object", properties, required: Object.keys(properties), additionalProperties: false });
+// Strict structured output. Keep in sync with the "Output" section of prompts/qa-review.md.
+export const REVIEW_SCHEMA = obj({
+  summary: str,
+  risks: { type: "array", items: obj({ file: str, line: { type: "integer" }, claim: str, evidence: str }) },
+  manual_tests: { type: "array", items: obj({ title: str, why: str }) },
+  new_scenarios: { type: "array", items: obj({ scenario: str, why: str }) },
+});
+export const REVIEW_FORMAT = { type: "json_schema", json_schema: { name: "qa_review", strict: true, schema: REVIEW_SCHEMA } };
+
 const TAGS = "pr_title|pr_body|diff|knowledge|existing_test_cases|other_checks|areas_touched";
 
 // Untrusted text must not be able to close one of our data blocks early.
@@ -56,7 +67,7 @@ export async function runReview({ repo, prNumber, expectedSha, reader, chat, sys
     throw new Error("No reviewable diff: every changed file was skipped, binary, or over the size limit.");
   }
 
-  const { data, usage } = await chat(systemPrompt, buildUserMessage({ pr, areas, checks, qa, diff }), null);
+  const { data, usage } = await chat(systemPrompt, buildUserMessage({ pr, areas, checks, qa, diff }), null, REVIEW_FORMAT);
   if (!data || typeof data !== "object") throw new Error("Model returned unparseable output; nothing to post.");
   // Size = what the model was actually shown, so a huge PR with a tiny shown part isn't over-credited.
   const changedLines = files.filter((f) => diff.shown.includes(f.filename)).reduce((n, f) => n + (f.additions ?? 0) + (f.deletions ?? 0), 0);
diff --git a/scripts/qa-review-run.test.mjs b/scripts/qa-review-run.test.mjs
index 5ef421b..a35db8f 100644
--- a/scripts/qa-review-run.test.mjs
+++ b/scripts/qa-review-run.test.mjs
@@ -2,7 +2,7 @@ import { test } from "node:test";
 import assert from "node:assert/strict";
 import { globToRegExp, mapAreas, annotatePatch, buildDiff, isSkipped } from "./qa-review-context.mjs";
 import { sanitize, validateReview, renderComment } from "./qa-review-render.mjs";
-import { escapeTags, runReview, buildUserMessage } from "./qa-review-run.mjs";
+import { escapeTags, runReview, buildUserMessage, REVIEW_FORMAT, REVIEW_SCHEMA } from "./qa-review-run.mjs";
 import { MARKER } from "./qa-review-comment.mjs";
 
 const PATCH = ["@@ -10,3 +10,4 @@ function x() {", " keep", "-old", "+new1", "+new2", " tail"].join("\n");
@@ -272,3 +272,20 @@ test("runReview prints discard details only for explicitly public repos", async
     console.log = orig;
   }
 });
+
+test("model call uses a strict schema whose keys match what the validator reads", async () => {
+  let seen;
+  await runReview(args({ chat: async (s, u, f, fmt) => ((seen = fmt), goodModel()) }));
+  assert.equal(seen, REVIEW_FORMAT);
+  assert.equal(REVIEW_FORMAT.json_schema.strict, true);
+  const top = REVIEW_SCHEMA;
+  assert.deepEqual(top.required.sort(), ["manual_tests", "new_scenarios", "risks", "summary"]);
+  assert.equal(top.additionalProperties, false);
+  // strict mode needs every property required and no extras, at every level
+  for (const key of ["risks", "manual_tests", "new_scenarios"]) {
+    const item = top.properties[key].items;
+    assert.deepEqual(item.required.sort(), Object.keys(item.properties).sort());
+    assert.equal(item.additionalProperties, false);
+  }
+  assert.ok(top.properties.manual_tests.items.properties.title); // the key the model was dropping
+});

From 9f686108bbc202a89f2b6499b1d2a96121b8ffb8 Mon Sep 17 00:00:00 2001
From: Aashil <bijuksheashil@gmail.com>
Date: Fri, 9 Oct 2026 15:05:38 +0545
Subject: [PATCH 6/7] docs(changelog): QA review tuned and put on hold, with
 why and how to resume

---
 CHANGELOG.md | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 891d424..1d5ae60 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,28 @@
 
 Short, dated summary of notable fixes and changes. For the full "why," see `PHASE2-SETUP.md` (Crisp triage design) or the linked PRs.
 
+## 2026-10-09 — QA review: tuned, then put ON HOLD (read this before resuming)
+
+**Status: on hold, manual dispatch only.** Nothing triggers it: no n8n flow, no org webhook. Deliberately not wired up (see "Why on hold").
+
+What this change does to the review job (all verified against real runs on public `themegrill/user-registration` PRs #1428, #1393, #1392, `post_comment=false`):
+- Findings are capped by change size in code (`limitsFor`): a one-line PR got 1 risk, 1 test and 3 scenarios before; now at most 1/2/1.
+- Prompt: no "if X relies on Y" speculation, no repeating the PR's own test steps, tests must be about the same feature.
+- Strict structured output (`REVIEW_FORMAT`; `chatJSONWithUsage` gained an optional 4th param, other callers unchanged). With loose JSON mode the model sometimes returned test suggestions without the `title` key, so 4-7 of them were discarded per run on #1392 and the comment looked empty (2 of 4 runs). Risks were never discarded in about 11 runs of that PR, so the `file:line` citations are reliable. Same PR, same model: results still vary run to run.
+- The comment no longer says "none found" when the model raised risks that failed verification. Discard reasons are logged for PUBLIC target repos only (they quote model/PR text); the review itself goes to the job log/summary only for explicitly public repos, never private ones (this repo is public).
+- Allowlist: added `themegrill/user-registration` (public; appears to be the same repo as `wpeverest/user-registration`, same PR numbers under both names). The `wpeverest` org token (`BOT_TOKEN`) gets 403 on the collaborator-permission lookup, so don't enroll repos by their `wpeverest/` name until that token is fixed.
+- Measured: about 9-13k input and 0.3-0.8k output tokens per review on `gpt-5.4-mini`. No price row in `pricing.mjs`, so cost is unmeasured (roughly a cent by estimate).
+
+### Why on hold
+- The reviewer only reads the diff; it never runs the plugin. It overlaps Copilot (already auto-requested), `security-review.yml` and PHPCS, and its test suggestions point QA at cases they already know. On its own it does not save QA time or catch bugs that reviewers miss.
+- `ThemeGrill/claudegrill` already runs real Playwright e2e on PRs (`pr / suite` check, `@claudegrill suite` to re-run), deterministic and with no AI key. Its agent tier (`pr-qa.yml`, `pr-command.yml`, skill `pr-qa-review`) is the closest thing to a QA agent that exists, but it runs Claude (needs `ANTHROPIC_API_KEY`, which this org does not use) and is deliberately switched off ("the team removed AI from the PR path"). Do not build a second e2e system next to it.
+
+### If resumed
+1. Spike locally first (no CI, nothing published): an OpenAI agent (`opencode`, as in the Crisp job) following a PR's "How to test" steps with Playwright against a site booted by claudegrill's `plugins/claudegrill/scripts/boot-wp.mjs` (Playground, pro licence supported). Candidate: `user-registration-pro#1610`. Judge verdicts on PRs where the right answer is known.
+2. Only if that is trustworthy: run it in a PRIVATE repo, not here. This repo is public, so its logs and run summaries are public, and an agent working on a private repo's PR would publish that code. (One dry run already leaked a private-PR review into a public summary; the run was deleted and the code now withholds it.)
+3. Then the trigger: org webhook -> n8n -> `repository_dispatch` into the private repo, reusing `qa-review-gate.mjs` (requester must have write access; the payload is untrusted). Use a dedicated spend-capped OpenAI key, not the shared one.
+- Open items: the pro repo's `BOT_TOKEN_THEMEGRILL` PAT lacks "Checks: read" (the "other checks" line says "not available" there); `@claudegrill suite` triggers only for collaborators, and `tg-autopilot` is one.
+
 ## 2026-10-09 — QA review: advisory review job (step 2), scoped to complement existing checks
 
 Found while inspecting the pilot repo (`user-registration-pro`): it already has PHPCS-on-PR (`pr-code-sniff.yml`), an AI security scan (`security-review.yml`), and `ThemeGrill/claudegrill`'s deterministic E2E suite with `.themegrill-qa/` test cases and knowledge. claudegrill's README says AI was deliberately removed from the PR path. So the original plan (own static tools + droplet WordPress sandbox) would have duplicated all of that, and was dropped for now. This bot is opt-in only (someone must request `tg-autopilot` as reviewer) and is scoped to what those checks don't do.

From 76a454593279c7a81e67f93ded34bd505e77d09e Mon Sep 17 00:00:00 2001
From: Aashil <bijuksheashil@gmail.com>
Date: Fri, 9 Oct 2026 15:06:17 +0545
Subject: [PATCH 7/7] docs(changelog): fold the three QA review entries into
 one short, readable entry

---
 CHANGELOG.md | 46 +++++++---------------------------------------
 1 file changed, 7 insertions(+), 39 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1d5ae60..ae7104b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,47 +2,15 @@
 
 Short, dated summary of notable fixes and changes. For the full "why," see `PHASE2-SETUP.md` (Crisp triage design) or the linked PRs.
 
-## 2026-10-09 — QA review: tuned, then put ON HOLD (read this before resuming)
+## 2026-10-09 — QA review bot: built, run by hand only, ON HOLD
 
-**Status: on hold, manual dispatch only.** Nothing triggers it: no n8n flow, no org webhook. Deliberately not wired up (see "Why on hold").
+A bot that reviews a PR on request: it reads the diff and the repo's `.themegrill-qa/` notes and writes one advisory comment (summary, risks tied to changed lines, existing test cases to run by hand). Files: `qa-review.yml`, `scripts/qa-review-*.mjs`, `prompts/qa-review.md`, `config/qa-review-repos.json`.
 
-What this change does to the review job (all verified against real runs on public `themegrill/user-registration` PRs #1428, #1393, #1392, `post_comment=false`):
-- Findings are capped by change size in code (`limitsFor`): a one-line PR got 1 risk, 1 test and 3 scenarios before; now at most 1/2/1.
-- Prompt: no "if X relies on Y" speculation, no repeating the PR's own test steps, tests must be about the same feature.
-- Strict structured output (`REVIEW_FORMAT`; `chatJSONWithUsage` gained an optional 4th param, other callers unchanged). With loose JSON mode the model sometimes returned test suggestions without the `title` key, so 4-7 of them were discarded per run on #1392 and the comment looked empty (2 of 4 runs). Risks were never discarded in about 11 runs of that PR, so the `file:line` citations are reliable. Same PR, same model: results still vary run to run.
-- The comment no longer says "none found" when the model raised risks that failed verification. Discard reasons are logged for PUBLIC target repos only (they quote model/PR text); the review itself goes to the job log/summary only for explicitly public repos, never private ones (this repo is public).
-- Allowlist: added `themegrill/user-registration` (public; appears to be the same repo as `wpeverest/user-registration`, same PR numbers under both names). The `wpeverest` org token (`BOT_TOKEN`) gets 403 on the collaborator-permission lookup, so don't enroll repos by their `wpeverest/` name until that token is fixed.
-- Measured: about 9-13k input and 0.3-0.8k output tokens per review on `gpt-5.4-mini`. No price row in `pricing.mjs`, so cost is unmeasured (roughly a cent by estimate).
-
-### Why on hold
-- The reviewer only reads the diff; it never runs the plugin. It overlaps Copilot (already auto-requested), `security-review.yml` and PHPCS, and its test suggestions point QA at cases they already know. On its own it does not save QA time or catch bugs that reviewers miss.
-- `ThemeGrill/claudegrill` already runs real Playwright e2e on PRs (`pr / suite` check, `@claudegrill suite` to re-run), deterministic and with no AI key. Its agent tier (`pr-qa.yml`, `pr-command.yml`, skill `pr-qa-review`) is the closest thing to a QA agent that exists, but it runs Claude (needs `ANTHROPIC_API_KEY`, which this org does not use) and is deliberately switched off ("the team removed AI from the PR path"). Do not build a second e2e system next to it.
-
-### If resumed
-1. Spike locally first (no CI, nothing published): an OpenAI agent (`opencode`, as in the Crisp job) following a PR's "How to test" steps with Playwright against a site booted by claudegrill's `plugins/claudegrill/scripts/boot-wp.mjs` (Playground, pro licence supported). Candidate: `user-registration-pro#1610`. Judge verdicts on PRs where the right answer is known.
-2. Only if that is trustworthy: run it in a PRIVATE repo, not here. This repo is public, so its logs and run summaries are public, and an agent working on a private repo's PR would publish that code. (One dry run already leaked a private-PR review into a public summary; the run was deleted and the code now withholds it.)
-3. Then the trigger: org webhook -> n8n -> `repository_dispatch` into the private repo, reusing `qa-review-gate.mjs` (requester must have write access; the payload is untrusted). Use a dedicated spend-capped OpenAI key, not the shared one.
-- Open items: the pro repo's `BOT_TOKEN_THEMEGRILL` PAT lacks "Checks: read" (the "other checks" line says "not available" there); `@claudegrill suite` triggers only for collaborators, and `tg-autopilot` is one.
-
-## 2026-10-09 — QA review: advisory review job (step 2), scoped to complement existing checks
-
-Found while inspecting the pilot repo (`user-registration-pro`): it already has PHPCS-on-PR (`pr-code-sniff.yml`), an AI security scan (`security-review.yml`), and `ThemeGrill/claudegrill`'s deterministic E2E suite with `.themegrill-qa/` test cases and knowledge. claudegrill's README says AI was deliberately removed from the PR path. So the original plan (own static tools + droplet WordPress sandbox) would have duplicated all of that, and was dropped for now. This bot is opt-in only (someone must request `tg-autopilot` as reviewer) and is scoped to what those checks don't do.
-
-- New `review` job in `qa-review.yml`, after the gate. It never checks out or runs PR code: it reads the diff and the base branch's `.themegrill-qa/` (`suite.json` area map, `testcase-index.json` titles, `knowledge.md`) through the API, plus the other checks' results, and makes ONE model call (`prompts/qa-review.md`, default `gpt-5.4-mini`, override with the `QA_REVIEW_MODEL` repo variable).
-- Output is one sticky advisory comment: summary, areas touched (deterministic, from `suite.json`), risks, existing test cases worth running by hand, and scenarios with no test case.
-- Findings are verified, not trusted (see the CHANGELOG 2026-10-05 note on uncalibrated confidence): a risk is shown only if its `file:line` is an ADDED line in the diff; a suggested test only if its title exists verbatim in the index. Everything else is dropped and counted in the comment footer. No confidence number is shown. Model output is stripped of URLs, `@mentions`, HTML and backticks. An unparseable or empty model answer fails the job instead of posting "no risks found".
-- Not verified yet against a real model call: output quality, and cost (`pricing.mjs` has no row for the default model, so cost shows only if you add one). Uses the shared `OPENAI_API_KEY`; a dedicated spend-capped key is advisable before n8n makes this fire automatically.
-- Still manual dispatch only; no n8n flow or org webhook yet.
-- **Privacy:** `themegrill/.github` is PUBLIC, so run summaries/logs are public. The first dry run wrote a review of a private-repo PR into a public run summary (one-line change; the run was deleted). The review body now goes to the run summary only when the target repo is explicitly public; for private repos it is posted to the PR only (`post_comment=true`). Never log or summarize review content, PR text or diffs from private repos in this repo's runs.
-- The bot's fine-grained PAT cannot read check runs (403, needs "Checks: read"); that section is best-effort and says "not available" until the PAT is updated.
-
-## 2026-10-09 — QA review: gate only (step 1 of a staged build, not a review yet)
-
-First piece of a PR QA-review agent (request `tg-autopilot` as reviewer -> deep review). This change adds only the front half: `qa-review.yml` (`repository_dispatch: qa-review` or manual `workflow_dispatch`), `scripts/qa-review-gate.mjs` and `config/qa-review-repos.json` (allowlist; pilot is `themegrill/user-registration-pro` only).
-
-- The gate re-verifies everything against the live API (the dispatch payload is untrusted): allowlisted repo, PR open and not a draft, same-repo (forks denied), `head_sha` current, requester is not the bot, requester has `write` or higher (triage is not enough, each review will spend LLM money). A wrong token identity (not `tg-autopilot`) fails the job rather than going silently green.
-- `scripts/qa-review-comment.mjs` keeps ONE sticky PR comment, matched by bot login plus marker. Manual runs post nothing unless `post_comment` is true. The "accepted" comment is a placeholder and says no findings will follow.
-- No n8n flow, org webhook, static checks, LLM review or droplet sandbox yet; the only way in is a manual dispatch. Not yet verified: that the real `BOT_TOKEN_THEMEGRILL` authenticates as `tg-autopilot` and can read the pilot repo.
+- **Run it:** Actions -> "QA review" -> Run workflow (`repo`, `pr`, optional `post_comment`). Only allowlisted repos, and the requester needs write access. Nothing triggers it automatically: there is no n8n flow or webhook.
+- **Built to not mislead:** a risk is shown only if its cited line is an added line in the diff, and a suggested test only if it exists in the repo's test index; anything else is dropped and counted. No confidence number. Review text for a private repo is never written to this public repo's run summary or logs.
+- **Why on hold:** it only reads code and never runs the plugin, and it overlaps Copilot, `security-review.yml` and PHPCS. `ThemeGrill/claudegrill` already runs the real Playwright e2e, and its AI agent tier needs an Anthropic key and is switched off on purpose. Don't build a second e2e system next to it.
+- **If resumed:** (1) spike locally first: an OpenAI agent (`opencode`) following a PR's "How to test" steps against a site from claudegrill's `boot-wp.mjs`, e.g. on `user-registration-pro#1610`; (2) only if it works, run it in a private repo, because this repo is public; (3) then add the n8n trigger and a dedicated, spend-capped OpenAI key.
+- **Gotchas:** use `themegrill/user-registration`, not the `wpeverest/` name (that org's token gets 403). The pro repo's token can't read check runs, so that line says "not available". The same PR can get somewhat different reviews run to run. About 9-13k input tokens per review; cost not measured.
 
 ## 2026-10-05 — wp.org triage files issues in the pro repo first