feat(web): port Home page from platform repo, wire up React Query + HeyAPI client#31067
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const mountedRef = useRef(true); | ||
|
|
||
| // Blink logic (paused during streaming) | ||
| useEffect(() => { | ||
| if (reduce || isStreaming) return; | ||
| mountedRef.current = true; | ||
|
|
||
| function scheduleBlink() { | ||
| blinkTimerRef.current = setTimeout(() => { | ||
| if (!mountedRef.current) return; | ||
| setIsBlinking(true); | ||
| setTimeout(() => { | ||
| if (!mountedRef.current) return; | ||
| setIsBlinking(false); | ||
| if (Math.random() < 0.2) { | ||
| setTimeout(() => { | ||
| if (!mountedRef.current) return; | ||
| setIsBlinking(true); | ||
| setTimeout(() => { | ||
| if (!mountedRef.current) return; | ||
| setIsBlinking(false); | ||
| scheduleBlink(); | ||
| }, 150); | ||
| }, 200); | ||
| } else { | ||
| scheduleBlink(); | ||
| } | ||
| }, 150); | ||
| }, randomBetween(3000, 7000)); | ||
| } | ||
|
|
||
| scheduleBlink(); | ||
|
|
||
| return () => { | ||
| mountedRef.current = false; | ||
| if (blinkTimerRef.current) clearTimeout(blinkTimerRef.current); | ||
| }; | ||
| }, [reduce, isStreaming]); |
There was a problem hiding this comment.
🟡 Shared mountedRef between blink and twitch effects allows orphaned inner timeouts to create duplicate animation chains
The blink effect (lines 164-198) and twitch effect (lines 200-225) share a single mountedRef and each set mountedRef.current = true on setup. The cleanup functions only clear the outermost timeout stored in blinkTimerRef/twitchTimerRef, but the inner setTimeout calls at lines 172, 176, 179 (blink) and 211 (twitch) are never tracked or cancelled.
When isStreaming toggles from false → true → false quickly (e.g., a stream errors out immediately), the cleanup phase sets mountedRef.current = false, but then the re-setup phase sets mountedRef.current = true before the orphaned inner timers fire. Those orphaned timers then see mountedRef.current === true, proceed to call setIsBlinking/setTwitchAngle and re-invoke scheduleBlink()/scheduleTwitch(), creating a second parallel animation chain. Each rapid toggle can accumulate additional chains, causing increasingly erratic animation behavior.
Prompt for agents
The blink and twitch effects in AnimatedAvatar share a single mountedRef (line 161), and each effect sets mountedRef.current = true during setup (lines 166, 203). The cleanup functions only clear the outermost timeout stored in blinkTimerRef/twitchTimerRef, but the inner setTimeout calls (lines 172, 176, 179 for blink; line 211 for twitch) are not tracked.
When effects re-run (e.g., isStreaming toggles), orphaned inner timers from the previous run see mountedRef.current = true (set by the new setup) and proceed, creating duplicate animation chains.
Fix approaches:
1. Use an AbortController or a per-effect-instance cancellation token (a separate ref or a local boolean closed over by each effect invocation) instead of the shared mountedRef. For example, use a local `let cancelled = false` inside each effect, set `cancelled = true` in cleanup, and check `cancelled` in all inner timeouts.
2. Store ALL inner timeout IDs in an array ref and clear them all during cleanup.
3. Use separate mounted refs for each effect instead of sharing one.
Approach 1 (local cancelled flag) is cleanest because it naturally scopes the cancellation to each effect invocation without shared mutable state.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 3818dec — replaced the shared mountedRef with a local let cancelled = false flag in each effect closure (Approach 1 from the prompt). Each effect now has its own cancellation scope, so rapid isStreaming toggles can't create orphaned animation chains.
The fix also removes the now-unused blinkTimerRef and twitchTimerRef refs — the outermost timer is returned from scheduleBlink()/scheduleTwitch() and cleared directly in the cleanup function.
![vex-assistant-bot[bot]](https://avatars.githubusercontent.com/in/3399303?s=60&v=4){kind=small}
There was a problem hiding this comment.
✦ APPROVE
Value: Ports the Home page domain from vellum-assistant-platform to the OSS repo, making it the first real feature page consuming the Vite + React Router v7 scaffold. Wires QueryClientProvider + HeyAPI client singleton, adds the avatar system (AnimatedAvatar, ChatAvatar, SVG compositor), and implements the complete home feed UI (greeting header, suggestion pills, feed list, detail panel with resizable split-view). Represents significant progress on LUM-1543 / LUM-1601. Reviewed at 3818dec2.
What this does:
- Full Home page domain under
apps/web/src/domains/home/: page shell, feed list, filter bar, recap rows, suggestion pills, detail panel (generic + tool-permission card) - Avatar system:
AnimatedAvatarwith streaming morph, breathing, blink, twitch, and reduced-motion support + SVG compositor +ChatAvatarwrapper +useAssistantAvatarhook ResizablePanelcomponent using Pointer Events API withsetPointerCapture, localStorage persistence, and proper ARIA separator semantics- React Query hooks:
useHomeFeedQuery(optimistic updates for status/dismiss/action, visibilitychange time-away tracking) anduseHomeStateQuery - HeyAPI
clientsingleton +QueryClientProviderinmain.tsx - Route wiring:
/homeadded toroutes.tsxwith theHomePagecomponent
Key observations:
- The animated avatar is the most complex component in this PR and it's well-isolated. 306 lines for SVG path wobble, breathing keyframe animation, blink scheduling (3-7s random intervals with 20% double-blink probability), twitch rotation (8-15s random intervals), and morph-path cycling during streaming (16 precomputed variants at 150ms intervals). All animations pause when
isStreamingorprefers-reduced-motionis active. - Devin's identified bug was real and is fixed. The initial implementation used a shared
mountedRefbetween the blink and twitch effects. Since each effect setsmountedRef.current = trueindependently, a rapidisStreamingtoggle could leave orphaned innersetTimeoutchains from a previously-unmounted effect cycle. The fix (3818dec2) replaces the shared ref with a locallet cancelled = falseper effect closure — each effect has its own cancellation scope, eliminating the leak. Good fix. - React Query optimistic updates are implemented correctly.
updateStatusandtriggerActionmutations use the full optimistic update pattern:cancelQueries→getQueryData→setQueryData→onErrorrollback →onSettledinvalidation. TheupdateStatushandler additionally filters dismissed items from the list optimistically. use-home-feed-query.tsdoes not fire-and-forget theinvalidateQueriescall inside thevisibilitychangehandler. Thevoidprefix explicitly acknowledges the promise is intentionally unhandled (it's a fire-and-not-await background refresh). This is acceptable for event handlers where you don't want to block the UI on the invalidation completing. The anti-pattern would be an async function without any error handling — but React Query catches its own errors internally.useIsMobileusesuseSyncExternalStore— the correct modern API for media query subscriptions in React 18+, avoids hydration mismatch issues withuseEffect+useState.ResizablePanelfollows web standards properly. UsesonPointerDown/Move/Up(not mouse events) withsetPointerCapture/releasePointerCapturefor reliable cross-device drag tracking. UseslocalStorage(not cookies) for width persistence. Properrole="separator"+aria-orientation="vertical".
Issue: Hard window.location.href redirects in routes.tsx instead of React Router navigation.
The HomePage route handler in routes.tsx uses window.location.href = "/assistant", window.location.href = "/assistant/conversations/${conversationId}", and window.location.href = "/assistant?prompt=..." for all three navigation callbacks (onStartNewChat, onOpenConversation, onSuggestionSelected). This is a full page reload in a React Router SPA. It's the correct behavior when the target routes exist in a separate app (the current setup), but if these routes are meant to be handled by the same apps/web router, it would be wrong.
Since this is the first real page port and the chat/conversation routes are presumably still in separate apps, this is likely intentional temporary wiring. But if the long-term plan is a unified SPA, these should become useNavigate calls. No change needed now — but flagging it since the web-move architecture discussion explicitly targets a unified app.
assertHasResponsehelper follows the HeyAPI fetch client pattern. However, all four API functions (fetchHomeFeed,fetchRelationshipState,updateFeedItemStatus,triggerFeedAction) throw genericErrorobjects on failure instead of enriched error types. The PR addslib/api-errors.tswithassertHasResponse, but doesn't yet use it in the home domain API. Minor — can be a follow-up.routes.tsxpassesassistantId="default"as a hardcoded string toHomePage. Presumably this will come from context/auth state in a future iteration when the chat/auth integration lands. Fine for a port.
Anti-patterns check: None found. No .frame() variants in LazyVStack (not Swift). No Context+useReducer for cross-component state. No barrel files beyond convention. No god components (each piece is properly decomposed).
Vellum Constitution — Distinct: porting the Home page is where the web app stops being scaffold and starts being app — this PR is that transition point.
devin-ai-integration
Bot
dismissed
vex-assistant-bot[bot]’s stale review
via
ea87889
|
Confirmed — |
Local Testing ResultsTested locally at All 3 tests passed.
ScreenshotsHome page empty state ( After clicking "New Chat" — SPA navigated to Direct navigation to Test methodology
|
…eyAPI client Part of LUM-1601 - Wire up QueryClientProvider in main.tsx with configured QueryClient - Add HeyAPI client singleton (src/lib/api-client.ts) for daemon endpoints - Add API error utilities (src/lib/api-errors.ts) - Port avatar system: types, API, SVG compositor, useAssistantAvatar hook, AnimatedAvatar + ChatAvatar components - Port Home domain: types, API layer, feed utils, useHomeFeedQuery + useHomeStateQuery hooks, all page components (greeting header, suggestion pills, feed list, filter bar, recap rows, detail panel) - Port shared components: ResizablePanel, useIsMobile hook - Enhance design library Button: add outlined variant, compact size, leftIcon/rightIcon/iconOnly props with proper Tailwind styling - Add /home route in routes.tsx - All files follow new repo conventions: kebab-case filenames, no 'use client' directives, no Next.js imports, .js extension imports Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
- Add preserveSymlinks to tsconfig.json and vite.config.ts so TypeScript resolves design library imports from the symlink path (apps/web/node_modules/) instead of the real path (packages/design-library/), ensuring a single copy of @types/react is used across both packages. - Add design library runtime deps (@radix-ui/react-slot, class-variance-authority, clsx, tailwind-merge) to apps/web dependencies since bun does not install file: dependency transitive deps in the linked package's node_modules. - Fix SVG path number regex in animated-avatar.tsx to handle compact SVG notation (numbers starting with . like .5 meaning 0.5). Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
When the design library's devDependencies are installed (e.g. for Storybook), its node_modules/react shadows the web app's copy, causing dual-type errors. The postinstall removes the design library's react copies so TypeScript resolves from the web app's single copy via preserveSymlinks. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
…nimatedAvatar Each blink/twitch useEffect now uses a local `let cancelled = false` flag instead of a shared mountedRef. This prevents orphaned inner timeouts from creating duplicate animation chains when isStreaming toggles rapidly — the previous shared ref could be reset to true by a new effect setup while orphaned callbacks from the old effect's cleanup phase were still pending. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
…in home routes Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
…l copy Replaces the local apps/web/src/components/resizable-panel.tsx with the design library version at @vellum/design-library/components/resizable-panel (landed in PR #31073). Deletes the now-redundant local file. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
devin-ai-integration
Bot
force-pushed
the
devin/1779142403-port-home-page-v2
branch
from
ea87889 to
cf265be
Compare
There was a problem hiding this comment.
✦ APPROVE
Value: Ports the Home page from vellum-assistant-platform to the OSS web app, making it the first real feature consuming the Vite + React Router v7 scaffold. Wires React Query + HeyAPI client singleton, adds the avatar system (AnimatedAvatar, ChatAvatar, SVG compositor), and implements the full home feed UI with optimistic updates and resizable detail panel. Significant progress on LUM-1543 / LUM-1601. Reviewed at cf265bed.
What this does:
- Full Home page domain under
apps/web/src/domains/home/: page shell, greeting header, suggestion pills, feed list, recap rows, detail panel (generic + tool-permission card) - Avatar system:
AnimatedAvatarwith streaming morph, breathing, blink, twitch, and reduced-motion support + SVG compositor +ChatAvatarwrapper +useAssistantAvatarhook - React Query hooks:
useHomeFeedQuery(optimistic updates for status/dismiss/action, visibilitychange time-away tracking) anduseHomeStateQuery - HeyAPI
clientsingleton +QueryClientProviderinmain.tsx - Route wired at
/homeviaHomePageRoutewithuseNavigatecallbacks
Commits since prior review (3818dec2):
b33a4e56— Devin's identifiedmountedRefbug fixed: per-effectlet cancelled = falsereplaces shared ref, eliminating orphaned timeout chains on rapidisStreamingtogglesb9b63760—window.location.hreffull-page reloads replaced withuseNavigateSPA navigation (addresses prior review concern)cf265bed—ResizablePanelimported from@vellum/design-libraryinstead of inline implementation
Anti-patterns check: Clean. No new barrel files. No Context+useReducer for cross-component state. No god components. All useEffect hooks have cleanup. useIsMobile correctly uses useSyncExternalStore with module-scope subscribe/getSnapshot. Optimistic update pattern follows the cancel→snapshot→mutate→rollback→invalidate sequence.
Non-blocking notes:
home/api.tsthrows plainErroron failure rather than a typedApiError. Callers can't discriminate 401 vs 500. The PR addslib/api-errors.tswithassertHasResponsebut doesn't yet use it in the home domain — fine to wire up when auth/runtime integration lands in a follow-up PR.api-client.tscreates a HeyAPI client withbaseUrl: "". The home feed endpoints are absolute paths (/v1/assistants/...), so this relies on the serving origin or a future proxy config. Expected given the PR's scope — backend wiring is a separate task.useIsMobileomitsgetServerSnapshot. Safe in a Vite SPA (no SSR), but if SSR is ever introduced, this will hydration-mismatch.
Merge gate: Vex approved. Codex gave 👍 reaction. Devin reviewed at 3818dec2 (found the mountedRef bug, which is now fixed). A fresh Devin re-review at current HEAD would be ideal since two additional commits landed after Devin's last pass.
Vellum Constitution — Distinct: this PR is the transition point where the web app stops being scaffold and starts being app.
1 participant
Prompt / plan
Port the Home page domain from
vellum-assistant-platformto this repo, making it convention-compliant. This also completes the runtime half of LUM-1601 by wiring upQueryClientProviderand the HeyAPI client singleton — with the Home page as the first real consumer.Replaces #31061 (closed due to stale remote branch causing pre-push hook issues).
What's included:
apps/web/src/domains/home/— page, feed list, filter bar, suggestion pills, recap rows, detail panel (generic + tool permission card)apps/web/src/domains/avatar/(SVG compositor, animated avatar, chat avatar, hooks) +apps/web/src/components/avatar/QueryClientProviderin app root, client config insrc/lib/api-client.ts, query client insrc/lib/query-client.tsResizablePanel(draggable split-pane with localStorage),useIsMobilehook/assistant/homematching the platform's current routingConvention compliance applied during migration:
"use client"directives (not needed in Vite SPA)useRouterfromnext/navigation).jsextensions to all imports (NodeNext module resolution)domains/structure per repo conventionsCross-package type resolution fix:
preserveSymlinks: trueto bothtsconfig.jsonandvite.config.tsto prevent dual@types/reactcopies when the design library is linked viafile:dependency@radix-ui/react-slot,class-variance-authority,clsx,tailwind-merge) toapps/web/package.jsonsince bun'sfile:dependencies don't install transitive depspostinstallscript to deduplicate react types for local devSVG path regex fix (discovered during migration):
animated-avatar.tsxto handle compact SVG notation (.5meaning0.5) — original regex/-?\d+\.?\d*/missed numbers starting with.AnimatedAvatar bug fix (identified by Devin Review):
mountedRefbetween blink and twitch effects with per-effectlet cancelled = falseflags. The shared ref allowed orphaned inner timeouts to create duplicate animation chains whenisStreamingtoggled rapidly — cleanup setmountedRef.current = false, but re-setup immediately set it back totruebefore orphaned timers fired. Each effect now has its own cancellation scope.Part of LUM-1601
Test plan
bun run typecheck— passes (zero errors)bun run lint— passesbun run build— passes/assistant/homewith correct empty-state UI (heading, "New Chat" button, "No items to show" text). No fatal console errors (only expected API 404s from missing backend).Link to Devin session: https://app.devin.ai/sessions/536ceadb023a4059908b0609b9833bc1
Requested by: @ashleeradka