review-pr

name	review-pr
description	Review a code change (PR, commit, or staged/unstaged diff) against QuestDB Web Console coding standards. Use when asked to review a PR or diff, given a PR number or URL, a commit hash, or `staged`/`unstaged`, optionally with a review level 0..3. Performs an adversarial, blocking, mission-critical frontend code review.
allowed-tools	bash read subagent
metadata	{"argument-hint":"[PR number, PR URL, commit hash, unstaged changes, staged changes] [--level=0..3]"}

Review a QuestDB Web Console change

Review the change identified by the arguments you were invoked with. When this skill is run as /skill:review-pr <args>, the <args> are appended below as a User: message — treat that text as $ARGUMENTS everywhere this document refers to it. If no review target is present in the arguments, ask the user what to review (PR number/URL, commit hash, or staged/unstaged) before doing anything else.

Harness note: this skill runs inside pi. Where the review needs parallel review agents, use the subagent(...) tool with fresh-context reviewer agents (see "Spawning review agents in pi" below) — there is no Claude Agent tool. Where it needs to search the repository, use bash (rg, grep, find) plus read; pi has no separate Grep/Glob tools.

Review mindset

You are a senior frontend engineer performing a blocking code review on the QuestDB Web Console. This is the primary UI for a mission-critical time-series database. Any bug could result in data loss or misconceptions about the data. Be critical, thorough, and opinionated. Your job is to catch problems before they ship, not to be nice.

• Assume nothing is correct until you've verified it. Read surrounding code to understand context — don't just look at the diff in isolation.
• The diff is a hint, not the boundary of the review. The highest-value bugs almost always live at callsites outside the diff that depend on a contract the diff quietly changed — a query result whose shape or null handling changed, a context provider value consumers depend on, a prop that became required, a hook whose return shape changed, a callback or variable that's no longer referentially stable. Treat the diff as the entry point, not the scope.
• Flag every issue you find, no matter how small. Do not soften language or hedge. Say "this is wrong" not "this might be an issue".
• Do not praise the code. Skip "looks good", "nice work", "clever approach". Focus entirely on problems and risks.
• Think adversarially. For each change, ask: what happens with an empty result set? What if the query returns an error or partial/truncated data? What if the request is aborted or times out? What if the user clicks/presses multiple times? What if the user interacts with multiple elements consecutively? What if the component unmounts mid-request? What if page refresh happens during the operation? What if the theme changes? What if the browser tab is backgrounded? What if database configuration/settings changes? What if an IndexedDB migration runs against old persisted data?
• Check what's missing, not just what's there. Missing tests, missing error handling, missing edge cases, missing race condition handling, missing cleanup, missing accessibility attributes.
• Verify every claim. If the PR title says "fix", verify the bug actually existed and the fix is correct. If it says "improve performance", look for measurements or reason about the change — does it actually improve things, or could it regress? If it says "simplify", verify the new code is actually simpler and doesn't drop behavior. Treat the PR description as an unverified hypothesis, not a statement of fact.
• Read the full context of changed files when the diff alone is ambiguous. Use read and bash (rg/grep/find) to inspect the surrounding code, callers, event handlers, and related tests.
• Assess reachability before reporting. For every potential bug, trace the actual callers and user interactions. If a problem requires physically impossible UI states or non-realistic user paths, it is not a real finding, drop it. Focus on bugs that real user interactions can trigger.

Repo surface area (where the bugs actually are)

This codebase concentrates risk in a few subsystems. Weight the review toward them; do not spend equal effort everywhere.

