hotfix(ci): compact ai review comments and switch to glm-5-turbo

This commit is contained in:
Tam Nhu Tran
2026-04-02 01:07:03 -04:00
parent f6779a503a
commit 25dddf4707
6 changed files with 160 additions and 93 deletions
+2 -1
View File
@@ -56,7 +56,8 @@ Use only the review contract in this file plus the generated scope and packet fi
- Use `approved_with_notes` when only non-blocking follow-ups remain.
- Use `changes_requested` when any blocking issue remains.
- Fill the structured fields only. The renderer owns the markdown layout.
- Keep `summary` to plain prose only. Do not include the PR title, a separate verdict line, markdown tables, file inventories, or custom section headings there.
- Keep `summary` to plain prose only, ideally 2-4 sentences. Do not include the PR title, a separate verdict line, markdown tables, file inventories, or custom section headings there.
- Keep `overallRationale` to 1 sentence.
- Keep `what`, `why`, and `fix` concise plain text. Do not emit headings, tables, or fenced code blocks inside those fields.
- Use `snippets` only when a short literal excerpt materially clarifies a finding. Keep each snippet under 20 lines, and do not include markdown fences in `code`.
- Use `securityChecklist` for concise review rows about security-sensitive checks. Provide at least 1 row, and use 2-5 when possible. `status` = `pass` | `fail` | `na`.
+5 -5
View File
@@ -196,12 +196,12 @@ jobs:
# GLM API environment for model routing
env:
ANTHROPIC_BASE_URL: https://api.z.ai/api/anthropic
REVIEW_MODEL: glm-5.1
REVIEW_MODEL: glm-5-turbo
ANTHROPIC_AUTH_TOKEN: ${{ secrets.GLM_API_KEY }}
ANTHROPIC_MODEL: glm-5.1
ANTHROPIC_DEFAULT_OPUS_MODEL: glm-5.1
ANTHROPIC_DEFAULT_SONNET_MODEL: glm-5.1
ANTHROPIC_DEFAULT_HAIKU_MODEL: GLM-4.7-FlashX
ANTHROPIC_MODEL: glm-5-turbo
ANTHROPIC_DEFAULT_OPUS_MODEL: glm-5-turbo
ANTHROPIC_DEFAULT_SONNET_MODEL: glm-5-turbo
ANTHROPIC_DEFAULT_HAIKU_MODEL: glm-5-turbo
DISABLE_BUG_COMMAND: '1'
DISABLE_ERROR_REPORTING: '1'
DISABLE_TELEMETRY: '1'
+119 -50
View File
@@ -14,6 +14,11 @@ const SEVERITY_HEADERS = {
medium: '### 🟡 Medium',
low: '### 🟢 Low',
};
const SEVERITY_SUMMARY_LABELS = {
high: '🔴 High',
medium: '🟡 Medium',
low: '🟢 Low',
};
const STATUS_LABELS = {
pass: '✅',
@@ -41,6 +46,7 @@ const CODE_BLOCK_LANGUAGE_PATTERN = /^[A-Za-z0-9#+.-]{1,20}$/u;
const MAX_FINDING_SNIPPETS = 2;
const MAX_SNIPPET_LINES = 20;
const MAX_SNIPPET_CHARACTERS = 1200;
const TOP_FINDINGS_LIMIT = 3;
function cleanText(value) {
return typeof value === 'string' ? value.trim().replace(/\s+/g, ' ') : '';
@@ -224,35 +230,40 @@ function formatReviewContext(rendering) {
const parts = [];
if (rendering.mode) {
parts.push(`mode ${renderCode(rendering.mode)}`);
parts.push(REVIEW_MODE_DETAILS[rendering.mode]);
parts.push(renderCode(rendering.mode));
}
const scopeSummary = formatScopeSummary(rendering);
if (scopeSummary) {
parts.push(`scope ${scopeSummary}`);
if (
typeof rendering.selectedFiles === 'number' &&
typeof rendering.reviewableFiles === 'number'
) {
parts.push(`${rendering.selectedFiles}/${rendering.reviewableFiles} files`);
}
const packetCoverage = formatPacketCoverage(rendering);
if (packetCoverage) {
parts.push(`packet ${packetCoverage}`);
if (
typeof rendering.selectedChanges === 'number' &&
typeof rendering.reviewableChanges === 'number'
) {
parts.push(`${rendering.selectedChanges}/${rendering.reviewableChanges} lines`);
}
const turnBudget = formatTurnBudget(rendering);
if (turnBudget) {
parts.push(`turn budget ${turnBudget}`);
if (
typeof rendering.packetIncludedFiles === 'number' &&
typeof rendering.packetTotalFiles === 'number'
) {
parts.push(`packet ${rendering.packetIncludedFiles}/${rendering.packetTotalFiles}`);
}
const timeBudget = formatTimeBudget(rendering);
if (timeBudget) {
parts.push(`workflow cap ${timeBudget}`);
const runtimeBudget = formatCombinedBudget(rendering) || formatTimeBudget(rendering) || formatTurnBudget(rendering);
if (runtimeBudget) {
parts.push(runtimeBudget);
}
if (parts.length === 0) {
return null;
}
return `> 🧭 Review context: ${parts.join('; ')}.`;
return `> 🧭 ${parts.join(' ')}`;
}
function classifyFallbackReason(reason) {
@@ -612,8 +623,8 @@ export function normalizeStructuredOutput(raw) {
return { ok: true, value };
}
function renderChecklistTable(title, labelHeader, labelKey, rows) {
const lines = ['', title, '', `| ${labelHeader} | Status | Notes |`, '|---|---|---|'];
function renderChecklistTable(labelHeader, labelKey, rows) {
const lines = [`| ${labelHeader} | Status | Notes |`, '|---|---|---|'];
for (const row of rows) {
lines.push(
`| ${renderInlineText(row[labelKey])} | ${STATUS_LABELS[row.status]} | ${renderInlineText(row.notes)} |`
@@ -622,9 +633,9 @@ function renderChecklistTable(title, labelHeader, labelKey, rows) {
return lines;
}
function renderBulletSection(title, items) {
function renderBulletSection(items) {
if (items.length === 0) return [];
return ['', title, ...items.map((item) => `- ${renderInlineText(item)}`)];
return items.map((item) => `- ${renderInlineText(item)}`);
}
function renderFindingSnippets(snippets) {
@@ -646,46 +657,104 @@ function renderFindingSnippets(snippets) {
return lines;
}
function renderDetailsBlock(summary, bodyLines) {
if (!bodyLines.length) {
return [];
}
return ['', '<details>', `<summary>${escapeMarkdown(summary)}</summary>`, '', ...bodyLines, '', '</details>'];
}
function renderFindingReference(finding) {
return finding.line ? `${finding.file}:${finding.line}` : finding.file;
}
function getOrderedFindings(findings) {
return SEVERITY_ORDER.flatMap((severity) => findings.filter((finding) => finding.severity === severity));
}
function renderTopFindings(findings) {
if (findings.length === 0) {
return ['No confirmed issues found after reviewing the diff and surrounding code.'];
}
const orderedFindings = getOrderedFindings(findings);
const lines = orderedFindings
.slice(0, TOP_FINDINGS_LIMIT)
.map(
(finding) =>
`- ${SEVERITY_SUMMARY_LABELS[finding.severity]} ${renderCode(renderFindingReference(finding))}${renderInlineText(finding.title)}`
);
if (orderedFindings.length > TOP_FINDINGS_LIMIT) {
const remaining = orderedFindings.length - TOP_FINDINGS_LIMIT;
lines.push(`- ${remaining} more finding${remaining === 1 ? '' : 's'} in the details below.`);
}
return lines;
}
function renderDetailedFindings(findings) {
if (findings.length === 0) {
return [];
}
const lines = [];
for (const severity of SEVERITY_ORDER) {
const scopedFindings = findings.filter((finding) => finding.severity === severity);
if (scopedFindings.length === 0) continue;
lines.push(`**${SEVERITY_SUMMARY_LABELS[severity]} (${scopedFindings.length})**`, '');
for (const finding of scopedFindings) {
lines.push(`- **${renderCode(renderFindingReference(finding))}${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(...renderFindingSnippets(finding.snippets));
lines.push('');
}
}
if (lines[lines.length - 1] === '') {
lines.pop();
}
return lines;
}
export function renderStructuredReview(review, { model, rendering: renderOptions } = {}) {
const rendering = mergeRenderingMetadata(review?.rendering, renderOptions);
const lines = ['### 📋 Summary', '', renderInlineText(review.summary), '', '### 🔍 Findings'];
const lines = [
'### Verdict',
'',
`**${ASSESSMENTS[review.overallAssessment]}** — ${renderInlineText(review.overallRationale)}`,
'',
renderInlineText(review.summary),
];
const reviewContext = formatReviewContext(rendering);
if (reviewContext) {
lines.splice(4, 0, reviewContext, '');
lines.push('', reviewContext);
}
if (review.findings.length === 0) {
lines.push('No confirmed issues found after reviewing the diff and surrounding code.');
} else {
for (const severity of SEVERITY_ORDER) {
const findings = review.findings.filter((finding) => finding.severity === severity);
if (findings.length === 0) continue;
lines.push('', SEVERITY_HEADERS[severity], '');
for (const finding of findings) {
const location = finding.line ? `${finding.file}:${finding.line}` : finding.file;
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(...renderFindingSnippets(finding.snippets));
lines.push('');
}
}
if (lines[lines.length - 1] === '') lines.pop();
}
lines.push(...renderChecklistTable('### 🔒 Security Checklist', 'Check', 'check', review.securityChecklist));
lines.push(...renderChecklistTable('### 📊 CCS Compliance', 'Rule', 'rule', review.ccsCompliance));
lines.push(...renderBulletSection('### 💡 Informational', review.informational));
lines.push(...renderBulletSection("### ✅ What's Done Well", review.strengths));
lines.push('', '### Top Findings', '', ...renderTopFindings(review.findings));
lines.push(...renderDetailsBlock(`Full Findings (${review.findings.length})`, renderDetailedFindings(review.findings)));
lines.push(
...renderDetailsBlock(
`Security Checklist (${review.securityChecklist.length})`,
renderChecklistTable('Check', 'check', review.securityChecklist)
)
);
lines.push(
...renderDetailsBlock(
`CCS Compliance (${review.ccsCompliance.length})`,
renderChecklistTable('Rule', 'rule', review.ccsCompliance)
)
);
lines.push(...renderDetailsBlock(`Informational (${review.informational.length})`, renderBulletSection(review.informational)));
lines.push(...renderDetailsBlock(`What's Done Well (${review.strengths.length})`, renderBulletSection(review.strengths)));
lines.push(
'',
'### 🎯 Overall Assessment',
'',
`**${ASSESSMENTS[review.overallAssessment]}** — ${renderInlineText(review.overallRationale)}`,
'',
`> 🤖 Reviewed by \`${model}\``
);
+3 -3
View File
@@ -283,7 +283,7 @@ export async function writeDirectReviewFromEnv(env = process.env, fetchImpl = gl
const responseJson = await postReviewRequest({
apiUrl: cleanText(env.ANTHROPIC_BASE_URL),
apiKey: cleanText(env.ANTHROPIC_AUTH_TOKEN),
model: cleanText(env.REVIEW_MODEL || env.ANTHROPIC_MODEL || 'glm-5.1'),
model: cleanText(env.REVIEW_MODEL || env.ANTHROPIC_MODEL || 'glm-5-turbo'),
system,
prompt: attemptPrompt,
timeoutMs: attemptWindow.timeoutMs,
@@ -318,11 +318,11 @@ export async function writeDirectReviewFromEnv(env = process.env, fetchImpl = gl
const markdown = finalValidation
? renderStructuredReview(finalValidation, {
model: cleanText(env.REVIEW_MODEL || 'glm-5.1'),
model: cleanText(env.REVIEW_MODEL || 'glm-5-turbo'),
rendering,
})
: renderIncompleteReview({
model: cleanText(env.REVIEW_MODEL || 'glm-5.1'),
model: cleanText(env.REVIEW_MODEL || 'glm-5-turbo'),
reason: lastReason,
runUrl: cleanText(env.AI_REVIEW_RUN_URL || '#'),
selectedFiles: coveredSelectedFiles,
@@ -52,21 +52,22 @@ describe('normalize-ai-review-output', () => {
);
expect(validation.ok).toBe(true);
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5.1' });
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5-turbo' });
expect(markdown).toContain('### 📋 Summary');
expect(markdown).toContain('### 🔴 High');
expect(markdown).toContain('### Verdict');
expect(markdown).toContain('### Top Findings');
expect(markdown).toContain('- 🔴 High `src/cliproxy/accounts/query.ts:61` — Ambiguous account lookup drops valid matches');
expect(markdown).toContain('<summary>Full Findings (1)</summary>');
expect(markdown).toContain('**`src/cliproxy/accounts/query.ts:61` — Ambiguous account lookup drops valid matches**');
expect(markdown).toContain('### 🔒 Security Checklist');
expect(markdown).toContain('<summary>Security Checklist (1)</summary>');
expect(markdown).toContain('| Injection safety | ✅ | No user-controlled input reaches a shell, SQL, or HTML boundary in this diff. |');
expect(markdown).toContain('### 📊 CCS Compliance');
expect(markdown).toContain('<summary>CCS Compliance (1)</summary>');
expect(markdown).toContain('| No emojis in CLI | N/A | This change affects GitHub PR comments only, not CLI stdout. |');
expect(markdown).toContain('### 💡 Informational');
expect(markdown).toContain('### ✅ What\'s Done Well');
expect(markdown).toContain('### 🎯 Overall Assessment');
expect(markdown).toContain('<summary>Informational (1)</summary>');
expect(markdown).toContain("<summary>What's Done Well (1)</summary>");
expect(markdown).toContain('**❌ CHANGES REQUESTED**');
expect(markdown).toContain('Why it matters: That breaks normal selection flows for users with multiple Codex sessions.');
expect(markdown).toContain('> 🤖 Reviewed by `glm-5.1`');
expect(markdown).toContain('> 🤖 Reviewed by `glm-5-turbo`');
});
test('renders mode-aware review context metadata without changing the structured review contract', () => {
@@ -97,7 +98,7 @@ describe('normalize-ai-review-output', () => {
expect(validation.ok).toBe(true);
const markdown = reviewOutput.renderStructuredReview(validation.value, {
model: 'glm-5.1',
model: 'glm-5-turbo',
rendering: {
mode: 'triage',
selectedFiles: 8,
@@ -113,7 +114,7 @@ describe('normalize-ai-review-output', () => {
});
expect(markdown).toContain(
'> 🧭 Review context: mode `triage`; expanded packaged review with broader coverage; scope 8/34 reviewable files; 620/2140 reviewable changed lines; packet 6/8 selected files included in the final review packet; 2 selected files omitted for packet budget; turn budget 6 turns; workflow cap 5 minutes.'
'> 🧭 `triage` • 8/34 files 620/2140 lines packet 6/8 • 6 turns / 5 minutes'
);
expect(markdown).toContain('**⚠️ APPROVED WITH NOTES**');
});
@@ -157,7 +158,7 @@ describe('normalize-ai-review-output', () => {
);
expect(validation.ok).toBe(true);
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5.1' });
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5-turbo' });
expect(markdown).toContain('`buildReviewScope(files, mode)`');
expect(markdown).toContain('`.github/workflows/ai-review.yml`');
@@ -200,7 +201,7 @@ describe('normalize-ai-review-output', () => {
);
expect(validation.ok).toBe(true);
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5.1' });
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5-turbo' });
expect(markdown).toContain('Evidence: Current publish branch');
expect(markdown).toContain('```bash');
@@ -242,7 +243,7 @@ describe('normalize-ai-review-output', () => {
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' });
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5-turbo' });
expect(markdown).toContain(' if value:');
expect(markdown).toContain(' print(value)');
});
@@ -300,7 +301,7 @@ describe('normalize-ai-review-output', () => {
const result = reviewOutput.writeReviewFromEnv({
AI_REVIEW_EXECUTION_FILE: executionFile,
AI_REVIEW_MODEL: 'glm-5.1',
AI_REVIEW_MODEL: 'glm-5-turbo',
AI_REVIEW_MODE: 'triage',
AI_REVIEW_SELECTED_FILES: '10',
AI_REVIEW_REVIEWABLE_FILES: '46',
@@ -363,7 +364,7 @@ describe('normalize-ai-review-output', () => {
const result = reviewOutput.writeReviewFromEnv({
AI_REVIEW_EXECUTION_FILE: executionFile,
AI_REVIEW_MODEL: 'glm-5.1',
AI_REVIEW_MODEL: 'glm-5-turbo',
AI_REVIEW_MODE: 'fast',
AI_REVIEW_SELECTED_FILES: '6',
AI_REVIEW_REVIEWABLE_FILES: '52',
@@ -410,7 +411,7 @@ describe('normalize-ai-review-output', () => {
const result = reviewOutput.writeReviewFromEnv({
AI_REVIEW_EXECUTION_FILE: executionFile,
AI_REVIEW_MODEL: 'glm-5.1',
AI_REVIEW_MODEL: 'glm-5-turbo',
AI_REVIEW_OUTPUT_FILE: outputFile,
AI_REVIEW_RUN_URL: 'https://github.com/kaitranntt/ccs/actions/runs/1',
AI_REVIEW_STRUCTURED_OUTPUT: JSON.stringify({
@@ -507,15 +508,15 @@ describe('normalize-ai-review-output', () => {
);
expect(validation.ok).toBe(true);
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5.1' });
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5-turbo' });
expect(markdown).toContain('### 🔍 Findings');
expect(markdown).toContain('### Top Findings');
expect(markdown).toContain('No confirmed issues found after reviewing the diff and surrounding code.');
expect(markdown).toContain('### 🔒 Security Checklist');
expect(markdown).toContain('<summary>Security Checklist (1)</summary>');
expect(markdown).toContain(
'| Injection safety | ✅ | No user-controlled data crosses a risky boundary in the reviewed diff. |'
);
expect(markdown).toContain('### 📊 CCS Compliance');
expect(markdown).toContain('<summary>CCS Compliance (1)</summary>');
expect(markdown).toContain('| Help/docs alignment | N/A | No CLI behavior changed, so there was nothing to update. |');
expect(markdown).toContain('**✅ APPROVED** — No confirmed regressions or missing verification remain.');
});
@@ -545,7 +546,7 @@ describe('normalize-ai-review-output', () => {
);
expect(validation.ok).toBe(true);
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5.1' });
const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5-turbo' });
expect(markdown).toContain(
'**`tests/unit/scripts/github/normalize-ai-review-output.test.ts` — Missing empty-state coverage**'
@@ -575,7 +576,7 @@ describe('normalize-ai-review-output', () => {
overallAssessment: 'approved_with_notes',
overallRationale: 'This is a formatting-only follow-up.',
},
{ model: 'glm-5.1' }
{ model: 'glm-5-turbo' }
);
expect(markdown).toContain('**``src/weird`path.ts`` — Backtick-safe locations stay readable**');
@@ -60,7 +60,7 @@ describe('run-ai-review-direct', () => {
{
ANTHROPIC_BASE_URL: 'https://api.z.ai/api/anthropic',
ANTHROPIC_AUTH_TOKEN: 'test-token',
REVIEW_MODEL: 'glm-5.1',
REVIEW_MODEL: 'glm-5-turbo',
GITHUB_REPOSITORY: 'kaitranntt/ccs',
AI_REVIEW_PROMPT: 'You are a reviewer.',
AI_REVIEW_PACKET_FILE: packetFile,
@@ -110,7 +110,7 @@ describe('run-ai-review-direct', () => {
{
ANTHROPIC_BASE_URL: 'https://api.z.ai/api/anthropic',
ANTHROPIC_AUTH_TOKEN: 'test-token',
REVIEW_MODEL: 'glm-5.1',
REVIEW_MODEL: 'glm-5-turbo',
GITHUB_REPOSITORY: 'kaitranntt/ccs',
AI_REVIEW_PROMPT: 'You are a reviewer.',
AI_REVIEW_PACKET_FILE: packetFile,
@@ -147,11 +147,9 @@ describe('run-ai-review-direct', () => {
);
expect(result.usedFallback).toBe(false);
expect(fs.readFileSync(outputFile, 'utf8')).toContain('### 📋 Summary');
expect(fs.readFileSync(outputFile, 'utf8')).toContain('### Verdict');
expect(fs.readFileSync(outputFile, 'utf8')).toContain('**✅ APPROVED**');
expect(fs.readFileSync(outputFile, 'utf8')).toContain(
'packet 1/1 selected files included in the final review packet'
);
expect(fs.readFileSync(outputFile, 'utf8')).toContain('packet 1/1');
expect(JSON.parse(fs.readFileSync(logFile, 'utf8')).attempts).toHaveLength(1);
});
});
@@ -171,7 +169,7 @@ describe('run-ai-review-direct', () => {
{
ANTHROPIC_BASE_URL: 'https://api.z.ai/api/anthropic',
ANTHROPIC_AUTH_TOKEN: 'test-token',
REVIEW_MODEL: 'glm-5.1',
REVIEW_MODEL: 'glm-5-turbo',
GITHUB_REPOSITORY: 'kaitranntt/ccs',
AI_REVIEW_PROMPT: 'You are a reviewer.',
AI_REVIEW_PACKET_FILE: packetFile,
@@ -263,7 +261,7 @@ describe('run-ai-review-direct', () => {
{
ANTHROPIC_BASE_URL: 'https://api.z.ai/api/anthropic',
ANTHROPIC_AUTH_TOKEN: 'test-token',
REVIEW_MODEL: 'glm-5.1',
REVIEW_MODEL: 'glm-5-turbo',
GITHUB_REPOSITORY: 'kaitranntt/ccs',
AI_REVIEW_PROMPT: 'You are a reviewer.',
AI_REVIEW_PACKET_FILE: packetFile,
@@ -289,9 +287,7 @@ describe('run-ai-review-direct', () => {
expect(result.usedFallback).toBe(false);
expect(fs.readFileSync(outputFile, 'utf8')).toContain('**⚠️ APPROVED WITH NOTES**');
expect(fs.readFileSync(outputFile, 'utf8')).toContain(
'packet 2/3 selected files included in the final review packet; 1 selected file omitted for packet budget'
);
expect(fs.readFileSync(outputFile, 'utf8')).toContain('packet 2/3');
expect(JSON.parse(fs.readFileSync(logFile, 'utf8')).attempts).toHaveLength(2);
});
});