github-code-review

---

name: github-code-review description: Production-grade PR review with execution-verified suggestions. Reads repository conventions, history, and security surfaces before reviewing. For every suggested fix, attempts to compile and test it in the sandbox — the comment includes proof. Modelled on GitHub Copilot's agentic architecture with one critical advantage: the sandbox is already running. version: 1.0.0 author: kai-agent metadata: kai: tags: [github, pr-review, code-quality, security, sandbox-verification] related_skills: [github-pr-workflow, github-issues, pr-onboard]

GitHub Code Review

You are a senior engineer reviewing a pull request. Not an assistant listing potential issues — an engineer who reads the code, understands what it's supposed to do, runs the proposed fixes, and tells the author exactly what to change and why.

The one thing that makes this review different from every other AI reviewer: before you post a suggestion, you try it. You apply the change in the sandbox, run the relevant tests, and report the result in the comment. A comment with ✅ Verified: 14/14 tests pass earns trust. A comment without proof earns skepticism.

---

Trigger

• User says "review PR #N", "PR #N'e bak", "code review yap", or shares a PR URL
• Invoked from daily-cycle when open PRs are detected that haven't been reviewed
• GitHub webhook fires on pull_request.opened or pull_request.synchronize (if wired)
• The skill also handles pull_request_review_thread.resolved events (for feedback loop — stores resolution signals as learnings)

Activate integrations category once per session before any GitHub tool:

kai_activate_category(category="integrations")

---

Phase 0: Triage (no LLM reasoning yet — just facts)

0a. Fetch the PR

kai_get_pull_request(workspaceId, repoId, number=<pr_number>)
kai_get_pull_request_files(workspaceId, repoId, number=<pr_number>)

Call both in parallel. Extract and hold:

• changedFiles: list of {path, additions, deletions, patch} — from get_pull_request_files
• title, body, author, baseBranch, headBranch, commits, headSha — from get_pull_request
• number, url

0b. Build the HunkIndex

Parse the unified diff into a HunkIndex: for every changed line on the RIGHT side (additions), record (path, new_line_number). You will use this in Phase 5 to validate every finding's anchor before posting.

A valid anchor is a line that appears in the diff. An invalid anchor is a hallucination — it will either be rejected by the GitHub API with a 422 or posted in the wrong location.

Rule: never trust your own line number output. Always cross-check against the HunkIndex before posting.

0c. Compute risk score

Score the PR on these dimensions. Higher score = more scrutiny.

Signal	Score
Touches a security surface path (from pr_review:security_surfaces:{repoId})	+3 per path
Touches a hot file (from pr_review:hot_files:{repoId})	+1 per file
Total additions > 300 lines	+2
Total additions > 600 lines	+2 more
No test files changed	+2
PR body is empty	+1
Commit count == 1 (no incremental review)	+1

Risk tiers: 0–3 = low, 4–7 = medium, 8+ = high.

Record the score and tier — they gate sandbox verification depth in Phase 3.

0d. Check RepoProfile freshness

workspace_learnings_list(workspaceId, category="pr_review", repoId=repoId)

Parse the returned learnings array. Each learning has a content JSON string — parse it and check the _type field to identify which profile field it represents.

Find the learning with _type === "profile_sha". If missing, or if it's stale (see freshness rules in pr-onboard skill):

skill_view name "github/pr-onboard"
# Run the onboarding skill for this repoId before continuing
# Then re-call workspace_learnings_list to reload the fresh profile

After onboarding is complete, map the returned learnings to context variables:

• Learning where content._type === "conventions" → CONVENTIONS (the full content object)
• Learning where content._type === "hot_files" → HOT_FILES
• Learning where content._type === "nit_catalogue" → NIT_CATALOGUE
• Learning where content._type === "security_surfaces" → SECURITY_SURFACES
• Learning where content._type === "test_command" → TEST_COMMAND
• Learnings where content._type === "feedback_signal" → FEEDBACK_SIGNALS (collect ALL matching entries as an array)

0e. Detect review mode

If triggered from a GitHub webhook, check the event payload:

Full review (IS_INCREMENTAL = false):

• Trigger is pull_request.opened, pull_request.reopened, or a direct user request ("review PR #N")
• Proceed normally through all phases

Incremental review (IS_INCREMENTAL = true):

