Skip to content

Commit 0ed4d13

Browse files
committed
Report each finding in the shape the protocol asks for
The example protocol defines a finding as File, Lines, Issue, Comment. The report was rendering a markdown heading and prose paragraphs instead, which is readable but is not the structure someone deciding what to fix can scan. Findings now carry labelled fields, with the rule code when there is one and the mechanism that traces the finding to the change. Lines is a single number, or a range only when the finding genuinely spans more than one. The quoted code sits below, still byte-checked against the file, rather than inside the sentence explaining the problem. The tests passed both before and after that change, which is the more useful half of this commit: they were asserting on the text of a finding and never on its shape, so the format could have drifted without anything noticing. They pin it now, and the mutation back to headings fails two of them. The other half was missing further upstream. Nothing told the model what a comment is for. The adversarial prompt now says it: the issue is one sentence naming the problem, the comment explains what is wrong and what it breaks in language someone who did not write the code can follow, and no code goes in the comment because the code that proves the finding is quoted separately. A report cannot make prose out of a comment full of code, so the instruction belongs where the comment is written rather than where it is rendered.
1 parent 982c6b1 commit 0ed4d13

5 files changed

Lines changed: 74 additions & 8 deletions

File tree

docs/DECISIONS.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,3 +729,16 @@ verified evidence, in writing, here.
729729
worktrees, bundle and logs that deleting a review removes. The filename uses
730730
the UTC day and turns branch slashes into hyphens, so a second export the
731731
same day replaces the first rather than accumulating near-identical files.
732+
- 2026-07-31 FIXED (maintainer question): the report renders each finding in
733+
the labelled structure the example protocol defines, File, Lines, Issue,
734+
Comment, rather than as a markdown heading and prose. It did not before, and
735+
the tests passed both before and after the change, which means they were
736+
asserting on the text of a finding and never on its shape. They pin the
737+
format now. Lines is a single number, or a range only when the finding
738+
genuinely spans more than one line.
739+
- 2026-07-31 FIXED: the adversarial prompt now says what a comment is for.
740+
Nothing told the model that the comment must be plain language a
741+
non-author can follow, or that code belongs in the quotation rather than in
742+
the sentence explaining the problem. A report cannot make prose out of a
743+
comment full of code, so the instruction belongs where the comment is
744+
written rather than where it is rendered.

src/lib/review/report.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -172,23 +172,32 @@ function completeness(input: ReportInput): string {
172172
);
173173
}
174174

175+
/**
176+
* One finding, in the structure the protocol defines.
177+
*
178+
* Labelled fields rather than prose, because a finding is read by someone
179+
* deciding what to fix: the file, the lines, what is wrong and what it breaks,
180+
* each findable without reading a paragraph. The comment is plain language by
181+
* contract; the code that proves it sits below, quoted and byte-checked,
182+
* rather than inside the sentence explaining it.
183+
*/
175184
function renderFinding(finding: ReportFinding): string[] {
176-
const rule = finding.ruleCode ? ` (rule ${finding.ruleCode})` : "";
177185
return [
178-
`#### ${finding.filePath}:${lineRange(finding)}${rule}`,
179-
"",
180-
`**${finding.issue}**`,
181-
"",
182-
finding.comment,
186+
`File: ${finding.filePath}`,
187+
`Lines: ${lineRange(finding)}`,
188+
...(finding.ruleCode ? [`Rule: ${finding.ruleCode}`] : []),
189+
`Issue: ${finding.issue}`,
190+
`Comment: ${finding.comment}`,
191+
...(finding.mechanism ? [`Mechanism: ${finding.mechanism}`] : []),
183192
"",
184-
...(finding.mechanism ? [`How it fails: ${finding.mechanism}`, ""] : []),
185193
...(finding.quotedCode ? ["```", finding.quotedCode, "```", ""] : []),
186194
];
187195
}
188196

197+
/** A single line, or a range when the finding genuinely spans one. */
189198
function lineRange(finding: ReportFinding): string {
190199
return finding.lineEnd > finding.lineStart
191-
? `${finding.lineStart}-${finding.lineEnd}`
200+
? `${finding.lineStart} - ${finding.lineEnd}`
192201
: String(finding.lineStart);
193202
}
194203

src/lib/review/stage-schemas.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const candidateFindingSchema = z.object({
4141
severity: z.enum(SEVERITIES),
4242
ruleCode: z.string().nullable(),
4343
issue: z.string().min(1),
44+
/** Plain language, no code: what is wrong and what it breaks. */
4445
comment: z.string().min(1),
4546
/** Input, mechanism, wrong output. A finding without one is a guess. */
4647
mechanism: z.string().min(1),

src/server/review/content.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,12 @@ export function renderAdversarialPrompt(
247247
"Cite line numbers exactly as shown in the left column above: those are the",
248248
"line numbers in the file as it stands after the change. A hunk you do not",
249249
"mention is treated as unreviewed and fails the run.",
250+
"",
251+
"For each finding: the issue is one sentence naming the problem, and the",
252+
"comment explains what is wrong and what it breaks in plain language that",
253+
"someone who did not write the code can follow. Put no code in the comment;",
254+
"the code that proves the finding is quoted separately and checked against",
255+
"the file.",
250256
].join("\n");
251257
}
252258

tests/lib/review/report.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,43 @@ describe("what the report says was found", () => {
8181
it("carries the quoted code that was checked against the file", () => {
8282
expect(renderReport(BASE)).toContain(" repository.save(order);");
8383
});
84+
85+
it("uses the labelled structure the protocol defines", () => {
86+
// File, lines, issue and comment, each findable without reading a
87+
// paragraph, because a finding is read by someone deciding what to fix.
88+
const report = renderReport(BASE);
89+
expect(report).toContain("File: app/src/orders/save.ts");
90+
expect(report).toContain("Lines: 4");
91+
expect(report).toContain("Rule: 1");
92+
expect(report).toContain("Issue: The await was removed, so the save is not waited for.");
93+
expect(report).toContain(
94+
"Comment: The caller returns before the write lands, and a failure is unobservable.",
95+
);
96+
});
97+
98+
it("gives a range only when the finding spans more than one line", () => {
99+
expect(renderReport(BASE)).toContain("Lines: 4");
100+
expect(renderReport(BASE)).not.toContain("Lines: 4 - 4");
101+
expect(
102+
renderReport({ ...BASE, confirmed: [{ ...FINDING, lineStart: 4, lineEnd: 9 }] }),
103+
).toContain("Lines: 4 - 9");
104+
});
105+
106+
it("keeps code out of the comment line and in the quotation", () => {
107+
// The comment is what a person reads to decide; the code is the evidence
108+
// underneath it, quoted and byte-checked.
109+
const report = renderReport(BASE);
110+
const commentLine = report.split("\n").find((line) => line.startsWith("Comment: "));
111+
expect(commentLine).toBeDefined();
112+
expect(commentLine).not.toContain("repository.save");
113+
expect(report).toContain("repository.save(order);");
114+
});
115+
116+
it("omits the rule line when a finding carries no rule code", () => {
117+
expect(renderReport({ ...BASE, confirmed: [{ ...FINDING, ruleCode: null }] })).not.toContain(
118+
"Rule:",
119+
);
120+
});
84121
});
85122

86123
describe("what the report says was examined", () => {

0 commit comments

Comments
 (0)