review-pr

Review the pull request $ARGUMENTS.

Review mindset

You are a senior QuestDB engineer performing a blocking code review. QuestDB is mission-critical software deployed on spacecraft — bugs can cause data loss or system failures that cannot be patched after deployment. There is zero tolerance for correctness issues, resource leaks, or undefined behavior. 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 contracts the diff quietly changed. 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".
• "Out of scope" is not a valid disposition — fix it while we're in there. CI is slow and every PR round-trip is expensive; the team deliberately consolidates fixes. Never advise splitting work into a follow-up PR, filing a ticket "for later", or deferring a confirmed bug because it pre-dates the diff. A pre-existing bug discovered in code the review visits — changed files, callers from the callsite inventory, cross-context exposures — is a reportable finding tagged pre-existing, with a suggested fix intended for THIS PR. The only acceptable reason to leave a confirmed bug unfixed is that fixing it would genuinely destabilize the change — then say so explicitly, naming the risk. Conversely, never penalize a PR for bundling multiple fixes; review each change on its merits.
• 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 inputs break this? What happens under concurrent access? What if this runs on a 10-billion-row table? What if the column is NULL? What if the partition is empty?
• Demand optimal algorithms. QuestDB is a performance-first database. "Works correctly" is necessary but not sufficient — the implementation must use the best known algorithm and data structure for the job. If a hash lookup gives O(1) but the code does a linear scan, that is a finding. If two passes over the data can be collapsed into one, that is a finding. If a value is recomputed on every call but could be cached, that is a finding. Do not accept "good enough" — ask "is there a faster way?" for every loop, every traversal, every data structure choice.
• Check what's missing, not just what's there. Missing tests, missing error handling, missing edge cases, missing documentation for non-obvious behavior.
• Untested code is broken code. Treat every new or changed production behavior that ships without a test proving it as a defect, not a nice-to-have. The burden of proof is on the PR: a behavioral change ships with a test whose assertion would fail if the change regressed, or it does not ship. "The change is simple" and "existing tests probably cover it" are not evidence — a named test located by a recorded search, with a stated failure link, is.
• 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 benchmarks or reason about the algorithmic change — does it actually improve things, or could it regress in other cases? Even if the PR doesn't claim to be about performance, evaluate whether the chosen algorithms and data structures are optimal — sub-optimal code that "works" is still a finding. 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/Grep/Glob to inspect the surrounding code, callers, and related tests.
• Assess reachability before reporting. For every potential bug, trace the actual callers and inputs. If a problem requires physically impossible conditions (billions of columns, corrupted JNI inputs, values that no caller can produce), it is not a real finding — drop it. Focus on bugs that real workloads can trigger, not theoretical edge cases that exist only in the type system.
• QuestDB runs with Java assertions enabled (-ea). Assertions are a valid guard for invariants that indicate corruption or internal bugs. Do NOT flag assert as insufficient — it is the preferred mechanism for conditions that should never occur in a non-corrupt database. Only flag an assert if the condition can plausibly be triggered by normal (non-corrupt) user operations.

Review level

Parse $ARGUMENTS for a level token: --level=N, -lN, or a bare single digit 0-3. If no level is given, default to 0. Strip the level token before feeding the remainder (PR number or URL) to gh 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 significantly higher token cost; reserve level 3 for high-stakes PRs (replication, JNI boundary changes, on-disk format, public API, security/ACL).

Level	What runs
0 (default)	Steps 1, 2, 2.6, 4. Skip Step 2.5. Skip Step 3 — no agent spawn; review the diff inline in the main loop, using Read/Grep on demand to resolve ambiguities. Skip Step 3b — verify each finding inline as you write it. Single-pass review covering correctness, NULL handling, algorithmic optimality, tests, and QuestDB standards on the diff itself. The performance checklist (including algorithm optimality) is mandatory at every level. When the diff touches test code, also apply the test-efficacy and test-code-quality anti-pattern checks inline (vacuous assertions, reflection overuse, reinvented helpers, javadoc bloat). Step 2.6 (test coverage map) is mandatory here as at every level — build it inline before writing findings; derive the behavioral-change rows directly from the diff since 2.5a is skipped.
1	Adds Step 2.5a (semantic delta only — skip 2.5b/2.5c/2.5d) plus Step 2.5e when test code is present. In Step 3, launch Agent 1 (correctness), Agent 3 (performance), Agent 5 (tests), Agent 6 (code quality), and — when the diff touches test code — Agent 12 (test efficacy) and Agent 13 (test-code quality) in parallel. Skip all other agents. Skip Step 3b — verify findings inline as you draft the report.
2	Full Step 2.5 (including 2.5e when test code is present), but in 2.5b restrict the callsite inventory to public/protected symbols (skip package-private and pub(crate)). In Step 3, launch Agents 1-8 (Agent 8 only if .rs files are present), plus Agent 11 (adversarial performance), plus Agents 12 and 13 when the diff touches test code. Skip Agent 9 (cross-context), Agent 10 (adversarial fresh-context), and Agent 14 (regression-test efficacy verification). Step 3b uses a single batched verification agent for all findings instead of one per finding.
3	Every step below as written, all 14 agents, 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 #1234 at level 2"). If the level was defaulted, mention that level 3 exists for full review.

Step 1: Gather PR context

Capture the PR identifier in $PR (the part of $ARGUMENTS left after stripping the level token), then fetch metadata, diff, and review comments in a single bash call so $PR is in scope for all three gh invocations:

PR='<PR number or URL from $ARGUMENTS, with any --level=N / -lN / bare-digit level token removed>'
gh pr view "$PR" --json number,title,body,labels,state
gh pr diff "$PR"
gh pr view "$PR" --comments

Step 2: PR title and description

Check against CLAUDE.md conventions:

• Title follows Conventional Commits: type(scope): description
• Description repeats the verb (e.g., fix(sql): fix ... not fix(sql): DECIMAL ...)
• Description speaks to end-user impact, not implementation internals
• If fixing an issue, Fixes #NNN is at the top of the body
• Tone is level-headed and analytical, no superlatives or bold emphasis on numbers
• Labels match the PR scope (SQL, Performance, C

---

Content truncated.