feat(planning): Add Generate Task Breakdown button to DiscoveryProgress

docs/code-review/2026-01-06-task-breakdown-button-review.md

@@ -0,0 +1,160 @@
+# Code Review Report: Feature 016-3 Task Breakdown Button
+
+**Date:** 2026-01-06
+**Reviewer:** Code Review Agent
+**Component:** DiscoveryProgress.tsx (Task Generation UI)
+**Risk Level:** Medium (Frontend state management, API calls, WebSocket handling)
+
+## Review Plan
+
+Based on context analysis, focused on:
+- ✅ Error handling (API failures, WebSocket errors)
+- ✅ State management (race conditions, state consistency)
+- ✅ XSS prevention (dynamic content rendering)
+- ✅ Test coverage verification
+- ❌ Skipped: LLM/ML security (not AI code)
+- ❌ Skipped: Cryptographic checks (no crypto involved)
+
+## Summary
+
+| Category | Status | Issues |
+|----------|--------|--------|
+| Security | ✅ PASS | 0 Critical, 0 High |
+| Reliability | ✅ PASS | 0 Critical, 1 Minor |
+| Performance | ✅ PASS | No concerns |
+| Maintainability | ✅ PASS | Well-structured |
+| Test Coverage | ✅ PASS | 89 unit tests, 6 E2E tests |
+
+## Files Reviewed
+
+1. `web-ui/src/components/DiscoveryProgress.tsx` (modified)
+2. `web-ui/src/types/index.ts` (modified)
+3. `web-ui/src/lib/api.ts` (modified)
+4. `web-ui/src/components/Dashboard.tsx` (modified)
+5. `web-ui/__tests__/components/DiscoveryProgress.test.tsx` (modified)
+6. `tests/e2e/test_task_breakdown.spec.ts` (created)
+
+## Detailed Findings
+
+### Security Analysis
+
+#### ✅ Authentication (A07)
+- **Status:** PASS
+- API calls use `projectsApi.generateTasks()` which routes through authenticated axios instance
+- JWT token automatically included via request interceptor in `lib/api.ts:20-28`
+- No bypass of auth middleware
+
+#### ✅ XSS Prevention (A03)
+- **Status:** PASS
+- All dynamic content rendered as text content (React auto-escapes)
+- Error messages from server displayed via `{taskGenerationError}` - safe JSX interpolation
+- No use of `dangerouslySetInnerHTML`
+
+#### ✅ Input Validation
+- **Status:** PASS
+- No direct user input in task generation flow
+- API endpoint receives only `projectId` (server-validated)
+
+### Reliability Analysis
+
+#### ✅ Error Handling
+- **Status:** PASS
+- `handleGenerateTaskBreakdown` catches all errors with try/catch
+- User-friendly error messages displayed
+- Error state allows retry via button
+
+**Code excerpt (DiscoveryProgress.tsx:276-288):**
+```typescript
+try {
+  await projectsApi.generateTasks(projectId);
+  // WebSocket messages will update UI progressively
+} catch (err) {
+  console.error('Failed to generate tasks:', err);
+  setIsGeneratingTasks(false);
+  if (err instanceof Error) {
+    setTaskGenerationError(`Failed to generate tasks: ${err.message}`);
+  } else {
+    setTaskGenerationError('Failed to generate tasks. Please try again.');
+  }
+}
+```
+
+#### ✅ Race Condition Prevention
+- **Status:** PASS
+- Guard clause at line 270: `if (isGeneratingTasks) return;`
+- Prevents duplicate API calls on rapid button clicks
+
+#### ✅ WebSocket Event Filtering
+- **Status:** PASS
+- All WebSocket handlers check `message.project_id !== projectId` before processing
+- Prevents cross-project state pollution
+
+#### ⚠️ Minor: WebSocket Error State Reset
+- **Priority:** Low
+- **Location:** DiscoveryProgress.tsx:405-440
+- **Observation:** When `planning_started` event arrives, previous error state is cleared. If API call fails but WebSocket message arrives anyway, states could momentarily conflict.
+- **Risk:** Minimal - server shouldn't send `planning_started` if API failed
+- **Recommendation:** No action required - current behavior is correct
+
+### State Management Analysis
+
+#### ✅ State Consistency
+- **Status:** PASS
+- Six related states managed together:
+  - `tasksGenerated`, `isGeneratingTasks`, `taskGenerationError`
+  - `taskGenerationProgress`, `issuesCount`, `tasksCount`
+- All states properly reset on new generation attempt
+
+#### ✅ Conditional Rendering
+- **Status:** PASS
+- Mutually exclusive UI states:
+  - Button: `!tasksGenerated && !isGeneratingTasks && !taskGenerationError`
+  - Progress: `isGeneratingTasks`
+  - Error: `taskGenerationError`
+  - Complete: `tasksGenerated`
+
+### Test Coverage
+
+#### Unit Tests: 89 passing
+- Button visibility tests (3)
+- Button click behavior (3)
+- WebSocket event handling (5)
+- Navigation (2)
+- Error handling (3)
+- Progress display (3)
+- Existing PRD tests updated (2)
+
+#### E2E Tests: 6 tests
+- Button display conditions
+- Loading state on click
+- WebSocket progress updates
+- Navigation to Tasks tab
+- Error state and retry
+- Button hidden during PRD generation
+
+### Issues Fixed During Review
+
+1. **TestID Mismatch** (Fixed)
+   - E2E test used `retry-generate-tasks-button`
+   - Component uses `retry-task-generation-button`
+   - Fixed in `tests/e2e/test_task_breakdown.spec.ts:257`
+
+## Recommendations
+
+### Immediate Actions
+None required - implementation is production-ready.
+
+### Future Improvements (Optional)
+1. Add loading skeleton during task generation for better perceived performance
+2. Consider adding progress percentage to task generation (if backend supports it)
+3. Add analytics tracking for task generation success/failure rates
+
+## Approval Status
+
+**✅ APPROVED FOR MERGE**
+
+The Feature 016-3 implementation follows security best practices, has comprehensive error handling, and includes thorough test coverage. The code is well-structured and maintainable.
+
+---
+
+*Review conducted following OWASP Web Application Security guidelines and Zero Trust principles.*