diff --git a/.github/review-prompt.md b/.github/review-prompt.md index 9113ac58..ed511dc1 100644 --- a/.github/review-prompt.md +++ b/.github/review-prompt.md @@ -49,7 +49,7 @@ Use only the review contract in this file plus the generated scope and packet fi - Return confirmed findings only. - Every finding must cite a file path and, when practical, a line number. -- Each finding may optionally include `snippets`: up to 2 short evidence blocks with `label`, `language`, and `code`. +- Each finding may optionally include `snippets`: up to 2 short evidence blocks with required `code`, plus optional `label` and `language`. - Keep the total finding count small unless the PR genuinely has several distinct problems. - If there are no confirmed findings, say so in the summary and return an empty findings array. - Use `approved` only when the diff is ready to merge as-is. diff --git a/scripts/github/normalize-ai-review-output.mjs b/scripts/github/normalize-ai-review-output.mjs index f22fe925..9be73e21 100644 --- a/scripts/github/normalize-ai-review-output.mjs +++ b/scripts/github/normalize-ai-review-output.mjs @@ -47,9 +47,15 @@ function cleanText(value) { } function cleanMultilineText(value) { - return typeof value === 'string' - ? value.trim().replace(/\r\n/g, '\n').replace(/\r/g, '\n') - : ''; + if (typeof value !== 'string') { + return ''; + } + + return value + .replace(/\r\n/g, '\n') + .replace(/\r/g, '\n') + .replace(/^\n+/u, '') + .replace(/\n+$/u, ''); } function escapeMarkdown(value) { diff --git a/scripts/github/run-ai-review-direct.mjs b/scripts/github/run-ai-review-direct.mjs index e8c25c34..a2f37959 100644 --- a/scripts/github/run-ai-review-direct.mjs +++ b/scripts/github/run-ai-review-direct.mjs @@ -101,7 +101,7 @@ Return a single object with these keys only: - overallRationale Each finding may optionally include: -- snippets: an array of up to 2 objects with keys label, language, and code +- snippets: an array of up to 2 objects with required code plus optional label and language If snippets are present: - keep code literal only, without markdown fences diff --git a/tests/unit/scripts/github/normalize-ai-review-output.test.ts b/tests/unit/scripts/github/normalize-ai-review-output.test.ts index e4afa2b0..afefb07e 100644 --- a/tests/unit/scripts/github/normalize-ai-review-output.test.ts +++ b/tests/unit/scripts/github/normalize-ai-review-output.test.ts @@ -209,6 +209,44 @@ describe('normalize-ai-review-output', () => { expect(markdown).toContain('```'); }); + test('preserves leading indentation inside literal finding snippets', () => { + const validation = reviewOutput.normalizeStructuredOutput( + JSON.stringify({ + summary: 'Indented snippets must stay literal.', + findings: [ + { + severity: 'low', + title: 'Indentation-sensitive example', + file: 'examples/sample.py', + line: 7, + what: 'The snippet must preserve its leading spaces.', + why: 'Python, YAML, and shell examples break when the renderer trims indentation.', + fix: 'Normalize newlines without trimming leading spaces from the first line.', + snippets: [ + { + language: 'python', + code: ' if value:\n print(value)', + }, + ], + }, + ], + securityChecklist: [{ check: 'Injection safety', status: 'pass', notes: 'Covered.' }], + ccsCompliance: [{ rule: 'Renderer-owned markdown', status: 'pass', notes: 'Covered.' }], + informational: [], + strengths: [], + overallAssessment: 'approved_with_notes', + overallRationale: 'This keeps literal evidence stable.', + }) + ); + + expect(validation.ok).toBe(true); + expect(validation.value.findings[0].snippets[0].code).toBe(' if value:\n print(value)'); + + const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5.1' }); + expect(markdown).toContain(' if value:'); + expect(markdown).toContain(' print(value)'); + }); + test('normalizes optional rendering metadata when present in structured output', () => { const validation = reviewOutput.normalizeStructuredOutput( JSON.stringify({