Align spec details view with enriched requirement metadata#433
Conversation
WalkthroughThe spec page now normalizes priority, guardrails, acceptance criteria, and impacted-file purpose metadata from plan output, renders that metadata in plan cards, and formats streamed thinking content before it reaches the timeline. ChangesSpec plan metadata and rendering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
app/(main)/task/[taskId]/spec/page.tsx (4)
516-520: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the
cnutility for the conditional priority-chip classes.The priority chip concatenates
priorityChipClass(...)into a template literal. Per the project convention, conditional class composition should go throughcn. Notecnisn't currently imported in this file, so add it to the existing@/lib/utilsimport.As per coding guidelines: "Use Tailwind CSS for styling and the `cn` utility for conditional classes".♻️ Proposed change
- {item.priority && ( - <span className={`inline-flex h-5 items-center rounded-full border px-2 text-[10px] font-medium leading-none capitalize ${priorityChipClass(item.priority)}`}> + {item.priority && ( + <span className={cn("inline-flex h-5 items-center rounded-full border px-2 text-[10px] font-medium leading-none capitalize", priorityChipClass(item.priority))}> {item.priority} </span> )}And add
cnto the import at Line 48:-import { getStreamEventPayload, normalizeMarkdownForPreview } from "`@/lib/utils`"; +import { cn, getStreamEventPayload, normalizeMarkdownForPreview } from "`@/lib/utils`";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(main)/task/[taskId]/spec/page.tsx around lines 516 - 520, The priority chip in the task spec page is building classes with a template literal instead of the project’s `cn` utility. Update the conditional priority-chip rendering to compose the static classes with `priorityChipClass(item.priority)` through `cn`, and add `cn` to the existing `@/lib/utils` import in this component so the class concatenation follows the shared styling convention.Source: Coding guidelines
115-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the guardrail/acceptance-criteria normalization into shared helpers.
The guardrail mapping (Lines 122-136) and acceptance-criteria mapping (Line 137) are duplicated almost verbatim in
specOutputToPlan(Lines 186-204). The sametypeof c === "string" ? c : (c?.text ?? JSON.stringify(c))pattern also recurs inFunctionalRequirementCard(Line 297). Centralizing avoids the two copies drifting apart as the API shape evolves.♻️ Suggested shared helpers
function normalizeGuardrails(raw: any): GuardrailItem[] | undefined { const list = (Array.isArray(raw) ? raw : []) .map((g: any) => { if (typeof g === "string") return { statement: g }; const statement = g?.statement ?? g?.text; if (!statement) return null; return { type: typeof g?.type === "string" ? g.type : undefined, statement: String(statement), rationale: typeof g?.rationale === "string" ? g.rationale : undefined, consequences: Array.isArray(g?.consequences) ? g.consequences.map((c: any) => String(c)) : undefined, }; }) .filter((g): g is GuardrailItem => g !== null); return list.length > 0 ? list : undefined; } function normalizeAcceptanceCriteria(raw: any): string[] { return (Array.isArray(raw) ? raw : []).map((c: any) => typeof c === "string" ? c : (c?.text ?? JSON.stringify(c)) ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(main)/task/[taskId]/spec/page.tsx around lines 115 - 137, The guardrail and acceptance-criteria normalization logic is duplicated in multiple places, so extract it into shared helpers and reuse them. Move the mapping/filtering currently in the spec page’s normalization block into functions like normalizeGuardrails and normalizeAcceptanceCriteria, then update specOutputToPlan and FunctionalRequirementCard to call those helpers instead of inlining the same conversions. Keep the behavior identical, including string handling and fallback serialization, so the API-shape logic stays consistent in one place.
1298-1306: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMemo reformats the full (growing) chunk content on every stream update.
Chunk content is appended in place during streaming (consecutive chunks are merged into the last item at Line 1010), so this memo re-runs
formatSpecThinkingMarkdown's regex passes over the entire, ever-growing chunk string on each chunk event — roughly quadratic in content length for long "thinking" outputs. For typical lengths this is fine, but consider formatting per item only when its content changes (e.g., caching by content) if you observe jank on long streams.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(main)/task/[taskId]/spec/page.tsx around lines 1298 - 1306, The current useMemo over displayedStreamItems repeatedly re-runs formatSpecThinkingMarkdown on the full accumulated chunk text whenever streamItems changes, which can become quadratic for long streaming outputs. Update the rendering path around displayedStreamItems so chunk formatting is cached or applied only when an individual chunk’s content changes, rather than remapping and reformatting the entire growing string on every stream update. Use the existing streamItems mapping logic and formatSpecThinkingMarkdown as the key symbols to adjust.
457-461: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
isSpecThinkingHeadingis defined but never used.This helper isn't referenced by
formatSpecThinkingMarkdownor anywhere else in the file. It looks like leftover from an earlier heuristic that the regex approach replaced. Remove it to avoid dead code (and a likely lint warning).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(main)/task/[taskId]/spec/page.tsx around lines 457 - 461, The helper is dead code: isSpecThinkingHeading is defined but not referenced anywhere in the file, including formatSpecThinkingMarkdown. Remove the unused function and any related import or leftover heuristic code so the module only keeps the active heading parsing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/`(main)/task/[taskId]/spec/page.tsx:
- Around line 516-520: The priority chip in the task spec page is building
classes with a template literal instead of the project’s `cn` utility. Update
the conditional priority-chip rendering to compose the static classes with
`priorityChipClass(item.priority)` through `cn`, and add `cn` to the existing
`@/lib/utils` import in this component so the class concatenation follows the
shared styling convention.
- Around line 115-137: The guardrail and acceptance-criteria normalization logic
is duplicated in multiple places, so extract it into shared helpers and reuse
them. Move the mapping/filtering currently in the spec page’s normalization
block into functions like normalizeGuardrails and normalizeAcceptanceCriteria,
then update specOutputToPlan and FunctionalRequirementCard to call those helpers
instead of inlining the same conversions. Keep the behavior identical, including
string handling and fallback serialization, so the API-shape logic stays
consistent in one place.
- Around line 1298-1306: The current useMemo over displayedStreamItems
repeatedly re-runs formatSpecThinkingMarkdown on the full accumulated chunk text
whenever streamItems changes, which can become quadratic for long streaming
outputs. Update the rendering path around displayedStreamItems so chunk
formatting is cached or applied only when an individual chunk’s content changes,
rather than remapping and reformatting the entire growing string on every stream
update. Use the existing streamItems mapping logic and
formatSpecThinkingMarkdown as the key symbols to adjust.
- Around line 457-461: The helper is dead code: isSpecThinkingHeading is defined
but not referenced anywhere in the file, including formatSpecThinkingMarkdown.
Remove the unused function and any related import or leftover heuristic code so
the module only keeps the active heading parsing logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c92f2ac-9c94-464f-bb09-3cbebe3ba2ee
📒 Files selected for processing (1)
app/(main)/task/[taskId]/spec/page.tsx
2 participants
Summary
This PR updates the spec details page to better reflect the current specification shape returned by workflows.
What changed
##Screenshot
Why
The latest spec output includes richer requirement metadata and more outcome-oriented descriptions. This update makes the spec page reflect that structure more clearly and keeps important implementation and verification context visible during review.
Validation