Code Review

5d: Edge cases

<!-- roles=orchestrator phases=3B,3C summary="Last-matching-row resolution for category combinations: workflow-only, docs-only, build-config, tests, mixed." -->

The rules below cover combinations not handled by the per-row tables above. Where a combination matches more than one row, the last matching row wins. Workflow-machinery cases are listed first because workflow-only diffs are common and short-circuit the entire code/test agent dispatch.

Staged-path normalization (run first). On a workflow-modifying plan the authored .claude/... edits live under docs/adr/<dir>/_workflow/staged-workflow/.claude/... (per §1.7) while the live tree stays at develop's state. The Step 5b workflow-review globs name live .claude/... paths, so a staged path matches none of them. Three glob-gated reviewers therefore miss and fail to launch: review-workflow-prompt-design, review-workflow-instruction-completeness, and review-workflow-hook-safety. (review-workflow-consistency and review-workflow-context-budget always run for this group; review-workflow-writing-style already fires via its docs/adr/**/*.md glob, and since its .claude/**/*.md glob also matches the normalized path, it fires regardless of which form this row is evaluated against.) Before evaluating the Step 5b per-agent triggers against the workflow-machinery subset of the diff, normalize each changed path: a path matching the anchored prefix docs/adr/<any-dir>/_workflow/staged-workflow/(\.claude/…) is replaced by its captured .claude/… remainder; the match is anchored after the docs/adr/<dir>/ head (the <dir> segment is variable). A path that does not match this exact anchored prefix passes through unchanged, including one that merely contains .claude/ lower down. A staged file then evaluates exactly as its live counterpart would, and the three glob-gated reviewers launch on the staged edit. Normalization runs ahead of the Step 5b glob match only; it does not edit the globs themselves and does not change the Step 5a file-set categorization (a staged file is already workflow-machinery by the docs/adr/<dir>/ rule).

• If the only category present is workflow-machinery: Skip all code-review and test-review agents (no Java code or tests to evaluate). Launch the workflow-review agents selected by the workflow-side rules.
• If the only categories are docs-only and workflow-machinery (any mix): Treat as workflow-machinery-only — skip code-review and test-review agents, launch the workflow-review group on the workflow-machinery files.
• If the diff mixes workflow-machinery with production-code or test categories: launch each group's agents on its in-scope files. Each group is dispatched with a pre-filtered IN_SCOPE_FILES list (see Step 6) so cross-contamination is bounded.
• If the only category present is docs-only: Skip all agents. Just report that only end-user documentation changed and no review is needed.
• If the only category present is build-config: Launch only review-code-quality (to check for misconfigurations) and review-security (to check for dependency changes). Skip all test-review and workflow-review agents.
• If the only category present is tests-only: Launch review-code-quality and review-bugs (test logic can have bugs too), plus review-concurrency if the concurrency category is present, plus the full test-review set selected by the test-side rules above.
• If the only categories are build-config and tests-only (e.g., a CI tweak plus the test it enables): Launch review-code-quality, review-security, and the full test-review set. The tests-only rule wins over build-config's "skip test-review" because the tests are the substantive change.
• If a file matches no category (e.g., .gitattributes): assign it the other category — it does not dispatch any agent. If other is the only category present, skip all agents and report that nothing reviewable changed.
• If in doubt about whether an agent is relevant: launch it. False positives (an agent finding nothing) are better than false negatives (missing a real issue).

Step 6: Dispatch selected review agents

<!-- roles=orchestrator phases=3B,3C summary="Launch selected agents in parallel with a scope-filtered file list and the group-specific prompt template." -->

Launch the selected agents in parallel using the Agent tool. Each agent receives a scope-filtered context: build IN_SCOPE_FILES per agent group from the triage categories assigned in Step 5a.

• Code-review group: files categorized as anything other than docs-only, workflow-machinery, or other. (Files categorized as build-config stay in scope — review-code-quality and review-security need to see pom.xml and CI changes per Step 5d.)
• Test-review group: files that match the agent's individual launch rule plus any file categorized tests-only.
• Workflow-review group: files categorized as workflow-machinery.

The IN_SCOPE_FILES list narrows the agent's focus; the full DIFF is still passed so the agent can read context lines around each change.

The Tooling section differs by group. Use the code/test variant for the code-review and test-review groups, and the workflow variant for the workflow-review group.

Prompt template — code-review and test-review groups

<!-- roles=orchestrator phases=3B,3C summary="The prompt body for the code and test review groups, including the PSI-over-grep tooling rule for Java symbols." -->

Review the following changes from your specialized perspective.

## Review Scope
{REVIEW_SCOPE}

## In-Scope Files
{IN_SCOPE_FILES}

## PR Description
{PR_DESCRIPTION}

## Commit Log
{COMMIT_LOG}

## Changed Files
{CHANGED_FILES}

