Review Code Implementation
Review code implementation with full severity tree procedure (Phase 10).
-> Full workflow in PROCESS.md
When to Use
Assigned to review code implementation after worker slices complete.
Given/When/Then/Should
Given code assignment when reviewing then apply end-user alignment criteria and verify production code paths should never approve without running quality gates
Given implementation when verifying then run the project's quality gates should never approve without passing checks
Given issues found when categorizing then use BLOCKER/IMPORTANT/MINOR severity with EAGER group creation should never skip creating empty severity groups
Given BLOCKER finding when wiring dependencies then add dual-parent relationship (severity group + slice) should never wire BLOCKER to only one parent
Severity Tree: EAGER Creation
ALWAYS create 3 severity group tasks per review round, even if some groups have no findings:
# Step 1: Create all 3 severity groups immediately (EAGER, not lazy) bd create --labels "aura:severity:blocker,aura:p10-impl:s10-review" \ --title "SLICE-1-REVIEW-A-1 BLOCKER" \ --description "--- references: slice: <slice-id> review: <review-id> --- BLOCKER findings for this review round" # Result: <blocker-group-id> bd create --labels "aura:severity:important,aura:p10-impl:s10-review" \ --title "SLICE-1-REVIEW-A-1 IMPORTANT" \ --description "--- references: slice: <slice-id> review: <review-id> --- IMPORTANT findings for this review round" # Result: <important-group-id> bd create --labels "aura:severity:minor,aura:p10-impl:s10-review" \ --title "SLICE-1-REVIEW-A-1 MINOR" \ --description "--- references: slice: <slice-id> review: <review-id> --- MINOR findings for this review round" # Result: <minor-group-id> # Step 2: Wire severity groups to review task bd dep add <review-id> --blocked-by <blocker-group-id> bd dep add <review-id> --blocked-by <important-group-id> bd dep add <review-id> --blocked-by <minor-group-id>
Adding Findings to Severity Groups
# BLOCKER finding — dual-parent relationship bd create --title "BLOCKER: <finding title>" \ --description "<finding details with file:line references>" bd dep add <blocker-group-id> --blocked-by <blocker-finding-id> bd dep add <slice-id> --blocked-by <blocker-finding-id> # IMPORTANT finding — single parent (severity group only) bd create --title "IMPORTANT: <finding title>" \ --description "<finding details>" bd dep add <important-group-id> --blocked-by <important-finding-id> # MINOR finding — single parent (severity group only) bd create --title "MINOR: <finding title>" \ --description "<finding details>" bd dep add <minor-group-id> --blocked-by <minor-finding-id>
Closing Empty Groups
Empty severity groups (no findings) are closed immediately:
# If no IMPORTANT findings were found: bd close <important-group-id> # If no MINOR findings were found: bd close <minor-group-id>
Dual-Parent BLOCKER Relationship
BLOCKER findings have two parents:
- •The severity group task (
aura:severity:blocker) — for categorization - •The slice they block — for dependency tracking
This ensures BLOCKERs both categorize under the severity tree AND block the slice they apply to.
IMPORTANT and MINOR findings do NOT block the slice — they are tracked in the follow-up epic.
Steps
- •
Read code changes and the URD:
bashbd show <slice-id> bd show <urd-id> # Read URD for requirements context
- •
Run quality gates:
bash# Run your project's type checking and test commands
- •
Apply review criteria (see End-User Alignment in
aura:reviewer.md) - •
Verify production code paths (see below)
- •
Create review task:
bashbd create --labels "aura:p10-impl:s10-review" \ --title "SLICE-1-REVIEW-A-1: <feature>" \ --description "--- references: slice: <slice-id> urd: <urd-id> --- VOTE: <ACCEPT|REVISE> - <justification>" bd dep add <slice-id> --blocked-by <review-id>
- •
Create severity tree (EAGER — all 3 groups immediately)
- •
Add findings to appropriate severity groups
- •
Close empty severity groups
- •
Cast vote via
bd comments add
Verify Production Code Paths
Given code implementation when reviewing then verify production code paths wired should never approve dual-export anti-pattern
- •
Check for dual-export anti-pattern:
Anti-pattern example:
go// WRONG: Test-only export func HandleCommand(argv []string, service Service) error { /* tested */ } // WRONG: Production-only command (not tested) var commandCmd = &cobra.Command{ Use: "command", RunE: func(cmd *cobra.Command, args []string) error { // TODO: wire up service return nil }, }Correct example:
go// CORRECT: Single command, both tested and used in production var commandCmd = &cobra.Command{ Use: "command", RunE: func(cmd *cobra.Command, args []string) error { service := NewService(RealDeps{}) result, err := service.DoThing(args) if err != nil { return err } fmt.Println(result) return nil }, } // Tests import commandCmd directly // import "myproject/cmd/thing" - •
Verify no TODO placeholders:
bashgrep -r "TODO" src/ # Should not find any in delivered code
- •
Check tests import production code:
- •Test file should import the actual CLI command or API endpoint
- •Not a separate test harness function
- •
Production code verified via code inspection:
- •No TODOs in CLI/API actions
- •Real dependencies wired (not mocks in production code)
- •Tests import production code
Follow-up Epic
Trigger: Review completion + ANY IMPORTANT or MINOR findings exist.
NOT gated on BLOCKER resolution.
Owner: Supervisor creates the follow-up epic (label aura:epic-followup).
Reviewing FOLLOWUP_SLICE-N (Follow-up Code Review)
When reviewing follow-up slices, use the same procedure:
- •Review task naming:
FOLLOWUP_SLICE-N-REVIEW-{axis}-{round} - •Same EAGER severity tree (BLOCKER/IMPORTANT/MINOR per review round)
- •No followup-of-followup: New IMPORTANT/MINOR findings from FOLLOWUP_SLICE review are tracked on the existing follow-up epic, not a new nested follow-up
- •The worker's completion handoff (h4) reports which original leaf tasks were resolved — verify these during review
Report Results
# Add vote comment to the review task bd comments add <review-id> "VOTE: ACCEPT - Implementation matches plan, tests comprehensive" # Or bd comments add <review-id> "VOTE: REVISE - BLOCKERs found, see severity tree for details"