Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
127 changes: 8 additions & 119 deletions src/create-prompt/templates/review-candidates-prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,25 @@ export function generateReviewCandidatesPrompt(
const includeSuggestions = context.includeSuggestions !== false;

const bodyFieldDescription = includeSuggestions
? " - `body`: Comment text starting with priority tag [P0|P1|P2], then title, then 1 paragraph explanation\n" +
" If you have **high confidence** a fix will address the issue and won't break CI, append a GitHub suggestion block:\n" +
"\n" +
" ```suggestion\n" +
" <replacement code>\n" +
" ```\n" +
"\n" +
" **Suggestion rules:**\n" +
" - Keep suggestion blocks ≤ 100 lines\n" +
" - Preserve exact leading whitespace\n" +
" - Use RIGHT-side anchors only; do not include removed/LEFT-side lines\n" +
" - For insert-only suggestions, repeat the anchor line unchanged, then append new lines"
? " - `body`: Comment text starting with priority tag [P0|P1|P2], then title, then 1 paragraph explanation.\n" +
" Follow the suggestion block rules from the review skill when including suggestions."
: " - `body`: Comment text starting with priority tag [P0|P1|P2], then title, then 1 paragraph explanation";

const sideFieldDescription = includeSuggestions
? ' - `side`: "RIGHT" for new/modified code (default). Use "LEFT" only for removed code **without** suggestions.\n' +
" If you include a suggestion block, choose a RIGHT-side anchor and keep it unchanged so the validator can reuse it."
: ' - `side`: "RIGHT" for new/modified code (default), "LEFT" only for removed code';

const skillInstruction = includeSuggestions
? "Invoke the 'review' skill to load the review methodology, then execute its **Pass 1: Candidate Generation** procedure — including suggestion block rules."
: "Invoke the 'review' skill to load the review methodology, then execute its **Pass 1: Candidate Generation** procedure. Do NOT include code suggestion blocks.";

return `You are a senior staff software engineer and expert code reviewer.

Your task: Review PR #${prNumber} in ${repoFullName} and generate a JSON file with **high-confidence, actionable** review comments that pinpoint genuine issues.

${skillInstruction}

<context>
Repo: ${repoFullName}
PR Number: ${prNumber}
Expand All @@ -66,113 +62,6 @@ Precomputed data files:
- Existing Comments: \`${commentsPath}\`
</context>

<understanding_phase>
**Step 0: Understand the PR intent**

1. Read the PR description from \`${descriptionPath}\` to understand the purpose and scope of the changes.
2. If the PR description contains a ticket URL (e.g., Jira, Linear, GitHub issue link) or a ticket ID, **always fetch it** using FetchUrl or the appropriate tool to understand the full requirements and acceptance criteria. This context is critical for evaluating whether the implementation is correct and complete.
</understanding_phase>

<review_guidelines>
- You are currently checked out to the PR branch.
- Review ALL modified files in the PR branch.
- Focus on: functional correctness, syntax errors, logic bugs, broken dependencies/contracts/tests, security issues, and performance problems.
- High-signal bug patterns to actively check for (only comment when evidenced in the diff):
- Null/undefined/Optional dereferences; missing-key errors on untrusted/external dict/JSON payloads
- Resource leaks (unclosed files/streams/connections; missing cleanup on error paths)
- Injection vulnerabilities (SQL injection, XSS, command/template injection) and auth/security invariant violations
- OAuth/CSRF invariants: state must be per-flow unpredictable and validated; avoid deterministic/predictable state or missing state checks
- Concurrency/race/atomicity hazards (TOCTOU, lost updates, unsafe shared state, process/thread lifecycle bugs)
- Missing error handling for critical operations (network, persistence, auth, migrations, external APIs)
- Wrong-variable/shadowing mistakes; contract mismatches (serializer/validated_data, interfaces/abstract methods)
- Type-assumption bugs (e.g., numeric ops on datetime/strings, ordering key type mismatches)
- Offset/cursor/pagination semantic mismatches (off-by-one, prev/next behavior, commit semantics)
- Do NOT duplicate comments already in \`${commentsPath}\`.
- Only flag issues you are confident about—avoid speculative or stylistic nitpicks.
</review_guidelines>

<triage_phase>
**Step 1: Analyze and group the modified files**

Before reviewing, you must triage the PR to enable parallel review:

1. Read the diff file (\`${diffPath}\`) to identify ALL modified files
2. Group the files into logical clusters based on:
- **Related functionality**: Files in the same module or feature area
- **File relationships**: A component and its tests, a class and its interface
- **Risk profile**: Security-sensitive files together, database/migration files together
- **Dependencies**: Files that import each other or share types

3. Document your grouping briefly, for example:
- Group 1 (Auth): src/auth/login.ts, src/auth/session.ts, tests/auth.test.ts
- Group 2 (API handlers): src/api/users.ts, src/api/orders.ts
- Group 3 (Database): src/db/migrations/001.ts, src/db/schema.ts

Guidelines for grouping:
- Aim for 3-6 groups to balance parallelism with context coherence
- Keep related files together so reviewers have full context
- Each group should be reviewable independently
</triage_phase>

<parallel_review_phase>
**Step 2: Spawn parallel subagents to review each group**

After grouping, use the Task tool to spawn parallel \`file-group-reviewer\` subagents. Each subagent will review one group of files independently.

**IMPORTANT**: Spawn ALL subagents in a single response to enable parallel execution.

For each group, invoke the Task tool with:
- \`subagent_type\`: "file-group-reviewer"
- \`description\`: Brief label (e.g., "Review auth module")
- \`prompt\`: Must include:
1. The PR context (repo, PR number, base/head refs)
2. The list of assigned files for this group
3. The relevant diff sections for those files (extract from \`${diffPath}\`)
4. Instructions to return a JSON array of findings

Example Task invocation for one group:
\`\`\`
Task(
subagent_type: "file-group-reviewer",
description: "Review auth module",
prompt: """
Review the following files from PR #${prNumber} in ${repoFullName}.

PR Context:
- Head SHA: ${prHeadSha}
- Base Ref: ${prBaseRef}

Assigned files:
- src/auth/login.ts
- src/auth/session.ts
- tests/auth.test.ts

Diff for these files:
<paste relevant diff sections here>

Return a JSON array of issues found. If no issues, return [].
"""
)
\`\`\`

Spawn all group reviewers in parallel by including multiple Task calls in one response.
</parallel_review_phase>

<aggregation_phase>
**Step 3: Aggregate subagent results**

After all subagents complete, collect and merge their findings:

1. **Collect results**: Each subagent returns a JSON array of comment objects
2. **Merge arrays**: Combine all arrays into a single comments array
3. **Add commit_id**: Add \`"commit_id": "${prHeadSha}"\` to each comment object
4. **Deduplicate**: If multiple subagents flagged the same location (same path + line), keep only one comment (prefer higher priority: P0 > P1 > P2)
5. **Filter existing**: Remove any comments that duplicate issues already in \`${commentsPath}\`
6. **Write reviewSummary**: Synthesize a 1-3 sentence overall assessment based on all findings

Write the final aggregated result to \`${reviewCandidatesPath}\` using the schema in \`<output_spec>\`.
</aggregation_phase>

<output_spec>
Write output to \`${reviewCandidatesPath}\` using this exact schema:

Expand Down
107 changes: 20 additions & 87 deletions src/create-prompt/templates/review-validator-prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,16 @@ export function generateReviewValidatorPrompt(

const includeSuggestions = context.includeSuggestions !== false;

const suggestionBlockRules = includeSuggestions
? "\n\nSuggestion block rules (minimal):\n" +
"* Preserve exact leading whitespace and keep blocks ≤ 100 lines\n" +
"* Use RIGHT-side anchors only; do not include removed/LEFT-side lines\n" +
"* For insert-only suggestions, repeat the anchor line unchanged, then append new lines\n" +
"* Do not change the anchor fields (path/side/line/startLine) from the candidate — only edit the body"
: "";
const skillInstruction = includeSuggestions
? "Invoke the 'review' skill to load the review methodology, then execute its **Pass 2: Validation** procedure — including suggestion block rules."
: "Invoke the 'review' skill to load the review methodology, then execute its **Pass 2: Validation** procedure. Do NOT include code suggestion blocks.";

return `You are validating candidate review comments for PR #${prNumber} in ${repoFullName}.

IMPORTANT: This is Phase 2 (validator) of a two-pass review pipeline.

${skillInstruction}

### Context

* Repo: ${repoFullName}
Expand All @@ -54,91 +52,30 @@ IMPORTANT: This is Phase 2 (validator) of a two-pass review pipeline.

### Inputs

Read:
Read these files before validating:
* PR Description: \`${descriptionPath}\`
* Candidates: \`${reviewCandidatesPath}\`
* Full PR Diff: \`${diffPath}\`
* Existing Comments: \`${commentsPath}\`

### Outputs

1) Write validated results to: \`${reviewValidatedPath}\`
2) Post ONLY the approved inline comments to the PR
3) Submit a PR review summary (if applicable)

=======================
If the diff is large, read in chunks (offset/limit). **Do not proceed until you have read the ENTIRE diff.**

## CRITICAL REQUIREMENTS
### Critical Requirements

1. You MUST read and validate **every** candidate before posting anything.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are ci specfic

2. For each candidate, confirm:
* It is a real, actionable bug (not speculative)
* There is a realistic trigger path and observable wrong behavior
* The anchor is valid (path + side + line/startLine correspond to the diff)
3. **Posting rule (STRICT):**
* Only post comments where \`status === "approved"\`.
* Never post rejected items.
4. Preserve ordering: keep results in the same order as candidates.

=======================

## Phase 1: Load context (REQUIRED)

1. Read the PR description:
Read \`${descriptionPath}\`

2. Read existing comments:
Read \`${commentsPath}\`

3. Read the COMPLETE diff:
Read \`${diffPath}\`
If large, read in chunks (offset/limit). **Do not proceed until you have read the ENTIRE diff.**

4. Read candidates:
Read \`${reviewCandidatesPath}\`

=======================

## Phase 2: Validate candidates
2. Preserve ordering: keep results in the same order as candidates.
3. **Posting rule (STRICT):** Only post comments where \`status === "approved"\`. Never post rejected items.

Apply the same Reporting Gate as review:

### Approve ONLY if at least one is true
* Definite runtime failure
* Incorrect logic with a concrete trigger path and wrong outcome
* Security vulnerability with realistic exploit
* Data corruption/loss
* Breaking contract change (discoverable in code/tests)

Reject if:
* It's speculative / "might" without a concrete trigger
* It's stylistic / naming / formatting
* It's not anchored to a valid changed line
* It's already reported (dedupe against existing comments)

### Deduplication (STRICT)

Before approving a candidate, check for duplicates:
1. **Among candidates**: If two or more candidates describe the same underlying bug (same root cause, even if anchored to different lines or worded differently), approve only the ONE with the best anchor and clearest explanation. Reject the rest with reason "duplicate of candidate N".
2. **Against existing comments**: If a candidate repeats an issue already covered by an existing PR comment (from \`${commentsPath}\`), reject it with reason "already reported in existing comments".
3. Same file + overlapping line range + same issue = duplicate, even if the body text differs.${suggestionBlockRules}

When rejecting, write a concise reason.

=======================

## Phase 3: Write review_validated.json (REQUIRED)

Write \`${reviewValidatedPath}\` with this schema:
### Output: Write \`${reviewValidatedPath}\`

\`\`\`json
{
"version": 1,
"meta": {
"repo": "owner/repo",
"prNumber": 123,
"headSha": "<head sha>",
"baseRef": "main",
"repo": "${repoFullName}",
"prNumber": ${prNumber},
"headSha": "${prHeadSha}",
"baseRef": "${prBaseRef}",
"validatedAt": "<ISO timestamp>"
},
"results": [
Expand All @@ -150,7 +87,7 @@ Write \`${reviewValidatedPath}\` with this schema:
"line": 42,
"startLine": null,
"side": "RIGHT",
"commit_id": "<head sha>"
"commit_id": "${prHeadSha}"
}
},
{
Expand All @@ -161,14 +98,14 @@ Write \`${reviewValidatedPath}\` with this schema:
"line": 10,
"startLine": null,
"side": "RIGHT",
"commit_id": "<head sha>"
"commit_id": "${prHeadSha}"
},
"reason": "Not a real bug because ..."
}
],
"reviewSummary": {
"status": "approved",
"body": "13 sentence overall assessment"
"body": "1-3 sentence overall assessment"
}
}
\`\`\`
Expand All @@ -177,21 +114,17 @@ Notes:
* Use \`commit_id\` = \`${prHeadSha}\`.
* \`results\` MUST have exactly one entry per candidate, in the same order.

Then write the file using the local file tool.

Tooling note:
* If the tools list includes \`ApplyPatch\` (common for OpenAI models like GPT-5.2), use \`ApplyPatch\` to create/update the file at the exact path.
* Otherwise, use \`Create\` (or \`Edit\` if overwriting) to write the file.

=======================

## Phase 4: Post approved items
### Post approved items

After writing \`${reviewValidatedPath}\`, post comments ONLY for \`status === "approved"\`:

* Collect all approved comments and submit them as a **single batched review** via \`github_pr___submit_review\`, passing them in the \`comments\` array parameter.
* Do **NOT** post comments individually — batch them all into one \`submit_review\` call.
* do **NOT** include a \`body\` parameter in \`submit_review\`.
* Do **NOT** include a \`body\` parameter in \`submit_review\`.
* Use \`github_comment___update_droid_comment\` to update the tracking comment with the review summary.
* Do **NOT** post the summary as a separate comment or as the body of \`submit_review\`.
* Do not approve or request changes.
Expand Down
Loading