feat(web): port identity/intelligence pages from platform#31271
Conversation
Port the Identity tab and its supporting components, libraries, and utilities from vellum-assistant-platform into the OSS web app. New domains/intelligence/ directory includes: - Identity tab with avatar management, constellation view, skills browser - Constellation visualization (layout engine, node rendering, viewport) - Skills tab with category sidebar, skill detail, install/remove - Apps components (library view, app/document viewer, Vercel deploy) - Memory v1/v2 API wrappers and formatting utilities - File markdown renderer with YAML frontmatter stripping Also adds: - Avatar customization panel and management modal components - uploadAvatarImage/deleteAvatar functions in avatar API - useDoubleClick hook - /assistant/identity route wired in routes.tsx Conventions applied: kebab-case files, .js import extensions, @/ aliases, removed platform-only eslint-disable directives. LUM-1654 Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
🤖 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:
|
| onSelect={() => setSelectedSkillId(skill.id)} | ||
| onInstall={() => handleInstall(skill)} | ||
| onRemove={() => handleRemove(skill)} | ||
| isInstalling={installingSkillId === skill.id} |
There was a problem hiding this comment.
🟡 isInstalling spinner never shows when skill.slug differs from skill.id
handleInstall passes skill.slug ?? skill.id to the mutation (line 169), and onMutate stores that value in installingSkillId (line 151). However, the isInstalling prop comparison on the list view (line 302) and detail view (line 237) both compare against skill.id instead of skill.slug ?? skill.id. When a skill has a slug that differs from its id, the installing spinner will never appear because installingSkillId (set to the slug) will never equal skill.id. The sibling identity-tab.tsx:309 correctly uses installingSkillId === (selectedSkill.slug ?? selectedSkill.id), confirming this is an oversight.
| isInstalling={installingSkillId === skill.id} | |
| isInstalling={installingSkillId === (skill.slug ?? skill.id)} |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in e8162b4 — now compares against skill.slug ?? skill.id in both the detail view (line 237) and list view (line 302). This was a pre-existing bug in the platform code that got ported over. Resolved.
| onBack={() => setSelectedSkillId(null)} | ||
| onInstall={() => handleInstall(selectedSkill)} | ||
| onRemove={() => handleRemove(selectedSkill)} | ||
| isInstalling={installingSkillId === selectedSkill.id} |
There was a problem hiding this comment.
🟡 isInstalling spinner never shows in detail view when skill.slug differs from skill.id
Same root cause as the list view: installingSkillId is set to skill.slug ?? skill.id via onMutate at skills-tab.tsx:151, but the detail-view comparison at line 237 uses selectedSkill.id. When the skill has a slug, the comparison will always be false and the installing indicator won't render.
| isInstalling={installingSkillId === selectedSkill.id} | |
| isInstalling={installingSkillId === (selectedSkill.slug ?? selectedSkill.id)} |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in e8162b4 — same fix as the list view. Both comparisons now use skill.slug ?? skill.id. Resolved.
| export async function deleteAvatar( | ||
| assistantId: string, | ||
| ): Promise<boolean> { | ||
| try { | ||
| const { error, response } = await client.post({ | ||
| url: "/v1/assistants/{assistant_id}/workspace/delete/", | ||
| path: { assistant_id: assistantId }, | ||
| body: { path: "data/avatar" }, | ||
| headers: { "Content-Type": "application/json" }, | ||
| }); | ||
| assertHasResponse(response, error, "Failed to delete avatar"); | ||
| return response.ok; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🚩 deleteAvatar is exported but never used
The deleteAvatar function added at apps/web/src/domains/avatar/api.ts:57-72 is not imported or called anywhere in the codebase. CONVENTIONS.md and STYLE_GUIDE.md emphasize removing dead code. This may be intentional (a utility for future use), but it's worth confirming whether it's needed in this PR or should be deferred.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Removed deleteAvatar in e8162b4. It was also unused in the platform repo — dead code that never got wired up. uploadAvatarImage (which IS used by avatar-management-modal.tsx) is kept. Resolved.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65b3c855e7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| const installMutation = useMutation({ | ||
| mutationFn: (slug: string) => installSkill(assistantId, slug), | ||
| onMutate: (slug) => setInstallingSkillId(slug), |
There was a problem hiding this comment.
Track install state with the same key used by skill rows
installingSkillId is set from the mutation argument (slug) but later compared against skill.id when rendering SkillRow/SkillDetail. For catalog skills where slug !== id, install-in-progress state never matches, so the Install button stays enabled and users can trigger repeated install requests while no spinner/disabled state is shown. Use a consistent identifier on both sides (for example, always skill.slug ?? skill.id) to prevent duplicate submissions and show correct progress UI.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e8162b4 — the isInstalling comparison now uses skill.slug ?? skill.id on both the list view and detail view, matching the value passed to installMutation.mutate(). Resolved.
… unused deleteAvatar - Fix isInstalling spinner: compare against skill.slug ?? skill.id (not just skill.id) to match the value passed to installMutation, so the spinner appears correctly for skills with slugs different from their ids. - Remove unused deleteAvatar export from avatar/api.ts. LUM-1654 Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
Test ResultsRan shell-based build verification against the PR branch. All compile-time checks pass. Results (6/6 passed)
Not tested (requires daemon)
These require a full assistant runtime ( |
…base conventions Move skills/, memories/, memory-v2/, and client.ts from domains/intelligence/lib/ to the domain root. Update all imports. Remove empty hooks/ directory. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
- Replace React.* namespace types with destructured imports (ReactNode, ChangeEvent, MouseEvent) in node-shell, library-view, avatar-management-modal - Flatten constellation-view/hooks/ subdirectory — move hooks to component level matching codebase convention Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
There was a problem hiding this comment.
✦ APPROVE
Value: Ports the Identity/Intelligence pages into the OSS repo, making the assistant's personality visualization, skills browser, memory viewer, and avatar management available in the Vite web app alongside the native macOS/iOS clients.
What this does: Creates domains/intelligence/ with the identity tab, constellation view, skills tab, memory viewers (v1 + v2), and avatar management — all ported from platform and adapted to OSS conventions. Adds a single /identity route.
Architecture — clean. intelligence/client.ts as the single re-export point for HeyAPI client + error utilities is a nice convention. All API modules (skills/api.ts, memories/api.ts, memory-v2/api.ts) consistently use assertHasResponse + throw new ApiError(response.status, ...) — correct per codebase standards.
isInstalling comparison bug (Devin P2) — verified fixed. e8162b4d now compares against skill.slug ?? skill.id in both list view (line 302) and detail view (line 237), matching the value passed to installMutation.mutate(slug). Correct.
deleteAvatar dead code — verified removed.
useEffect — clean. Only one effect in skills-tab.tsx: a debounce timer with proper clearTimeout cleanup. All server state is in React Query. No async effects, no stale closure risk.
No Zustand misuse. This domain correctly uses React Query for server state and useState for local UI state — no Zustand needed and none introduced.
One non-blocking note: --ghost-hover is used on two non-Button elements in skills-tab.tsx:
- Line 464: the filter dropdown trigger (
<button aria-haspopup="listbox">) - Line 545: listbox option rows inside the popover
Per our semantic token conventions, --ghost-hover is for ghost Button hover states only; menus, list rows, and popovers should use --surface-hover. These were ported as-is from the platform — worth a follow-up polish pass but not a blocker.
Reviewed at 78681b08 · CI: ✅ Lint/Type/Build, Socket Security
Vellum Constitution — Yours: the skills browser and constellation view give users a tangible window into their assistant's capabilities, deepening the sense that this is genuinely theirs.
|
Good catch on the |
Prompt / plan
Port the Identity/Intelligence pages UI from
vellum-assistant-platforminto the OSS repo, preserving behavior and adapting to OSS conventions.Closes LUM-1654
What changed
Creates
domains/intelligence/with all ported components and supporting modules:Identity tab (
components/identity-tab.tsx): Main identity page showing assistant name, role, personality, hatched date with inline editing. Integrates avatar management, constellation visualization, and skills browser.Constellation visualization (
components/constellation-view/): Interactive graph layout engine rendering skills as orbital nodes around a central identity. Includes viewport controls (zoom, pan), node popovers, edge rendering, and responsive legend.Skills browser (
components/skills/): Full skills management UI with category sidebar filtering, skill detail view with README rendering, install/remove mutations, and search.Apps/library viewer (
components/apps/): App library grid with pinned/recent sections, search, import/export, Vercel deploy flow, app viewer container, and document viewer.Domain modules (at domain root, following codebase conventions — no
lib/subdirectory):skills/— Skill types, category classification, daemon API wrappersmemories/— Memory v1 types, formatting, kind classification, APImemory-v2/— Memory v2 types, API, TanStack Query hooksclient.ts— Daemon HTTP client helperShared utilities:
hooks/use-double-click.ts— Double-click detection hookcomponents/file-markdown.tsx— Markdown renderer with YAML frontmatter strippingcomponents/mobile-sidebar-drawer.tsx— Slide-out drawer for mobile layoutsAvatar management (additions to existing domain):
components/avatar/avatar-customization-panel.tsx— Character trait buildercomponents/avatar/avatar-management-modal.tsx— Modal with build/upload flowsdomains/avatar/api.ts— AddeduploadAvatarImage()Route:
/assistant/identitywired inroutes.tsxunderChatLayout.Bug fix during port
Fixed a pre-existing bug from the platform:
isInstallingspinner comparison inskills-tab.tsxusedskill.idbut the mutation storesskill.slug ?? skill.id— when slug differs from id, the spinner never showed. Now both sides useskill.slug ?? skill.id.Conventions applied
.jsimport extensions (NodeNext resolution)@/path aliaseslib/subdirectory) —skills/,memories/,memory-v2/,client.tseslint-disabledirectives (no-restricted-syntax)"use client"directivesAssistant.createdfield name)deleteAvatarexport (dead code in platform too)Test plan
bunx tsc --noEmitpasses with 0 errorsbun run lintpasses with 0 errorsbun run buildpassesLink to Devin session: https://app.devin.ai/sessions/c4696ace57fe43649f2475d7661ac273
Requested by: @ashleeradka