feat(ux): stabilize frontend rehydration and fix chat persistence - #51
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideRefactors dashboard state usage around the analysis store/nucleus, hardens dimension rendering to avoid hollow dashboards, wires a new export dropdown into the console top bar, stabilizes chat persistence/reset behavior, and polishes global UI details like scrollbars, z‑index layering, and prompt output cleanup. Sequence diagram for chat reset on analysis or URL changesequenceDiagram
actor User
participant DashboardContainer
participant useChatStore
User->>DashboardContainer: setUrl(newUrl) / startAnalysis
DashboardContainer->>DashboardContainer: useEffect([nucleus.analysis.id, url])
alt nucleus.analysis.id exists or url exists
DashboardContainer->>useChatStore: getState().reset()
useChatStore->>useChatStore: reset state (conversations=[], activeId=null, messagesByConv={})
end
Sequence diagram for export dropdown actions in TopBarsequenceDiagram
actor User
participant TopBar
participant DashboardContainer
participant Browser
User->>TopBar: click Export button
TopBar->>TopBar: setExportOpen(true)
User->>TopBar: select Export as PDF
TopBar-->>DashboardContainer: onExport("pdf")
DashboardContainer->>DashboardContainer: handleExport("pdf")
DashboardContainer->>Browser: window.open(/api/analyses/{id}/export?format=pdf&scope=full)
User->>TopBar: select Export as Markdown
TopBar-->>DashboardContainer: onExport("markdown")
DashboardContainer->>DashboardContainer: handleExport("markdown")
DashboardContainer->>Browser: URL.createObjectURL(blob)
DashboardContainer->>Browser: a.click() to download .md file
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughPR adds PDF/Markdown export to the dashboard TopBar via a new callback and dropdown UI, introduces CSS z-index tokens for consistent UI layering, rewires DashboardContainer to use Zustand stores, updates dimension and pane rendering logic to reflect analysis state, and refines UCIS output by tightening prompts and stripping completion tags. ChangesDashboard export, stacking, and output quality
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
|
❌ Pipeline Status: All checks passed. Ready to merge. |
![sourcery-ai[bot]](https://avatars.github.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In TopBar.tsx,
dropdownItemis typed asReact.CSSPropertiesbutReactisn’t imported as a namespace, which will break type-checking; consider importingtype CSSPropertiesfromreactand using that instead. - In ChatDock, setting
zIndex: 'var(--z-dock)' as anyrelies on a type escape hatch and passes a string where a number is expected; it would be more robust to move this to a CSS class that uses the custom property for z-index. - The chat reset effect in DashboardContainer now runs whenever
urlchanges, which means typing or editing the URL field will clear the chat; if the intent is to reset only when a new analysis starts or navigation actually occurs, consider keying the effect offnucleus.analysis?.idor the router location instead of the raw input value.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In TopBar.tsx, `dropdownItem` is typed as `React.CSSProperties` but `React` isn’t imported as a namespace, which will break type-checking; consider importing `type CSSProperties` from `react` and using that instead.
- In ChatDock, setting `zIndex: 'var(--z-dock)' as any` relies on a type escape hatch and passes a string where a number is expected; it would be more robust to move this to a CSS class that uses the custom property for z-index.
- The chat reset effect in DashboardContainer now runs whenever `url` changes, which means typing or editing the URL field will clear the chat; if the intent is to reset only when a new analysis starts or navigation actually occurs, consider keying the effect off `nucleus.analysis?.id` or the router location instead of the raw input value.
## Individual Comments
### Comment 1
<location path="web/components/templates/console/TopBar.tsx" line_range="15-20" />
<code_context>
}
-export function TopBar({ search, onSearchChange, onSearchSubmit, tier, account }: TopBarProps) {
+export function TopBar({ search, onSearchChange, onSearchSubmit, onExport, tier, account }: TopBarProps) {
+ const [exportOpen, setExportOpen] = useState(false);
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid rendering the export control when no export handler is provided.
The Export control currently renders even when `onExport` is undefined, making it look clickable but doing nothing. Please render this block only when `onExport` is provided, or at least disable the button when it isn’t.
Suggested implementation:
```typescript
import { ReactNode, useState } from 'react';
import { Icon } from '@/components/templates/_shared/primitives';
export interface TopBarProps {
search: string;
onSearchChange: (value: string) => void;
onSearchSubmit?: () => void;
onExport?: (format: 'pdf' | 'markdown') => void;
tier?: string;
account?: ReactNode;
}
```
To fully implement "Avoid rendering the export control when no export handler is provided", you should adjust the JSX where the Export UI is rendered inside `TopBar`:
1. Ensure the `TopBar` component signature includes `onExport` (and `account` if used in JSX):
```ts
export function TopBar({ search, onSearchChange, onSearchSubmit, onExport, tier, account }: TopBarProps) {
```
2. Wrap the export trigger UI so it only renders when `onExport` is provided, e.g.:
```tsx
{onExport && (
<Button onClick={() => setExportOpen(true)} /* ...rest */>
Export
</Button>
)}
```
Alternatively, if you must always render the button for layout reasons, at least disable it:
```tsx
<Button
onClick={() => onExport && setExportOpen(true)}
disabled={!onExport}
/* ...rest */
>
Export
</Button>
```
3. If there is an export modal/popover that calls `onExport`, keep that guarded as well:
```tsx
const handleExport = (format: 'pdf' | 'markdown') => {
if (!onExport) return;
onExport(format);
setExportOpen(false);
};
```
These changes ensure the export control is not presented as interactable when `onExport` is `undefined`, matching your review comment.
</issue_to_address>
### Comment 2
<location path="web/components/templates/console/ChatDock.tsx" line_range="100" />
<code_context>
right: 0,
bottom: 0,
- zIndex: 50,
+ zIndex: 'var(--z-dock)' as any,
borderTop: '1px solid var(--line)',
background: 'rgb(11 14 20 / 0.97)',
</code_context>
<issue_to_address>
**suggestion:** Using `as any` for `zIndex` suggests the styling might be better handled via CSS classes.
Casting to `any` to support a CSS variable weakens type safety and can hide future issues. Since `--z-dock` already lives in `globals.css`, prefer a dedicated class (e.g. `.hx-chatdock`) that sets `z-index: var(--z-dock);` and keep the inline `zIndex` as a typed numeric value.
Suggested implementation:
```typescript
left: 0,
right: 0,
bottom: 0,
zIndex: 50,
borderTop: '1px solid var(--line)',
background: 'rgb(11 14 20 / 0.97)',
backdropFilter: 'blur(12px)',
```
To fully align with your comment about preferring a CSS class that uses `var(--z-dock)`:
1. In `globals.css`, add a class such as:
```css
.hx-chatdock {
z-index: var(--z-dock);
}
```
2. In `ChatDock.tsx`, ensure the root container of the dock has `className="hx-chatdock"` (or merged via something like `clsx` if it already has classes).
You may then choose to:
- Either rely purely on the class for `z-index` (remove the inline `zIndex`), **or**
- Keep the inline numeric `zIndex` as a fallback and let the class override it via CSS specificity if needed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+15
to
20
| export function TopBar({ search, onSearchChange, onSearchSubmit, onExport, tier, account }: TopBarProps) { | ||
| const [exportOpen, setExportOpen] = useState(false); | ||
|
|
||
| return ( | ||
| <div style={{ display: "flex", alignItems: "center", gap: 12, padding: "12px 24px" }}> | ||
| <label |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid rendering the export control when no export handler is provided.
The Export control currently renders even when onExport is undefined, making it look clickable but doing nothing. Please render this block only when onExport is provided, or at least disable the button when it isn’t.
Suggested implementation:
import { ReactNode, useState } from 'react';
import { Icon } from '@/components/templates/_shared/primitives';
export interface TopBarProps {
search: string;
onSearchChange: (value: string) => void;
onSearchSubmit?: () => void;
onExport?: (format: 'pdf' | 'markdown') => void;
tier?: string;
account?: ReactNode;
}To fully implement "Avoid rendering the export control when no export handler is provided", you should adjust the JSX where the Export UI is rendered inside TopBar:
- Ensure the
TopBarcomponent signature includesonExport(andaccountif used in JSX):export function TopBar({ search, onSearchChange, onSearchSubmit, onExport, tier, account }: TopBarProps) {
- Wrap the export trigger UI so it only renders when
onExportis provided, e.g.:Alternatively, if you must always render the button for layout reasons, at least disable it:{onExport && ( <Button onClick={() => setExportOpen(true)} /* ...rest */> Export </Button> )}
<Button onClick={() => onExport && setExportOpen(true)} disabled={!onExport} /* ...rest */ > Export </Button>
- If there is an export modal/popover that calls
onExport, keep that guarded as well:const handleExport = (format: 'pdf' | 'markdown') => { if (!onExport) return; onExport(format); setExportOpen(false); };
These changes ensure the export control is not presented as interactable when onExport is undefined, matching your review comment.
| right: 0, | ||
| bottom: 0, | ||
| zIndex: 50, | ||
| zIndex: 'var(--z-dock)' as any, |
There was a problem hiding this comment.
suggestion: Using as any for zIndex suggests the styling might be better handled via CSS classes.
Casting to any to support a CSS variable weakens type safety and can hide future issues. Since --z-dock already lives in globals.css, prefer a dedicated class (e.g. .hx-chatdock) that sets z-index: var(--z-dock); and keep the inline zIndex as a typed numeric value.
Suggested implementation:
left: 0,
right: 0,
bottom: 0,
zIndex: 50,
borderTop: '1px solid var(--line)',
background: 'rgb(11 14 20 / 0.97)',
backdropFilter: 'blur(12px)',To fully align with your comment about preferring a CSS class that uses var(--z-dock):
- In
globals.css, add a class such as:.hx-chatdock { z-index: var(--z-dock); }
- In
ChatDock.tsx, ensure the root container of the dock hasclassName="hx-chatdock"(or merged via something likeclsxif it already has classes).
You may then choose to:
- Either rely purely on the class for
z-index(remove the inlinezIndex), or - Keep the inline numeric
zIndexas a fallback and let the class override it via CSS specificity if needed.
TechHypeXP
added a commit
that referenced
this pull request
Jun 5, 2026
* fix(ci): un-wedge type-check — collapse redundant ternary + dedupe @types/react Two errors broke the Type Check gate on main (introduced via #51), stranding the #51 UX work undeployed (Build/Deploy were skipped): 1. DashboardContainer.tsx:277 (TS2367): the block is already inside `{store.status !== 'idle' && (...)}`, so the inner `!== 'idle'` ternary was provably-always-true. Collapsed to its truthy branch (dead `: null` removed). 2. Duplicate @types/react (Link/ForceGraph2D "cannot be used as a JSX component"): a transitive dep with a loose peer ('@types/react': '>=18.0.0') floated 19.2.15 alongside the pinned 19.0.0 → two incompatible ReactNode defs. Added a pnpm `overrides` block pinning @types/react + @types/react-dom to 19.0.0. NOTE: pnpm 11.5.1 no longer reads `pnpm.overrides` from package.json — it lives in pnpm-workspace.yaml. Verified: web tsc --noEmit = 0 errors, eslint = 0. No major version bumps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): address CodeRabbit review — align @types/react to 19.2.x + state-aware synthesis placeholder Clears the two CodeRabbit comments on #52: 1. @types/react / @types/react-dom: 19.0.0 → ^19.2.0 (resolves to a single 19.2.16) — aligns types with the React 19.2.6 runtime (covers useEffectEvent, <Activity>, etc.) AND still collapses the duplicate that caused the JSX-component errors. Verified: lockfile resolves ONE @types/react version; web tsc 0. 2. DashboardContainer synthesis placeholder is now state-aware: 'complete' with zero dimensions → "No synthesis dimensions were produced"; 'error' → failure note; otherwise the spinner + "Preparing…". Mirrors the graph placeholder pattern. Gates: web tsc 0, eslint 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Kelly Bakri <noreply@github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
TechHypeXP
added a commit
that referenced
this pull request
Jul 31, 2026
Co-authored-by: Kelly Bakri <noreply@github.com>
TechHypeXP
pushed a commit
that referenced
this pull request
Jul 31, 2026
…ypes/react Two errors broke the Type Check gate on main (introduced via #51), stranding the #51 UX work undeployed (Build/Deploy were skipped): 1. DashboardContainer.tsx:277 (TS2367): the block is already inside `{store.status !== 'idle' && (...)}`, so the inner `!== 'idle'` ternary was provably-always-true. Collapsed to its truthy branch (dead `: null` removed). 2. Duplicate @types/react (Link/ForceGraph2D "cannot be used as a JSX component"): a transitive dep with a loose peer ('@types/react': '>=18.0.0') floated 19.2.15 alongside the pinned 19.0.0 → two incompatible ReactNode defs. Added a pnpm `overrides` block pinning @types/react + @types/react-dom to 19.0.0. NOTE: pnpm 11.5.1 no longer reads `pnpm.overrides` from package.json — it lives in pnpm-workspace.yaml. Verified: web tsc --noEmit = 0 errors, eslint = 0. No major version bumps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TechHypeXP
added a commit
that referenced
this pull request
Jul 31, 2026
* fix(ci): un-wedge type-check — collapse redundant ternary + dedupe @types/react Two errors broke the Type Check gate on main (introduced via #51), stranding the #51 UX work undeployed (Build/Deploy were skipped): 1. DashboardContainer.tsx:277 (TS2367): the block is already inside `{store.status !== 'idle' && (...)}`, so the inner `!== 'idle'` ternary was provably-always-true. Collapsed to its truthy branch (dead `: null` removed). 2. Duplicate @types/react (Link/ForceGraph2D "cannot be used as a JSX component"): a transitive dep with a loose peer ('@types/react': '>=18.0.0') floated 19.2.15 alongside the pinned 19.0.0 → two incompatible ReactNode defs. Added a pnpm `overrides` block pinning @types/react + @types/react-dom to 19.0.0. NOTE: pnpm 11.5.1 no longer reads `pnpm.overrides` from package.json — it lives in pnpm-workspace.yaml. Verified: web tsc --noEmit = 0 errors, eslint = 0. No major version bumps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): address CodeRabbit review — align @types/react to 19.2.x + state-aware synthesis placeholder Clears the two CodeRabbit comments on #52: 1. @types/react / @types/react-dom: 19.0.0 → ^19.2.0 (resolves to a single 19.2.16) — aligns types with the React 19.2.6 runtime (covers useEffectEvent, <Activity>, etc.) AND still collapses the duplicate that caused the JSX-component errors. Verified: lockfile resolves ONE @types/react version; web tsc 0. 2. DashboardContainer synthesis placeholder is now state-aware: 'complete' with zero dimensions → "No synthesis dimensions were produced"; 'error' → failure note; otherwise the spinner + "Preparing…". Mirrors the graph placeholder pattern. Gates: web tsc 0, eslint 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Kelly Bakri <noreply@github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses 'partial rehydration' and 'hollow dashboard' issues:
useChatStore.tsand triggered it on URL change inDashboardContainer.tsx.Summary by Sourcery
Improve dashboard robustness, chat session handling, and export capabilities for analysis outputs.
New Features:
Bug Fixes:
Enhancements:
Summary by cubic
Stabilizes frontend rehydration and fixes sticky chat sessions by resetting chat state on new analyses and URL changes. Improves streaming UX with safer rendering, export controls, and small visual polish.
Bug Fixes
--z-dockso the Chat dock sits above the Processing Log.New Features
/api/analyses/{id}/export?format=pdf&scope=fulland Markdown download.Written for commit bedb686. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI Improvements