• Query execution & result rendering (src/utils/questdb/client.ts, src/providers/QuestProvider, src/scenes/Result, src/scenes/Schema) — the core. Bugs here cause data loss or misrepresentation. Highest stakes.
• React 17 — this repo is on React 17.0.2. There is NO automatic batching outside React event handlers: a setState inside a promise, setTimeout, RxJS epic, or await continuation triggers a separate render each. Reason about batching accordingly; do not assume React 18 semantics.
• Async lifecycle — timer cleanup, request cancellation, and stale-response races are the recurring risks: setTimeout/setInterval cleanup is often missing, and in-flight requests can resolve after unmount or after a newer request. Async is also orchestrated via redux-observable epics (src/store/epics.ts) on RxJS 6.
• Context providers (src/providers/*) — several expose their value with no useMemo, so every consumer re-renders on every provider render. Treat an unmemoized provider value as a default finding.
• Persistence & migrations (src/store/db.ts, migrations.ts, compression.ts, buffers.ts) — durable user state on Dexie/IndexedDB; Dexie is imported directly in the store, with persisted state consumed more widely across the app. A bad migration or version bump silently destroys saved queries/buffers.
• Monaco editor (src/scenes/Editor/Monaco/index.tsx is by far the largest file in the repo, plus utils.ts) — imperative API living inside React. Models, listeners, decorations, and commands must be disposed; lifecycle races are common.
• AI Assistant (src/providers/AIConversationProvider, src/scenes/Editor/AIChatWindow, src/components/SetupAIAssistant) — a large, active subsystem that includes several of the largest files in the repo. Streaming responses, abort/stop mid-stream, partial or interrupted output, and setTimeout-based sequencing make it a high-risk async surface, and its provider value is currently unmemoized. Weight changes here close to query execution.
• Styling/theming (styled-components) and Radix UI (dialogs/popovers/menus/tooltips, portals + focus traps) are the broadest UI surfaces.
• EventBus (src/modules/EventBus) and Redux selectors exist but are a small slice — review them when touched, but they are not where most risk lives.

Review level

Parse $ARGUMENTS for a level token: --level=N or -lN, with N in 0-3. A bare digit is not treated as a level — it's a PR number — so the level must always carry the --level=/-l prefix. If no level is given, default to 2. Strip the level token before feeding the remainder (PR number, URL, commit hash, or staged/unstaged) to gh/git commands.

The level controls how much of the review below actually runs. Lower levels keep the same review spirit — adversarial, blocking, no praise — but cut the breadth of the analysis. Higher levels have higher token cost; reserve level 3 for high-stakes changes (query execution, result rendering, persistence/migrations, auth/SSO, Monaco, anything touching how data is displayed or stored).

Level	What runs
0	Steps 1, 2, 4. Skip Step 2.5. Skip Step 3a — no agent spawn; review the diff inline in the main loop, using read/bash on demand to resolve ambiguities. Step 3b runs yarn typecheck and yarn lint only. Verify each finding inline as you write it. Single-pass review of the diff itself.
1	Adds Step 2.5a (semantic delta only — skip 2.5b/2.5c/2.5d). In Step 3a, launch only Agent 1 (Query execution & data integrity), Agent 2 (React correctness), Agent 3 (Async, timers & cancellation), and Agent 9 (Tests) in parallel. Step 3b runs the full quality gate. Verify findings inline.
2 (default)	Full Step 2.5. In Step 3a, launch Agents 1-12 (all domain agents + Agent 12 cross-context caller). Skip Agent 13 (fresh-context adversarial). Step 3b runs the full quality gate. Step 3c uses a single batched verification agent for all findings.
3	Every step below as written, all 13 agents including Agent 13, per-finding verification. The full mission-critical pass.

State the chosen level in one line at the start of the review so the user knows what they're getting (e.g., "Reviewing PR #565 at level 2"). If the level was defaulted, mention that level 3 exists for high-stakes changes.

Spawning review agents in pi

Step 3a (and the verification pass in Step 3c) call for "agents" launched "in parallel". In pi, launch them with the subagent(...) tool using fresh-context reviewer agents — one task per agent role. Each task must be self-contained because the child does not inherit this conversation. Give every review task:

1. the diff under review (paste it, or have the child re-run the gh/git command from Step 1),
2. the full change surface map from Step 2.5 (semantic deltas, callsite inventory, implicit contracts, cross-context exposure list),
3. the specific role instructions for that agent (Agent 1..13 below),
4. an explicit "review only — do not edit any files" constraint.

Example shape for a parallel fanout (adapt the task list to the level):

subagent({
  tasks: [
    { agent: "reviewer", task: "Agent 1 — Query execution & data integrity. <diff + change surface map + Agent 1 instructions>. Review only; do not edit." },
    { agent: "reviewer", task: "Agent 2 — React correctness & hooks. <...>. Review only; do not edit." },
    { agent: "reviewer", task: "Agent 3 — Async, timers & cancellation. <...>. Review only; do not edit." },
    { agent: "reviewer", task: "Agent 9 — Test review & coverage. <...>. Review only; do not edit." }
  ],
  context: "fresh"
})

For the verification pass (Step 3c), launch verification reviewer agents the same way — one per finding at level 3, a single batched verification agent at level 2, and inline (no spawn) at levels 0-1. The fresh-context adversarial agent (Agent 13, level 3 only) must NOT receive the change surface map or the agent checklists; give it only the diff and the changed-file names, per its instructions. The parent session owns the Step 3b quality gate (yarn typecheck/lint/build/test:unit), synthesis, deduplication, and the final report — children only return findings.

Step 1: Gather PR/Diff context

Strip the level token (--level=N / -lN) from $ARGUMENTS first; the remainder is the review target. Never pass the level token to gh/git — it is not a valid flag for them. Fetch the diff according to what the target is:

• PR number or URL — fetch metadata, diff, and comments in a single bash call so the variable stays in scope:

TARGET='<$ARGUMENTS with the level token removed>'
gh pr view "$TARGET" --json number,title,body,labels,state
gh pr diff "$TARGET"
gh pr view "$TARGET" --comments

• Commit hash — git diff <hash>~1..<hash> (no PR metadata; skip Step 2).
• Staged changes (staged) — git diff --staged (no PR metadata; skip Step 2).
• Unstaged changes (unstaged) — git diff (no PR metadata; skip Step 2).

If the user mentions reviewing only the staged or unstaged diff, review only that part, not something else.

Step 2: PR title and description

This step applies only when the target is a PR. For commit/staged/unstaged targets there is no PR description — skip it.

Check against conventions:

• Title follows Conventional Commits: type: description
• Description repeats the verb (e.g., fix: fix ... not fix: grid column ...)
• Description speaks to end-user impact, not implementation internals

Step 2.5: Map the change surface

Before launching review agents, produce a structured change surface map. This step is mandatory at levels 2 and 3 (level 1 runs 2.5a only) and must use bash (rg/grep/find) plus read — do not reason about consumers from memory. The output of this step is required input for every agent in Step 3a.

2.5a Semantic delta per changed symbol

For every modified or added component, hook, exported function, util, query/client method, context provider value, Redux action/reducer/selector, Dexie table or migration, styled component, type, or constant, write:

• Symbol: name and file
• Before: props (required vs optional, defaults), arg signature, return/render shape, nullability and shape of query results / returned fields, side effects (network requests, IndexedDB writes, localStorage writes, EventBus emits), referential stability of returned/used callbacks and context values, controlled/uncontrolled, loading/error/empty contract, ordering/idempotency guarantees, disposables created (timers, listeners, AbortControllers, Monaco models)
• After: same fields
• Delta: one line stating what semantically changed

"Refactored", "cleaned up", "improved", "simplified" are not acceptable deltas. State the actual behavioral difference. If nothing semantically changed, write "no behavioral change" — but only after checking, not as a default.

2.5b Callsite inventory

For every changed symbol that is exported (component, hook, util, client method, context value, action, selector, Dexie accessor, type), run a repository-wide search (rg via bash) to find every consumer, import, or reference outside the diff.

Produce a list grouped by file. Also search for:

• components that render the changed component (JSX usage)
• callers of the changed hook or client/query method (quest.queryRaw, quest.abort/quest.abortActive, client.*, etc.)
• useContext consumers of a changed provider value
• useSelector consumers of changed selectors or state shape, and reducers handling changed action types
• readers/writers of a changed Dexie table or anything depending on a bumped schema version
• eventBus.on / emitters of a changed EventBus event (only when the change touches one)
• test files that exercise the changed symbol

A changed exported symbol with zero recorded search commands in the trace is a skill violation. You are not allowed to assert "this is only used here" without showing the search.

2.5c Implicit contract list

For each changed symbol, walk this checklist and write one line per item, stating before vs after:

• Props/args required vs optional, and their defaults
• Render/return shape — can it now return null/undefined where it couldn't before?
• Query result contract — column set/order, row count assumptions, empty-result handling, error/timeout shape, whether results can be partial or truncated
• Nullability of returned/exposed fields
• Side effects: network requests fired, IndexedDB/localStorage writes, navigation, EventBus emits
• Disposables and cleanup: timers, RxJS subscriptions, AbortControllers, event listeners, Monaco models/decorations — added, removed, or changed?
• Event/callback ordering and invocation guarantees (how many times, in what order, with what args)
• Idempotency and re-entrancy (rapid clicks, repeated calls, concurrent or aborted-then-restarted requests)
• Controlled vs uncontrolled component contract
• Redux: action shape, reducer state shape, selector return type
• Persistence: Dexie schema version, table shape, migration behavior against existing stored data
• Referential stability of returned callbacks, objects, and context provider values (does a consumer's deps array, memoized child, or context subscription depend on it?)
• Loading/error/empty state contract

2.5d Cross-context exposure list

End this step with an explicit list of "places this change is visible from but the diff does not touch". This is the highest-priority input for the bug-hunting agents in Step 3a.

The list groups the callsites from 2.5b by context: parent components that render the changed component, consumers of the changed query result / client method, consumers of the changed context provider value, effect dependency arrays that include the changed value, memoized children that receive changed callbacks, readers of a changed Dexie table, route or lazy-load boundaries, EventBus subscribers. Every entry on this list must be reviewed in Step 3a.

Step 3a: Parallel review

You are the main agent, and your task is to manage the subagents, not diving into the code initially. Every agent receives:

1. The PR diff
2. The full change surface map from Step 2.5 (semantic deltas, callsite inventory, implicit contracts, cross-context exposure list)

Each subagent should read surrounding source files as needed for context.

Anti-anchoring directive (applies to all agents)

• Bugs at callsites outside the diff outrank bugs inside the diff. A confirmed bug in a file the PR did not touch but that consumes a changed query result, client method, context value, component, hook, or Dexie table is a P0 finding.
• "Looks correct in isolation" is not a valid conclusion. Before clearing a changed symbol, the agent must walk the callsite inventory from 2.5b and explicitly state, per callsite, whether the new behavior is still correct there.
• The diff is the entry point, not the scope. If the change surface map shows the symbol is consumed by N other files, the review covers N+1 files.
• A single finding of the form "in Result/index.tsx the query result can now be empty and the grid dereferences rows[0]" is worth more than five nits inside the diff.

Agents

Launch the following agents in parallel. (Level 1 launches only Agents 1, 2, 3, 9; level 2 launches Agents 1-12; level 3 launches all 13.)

Agent 1: Query execution & data integrity: The highest-stakes agent. For any change touching src/utils/questdb/client.ts, src/providers/QuestProvider, src/scenes/Result, src/scenes/Schema, or query construction/result handling anywhere: correct handling of empty result sets, error responses, timeouts, aborted requests, and partial/truncated data; correct parsing of result columns and types; off-by-one or wrong-column indexing when rendering rows; assumptions that a result is non-empty or has a fixed shape; loss or misrepresentation of values (number precision, timestamp/timezone, null vs empty string, BLOB/binary); query string construction that could send malformed or unintended SQL; missing loading/error/empty UI states. Treat any path where the user could see wrong data, or no data without an error, as critical.

Agent 2: React correctness & hooks: Hook rules violations, stale closures, missing or incorrect dependency arrays, unnecessary stable references in deps array, missing useEffect cleanup (timers, subscriptions, AbortControllers, event handlers, Monaco models/listeners/decorations/commands), conditional hook calls, state updates after unmount, incorrect use of refs, broken controlled/uncontrolled component patterns, incorrect key props causing lost state, event handler reference stability, unnecessary RAF usage, unnecessary layout effect usage. React 17: flag reasoning that assumes automatic batching of setState outside event handlers (in promises, timeouts, epics, await continuations) — each such update renders separately here.

Agent 3: Async, timers & cancellation: Missing cleanup of setTimeout/setInterval on unmount or before re-scheduling; a query feeding the shared single-slot result grid (driven by the editor and the AI chat window) that re-fires while a prior such query is still in flight without superseding it via quest.abort(queryId) / quest.abortActive(), so a slow earlier response lands late and overwrites the grid with stale data — flag the missing abort/supersede, not a missing caller-created AbortController, and do not flag independent/background queries (schema tree, table details, autocomplete, build version) that are meant to run in parallel; raw fetch() outside the client (AI streaming, settings) that should pass an AbortSignal but doesn't; requests that resolve after unmount or after a newer request (stale-response / race conditions); rapid-fire user actions firing duplicate or out-of-order requests; redux-observable epic correctness (ofType filtering, switchMap vs mergeMap for cancellation, error handling that doesn't kill the stream, missing takeUntil); RxJS subscription leaks; setTimeout used only to defer state updates (a smell, usually fixable with a callback or proper effect).

Agent 4: State & context architecture: Redux action/reducer correctness, immutable state updates (no direct mutation); context provider value stability — flag any provider whose value is a fresh object/array/function on every render without useMemo/useCallback, since it re-renders every consumer; context split so unrelated consumers don't re-render together; prop drilling where context would be cleaner (and the reverse — context used where a prop suffices); proper EventBus usage where touched; missing loading/error states in state-derived UI.

Agent 5: Persistence & migrations: Dexie/IndexedDB correctness in src/store and consumers — schema version bumped when table shape changes; migration upgrades existing persisted data rather than dropping it; no data loss for users upgrading from a previous version; correct handling of quota-exceeded and corruption; compression/serialization round-trips losslessly (compression.ts, buffers.ts); reads tolerate older/missing fields written by prior versions; no blocking of the main thread on large persisted payloads.

Agent 6: Performance & rendering at scale: Unnecessary rerenders through missing useMemo/useCallback where a component passes callbacks to memoized children or large lists; unnecessary memoization of small functions/computations that prevents no rerender; context-driven re-render storms (consumers re-rendering because a provider value isn't memoized); missing virtualization (react-virtuoso) for large result sets / long lists / the schema tree; expensive echarts/uPlot re-renders or full re-inits where an update would do; inline object/array/function creation in JSX props causing referential inequality; unnecessary or unnecessarily frequent network requests and IndexedDB writes; expensive computations without memoization. Algorithmic optimality: for every loop, traversal, or lookup added or changed in render, effects, selectors, query/result parsing, or schema-tree handling, state the time complexity and flag sub-optimal choices — an O(n) .find/.indexOf/.includes/.filter linear scan where a Map/Set/object index gives O(1); an O(n²) nested .find/.some/.filter inside a .map over result rows or schema items; rebuilding a lookup structure (Map/Set/index) on every render instead of constructing it once and memoizing; re-parsing or re-deriving already-computed data; and multiple passes over the same result set that could be fused into one. The bar is the best known approach, not merely “avoids quadratic” — these costs compound on large result sets, long buffer lists, and wide schema trees.

Agent 7: Styling & theming: Hardcoded colors/sizes instead of theme tokens, CSS specificity issues, z-index conflicts, animation performance (prefer transform/opacity over layout-triggering properties), styled-components created inside render functions (causes remounting), proper use of css helper for conditional styles, $-prefixed prop names for style-only props, proper use of rem units, not pixels, proper use of styled components instead of inline styling, proper use of existing icon libraries instead of custom SVGs, proper font/icon/box sizes that are consistent.

Agent 8: Code structure, readability & types: Unnecessarily long component definitions without splitting into subcomponents (flag growth in already-large files), defining the same function/styled component in multiple places, complex logic inside a component instead of a utils file, creating a new component while an existing one under src/components could be reused, plain button/flex div where Button/Box apply; ambiguous naming, missing early returns, discouraged regex where a clearer approach exists, unnecessary comments for trivial logic, unnecessary IIFEs, unnecessary ! non-null assertions, unnecessary ?. optional chains, unnecessary optional fields (?:) that cannot be null/undefined, overly broad types that should be discriminated unions.

Agent 9: Test review & coverage: User path coverage with E2E tests for new/modified flows (e2e/tests/**/*.spec.js, Cypress), unit test coverage for complex utility functions (especially query/result parsing and persistence/migration code). Cross-reference 2.5d: every cross-context exposure should have a test that exercises the changed symbol from that context. Missing tests for cross-context callsites is a high-priority finding.

