Skip to content

Commit 3fb94f4

Browse files
committed
fix: semantically structure judge critique
1 parent 3de3a5d commit 3fb94f4

5 files changed

Lines changed: 96 additions & 4 deletions

File tree

api/src/judge/judge.model.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ export interface JudgeAssessInput extends JudgePromptContext {
2323
export interface JudgeVerdict {
2424
/** True when every part of the goal is evidenced in the committed changes (or genuinely blocked). */
2525
passed: boolean;
26-
/** When not passed, a specific, actionable list of what remains — fed verbatim to the next agent. */
26+
/** When not passed, a specific, actionable list of what remains — fed to the next agent (with its
27+
* heading hierarchy re-leveled to nest cleanly under the prompt). Stored verbatim for humans. */
2728
critique: string;
2829
}
2930

api/src/judge/judge.prompts.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export function buildJudgePrompt(ctx: JudgePromptContext): string {
2020
2121
## Critique
2222
23-
Under that heading, if not passed, write a concise, specific, actionable checklist of exactly what still needs doing — name files, functions, and the concrete remaining steps. This is plain markdown, so code fences, quotes and lists are all fine and need no escaping; it is handed verbatim to the next agent as its to-do list. If passed, instead write a short summary of what you found. Keep the JSON block exactly as shown — \`passed\` is the only field, and the critique never goes inside it.`,
23+
Under that heading, if not passed, write a concise, specific, actionable checklist of exactly what still needs doing — name files, functions, and the concrete remaining steps. This is plain markdown handed to the next agent as its to-do list, so code fences, quotes and lists are all fine and need no escaping (the verdict above is the only JSON). Use nested headings/sub-bullets freely to group the work — the orchestrator re-levels your heading depths to fit the next prompt, so just keep the structure internally consistent. If passed, instead write a short summary of what you found. Keep the JSON block exactly as shown — \`passed\` is the only field, and the critique never goes inside it.`,
2424
]
2525
.filter(Boolean)
2626
.join('\n\n');

api/src/judge/judge.utility.spec.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { parseJudgeVerdict } from './judge.utility.js';
1+
import { parseJudgeVerdict, relevelCritique } from './judge.utility.js';
22

33
describe('parseJudgeVerdict', () => {
44
it('parses a passed verdict (JSON block only)', () => {
@@ -49,3 +49,32 @@ describe('parseJudgeVerdict', () => {
4949
expect(parseJudgeVerdict('no verdict here')).toBeNull();
5050
});
5151
});
52+
53+
describe('relevelCritique', () => {
54+
it('shifts headings so the shallowest sits at the base, preserving relative depth', () => {
55+
expect(relevelCritique('## Tests\nbody\n### Edge cases\nmore', 3)).toBe(
56+
'### Tests\nbody\n#### Edge cases\nmore',
57+
);
58+
});
59+
60+
it('leaves headings inside fenced code blocks untouched', () => {
61+
const input = '## Fix\n```sh\n# this is a shell comment, not a heading\n```\n### Then';
62+
expect(relevelCritique(input, 3)).toBe(
63+
'### Fix\n```sh\n# this is a shell comment, not a heading\n```\n#### Then',
64+
);
65+
});
66+
67+
it('returns the text unchanged when it has no headings', () => {
68+
const input = '- item one\n- item two\n```ts\nconst x = 1;\n```';
69+
expect(relevelCritique(input, 3)).toBe(input);
70+
});
71+
72+
it('is a no-op when the shallowest heading is already at the base', () => {
73+
const input = '### Already\n#### Deeper';
74+
expect(relevelCritique(input, 3)).toBe(input);
75+
});
76+
77+
it('clamps shifted levels at h6', () => {
78+
expect(relevelCritique('##### Deep\n###### Deeper', 3)).toBe('### Deep\n#### Deeper');
79+
});
80+
});

api/src/judge/judge.utility.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,65 @@ export function parseJudgeVerdict(stdout: string): JudgeVerdict | null {
2121
return { passed, critique: extractCritique(stdout) };
2222
}
2323

24+
const HEADING_RE = /^[ \t]*(#{1,6})[ \t]+(.+?)[ \t]*#*$/;
25+
26+
/**
27+
* Re-bases the headings in the judge's freeform critique so the shallowest sits at `base` —
28+
* keeping the critique's own relative hierarchy but nesting it cleanly beneath the surrounding
29+
* prompt sections instead of colliding with them. Headings inside fenced code blocks are left
30+
* untouched. Mirrors the persist_state plugin's _relevel_headings so injected agent text behaves
31+
* consistently across the system. Returns the text unchanged when it has no headings.
32+
*/
33+
export function relevelCritique(text: string, base = 3): string {
34+
const lines = text.split('\n');
35+
36+
const levels: number[] = [];
37+
let inFence = false;
38+
for (const line of lines) {
39+
const s = line.trimStart();
40+
if (s.startsWith('```') || s.startsWith('~~~')) {
41+
inFence = !inFence;
42+
continue;
43+
}
44+
if (inFence) {
45+
continue;
46+
}
47+
const m = HEADING_RE.exec(line);
48+
if (m) {
49+
levels.push(m[1].length);
50+
}
51+
}
52+
53+
if (levels.length === 0) {
54+
return text;
55+
}
56+
57+
const shift = base - Math.min(...levels);
58+
if (shift === 0) {
59+
return text;
60+
}
61+
62+
inFence = false;
63+
return lines
64+
.map((line) => {
65+
const s = line.trimStart();
66+
if (s.startsWith('```') || s.startsWith('~~~')) {
67+
inFence = !inFence;
68+
return line;
69+
}
70+
if (inFence) {
71+
return line;
72+
}
73+
const m = HEADING_RE.exec(line);
74+
if (!m) {
75+
return line;
76+
}
77+
const level = Math.max(1, Math.min(6, m[1].length + shift));
78+
return `${'#'.repeat(level)} ${m[2].trim()}`;
79+
})
80+
.join('\n');
81+
}
82+
2483
/**
2584
* The critique is everything after the JSON verdict block. We strip the fenced ```json {...} ```
2685
* block (or a bare {...} object as a fallback) and an optional leading "Critique" heading,

api/src/orchestrator/orchestrator.service.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { buildVerifyPrompt } from '../verify/verify.prompts.js';
2727
import { parseVerifyCommand } from '../verify/verify.utility.js';
2828
import { VerifyService } from '../verify/verify.service.js';
2929
import { JudgeService } from '../judge/judge.service.js';
30+
import { relevelCritique } from '../judge/judge.utility.js';
3031
import { buildSummaryPrompt } from '../summary/summary.prompts.js';
3132
import { buildPrBody } from '../summary/summary.utility.js';
3233
import { WorkspaceService } from '../workspace/workspace.service.js';
@@ -1129,7 +1130,9 @@ export class OrchestratorService {
11291130
break;
11301131
}
11311132

1132-
critique = verdict.critique;
1133+
// Re-level the judge's critique headings so they nest under the next prompt's sections
1134+
// instead of colliding with them; the verbatim critique stays in the run's stdout.
1135+
critique = relevelCritique(verdict.critique);
11331136

11341137
this.logger.log(
11351138
`[job ${p.job.id}] ${p.phase} judged incomplete (attempt ${attempt}); continuing with judge critique`,

0 commit comments

Comments
 (0)