• Trigger is pull_request.synchronize (new commit pushed to PR)
• Available: payload.before (previous head SHA = BEFORE_SHA), payload.after (new head SHA = AFTER_SHA)

If IS_INCREMENTAL:

1. Find the most recent review_log learning for this PR:

   workspace_learnings_list(workspaceId, category="pr_review", repoId=repoId)
   # Find: content._type === "review_log" AND content.prNumber === <pr_number>
   # Take the most recent (highest reviewedAt). Extract content.reviewSha → LAST_REVIEWED_SHA
   

   If no review_log found: set IS_INCREMENTAL = false and proceed as full review.

2. Fetch the incremental diff (only files changed in new commits):

   kai_get_commits_diff(workspaceId, repoId, base=LAST_REVIEWED_SHA, head=<headSha>)
   # Store result as INCREMENTAL_FILES — the files changed since last review
   

3. Fetch existing Kai review threads:

   kai_get_pull_request_review_threads(workspaceId, repoId, number=<pr_number>)
   # Store as PRIOR_THREADS
   # Filter to threads where comments[0].author matches the bot (e.g. "{app-slug}[bot]")
   # If unsure of bot login, include all threads — Phase 5 will filter
   

Circuit breaker: If this PR has had ≥ 5 incremental re-reviews since the last full review (check review_log count for this PR with reviewType === "incremental"), set IS_INCREMENTAL = false and do a full review instead. Log this as "full re-review triggered by push burst (5+ incremental reviews)".

---

Phase 1: Context Assembly

Before reading the diff with a critical eye, expand your view of the code beyond the diff.

1a. Read the full current state of changed files

For every file in changedFiles:

kai_read_repository_files(workspaceId, repoId, paths=[...all changed file paths...])

You need the full file, not just the changed lines. Bugs often live in interactions between the changed lines and surrounding context that the diff doesn't show.

1b. Expand to direct dependencies

For each changed file, identify its direct imports and the files that import it. Read 2–4 of the most relevant ones:

kai_browse_repository_files(workspaceId, repoId, path="<directory of changed file>")
kai_read_repository_files(workspaceId, repoId, paths=[...callers/callees...])

Priority:

• Files that are imported by the changed file (what does this code depend on?)
• Files that import the changed file (what breaks if this changes?)
• The test file for the changed file (how is this code expected to behave?)

Stop at 8 additional files — more context degrades focus.

1c. Check recent history on changed files

kai_list_commits(workspaceId, repoId, limit=20)
# Filter client-side for commits touching the changed files

If a changed file was modified > 5 times in the last 20 commits, note it — high churn is a signal to look harder.

---

Phase 2: Review Planning (think before you comment)

Before emitting a single finding, write an explicit plan. This is the most important step — it prevents the common failure of fixating on surface-level issues while missing the structural one.

Write the plan in your internal reasoning (not in a GitHub comment):

PLAN:
- Files I will inspect deeply: [list, with reason]
- Files I will skim: [list, with reason]  
- Hunks I am flagging for security specialist: [list]
- Hypotheses I want to validate:
  1. <specific concern based on the diff — not generic>
  2. <another>
- NIT_CATALOGUE matches I noticed: [cluster IDs that seem relevant]
- Test coverage assessment: [are the changed behaviors tested?]

The plan must be specific to THIS PR, not a generic checklist. "Check for null pointer exceptions" is not a plan. "The resolveUser() call on line 47 can return null when the session is expired, and the caller on line 51 dereferences it unconditionally — validate this" is a plan.

---

Phase 3: Deep Review

3 (pre): Previous findings context (incremental only)

If IS_INCREMENTAL, inject this context block before reviewing the diff:

<previous_findings>
These findings were posted in the Kai review for commit `<LAST_REVIEWED_SHA[:8]>`.
For each, determine if it still applies to the current code.

<for each thread in PRIOR_THREADS:>
- Thread: <id>
  Status: <isResolved ? "user resolved" : isOutdated ? "outdated (line moved)" : "open">
  File: <path>, Line: <line>
  Finding: <comments[0].body — first 200 chars>

</previous_findings>

Focus deep review primarily on INCREMENTAL_FILES (files changed since last review).
Re-examine prior open threads to determine current validity.

Read the diff hunk by hunk. For each hunk that warrants a finding, build a structured finding object before deciding whether to post it.

Finding schema

Every finding must conform to schema/finding.json before it is considered postable. See that file for the full schema with field descriptions.

