From c85d44a0a0e09334185d210dc727e4b1c041a413 Mon Sep 17 00:00:00 2001
From: Trevin Chow <trevin@trevinchow.com>
Date: Sat, 22 Aug 2026 08:40:39 -0700
Subject: [PATCH] fix(ce-code-review): stop the reviewer template from
 embedding the diff twice

The closing note of the review-context block named {file_list} and {diff}
in slot syntax, so prompt assembly filled them a second time and then told
the reviewer the inline content "may be file paths" to Read. The note now
refers to the Changed files: and Diff: labels the reviewer actually sees
and applies only to a lone staged path. A contract test pins each
placeholder to one occurrence inside the fenced template.

Fixes #1509

Claude-Session: https://claude.ai/code/session_012uPY9QcVF4ueBgrpegPcvC
---
 .../references/subagent-template.md           |  2 +-
 tests/review-skill-contract.test.ts           | 21 +++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/skills/ce-code-review/references/subagent-template.md b/skills/ce-code-review/references/subagent-template.md
index 2220d5822..dea80f0cd 100644
--- a/skills/ce-code-review/references/subagent-template.md
+++ b/skills/ce-code-review/references/subagent-template.md
@@ -183,7 +183,7 @@ Changed files: {file_list}
 Diff:
 {diff}
 
-(For a large staged review, `{file_list}` and `{diff}` may be **file paths** rather than inline content. When a value above is a path, Read that file to get the full list/diff before reviewing — never treat the path string itself as the content to review.)
+(The `Changed files:` and `Diff:` values above are either inline content or, for a large staged review, a single file path each. Inline content is authoritative: review it as given. A lone file path is not the content — Read that file to get the full list/diff before reviewing.)
 </review-context>
 ```
 
diff --git a/tests/review-skill-contract.test.ts b/tests/review-skill-contract.test.ts
index 6408c10bb..7aa23988b 100644
--- a/tests/review-skill-contract.test.ts
+++ b/tests/review-skill-contract.test.ts
@@ -1531,3 +1531,24 @@ describe("testing-reviewer contract", () => {
     expect(content).toContain("Non-behavioral changes")
   })
 })
+
+describe("ce-code-review dispatch templates", () => {
+  // #1509: a placeholder named in the template's prose gets filled like a slot, so
+  // `{diff}` in the closing note re-emitted the whole diff into every reviewer prompt.
+  // Each placeholder may appear only once inside the fenced template, as its slot.
+  test("the reviewer template names each placeholder once, as a slot", async () => {
+    const content = await readRepoFile(
+      "skills/ce-code-review/references/subagent-template.md",
+    )
+    // The template fence nests a ```json example, so bound it by the heading that follows it.
+    const fenced = content.match(/^```\n[\s\S]*?^```\n\n## Variable Reference/m)?.[0]
+    expect(fenced).toBeDefined()
+    const counts = new Map<string, number>()
+    for (const m of fenced!.matchAll(/\{([a-z_]+)\}/g)) {
+      counts.set(m[1], (counts.get(m[1]) ?? 0) + 1)
+    }
+    for (const name of ["file_list", "diff"]) {
+      expect(counts.get(name)).toBe(1)
+    }
+  })
+})