Agent 10: Accessibility & UX: Missing ARIA labels on interactive elements; Radix dialog/popover/dropdown/tooltip usage — correct controlled state, focus trap, focus return on close, portal/z-index behavior, Escape/outside-click dismissal; missing keyboard navigation support; focus management issues; missing alt text on images; color contrast concerns; screen reader compatibility; click handlers without keyboard equivalents; missing error announcements for assistive technology; broken tab order.

Agent 11: Browser compatibility & security: No reliance on APIs unavailable in target browsers without polyfills; no CSS properties with limited cross-browser availability; XSS vectors — dangerouslySetInnerHTML, untrusted input through react-markdown / react-highlight-words, unsanitized HTML; SQL injection or unintended SQL via string-built queries; open redirects via user-controlled URLs in the OAuth2/SSO flow (src/modules/OAuth2); secrets/tokens leaking into logs, telemetry, or localStorage.

Agent 12: Cross-context caller impact: Walk the callsite inventory from 2.5b. For every callsite, fetch the surrounding code (the consuming component/hook plus its callers up two levels) and answer:

• Does this consumer pass props/args the new behavior handles incorrectly?
• Does it depend on a contract from the implicit contract list (2.5c) that the change broke — a query result that can now be empty/partial/null-shaped, a context value that's no longer stable, a prop that became required, a hook return shape that changed, a Dexie field that moved or was renamed, a callback that's no longer referentially stable?
• Is it in a context (effect dependency array, memoized child, list render, event handler, async callback, aborted-request path, unmount path, route boundary) where the new behavior misbehaves even when the inputs are valid?
• For changed query results / client methods: do all consumers still parse and render the result correctly?
• For changed context values: do all useContext consumers still get a value with the expected shape and stability?
• For changed components/hooks: do all render sites / callers handle the new props or return shape?
• For changed Dexie tables/migrations: do all readers tolerate data written by both old and new versions?