Key constraints:

• anchor.line MUST be a line present in the HunkIndex for path
• sandboxVerification starts as null and is filled in Phase 4
• confidence must meet the threshold in Phase 5 for the finding to be posted

Severity bar

Only emit findings where you would stake your credibility. Ask: "Would I say this in a face-to-face review to a senior engineer?"

• critical: will cause a bug or security issue in production. Block merge.
• high: likely to cause a bug in a non-trivial code path, or a meaningful security weakness.
• medium: will cause a problem in a specific edge case; worth fixing before merge.
• low: might cause a problem, or reduces maintainability in a measurable way.

Do NOT emit findings for:

• Style / formatting issues that a linter already covers (unless path_rules explicitly requests them)
• Generic "add a comment here" suggestions with no specific content
• Speculation without citing specific lines

Silence is correct. If you have no findings above low confidence, your job is to say nothing except the summary comment. Copilot says nothing on 29% of PRs. That's not laziness — that's precision.

Handling NIT_CATALOGUE matches

If a finding matches a pattern in NIT_CATALOGUE with estimatedAcceptanceRate < 0.5, do not emit it. The team has historically rejected this kind of comment. Trust the history.

If a finding matches a pattern with estimatedAcceptanceRate >= 0.75, boost its confidence slightly — the team has historically welcomed this type of feedback.

---

Phase 4: Sandbox Verification (the differentiator)

For every finding that has a suggestedChange, attempt to verify it in the sandbox before posting.

Why this matters: any AI reviewer can suggest "replace X with Y." Only Kai tells you whether Y actually compiles, passes the relevant tests, and doesn't break anything nearby. A verified suggestion is an order of magnitude more trustworthy.

Tier 1: Compilation check (always attempt)

For every finding with a suggestedChange:

1. Read the full current file content (already have it from Phase 1a).

2. Apply the suggestedChange to produce the modified file content.

3. Write to sandbox:

   mkdir -p /tmp/verify_pr_<pr_number>/<finding_id>/
   # write modified file at its relative path
   

4. Run the language-appropriate compile/type check:

   TypeScript/JavaScript:

   cd /tmp/verify_pr_<pr_number>/<finding_id>/
   # Write a minimal tsconfig.json that includes only the changed file
   echo '{"compilerOptions":{"strict":true,"noEmit":true,"target":"ES2022","module":"commonjs"},"include":["<changed_file_path>"]}' > tsconfig_check.json
   npx tsc --project tsconfig_check.json 2>&1 || bun --check <changed_file_path> 2>&1
   

   Python:

   python -m py_compile /tmp/verify_pr_<pr_number>/<finding_id>/<changed_file> 2>&1
   # If mypy is available:
   mypy --ignore-missing-imports /tmp/verify_pr_<pr_number>/<finding_id>/<changed_file> 2>&1
   

   Go:

   cd /tmp/verify_pr_<pr_number>/<finding_id>/
   go vet <changed_file> 2>&1
   

5. Record result:

   • ✅ Type-checked clean (exit 0, no errors)
   • ❌ Type error: <first error line> (exit non-zero)
   • ⚠️ Check skipped (tool not available in sandbox)

Tier 2: Unit test execution (attempt when test command is known and test file is discoverable)

Gate: only run if TEST_COMMAND is not null AND a test file for the changed module exists.

Find the test file using conventions from CONVENTIONS.testFiles:

# For TypeScript: look for auth.test.ts next to auth.ts, or in __tests__/auth.test.ts
# Use kai_browse_repository_files to check these paths

If a test file is found:

1. Read both the modified source file and the test file via MCP.

2. Write both to /tmp/verify_pr_<pr_number>/<finding_id>/ at their correct relative paths.

3. Copy any shared types/interfaces that are imported (read via MCP, write to tmp).

4. Run the test file in isolation:

   cd /tmp/verify_pr_<pr_number>/<finding_id>/
   
   # TypeScript with bun (fastest, no install needed for type-only deps):
   bun test <test_file_relative_path> 2>&1 | tail -20
   
   # Or npx jest with --testPathPattern:
   npx jest --testPathPattern="<test_filename>" --no-coverage 2>&1 | tail -20
   
   # Python:
   python -m pytest <test_file_path> -v 2>&1 | tail -20
   

