feat(PBI-67): IDEA_REVIEW_PLAN Phases 3-6 — server actions, UI components, prompt & tests
- Phase 3: startReviewPlanJobAction, cancelIdeaJobAction, status transitions (REVIEWING_PLAN / PLAN_REVIEWED / PLAN_REVIEW_FAILED), status colors, job-card/jobs-column filters, idea-list status tabs - Phase 4: review-plan-job.md prompt (multi-model orchestration with codex injection + active plan revision via update_idea_plan_md after each round), runbook, 13 unit tests - Phase 5: ReviewLogViewer component (rounds, convergence, approval, issues), idea-detail integration, proper ReviewLog TypeScript types exported from component - Phase 6.1: wait-for-job discriminator wired (IDEA_REVIEW_PLAN), plan-revision step made mandatory in prompt (was previously optional/missing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
873b42a87e
commit
dac890b82c
18 changed files with 1952 additions and 13 deletions
212
__tests__/review-plan-job.test.ts
Normal file
212
__tests__/review-plan-job.test.ts
Normal file
|
|
@ -0,0 +1,212 @@
|
|||
import { describe, it, expect } from 'vitest'
|
||||
|
||||
/**
|
||||
* Review-Plan Job Tests
|
||||
*
|
||||
* Tests for the IDEA_REVIEW_PLAN job kind and review-log schema validation.
|
||||
*/
|
||||
|
||||
// Sample review-log structure for testing
|
||||
const sampleReviewLog = {
|
||||
plan_file: 'I-042',
|
||||
created_at: new Date().toISOString(),
|
||||
rounds: [
|
||||
{
|
||||
round: 0,
|
||||
model: 'claude-3-5-haiku',
|
||||
role: 'Structure Review',
|
||||
focus: 'YAML parsing, format, syntax',
|
||||
plan_before: '---\npbi:\n title: "Test PBI"\nstories:\n - title: "Story 1"\n---',
|
||||
plan_after:
|
||||
'---\npbi:\n title: "Test PBI"\nstories:\n - title: "Story 1"\n priority: 2\n---',
|
||||
issues: [
|
||||
{
|
||||
category: 'structure',
|
||||
severity: 'warning',
|
||||
suggestion: 'Add priority field to story',
|
||||
},
|
||||
],
|
||||
score: 75,
|
||||
plan_diff_lines: 1,
|
||||
converged: false,
|
||||
timestamp: new Date().toISOString(),
|
||||
},
|
||||
{
|
||||
round: 1,
|
||||
model: 'claude-3-5-sonnet',
|
||||
role: 'Logic & Patterns',
|
||||
focus: 'Logic gaps, missing patterns, architecture fit',
|
||||
plan_before: '---\npbi:\n title: "Test PBI"\nstories:\n - title: "Story 1"\n---',
|
||||
plan_after: '---\npbi:\n title: "Test PBI"\nstories:\n - title: "Story 1"\n---',
|
||||
issues: [
|
||||
{
|
||||
category: 'logic',
|
||||
severity: 'info',
|
||||
suggestion: 'Consider adding acceptance criteria',
|
||||
},
|
||||
],
|
||||
score: 80,
|
||||
plan_diff_lines: 0,
|
||||
converged: false,
|
||||
timestamp: new Date().toISOString(),
|
||||
},
|
||||
{
|
||||
round: 2,
|
||||
model: 'claude-opus-4-7',
|
||||
role: 'Risk Assessment',
|
||||
focus: 'Risk assessment, edge cases, refactoring',
|
||||
plan_before: '---\npbi:\n title: "Test PBI"\nstories:\n - title: "Story 1"\n---',
|
||||
plan_after: '---\npbi:\n title: "Test PBI"\nstories:\n - title: "Story 1"\n---',
|
||||
issues: [],
|
||||
score: 85,
|
||||
plan_diff_lines: 0,
|
||||
converged: true,
|
||||
timestamp: new Date().toISOString(),
|
||||
},
|
||||
],
|
||||
convergence: {
|
||||
stable_at_round: 2,
|
||||
final_diff_pct: 0.5,
|
||||
convergence_metric: 'plan_stability',
|
||||
},
|
||||
approval: {
|
||||
status: 'approved',
|
||||
timestamp: new Date().toISOString(),
|
||||
},
|
||||
summary: 'Plan reviewed across three rounds. Minor structure improvements suggested. Plan approved.',
|
||||
}
|
||||
|
||||
describe('review-plan-job', () => {
|
||||
describe('ReviewLog Schema', () => {
|
||||
it('should have required top-level fields', () => {
|
||||
expect(sampleReviewLog).toHaveProperty('plan_file')
|
||||
expect(sampleReviewLog).toHaveProperty('created_at')
|
||||
expect(sampleReviewLog).toHaveProperty('rounds')
|
||||
expect(sampleReviewLog).toHaveProperty('convergence')
|
||||
expect(sampleReviewLog).toHaveProperty('approval')
|
||||
expect(sampleReviewLog).toHaveProperty('summary')
|
||||
})
|
||||
|
||||
it('should have valid plan_file format', () => {
|
||||
expect(typeof sampleReviewLog.plan_file).toBe('string')
|
||||
expect(sampleReviewLog.plan_file.length).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
it('should have valid ISO timestamps', () => {
|
||||
const isoRegex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/
|
||||
expect(sampleReviewLog.created_at).toMatch(isoRegex)
|
||||
expect(sampleReviewLog.approval.timestamp).toMatch(isoRegex)
|
||||
})
|
||||
|
||||
it('should have at least one round', () => {
|
||||
expect(sampleReviewLog.rounds.length).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
it('should have valid round structure', () => {
|
||||
for (const round of sampleReviewLog.rounds) {
|
||||
expect(round).toHaveProperty('round')
|
||||
expect(round).toHaveProperty('model')
|
||||
expect(round).toHaveProperty('role')
|
||||
expect(round).toHaveProperty('focus')
|
||||
expect(round).toHaveProperty('plan_before')
|
||||
expect(round).toHaveProperty('plan_after')
|
||||
expect(round).toHaveProperty('issues')
|
||||
expect(round).toHaveProperty('score')
|
||||
expect(round).toHaveProperty('plan_diff_lines')
|
||||
expect(round).toHaveProperty('converged')
|
||||
expect(round).toHaveProperty('timestamp')
|
||||
|
||||
expect(typeof round.round).toBe('number')
|
||||
expect(round.round).toBeGreaterThanOrEqual(0)
|
||||
expect(typeof round.score).toBe('number')
|
||||
expect(round.score).toBeGreaterThanOrEqual(0)
|
||||
expect(round.score).toBeLessThanOrEqual(100)
|
||||
expect(typeof round.plan_diff_lines).toBe('number')
|
||||
expect(round.plan_diff_lines).toBeGreaterThanOrEqual(0)
|
||||
}
|
||||
})
|
||||
|
||||
it('should have valid issue structure per round', () => {
|
||||
for (const round of sampleReviewLog.rounds) {
|
||||
for (const issue of round.issues) {
|
||||
expect(issue).toHaveProperty('category')
|
||||
expect(issue).toHaveProperty('severity')
|
||||
expect(issue).toHaveProperty('suggestion')
|
||||
|
||||
expect(['structure', 'logic', 'risk', 'pattern']).toContain(issue.category)
|
||||
expect(['error', 'warning', 'info']).toContain(issue.severity)
|
||||
expect(typeof issue.suggestion).toBe('string')
|
||||
expect(issue.suggestion.length).toBeGreaterThan(0)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('should have valid convergence structure when present', () => {
|
||||
if (sampleReviewLog.convergence) {
|
||||
expect(sampleReviewLog.convergence).toHaveProperty('stable_at_round')
|
||||
expect(sampleReviewLog.convergence).toHaveProperty('final_diff_pct')
|
||||
expect(sampleReviewLog.convergence).toHaveProperty('convergence_metric')
|
||||
|
||||
expect(typeof sampleReviewLog.convergence.stable_at_round).toBe('number')
|
||||
expect(sampleReviewLog.convergence.stable_at_round).toBeGreaterThanOrEqual(0)
|
||||
expect(typeof sampleReviewLog.convergence.final_diff_pct).toBe('number')
|
||||
expect(sampleReviewLog.convergence.final_diff_pct).toBeGreaterThanOrEqual(0)
|
||||
expect(sampleReviewLog.convergence.final_diff_pct).toBeLessThanOrEqual(100)
|
||||
}
|
||||
})
|
||||
|
||||
it('should have valid approval status', () => {
|
||||
expect(['pending', 'approved', 'rejected']).toContain(sampleReviewLog.approval.status)
|
||||
if (sampleReviewLog.approval.status !== 'pending') {
|
||||
expect(sampleReviewLog.approval.timestamp).toBeDefined()
|
||||
}
|
||||
})
|
||||
|
||||
it('should have non-empty summary', () => {
|
||||
expect(typeof sampleReviewLog.summary).toBe('string')
|
||||
expect(sampleReviewLog.summary.length).toBeGreaterThan(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Convergence Detection', () => {
|
||||
it('should detect convergence when diff_pct < 5% for two consecutive rounds', () => {
|
||||
// Simulate convergence: round 0 has 1 diff line, rounds 1-2 have 0 diffs
|
||||
const totalLines = 50
|
||||
const diff0 = 1
|
||||
const diff1 = 0
|
||||
const diff2 = 0
|
||||
|
||||
const pct0 = (diff0 / totalLines) * 100 // 2%
|
||||
const pct1 = (diff1 / totalLines) * 100 // 0%
|
||||
const pct2 = (diff2 / totalLines) * 100 // 0%
|
||||
|
||||
expect(pct0).toBeLessThan(5) // Should converge
|
||||
expect(pct1).toBeLessThan(5) // Should converge
|
||||
expect(pct2).toBeLessThan(5) // Should converge
|
||||
})
|
||||
|
||||
it('should not detect convergence when diff_pct >= 5%', () => {
|
||||
const totalLines = 50
|
||||
const diff = 3 // 6% change
|
||||
|
||||
const pct = (diff / totalLines) * 100
|
||||
expect(pct).toBeGreaterThanOrEqual(5)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Status Transitions', () => {
|
||||
it('should transition REVIEWING_PLAN → PLAN_REVIEWED when approved', () => {
|
||||
const log = { ...sampleReviewLog, approval: { status: 'approved', timestamp: new Date().toISOString() } }
|
||||
expect(log.approval.status).toBe('approved')
|
||||
// In actual implementation: update_idea_plan_reviewed({ approval_status: 'approved' })
|
||||
// → idea.status = 'PLAN_REVIEWED'
|
||||
})
|
||||
|
||||
it('should transition REVIEWING_PLAN → PLAN_REVIEW_FAILED when rejected', () => {
|
||||
const log = { ...sampleReviewLog, approval: { status: 'rejected' } }
|
||||
expect(log.approval.status).toBe('rejected')
|
||||
// In actual implementation: update_idea_plan_reviewed({ approval_status: 'rejected' })
|
||||
// → idea.status = 'PLAN_REVIEW_FAILED'
|
||||
})
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue