feat(chat): add multi-model response panels#9855
feat(chat): add multi-model response panels#9855nmgarza5 wants to merge 5 commits intomulti-model-4a-selectorfrom
Conversation
Greptile SummaryThis PR introduces the response panel UI for multi-model chat, adding Key findings:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
MMV["MultiModelResponseView"] -->|"responses[]"| GM["Generation Mode\n(side-by-side, overflow-x-auto)"]
MMV -->|"preferredIndex set\n& not generating\n& ≥2 visible panels"| SM["Selection Mode\n(carousel + translateX)"]
GM --> MMP1["MultiModelPanel A\n(full width)"]
GM --> MMP2["MultiModelPanel B\n(full width)"]
GM --> MMP3["MultiModelPanel C (hidden)\n(HIDDEN_PANEL_W strip)"]
SM --> PREF["Preferred Panel\n(dynamicPrefW, centered)"]
SM --> NONPREF["Non-preferred Panels\n(SELECTION_PANEL_W, peeking,\nheight-capped + 50% opacity)"]
SM --> HIDDENSM["Hidden Panels\n(HIDDEN_PANEL_W)"]
PREF --> AM1["AgentMessage\n(hideFooter=false)"]
NONPREF --> AM2["AgentMessage\n(hideFooter=true,\npointer-events-none)"]
MMP1 -->|"click (non-preferred)"| SEL["handleSelectPreferred\n→ setPreferredIndex\n→ onMessageSelection(nodeId)"]
MMP1 -.->|"⚠ click (preferred)\nmissing isPreferred guard"| SEL
Prompt To Fix All With AIThis is a comment left during a code review.
Path: web/src/app/app/message/MultiModelPanel.tsx
Line: 60-62
Comment:
**Preferred panel click re-fires `onMessageSelection`**
`handlePanelClick` only guards against `isHidden` — it does not check `isPreferred`. The component's own JSDoc (line 42) explicitly states "Clicking anywhere on a visible **non-preferred** panel marks it as preferred." When a preferred panel is clicked, `onSelect()` is invoked → `handleSelectPreferred` in `MultiModelResponseView` → `onMessageSelection(response.nodeId)` fires again for the already-selected node.
Depending on what the parent's `onMessageSelection` does (e.g. switching a conversation branch, recording analytics), this can produce unexpected side effects on an innocuous click. The `cursor-pointer` class on line 112 also persists on preferred panels, giving users no visual indication that clicking is inert.
```suggestion
const handlePanelClick = useCallback(() => {
if (!isHidden && !isPreferred) onSelect();
}, [isHidden, isPreferred, onSelect]);
```
And condition `cursor-pointer` on line 112 accordingly:
```tsx
className={cn(
"flex flex-col gap-3 min-w-0 rounded-16 transition-colors",
!isPreferred && "cursor-pointer hover:bg-background-tint-02"
)}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: web/src/app/app/message/MultiModelResponseView.tsx
Line: 295-308
Comment:
**Inline ref callback disconnects ResizeObserver every render**
The `ref={(el) => { ... }}` on line 295 creates a new function identity on every render. React treats a changed ref callback by calling the old one with `null` (teardown) then the new one with the element (setup). This means on every render:
1. `preferredPanelRef(null)` is called for the preferred panel → `preferredRoRef.current.disconnect()` and `setPreferredPanelHeight(null)` are issued.
2. `preferredPanelRef(el)` is called immediately after → a brand-new `ResizeObserver` is created and `setPreferredPanelHeight(el.offsetHeight)` is re-measured.
While React 18 batching should prevent a visible flicker from the `null` → measured height transition, the ResizeObserver is still torn down and rebuilt on every render (including the rapid re-renders driven by `setOverflowingPanels` and `setPreferredPanelHeight` themselves). Consider extracting the per-panel ref into a stable `useCallback` or a `useRef`-based imperative handle to avoid this churn.
How can I resolve this? If you propose a fix, please make it concise.Reviews (10): Last reviewed commit: "fix(chat): conditional fade overlay and ..." | Re-trigger Greptile |
There was a problem hiding this comment.
2 issues found across 4 files
Confidence score: 3/5
- Potential user-facing mismatch:
MultiModelResponseView.tsxindexesresponseswithmodelIndexeven though it’s treated as an identifier elsewhere, which can select the wrong response; consider usingfind. - Risk is moderate rather than critical because the issue is localized but can surface as incorrect response display for some models.
- Pay close attention to
web/src/app/app/message/MultiModelResponseView.tsxandweb/src/app/app/message/MultiModelPanel.tsx- response selection logic and unusedmodelIndexprop.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/src/app/app/message/MultiModelResponseView.tsx">
<violation number="1" location="web/src/app/app/message/MultiModelResponseView.tsx:142">
P2: `modelIndex` is treated as an identifier elsewhere (via `findIndex`), so indexing the array with it can select the wrong response. Use `find` instead of `responses[modelIndex]`.</violation>
</file>
<file name="web/src/app/app/message/MultiModelPanel.tsx">
<violation number="1" location="web/src/app/app/message/MultiModelPanel.tsx:16">
P2: `modelIndex` is added to the component API but never used, which creates a muddy interface and dead code. Remove it from the props contract unless it is actually needed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
7c40642 to
f53a644
Compare
|
Preview Deployment
|
f53a644 to
61cf5be
Compare
f7c1f8b to
2d3c838
Compare
0dc5718 to
ab90cf1
Compare
b2f2e4b to
2799ead
Compare
MultiModelResponseView renders 2-3 LLM responses side-by-side with generation and selection modes. MultiModelPanel wraps AgentMessage for each model's output. Adds hideFooter prop to AgentMessage for non-preferred panel styling.
translateX(calc(50% - ...)) used 50% of the track's own width, not the container's. Use measured trackContainerW for correct centering.
- Use responses.find() instead of responses[modelIndex] array indexing - Guard preferredIdx === -1 to prevent NaN in carousel transform - Remove unused SvgEyeClosed import - Remove unused isHighlighted field from MultiModelResponse - Remove unused modelIndex prop from MultiModelPanel
- Only show bottom gradient on non-preferred panels that actually overflow the preferred panel height cap - Always dim non-preferred panels at 50% opacity (not just when capped) - Add preferredIdx !== -1 guard and responses.find() for safer lookups
2799ead to
c11d5fb
Compare
Description
Adds the response panel UI for multi-model chat — displays side-by-side model responses in generation and selection modes.
MultiModelResponseView.tsx— orchestrates panel layout with generation/selection mode switchingMultiModelPanel.tsx— individual model response panel (rendersAgentMessagewith model header)interfaces.ts—MultiModelResponsetype definitionAgentMessage.tsx— addshideFooterprop to suppress footer in multi-model panelsStacked on #9854 (PR4a: selector + hook).
How Has This Been Tested?
Additional Options