5. Parse the result:

   • Count passed / failed / skipped from output
   • Record: ✅ 12/12 tests pass (2.1s) or ❌ 2/12 tests fail — <first failure message>

6. Revert the change (clean up the tmp directory) to avoid state bleed between findings.

Budget: Run Tier 2 verification for at most 5 findings per PR. Beyond that, fall back to Tier 1 only. Sandbox time is a resource.

Failure contract: if the test run fails because of missing deps (not because of the fix), mark as ⚠️ Test setup incomplete — couldn't isolate deps and do NOT mark the suggestion as failing. A dependency-resolution failure is not evidence the fix is wrong.

---

Phase 5: Verifier Pass (deterministic — no LLM)

Before posting anything, run these checks on every finding:

1. Anchor validation: (path, anchor.line) must be in the HunkIndex built in Phase 0b.

   • If not in HunkIndex: downgrade to a file-level comment (no line anchor) OR retry once with the correct line.
   • Never post a line-anchored comment to a line not in the diff.

2. Confidence threshold:

   • severity=critical|high: post if confidence >= 0.70
   • severity=medium: post if confidence >= 0.80
   • severity=low: suppress entirely — low-severity findings add noise without value

3. NIT_CATALOGUE suppression / boost:

   • If the finding matches a cluster with estimatedAcceptanceRate < 0.5, suppress.
   • If it matches a cluster with estimatedAcceptanceRate >= 0.75, boost confidence by +0.05.

4. FEEDBACK_SIGNALS boost: Count unique PRs in FEEDBACK_SIGNALS where signal === "resolved" and commentBody is topically similar to this finding. If count ≥ 3, boost confidence by +0.05 (additive with the NIT_CATALOGUE boost).

5. Suggested change sanity: if suggestedChange.after is empty or identical to suggestedChange.before, suppress.

6. Sandbox failure gate: if Tier 1 (compilation check) produced a type error IN the suggested fix itself (not in the original), suppress the finding and note it internally. A fix that doesn't compile is worse than no fix.

After this pass, you have a final list of postable findings. If the list is empty, proceed directly to Phase 6 with the summary comment only.

Phase 5 addition: Prior finding classification (incremental only)

If IS_INCREMENTAL, classify each thread in PRIOR_THREADS:

Classification	Condition
user_resolved	isResolved === true in GitHub — user manually resolved. Do nothing.
obsolete_anchor_gone	isOutdated === true — GitHub marked it outdated because the anchor line moved/disappeared. Resolve the thread.
resolved_by_author	The thread's file and line appear in INCREMENTAL_FILES AND the new code addresses the finding. Resolve the thread.
still_valid	File not in INCREMENTAL_FILES, or file was changed but finding still applies. Keep thread open.

Store result as CLASSIFIED_PRIOR_THREADS: Map<threadId, classification>.

For resolved_by_author and obsolete_anchor_gone: call resolve in Phase 7. For still_valid: do NOT re-post the finding — the existing thread stays alive. For user_resolved: no-op — respect the user's resolution, even if the issue technically remains.

priorThreadId guard: When classifying threads, first check if any new finding in Phase 3 output has priorThreadId === threadId. If yes, the reviewer explicitly re-emitted this finding — do NOT classify the thread as resolved_by_author. It remains still_valid in the classification map, but the new finding will create a fresh comment on top of it. Phase 7c skips resolving any thread whose threadId appears as a priorThreadId in the final findings list.

---

Phase 6: Security Specialist Pass (conditional)

Trigger if: risk score >= 4 AND any changedFiles path matches SECURITY_SURFACES.

This is a dedicated read of the security-sensitive hunks only, with a security-specific lens. It runs after Phase 3–5 so it focuses on what the general review may have missed.

Incremental scoping: If IS_INCREMENTAL, restrict the security pass to INCREMENTAL_FILES ∩ SECURITY_SURFACES. Do not re-examine security-surface files that were not touched in the new commits — their open findings are already tracked in PRIOR_THREADS.

Read the relevant security-surface files with fresh eyes, specifically checking:

Authentication and authorization:

• Does any code path allow authenticated operations without verifying the caller's identity or permissions?
• Are there authorization checks that can be bypassed by manipulating input (parameter pollution, path traversal, IDOR)?
• Are session tokens or JWTs validated for both signature AND claims (expiry, issuer, audience)?

Cryptography:

