Sync SDLC reviewer conventions with PR reviewer

[james-in-a-box]

Summary

• Synced SDLC reviewer inline conventions with PR reviewer's review-conventions.md
• Added "Verify end-to-end functionality" procedural step (was present in PR reviewer but missing from SDLC reviewer)
• Added verdict classification guidance (needs_revision vs approved) matching PR reviewer's blocking criteria
• Created shared/prompts/REVIEWER-SYNC.md with sync guide and modification checklist

Context

PR #955 added severity classification, non-functional feature blocking, and
pre-existing issue handling to the PR reviewer but the SDLC reviewer's inline
conventions in _build_review_prompt() were not updated to match. This PR
brings them into alignment and adds a guidance doc so future changes stay in sync.

Issue: #954

Test plan:

• Review orchestrator/routes/pipelines.py changes against action/review-conventions.md for parity
• Verify REVIEWER-SYNC.md accurately describes both reviewer surfaces
• Confirm docs/index.md and github-automation.md link to the sync guide

Authored-by: egg

[egg-reviewer]

No agent-mode design concerns.

The changes are review-criteria content (severity classification, end-to-end verification, false-analogy guidance) and a sync guide for keeping the two reviewer surfaces aligned. The SDLC reviewer's JSON verdict is consumed by pipeline automation (a genuine downstream consumer), so structured output is appropriate. The new procedural step and verdict classification guidance provide domain context (what is blocking vs non-blocking) rather than micromanaging how the agent works.

— Authored by egg

[egg-reviewer]

Review: Sync SDLC reviewer conventions with PR reviewer

The intent of this PR — aligning the two reviewer surfaces and creating a sync guide — is sound. However, the PR has a significant correctness issue: it claims the inline fallbacks match the shared file, but they don't. The REVIEWER-SYNC.md itself states "Inline fallbacks must match the shared file content," yet this PR widens the gap rather than closing it.

Blocking: Python inline fallback (_get_code_review_criteria) diverges significantly from code-review-criteria.md

File: orchestrator/routes/pipelines.py lines 919–969

The Python inline fallback is missing substantial content that exists in the shared file (shared/prompts/code-review-criteria.md). This PR adds severity classification and end-to-end checking to the Python fallback, but does not address the pre-existing omissions, making the delta larger than before. Missing items:

1. Introduction preamble: The shared file opens with "Be extremely thorough..." — the Python fallback starts directly at ### Security with no context.
2. "Insecure cryptography or randomness": Present in shared file (line 14) and shell fallback (line 48), missing from Python fallback.
3. "Incorrect algorithm complexity for data size": Present in shared file (line 22) and shell fallback (line 56), missing from Python fallback.
4. "State corruption scenarios": Present in shared file (line 30) and shell fallback (line 64), missing from Python fallback.
5. "Missing or incorrect abstractions": Present in shared file (line 35) and shell fallback (line 69), missing from Python fallback.
6. Entire Testing section: Present in shared file (lines 38–41), missing from Python fallback.
7. Entire Documentation section: Present in shared file (lines 43–45), missing from Python fallback.
8. "How to Review" section: Present in shared file (lines 47–54), missing from Python fallback.
9. "Skip" section: Present in shared file (lines 77–81), missing from Python fallback.
10. End-to-end functionality description: Shared file has a detailed paragraph with an example (e.g., config is read at build time but only available at runtime). Python fallback has a one-liner: verify new features work in their real execution environment. The shell fallback matches the shared file.
11. "Do not dismiss" paragraph: Shared file has extended text ending with "The fact that the PR is already changing this code makes it the natural place to fix it." Both fallbacks truncate this.
12. Pre-existing blocking bullet: Shared file line 64 has extra elaboration (if the PR touches code that already has bugs...). Both fallbacks omit this.

This is exactly the kind of drift the REVIEWER-SYNC.md is created to prevent. The PR modifies _get_code_review_criteria() — this is the natural place to fix the omissions. The Python fallback should either match the shared file or the function should be refactored to avoid maintaining a parallel copy.

Non-blocking: Three-way text inconsistencies in severity/do-not-dismiss wording

All three versions of the "Do not dismiss" paragraph have slightly different text:

• Shared file (line 73): longest version, includes "it's not acceptable to ship it just because the bug was there before"
• Shell fallback (line 98): shorter version, omits that sentence
• Python fallback (lines 957-961): same shorter version as shell

And the "Pre-existing" blocking bullet:

• Shared file (line 64): includes elaboration with example
• Shell fallback (line 89): no elaboration
• Python fallback (lines 949-950): no elaboration

If the inline fallbacks are meant to match the shared file, they should match. If they're intentionally abbreviated, the REVIEWER-SYNC.md should say so instead of stating they "must match."

Non-blocking: REVIEWER-SYNC.md is accurate but could note the current drift

The sync guide accurately describes the architecture and provides a useful checklist. However, given that significant drift already exists (as detailed above), it would be helpful to acknowledge the current state — either by adding a note about known drift or by actually fixing the drift in this PR.

Non-blocking: Documentation links look correct

The additions to docs/index.md and docs/guides/github-automation.md are well-placed and the relative paths are correct.

Summary

The PR's concept is good — creating a sync guide and aligning verdict classification is valuable. But the core claim that the reviewers are "in sync" is undermined by the Python inline fallback being substantially incomplete. The PR adds new content (severity classification, end-to-end checks) to the fallback while leaving existing gaps unaddressed, which contradicts the stated goal.

Recommendation: Fix the Python inline fallback to match code-review-criteria.md, or document why it's intentionally abbreviated. As written, the REVIEWER-SYNC.md tells future contributors that "inline fallbacks must match the shared file content" — a rule this PR itself violates.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

3 previous review(s) hidden.