feat: add AI chat prototype to journey cards (NES-1554)#9037
Open
jaco-brink wants to merge 60 commits intomainfrom
Open
feat: add AI chat prototype to journey cards (NES-1554)#9037jaco-brink wants to merge 60 commits intomainfrom
jaco-brink wants to merge 60 commits intomainfrom
Conversation
Add a streaming AI chat feature gated by the showAssistant field on journeys. Includes chat drawer UI with Radix/Vaul primitives, context extraction from journey blocks, interaction starters, markdown rendering, and a switchable provider backend (apologist/gemini/openai). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add AI disclaimer header and update heading to "How would you like to go deeper?" - Rework quick-action buttons to horizontal pill-style with lucide-react icons - Add close button to drawer, replace Send with arrow icon button - Update input placeholder to "Ask me anything", remove extra container padding Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On the final journey card, replace the floating sparkle button with a pinned-open chat bar at 25vh height. Includes Explain and Reflect interaction starter buttons and an always-visible prompt input. Tapping a starter or submitting a message opens the full AiChat drawer with the message pre-loaded. - New LastCardChatBar component (mobile only, hidden at lg+) - Conductor conditionally renders LastCardChatBar vs StepFooter - OverlayContent adjusts bottom spacing for last-card chat bar - AiChat accepts optional initialMessage prop for pre-loaded messages - Desktop (lg+) retains standard StepFooter with sparkle button - TODO placeholder for third interaction starter button (awaiting Aaron) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove fixed 25vh height from LastCardChatBar so it sizes to its content instead of leaving empty space above the buttons. Remove the border-top separator and flex-end justification that made the gap visible. Update OverlayContent last-card bottom margin from 25vh to 120px to match the actual chat bar content height. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- LastCardChatBar starter buttons: change text to white, border to semi-transparent white, add subtle white background tint for visibility against the dark card overlay - LastCardChatBar starter icons: use white instead of purple for contrast on dark backgrounds - Textarea: add explicit color #1a1a1a so typed text is always readable against the #f5f5f5 background, regardless of inherited theme color (fixes invisible input text on both mobile and desktop) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds a feature-flag gated AI chat backend and client UI: a new POST /api/chat endpoint with multi-provider streaming, ~20 new chat-related UI components/hooks/tests/i18n entries, LaunchDarkly client caching changes, and supporting deps and polyfills. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser/UI
participant AiChat as AiChat Component
participant API as /api/chat Endpoint
participant Provider as AI Provider (Gemini/OpenAI/OpenRouter/Apologist)
Browser->>AiChat: User submits message
AiChat->>API: POST /api/chat (messages + language)
API->>API: Check getFlags().apologistChat
alt flag missing/false
API-->>AiChat: 404 / block
else flag true
API->>API: Validate body.messages
API->>API: Resolve provider from env
API->>Provider: Start streaming (system prompt + messages)
Provider-->>API: Streamed chunks
API-->>AiChat: Stream chunks to client
AiChat->>Browser: Render assistant messages (typewriter)
end
sequenceDiagram
participant User as User
participant Button as AiChatButton
participant Overlay as ChatOverlay
participant AiChat as AiChat (in Overlay)
User->>Button: Click
Button->>Button: set overlayOpen = true
Button->>Overlay: Render open=true
Overlay->>Overlay: Load AiChat (dynamic, ssr:false)
AiChat->>AiChat: Initialize useChat, focus input
User->>AiChat: Submit message -> triggers /api/chat flow
User->>Overlay: Click backdrop or close
Overlay->>Button: onClose -> set overlayOpen = false
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Merge origin/main into feature branch to resolve PR #9037 conflicts: - package.json: kept vaul dependency from feature branch, took updated vercel version (^51.5.1) from main - pnpm-lock.yaml: accepted main's version and regenerated via pnpm install to incorporate both sides' dependency changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
View your CI Pipeline Execution ↗ for commit 4bc2156
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 8a76954
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 8a76954 ☁️ Nx Cloud last updated this comment at |
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
- Add openrouter provider for dev/QA (avoids apologist token burn) - Replace server-side smoothStream with client-side typewriter (provider-agnostic, avoids OpenRouter chunk-parsing issues) - Add bouncing-dot typing indicator while awaiting first token - Hide Copy action until typewriter animation completes - Remove Retry/regenerate on successful responses (fits product goal of capturing unbiased questions) - Add streamText onError logging Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… of https://github.com/JesusFilm/core into feature/nes-1554-apologist-chat-slice-1-prototype-base
jaco-brink
commented
Apr 23, 2026
Collaborator
Author
jaco-brink
left a comment
jaco-brink
left a comment
There was a problem hiding this comment.
Review summary
Scope-filtered against the World-Cup Apologist Chat Journey project — prompt-injection on language (NES-1582) and rate-limiting/abuse protection (NES-1580 cluster) are already tracked, so not re-raised here.
Critical: 1 — isLastCard guard in OverlayContent is stale after the Conductor refactor; pinned bar overlaps card content on non-last cards.
Concern: 3 — breakpoint overlap at sm/md, missing i18n wrappers, suspicious apologist modelId string.
Nit: 1 — dead isDesktop var in PinnedChatBar.
Draft PR → posting as COMMENT.
- Drop isLastCard gate from OverlayContent pinnedChatActive so mb
reservation aligns with Conductor's per-card showPinnedChat, and
extend the 120px reservation through sm/md breakpoints where the
PinnedChatBar is visible.
- Wrap PinnedChatBar drawer title and Drawer close aria-label with
t('libs-journeys-ui'); remove unused isDesktop/useTheme/useMediaQuery
from PinnedChatBar.
- Wrap four PromptInput user-facing strings with t(); add new keys
(Ask me anything, Chat message input, Stop generating, Send message,
Close chat) to en locale.
- Move apologist modelId to APOLOGIST_MODEL_ID env var (default matches
the gateway-specific format from scripts/apologist-stream-test.sh).
Collaborator
Author
Review feedback addressed (cb0742a)Fixed:
Fixed (adjusted):
|
The on-submit drawer was the primary "widget, not native" signal in both recordings. AiChat already owns its own PromptInput, Conversation, typing indicator, error+retry bubble, and stop control — the PinnedChatBar's duplicate input and Drawer wrapper were redundant. PinnedChatBar now renders AiChat directly in an absolute-positioned container anchored to the card bottom, capped at min(70vh, 100%). Height is content-driven up to the cap; Conversation's overflow-y: auto takes over beyond it. No more Chat/× chrome. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Once messages push the chat surface to its cap, there was no way back to the card behind it. Added a subtle pill-shaped handle at the top of the chat surface (visible only when messages exist) that toggles a collapsed state. Collapsed hides Conversation via display:none — preserves scroll position, useChat message state, and in-flight streaming — so re-opening resumes exactly where it left off. iOS sheet pattern over a × button: signals "this can slide away" rather than "close this widget". Re-uses the Drawer component's 48x4 pill for visual consistency. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
If the chat is collapsed when the user sends a new message or retries after an error, the response would stream in invisibly. Force-expand on the three user-driven entry points (handleSubmit, handleRetry, and the initialMessage effect) so answers always land in view. Leaves iOS-sheet mental model intact otherwise — background content changes don't reopen a dismissed surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…1570) Three UX fixes on wider viewports: - Drag handle is now a mobile affordance only (xs). On sm+ it would duplicate desktop dismiss controls and implies a gesture mouse users don't make. - AiChat gains a top-right close/expand icon on sm+ that collapses the conversation in place — same semantics as the mobile handle but expressed as a click target. New `collapsible` prop lets callers that wrap AiChat in their own dismissible container (Drawer) opt out. - Conversation and PromptInput columns cap at 48rem centered on sm+, matching ChatGPT's readable line-length on wide screens. Drawer chrome gets the same treatment: its pill handle hides on sm+, its top corners square off so it reads as a surface edge rather than a floating widget, and the Paper caps at 48rem centered. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ransform (NES-1570) translateX(-50%) on the Drawer Paper collided with MuiDrawer's own transform-based slide-up transition, leaving the panel stranded in the bottom-right corner. Centre via positional CSS instead — left: max(0px, calc(50% - 24rem)) gives a centered 48rem panel on sm+ and clamps to full-width below that, without touching transform. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…dChatBar (NES-1570) Desktop chat drops the Drawer panel and adopts a ChatGPT-style ambient overlay: a floating capsule prompt input centred at the bottom of the viewport, with plain-prose assistant responses streaming above it and a small close button tucked above the column. The card behind stays fully interactive until the user actually clicks into the input. - New `ChatOverlay` component replaces the Drawer branch in `AiChatButton`. Fixed-position layer, pointer-events routed only to the input column and close button. - `AiChat` gains a `variant` prop (`panel` default, `overlay` for desktop) that propagates to `Message.plain` and `PromptInput.variant='floating'`. Panel callers (PinnedChatBar on mobile) are unchanged. - `Message` learns a `plain` mode: assistant messages render as plain prose with no bubble; user messages keep their pill. - `PromptInput` gains a `floating` variant: rounded capsule, shadow, no top border — designed to stand alone over a card. - `Conductor` shifts breakpoints so PinnedChatBar is xs-only and the StepFooter (with AiChatButton → ChatOverlay) appears from sm+. This gives mobile its existing inline UX and desktop the new overlay UX. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…erlay (NES-1570) The transparent overlay rendered dark assistant prose directly over the dark card background, leaving the response unreadable. ChatGPT's equivalent works because their entire app is dark; ours lives over variable card imagery, so the overlay needs its own colour context. - ChatOverlay now paints a full-viewport dark scrim (rgba(10,10,15,0.55) + 6px backdrop blur) behind the column. Clicking the scrim closes the overlay, matching standard modal behaviour. - Message `plain` mode switches assistant text to light (rgba(255,255,255,0.92)) for contrast against the scrim. User pills and bubble-mode messages (mobile) stay dark-on-light. - Actions (Copy) learns a `plain` prop so the action row picks up the same light treatment. Retry button colour also branches on the overlay variant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…m (NES-1570) Three cosmetic follow-ups on the desktop ChatOverlay: - Scrim now derives from theme palette (alpha(grey[900], 0.7)) instead of a hard-coded near-black, so it reads as the app's own dark tone. - Conversation scrollbar picks up thin width and a rounded grey thumb (rgba(128,128,128,0.5)) that stays visible on both the white mobile panel and the dark desktop scrim. - PromptInput in floating mode now nests a matching rounded input (radius 9999, transparent bg) inside the capsule. The previous 3-unit inner radius and grey.50 fill left a visible seam at the top-left corner where the inner rectangle met the outer pill. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Integrates the UX iteration on the apologist chat surface: - Mobile: PinnedChatBar renders AiChat inline with a drag handle. - Desktop: new ambient ChatOverlay replaces the drawer — floating capsule input over a theme-driven scrim, plain assistant prose, X button tucked above the column, rounded scrollbar. - Breakpoint split moved from lg to sm so mobile and desktop UX diverge at the phone/tablet boundary.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (14)
libs/journeys/ui/src/components/Drawer/Drawer.tsx (3)
55-69: Hard-coded colors bypass the MUI theme.
color: '#1a1a1a',bgcolor: '#e0e0e0'(Line 87), andcolor: '#666'(Line 94) won't respond to theme changes (e.g., dark mode) and duplicate values across the component. Since the PR description mentions contrast fixes and "hard-coded light colors on chat surfaces," this appears intentional for the prototype — just flagging for a follow-up pass to migrate totheme.palettetokens (e.g.,text.primary,divider,text.secondary) once the prototype stabilizes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/Drawer/Drawer.tsx` around lines 55 - 69, Replace hard-coded hex colors in Drawer.tsx with MUI theme palette tokens: update the PaperProps.sx color: '#1a1a1a' to use 'text.primary', replace the bgcolor '#e0e0e0' usage (the other Paper/box instance at the noted location) with an appropriate token like 'background.paper' or 'divider', and change the '#666' usage to 'text.secondary'; make these changes in the sx objects (PaperProps and the other style blocks referenced) so the component responds to dark mode and centralized theme changes and keep the token choices consistent across the component.
90-97: Chat-specific aria-label inside a genericDrawercomponent.
aria-label={t('Close chat')}hard-codes a chat use case into what is otherwise a reusableDrawer/DrawerContentprimitive. If this component is reused outside the AI chat surface (the barrel export atDrawer/index.tsmakes it generally importable), the screen-reader announcement will be misleading.Consider making the close label configurable (falling back to a generic default) so the component stays reusable:
♻️ Proposed refactor
interface DrawerContentProps { children: ReactNode title: string + closeLabel?: string } export function DrawerContent({ children, - title + title, + closeLabel }: DrawerContentProps): ReactElement { const { t } = useTranslation('libs-journeys-ui') ... <IconButton onClick={() => onOpenChange(false)} - aria-label={t('Close chat')} + aria-label={closeLabel ?? t('Close')} size="small"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/Drawer/Drawer.tsx` around lines 90 - 97, The IconButton in the Drawer/DrawerContent component uses a chat-specific aria-label (aria-label={t('Close chat')}); change the component API to accept a configurable close label prop (e.g., closeLabel or closeAriaLabel) with a sensible default (e.g., t('Close')) and use that prop for aria-label on the IconButton; update the component props/interface (and any defaultProps or destructuring in the Drawer/DrawerContent function) so consumers can override the label when reusing Drawer outside the chat surface while preserving the current translation-based default.
81-89: Visual-only drag handle.The pill on mobile looks like a drag-to-dismiss affordance but there's no swipe/drag handler wired up — users may try to drag it and get no feedback. Fine as a visual cue for the prototype; consider either adding swipe-to-close via
SwipeableDrawerlater or making this clearly decorative. No action required now.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/Drawer/Drawer.tsx` around lines 81 - 89, The pill element in Drawer (the Box inside the Drawer component) appears to be a drag handle but has no swipe behavior; either make it clearly decorative by marking the Box as non-interactive (add aria-hidden="true", role="presentation" and sx={{ pointerEvents: 'none' }}) and update its styling comment to indicate it's purely visual, or replace the mobile Drawer with MUI's SwipeableDrawer and wire the existing onClose/onOpen handlers so the pill actually supports swipe-to-close; locate the Box in Drawer.tsx (the Box with width:48 height:4 borderRadius:9999) and apply one of these two changes.libs/journeys/ui/src/libs/isLastCard/useIsLastCard.ts (1)
7-9: Optional: tighten theTreeBlock<StepFields>cast.
blockHistoryis typed asTreeBlock[](anyBlockFields), so the cast toTreeBlock<StepFields>is unchecked. In practice history only containsStepBlocks, but a small runtime guard would make the cast sound and avoid silently feeding a non-step intogetNextBlock.♻️ Optional narrowing
- const activeBlock = blockHistory[blockHistory.length - 1] as - | TreeBlock<StepFields> - | undefined - if (activeBlock == null || treeBlocks.length === 0) return false + const lastHistoryBlock = blockHistory[blockHistory.length - 1] + if (lastHistoryBlock == null || treeBlocks.length === 0) return false + if (lastHistoryBlock.__typename !== 'StepBlock') return false + const activeBlock = lastHistoryBlock as TreeBlock<StepFields> return getNextBlock({ activeBlock }) === undefined🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/libs/isLastCard/useIsLastCard.ts` around lines 7 - 9, The current unchecked cast of activeBlock to TreeBlock<StepFields> can allow non-step blocks into getNextBlock; add a runtime narrowing guard before using activeBlock as a Step block: check the block kind/fields (e.g., activeBlock && activeBlock.fields?.type === 'step' or use an existing isStepBlock type guard) and only call getNextBlock or treat it as TreeBlock<StepFields> when that check passes, otherwise return/handle the non-step case early; update the variable use in useIsLastCard (the activeBlock binding) so the cast is removed and the narrowed branch uses a proper StepFields-typed variable.libs/journeys/ui/src/components/Response/Response.tsx (1)
11-16: Consider a typedcomponentsoverride and sanitization policy for AI/user content.
react-markdownv10 escapes HTML and blocksjavascript:URLs by default, so raw XSS via markdown alone is unlikely. However, rendering arbitrary AI output as markdown with nocomponents,remarkPlugins, orurlTransformoverrides means:
- Links (including from model output) render with
target=_selfand norel="noopener noreferrer"; external links from a hallucinated model could be a phishing vector.- Images (
) will trigger network requests to arbitrary hosts, which can be a privacy/SSRF-adjacent concern for logged-in users.For a prototype this is fine, but worth tightening before exposing widely — e.g., override
ato forcetarget="_blank" rel="noopener noreferrer ugc"and disable or allow-listimg.react-markdown v10 default urlTransform and HTML handling defaults🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/Response/Response.tsx` around lines 11 - 16, The Response component renders unmodified markdown via the Markdown element (in Response.tsx) which can produce unsafe links/images; update Response to pass a typed components override and sanitization: provide a custom components.a that forces target="_blank" and rel="noopener noreferrer ugc", implement urlTransform or use rehype-sanitize to block javascript: and other unsafe schemes, and replace or disable the image renderer (components.img) to either disallow remote images or validate/allowlist sources; ensure the props/types for the Markdown components override are strongly typed so the change is compile-time safe.libs/journeys/ui/src/components/Message/Message.tsx (1)
36-57: Hard-coded colors bypass the MUI theme.Colors like
#6D28D9,#f5f5f5,#1a1a1aand rgba literals aren't resolvable by theming (dark mode, white-label, etc.) and duplicate design tokens across components. If the prototype color parity is intentional short-term (PR description alludes to this), consider at minimum extracting the palette into a shared constants module so the values can be swapped in one place later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/Message/Message.tsx` around lines 36 - 57, The Box in Message.tsx uses hard-coded color literals in its sx prop (bgcolor, color) which bypass the MUI theme; replace those literals by reading from the theme palette or a shared design token module: update the sx to be a function receiving theme and use theme.palette.<semanticKey>.main/contrastText (or import shared constants like MESSAGE_TOKENS.userBg/assistantBg/plainAssistantBg) and swap '#6D28D9', '#f5f5f5', '#1a1a1a' and the rgba value with those theme/token references so dark mode and white‑labeling work consistently; create or reuse a small constants file (e.g., MESSAGE_TOKENS) if palette keys aren’t yet available and use those tokens in the sx logic that currently branches on isUser and isPlainAssistant.libs/journeys/ui/src/components/Actions/Actions.tsx (1)
6-15: Rename component to better reflect its chat-message context.
Actionsis too generic for a message-level copy action bar inside the chat feature. Per the naming conventions used elsewhere in the codebase (%UiType%%ComponentFunction%), consider renaming toMessageActionsorChatMessageActionsto be more descriptive at call sites and reduce collision risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/Actions/Actions.tsx` around lines 6 - 15, Rename the generic Actions component to a more descriptive chat message name: update the interface ActionsProps to MessageActionsProps (or ChatMessageActionsProps) and rename the exported function Actions to MessageActions (or ChatMessageActions), adjust the props destructuring signature accordingly, and update all local exports and any import sites that reference Actions/ActionsProps so they import the new symbol name; ensure any tests, storybook entries, and type annotations are updated to the new names to avoid breaking references.libs/journeys/ui/src/components/Card/OverlayContent/OverlayContent.tsx (1)
76-81: ReusehasAiChatButtonto avoid duplicated gating logic.The variant and
showAssistantchecks duplicate the logic inhasAiChatButton(libs/journeys/ui/src/components/Card/utils/getFooterElements/getFooterElements.ts), which is already used alongsideflags.apologistChatinStepFooter.tsx. Reusing the helper keeps the pinned-chat visibility condition in one place.♻️ Proposed refactor
-import { getFooterMobileSpacing } from '../utils/getFooterElements' +import { + getFooterMobileSpacing, + hasAiChatButton +} from '../utils/getFooterElements' ... - const flags = useFlags() - const pinnedChatActive = - flags.apologistChat === true && - journey?.showAssistant === true && - variant !== 'admin' && - variant !== 'embed' + const flags = useFlags() + const pinnedChatActive = + flags.apologistChat === true && hasAiChatButton({ journey, variant })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/Card/OverlayContent/OverlayContent.tsx` around lines 76 - 81, The pinnedChatActive computation duplicates gating logic—replace the inline checks in OverlayContent (the useFlags + pinnedChatActive block) with the existing helper hasAiChatButton from getFooterElements so visibility logic is centralized; specifically, import and call hasAiChatButton(journey, variant) and combine it with flags.apologistChat (i.e. pinnedChatActive = flags.apologistChat === true && hasAiChatButton(...)) instead of rechecking journey?.showAssistant and variant here so behavior matches StepFooter.tsx.libs/journeys/ui/src/components/AiChatButton/AiChatButton.tsx (1)
30-39: Redundant keyboard handling on MUIIconButton.
IconButtonrenders a native<button>, which is already focusable and triggersonClickon Enter/Space. BothtabIndex={0}and theonKeyDownhandler are redundant and add maintenance overhead.♻️ Proposed simplification
<IconButton onClick={handleClick} aria-label={t('Open AI chat')} - tabIndex={0} - onKeyDown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault() - handleClick() - } - }} sx={{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/AiChatButton/AiChatButton.tsx` around lines 30 - 39, The IconButton is handling keyboard activation redundantly—remove the explicit tabIndex={0} and the onKeyDown handler from the IconButton so the native button behavior (Enter/Space activating onClick) is used; keep onClick={handleClick} and aria-label intact. Locate the IconButton usage in AiChatButton.tsx and delete the tabIndex and the onKeyDown prop while leaving handleClick and aria-label unchanged to preserve accessibility and simplify the component.libs/journeys/ui/src/components/PromptInput/PromptInput.tsx (2)
111-157: Hardcoded hex colors bypass the theme paletteColors like
#1a1a1a,#9e9e9e,#6D28D9,#5B21B6, etc. are inlined rather than sourced from the MUI theme. The PR description notes this is intentional "hard-coded light colors on chat surfaces" for prototype parity, so flagging only as a follow-up — worth migrating totheme.palettetokens (or a dedicated chat palette extension) before this graduates out of prototype, so the chat surface can respect theme modes / RTL color swaps and be restyled without code changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/PromptInput/PromptInput.tsx` around lines 111 - 157, The component PromptInput.tsx currently uses hardcoded hex colors in the sx props and nested selectors (e.g., '& .MuiInputBase-input', '& .MuiInputBase-input::placeholder', and the IconButton sx blocks for the submit/stop buttons); replace those literal color strings (like '#1a1a1a', '#9e9e9e', '#6D28D9', '#5B21B6', '#e0e0e0', '#999', '#666') with values resolved from the MUI theme (e.g., theme.palette.* tokens or a dedicated chat palette extension) by accessing theme in the sx callbacks or using classes that map to palette tokens so the InputBase placeholder, input text color, button background, hover and disabled states all come from theme.palette rather than hardcoded hex values.
38-38:textareaRefis unused
textareaRefis created and wired toTextField.inputRefbut never read from anywhere else in the component. Unless you're planning to use it (e.g. to focus the input after send or when the prompt bar mounts), the ref and theuseRefimport can be dropped.♻️ Proposed cleanup
-import { - FormEvent, - KeyboardEvent, - ReactElement, - useCallback, - useRef -} from 'react' +import { FormEvent, KeyboardEvent, ReactElement, useCallback } from 'react' ... - const { t } = useTranslation('libs-journeys-ui') - const textareaRef = useRef<HTMLTextAreaElement>(null) + const { t } = useTranslation('libs-journeys-ui') ... - <TextField - inputRef={textareaRef} - value={input} + <TextField + value={input}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/PromptInput/PromptInput.tsx` at line 38, The textareaRef created with useRef<HTMLTextAreaElement>(null) is never read; remove the unused textareaRef and its wiring to TextField.inputRef and also remove the unused useRef import from the module, or alternatively actually use textareaRef (for example to call textareaRef.current?.focus() after send/mount) — update the component to either eliminate textareaRef and the inputRef prop on TextField or implement the focus/DOM access logic where needed (reference symbols: textareaRef, TextField.inputRef, useRef).apps/journeys/src/components/Conductor/Conductor.tsx (1)
76-80: Variant gating is duplicated inPinnedChatBar
PinnedChatBaralready short-circuits tonullwhenvariant === 'admin' || variant === 'embed'(seelibs/journeys/ui/src/components/PinnedChatBar/PinnedChatBar.tsxlines 25-27), so thevariant !== 'admin' && variant !== 'embed'clauses here are redundant. Either drop the duplicate check inConductor(lettingPinnedChatBarown the rule) or drop it inPinnedChatBar— keeping it in both means future variant rules have to be kept in sync in two places. Not blocking; defensive duplication is fine if intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys/src/components/Conductor/Conductor.tsx` around lines 76 - 80, The variant gating for admin/embed is duplicated: remove the redundant checks from Conductor by updating the showPinnedChat calculation (remove the "variant !== 'admin' && variant !== 'embed'" clause) so that PinnedChatBar retains responsibility for short-circuiting; keep the rest of the predicate (apologistChatEnabled && journey?.showAssistant === true) and rely on PinnedChatBar to enforce variant-based visibility (see showPinnedChat and PinnedChatBar).apps/journeys/pages/api/chat/index.ts (1)
96-110: No auth or rate limiting on this endpointOnce
apologistChatistrue,/api/chatis world-reachable, unauthenticated, and each successful POST triggers an upstream LLM stream (billable). For a prototype that's acceptable with a tight feature-flag audience, but worth tracking before this rolls out broadly — at minimum a per-IP/per-visitor rate limit and a request-size cap would protect against cost abuse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/journeys/pages/api/chat/index.ts` around lines 96 - 110, The handler currently allows unauthenticated, unlimited access once getFlags().apologistChat is true; add authentication and rate/size protections at the start of the exported handler: validate the requester (e.g., call a verifyAuth or validateApiKey function) and reject 401/403 when missing/invalid, enforce a per-IP or per-user rate limit (e.g., via an applyRateLimit or checkRateLimit helper that returns 429 when exceeded), and enforce a request body size cap (e.g., bodySizeLimit or validatePayloadSize before parsing) to return 413 for oversized requests; keep these checks before any upstream LLM call and reuse clear identifiers from this file (handler, getFlags, apologistChat) so the changes are easy to locate and test.libs/journeys/ui/src/components/AiChat/AiChat.tsx (1)
72-92: RAF keeps running after reveal completes.Once
!isStreaming && revealed >= target.length, the effect still schedules a newrequestAnimationFrameevery frame with a no-opsetRevealed. Cheap, but unnecessary for every open assistant bubble. Consider breaking the loop when the reveal is caught up and streaming has ended (addisStreamingto deps and re-arm when it flips back true).♻️ Sketch
useEffect(() => { if (!enabled) return let raf = 0 let last = performance.now() const tick = (now: number): void => { const deltaSec = (now - last) / 1000 last = now - setRevealed((prev) => { - const targetLength = targetRef.current.length - if (prev >= targetLength) return prev - const step = Math.max( - 1, - Math.floor(TYPEWRITER_CHARS_PER_SEC * deltaSec) - ) - return Math.min(targetLength, prev + step) - }) - raf = requestAnimationFrame(tick) + let caughtUp = false + setRevealed((prev) => { + const targetLength = targetRef.current.length + if (prev >= targetLength) { + caughtUp = true + return prev + } + const step = Math.max(1, Math.floor(TYPEWRITER_CHARS_PER_SEC * deltaSec)) + return Math.min(targetLength, prev + step) + }) + if (caughtUp && !isStreaming) return + raf = requestAnimationFrame(tick) } raf = requestAnimationFrame(tick) return () => cancelAnimationFrame(raf) - }, [enabled]) + }, [enabled, isStreaming])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/AiChat/AiChat.tsx` around lines 72 - 92, The RAF loop in the useEffect (tick/requestAnimationFrame) keeps running even after the typewriter reveal is complete; modify the effect for the component using targetRef, setRevealed and TYPEWRITER_CHARS_PER_SEC to stop re-arming when the content is fully revealed and streaming has ended: add isStreaming to the effect deps, and inside tick check the current revealed/targetLength and isStreaming — if revealed >= targetLength and !isStreaming, cancel the RAF and return without scheduling a new requestAnimationFrame; keep the existing cleanup that cancels raf so when isStreaming flips back to true the effect will re-arm and resume.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/journeys/pages/api/chat/index.ts`:
- Around line 177-190: The system prompt builder (function buildSystemMessage)
directly interpolates the request body language (ChatRequestBody.language),
allowing prompt-injection; validate the language before concatenation by
checking it against a strict allow-list of supported languages or a BCP-47 regex
and only append the "Respond in the following language" line when the value
passes validation; if validation fails, either omit the language line or fall
back to a safe default (e.g., "en") and ensure the validation logic is
implemented where ChatRequestBody.language is consumed so buildSystemMessage
receives only vetted values.
- Around line 112-126: Validate and sanitize req.body before using it: replace
the unchecked cast to ChatRequestBody with runtime validation (e.g., a Zod
schema) that enforces messages as an array of objects with required
role/content/parts shapes, a capped messages.length (e.g., MAX_MESSAGES), a
maximum total character count across messages (e.g., MAX_TOTAL_CHARS), and a
constrained language string; perform this validation in the API handler before
calling resolveChatModel/buildSystemMessage/convertToModelMessages and return
400 on schema violations. Additionally, add simple rate limiting by IP/visitor
in this route (memory store or existing middleware) to prevent abuse while
apologistChat is enabled, and ensure error responses include clear validation
failure messages.
In `@libs/journeys/ui/src/components/Actions/Actions.tsx`:
- Around line 18-20: The handleCopy useCallback currently calls
navigator.clipboard.writeText(content) without error handling; wrap that call in
a try/catch inside handleCopy to catch rejected promises, and on success/failure
update or invoke a feedback mechanism (e.g., set a local success/error state,
call a provided onCopy callback or show a toast) so the user gets notified of
copy outcome; ensure handleCopy remains memoized and still depends on content.
In `@libs/journeys/ui/src/components/AiChat/AiChat.tsx`:
- Around line 175-183: The keyboard handler handleHandleKeyDown duplicates
native button behavior and causes double-toggle: remove the onKeyDown handler
and delete the handleHandleKeyDown function (and its KeyboardEvent import if
unused) so IconButton relies on its native onClick which calls
handleToggleCollapse and setCollapsed; also rename any remaining occurrences to
a descriptive name if needed and ensure no other code references
handleHandleKeyDown (repeat the same removal for the similar block around lines
267-281).
- Around line 126-157: The TypingIndicator component uses a hard-coded
aria-label ("Assistant is typing"); replace it with a translated string via the
existing t() translation function (e.g., aria-label={t('aiChat.typing')}),
ensuring you import/use the same translation hook used elsewhere in AiChat.tsx
and add the new key (aiChat.typing) to the libs-journeys-ui en locale file with
the appropriate English value.
In
`@libs/journeys/ui/src/components/Card/utils/getFooterElements/getFooterElements.ts`:
- Around line 47-53: hasAiChatButton currently only checks journey.showAssistant
and variant, causing getFooterMobileSpacing/getFooterMobileHeight to reserve
space when flags.apologistChat is false; change hasAiChatButton({ journey,
variant }: JourneyInfoProps) to accept a resolved boolean (e.g., apologistChat:
boolean) or the full flags object and include apologistChat in its return
(return variant !== 'admin' && variant !== 'embed' && journey?.showAssistant ===
true && apologistChat === true), then update callers (StepFooter and the spacing
helpers getFooterMobileSpacing/getFooterMobileHeight) to pass the apologistChat
flag (or the pre-computed boolean) so spacing is computed from the same boolean
that actually controls button rendering.
In `@libs/journeys/ui/src/components/ChatOverlay/ChatOverlay.tsx`:
- Around line 24-88: ChatOverlay is rendering a modal-like overlay with Boxes
but lacks Escape-to-close and focus management; update ChatOverlay to use MUI
Modal or Dialog (wrapping the existing outer Box and backdrop) so you get
built-in aria-modal, escape-key handling, focus trap, and scroll lock, and pass
onClose as the Modal/Dialog onClose prop; ensure the IconButton close still
calls onClose and that AiChat remains inside the Modal/Dialog so initial focus
is trapped on open and returns to the opener when closed.
In `@libs/shared/ui/src/libs/getLaunchDarklyClient/getLaunchDarklyClient.ts`:
- Around line 38-49: The timeout timer created inside the Promise.race leaks
because the setTimeout callback isn’t cleared when
launchDarklyClient.waitForInitialization resolves; fix by capturing the timer id
in an outer-scoped variable (e.g., let initTimer: NodeJS.Timeout | null), assign
it inside the new Promise that calls setTimeout, and after the Promise.race
await (or in a finally block) call clearTimeout(initTimer) (and null it) so the
timer is cancelled; reference the existing
launchDarklyClient.waitForInitialization call and INIT_TIMEOUT_SECONDS constant
when making this change.
- Around line 31-36: The cached LaunchDarkly client at
globalForLD.__launchDarklyClient can remain in a failed or stuck state after
init() times out; modify getLaunchDarklyClient so that when
waitForInitialization() rejects or times out you explicitly clear
globalForLD.__launchDarklyClient (set to undefined/null) before returning the
fallback stub or throwing, allowing subsequent calls to create a fresh client;
specifically wrap the waitForInitialization() call on the launchDarklyClient
instance with try/catch/finally and in the failure path clear
globalForLD.__launchDarklyClient and then proceed to return the stub or
propagate the error.
---
Nitpick comments:
In `@apps/journeys/pages/api/chat/index.ts`:
- Around line 96-110: The handler currently allows unauthenticated, unlimited
access once getFlags().apologistChat is true; add authentication and rate/size
protections at the start of the exported handler: validate the requester (e.g.,
call a verifyAuth or validateApiKey function) and reject 401/403 when
missing/invalid, enforce a per-IP or per-user rate limit (e.g., via an
applyRateLimit or checkRateLimit helper that returns 429 when exceeded), and
enforce a request body size cap (e.g., bodySizeLimit or validatePayloadSize
before parsing) to return 413 for oversized requests; keep these checks before
any upstream LLM call and reuse clear identifiers from this file (handler,
getFlags, apologistChat) so the changes are easy to locate and test.
In `@apps/journeys/src/components/Conductor/Conductor.tsx`:
- Around line 76-80: The variant gating for admin/embed is duplicated: remove
the redundant checks from Conductor by updating the showPinnedChat calculation
(remove the "variant !== 'admin' && variant !== 'embed'" clause) so that
PinnedChatBar retains responsibility for short-circuiting; keep the rest of the
predicate (apologistChatEnabled && journey?.showAssistant === true) and rely on
PinnedChatBar to enforce variant-based visibility (see showPinnedChat and
PinnedChatBar).
In `@libs/journeys/ui/src/components/Actions/Actions.tsx`:
- Around line 6-15: Rename the generic Actions component to a more descriptive
chat message name: update the interface ActionsProps to MessageActionsProps (or
ChatMessageActionsProps) and rename the exported function Actions to
MessageActions (or ChatMessageActions), adjust the props destructuring signature
accordingly, and update all local exports and any import sites that reference
Actions/ActionsProps so they import the new symbol name; ensure any tests,
storybook entries, and type annotations are updated to the new names to avoid
breaking references.
In `@libs/journeys/ui/src/components/AiChat/AiChat.tsx`:
- Around line 72-92: The RAF loop in the useEffect (tick/requestAnimationFrame)
keeps running even after the typewriter reveal is complete; modify the effect
for the component using targetRef, setRevealed and TYPEWRITER_CHARS_PER_SEC to
stop re-arming when the content is fully revealed and streaming has ended: add
isStreaming to the effect deps, and inside tick check the current
revealed/targetLength and isStreaming — if revealed >= targetLength and
!isStreaming, cancel the RAF and return without scheduling a new
requestAnimationFrame; keep the existing cleanup that cancels raf so when
isStreaming flips back to true the effect will re-arm and resume.
In `@libs/journeys/ui/src/components/AiChatButton/AiChatButton.tsx`:
- Around line 30-39: The IconButton is handling keyboard activation
redundantly—remove the explicit tabIndex={0} and the onKeyDown handler from the
IconButton so the native button behavior (Enter/Space activating onClick) is
used; keep onClick={handleClick} and aria-label intact. Locate the IconButton
usage in AiChatButton.tsx and delete the tabIndex and the onKeyDown prop while
leaving handleClick and aria-label unchanged to preserve accessibility and
simplify the component.
In `@libs/journeys/ui/src/components/Card/OverlayContent/OverlayContent.tsx`:
- Around line 76-81: The pinnedChatActive computation duplicates gating
logic—replace the inline checks in OverlayContent (the useFlags +
pinnedChatActive block) with the existing helper hasAiChatButton from
getFooterElements so visibility logic is centralized; specifically, import and
call hasAiChatButton(journey, variant) and combine it with flags.apologistChat
(i.e. pinnedChatActive = flags.apologistChat === true && hasAiChatButton(...))
instead of rechecking journey?.showAssistant and variant here so behavior
matches StepFooter.tsx.
In `@libs/journeys/ui/src/components/Drawer/Drawer.tsx`:
- Around line 55-69: Replace hard-coded hex colors in Drawer.tsx with MUI theme
palette tokens: update the PaperProps.sx color: '#1a1a1a' to use 'text.primary',
replace the bgcolor '#e0e0e0' usage (the other Paper/box instance at the noted
location) with an appropriate token like 'background.paper' or 'divider', and
change the '#666' usage to 'text.secondary'; make these changes in the sx
objects (PaperProps and the other style blocks referenced) so the component
responds to dark mode and centralized theme changes and keep the token choices
consistent across the component.
- Around line 90-97: The IconButton in the Drawer/DrawerContent component uses a
chat-specific aria-label (aria-label={t('Close chat')}); change the component
API to accept a configurable close label prop (e.g., closeLabel or
closeAriaLabel) with a sensible default (e.g., t('Close')) and use that prop for
aria-label on the IconButton; update the component props/interface (and any
defaultProps or destructuring in the Drawer/DrawerContent function) so consumers
can override the label when reusing Drawer outside the chat surface while
preserving the current translation-based default.
- Around line 81-89: The pill element in Drawer (the Box inside the Drawer
component) appears to be a drag handle but has no swipe behavior; either make it
clearly decorative by marking the Box as non-interactive (add
aria-hidden="true", role="presentation" and sx={{ pointerEvents: 'none' }}) and
update its styling comment to indicate it's purely visual, or replace the mobile
Drawer with MUI's SwipeableDrawer and wire the existing onClose/onOpen handlers
so the pill actually supports swipe-to-close; locate the Box in Drawer.tsx (the
Box with width:48 height:4 borderRadius:9999) and apply one of these two
changes.
In `@libs/journeys/ui/src/components/Message/Message.tsx`:
- Around line 36-57: The Box in Message.tsx uses hard-coded color literals in
its sx prop (bgcolor, color) which bypass the MUI theme; replace those literals
by reading from the theme palette or a shared design token module: update the sx
to be a function receiving theme and use
theme.palette.<semanticKey>.main/contrastText (or import shared constants like
MESSAGE_TOKENS.userBg/assistantBg/plainAssistantBg) and swap '#6D28D9',
'#f5f5f5', '#1a1a1a' and the rgba value with those theme/token references so
dark mode and white‑labeling work consistently; create or reuse a small
constants file (e.g., MESSAGE_TOKENS) if palette keys aren’t yet available and
use those tokens in the sx logic that currently branches on isUser and
isPlainAssistant.
In `@libs/journeys/ui/src/components/PromptInput/PromptInput.tsx`:
- Around line 111-157: The component PromptInput.tsx currently uses hardcoded
hex colors in the sx props and nested selectors (e.g., '& .MuiInputBase-input',
'& .MuiInputBase-input::placeholder', and the IconButton sx blocks for the
submit/stop buttons); replace those literal color strings (like '#1a1a1a',
'#9e9e9e', '#6D28D9', '#5B21B6', '#e0e0e0', '#999', '#666') with values resolved
from the MUI theme (e.g., theme.palette.* tokens or a dedicated chat palette
extension) by accessing theme in the sx callbacks or using classes that map to
palette tokens so the InputBase placeholder, input text color, button
background, hover and disabled states all come from theme.palette rather than
hardcoded hex values.
- Line 38: The textareaRef created with useRef<HTMLTextAreaElement>(null) is
never read; remove the unused textareaRef and its wiring to TextField.inputRef
and also remove the unused useRef import from the module, or alternatively
actually use textareaRef (for example to call textareaRef.current?.focus() after
send/mount) — update the component to either eliminate textareaRef and the
inputRef prop on TextField or implement the focus/DOM access logic where needed
(reference symbols: textareaRef, TextField.inputRef, useRef).
In `@libs/journeys/ui/src/components/Response/Response.tsx`:
- Around line 11-16: The Response component renders unmodified markdown via the
Markdown element (in Response.tsx) which can produce unsafe links/images; update
Response to pass a typed components override and sanitization: provide a custom
components.a that forces target="_blank" and rel="noopener noreferrer ugc",
implement urlTransform or use rehype-sanitize to block javascript: and other
unsafe schemes, and replace or disable the image renderer (components.img) to
either disallow remote images or validate/allowlist sources; ensure the
props/types for the Markdown components override are strongly typed so the
change is compile-time safe.
In `@libs/journeys/ui/src/libs/isLastCard/useIsLastCard.ts`:
- Around line 7-9: The current unchecked cast of activeBlock to
TreeBlock<StepFields> can allow non-step blocks into getNextBlock; add a runtime
narrowing guard before using activeBlock as a Step block: check the block
kind/fields (e.g., activeBlock && activeBlock.fields?.type === 'step' or use an
existing isStepBlock type guard) and only call getNextBlock or treat it as
TreeBlock<StepFields> when that check passes, otherwise return/handle the
non-step case early; update the variable use in useIsLastCard (the activeBlock
binding) so the cast is removed and the narrowed branch uses a proper
StepFields-typed variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d54435a1-3b51-497f-a8cb-1adb131b2cc2
⛔ Files ignored due to path filters (5)
apps/journeys/__generated__/GetJourney.tsis excluded by!**/__generated__/**apps/journeys/__generated__/JourneyFields.tsis excluded by!**/__generated__/**libs/journeys/ui/src/libs/JourneyProvider/__generated__/JourneyFields.tsis excluded by!**/__generated__/**libs/journeys/ui/src/libs/useJourneyQuery/__generated__/GetJourney.tsis excluded by!**/__generated__/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
apps/journeys/pages/_app.tsxapps/journeys/pages/api/chat/index.spec.tsapps/journeys/pages/api/chat/index.tsapps/journeys/setupTests.tsapps/journeys/src/components/Conductor/Conductor.apologistChat.spec.tsxapps/journeys/src/components/Conductor/Conductor.spec.tsxapps/journeys/src/components/Conductor/Conductor.tsxlibs/journeys/ui/src/components/Actions/Actions.tsxlibs/journeys/ui/src/components/Actions/index.tslibs/journeys/ui/src/components/AiChat/AiChat.tsxlibs/journeys/ui/src/components/AiChat/index.tslibs/journeys/ui/src/components/AiChatButton/AiChatButton.tsxlibs/journeys/ui/src/components/AiChatButton/index.tslibs/journeys/ui/src/components/Card/OverlayContent/OverlayContent.tsxlibs/journeys/ui/src/components/Card/utils/getFooterElements/getFooterElements.tslibs/journeys/ui/src/components/Card/utils/getFooterElements/index.tslibs/journeys/ui/src/components/ChatOverlay/ChatOverlay.tsxlibs/journeys/ui/src/components/ChatOverlay/index.tslibs/journeys/ui/src/components/Conversation/Conversation.tsxlibs/journeys/ui/src/components/Conversation/index.tslibs/journeys/ui/src/components/Drawer/Drawer.tsxlibs/journeys/ui/src/components/Drawer/index.tslibs/journeys/ui/src/components/Message/Message.tsxlibs/journeys/ui/src/components/Message/index.tslibs/journeys/ui/src/components/PinnedChatBar/PinnedChatBar.tsxlibs/journeys/ui/src/components/PinnedChatBar/index.tslibs/journeys/ui/src/components/PromptInput/PromptInput.tsxlibs/journeys/ui/src/components/PromptInput/index.tslibs/journeys/ui/src/components/Response/Response.tsxlibs/journeys/ui/src/components/Response/index.tslibs/journeys/ui/src/components/StepFooter/StepFooter.apologistChat.spec.tsxlibs/journeys/ui/src/components/StepFooter/StepFooter.tsxlibs/journeys/ui/src/libs/JourneyProvider/journeyFields.tsxlibs/journeys/ui/src/libs/isLastCard/index.tslibs/journeys/ui/src/libs/isLastCard/useIsLastCard.tslibs/locales/en/libs-journeys-ui.jsonlibs/shared/ui/src/libs/getLaunchDarklyClient/getLaunchDarklyClient.tspackage.json
💤 Files with no reviewable changes (1)
- apps/journeys/src/components/Conductor/Conductor.spec.tsx
edmonday
reviewed
Apr 24, 2026
Contributor
edmonday
left a comment
edmonday
left a comment
There was a problem hiding this comment.
Review summary
Focused review on the two Critical items in /api/chat. Leaving the Concern-level items (LaunchDarkly timer/timeout, pervasive hardcoded hex colors, unused useIsLastCard, Conductor breakpoint duplication) for a follow-up pass — CodeRabbit has already raised several of them.
Critical (please address before flipping the flag on in prod):
apps/journeys/pages/api/chat/index.ts:112— no runtime validation ofreq.body; unbounded LLM spend + client-injectedsystemrole.apps/journeys/pages/api/chat/index.ts:186—languageis unvalidated user input concatenated into the system prompt; prompt-injection is trivial once the flag is on.
Both are public via POST — the route has no auth, middleware excludes /api/*, and the journeys app only has anonymous Firebase visitors. The flag gate is the only thing between an attacker and an LLM bill.
Collaborator
Author
|
Thanks @edmonday — responding to the focused review. Critical (both addressed as planned-and-tracked):
All three are M4 tickets — the full M4 'Public-launch safety floor' milestone lands before the Concerns (parked, not this PR):
Happy to block merge on any of these if you'd prefer — otherwise keeping Slice 1 tight and rolling the safety + cleanup work forward as planned. |
…nt (NES-1554) - Actions: wrap clipboard writeText in try/catch so permission/focus rejections don't surface as unhandled promise rejections. - AiChat: translate TypingIndicator aria-label via t(), add "Assistant is typing" to libs-journeys-ui en locale. - AiChat: remove handleHandleKeyDown — MUI IconButton already fires onClick on Space/Enter, so the extra onKeyDown caused a double-toggle that left mobile keyboard users unable to collapse the chat. - getLaunchDarklyClient: clear the cached client on init failure so subsequent calls can retry with a fresh instance rather than re-racing the same stuck instance and paying the 10s timeout on every request. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
Author
Review feedback addressed (f087f1f)All 9 unresolved threads now closed — all 9 resolved, 4 fixed in this PR, 5 deferred to planned/cleanup tickets with explicit links. Fixed in this PR:
Deferred to planned safety work (M4 — Public-launch safety floor):
Rationale: apologist chat is gated behind internal-only Deferred to Cleanup & Tech Debt tickets:
Note:
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/journeys/ui/src/components/AiChat/AiChat.tsx (1)
62-97: Cancel the rAF loop once the typewriter reaches the target.The effect keeps scheduling
requestAnimationFrame(tick)for every frame as long asenabledis true, even afterrevealed >= targetRef.current.lengthand streaming has stopped.setRevealedbails out of re-renders (same value), but the rAF scheduling itself continues for the lifetime of the last assistant bubble — wasteful CPU/battery for an otherwise idle chat. Stop the loop when fully revealed and streaming is done, and resume only if the target grows again.♻️ Proposed refactor
- useEffect(() => { - if (!enabled) return - let raf = 0 - let last = performance.now() - const tick = (now: number): void => { - const deltaSec = (now - last) / 1000 - last = now - setRevealed((prev) => { - const targetLength = targetRef.current.length - if (prev >= targetLength) return prev - const step = Math.max( - 1, - Math.floor(TYPEWRITER_CHARS_PER_SEC * deltaSec) - ) - return Math.min(targetLength, prev + step) - }) - raf = requestAnimationFrame(tick) - } - raf = requestAnimationFrame(tick) - return () => cancelAnimationFrame(raf) - }, [enabled]) + useEffect(() => { + if (!enabled) return + let raf = 0 + let last = performance.now() + const tick = (now: number): void => { + const deltaSec = (now - last) / 1000 + last = now + let done = false + setRevealed((prev) => { + const targetLength = targetRef.current.length + if (prev >= targetLength) { + done = !isStreaming + return prev + } + const step = Math.max( + 1, + Math.floor(TYPEWRITER_CHARS_PER_SEC * deltaSec) + ) + return Math.min(targetLength, prev + step) + }) + if (done) return + raf = requestAnimationFrame(tick) + } + raf = requestAnimationFrame(tick) + return () => cancelAnimationFrame(raf) + }, [enabled, isStreaming])Adding
isStreamingto deps re-starts the loop if new chunks arrive after a pause.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/journeys/ui/src/components/AiChat/AiChat.tsx` around lines 62 - 97, The useTypewriter effect currently keeps scheduling requestAnimationFrame even after revealed >= target length and streaming has stopped; modify the effect in useTypewriter so the tick function checks current target length and isStreaming and calls cancelAnimationFrame(raf) / avoids requesting a new frame when revealed >= targetLength && !isStreaming (i.e., stop the loop once fully revealed and not streaming), and update the effect dependency array to include isStreaming and target (so the loop restarts when streaming resumes or the target grows); reference the revealed state, targetRef.current, isStreaming param, and the raf variable/requestAnimationFrame/cancelAnimationFrame when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/journeys/ui/src/components/AiChat/AiChat.tsx`:
- Around line 62-97: The useTypewriter effect currently keeps scheduling
requestAnimationFrame even after revealed >= target length and streaming has
stopped; modify the effect in useTypewriter so the tick function checks current
target length and isStreaming and calls cancelAnimationFrame(raf) / avoids
requesting a new frame when revealed >= targetLength && !isStreaming (i.e., stop
the loop once fully revealed and not streaming), and update the effect
dependency array to include isStreaming and target (so the loop restarts when
streaming resumes or the target grows); reference the revealed state,
targetRef.current, isStreaming param, and the raf
variable/requestAnimationFrame/cancelAnimationFrame when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 80c629d1-47c9-4704-831a-784f2a0c21b6
📒 Files selected for processing (4)
libs/journeys/ui/src/components/Actions/Actions.tsxlibs/journeys/ui/src/components/AiChat/AiChat.tsxlibs/locales/en/libs-journeys-ui.jsonlibs/shared/ui/src/libs/getLaunchDarklyClient/getLaunchDarklyClient.ts
✅ Files skipped from review due to trivial changes (1)
- libs/locales/en/libs-journeys-ui.json
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/shared/ui/src/libs/getLaunchDarklyClient/getLaunchDarklyClient.ts
- libs/journeys/ui/src/components/Actions/Actions.tsx
This comment has been minimized.
This comment has been minimized.
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.
Summary
Related Issues
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests