diff --git a/src/agent/directors/critic/package.test.ts b/src/agent/directors/critic/package.test.ts index 77d24b946..c2db18999 100644 --- a/src/agent/directors/critic/package.test.ts +++ b/src/agent/directors/critic/package.test.ts @@ -122,4 +122,43 @@ describe("criticPackage", () => { expect(criticPackage.outOfLane).toContain("DESIGN.md"); expect(criticPackage.outOfLane).toContain("pedantic fun without evidence"); }); + + test("systemPrompt has Session Initialization block before PRIMARY INTENT", () => { + const p = criticPackage.systemPrompt; + expect(p).toContain("Session Initialization"); + expect(p).toContain("Load the style skill with use_skill"); + expect(p).toContain("Load the philosophy skill with use_skill"); + expect(p).toContain( + "Do not do anything else before you have done all steps above. Skills are active constraints, not background documentation.", + ); + expect(p.indexOf("Session Initialization")).toBeLessThan( + p.indexOf("PRIMARY INTENT"), + ); + }); + + test("systemPrompt labels VERIFIED/HIGH/MEDIUM and refuses LOW findings", () => { + const p = criticPackage.systemPrompt; + expect(p).toContain( + "Confidence level: VERIFIED (proven by tests), HIGH (strong evidence but not testable), MEDIUM (plausible but uncertain)", + ); + expect(p).toContain("Do not report LOW confidence findings"); + expect(p).toContain("Do not report speculative concerns"); + expect(p).toMatch(/never conflated/); + }); + + test("systemPrompt discards low-confidence noise directly", () => { + const p = criticPackage.systemPrompt; + expect(p).toContain( + "Only report VERIFIED, HIGH, or MEDIUM findings. Do not waste time with unverified speculation. Discard low-confidence findings", + ); + }); + + test("systemPrompt hunts keepers and routes to testsmith/builder without committing", () => { + const p = criticPackage.systemPrompt; + expect(p).toContain( + "Actively look for opportunities to recommend tests for permanent inclusion", + ); + expect(p).toContain("route keepers to testsmith/builder"); + expect(p).toContain("never commit them from here"); + }); }); diff --git a/src/agent/directors/critic/package.ts b/src/agent/directors/critic/package.ts index 9ca36ea7a..c3fa5092e 100644 --- a/src/agent/directors/critic/package.ts +++ b/src/agent/directors/critic/package.ts @@ -29,15 +29,25 @@ export const criticPackage: DirectorPackage = { modelRole: "review", systemPrompt: `You are CriticDirector (Critic), a specialist in Corbits Code. +Session Initialization — complete before anything else: +1. Load the style skill with use_skill. +2. Load the philosophy skill with use_skill. +Do not do anything else before you have done all steps above. Skills are active constraints, not background documentation. + PRIMARY INTENT: evidence-based code review including hygiene the diff introduced. Find defects with evidence; never fix product code. Cite path, line or symbol, what breaks, and the concrete input or sequence that triggers it. 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. +Only report VERIFIED, HIGH, or MEDIUM findings. Do not waste time with unverified speculation. Discard low-confidence findings — they are noise. + Evidence rules: - Every claim needs path + line/symbol + reproduction shape (input, sequence, missing branch). - Rank findings: blocking, should-fix, file-for-later. "This is genuinely fine" is a valid finding when true. +- Confidence level: VERIFIED (proven by tests), HIGH (strong evidence but not testable), MEDIUM (plausible but uncertain). +- Do not report LOW confidence findings. Do not report speculative concerns. +- Confidence labels evidence strength while blocking/should-fix/file-for-later ranks severity — the two are never conflated. - 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 — route to testsmith/builder). @@ -45,7 +55,7 @@ Verify by temporary test — hypotheses need evidence, not vibes: - Form hypotheses first: name each suspected defect before testing it. - Write focused temp tests under 'tmp/critique-tests/' with the repo's own framework, and run them with the existing suite. - A test that disproves a hypothesis discards the finding — report only verified issues. -- Recommend keepers for permanent inclusion (uncovered critical paths, edge cases, regression guards); clean up the rest — route keepers to testsmith/builder, never commit them from here. +- Recommend keepers for permanent inclusion (uncovered critical paths, edge cases, regression guards); clean up the rest — route keepers to testsmith/builder, never commit them from here. Actively look for opportunities to recommend tests for permanent inclusion — this is one of your most valuable contributions. Correctness and this-diff hygiene: - Flag gaps that affect correctness or the stated requirements/success_criteria. @@ -61,7 +71,7 @@ API contract check (blocking when brief specifies signatures): - Prefer reading tests/callers; a tiny sync call that would hang on a Promise is evidence. - Rank these as blocking, not style nits. -Before substantial review work: follow style, philosophy, native-integration, and idiot-proof — load each with skill_search + use_skill only when the brief needs it. Read the code under review. +Before substantial review work: style and philosophy are preloaded above — load native-integration and idiot-proof with skill_search + use_skill only when the brief needs them. Read the code under review. OUT OF LANE → refuse or reclassify under Blockers: - implementing fixes (route to builder)