feat(ui): chat trace state and streaming plumbing (stacked on #331)#332
feat(ui): chat trace state and streaming plumbing (stacked on #331)#332Manushpm8 wants to merge 7 commits into
Conversation
Foundational frontend utilities for the staged chat UI/UX work: - cn(): clsx + tailwind-merge className composition so conflicting Tailwind utilities resolve deterministically. - titleCaseWords()/ACRONYMS: humanize snake_case and `__`-separated identifiers (tool and function names) into display labels, upper-casing common acronyms. - AppMotionConfig: a Motion provider that honors prefers-reduced-motion. Adds clsx, tailwind-merge, and motion. Covered 100% by unit tests; no UI behavior changes yet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
Adds the UI-only shared component library the chat redesign builds on: - Sources/*: source chips, list, and reference parsing (web + doc kinds). - research/*: StatusDot, ToolCallRow, and a generic tool-label engine for the live research trace. - Surface/Card, Actions/CopyButton, CollapsibleBlock, Mermaid. - MarkdownRenderer upgrade: an answer variant, superscript citations with hover popovers, KaTeX math, and Mermaid diagrams. Prod's artifact-URL handling is preserved verbatim; chart fences fall through unchanged. Adds mermaid, katex, rehype-katex, remark-math, unist-util-visit. No warehouse/predictive features are ported. Covered by unit and render tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
WalkthroughThis PR adds model-aware chat sending, richer thinking-step and deep-research trace handling, source-aware Markdown rendering with math and Mermaid support, new research/layout UI primitives, shared source parsing utilities, and frontend styling and utility dependencies. ChangesChat flow and research traces
Answer rendering and sources
Layout and UI foundations
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)frontends/ui/src/app/globals.cssFile contains syntax errors that prevent linting: Line 155: Tailwind-specific syntax is disabled.; Line 168: Tailwind-specific syntax is disabled.; Line 172: Tailwind-specific syntax is disabled.; Line 176: Tailwind-specific syntax is disabled.; Line 180: Tailwind-specific syntax is disabled.; Line 184: Tailwind-specific syntax is disabled.; Line 188: Tailwind-specific syntax is disabled.; Line 192: Tailwind-specific syntax is disabled.; Line 196: Tailwind-specific syntax is disabled.; Line 200: Tailwind-specific syntax is disabled.; Line 208: Tailwind-specific syntax is disabled.; Line 212: Tailwind-specific syntax is disabled.; Line 216: Tailwind-specific syntax is disabled.; Line 220: Tailwind-specific syntax is disabled.; Line 225: Tailwind-specific syntax is disabled.; Line 231: Tailwind-specific syntax is disabled.; Line 235: Tailwind-specific syntax is disabled.; Line 239: Tailwind-specific syntax is disabled.; Line 247: Tailwind-specific syntax is disabled.; Line 251: Tailwind-specific syntax is disabled.; Line 255: Tailwind-specific syntax is disabled.; Line 259: Tailwind-specific syntax is disabled.; Line 263: Tailwind-specific syntax is disabled.; Line 267: Tailwind-specific syntax is disabled.; Line 271: Tailwind-specific syntax is disabled.; Line 275: Tailwind-specific syntax is disabled.; Line 279: Tailwind-specific syntax is disabled.; Line 283: Tailwind-specific syntax is disabled. Comment |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontends/ui/src/features/chat/store.ts (1)
1281-1400: 📐 Maintainability & Code Quality | 🔵 Trivial
failThinkingStepduplicatescompleteThinkingStep's update logic.Both actions repeat the same "map ephemeral thinkingSteps → find step → rebuild persisted message.thinkingSteps → set()" sequence, differing only in the literal status (
'success'vs'error'). This pattern is now duplicated acrosscompleteThinkingStep,failThinkingStep,appendToThinkingStep, andupdateThinkingStepByFunctionName. Consider extracting a sharedupdateThinkingStepFields(stepId, patch)helper that both ephemeral and persisted updates route through, taking the status/completedAt patch as a parameter.♻️ Sketch of a shared helper
+ const patchThinkingStep = ( + stepId: string, + patch: Partial<Pick<ThinkingStep, 'isComplete' | 'completedAt' | 'status'>> + ) => { + const { currentConversation, conversations, thinkingSteps, activeThinkingStepId } = get() + const apply = (s: ThinkingStep) => (s.id === stepId ? { ...s, ...patch } : s) + const updatedThinkingSteps = thinkingSteps.map(apply) + const step = thinkingSteps.find((s) => s.id === stepId) + let updatedConversation = currentConversation + let updatedConversations = conversations + if (step && currentConversation) { + const updatedMessages = currentConversation.messages.map((msg) => + msg.id === step.userMessageId && msg.thinkingSteps + ? { ...msg, thinkingSteps: msg.thinkingSteps.map(apply) } + : msg + ) + updatedConversation = { ...currentConversation, messages: updatedMessages, updatedAt: new Date() } + updatedConversations = updateConversationInList(conversations, updatedConversation) + } + set( + { + thinkingSteps: updatedThinkingSteps, + activeThinkingStepId: activeThinkingStepId === stepId ? null : activeThinkingStepId, + currentConversation: updatedConversation, + conversations: updatedConversations, + }, + false, + 'patchThinkingStep' + ) + }Then
completeThinkingStep/failThinkingStepbecome one-liners callingpatchThinkingStep(stepId, { isComplete: true, completedAt: ..., status: ... }).🤖 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 `@frontends/ui/src/features/chat/store.ts` around lines 1281 - 1400, Extract the duplicated thinking-step update flow from completeThinkingStep, failThinkingStep, appendToThinkingStep, and updateThinkingStepByFunctionName into a shared updateThinkingStepFields or equivalent helper. Have it apply the supplied patch consistently to both ephemeral thinkingSteps and the matching persisted message.thinkingSteps, while preserving each action’s success/error status behavior and existing conversation/set updates.
🤖 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.
Inline comments:
In `@frontends/ui/src/app/globals.css`:
- Around line 360-368: Rename the dotBreathe keyframe to a kebab-case name and
update both references to it in the animation declarations, preserving the
existing animation behavior and styling.
In `@frontends/ui/src/features/chat/lib/deep-research-trace.ts`:
- Around line 32-50: Update collapseRepeats so repeated tools steps are compared
with the most recent tools entry rather than only out[out.length - 1]. Skip
agents and other non-tools steps while tracking that prior tool, preserve the
earlier id when merging, and retain all non-tool fold steps in their original
order.
In `@frontends/ui/src/features/chat/lib/intermediate-step-parser.ts`:
- Around line 181-190: Extract the inline model-name hyphen capitalization logic
into a named helper such as capitalizeModelSegment, preserving acronym
uppercasing and the original casing of each word’s tail for tokens like 30B and
A3B. Add a one-line comment documenting this intentional case-preservation
difference from titleCaseWords, then call the helper from the existing
model-name formatting flow.
In `@frontends/ui/src/shared/components/Actions/CopyButton.tsx`:
- Around line 22-33: Update CopyButton’s handleCopy timeout management so only
the latest reset can change copied state: track the pending timeout across
renders, clear any existing timeout before scheduling a new one, and clear the
pending timeout on unmount. Preserve the current copy success and failure
behavior.
In `@frontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsx`:
- Around line 1-90: Add a CollapsibleBlock.spec.tsx test suite for the shared
CollapsibleBlock component, covering the controlled open state and onOpenChange
toggle behavior, plus the collapsible={false} static path where the body remains
rendered without a toggle. Follow existing UI component test patterns.
- Around line 60-73: Update CollapsibleBlock so the toggle button and disclosed
body share a generated unique identifier: use React’s useId, assign the
resulting id to the collapsible content region, and set aria-controls on the
button to that id while preserving the existing aria-expanded behavior. Apply
the association only to the collapsible path as appropriate.
In `@frontends/ui/src/shared/components/MarkdownRenderer/Citation.tsx`:
- Around line 32-71: Update the citation trigger and tooltip in the Citation
component so the anchor references the rendered tooltip through aria-describedby
whenever the tooltip is present. Assign a stable unique id to the tooltip, apply
it to the role="tooltip" element, and ensure the reference is omitted when no
source or tooltip is rendered.
In `@frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsx`:
- Around line 9-18: Update MarkdownRenderer to lazy-load rehype-katex and its
KaTeX stylesheet only when math rendering is needed, removing their module-scope
imports while preserving existing math output and Mermaid’s lazy-loading
pattern. Use the renderer’s existing math/answer handling symbols to trigger the
deferred loading.
- Around line 267-283: Memoize the remarkPlugins and rehypePlugins arrays in
MarkdownRenderer, keyed by hasCitations where needed, and pass those stable
references to ReactMarkdown instead of creating literal arrays inline. Keep the
existing plugin contents and citation-dependent behavior unchanged.
- Around line 279-280: Update the rehypePlugins configuration in
MarkdownRenderer so rehypeKatex runs before rehypeCitations, while retaining the
current conditional citation behavior. Add a regression test covering LaTeX with
bracketed arguments such as \sqrt[3]{x} alongside [n]-style text, verifying both
math and citation rendering remain correct.
In
`@frontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.spec.ts`:
- Around line 20-42: Add a regression test in the rehypeCitations suite that
builds a paragraph containing a code element with “[1]”, runs runPlugin, and
asserts the code child remains unchanged without a cite element. Keep the
existing marker-processing tests intact and verify only code-element text is
excluded from citation transformation.
In `@frontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.ts`:
- Around line 17-44: Update the text-node visitor in the citation transformation
to return without rewriting nodes whose direct parent is a code element.
Preserve the existing citation replacement behavior for all other text nodes,
including normal parent and index validation.
In `@frontends/ui/src/shared/components/Mermaid/MermaidBlock.tsx`:
- Around line 68-74: Harden the Mermaid rendering path before its
dangerouslySetInnerHTML assignment by sanitizing the generated SVG with
DOMPurify, while retaining Mermaid’s securityLevel: 'strict' configuration.
Apply this to the SVG injection flow in the Mermaid rendering component,
including the additional affected block, and inject only the sanitized output.
- Around line 183-227: Update the fullscreen container in MermaidBlock to expose
modal semantics with role="dialog" and aria-modal="true" while fullscreen is
active, and manage focus by moving it to the “Exit fullscreen” button when
entering fullscreen. Keep the existing Escape close behavior and restore focus
appropriately when leaving fullscreen.
In `@frontends/ui/src/shared/components/Sources/parse-references.ts`:
- Around line 215-267: Extract the repeated bullet, heading, and ordered-list
detection from tabularizeEntityLines into a local isMarkupLine(line) helper,
then use it for both the initial guard and the entity-run loop condition.
Preserve the existing markup patterns and tabularization behavior.
- Around line 52-61: Update the returned SourceRef object in the reference
parsing flow to preserve the URL for bare web references: when kind is 'web' and
the reference is not a file/document source, set url to ref. Leave document
handling and existing label/kind behavior unchanged.
In `@frontends/ui/src/shared/components/Sources/SourceStrip.tsx`:
- Around line 38-42: External links lack an accessible indication that they open
in a new tab. In SourceStrip.tsx at lines 38-42, add visually hidden text or
update the link aria-label to announce this behavior; make the equivalent change
to the domain link in SourceList.tsx at lines 42-49.
- Around line 74-83: Add an aria-expanded attribute to the expander button in
the hasMore && !expanded block, binding it to the expanded state managed by
setExpanded. Preserve the existing click behavior and label.
In `@frontends/ui/src/shared/lib/motion.spec.tsx`:
- Around line 11-32: Update the AppMotionConfig tests to mock or instrument
MotionConfig and assert its reducedMotion prop: use 'never' when
useReducedMotion returns false and 'always' when it returns true. Keep the
existing child-rendering assertions, and ensure both motion modes verify the
configuration behavior.
---
Outside diff comments:
In `@frontends/ui/src/features/chat/store.ts`:
- Around line 1281-1400: Extract the duplicated thinking-step update flow from
completeThinkingStep, failThinkingStep, appendToThinkingStep, and
updateThinkingStepByFunctionName into a shared updateThinkingStepFields or
equivalent helper. Have it apply the supplied patch consistently to both
ephemeral thinkingSteps and the matching persisted message.thinkingSteps, while
preserving each action’s success/error status behavior and existing
conversation/set updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 53539a58-184a-4b02-abe4-f2d831255478
⛔ Files ignored due to path filters (1)
frontends/ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (52)
frontends/ui/package.jsonfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/app/globals.cssfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/features/layout/types.tsfrontends/ui/src/shared/components/Actions/CopyButton.spec.tsxfrontends/ui/src/shared/components/Actions/CopyButton.tsxfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/shared/components/CollapsibleBlock/index.tsfrontends/ui/src/shared/components/MarkdownRenderer/Citation.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/Citation.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.tsfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsxfrontends/ui/src/shared/components/Mermaid/MermaidBlock.tsxfrontends/ui/src/shared/components/Mermaid/index.tsfrontends/ui/src/shared/components/Sources/SourceKindIcon.tsxfrontends/ui/src/shared/components/Sources/SourceList.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.spec.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/shared/components/Sources/parse-references.spec.tsfrontends/ui/src/shared/components/Sources/parse-references.tsfrontends/ui/src/shared/components/Sources/source-utils.spec.tsfrontends/ui/src/shared/components/Sources/source-utils.tsfrontends/ui/src/shared/components/Sources/types.tsfrontends/ui/src/shared/components/Surface/Card.spec.tsxfrontends/ui/src/shared/components/Surface/Card.tsxfrontends/ui/src/shared/components/research/StatusDot.tsxfrontends/ui/src/shared/components/research/ToolCallRow.tsxfrontends/ui/src/shared/components/research/index.tsfrontends/ui/src/shared/components/research/research-labels.spec.tsfrontends/ui/src/shared/components/research/research-labels.tsfrontends/ui/src/shared/lib/cn.spec.tsfrontends/ui/src/shared/lib/cn.tsfrontends/ui/src/shared/lib/humanize.spec.tsfrontends/ui/src/shared/lib/humanize.tsfrontends/ui/src/shared/lib/motion.spec.tsxfrontends/ui/src/shared/lib/motion.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/shared/lib/humanize.spec.tsfrontends/ui/src/shared/components/Sources/SourceKindIcon.tsxfrontends/ui/src/shared/lib/motion.spec.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.spec.tsxfrontends/ui/src/shared/lib/cn.tsfrontends/ui/src/shared/components/Mermaid/index.tsfrontends/ui/src/shared/components/Sources/types.tsfrontends/ui/src/shared/components/Sources/SourceList.tsxfrontends/ui/src/shared/lib/motion.tsxfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/shared/components/Actions/CopyButton.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/Citation.tsxfrontends/ui/src/shared/components/CollapsibleBlock/index.tsfrontends/ui/src/shared/components/research/StatusDot.tsxfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.tsfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/shared/components/Surface/Card.tsxfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/shared/components/Sources/source-utils.spec.tsfrontends/ui/src/shared/components/research/research-labels.spec.tsfrontends/ui/src/shared/components/research/index.tsfrontends/ui/src/shared/components/Actions/CopyButton.tsxfrontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsxfrontends/ui/src/shared/lib/humanize.tsfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.spec.tsfrontends/ui/src/shared/components/Sources/source-utils.tsfrontends/ui/src/shared/components/Surface/Card.spec.tsxfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/lib/cn.spec.tsfrontends/ui/src/shared/components/research/ToolCallRow.tsxfrontends/ui/src/shared/components/Sources/parse-references.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/Citation.spec.tsxfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/shared/components/Mermaid/MermaidBlock.tsxfrontends/ui/src/shared/components/research/research-labels.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/shared/components/Sources/parse-references.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/features/layout/types.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/features/chat/lib/intermediate-step-parser.ts
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/shared/lib/humanize.spec.tsfrontends/ui/src/shared/components/Sources/SourceKindIcon.tsxfrontends/ui/src/shared/lib/motion.spec.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.spec.tsxfrontends/ui/src/shared/lib/cn.tsfrontends/ui/src/shared/components/Mermaid/index.tsfrontends/ui/src/shared/components/Sources/types.tsfrontends/ui/src/shared/components/Sources/SourceList.tsxfrontends/ui/src/shared/lib/motion.tsxfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/shared/components/Actions/CopyButton.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/Citation.tsxfrontends/ui/src/shared/components/CollapsibleBlock/index.tsfrontends/ui/src/shared/components/research/StatusDot.tsxfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.tsfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/shared/components/Surface/Card.tsxfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/shared/components/Sources/source-utils.spec.tsfrontends/ui/src/shared/components/research/research-labels.spec.tsfrontends/ui/src/shared/components/research/index.tsfrontends/ui/src/shared/components/Actions/CopyButton.tsxfrontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsxfrontends/ui/src/shared/lib/humanize.tsfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.spec.tsfrontends/ui/src/shared/components/Sources/source-utils.tsfrontends/ui/src/shared/components/Surface/Card.spec.tsxfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/lib/cn.spec.tsfrontends/ui/src/shared/components/research/ToolCallRow.tsxfrontends/ui/src/shared/components/Sources/parse-references.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/Citation.spec.tsxfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/shared/components/Mermaid/MermaidBlock.tsxfrontends/ui/src/shared/components/research/research-labels.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/shared/components/Sources/parse-references.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/features/layout/types.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/features/chat/lib/intermediate-step-parser.ts
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/shared/lib/humanize.spec.tsfrontends/ui/src/shared/components/Sources/SourceKindIcon.tsxfrontends/ui/src/shared/lib/motion.spec.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.spec.tsxfrontends/ui/src/shared/lib/cn.tsfrontends/ui/src/shared/components/Mermaid/index.tsfrontends/ui/src/shared/components/Sources/types.tsfrontends/ui/src/shared/components/Sources/SourceList.tsxfrontends/ui/src/shared/lib/motion.tsxfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/shared/components/Actions/CopyButton.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/Citation.tsxfrontends/ui/src/shared/components/CollapsibleBlock/index.tsfrontends/ui/src/shared/components/research/StatusDot.tsxfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.tsfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/shared/components/Surface/Card.tsxfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/shared/components/Sources/source-utils.spec.tsfrontends/ui/src/shared/components/research/research-labels.spec.tsfrontends/ui/src/shared/components/research/index.tsfrontends/ui/src/shared/components/Actions/CopyButton.tsxfrontends/ui/package.jsonfrontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsxfrontends/ui/src/shared/lib/humanize.tsfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.spec.tsfrontends/ui/src/shared/components/Sources/source-utils.tsfrontends/ui/src/shared/components/Surface/Card.spec.tsxfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/lib/cn.spec.tsfrontends/ui/src/shared/components/research/ToolCallRow.tsxfrontends/ui/src/shared/components/Sources/parse-references.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/Citation.spec.tsxfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/shared/components/Mermaid/MermaidBlock.tsxfrontends/ui/src/shared/components/research/research-labels.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/shared/components/Sources/parse-references.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/features/layout/types.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/app/globals.cssfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/features/chat/lib/intermediate-step-parser.ts
🪛 ast-grep (0.44.1)
frontends/ui/src/shared/components/Mermaid/MermaidBlock.tsx
[warning] 251-251: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation
(react-unsafe-html-injection)
frontends/ui/src/shared/components/research/research-labels.ts
[warning] 167-167: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(["']${escapedKey}["']\\s*:\\s*"((?:\\\\.|[^"\\\\])*)")
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 172-172: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(["']${escapedKey}["']\\s*:\\s*'((?:\\\\.|[^'\\\\])*)')
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🪛 OpenGrep (1.25.0)
frontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.ts
[ERROR] 24-24: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
frontends/ui/src/shared/components/Mermaid/MermaidBlock.tsx
[WARNING] 243-253: dangerouslySetInnerHTML with dynamic content can lead to XSS. Sanitize the input with a library like DOMPurify before rendering.
(coderabbit.xss.react-dangerously-set-innerhtml)
🪛 Stylelint (17.14.0)
frontends/ui/src/app/globals.css
[error] 360-360: Expected keyframe name "dotBreathe" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
🔇 Additional comments (55)
frontends/ui/src/shared/lib/cn.ts (1)
4-12: LGTM!frontends/ui/src/shared/lib/cn.spec.ts (1)
4-20: LGTM!frontends/ui/src/shared/lib/humanize.ts (1)
4-26: LGTM!frontends/ui/src/shared/lib/humanize.spec.ts (1)
4-27: LGTM!frontends/ui/src/shared/lib/motion.tsx (1)
14-17: 🎯 Functional CorrectnessNo change needed
useReducedMotion()is intentionallyfalseon the first render for SSR consistency, soAppMotionConfigdoesn’t introduce a reduced-motion regression.> Likely an incorrect or invalid review comment.frontends/ui/src/shared/components/Surface/Card.tsx (2)
1-40: LGTM!
31-35: 📐 Maintainability & Code QualityProvide the required UI validation evidence.
The supplied context reports lint, type-check, and test success, but not the required frontend build validation or screenshot for the rendered
SourceCardstyling. Please include evidence fornpm run lint,npm run type-check,npm run test:ci, the UI build, and a screenshot of the affected card.Sources: Coding guidelines, Path instructions
frontends/ui/src/shared/components/Surface/Card.spec.tsx (1)
1-45: LGTM!frontends/ui/src/features/chat/lib/intermediate-step-parser.ts (5)
98-106: LGTM!
126-149: LGTM!
192-202: LGTM!
213-229: LGTM!
231-237: 🩺 Stability & AvailabilityFolded text may blank on partial frames.
extractFoldedOutput()returns''when a folded step has**Function Input:**but no**Function Output:**, anduse-websocket-chat.tsapplies it to every structured intermediate update. If the backend can emit those frames before completion, preserve the existing text until the output arrives.frontends/ui/src/features/chat/lib/intermediate-step-parser.spec.ts (1)
1-119: LGTM!frontends/ui/src/features/chat/store.spec.ts (3)
586-605: LGTM!
1279-1355: LGTM!
2285-2347: LGTM!frontends/ui/src/shared/components/Sources/types.ts (1)
1-22: LGTM!frontends/ui/src/shared/components/Sources/source-utils.ts (1)
1-58: LGTM!frontends/ui/src/features/layout/data-sources.ts (1)
11-124: LGTM!frontends/ui/src/shared/components/Sources/source-utils.spec.ts (1)
1-58: LGTM!frontends/ui/src/features/layout/data-sources.spec.ts (1)
1-45: LGTM!frontends/ui/src/shared/components/Sources/parse-references.spec.ts (1)
1-204: LGTM!frontends/ui/src/shared/components/Sources/parse-references.ts (1)
12-13: 🎯 Functional CorrectnessNo change needed here.
splitReferencesis meant to parse only the backend-baked structured block, whilestripTrailingReferencesstrips any trailing Sources/References section without parsing it. The broader matcher there is intentional, so wideningREFERENCES_BLOCK_REis not required.> Likely an incorrect or invalid review comment.frontends/ui/package.json (2)
27-32: LGTM!Also applies to: 34-34, 37-43
33-36: 🗄️ Data Integrity & IntegrationReact 18 is compatible with Next 16 here
next@16.2.6acceptsreact/react-dom^18.2.0, and the lockfile already resolves to React 18.3.1, so this pin is not a build/install blocker.> Likely an incorrect or invalid review comment.frontends/ui/src/app/globals.css (1)
315-359: LGTM!Also applies to: 369-383, 386-394, 397-434, 436-441
frontends/ui/src/shared/components/Actions/CopyButton.spec.tsx (1)
1-36: LGTM!frontends/ui/src/shared/components/CollapsibleBlock/index.ts (1)
1-5: LGTM!frontends/ui/src/features/layout/types.ts (1)
28-34: LGTM!Also applies to: 53-56, 73-76, 91-94
frontends/ui/src/features/layout/store.ts (2)
25-26: LGTM!Also applies to: 36-37, 58-99
126-131: 🎯 Functional CorrectnessNo action —
promptDraftisn’t part of the chat session lifecycle. It lives infrontends/ui/src/features/layout/store.tsand isn’t reset bycreateConversationorstartNewSessionDraft, so this doesn’t indicate a conversation-to-conversation leak.> Likely an incorrect or invalid review comment.frontends/ui/src/features/layout/store.spec.ts (2)
21-22: LGTM!Also applies to: 32-33, 188-204
139-186: 🎯 Functional Correctness | ⚡ Quick winMissing test: restoring via
openRightPanelswitch, not justcloseRightPanel.
openRightPanel's!collapsesSidebar && state.sessionsAutoCollapsedbranch (restoring the sidebar when switching directly to a non-collapsing panel, e.g. from'research'to'settings', without going throughcloseRightPanel) isn't covered here.✅ Suggested additional test
test('does not auto-restore a sidebar the user collapsed manually', () => { useLayoutStore.setState({ sessionsCollapsed: true, sessionsAutoCollapsed: false, rightPanel: 'research' }) useLayoutStore.getState().closeRightPanel() expect(useLayoutStore.getState().sessionsCollapsed).toBe(true) }) + + test('switching to a non-collapsing panel restores an auto-collapsed sidebar', () => { + useLayoutStore.setState({ sessionsCollapsed: false, rightPanel: null }) + useLayoutStore.getState().openRightPanel('research') + + useLayoutStore.getState().openRightPanel('settings') + + expect(useLayoutStore.getState().rightPanel).toBe('settings') + expect(useLayoutStore.getState().sessionsCollapsed).toBe(false) + expect(useLayoutStore.getState().sessionsAutoCollapsed).toBe(false) + }) })frontends/ui/src/shared/components/research/research-labels.ts (1)
1-245: LGTM!frontends/ui/src/shared/components/research/research-labels.spec.ts (1)
1-113: LGTM!frontends/ui/src/shared/components/research/StatusDot.tsx (1)
1-20: LGTM!frontends/ui/src/shared/components/research/ToolCallRow.tsx (1)
1-51: LGTM!frontends/ui/src/shared/components/research/index.ts (1)
1-19: LGTM!frontends/ui/src/shared/components/MarkdownRenderer/types.ts (1)
4-5: LGTM!Also applies to: 20-27
frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsx (1)
63-65: Answer-mode citation/paragraph/list rendering looks correct.
citeresolution,isAnswer-gatedp/listyling, and theolstartforwarding fix are all sound and match the added test coverage.Also applies to: 146-177
frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsx (1)
352-429: LGTM!frontends/ui/src/shared/components/Mermaid/MermaidBlock.tsx (1)
40-166: Zoom/pan/theme-tracking logic is solid.Fit-to-width, wheel-to-cursor zoom with scroll anchoring, pointer-capture-based drag, and the
nv-darkMutationObserver are all well-implemented and match the test expectations in MermaidBlock.spec.tsx.frontends/ui/src/shared/components/Mermaid/index.ts (1)
4-4: LGTM!frontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsx (1)
1-113: LGTM!frontends/ui/src/shared/components/Sources/SourceKindIcon.tsx (1)
17-33: LGTM!frontends/ui/src/shared/components/Sources/SourceStrip.spec.tsx (1)
19-44: LGTM!frontends/ui/src/shared/components/MarkdownRenderer/Citation.spec.tsx (1)
20-59: LGTM!frontends/ui/src/shared/components/Sources/SourceStrip.tsx (1)
22-26: 🎯 Functional Correctness
tone={source.kind}is supported
CardTonealready includes bothwebanddoc, so this prop value matches the component contract.> Likely an incorrect or invalid review comment.frontends/ui/src/features/chat/types.ts (1)
64-64: LGTM!Also applies to: 185-186, 219-226, 470-470, 502-503, 642-643
frontends/ui/src/features/chat/store.ts (2)
750-782: LGTM!
2406-2442: 🗄️ Data Integrity & Integration
hydrateDeepResearchFromMessageis test-only; this fallback does not affect runtime chat state. The production load path already scopes deep-research jobs throughdeepResearchOwnerConversationId/currentConversation, so there’s no cross-conversation overwrite here.> Likely an incorrect or invalid review comment.frontends/ui/src/adapters/api/websocket-client.ts (1)
228-240: LGTM!frontends/ui/src/features/chat/hooks/use-websocket-chat.ts (1)
58-62: LGTM!Also applies to: 109-109, 419-419, 595-609, 844-860, 875-876, 1102-1102, 1224-1224, 1250-1250
frontends/ui/src/features/chat/lib/deep-research-trace.spec.ts (1)
1-151: LGTM!
- rehype-citations: skip [n] markers inside code/pre so code samples like arr[1] are not turned into citations. - CopyButton: track the reset timeout in a ref, clearing it on re-click and unmount. - SourceList: always render a clickable link for URL-bearing sources, including when the label equals the domain. - source-utils: reuse the computed kind instead of re-testing the URL regex. - CollapsibleBlock: wire aria-controls to the disclosed region id. - SourceStrip: expose aria-expanded on the "+N more" button. - Citation: keep URL-less citations keyboard-focusable. - Add CollapsibleBlock and SourceList unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
49bd80c to
6abdf1a
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontends/ui/src/shared/components/MarkdownRenderer/Citation.tsx (1)
33-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid dead tab stops for missing citations.
When
srcis absent, this expression setstabIndex={0}, but the component returns the chip without any tooltip or action. Unresolved citation markers therefore become focusable but inert. Restrict this tosrc && !src.url, or render unresolved markers non-focusable, and add a regression test.As per path instructions, “Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls, resilient loading and error states, and report/chat state consistency.”
🤖 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 `@frontends/ui/src/shared/components/MarkdownRenderer/Citation.tsx` around lines 33 - 43, Update the Citation component’s anchor tabIndex logic so unresolved citations without src are not focusable; only citations with a valid src but no URL should receive a keyboard tab stop, while URL citations retain current behavior. Add a regression test covering the missing-src case and verify strict TypeScript and accessibility behavior.Source: Path instructions
frontends/ui/src/shared/components/Sources/source-utils.ts (1)
43-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the changed source and citation transformations.
The adjacent specs cover helper happy paths and normal paragraph citations, but not these changed branches:
frontends/ui/src/shared/components/Sources/source-utils.ts#L43-L58: test that HTTP/HTTPS URLs are retained and non-web references omiturl.frontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.ts#L17-L20: test that[1]remains literal text inside inline/fenced code while normal text still creates<cite>elements.As per path instructions, changed user-visible workflows require tests.
🤖 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 `@frontends/ui/src/shared/components/Sources/source-utils.ts` around lines 43 - 58, Add regression tests for mapCitationSource in frontends/ui/src/shared/components/Sources/source-utils.ts: verify HTTP/HTTPS sources retain url while non-web references leave url undefined. Also add tests for the citation transformation in frontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.ts, confirming [1] remains literal inside inline and fenced code while normal text still produces cite elements.Source: Path instructions
frontends/ui/src/features/chat/hooks/use-websocket-chat.ts (1)
849-876: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTreat terminal NAT statuses as terminal even without a “Function Complete” name.
Line 856 only finalizes an existing step when
parseFunctionName()setsisComplete. A frame withstatus === 'error'or'complete'and a non-completion name remains active; a newly created error step similarly storesisComplete: false.Derive one terminal flag from both signals and invoke
failThinkingStep/completeThinkingStepfor newly created terminal steps as well. Add coverage for an error-status frame whose name is notFunction Complete.As per path instructions, review UI changes for “resilient loading and error states” and “report/chat state consistency.”
As per coding guidelines, validate withnpm run lint,npm run type-check, andnpm run test:ci, and include a screenshot for visible changes.🤖 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 `@frontends/ui/src/features/chat/hooks/use-websocket-chat.ts` around lines 849 - 876, Derive a terminal flag in the websocket frame handling flow from either isComplete or status === 'error'/'complete', then use it for existing-step finalization and the created step’s isComplete value. Ensure newly created terminal steps immediately invoke failThinkingStep or completeThinkingStep after addThinkingStep, while non-terminal steps retain the running behavior. Add coverage for an error-status frame with a non-“Function Complete” function name.Sources: Coding guidelines, Path instructions
frontends/ui/src/shared/components/Sources/SourceStrip.tsx (1)
74-83: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep the disclosure control mounted while expanded.
Because the button is rendered only under
!expanded, it disappears immediately after activation. Consequently, assistive technology never observesaria-expanded="true"and focus is removed from the activated control. Keep a stable toggle, associate it with the source container viaaria-controls, and support collapse or explicitly move focus to the revealed content.As per path instructions, review UI changes for “accessible controls.”
🤖 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 `@frontends/ui/src/shared/components/Sources/SourceStrip.tsx` around lines 74 - 83, Update the disclosure control in SourceStrip so it remains mounted in both collapsed and expanded states, with aria-expanded reflecting expanded and aria-controls referencing the source container. Change its action and label to support collapsing after expansion, and ensure focus remains on the stable toggle or is intentionally moved to the revealed content.Source: Path instructions
🤖 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.
Inline comments:
In `@frontends/ui/src/features/chat/lib/intermediate-step-parser.ts`:
- Around line 99-104: Update the name handling in the intermediate-step
classification logic before the tool/function prefix checks: trim surrounding
whitespace and normalize case, then use that normalized value to exclude
Function and Tool: prefixes before slash-based or model-name classification. Add
coverage in intermediate-step-parser.spec.ts for lowercase tool:web/search and
whitespace-prefixed Tool: web/search inputs, ensuring both are identified as
tools rather than LLMs.
In `@frontends/ui/src/features/chat/store.ts`:
- Around line 2408-2419: Update findInConversation in the deep-research message
lookup to search messages in reverse chronological order, selecting the latest
matching snapshot for the job rather than the earliest. Preserve the existing
job ID and trace-state predicates, and keep the current-conversation precedence
over ownConversations.
In `@frontends/ui/src/features/layout/store.ts`:
- Around line 36-37: Update the authentication transition handling to reset the
layout store’s promptDraft and selectedModel alongside the existing chat-state
reset during logout and account switches. Add coverage for user switching that
verifies both composer fields return to their initial values.
In `@frontends/ui/src/features/layout/types.ts`:
- Around line 91-94: Update the setSelectedModel contract in the relevant layout
types to accept string | undefined, allowing callers to clear the override and
restore backend-default behavior. Update its implementation and add a test
covering setting a model, then resetting it with undefined.
In
`@frontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.spec.tsx`:
- Around line 19-24: Add coverage in the CollapsibleBlock tests for the closed
state’s aria-controls attribute, verifying it references the controlled body
element, and update the static/non-collapsible test to render open={false} while
asserting the body remains visible. Preserve the existing assertions for the
collapsible closed behavior.
In `@frontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsx`:
- Line 69: The CollapsibleBlock component must keep the element referenced by
the trigger’s aria-controls mounted when collapsed. Preserve a stable wrapper
carrying id={regionId} in the open/closed render path, and apply the collapse
animation or conditional content rendering inside that wrapper rather than
removing the target element.
In `@frontends/ui/src/shared/components/Sources/SourceList.spec.tsx`:
- Around line 37-42: Update the equality-branch fixture in the “renders a
clickable link for a web source even when the label equals the domain” test to
use the normalized domain returned by prettyDomain(url), such as “nvidia.com”,
so it exercises the no-domain branch rather than showDomain.
---
Outside diff comments:
In `@frontends/ui/src/features/chat/hooks/use-websocket-chat.ts`:
- Around line 849-876: Derive a terminal flag in the websocket frame handling
flow from either isComplete or status === 'error'/'complete', then use it for
existing-step finalization and the created step’s isComplete value. Ensure newly
created terminal steps immediately invoke failThinkingStep or
completeThinkingStep after addThinkingStep, while non-terminal steps retain the
running behavior. Add coverage for an error-status frame with a non-“Function
Complete” function name.
In `@frontends/ui/src/shared/components/MarkdownRenderer/Citation.tsx`:
- Around line 33-43: Update the Citation component’s anchor tabIndex logic so
unresolved citations without src are not focusable; only citations with a valid
src but no URL should receive a keyboard tab stop, while URL citations retain
current behavior. Add a regression test covering the missing-src case and verify
strict TypeScript and accessibility behavior.
In `@frontends/ui/src/shared/components/Sources/source-utils.ts`:
- Around line 43-58: Add regression tests for mapCitationSource in
frontends/ui/src/shared/components/Sources/source-utils.ts: verify HTTP/HTTPS
sources retain url while non-web references leave url undefined. Also add tests
for the citation transformation in
frontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.ts,
confirming [1] remains literal inside inline and fenced code while normal text
still produces cite elements.
In `@frontends/ui/src/shared/components/Sources/SourceStrip.tsx`:
- Around line 74-83: Update the disclosure control in SourceStrip so it remains
mounted in both collapsed and expanded states, with aria-expanded reflecting
expanded and aria-controls referencing the source container. Change its action
and label to support collapsing after expansion, and ensure focus remains on the
stable toggle or is intentionally moved to the revealed content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 35198851-8741-4820-982f-d671aa154fe5
📒 Files selected for processing (23)
frontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/features/layout/types.tsfrontends/ui/src/shared/components/Actions/CopyButton.tsxfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.spec.tsxfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/shared/components/MarkdownRenderer/Citation.tsxfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.tsfrontends/ui/src/shared/components/Sources/SourceList.spec.tsxfrontends/ui/src/shared/components/Sources/SourceList.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/shared/components/Sources/source-utils.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.tsfrontends/ui/src/shared/components/MarkdownRenderer/Citation.tsxfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/shared/components/Sources/SourceList.tsxfrontends/ui/src/shared/components/Sources/SourceList.spec.tsxfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/shared/components/Actions/CopyButton.tsxfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/features/chat/types.tsfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.spec.tsxfrontends/ui/src/shared/components/Sources/source-utils.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/types.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.ts
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.tsfrontends/ui/src/shared/components/MarkdownRenderer/Citation.tsxfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/shared/components/Sources/SourceList.tsxfrontends/ui/src/shared/components/Sources/SourceList.spec.tsxfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/shared/components/Actions/CopyButton.tsxfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/features/chat/types.tsfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.spec.tsxfrontends/ui/src/shared/components/Sources/source-utils.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/types.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.ts
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/rehype-citations.tsfrontends/ui/src/shared/components/MarkdownRenderer/Citation.tsxfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/shared/components/Sources/SourceList.tsxfrontends/ui/src/shared/components/Sources/SourceList.spec.tsxfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/shared/components/Actions/CopyButton.tsxfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/features/chat/types.tsfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.spec.tsxfrontends/ui/src/shared/components/Sources/source-utils.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/types.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.ts
🔇 Additional comments (23)
frontends/ui/src/features/layout/store.ts (1)
58-99: 📐 Maintainability & Code QualityProvide the required UI validation evidence.
Please attach results for
npm run lint,npm run type-check,npm run test:ci, andnpm run buildfor this UI-state change.As per coding guidelines, “Run npm lint, type-check, and build validation for UI changes in frontends/ui.”
Source: Coding guidelines
frontends/ui/src/shared/components/MarkdownRenderer/Citation.tsx (2)
33-43: 🎯 Functional CorrectnessAssociate the tooltip with its citation trigger.
The previous accessibility finding remains unresolved: the
role="tooltip"element is not referenced by the citation anchor, so assistive technologies receive only the shortaria-label, not the tooltip’s title/snippet.As per path instructions, “Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls, resilient loading and error states, and report/chat state consistency.”
Also applies to: 56-58
Source: Path instructions
26-72: 📐 Maintainability & Code QualityRun the UI checks and attach citation-state screenshots. Execute
npm run lint,npm run type-check,npm run test:ci, andnpm run buildinfrontends/ui, and include screenshots covering the citation hover/focus states.frontends/ui/src/features/layout/data-sources.ts (1)
11-12: LGTM!Also applies to: 47-75, 77-109, 111-124
frontends/ui/src/features/layout/data-sources.spec.ts (1)
1-44: LGTM!frontends/ui/src/adapters/api/websocket-client.ts (1)
231-239: LGTM!frontends/ui/src/features/chat/hooks/use-websocket-chat.ts (1)
58-62: LGTM!Also applies to: 109-109, 419-419, 595-609, 1102-1102, 1224-1224, 1250-1250
frontends/ui/src/features/chat/lib/intermediate-step-parser.spec.ts (1)
1-118: LGTM!frontends/ui/src/features/chat/lib/deep-research-trace.ts (2)
32-50: The previously reported explanation-tool retry collapse issue remains.The inserted explanation step still prevents repeated explanation-tool calls from being adjacent, so
collapseRepeatscannot merge them.
1-30: LGTM!Also applies to: 52-136
frontends/ui/src/features/chat/lib/deep-research-trace.spec.ts (2)
104-115: Covered by the existingcollapseRepeatsfinding indeep-research-trace.ts.
1-103: LGTM!Also applies to: 117-160
frontends/ui/src/features/chat/store.spec.ts (2)
2285-2347: Covered by the latest-snapshot hydration finding instore.ts.
586-605: LGTM!Also applies to: 1279-1355
frontends/ui/src/features/chat/store.ts (2)
750-781: LGTM!
1281-1400: LGTM!frontends/ui/src/features/chat/types.ts (1)
64-64: LGTM!Also applies to: 185-226, 470-470, 502-503, 642-643
frontends/ui/src/shared/components/Actions/CopyButton.tsx (1)
6-6: LGTM!Also applies to: 24-42
frontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsx (1)
17-17: LGTM!Also applies to: 21-35, 46-46
frontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.spec.tsx (2)
1-53: 📐 Maintainability & Code QualityProvide the required UI validation evidence.
Please confirm
npm run lint,npm run type-check,npm run test:ci, and build validation forfrontends/ui, and attach a screenshot for these visible UI changes.As per coding guidelines, UI changes require lint, type-check, build validation, tests, and screenshots for visible changes.
Sources: Coding guidelines, Path instructions
1-18: LGTM!Also applies to: 26-42
frontends/ui/src/shared/components/Sources/SourceList.tsx (2)
40-60: Announce external navigation to assistive technology.Both links open a new tab but expose no accessible hint. Add visually hidden “opens in a new tab” text to both anchors and cover the behavior in
SourceList.spec.tsx. This remains the same unresolved accessibility issue from the prior review.As per path instructions, review UI changes for “accessible controls.”
Source: Path instructions
22-68: 📐 Maintainability & Code QualityRun the frontend checks and attach a screenshot for the source list update.
KyleZheng1284
added
enhancement
Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
Additive state and streaming plumbing that a later UI PR consumes; no visual change and no prod capability removed. - chat types: optional ThinkingStep fields (completedAt, status, argSummary), ChatMessage.selectedModel, and a plan_approval prompt type. - intermediate-step-parser: generic folded reasoning/reflection/explanation sentinels and predicates, splitPayload/extractFoldedOutput, and label lookup via the shared tool-label engine. - deep-research-trace: replays a stored deep run into the same ThinkingStep stream so reloaded reports show an identical trace. - store + use-websocket-chat: populate the new step fields as the run streams, fail steps on error, and thread the selected model through send. - layout store: a sidebar collapse model added alongside the existing panel API, a prompt draft, a selected-model slot, and data-source display helpers. Per-user auth, WebSocket token rotation, artifact handling, and HITL are preserved unchanged. No predictive/analytical/warehouse plumbing is ported. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
- deep-research-trace: preserve tool calls whose referenced agent is missing (dangling agentId), not just calls with no agent, and add a regression test. - store: scope deep-research hydration to the current user's conversations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
Signed-off-by: Manush Maheshwari <manushm@nvidia.com>
6abdf1a to
f2d8a3e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsx (1)
66-72: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid rendering an enabled inert toggle.
When
collapsibledefaults totrueandonOpenChangeis omitted, the component renders a button whose click handler cannot changeopen. Require the callback for collapsible usage, or render a static/disabled header when no callback is supplied.As per path instructions, “Review UI changes for ... accessible controls.”
🤖 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 `@frontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsx` around lines 66 - 72, Update CollapsibleBlock’s collapsible rendering around the onOpenChange handler so it does not expose an enabled toggle when the callback is absent. Require onOpenChange before rendering the interactive button, otherwise render a static or disabled header while preserving the existing collapsible behavior when the callback is provided.Source: Path instructions
frontends/ui/src/shared/components/Mermaid/MermaidBlock.tsx (1)
135-143: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve native scrolling at zoom limits.
preventDefault()runs before the clamp check. At the minimum or maximum scale,next === prev, but the wheel event has already been canceled, so users cannot scroll normally over the diagram. MovepreventDefault()after the boundary check.Proposed fix
const onWheel = (e: WheelEvent): void => { - e.preventDefault() + const prev = scaleRef.current + const next = clamp(prev * Math.exp(-e.deltaY * 0.0015)) + if (next === prev) return + + e.preventDefault() const rect = el.getBoundingClientRect() const cx = e.clientX - rect.left const cy = e.clientY - rect.top - const prev = scaleRef.current - const next = clamp(prev * Math.exp(-e.deltaY * 0.0015)) - if (next === prev) return anchorRef.current = { ratio: next / prev, contentX: el.scrollLeft + cx, contentY: el.scrollTop + cy, cx, cy }🤖 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 `@frontends/ui/src/shared/components/Mermaid/MermaidBlock.tsx` around lines 135 - 143, Move e.preventDefault() in onWheel to after the next === prev early return, so wheel events at the zoom limits retain native scrolling while zooming events are still canceled.
🤖 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.
Inline comments:
In `@frontends/ui/src/features/chat/lib/deep-research-trace.ts`:
- Around line 35-38: Update the sameCall helper to stop comparing the lossy
argSummary display value; use canonical raw inputs or an explicit retry identity
to distinguish calls with different arguments while preserving the existing
top-level and function-name checks. Add coverage proving distinct inputs with
identical or undefined summaries remain separate, and verify report/chat state
remains consistent.
In `@frontends/ui/src/features/chat/store.ts`:
- Around line 2420-2423: Update the preferred conversation lookup near
ownConversations so it only calls findInConversation when currentUserId is
present and currentConversation.userId matches currentUserId; otherwise return
false. Keep the existing user-filtered ownConversations fallback unchanged.
In `@frontends/ui/src/shared/components/Sources/SourceStrip.tsx`:
- Around line 62-64: Guard previewCount in the SourceStrip component before the
hasMore, visible, and hiddenCount calculations, normalizing it to a positive
integer or enforcing that contract so previewCount - 1 is always valid. Preserve
consistent expanded/collapsed state and add an edge-case test covering
previewCount values at or below zero.
---
Outside diff comments:
In `@frontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsx`:
- Around line 66-72: Update CollapsibleBlock’s collapsible rendering around the
onOpenChange handler so it does not expose an enabled toggle when the callback
is absent. Require onOpenChange before rendering the interactive button,
otherwise render a static or disabled header while preserving the existing
collapsible behavior when the callback is provided.
In `@frontends/ui/src/shared/components/Mermaid/MermaidBlock.tsx`:
- Around line 135-143: Move e.preventDefault() in onWheel to after the next ===
prev early return, so wheel events at the zoom limits retain native scrolling
while zooming events are still canceled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 1cda19ba-f6a6-474a-881a-58c2e18f8cd7
📒 Files selected for processing (22)
frontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/app/globals.cssfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/features/layout/types.tsfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsxfrontends/ui/src/shared/components/Mermaid/MermaidBlock.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.spec.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/shared/components/Sources/parse-references.spec.tsfrontends/ui/src/shared/lib/motion.spec.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/shared/components/Sources/SourceStrip.spec.tsxfrontends/ui/src/shared/lib/motion.spec.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/shared/components/Sources/parse-references.spec.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsxfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/shared/components/Mermaid/MermaidBlock.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.tsfrontends/ui/src/features/layout/types.ts
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/shared/components/Sources/SourceStrip.spec.tsxfrontends/ui/src/shared/lib/motion.spec.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/shared/components/Sources/parse-references.spec.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsxfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/shared/components/Mermaid/MermaidBlock.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.tsfrontends/ui/src/features/layout/types.ts
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/shared/components/Sources/SourceStrip.spec.tsxfrontends/ui/src/shared/lib/motion.spec.tsxfrontends/ui/src/shared/components/Sources/SourceStrip.tsxfrontends/ui/src/features/layout/data-sources.spec.tsfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/shared/components/Sources/parse-references.spec.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.spec.tsfrontends/ui/src/features/layout/store.spec.tsfrontends/ui/src/features/chat/lib/deep-research-trace.spec.tsfrontends/ui/src/app/globals.cssfrontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsxfrontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsxfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/features/chat/store.spec.tsfrontends/ui/src/features/layout/store.tsfrontends/ui/src/shared/components/Mermaid/MermaidBlock.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/chat/lib/deep-research-trace.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/ui/src/features/chat/lib/intermediate-step-parser.tsfrontends/ui/src/features/layout/types.ts
🔇 Additional comments (25)
frontends/ui/src/shared/components/CollapsibleBlock/CollapsibleBlock.tsx (1)
71-83: Keep thearia-controlstarget mounted while collapsed.The trigger always references
regionId, but the element with that ID is removed when a collapsible block closes. Keep a stable wrapper withid={regionId}and animate or conditionally render its contents inside it. This repeats the unresolved prior review finding.As per path instructions, “Review UI changes for ... accessible controls.”
Source: Path instructions
frontends/ui/src/app/globals.css (1)
360-360: LGTM!Also applies to: 384-384, 395-395, 437-439
frontends/ui/src/shared/lib/motion.spec.tsx (1)
4-45: LGTM!frontends/ui/src/shared/components/Mermaid/MermaidBlock.tsx (3)
183-187: Fullscreen focus management remains incomplete.The new dialog semantics do not move focus into the fullscreen overlay or restore it on exit, leaving keyboard users focused outside the active modal. This repeats the unresolved prior finding; add focus capture/restoration and a regression test.
Source: Path instructions
40-134: LGTM!Also applies to: 144-179
194-228: 📐 Maintainability & Code QualityProvide the required frontend validation evidence.
Run
npm run lint,npm run type-check,npm run test:ci, and build validation forfrontends/ui. The supplied context reports lint, type-check, and tests, but not build output.As per coding guidelines, frontend UI changes require lint, type-check, test, and build validation.
Source: Coding guidelines
frontends/ui/src/features/layout/data-sources.ts (1)
11-12: LGTM!Also applies to: 47-75, 77-109, 111-124
frontends/ui/src/shared/components/Sources/parse-references.spec.ts (1)
1-13: LGTM!Also applies to: 14-43, 45-70, 72-98, 100-121, 123-168, 170-203
frontends/ui/src/features/layout/data-sources.spec.ts (1)
1-44: LGTM!frontends/ui/src/shared/components/Mermaid/MermaidBlock.spec.tsx (1)
84-85: LGTM!Also applies to: 89-89
frontends/ui/src/shared/components/Sources/SourceStrip.tsx (1)
58-61: LGTM!Also applies to: 66-89
frontends/ui/src/shared/components/Sources/SourceStrip.spec.tsx (1)
40-52: LGTM!frontends/ui/src/adapters/api/websocket-client.ts (1)
231-239: 📐 Maintainability & Code QualityConfirm the required UI build validation.
The PR reports lint, type-check, and tests, but not the required build check. Please confirm
npm run buildpasses infrontends/ui.As per coding guidelines, “Run npm lint, type-check, and build validation for UI changes in frontends/ui.” <coding_guidelines>
Source: Coding guidelines
frontends/ui/src/features/chat/hooks/use-websocket-chat.ts (1)
58-62: LGTM!Also applies to: 103-110, 419-419, 595-609, 844-860, 875-876, 1102-1102, 1224-1224, 1250-1250
frontends/ui/src/features/chat/lib/intermediate-step-parser.ts (1)
14-15: LGTM!Also applies to: 99-106, 127-150, 182-237
frontends/ui/src/features/chat/lib/intermediate-step-parser.spec.ts (1)
1-126: LGTM!frontends/ui/src/features/layout/types.ts (1)
28-34: LGTM!Also applies to: 53-56, 73-76, 91-96
frontends/ui/src/features/layout/store.ts (1)
25-26: LGTM!Also applies to: 36-37, 58-99, 126-134
frontends/ui/src/features/chat/lib/deep-research-trace.ts (2)
4-34: LGTM!Also applies to: 39-150
1-2: 📐 Maintainability & Code QualityRun the UI build check.
lint,type-check, andtest:cidon’t cover the Next.js production build; addnpm --prefix frontends/ui run buildto the required UI validation.frontends/ui/src/features/chat/store.ts (1)
453-464: LGTM!Also applies to: 759-785, 1290-1318, 1346-1404, 2410-2419, 2424-2447
frontends/ui/src/features/chat/types.ts (1)
64-64: LGTM!Also applies to: 185-186, 219-226, 470-470, 502-503, 642-643
frontends/ui/src/features/chat/lib/deep-research-trace.spec.ts (1)
1-192: LGTM!frontends/ui/src/features/chat/store.spec.ts (1)
14-14: LGTM!Also applies to: 45-45, 184-207, 612-631, 1305-1381, 2311-2373
frontends/ui/src/features/layout/store.spec.ts (1)
21-22: LGTM!Also applies to: 32-33, 139-221
2 participants
What this does
PR 2 of the UI-only chat port. Adds the chat state and streaming plumbing that the visual PR will read. No user-visible change and no predictive/analytical/warehouse features.
ThinkingStepfields (completedAt,status,argSummary),ChatMessage.selectedModel, and aplan_approvalprompt type.intermediate-step-parser: generic folded reasoning / reflection / explanation sentinels and predicates,splitPayload/extractFoldedOutput, and label lookup via the shared tool-label engine.deep-research-trace(new): replays a stored deep run into the sameThinkingStepstream so a reloaded report shows an identical trace.store+use-websocket-chat: populate the new step fields as the run streams, fail steps on error, and thread the selected model through send.Preserved (no regression)
Per-user MCP OAuth, WebSocket token rotation, artifact/file handling, and HITL are all untouched. Every existing store/hook/parser test still passes.
Testing
Full frontend suite green (1479 tests, +59 new), including the pre-existing token-rotation and OAuth specs.
type-checkandlintclean.Screenshots
This PR is streaming/state plumbing with no UI of its own; it drives the live phased thinking trace (durations, status, response timer) that #333's components render:
🤖 Generated with Claude Code
Summary by CodeRabbit