Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions src/agent/directors/critique/package.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,29 @@ describe("critiquePackage", () => {
expect(critiquePackage.systemPrompt).toMatch(/PRIMARY INTENT/i);
});

test("systemPrompt is evidence-based and never-fix", () => {
expect(critiquePackage.systemPrompt).toMatch(/evidence-based/i);
expect(critiquePackage.systemPrompt).toMatch(/never fix/i);
expect(critiquePackage.systemPrompt).toMatch(/permanent tests/i);
expect(critiquePackage.systemPrompt).toContain("testsmith/build");
expect(critiquePackage.systemPrompt).toContain("route to build");
test("systemPrompt identity is Critic / CriticDirector (package id stays critique)", () => {
const p = critiquePackage.systemPrompt;
expect(p).toMatch(/CriticDirector \(Critic\)/);
expect(p).toMatch(/review lane only/i);
expect(p).not.toMatch(/CritiqueDirector/);
});

test("systemPrompt is evidence-based defects, never-fix", () => {
const p = critiquePackage.systemPrompt;
expect(p).toMatch(/evidence-based/i);
expect(p).toMatch(/defects with evidence/i);
expect(p).toMatch(/never fix/i);
expect(p).toMatch(/permanent tests/i);
expect(p).toContain("testsmith/build");
expect(p).toContain("route to build");
});

test("systemPrompt has blinders-on / brief-scoped review", () => {
const p = critiquePackage.systemPrompt;
expect(p).toMatch(/BLINDERS ON/i);
expect(p).toMatch(/success_criteria/i);
expect(p).toMatch(/Do not wander/i);
expect(p).toMatch(/invent defects from vibes/i);
});

test("systemPrompt is correctness-only / anti-over-engineering", () => {
Expand Down Expand Up @@ -50,6 +67,19 @@ describe("critiquePackage", () => {
expect(critiquePackage.systemPrompt).toMatch(/Rank these as blocking, not style nits/i);
});

test("systemPrompt has no tool-schema restatement or fake caps", () => {
const p = critiquePackage.systemPrompt;
expect(p).not.toMatch(/parameters?:/i);
expect(p).not.toMatch(/fan-out/i);
expect(p).not.toMatch(/at most \d+/i);
expect(p).not.toMatch(/turn budget/i);
expect(p).not.toMatch(/scheduler/i);
expect(p).not.toMatch(/Prefer grep\/search_files/i);
expect(p).not.toMatch(/Shell find\/rg/i);
expect(p).not.toMatch(/Write tools are not mounted/i);
expect(p).not.toMatch(/via run_shell/i);
});

test("spawn.maySpawn is false", () => {
expect(critiquePackage.spawn.maySpawn).toBe(false);
});
Expand Down
20 changes: 11 additions & 9 deletions src/agent/directors/critique/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import type { DirectorPackage } from "../types.js";
import { REVIEW_TOOLS } from "../tool-sets.js";

/**
* Critique leaf (CL-5819).
* Evidence-based code review — find defects with proof; never fix product code.
* Critique leaf (CL-5819 / CL-7021).
* Critic identity — defects with evidence; never fix product code.
* Package id/path stays `critique` (global rename is out of scope).
*/
export const critiquePackage: DirectorPackage = {
id: "critique",
Expand All @@ -21,18 +22,19 @@ export const critiquePackage: DirectorPackage = {
spawn: { maySpawn: false },
tier: "leaf",
modelRole: "review",
systemPrompt: `You are CritiqueDirector, a specialist in Corbits Code.
systemPrompt: `You are CriticDirector (Critic), a specialist in Corbits Code.

PRIMARY INTENT: evidence-based code review. Find defects; never fix product code. Cite file, line or symbol, what breaks, and the concrete input or sequence that triggers it.
PRIMARY INTENT: evidence-based code review. Find defects with evidence; never fix product code. Cite path, line or symbol, what breaks, and the concrete input or sequence that triggers it.

Before substantial review work: follow style and philosophy conventions (baked; use_skill is not mounted on workers). Read the code under review; do not invent defects from vibes.
You are the review lane only — not an implementer, not an explorer, not an orchestrator. Do not ship fixes. Do not become greybeard or neckbeard as your primary job.

BLINDERS ON: Stay on the brief's success_criteria and the code under review. Do not wander into unrelated files, invent defects from vibes, or expand into architecture/style campaigns outside the ask.

Evidence rules:
- Every claim needs path + line/symbol + reproduction shape (input, sequence, missing branch).
- Prefer grep/search_files/lsp/read_file over shell walks. Shell find/rg -r are blocked — do not work around.
- Rank findings: blocking, should-fix, file-for-later. "This is genuinely fine" is a valid finding when true.
- Call out gaps: what you did not cover so the parent does not assume closed.
- Recommend permanent tests the suite should keep (name the scenario; do not implement them here).
- Recommend permanent tests the suite should keep (name the scenario; do not implement them here — route to testsmith/build).

Correctness-only / anti-over-engineering:
- Flag only gaps that affect correctness or the stated requirements/success_criteria.
Expand All @@ -43,10 +45,10 @@ API contract check (blocking when brief specifies signatures):
- Compare public exports against the brief and existing call sites/tests.
- Sync → async (returning Promise when callers expect a plain value) is a blocking correctness defect.
- Signature parameter order/optionality/return-type drift vs brief is blocking.
- Prefer reading tests/callers; a tiny sync call via run_shell that would hang on a Promise is evidence.
- Prefer reading tests/callers; a tiny sync call that would hang on a Promise is evidence.
- Rank these as blocking, not style nits.

Write tools are not mounted. Repro via read/shell only; recommend permanent tests for testsmith/build.
Before substantial review work: follow style and philosophy conventions (baked; use_skill is not mounted on workers). Read the code under review.

OUT OF LANE → refuse or reclassify under Blockers:
- implementing fixes (route to build)
Expand Down
Loading