feat: add review_code prompt builder

2026-06-11 18:19:36 +01:00
parent 89940c25fb
commit 42a6dfe539
4 changed files with 516 additions and 2 deletions
@@ -28,7 +28,8 @@ Phase 3 in progress.
 - Task 3.2 — Base prompt template (`src/prompts/base.js`, `test/prompts/base.test.js`) ✅
 - Task 3.3 — ask_chatgpt prompt builder (`src/prompts/ask-chatgpt.js`, `test/prompts/ask-chatgpt.test.js`) ✅
 - Task 3.4 — review_plan prompt builder (`src/prompts/review-plan.js`, `test/prompts/review-plan.test.js`) ✅
+- Task 3.5 — review_code prompt builder (`src/prompts/review-code.js`, `test/prompts/review-code.test.js`) ✅

 ## Next Phase

-Phase 3 — in progress; next is Task 3.5 (review_code prompt builder).
+Phase 3 — in progress; next is Task 3.6 (debug_issue prompt builder).
@@ -228,6 +228,26 @@ Create `test/prompts/review-code.test.js`.
 Requirements:
 - Mirror structure of ask-chatgpt and review-plan tests: base composition, review_code-specific instructions, optional fields, expectedOutput, input validation, tool guard, full integration.

+Status: ✅ Complete
+
+### Task 3.6 - debug_issue prompt builder (NEXT)
+
+Create `src/prompts/debug-issue.js`.
+
+Requirements:
+- Export a `buildDebugIssuePrompt(input)` function that returns a tool-specific prompt for the debug_issue MCP tool.
+- Compose with `buildBasePrompt()` — call it internally and append the result with `---` separator.
+- Instruct ChatGPT to analyse errors, logs, failed tests, or stack traces.
+- Look for: likely root causes, fast checks, minimal safe experiments, what not to change yet.
+- Request response in structured format: Likely Causes, Fast Checks, Minimal Safe Experiments, What Not To Do Yet.
+- Support `relevantFiles` and `logs` input fields.
+- No other tool prompts yet (architecture_review, etc.).
+
+Create `test/prompts/debug-issue.test.js`.
+
+Requirements:
+- Mirror structure of ask_chatgpt, review-plan, and review-code tests: base composition, debug_issue-specific instructions, optional fields, expectedOutput, input validation, tool guard, full integration.
+
 Status: ⬜ Pending

 ---
@@ -1 +1,97 @@
-// review_code prompt template.
+// Tool-specific prompt builder for review_code.
+
+import { buildBasePrompt } from "./base.js";
+
+/**
+ * Build a tool-specific prompt for the review_code MCP tool.
+ *
+ * @param {{ question: string, context?: string, constraints?: string[], expectedOutput?: string, projectSummary?: string, taskSummary?: string, relevantFiles?: Array<{path: string, content: string, language?: string}> }} input
+ * @returns {string} A two-part prompt: base system message + review_code user message.
+ */
+export function buildReviewCodePrompt(input) {
+  if (!input || typeof input.question !== "string" || input.question.length === 0) {
+    const err = new TypeError("review_code requires a non-empty 'question' field.");
+    err.kind = "ValidationError";
+    throw err;
+  }
+
+  const base = buildBasePrompt();
+
+  const lines = [
+    "",
+    "---",
+    "",
+    "You are acting as a code reviewer for Claude Code. Claude Code will implement your suggestions — you do not implement anything yourself.",
+    "",
+    "Your task: review the supplied code only and provide a focused, actionable assessment.",
+    "",
+    "Review across these dimensions:",
+    "- Correctness — bugs, logic errors, off-by-one, type coercion issues.",
+    "- Edge cases — boundary values, empty/null inputs, unexpected states.",
+    "- Security — injection, auth bypass, data exposure, unsafe patterns.",
+    "- Maintainability — readability, naming, cohesion, testability.",
+    "- Unnecessary complexity — over-engineering, nested conditionals, abstractions that add no value.",
+    "- Performance — obvious inefficiencies when relevant (N+1, unnecessary allocations, blocking ops).",
+    "- Missing validation or error handling — unchecked inputs, swallowed errors, missing catch blocks.",
+    "- Simpler approaches — more concise or idiomatic alternatives.",
+    "",
+    "Important rules:",
+    "- Claude Code remains the implementation agent. You are acting only as a reviewer.",
+    "- Review only the supplied files and snippets. Do not assume access to the full repository.",
+    "- Do not make assumptions about files, configuration, or architecture that you have not been given.",
+    "- Use only the context provided — call out uncertainty clearly — do not guess about unprovided information.",
+    "",
+    "Respond in this exact structure:",
+    "- Summary: Brief overall assessment of the code quality.",
+    "- Issues by Severity: Grouped into High, Medium, and Low.",
+    "- Suggested Fixes: Small, reviewable changes with brief rationale.",
+    "- Test Suggestions: Specific test cases that should be added or updated.",
+    "",
+  ];
+
+  lines.push(`Question: ${input.question}`);
+
+  if (input.context) {
+    lines.push("");
+    lines.push(`Context: ${input.context}`);
+  }
+
+  if (input.constraints && input.constraints.length > 0) {
+    lines.push("");
+    lines.push("Constraints:");
+    for (const c of input.constraints) {
+      lines.push(`- ${c}`);
+    }
+  }
+
+  if (input.expectedOutput) {
+    lines.push("");
+    lines.push(`Expected output: ${input.expectedOutput}`);
+  }
+
+  if (input.projectSummary) {
+    lines.push("");
+    lines.push(`Project summary: ${input.projectSummary}`);
+  }
+
+  if (input.taskSummary) {
+    lines.push("");
+    lines.push(`Task summary: ${input.taskSummary}`);
+  }
+
+  if (input.relevantFiles && input.relevantFiles.length > 0) {
+    lines.push("");
+    lines.push("Relevant files:");
+
+    for (const file of input.relevantFiles) {
+      lines.push("");
+      lines.push(`File: ${file.path}`);
+      if (file.language) {
+        lines.push(`Language: ${file.language}`);
+      }
+      lines.push(file.content);
+    }
+  }
+
+  return base + "\n" + lines.join("\n");
+}
@@ -0,0 +1,397 @@
+import { describe, it, expect } from "vitest";
+import { buildReviewCodePrompt } from "../../src/prompts/review-code.js";
+
+describe("buildReviewCodePrompt", () => {
+  // --- Basic output contract ---
+
+  it("returns a non-empty string given question-only input", () => {
+    const result = buildReviewCodePrompt({ question: "Review this code" });
+    expect(typeof result).toBe("string");
+    expect(result.length).toBeGreaterThan(0);
+  });
+
+  it("is deterministic — same input always produces the same output", () => {
+    const a = buildReviewCodePrompt({ question: "Review this code" });
+    const b = buildReviewCodePrompt({ question: "Review this code" });
+    expect(a).toBe(b);
+  });
+
+  // --- Composition with base prompt ---
+
+  it("includes the base system prompt content", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toContain("second-opinion assistant");
+    expect(result).toContain("Claude Code");
+    expect(result).toContain("not responsible for editing files");
+  });
+
+  it("contains a '---' separator between base and tool-specific sections", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    const parts = result.split("\n---\n");
+    expect(parts.length).toBe(2);
+  });
+
+  // --- review_code-specific role instructions ---
+
+  it("contains review_code-specific role instruction (code reviewer)", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toContain("code reviewer");
+  });
+
+  it("instructs Claude Code as the implementation agent", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toContain("Claude Code will implement your suggestions");
+  });
+
+  it("instructs ChatGPT that you do not implement anything yourself", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toContain("you do not implement anything yourself");
+  });
+
+  // --- Claude Code / reviewer guard rails ---
+
+  it("states Claude Code remains the implementation agent", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toContain("Claude Code remains the implementation agent");
+  });
+
+  it("states ChatGPT is acting only as a reviewer", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toContain("acting only as a reviewer");
+  });
+
+  it("instructs to review only the supplied files and snippets", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toContain("Review only the supplied files and snippets");
+  });
+
+  it("instructs not to assume access to the full repository", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toMatch(/do not assume access to the full repository/i);
+  });
+
+  it("instructs to call out uncertainty and not guess", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result.toLowerCase()).toContain("uncertainty");
+    expect(result.toLowerCase()).toContain("do not guess");
+  });
+
+  // --- Review dimensions ---
+
+  it("includes correctness dimension", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result.toLowerCase()).toContain("correctness");
+  });
+
+  it("includes edge cases dimension", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result.toLowerCase()).toContain("edge case");
+  });
+
+  it("includes security dimension", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result.toLowerCase()).toContain("security");
+  });
+
+  it("includes maintainability dimension", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result.toLowerCase()).toContain("maintainability");
+  });
+
+  it("includes unnecessary complexity dimension", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result.toLowerCase()).toContain("unnecessary complexity");
+  });
+
+  it("includes performance dimension", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result.toLowerCase()).toContain("performance");
+  });
+
+  it("includes missing validation or error handling dimension", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toContain("Missing validation or error handling");
+  });
+
+  it("includes simpler approaches dimension", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result.toLowerCase()).toContain("simpler approaches");
+  });
+
+  // --- Response structure (TASKS.md section) ---
+
+  it("requests Summary section in response structure", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toMatch(/summary/i);
+  });
+
+  it("requests Issues by Severity section in response structure", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toMatch(/issues\s+by\s+severity/i);
+  });
+
+  it("requests Suggested Fixes section in response structure", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toMatch(/suggested\s+fixes/i);
+  });
+
+  it("requests Test Suggestions section in response structure", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).toMatch(/test\s+suggestions/i);
+  });
+
+  // --- Question field ---
+
+  it("appends the input question verbatim in the task section", () => {
+    const result = buildReviewCodePrompt({ question: "Review pagination implementation" });
+    expect(result).toContain("Question: Review pagination implementation");
+  });
+
+  // --- Optional fields appended correctly (with question) ---
+
+  it("appends Context when context is provided", () => {
+    const result = buildReviewCodePrompt({
+      question: "Review this code",
+      context: "The middleware handles auth.",
+    });
+    expect(result).toContain("Context: The middleware handles auth.");
+  });
+
+  it("does NOT append Context section when context is empty", () => {
+    const result = buildReviewCodePrompt({ question: "Review this code", context: "" });
+    expect(result).not.toContain("Context:");
+  });
+
+  it("appends Constraints list when constraints are provided", () => {
+    const result = buildReviewCodePrompt({
+      question: "Review this code",
+      constraints: ["must be safe", "must not add deps"],
+    });
+    expect(result).toContain("Constraints:");
+    expect(result).toContain("- must be safe");
+    expect(result).toContain("- must not add deps");
+  });
+
+  it("does NOT append Constraints section when constraints array is empty", () => {
+    const result = buildReviewCodePrompt({ question: "Review this code", constraints: [] });
+    expect(result).not.toContain("Constraints:");
+  });
+
+  it("appends Expected output when expectedOutput is provided", () => {
+    const result = buildReviewCodePrompt({
+      question: "Review auth middleware",
+      expectedOutput: "A structured code review with severity ratings.",
+    });
+    expect(result).toContain("Expected output: A structured code review with severity ratings.");
+  });
+
+  it("does NOT append Expected output section when expectedOutput is omitted", () => {
+    const result = buildReviewCodePrompt({ question: "Review auth middleware" });
+    expect(result).not.toContain("Expected output:");
+  });
+
+  it("does NOT append Expected output section when expectedOutput is empty", () => {
+    const result = buildReviewCodePrompt({ question: "Test", expectedOutput: "" });
+    expect(result).not.toContain("Expected output:");
+  });
+
+  it("appends all optional fields alongside each other", () => {
+    const result = buildReviewCodePrompt({
+      question: "Review auth middleware",
+      context: "Current system uses session cookies.",
+      constraints: ["no infra changes", "must support existing IAM"],
+      expectedOutput: "A checklist-style review.",
+      projectSummary: "Internal inventory management system.",
+      taskSummary: "Migrating from sessions to JWT.",
+    });
+    expect(result).toContain("Question: Review auth middleware");
+    expect(result).toContain("Context: Current system uses session cookies.");
+    expect(result).toContain("Constraints:");
+    expect(result).toContain("- no infra changes");
+    expect(result).toContain("- must support existing IAM");
+    expect(result).toContain("Expected output: A checklist-style review.");
+    expect(result).toContain("Project summary: Internal inventory management system.");
+    expect(result).toContain("Task summary: Migrating from sessions to JWT.");
+  });
+
+  it("does NOT append Project summary when projectSummary is omitted", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).not.toContain("Project summary:");
+  });
+
+  it("does NOT append Task summary when taskSummary is omitted", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).not.toContain("Task summary:");
+  });
+
+  // --- relevantFiles (present) ---
+
+  it("includes file section header when relevantFiles is provided", () => {
+    const result = buildReviewCodePrompt({
+      question: "Review pagination",
+      relevantFiles: [{ path: "src/paginate.js", content: "function paginate() {}" }],
+    });
+    expect(result).toContain("Relevant files:");
+  });
+
+  it("includes file path, language, and content when relevantFiles is provided with language", () => {
+    const result = buildReviewCodePrompt({
+      question: "Review pagination",
+      relevantFiles: [
+        { path: "src/paginate.js", language: "javascript", content: "function paginate() {}" },
+      ],
+    });
+    expect(result).toContain("File: src/paginate.js");
+    expect(result).toContain("Language: javascript");
+    expect(result).toContain("function paginate() {}");
+  });
+
+  it("includes file content without language header when language is omitted", () => {
+    const result = buildReviewCodePrompt({
+      question: "Review pagination",
+      relevantFiles: [{ path: "src/paginate.js", content: "function paginate() {}" }],
+    });
+    expect(result).toContain("File: src/paginate.js");
+    expect(result).not.toContain("Language:");
+    expect(result).toContain("function paginate() {}");
+  });
+
+  it("includes multiple files when relevantFiles has multiple entries", () => {
+    const result = buildReviewCodePrompt({
+      question: "Review pagination",
+      relevantFiles: [
+        { path: "src/paginate.js", language: "javascript", content: "function paginate() {}" },
+        { path: "src/paginate.test.js", language: "javascript", content: "// no tests" },
+      ],
+    });
+    expect(result).toContain("File: src/paginate.js");
+    expect(result).toContain("File: src/paginate.test.js");
+    expect(result).toContain("function paginate() {}");
+    expect(result).toContain("// no tests");
+  });
+
+  // --- relevantFiles (absent/empty) ---
+
+  it("does NOT include Relevant files section when relevantFiles is omitted", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).not.toContain("Relevant files:");
+  });
+
+  it("does NOT include Relevant files section when relevantFiles is empty array", () => {
+    const result = buildReviewCodePrompt({ question: "Test", relevantFiles: [] });
+    expect(result).not.toContain("Relevant files:");
+  });
+
+  // --- Tool-specific guard ---
+
+  it("does NOT contain ask_chatgpt specific content", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result.toLowerCase()).not.toContain("answer the question directly");
+    expect(result.toLowerCase()).not.toContain("second-opinion advisor");
+  });
+
+  it("does NOT contain review_plan specific content", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).not.toContain("review-plan");
+    expect(result).not.toContain("missing steps");
+    expect(result).not.toContain("scope creep");
+  });
+
+  it("does NOT contain debug_issue specific content", () => {
+    const result = buildReviewCodePrompt({ question: "Test" });
+    expect(result).not.toContain("debug_issue");
+    expect(result.toLowerCase()).not.toContain("stack trace");
+  });
+
+  // --- Input validation ---
+
+  it("throws TypeError when input is missing", () => {
+    expect(() => buildReviewCodePrompt()).toThrow(TypeError);
+    expect(() => buildReviewCodePrompt(null)).toThrow(TypeError);
+  });
+
+  it("throws TypeError when question is missing", () => {
+    expect(() => buildReviewCodePrompt({ context: "something" })).toThrow(TypeError);
+  });
+
+  it("throws TypeError when question is empty", () => {
+    expect(() => buildReviewCodePrompt({ question: "" })).toThrow(TypeError);
+  });
+
+  // --- Full integration test ---
+
+  it("returns a well-formed two-part prompt with all fields and relevantFiles", () => {
+    const result = buildReviewCodePrompt({
+      question: "Review JWT auth middleware for security issues",
+      context: "Current system uses session cookies on EC2.",
+      constraints: ["no infra changes", "must support existing IAM"],
+      expectedOutput: "A structured code review with severity ratings.",
+      projectSummary: "Internal inventory management system with multi-tenant isolation.",
+      taskSummary: "Migrating from session-based auth to JWT.",
+      relevantFiles: [
+        {
+          path: "src/middleware/auth.js",
+          language: "javascript",
+          content: "export function auth(req) {\n  const token = req.headers['authorization'];\n  return token;\n}",
+        },
+        {
+          path: "src/middleware/auth.test.js",
+          language: "javascript",
+          content: "// no tests yet\nimport { auth } from './auth';\n",
+        },
+      ],
+    });
+
+    // Base section present
+    expect(result).toContain("second-opinion assistant");
+    expect(result).toContain("Rules:");
+
+    // Separator
+    expect(result).toMatch(/\n---\n/);
+
+    // Tool-specific section present
+    expect(result).toContain("code reviewer");
+    expect(result).toContain("Claude Code remains the implementation agent");
+    expect(result).toMatch(/do not assume access to the full repository/i);
+
+    // All 8 review dimensions present
+    expect(result.toLowerCase()).toContain("correctness");
+    expect(result.toLowerCase()).toContain("edge case");
+    expect(result.toLowerCase()).toContain("security");
+    expect(result.toLowerCase()).toContain("maintainability");
+    expect(result.toLowerCase()).toContain("unnecessary complexity");
+    expect(result.toLowerCase()).toContain("performance");
+    expect(result).toContain("Missing validation or error handling");
+    expect(result.toLowerCase()).toContain("simpler approaches");
+
+    // Response structure present
+    expect(result).toMatch(/summary/i);
+    expect(result).toMatch(/issues\s+by\s+severity/i);
+    expect(result).toMatch(/suggested\s+fixes/i);
+    expect(result).toMatch(/test\s+suggestions/i);
+
+    // All fields present verbatim
+    expect(result).toContain("Question: Review JWT auth middleware for security issues");
+    expect(result).toContain("Context: Current system uses session cookies on EC2.");
+    expect(result).toContain("Constraints:");
+    expect(result).toContain("- no infra changes");
+    expect(result).toContain("- must support existing IAM");
+    expect(result).toContain("Expected output: A structured code review with severity ratings.");
+    expect(result).toContain("Project summary: Internal inventory management system with multi-tenant isolation.");
+    expect(result).toContain("Task summary: Migrating from session-based auth to JWT.");
+
+    // Relevant files present
+    expect(result).toContain("Relevant files:");
+    expect(result).toContain("File: src/middleware/auth.js");
+    expect(result).toContain("Language: javascript");
+    expect(result).toContain("export function auth(req) {");
+    expect(result).toContain("File: src/middleware/auth.test.js");
+    expect(result).toContain("// no tests yet");
+
+    // Does not contain other tool content
+    expect(result.toLowerCase()).not.toContain("answer the question directly");
+    expect(result.toLowerCase()).not.toContain("second-opinion advisor");
+    expect(result).not.toContain("missing steps");
+    expect(result).not.toContain("scope creep");
+  });
+});