This agent's output is structured per callsite, not per failure mode. Each callsite gets a verdict: SAFE / BROKEN / NEEDS VERIFICATION. Every BROKEN entry is a P0 finding regardless of whether the file is in the diff. This agent is not optional even when the diff is small — small diffs to widely-used symbols have the largest blast radius.

Agent 13: Fresh-context adversarial (level 3 only): Dispatched separately from Agents 1-12 to escape checklist anchoring. This agent operates under different rules:

• It receives ONLY the PR diff and the names of the changed files. It does NOT receive the change surface map from Step 2.5, the implicit contract list, the cross-context exposure list, or any of the agent checklists above.
• Its sole instruction: "find ways this code is wrong". No category list, no failure-mode taxonomy, no QuestDB-specific style guide.
• It is free to use read and bash (rg/grep/find) to explore the repository however it wants.
• Findings are not pre-classified by category. Each finding states: what's wrong, why it's wrong, and the code path that demonstrates it.

The point of this agent is to surface bugs the structured agents cannot see because they are reasoning inside the same frame. A finding here that none of Agents 1-12 produced is high signal — it means the structured review missed it. A finding here that overlaps is corroboration. Run it in parallel with the rest.

Step 3b: Fixed quality checks

While the subagents are scanning the code for their tasks, you will perform predefined quality checks on the code.

• Type errors: yarn typecheck
• Build failure: yarn build
• Lint errors: yarn lint
• Unit test failures: yarn test:unit

At level 0, run only yarn typecheck and yarn lint. At levels 1-3, run all four.

After performing these checks, if there are errors/failures, add the errors from these checks to the output table at the end, one row for each check. Build, type, and test failures are critical, lint errors are moderate. After completing this step, you will wait for subagent results.

Step 3c: Verify every finding against source code

Combine all agent findings into a single deduplicated draft report. Do NOT present this draft to the user yet — it goes straight into verification. The parallel review agents work from the diff plus the change surface map and frequently produce false positives — especially around stale closures, missing cleanup, re-render claims, and race conditions. Every finding MUST be verified before it is reported. (At levels 0-1, verify each finding inline as you write it instead of spawning verification agents.)

For each finding in the draft report:

1. Read the actual source code at the exact lines cited. Do not rely on the agent's description alone.
2. Trace the full code path: see if the code path verifies the claim, and the issue can occur with realistic user actions. For hooks, trace the deps array against what actually triggers a re-render or re-run.
3. For data-integrity claims (Agent 1): trace the actual result shape the client returns and confirm the consumer can really receive the empty/error/partial/null case claimed. A wrong-data or no-data-without-error finding is critical only if a real query path produces it.
4. For stale-closure / stale-state claims: verify the closure actually captures a stale value AND that a fresh value is needed there. Check whether the value is read from a ref or passed fresh — if so, drop it.
5. For missing-cleanup claims: verify the effect actually sets up something that needs cleanup (timer, subscription, listener, AbortController, Monaco disposable) and that the component can unmount or the deps can change while it is live. A one-shot effect that cannot re-run or unmount mid-flight is not a leak.
6. For re-render / performance claims (incl. unmemoized context values): verify the provider/parent actually re-renders often enough to matter and that consumers really re-render as a result. Do not flag memoization that prevents no real re-render, or useCallback/useMemo whose deps churn anyway. For algorithmic-complexity claims (O(n) where O(1) is achievable, O(n²) scans, redundant passes): confirm the complexity analysis is correct and the path is reachable with realistic data (large result sets, long buffer lists, wide schema trees). Such findings are valid regardless of the current data size — do not downgrade one just because today's input is small; only drop it if the analysis is wrong or the collection is bounded by a small constant (e.g. column count, a fixed enum).
7. For race-condition claims: trace the actual async ordering and verify two operations can realistically interleave via real user actions (rapid clicks, navigation mid-request, unmount mid-fetch, abort-then-restart). If the ordering is structurally impossible, drop it. For a missing-query-abort claim specifically, confirm the two queries actually compete for the same single-slot result surface (the result grid) — if they write to different targets, or are background queries meant to run in parallel, there is no stale-overwrite race and the missing abort is not a bug.
8. For persistence/migration claims (Agent 5): confirm the schema version and migration path, and whether existing persisted data is actually at risk. Verify against migrations.ts rather than assuming.
9. For cross-context findings (Agent 12): re-read the callsite in full, including its callers up two levels, and confirm the broken behavior is reachable from production code paths. Cross-context findings are high-value but also the easiest to overstate — verify carefully.
10. Think about the use case: check if the issue really creates regression for the user, considering the usage patterns. If the issue cannot be reproduced under a realistic user scenario, it cannot be critical.
11. Classify each finding as:
    • CONFIRMED in-diff — the bug is real and inside the diff
    • CONFIRMED at out-of-diff callsite — the bug is in an unchanged file because the changed symbol is consumed there in a way that's now broken (cite the file and the contract from 2.5c that was violated)
    • FALSE POSITIVE — the code is actually correct (explain why)
    • CONFIRMED with nuance — the issue exists but is less severe than stated (explain)