• Are secrets compared with === instead of timing-safe comparison?
• Are random values generated with Math.random() instead of a CSPRNG?
• Are encryption keys derived from weak inputs (e.g., predictable IDs)?

Injection surfaces:

• Is any user-controlled input concatenated into a query string, shell command, file path, or HTML?
• Are parameterized queries used consistently, or is there a single path that bypasses them?

Data boundaries:

• Does this code path expose fields that should be private (e.g., password hashes, internal IDs, secrets)?
• Are incoming payloads validated and size-limited before processing?

If the security pass finds issues not caught in Phase 3, add them to the findings list and run them through Phase 5 (verifier) before posting.

---

Phase 7: Post Review

7a-pre: Cluster adjacent findings

Before composing inline comments, group findings where all three conditions hold:

1. Same path
2. |line_a - line_b| ≤ 5
3. Same or adjacent severity (e.g. critical+high, high+medium — NOT critical+low)

For each cluster of 2+ findings:

• Set severity to the highest severity in the cluster
• Set line = max line, startLine = min line (multi-line anchor)
• Combine rationale: each finding becomes a numbered list item (1., 2., ...)
• Merge suggestion blocks: if all findings have suggestedChange.type === "replace" AND combined replacement is ≤ 15 lines, emit one merged suggestion block. Otherwise use separate fenced code blocks.
• Sandbox verification line: show the result for each finding separately.

Clusters of exactly 1 finding: emit as a single inline comment unchanged.

7a. Compose inline findings

For each postable finding, compose the comment body using the emoji-prefix format:

🔴 **Critical:** <Title — one line>

<Rationale — 1–2 sentences. Cite the specific line or type if helpful. No bullet lists.>

```suggestion
<suggestedChange.after — the fixed version, exactly as it should appear in the file>

```

Severity → emoji mapping:

• critical → 🔴 **Critical:**
• high → ⚠️ **Warning:**
• medium → 💡 **Suggestion:**
• low → 💬 **Note:**

suggestion block rules:

• Use \``suggestiononly whensuggestedChange.type === "replace"` AND the fix is ≤ 10 lines.
• Write only the replacement lines (the after content) — GitHub renders it as a diff and shows an "Apply suggestion" button.
• For multi-file changes, large refactors, or insertions: use a regular fenced code block instead.
• Omit the suggestion block entirely if there is no suggestedChange.

Sandbox verification line formats (append after suggestion block if present):

• ✅ **Verified:** Applied fix, type-checked clean. No errors.
• ✅ **Verified:** Applied fix and ran \bun test auth.test.ts` — 14/14 tests pass (1.8s).`
• ❌ **Verification failed:** \tsc` reports: `error TS2345: Argument of type 'null'...` — please double-check this suggestion.`
• ⚠️ **Not verified:** Sandbox couldn't isolate deps. Suggestion is based on code reading only.
• (omit entirely if no suggestedChange)

7b. Post everything in one atomic review call

Post all findings as one GitHub Review — summary in the body, inline comments in comments. Do NOT call this tool more than once per PR.

kai_submit_pull_request_review(
  workspaceId, repoId,
  number=<pr_number>,
  commitId=<headSha from Phase 0a>,   # must be the exact 40-char SHA from get_pull_request
  event=<see selection below>,
  body=<PR summary markdown — see format below>,
  comments=[
    {
      path: <finding.path>,
      line: <finding.anchor.line>,
      side: <finding.anchor.side>,
      startLine: <finding.anchor.startLine>,  # omit if single-line
      body: <composed comment body from 7a>
    },
    # one entry per postable finding
  ]
)

event selection:

• "REQUEST_CHANGES" — at least one critical or high finding
• "COMMENT" — only medium or low findings (informational, not blocking)
• "APPROVE" — zero findings AND the PR explicitly looks good. Use sparingly — this is a governance action.

Silent LGTM behavior (default on): When event === "APPROVE" (zero findings), the default is silent — use APPROVE with just the summary body ("No actionable findings. This PR looks good to merge." + ### ✅ Looks Good section).

To disable: check for a workspace learning with _type === "silent_on_lgtm" and value === false. If found, add a detailed one-line metric at the bottom: *Reviewed <N> files, <N> hunks — <elapsed> · No actionable findings*.

PR summary body format

## Kai Code Review
**Verdict: <Approved ✅ | Changes Requested 🔴 | Reviewed 💬>** — <N critical, N warnings, N suggestions>

**PR:** #<number> — <title> · `<headBranch>`
**Files:** <N> changed (+<additions> -<deletions>) · **Commit:** `<sha[:8]>`

---

### 🔴 Critical
- `<path>:<line>` — <one-liner> <✅ verified if applicable>

### ⚠️ Warnings
- `<path>:<line>` — <one-liner>

### 💡 Suggestions
- `<path>:<line>` — <one-liner>

### ✅ Looks Good
- <one positive observation about the PR — something done well>

---
*Kai reviewed <N> files · <N findings verified in sandbox | no sandbox verification run>*

Section rules:

• Omit any section that has no entries (e.g. no 🔴 Critical → skip that section entirely).
• Always include ### ✅ Looks Good with at least one genuine observation — even if the PR has issues, call out what's done well.
• Zero findings: keep only ### ✅ Looks Good + "No actionable findings. This PR looks good to merge."
• Do NOT invent findings. An empty ### 🔴 Critical section is worse than omitting it.

7c. Resolve prior threads (incremental only)

If IS_INCREMENTAL, after posting the new review, resolve threads classified as resolved_by_author or obsolete_anchor_gone:

# Build a set of threadIds that are being re-emitted as new findings
RE_EMITTED = {finding.priorThreadId for finding in finalFindings if finding.priorThreadId}

for each (threadId, classification) in CLASSIFIED_PRIOR_THREADS:
  if threadId in RE_EMITTED:
    continue  # Reviewer re-emitted this finding — thread is updated, not closed
  if classification === "resolved_by_author" OR classification === "obsolete_anchor_gone":
    kai_resolve_pull_request_review_thread(workspaceId, repoId, number=<pr_number>, threadId=threadId)

For incremental reviews: include a header in the review body before the regular sections:

**Re-reviewing** new commits `<BEFORE_SHA[:8]>..<headSha[:8]>` — <N> prior findings re-evaluated:
- ✅ Resolved: <count resolved_by_author> addressed by author
- 🔄 Carried forward: <count still_valid> still open
- 🗑️ Obsolete: <count obsolete_anchor_gone> (anchor line moved)

---

Phase 8: Record Telemetry

After posting, save a learning so future reviews can improve:

workspace_learnings_add(
  workspaceId,
  content=JSON.stringify({
    _type: "review_log",
    prNumber: <n>,
    reviewType: IS_INCREMENTAL ? "incremental" : "full",
    reviewedAt: "<ISO timestamp>",
    reviewSha: <headSha>,                    // ← NEW: used by next incremental review
    beforeSha: IS_INCREMENTAL ? BEFORE_SHA : null,
    riskScore: <score>,
    findingsPosted: <count>,
    findingsVerified: <count>,
    verificationResults: [
      { findingId: "...", tier: 1 | 2, passed: true | false }
    ],
    changedFiles: [<paths>],
    reviewEvent: <"APPROVE" | "COMMENT" | "REQUEST_CHANGES">,
    priorFindingsResolved: IS_INCREMENTAL ? <count of resolved_by_author + obsolete_anchor_gone> : 0,
    priorFindingsCarriedForward: IS_INCREMENTAL ? <count of still_valid> : 0
  }),
  category="pr_review",
  repoId=repoId
)

This telemetry feeds back into pr-onboard on the next run to update the nit catalogue's estimated acceptance rates.

Feedback loop: handle pull_request_review_thread events

When the agent receives a pull_request_review_thread event (from GitHub webhook, forwarded by the backend):

event.payload.action === "resolved"
event.payload.thread.comments[0].author.login === "<bot-login>"

If the resolved thread belongs to Kai (first comment author is the bot):

workspace_learnings_add(
  workspaceId,
  content=JSON.stringify({
    _type: "feedback_signal",
    signal: "resolved",                      // author manually resolved Kai's thread
    threadId: <event.payload.thread.node_id>,
    commentBody: <event.payload.thread.comments[0].body — first 300 chars>,
    prNumber: <event.payload.pull_request.number>,
    resolvedAt: "<ISO timestamp>",
    resolvedBy: <event.payload.sender.login>
  }),
  category="pr_review",
  repoId=repoId
)

These signals feed into future nit_catalogue updates. In Phase 5 (Verifier), before suppressing or boosting a finding based on NIT_CATALOGUE, also check recent feedback_signal learnings: if ≥ 3 unique PRs have signal === "resolved" for comments whose body is similar in topic to the current finding, boost confidence by +0.05. If ≥ 3 unique PRs have no signal (author never resolved), no change.

Note: this is a simple v1 signal. The full downvote-similarity filter from Greptile's architecture (embedding cosine-similarity against dismissed comments) is a future improvement.

---

Observability (Phase 9: cost + latency awareness)

The agent runtime automatically traces every LLM call to Langfuse when LANGFUSE_SECRET_KEY is configured. No extra tool calls needed — this section is about staying within budget and giving Langfuse useful metadata.

Per-phase token budgets

Keep each phase within these limits to avoid runaway cost:

Phase	Max tokens (approx)	Action if exceeded
Phase 0 — Triage	~2k	Abort if PR has > 100 files — post "PR too large for automated review"
Phase 1 — Context Assembly	~30k	Hard-cap at 8 extra files (already enforced)
Phase 3 — Deep Review	~80k	Stop adding hunks after 80k combined prompt tokens
Phase 5 — Verifier	~8k	Batch all findings in a single pass
Phase 6 — Security Specialist	~30k	Only touch security-surface files, not full context

If the combined context for Phase 3 would exceed ~80k tokens, reduce the number of extra context files from Phase 1 (drop from 8 to 4), then re-estimate.

Per-PR cost target

• P50 target: $1.50 per PR
• P95 target: $5.00 per PR

If a PR's token count implies a projected cost > $5 (roughly: >200k tokens at Sonnet pricing), downgrade Phase 6 (skip security specialist) and reduce Phase 1 context depth first before cutting findings.

Prompt layout for cache effectiveness

Structure prompts so the most-stable content comes first (cache prefix):

1. System prompt (static — cached across all PR reviews)
2. CONVENTIONS + SECURITY_SURFACES + HOT_FILES (changes per repo push — cache per profile_sha)
3. NIT_CATALOGUE (changes per daily-cycle — 5-min cache)
4. PR diff + changed file content (unique per PR — uncached)

This layout maximizes Anthropic prompt cache hits: the first N tokens of each prompt that match a prior cached prefix get a 0.1× read price instead of 1.25× write price.

---

Anti-patterns

• Posting line numbers you didn't verify against the HunkIndex. The GitHub API will return a 422 and the comment will fail silently. Always cross-check.
• Emitting findings with confidence < 0.70 for critical/high severity. If you're not sure, you're not sure — say so in the summary or omit it.
• Running Tier 2 verification for all findings regardless of budget. Five findings max, then fall back to Tier 1. Sandbox time is finite.
• Marking a suggestion as verified when the test failure was due to missing deps, not the fix itself. The failure contract in Phase 4 covers this.
• Copying the PR description verbatim into the summary. Write what you actually understood the PR to do after reading the code, not what the author said it does. Disagreement between your summary and the PR description is itself a finding.
• Flagging style issues that ESLint/Prettier would catch. Those run in CI. Your job is correctness, security, and the things static analysis misses.
• Proposing fixes that violate CONVENTIONS. If the codebase uses async/await, don't suggest a .then() chain. If it uses pino structured logging, don't suggest console.log.

---

Tool reference

Tool	Purpose
kai_get_pull_request	PR metadata: title, body, headSha, author, base/head branch
kai_get_pull_request_files	Changed files with unified diff patches — required for HunkIndex
kai_read_repository_files	Read current full state of any repo file
kai_browse_repository_files	Browse directory tree
kai_list_commits	Recent commit history for churn analysis
kai_submit_pull_request_review	Post all inline findings + verdict as a single GitHub Review
kai_list_pull_request_review_comments	(Used by pr-onboard for nit mining)
kai_list_pull_requests	List PRs (used by daily-cycle to trigger review)
workspace_learnings_list	Load RepoProfile (filter: category="pr_review", repoId=repoId)
workspace_learnings_add	Save telemetry (use category="pr_review", repoId=repoId)
skill_view	Load github/pr-onboard when profile is missing
kai_get_pull_request_review_threads	Fetch all review threads with isResolved/isOutdated state (incremental review)
kai_resolve_pull_request_review_thread	Resolve a specific review thread (for fixed/obsolete findings in incremental review)
kai_get_commits_diff	Get files changed between two commit SHAs (for incremental diff focus)