Menu ▾ ▴

#17 Raise signal and structure of general code review prompt

closed
nobody
None
2026-07-05
2026-07-03
Anonymous
No

Originally created by: theBGuy

This change revises the general AI code review prompt to improve the quality and clarity of automated PR reviews, ensuring issues are flagged only when they are concrete and actionable. The goal is to minimize noise, prioritize high-value findings, and provide clear severity definitions for reviewers and users.

AI code review logic

  • Refines severity definitions in src/lib/ai/prompt.ts, making the difference between blocker, should-fix, and nit explicit and requiring findings to cite the concrete trigger (input/state/code path) for real problems.
  • Orders findings by severity in src/lib/ai/prompt.ts and clarifies grouping under ### headings, improving signal by instructing the reviewer to favor confident, high-value issues and omit uncertain nits.
  • Instructs not to flag trivial formatting or tests, to avoid padding or filler, and never to invent findings outside the diff in src/lib/ai/prompt.ts.

Documentation

  • Adds a detailed entry in CHANGELOG.md summarizing the new review prompt criteria and intended improvements to review quality.

Related

Tickets: #148
Tickets: #93

Discussion

  • Anonymous

    Anonymous - 2026-07-03

    Originally posted by: theBGuy

    AI review (sonnet) · automated

    The revision sharpens the general review prompt with explicit severity definitions, a "concrete case" requirement for real findings, and anti-noise guardrails. The direction is right, but one instruction introduces a structural ambiguity that could produce inconsistent model output.

    Correctness

    should-fix — src/lib/ai/prompt.ts, GENERAL_REVIEW_SYSTEM line 189

    "Ordered by severity and grouped under ### headings by theme" are structurally contradictory. If findings are grouped into themed sections (### Correctness, ### Edge cases), a global severity ordering doesn't apply across the document — a blocker under ### Correctness would be separated from a blocker under ### Security by a heading boundary. A model following this literally may either ignore the theme grouping (producing a flat severity-sorted list), ignore severity ordering (producing theme-grouped output with random severity interleaving), or oscillate between runs.

    The security prompt right below this constant (line 229) solves the same problem cleanly with "ordered by severity then confidence" and no competing theme grouping. For the general prompt, you likely want one of these:

    • Option A (theme grouping wins): Keep theme grouping, and add "within each group, list blockers before should-fixes before nits" — the original intent of severity ordering without the contradiction.
    • Option B (severity wins): Drop the ###-by-theme grouping entirely and say "ordered by severity, then grouped under ### headings by theme within each severity tier" — i.e. theme sub-groups inside a severity tier.

    Option A is simpler and matches what most reviewers actually produce.

    Readability

    nit — src/lib/ai/prompt.ts, GENERAL_REVIEW_SYSTEM line 195

    "Don't invent missing tests for trivial changes" uses "trivial" as a threshold, which the model will interpret arbitrarily — a one-line bug fix is "trivial" by some readings but arguably the best candidate for a missing-test finding. Compare the security prompt's sharper rule ("report only when you can name a concrete attack path"). A grounded rewrite: "don't flag missing tests where the change introduces no new behavior (e.g. renames, reformats, or reorganizes only)" — anchors the exclusion to the actual condition, not perceived size.

    nit — CHANGELOG.md, insertion position

    The new entry is inserted between the security-review entry and the "agent composer" entry rather than at the top of ## [Unreleased]. If the section follows newest-first ordering, it should precede the security review entry. If entries here are grouped by feature area (prompt improvements together), the position is defensible — but it's worth checking against convention, since a reader scanning the top of [Unreleased] won't see this entry first.

    Overall the change is well-targeted and the concrete additions (severity definitions, the triggering-case requirement, "no filler") are direct improvements over the original. Resolving the severity-vs-theme ordering conflict is the only thing worth fixing before merging.

     

  • Anonymous

    Anonymous - 2026-07-03

    Originally posted by: theBGuy

    AI security audit (sonnet) · automated

    No security issues introduced by these changes. The diff rewrites a static string constant (GENERAL_REVIEW_SYSTEM) and updates CHANGELOG.md; no data flows, sinks, authentication paths, secrets handling, dependency refs, or untrusted-input processing are touched.

     

  • Anonymous

    Anonymous - 2026-07-03

    Originally posted by: theBGuy

    AI review (sonnet) · automated

    The change sharpens the general review prompt with explicit severity definitions, an ordering model (theme-first, severity within theme), a concrete-trigger requirement for real findings, and tightened anti-noise rules. Both previous findings are fixed; what remains is one minor inaccuracy in the CHANGELOG prose.

    Resolved since last review

    • Correctness (was should-fix) — The "ordered by severity and grouped by theme" contradiction is resolved. The current wording — "grouped under ### headings by theme… and within each group order them by severity — blockers first, then should-fixes, then nits" — makes theme grouping primary and severity ordering secondary with no ambiguity. This is exactly Option A from the previous review.

    • Readability (was nit) — "trivial changes" as a threshold for the missing-tests rule is gone. The replacement — "changes that introduce no new behavior (renames, reformatting, or pure reorganization)" — is grounded and concrete.

    Readability

    nit — CHANGELOG.md, line 573

    "Findings are now ordered by severity" slightly misrepresents what the prompt actually instructs. The prompt's real model is theme-first, severity within each theme — findings in the same theme group are severity-ordered, but a blocker in ### Edge cases sits below all of ### Correctness. The CHANGELOG sentence implies a flat global severity sort. This is likely to confuse anyone reading the changelog and then noticing the output is grouped by theme rather than pure severity rank. Suggest: "Findings are grouped by theme and ordered by severity within each group" — matches the prompt exactly.

    nit — CHANGELOG.md, line 576

    "include a finding only when it's confident" is subject-ambiguous — "it" could refer to the finding itself rather than the reviewer. A quick clarify: "include a finding only when the reviewer is confident" matches the prompt's actual phrasing ("if you are confident it is real").

    Otherwise the change is solid. The structural improvements to the prompt are meaningful — the nested sub-list is cleaner than the original run-on bullet, the severity definitions are precise, and the "no filler" paragraph properly lands outside the instruction list as a top-level directive. No issues with the prompt logic itself.

     

  • Anonymous

    Anonymous - 2026-07-03

    Ticket changed by: theBGuy

    • status: open --> closed
     

Log in to post a comment.

MongoDB Logo MongoDB