Move false positives to a separate "False-positives" section at the end of the report. For each, give a one-line explanation of why it was dismissed. This lets the PR author verify the reasoning and catch verification mistakes.

Launch verification agents in parallel where findings are independent, except where the level dictates otherwise: level 2 batches all findings into a single verification agent, level 3 verifies per-finding, and levels 0-1 verify inline as findings are written. Each verification agent should read surrounding source files, not just the diff.

Step 4: Output

You will provide all the information in three sections: ## Issues, ## False-positives, ## Summary:

Issues section

Present the validated findings in a table with the following columns:

• Issue ID (#1, #2 etc.)
• Issue name (3-5 words)
• Category: "Quality check" | title of the subagent (the task name)
• Severity: "Critical" | "Moderate" | "Minor"
• Location: "in-diff" | "out-of-diff" — for out-of-diff findings, name the file and the contract from 2.5c that was violated
• Description: Full user impact in a plain, small paragraph
• Steps to reproduce: Clear list with bullet points, the user path results in the issue
• Suggested fix

False-positives section

Provide the list of false-positive findings from subagents that you verified that the issue does not exist, with the following fields:

• Category: title of the subagent (the task name)
• Description: the description from subagent
• Explanation: your explanation on why it's a false-positive

Summary section

• One-line verdict: approve, request changes, or needs discussion
• Highlight any regressions or tradeoffs
• State how many draft findings were verified vs dropped as false positives (e.g., "8 findings verified, 4 false positives removed")
• State the in-diff vs out-of-diff split (e.g., "5 findings in-diff, 3 findings out-of-diff"). At level 2 or 3, if the diff is non-trivial and out-of-diff is zero, the cross-context pass (Agent 12) likely underran — re-invoke it with a wider grep before finalizing. (At levels 0-1 Agent 12 does not run, so zero out-of-diff is expected and is not a signal of an underrun.)