## Skip These Files (generated code)
- core/src/main/java/com/jetbrains/youtrackdb/internal/core/sql/parser/*
- Any files under generated-sources/ or generated-test-sources/
- Generated Gremlin DSL classes

## Tooling
Use **mcp-steroid PSI find-usages / find-implementations / type-hierarchy
via `steroid_execute_code`, not grep**, for any reference-accuracy
question about a Java symbol in this diff (callers/overrides/usages of
a method, field, class, or annotation; whether a slot is genuinely
unused; whether a renamed symbol still has stale references; for test
review, which production methods a test exercises and where else they
are called). Grep is acceptable for filename globs, unique string
literals, and orientation reads, but the load-bearing answer behind a
finding must be PSI-backed when the mcp-steroid MCP server is
reachable per the SessionStart hook (`steroid_list_projects` once at
the start confirms the open project matches the working tree). Fall
back to grep with an explicit reference-accuracy caveat in the finding
only when mcp-steroid is unreachable. See `CLAUDE.md` § MCP Steroid →
"Grep vs PSI — when to switch" for the full routing rule.

## Diff
The full diff is at: {DIFF_FILE}
Read it with the `Read` tool before forming findings. For diffs over
2000 lines, read the file in chunks using the `offset` and `limit`
parameters. Do not infer diff content from {CHANGED_FILES} alone — the
file list does not show what changed inside each file.

Prompt template — workflow-review group

<!-- roles=orchestrator phases=3B,3C summary="The prompt body for the workflow-review group, noting PSI does not apply to markdown, shell, and JSON files." -->

Review the following changes from your specialized perspective.

## Review Scope
{REVIEW_SCOPE}

## In-Scope Files
{IN_SCOPE_FILES}

## PR Description
{PR_DESCRIPTION}

## Commit Log
{COMMIT_LOG}

## Changed Files
{CHANGED_FILES}

## Tooling
PSI does not apply — these are markdown, shell, and JSON files, not Java
symbols. Use `Read` on the in-scope files and on any referenced skill,
agent, hook, or output-style file. Use `Grep` for "is this string
mentioned anywhere else in the repo" questions. The Java-targeted
mcp-steroid PSI rule does not apply to workflow-machinery review.

## Diff
The full diff is at: {DIFF_FILE}
Read it with the `Read` tool before forming findings. For diffs over
2000 lines, read the file in chunks using the `offset` and `limit`
parameters.

Handling missing fields

<!-- roles=orchestrator phases=3B,3C summary="Treat the missing-PR and uncommitted sentinels as no signal; do not infer requirements from their absence." -->

If PR_DESCRIPTION is the literal string "No PR associated with these changes." or COMMIT_LOG is the uncommitted-changes sentinel, treat the field as carrying no signal. Do not infer requirements from the absence.

The 16 possible agents (launch only those selected in Step 5):

Code-review agents:

1. review-code-quality — code quality, conventions, readability
2. review-bugs — bugs, logic errors, null safety, resource leaks, RID handling, state-machine / lifecycle (single-threaded sequential reasoning)
3. review-concurrency — races, visibility / publication, lock-ordering / deadlock, compound-op atomicity (multi-thread interleaving reasoning)
4. review-crash-safety — WAL correctness, durability, crash recovery
5. review-security — injection, auth, data exposure, dependencies
6. review-performance — algorithmic complexity, allocations, lock contention, I/O

Test-review agents: 7. review-test-quality — behavior-driven quality (assertion precision, exception testing) plus completeness (corner cases, boundary conditions, test data quality) 8. review-test-structure — isolation, independence, readability, documentation 9. review-test-concurrency — concurrent behavior testing quality 10. review-test-crash-safety — crash/recovery test quality, production assert statements

Workflow-review agents: 11. review-workflow-consistency — cross-file references, threshold sync, hook wiring, recipe paths, glossary drift 12. review-workflow-prompt-design — prompts-as-prompts-to-an-LLM: description discriminability, deterministic decision rules, clean-context invocation, sub-agent delegation annotations 13. review-workflow-instruction-completeness — branch coverage, gate resume paths, sub-agent input/output handshake, error recovery, loop termination 14. review-workflow-hook-safety — shell hygiene, /tmp collision safety, hook performance, secret hygiene, JSON schema validity 15. review-workflow-context-budget — always-loaded surface (descriptions, CLAUDE.md, SessionStart stdout), load-on-demand discipline, instant per-operation consumption (sub-agent delegation, output caps, /tmp staging, targeted reads) 16. review-workflow-writing-style — house-style: AI-tells, banned sentence and analysis patterns, BLUF lead, soft section length cap with template-bound exemptions, repo-anchored voice.

Set subagent_type to the agent name. The agent's frontmatter declares its model; do not override it from the dispatch call unless the user explicitly asks for a different model.

Step 7: Synthesize the results

<!-- roles=orchestrator phases=3B,3C summary="Map sub-agent severities, deduplicate, attribute, and summarize into one unified report, not a concatenation." -->

After all selected agents complete, produce a unified review report. Do NOT simply concatenate the outputs. Instead:

1. Map sub-agent severities to synthesized severities. Most sub-agents emit findings under Critical / Recommended / Minor, but several older code-review agents use legacy scales (review-bugs and review-concurrency share one, review-crash-safety and review-security each have their own). Translate as:

   • Critical → blocker (all agents)
   • Recommended → should-fix (most agents)
   • Minor → suggestion (most agents)
   • Likely Issues → should-fix (review-bugs, review-concurrency)
   • Potential Concerns → suggestion (review-bugs, review-concurrency)
   • Concerning → should-fix (review-crash-safety)
   • Informational → suggestion (review-crash-safety)
   • High → blocker (review-security)
   • Medium → should-fix (review-security)
   • Low → suggestion (review-security)

   Apply this mapping verbatim. The "do not soften" rule below means: do not demote a sub-agent's blocker-level finding to anything below blocker. The only legal override is promoting (e.g., raising a should-fix to blocker because another sub-agent's blocker-level finding on the same line escalates the severity).

2. Deduplicate. Findings merge when they share the same (file, line-range, root issue). Different review dimensions on the same line merge into one finding listing all dimensions. Different lines do not merge. Workflow-review findings on a shell or JSON file may merge with code-review findings on the same file when they describe the same root issue.

3. Attribute. For each finding, indicate which review dimension(s) identified it. Use a short label (e.g., [code-quality], [bugs], [concurrency], [test-quality], [test-crash-safety], [workflow-consistency], [workflow-hook-safety], [workflow-writing-style]).

4. Summarize. Write a brief overall assessment (2-3 sentences). Cover whichever of code, tests, and workflow machinery actually appear in the diff. Do not write about a dimension that produced no findings.

Handling agent failures and empty output

<!-- roles=orchestrator phases=3B,3C summary="Record failed reviewers, propagate preface notes, emit All clear when empty, and remove the temp diff file." -->

• If a sub-agent returns empty output, errors out, or times out, add it to a ### Failed reviewers section at the top of the report with a one-line note and continue. Do not block the synthesis.
• If a sub-agent emits agent-specific preface sections beyond Critical / Recommended / Minor (for example, review-workflow-context-budget emits Always-loaded delta and Instant-consumption delta tables when any axis is affected), propagate them under a ### Reviewer notes section after the severity blocks rather than dropping them silently. These preface sections are optional: review-workflow-context-budget omits them in the no-impact case, and absence is not a failure.
• If all selected agents return zero findings, emit an explicit ### All clear block under the overall assessment instead of an empty report.
• After the synthesized report is produced (whether with findings or ### All clear), remove the temp diff file from Step 3: rm "$DIFF_FILE". The file is no longer needed once every sub-agent has returned.

Output format

<!-- roles=orchestrator phases=3B,3C summary="The synthesized report layout: failed reviewers, assessment, blockers, should-fix, suggestions, notes, and questions." -->

## Review: {REVIEW_SCOPE}

### Failed reviewers
[Omit if none. One line per failed agent.]

### Overall assessment
[2-3 sentences. Cover whichever of code, tests, and workflow machinery
produced findings — skip the dimensions that returned clean.]

### Blockers
[Must fix before merge]

1. **[Dimension]** `path/to/file.ext` (line X-Y)
   - **Issue**: ...
   - **Suggestion**: ...

### Should-fix
[Should fix before merge]

1. **[Dimension]** `path/to/file.ext` (line X-Y)
   - **Issue**: ...
   - **Suggestion**: ...

### Suggestions
[Recommended improvements]

1. **[Dimension]** `path/to/file.ext` (line X-Y)
   - **Issue**: ...
   - **Suggestion**: ...

### Reviewer notes
[Agent-specific preface sections propagated verbatim. Omit if none.]

### Questions for the author
[Clarifying questions aggregated from all reviewers. Omit if none.]

If a priority level has no findings, omit it entirely. If all priority levels are empty, replace them with ### All clear and a one-line note.

Important rules

<!-- roles=orchestrator phases=3B,3C summary="Standing rules: gh CLI for GitHub, parallel dispatch, no self-added findings, no severity softening, large-diff abort." -->

• Always use gh CLI for GitHub API calls, not WebFetch.
• All selected agents must run in parallel — do not wait for one before launching the next.
• Only launch agents selected by the triage step — do not launch agents for irrelevant dimensions.
• Do not add your own review findings — only synthesize what the agents report.
• Do not soften or dismiss agent findings — keep Critical at blocker, Recommended at should-fix, Minor at suggestion. Promotion is allowed when another sub-agent's higher-severity finding on the same line argues for it.
• If the diff is very large (>500 files or >50,000 lines), abort and ask the user to scope the review more narrowly (by directory, module, or commit range). The file-based diff hand-off in Step 3 lifts the per-prompt limit, so the practical ceiling is the per-agent context window — only very large reviews need user-side scoping.
• Standalone command: This command uses the same dimensional review agents as the Phase 3 workflow but with a different context structure (PR description and commit log instead of implementation plan and track file). Severity scale uses the same blocker/should-fix/suggestion levels as the workflow (see .claude/workflow/review-iteration.md).