mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 02:11:28 +00:00
hotfix(ci): improve ai review comment formatting
This commit is contained in:
@@ -35,12 +35,19 @@ const RENDERER_OWNED_MARKUP_PATTERNS = [
|
||||
{ pattern: /```/u, reason: 'code fence' },
|
||||
];
|
||||
|
||||
const INLINE_CODE_TOKEN_PATTERN =
|
||||
/\b[A-Za-z_][A-Za-z0-9_.]*\([^()\n]*\)|(?<![\w`])\.?[\w-]+(?:\/[\w.-]+)+\.[\w.-]+(?::\d+)?|\b[\w.-]+\/[\w.-]+@[\w.-]+\b|--[a-z0-9][a-z0-9-]*\b|\b[A-Z][A-Z0-9]*_[A-Z0-9_]+\b|\b[a-z][a-z0-9]*(?:_[a-z0-9]+)+\b/gu;
|
||||
|
||||
function cleanText(value) {
|
||||
return typeof value === 'string' ? value.trim().replace(/\s+/g, ' ') : '';
|
||||
}
|
||||
|
||||
function escapeMarkdown(value) {
|
||||
return String(value).replace(/\\/g, '\\\\').replace(/([`*_{}[\]<>|])/g, '\\$1');
|
||||
}
|
||||
|
||||
function escapeMarkdownText(value) {
|
||||
return cleanText(value).replace(/\\/g, '\\\\').replace(/([`*_{}[\]<>|])/g, '\\$1');
|
||||
return escapeMarkdown(cleanText(value));
|
||||
}
|
||||
|
||||
function renderCode(value) {
|
||||
@@ -50,6 +57,30 @@ function renderCode(value) {
|
||||
return `${fence}${text}${fence}`;
|
||||
}
|
||||
|
||||
function renderInlineText(value) {
|
||||
const text = cleanText(value);
|
||||
if (!text) {
|
||||
return '';
|
||||
}
|
||||
|
||||
let rendered = '';
|
||||
let lastIndex = 0;
|
||||
for (const match of text.matchAll(INLINE_CODE_TOKEN_PATTERN)) {
|
||||
const token = match[0];
|
||||
const index = match.index ?? 0;
|
||||
if (index < lastIndex) {
|
||||
continue;
|
||||
}
|
||||
|
||||
rendered += escapeMarkdown(text.slice(lastIndex, index));
|
||||
rendered += renderCode(token);
|
||||
lastIndex = index + token.length;
|
||||
}
|
||||
|
||||
rendered += escapeMarkdown(text.slice(lastIndex));
|
||||
return rendered;
|
||||
}
|
||||
|
||||
function parsePositiveInteger(value) {
|
||||
if (value === null || value === undefined || value === '') {
|
||||
return null;
|
||||
@@ -490,7 +521,7 @@ function renderChecklistTable(title, labelHeader, labelKey, rows) {
|
||||
const lines = ['', title, '', `| ${labelHeader} | Status | Notes |`, '|---|---|---|'];
|
||||
for (const row of rows) {
|
||||
lines.push(
|
||||
`| ${escapeMarkdownText(row[labelKey])} | ${STATUS_LABELS[row.status]} | ${escapeMarkdownText(row.notes)} |`
|
||||
`| ${renderInlineText(row[labelKey])} | ${STATUS_LABELS[row.status]} | ${renderInlineText(row.notes)} |`
|
||||
);
|
||||
}
|
||||
return lines;
|
||||
@@ -498,12 +529,12 @@ function renderChecklistTable(title, labelHeader, labelKey, rows) {
|
||||
|
||||
function renderBulletSection(title, items) {
|
||||
if (items.length === 0) return [];
|
||||
return ['', title, ...items.map((item) => `- ${escapeMarkdownText(item)}`)];
|
||||
return ['', title, ...items.map((item) => `- ${renderInlineText(item)}`)];
|
||||
}
|
||||
|
||||
export function renderStructuredReview(review, { model, rendering: renderOptions } = {}) {
|
||||
const rendering = mergeRenderingMetadata(review?.rendering, renderOptions);
|
||||
const lines = ['### 📋 Summary', '', escapeMarkdownText(review.summary), '', '### 🔍 Findings'];
|
||||
const lines = ['### 📋 Summary', '', renderInlineText(review.summary), '', '### 🔍 Findings'];
|
||||
const reviewContext = formatReviewContext(rendering);
|
||||
|
||||
if (reviewContext) {
|
||||
@@ -520,10 +551,10 @@ export function renderStructuredReview(review, { model, rendering: renderOptions
|
||||
lines.push('', SEVERITY_HEADERS[severity], '');
|
||||
for (const finding of findings) {
|
||||
const location = finding.line ? `${finding.file}:${finding.line}` : finding.file;
|
||||
lines.push(`- **${renderCode(location)} — ${escapeMarkdownText(finding.title)}**`);
|
||||
lines.push(` Problem: ${escapeMarkdownText(finding.what)}`);
|
||||
lines.push(` Why it matters: ${escapeMarkdownText(finding.why)}`);
|
||||
lines.push(` Suggested fix: ${escapeMarkdownText(finding.fix)}`);
|
||||
lines.push(`- **${renderCode(location)} — ${renderInlineText(finding.title)}**`);
|
||||
lines.push(` Problem: ${renderInlineText(finding.what)}`);
|
||||
lines.push(` Why it matters: ${renderInlineText(finding.why)}`);
|
||||
lines.push(` Suggested fix: ${renderInlineText(finding.fix)}`);
|
||||
lines.push('');
|
||||
}
|
||||
}
|
||||
@@ -539,7 +570,7 @@ export function renderStructuredReview(review, { model, rendering: renderOptions
|
||||
'',
|
||||
'### 🎯 Overall Assessment',
|
||||
'',
|
||||
`**${ASSESSMENTS[review.overallAssessment]}** — ${escapeMarkdownText(review.overallRationale)}`,
|
||||
`**${ASSESSMENTS[review.overallAssessment]}** — ${renderInlineText(review.overallRationale)}`,
|
||||
'',
|
||||
`> 🤖 Reviewed by \`${model}\``
|
||||
);
|
||||
|
||||
@@ -118,6 +118,56 @@ describe('normalize-ai-review-output', () => {
|
||||
expect(markdown).toContain('**⚠️ APPROVED WITH NOTES**');
|
||||
});
|
||||
|
||||
test('auto-formats code-like tokens while keeping markdown structure renderer-owned', () => {
|
||||
const validation = reviewOutput.normalizeStructuredOutput(
|
||||
JSON.stringify({
|
||||
summary:
|
||||
'buildReviewScope(files, mode) now feeds .github/workflows/ai-review.yml through workflow_dispatch with AI_REVIEW_PACKET_FILE and --max-turns coverage.',
|
||||
findings: [
|
||||
{
|
||||
severity: 'medium',
|
||||
title: 'pull_request_target fallback still references old_marker_path',
|
||||
file: '.github/workflows/ai-review.yml',
|
||||
line: 181,
|
||||
what: 'The workflow_dispatch smoke test still leaves old_marker_path in one branch.',
|
||||
why: 'That makes pull_request_target reruns harder to reason about for maintainers.',
|
||||
fix: 'Rename old_marker_path and keep workflow_dispatch aligned with AI_REVIEW_PACKET_FILE.',
|
||||
},
|
||||
],
|
||||
securityChecklist: [
|
||||
{
|
||||
check: 'workflow_dispatch safety',
|
||||
status: 'pass',
|
||||
notes: 'workflow_dispatch stays scoped to .github/workflows/ai-review.yml only.',
|
||||
},
|
||||
],
|
||||
ccsCompliance: [
|
||||
{
|
||||
rule: 'Renderer-owned markdown',
|
||||
status: 'pass',
|
||||
notes: 'The normalizer still owns headings, tables, and code fences.',
|
||||
},
|
||||
],
|
||||
informational: ['Use --max-turns only for legacy fallbacks.'],
|
||||
strengths: ['AI_REVIEW_PACKET_FILE now renders as code.'],
|
||||
overallAssessment: 'approved_with_notes',
|
||||
overallRationale:
|
||||
'The renderer can format buildReviewScope(files, mode) and .github/workflows/ai-review.yml safely.',
|
||||
})
|
||||
);
|
||||
|
||||
expect(validation.ok).toBe(true);
|
||||
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5.1' });
|
||||
|
||||
expect(markdown).toContain('`buildReviewScope(files, mode)`');
|
||||
expect(markdown).toContain('`.github/workflows/ai-review.yml`');
|
||||
expect(markdown).toContain('`workflow_dispatch`');
|
||||
expect(markdown).toContain('`AI_REVIEW_PACKET_FILE`');
|
||||
expect(markdown).toContain('`--max-turns`');
|
||||
expect(markdown).toContain('`pull_request_target`');
|
||||
expect(markdown).toContain('`old_marker_path`');
|
||||
});
|
||||
|
||||
test('normalizes optional rendering metadata when present in structured output', () => {
|
||||
const validation = reviewOutput.normalizeStructuredOutput(
|
||||
JSON.stringify({
|
||||
|
||||
Reference in New Issue
Block a user