fix(web): iOS prechat safe-area + back-nav persistence (LUM-1763)#31416
Conversation
![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: iOS users can no longer get silently dropped back to the name step after a hot-reload or OS memory reclaim mid-onboarding, and the back-button row no longer clips under the Dynamic Island — two high-friction first-run moments fixed in one shot.
What this does: Five commits bundled around the prechat/onboarding surface: (1) sessionStorage-based screen persistence in PreChatFlow so native iOS users restore to the correct step on reload; (2) safe-area inset fix on both onboarding screens; (3) native retire redirect to /prechat instead of /privacy on iOS; (4) conditional traits-fetch in useAssistantAvatar to avoid 404s on every SSE invalidation when a custom image exists; (5) refactors AssistantIdentity + fetchAssistantIdentity out of chat/api/assistant.ts into src/assistant/identity.ts as a cleaner cross-domain boundary.
pre-chat-flow.tsx — back-nav persistence ✅
Lazy useState initializer restores screen position from sessionStorage, all three write/clear sites are wrapped in try/catch for private-mode safety, and clears on both "back from vibe" and finishNativePreChat. No edge cases missed. One minor thing: onContinue and onSkip both repeat the same setScreen(1) + sessionStorage.setItem block — a small const goToVibeStep = () => { ... } closure inside screen === 0 arm would de-dup that. Non-blocking.
name-step-screen.tsx + vibe-step-screen.tsx — safe-area ✅
calc(var(--safe-area-inset-top, env(safe-area-inset-top, 0px)) + 1rem) is the right pattern: Capacitor injects --safe-area-inset-top as a CSS variable, the env() fallback covers plain WKWebView, and the 0px fallback keeps desktop at 1rem. Both screens are consistent.
retire-assistant.tsx — native re-onboarding redirect ✅
isNativePlatform() gate is correct — skipping the privacy step for iOS re-onboarding is the right call since consent was already captured at first install.
use-assistant-avatar.ts — conditional traits fetch ✅
Sequential await fetchCharacterTraits(id) gated on imageUrl is intentional and correct inside the React Query queryFn. The test file (use-assistant-avatar.test.tsx) covers both the skip-traits-on-custom-image and fetch-traits-on-no-image paths cleanly.
src/assistant/identity.ts — domain boundary refactor ✅
Moving AssistantIdentity + fetchAssistantIdentity out of the chat domain is the right call — identity is an assistant-level property, not a chat concern. All 5 import sites updated correctly. .cross-domain-allowlist.json updated to remove "chat" from identity-tab.tsx. Clean.
home-markdown-content.tsx — markdown renderer ✅
Design-token styling throughout (--content-secondary, --content-link, --border-base, color-mix). Minor note: no pre override, so fenced code blocks fall back to default react-markdown rendering — probably fine for feed content, but worth a follow-up if multi-line code blocks start appearing in feed items.
Also: the a override renders with target="_blank". On iOS WKWebView without a WKUIDelegate createWebViewWith implementation, target="_blank" links silently do nothing (WKWebView won't navigate to a new "tab"). If the feed can surface external URLs, worth checking that the native client handles them — or swapping to target="_self" on native. Non-blocking for now.
CI — 7/7 green (Lint, Type Check, Build, Test, Socket × 2). ✅
Vellum Constitution — Trust-seeking: users who close the app mid-onboarding return to exactly where they left off, and the UI never slides under the system chrome.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const [components, imageUrl] = await Promise.all([ | ||
| fetchCharacterComponents(id), | ||
| fetchCharacterTraits(id), | ||
| fetchAvatarImageUrl(id), | ||
| ]); | ||
| // Skip the traits fetch when a custom image exists — the traits | ||
| // file is intentionally deleted on the daemon side in that case, | ||
| // so requesting it just generates 404s on every SSE-driven | ||
| // reconnect invalidation. `AvatarRenderer` only reads `traits` | ||
| // when there is no `customImageUrl`. | ||
| const traits = imageUrl ? null : await fetchCharacterTraits(id); |
There was a problem hiding this comment.
🚩 Sequential traits fetch adds latency for the common case (no custom avatar)
The previous implementation fetched components, traits, and imageUrl all in parallel via Promise.all. The new implementation fetches components and imageUrl in parallel, then fetches traits sequentially only if imageUrl is falsy (apps/web/src/domains/avatar/use-assistant-avatar.ts:47). This means for the majority of users who don't have a custom avatar image, the traits fetch now waits for fetchAvatarImageUrl to resolve before starting — adding one network round-trip of latency to avatar loading. The trade-off (avoiding guaranteed 404s when a custom image exists) is documented in the comment, but reviewers should consider whether the common-case latency regression is acceptable or whether a parallel fetch with post-hoc discard would be preferable.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Thanks — flagging this as outdated. The avatar fetch change isn't actually in this PR's diff anymore; it shipped via #31407 (merged) where this same trade-off was accepted by reviewers. Quick recap of the reasoning in case it helps anyone reading this thread later:
- The traits fetch returns a guaranteed 404 when a custom image exists (the daemon deletes the traits file in that branch), and SSE reconnects invalidate
SYNC_TAGS.assistantAvatarrepeatedly — so the platform was generating a fresh 404 on every reconnect. AvatarRendereronly readstraitswhen there's no custom image, so the fetch was pure waste in the custom-image branch.- The extra RTT only applies in the no-custom-image path, which is where traits is actually needed — and
staleTime: Infinityon the query means this runs only on real reconnects, not on every render.
If the latency turns out to matter in practice we can revisit with a parallel-fetch-with-discard pattern; happy to follow up if telemetry says it does.
Generated by Claude Code
Two non-blocking observations from the vex-bot review on #31412 and #31416 (both files surfaced the same gaps): 1. **Missing element overrides** — react-markdown was falling back to browser defaults for tables and fenced code blocks. Added overrides for `pre`, `table`, `thead`, `tbody`, `tr`, `th`, `td` styled with the home feed's design tokens. Tables wrap in a horizontal-scroll container so wide tables don't blow out the condensed panel layout. 2. **target="_blank" on iOS WKWebView silently fails** — without a `WKUIDelegate createWebViewWith` implementation, the webview won't open a new tab. Route through Capacitor's `openUrl()` helper (already used elsewhere for OAuth/Stripe flows) which presents `SFSafariViewController`. Web keeps the default new-tab behavior; the `href` is preserved on both platforms so right-click → copy link still works. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2
Non-blocking observation from the vex-bot review on #31416: `onContinue` and `onSkip` on the iOS NameStepScreen had the same 4-line body (setScreen + sessionStorage.setItem in try/catch), which invites drift if one transition path changes and the other doesn't. Extract a `goToVibeStep` closure inside the screen===0 arm so both callsites point at the same code. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0c5f4ec5e
ℹ️ 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 saved = sessionStorage.getItem("prechat_native_screen"); | ||
| if (saved === "1") return 1; |
There was a problem hiding this comment.
Reading prechat_native_screen unconditionally from sessionStorage means a stale value from a prior onboarding attempt can skip the name step for a different login in the same webview session (for example, user A reaches vibe step, logs out, then user B logs in and lands directly on vibe). Because the key is not user-scoped or cleared on auth transitions, this introduces cross-session state leakage in onboarding flow progression.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — real concern. This PR merged before I could push the fix, so I opened #31431 to address it: the persisted key is now prechat_native_screen:${userId}, the read moves into a useLayoutEffect keyed on the user-scoped key (so the restore runs once userId is known, before paint), and all write/clear sites use the same scoped key. If user B logs in after user A left a stale key behind, they read their own key (which doesn't exist) and start at the name step.
Generated by Claude Code
… summary (LUM-1759) (#31412) * refactor(web): lift fetchAssistantIdentity + AssistantIdentity into assistant/ (LUM-1753.4) These were sitting in `domains/chat/api/assistant.ts` — the runtime identity of the assistant has nothing to do with chat. It's a property of the assistant itself, queried by chat, the identity tab, contacts, intelligence, onboarding, the sync router, and several stream handlers. Misplaced since day one. Now lives at `apps/web/src/assistant/identity.ts`. The `chat` file is renamed only in its header comment — it now solely owns chat-context bootstrapping (`getChatContext`, `fetchAssistantId`). The `domains/chat/api/assistant.ts` filename stays for now to avoid churn; can be renamed in a follow-up if it bothers anyone. Import sites updated: identity-tab, use-assistant-identity-init, chat-page (two lines), chat-route-content, contacts-page, chat-utils. Cross-domain allow-list: 119 → 118 imports. Small numeric drop — the structural win is that `fetchAssistantIdentity` no longer sits in another feature's folder. Verified locally: lint, typecheck, tests all green. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2 * fix(web): skip avatar character-traits fetch when custom image exists (LUM-1764) Back-port of vellum-ai/vellum-assistant-platform#7468. `useAssistantAvatar()` was unconditionally fetching character traits alongside the components blob and avatar image. When an assistant has a custom avatar image, the traits file is intentionally deleted on the daemon side — so every SSE reconnect (which invalidates `SYNC_TAGS.assistantAvatar`) produced a fresh 404 on the traits endpoint. 308 of them in the platform team's local daemon log. `AvatarRenderer` only reads `traits` when there is no custom image, so the fetch was pure waste in that branch. Switch to: - Fetch components + imageUrl in parallel first. - If imageUrl is non-null, skip the traits fetch entirely. - Otherwise fall through to the original behavior. No new APIs, no behavior change for assistants without a custom image. Adds a unit test mirroring the platform PR's coverage: - skips traits fetch when image resolves - still fetches traits when no image The other half of LUM-1764 — back-porting platform#7494 (remove unused pro-upgrade-machine endpoint) — is blocked. The OSS repo still has an active caller (`compute-upgrade-card.tsx`) that needs to migrate to the unified machine-resize endpoint first. Tracking on the issue. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2 * fix(web): home detail panel overflow menu + dynamic greeting + markdown summary (LUM-1759) Back-port of vellum-ai/vellum-assistant-platform#7467 — the only PR from LUM-1759's set that wasn't already integrated in OSS (7413, 7414, 7415, 7416, 7421 had already landed via earlier home work). Changes: - `home-detail-panel.tsx` — replace the inline mark-as-read toggle button with an overflow menu (`<Menu.Root>`) that contains both "Mark as read / unread" and a new "Dismiss" action. Renames "Go to Thread" → "Go to Convo". Adds the required `onDismiss` prop. - `home-generic-detail.tsx` — render `item.summary` as markdown via the new `HomeMarkdownContent` component, so feed items can use bold/italic/links/lists/etc. for richer summaries. - `home-markdown-content.tsx` (new) — `react-markdown` + GFM with design-token styling tuned for the condensed detail panel. - `home-greeting-header.tsx` — accept an optional `greeting` prop (daemon-supplied), fall back to the existing static string. Wraps both the avatar/greeting and the New Chat button so the greeting can truncate cleanly on narrow widths. - `home-page.tsx` — pass `feedQuery.data?.contextBanner?.greeting` to the header and the new `onDismiss={handleDismissItem}` to both mobile and desktop detail panel mounts. The `contextBanner.greeting` field was already on the home feed response type, so this just wires it through the UI. Verified locally: lint, typecheck clean. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2 * chore(web): address review feedback on home-markdown-content Two non-blocking observations from the vex-bot review on #31412 and #31416 (both files surfaced the same gaps): 1. **Missing element overrides** — react-markdown was falling back to browser defaults for tables and fenced code blocks. Added overrides for `pre`, `table`, `thead`, `tbody`, `tr`, `th`, `td` styled with the home feed's design tokens. Tables wrap in a horizontal-scroll container so wide tables don't blow out the condensed panel layout. 2. **target="_blank" on iOS WKWebView silently fails** — without a `WKUIDelegate createWebViewWith` implementation, the webview won't open a new tab. Route through Capacitor's `openUrl()` helper (already used elsewhere for OAuth/Stripe flows) which presents `SFSafariViewController`. Web keeps the default new-tab behavior; the `href` is preserved on both platforms so right-click → copy link still works. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2 --------- Co-authored-by: Claude <noreply@anthropic.com>
…(LUM-1763) Back-port of vellum-ai/vellum-assistant-platform#7461. Two related iOS prechat polish fixes that landed on the platform side after the onboarding paths were frozen: 1. **Safe-area top spacing.** `NameStepScreen` and `VibeStepScreen` were using a flat `pt-4`, which on iOS slides the back-button row under the status bar / Dynamic Island. Switch to `calc(var(--safe-area-inset-top, env(safe-area-inset-top, 0px)) + 1rem)` so the standard 1rem padding sits on top of the system inset. 2. **Back-nav persistence.** `PreChatFlow` resets to screen 0 on every mount, so an iOS user who taps through to the vibe step and then hot-reloads (or returns after the OS reclaims memory) is silently dropped back to the name step. Remember the position in `sessionStorage` under `prechat_native_screen`; restore on mount; clear on back-from-vibe and on finishNativePreChat. Storage writes are wrapped in try/catch so private-mode contexts don't throw. The other PR in LUM-1763 (#7355 — alex-nork's Sanity connection for the content-automation cohort) is deferred as a cohort- specific feature, not a small drift fix. Verified locally: lint, typecheck clean. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2
Non-blocking observation from the vex-bot review on #31416: `onContinue` and `onSkip` on the iOS NameStepScreen had the same 4-line body (setScreen + sessionStorage.setItem in try/catch), which invites drift if one transition path changes and the other doesn't. Extract a `goToVibeStep` closure inside the screen===0 arm so both callsites point at the same code. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2
ashleeradka
force-pushed
the
ashlee/lum-1763-drift-prechat-ios-safe-area-7461
branch
from
58a8b10 to
279c1e2
Compare
Follow-up to LUM-1763 (#31416 merged) addressing the Codex P2 comment on that PR: > Reading prechat_native_screen unconditionally from > sessionStorage means a stale value from a prior onboarding > attempt can skip the name step for a different login in the > same webview session (user A reaches vibe step, logs out, then > user B logs in and lands directly on vibe). Fix: key the persisted screen by userId. Read happens in a `useLayoutEffect` once userId is known (vs the previous lazy useState initializer that didn't have access to it), so we restore before paint without risking a flash to the wrong step. Write/clear sites now use the same user-scoped key. If the prior session never logged out cleanly, user B reads "their" key which doesn't exist, and starts fresh at the name step — no leak. Verified locally: lint, typecheck clean. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2
Follow-up to LUM-1763 (#31416 merged) addressing the Codex P2 comment on that PR: > Reading prechat_native_screen unconditionally from > sessionStorage means a stale value from a prior onboarding > attempt can skip the name step for a different login in the > same webview session (user A reaches vibe step, logs out, then > user B logs in and lands directly on vibe). Fix: key the persisted screen by userId. Read happens in a `useLayoutEffect` once userId is known (vs the previous lazy useState initializer that didn't have access to it), so we restore before paint without risking a flash to the wrong step. Write/clear sites now use the same user-scoped key. If the prior session never logged out cleanly, user B reads "their" key which doesn't exist, and starts fresh at the name step — no leak. Verified locally: lint, typecheck clean. https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2 Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Closes LUM-1763 (partial — #7355 Sanity feature deferred, see status note).
Summary
Back-port of
vellum-ai/vellum-assistant-platform#7461. Two iOS prechat polish fixes that landed on platform after the OSS onboarding paths were frozen:NameStepScreenandVibeStepScreenwere using a flatpt-4, which on iOS slides the back-button row under the status bar / Dynamic Island. Switch tocalc(var(--safe-area-inset-top, env(safe-area-inset-top, 0px)) + 1rem)so the standard 1rem padding sits on top of the system inset.PreChatFlowresets to screen 0 on every mount, so an iOS user who taps through to the vibe step and hot-reloads (or returns after the OS reclaims memory) is silently dropped back to the name step. Remember the position insessionStorageunderprechat_native_screen; restore on mount; clear on back-from-vibe and onfinishNativePreChat. Storage writes are wrapped in try/catch so private-mode contexts don't throw.LUM-1763 status
Verified locally
bun run lint— 0 errors.bun run typecheck— clean.Test plan
https://claude.ai/code/session_01JPGu4yGPTL4JoLaPuqpEW2
Generated by Claude Code