Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a complete "Legal Assistant" agent kit under Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client (Browser)
participant NextApp as Next.js App
participant FlowFS as Flow Files
participant LamaticAPI as Lamatic API
Note over Browser,NextApp: User opens Legal Assistant page
Browser->>NextApp: GET / (SSR)
NextApp->>FlowFS: read flow config / flows/.../config.json
NextApp-->>Browser: render page + widget script tags
Browser->>NextApp: widget init (flowId, apiUrl, projectId)
NextApp->>LamaticAPI: fetch widget config (askConfig/chatConfig)
LamaticAPI-->>NextApp: widget config
NextApp-->>Browser: inject widget root & module script
Browser->>LamaticAPI: executeFlow (question, jurisdiction, context)
LamaticAPI-->>NextApp: flow execution result
NextApp-->>Browser: parsed structured response (answer, refs, nextSteps, disclaimer)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (22)
kits/assistant/legal/components/ui/kbd.tsx-18-26 (1)
18-26:⚠️ Potential issue | 🟡 MinorType/element mismatch: props typed as
divbut rendered askbd.The component accepts
React.ComponentProps<'div'>but renders a<kbd>element. This inconsistency can cause confusion and potential type issues if div-specific props are passed.If
KbdGroupis intended as a container for grouping multiple keyboard keys, a<div>or<span>would be more semantically appropriate:🔧 Proposed fix: align element with type signature
function KbdGroup({ className, ...props }: React.ComponentProps<'div'>) { return ( - <kbd + <div data-slot="kbd-group" className={cn('inline-flex items-center gap-1', className)} {...props} /> ) }Alternatively, if
<kbd>is intentional, update the type:-function KbdGroup({ className, ...props }: React.ComponentProps<'div'>) { +function KbdGroup({ className, ...props }: React.ComponentProps<'kbd'>) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/kbd.tsx` around lines 18 - 26, KbdGroup is typed as React.ComponentProps<'div'> but renders a <kbd>, causing a prop/element mismatch; either change the rendered element to match the type (e.g., render a <div> or <span> inside the KbdGroup component) or update the prop type to React.ComponentProps<'kbd'> (and adjust any consumers if needed). Locate the KbdGroup function and update the JSX element or the type signature so the component's declared props (React.ComponentProps<'div'> vs React.ComponentProps<'kbd'>) match the actual rendered element (<kbd>).kits/assistant/legal/components/ui/carousel.tsx-96-105 (1)
96-105:⚠️ Potential issue | 🟡 MinorMissing cleanup for
reInitevent listener.The effect subscribes to both
reInitandselectevents but only unsubscribes fromselectin the cleanup function. This causes a memory leak if the component unmounts or the effect re-runs.🐛 Proposed fix
return () => { + api?.off('reInit', onSelect) api?.off('select', onSelect) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/carousel.tsx` around lines 96 - 105, The effect in React.useEffect within carousel.tsx subscribes to api events 'reInit' and 'select' but only removes the 'select' listener on cleanup; update the cleanup to remove both listeners by calling api.off('reInit', onSelect) and api.off('select', onSelect) (or api?.off for safety) so both subscriptions tied to onSelect are unsubscribed when api changes or component unmounts; locate the useEffect that references api, onSelect and the event names 'reInit' and 'select' to apply the change.kits/assistant/legal/styles/globals.css-4-4 (1)
4-4:⚠️ Potential issue | 🟡 MinorConfigure CSS linter to recognize Tailwind v4 directives.
The CSS linter setup (Biome and Stylelint) does not recognize Tailwind v4-specific directives (
@custom-variant,@theme,@apply). Lines 4, 77, 120, and 123 of this file will fail static analysis. Either add Tailwind support to the linter configuration or exclude this file from linting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/styles/globals.css` at line 4, The CSS linter is flagging Tailwind v4 directives (`@custom-variant`, `@theme`, `@apply`) in globals.css; update the linter so these directives are recognized or exclude this file from linting: either add Tailwind v4 support to Biome/Stylelint by enabling the Tailwind plugin/processor and relevant rule overrides, or add an ignore/glob entry for kits/assistant/legal/styles/globals.css (or add /* biome-disable *///* stylelint-disable */ comments guarded appropriately) so `@custom-variant`, `@theme` and `@apply` are not reported.kits/assistant/legal/components/ui/tooltip.tsx-21-27 (1)
21-27:⚠️ Potential issue | 🟡 MinorDon't create a new provider for every tooltip.
Line 25 wraps each
Tooltipin its ownTooltipProvider, which prevents tooltips from sharing provider-scoped timing state like skip-delay (the behavior where consecutive tooltips open instantly). Each provider instance maintains its own timing refs and timer, so nested providers break this shared behavior and prevent a parent provider from configuring tooltip delays centrally. ExportTooltipPrimitive.Rootdirectly and let the app place one provider higher in the tree.Suggested fix
function Tooltip({ ...props }: React.ComponentProps<typeof TooltipPrimitive.Root>) { - return ( - <TooltipProvider> - <TooltipPrimitive.Root data-slot="tooltip" {...props} /> - </TooltipProvider> - ) + return <TooltipPrimitive.Root data-slot="tooltip" {...props} /> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/tooltip.tsx` around lines 21 - 27, The Tooltip component currently wraps every instance in its own TooltipProvider which prevents shared provider-scoped timing state; remove the per-instance provider by stopping the use of TooltipProvider inside the Tooltip component and instead export/use TooltipPrimitive.Root directly (i.e., have Tooltip return or re-export TooltipPrimitive.Root with props and data-slot="tooltip"), so a single TooltipProvider can be placed at the app root to manage shared timing like skip-delay; update any exports to expose TooltipPrimitive.Root rather than a provider-wrapping wrapper.kits/assistant/legal/components/legal-ask-widget.tsx-128-131 (1)
128-131:⚠️ Potential issue | 🟡 MinorHandle
unknownseparately in the helper copy.The fallback branch currently labels
unknownas “Chat trigger detected”, which conflicts with the error set above and misleads the user.💡 Suggested fix
{triggerType === "askTriggerNode" ? "Ask trigger detected. Click the Ask button to open Lamatic Ask Widget." - : "Chat trigger detected. The Lamatic Chat Widget should appear automatically."} + : triggerType === "chatTriggerNode" + ? "Chat trigger detected. The Lamatic Chat Widget should appear automatically." + : "Unable to detect the exported flow trigger type."}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/legal-ask-widget.tsx` around lines 128 - 131, The helper copy currently conflates the fallback branch for triggerType with "Chat trigger detected", which mislabels the "unknown" case; update the conditional rendering in the component that uses triggerType (in legal-ask-widget.tsx) to handle "unknown" explicitly instead of falling through to the chat message — add a separate branch for triggerType === "unknown" with an appropriate message (e.g., an error or neutral guidance) and keep the existing messages for "askTriggerNode" and "chatTriggerNode" so the UI no longer misleads users.kits/assistant/legal/components/legal-ask-widget.tsx-17-25 (1)
17-25:⚠️ Potential issue | 🟡 MinorReset the widget status before each init attempt.
readysurvives across prop changes. If a previously loaded widget rerenders with missing config or an unknown trigger, the card displays both the old success state and the new error message.💡 Suggested fix
useEffect(() => { + setReady(false) + setError("") + if (!flowId || !apiUrl || !projectId) { setError("Missing Flow ID, API URL, or Project ID for Ask Widget initialization.") return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/legal-ask-widget.tsx` around lines 17 - 25, Reset the widget status at the start of the initialization effect so prior success state doesn't persist across rerenders: inside the useEffect that checks flowId, apiUrl, projectId and triggerType (the block containing useEffect, flowId, apiUrl, projectId, triggerType, setError), call the state setter that controls the ready/success flag (setReady) to false (or the initial status) before performing the validation checks so any previous ready state is cleared on each init attempt and only setReady(true) after a successful initialization.kits/assistant/legal/components/ui/use-toast.ts-152-161 (1)
152-161:⚠️ Potential issue | 🟡 MinorCompose
props.onOpenChangeinstead of replacing it.
toast()currently overwrites anyonOpenChangesupplied by the caller, so consumers cannot observe close/open transitions.💡 Suggested fix
open: true, onOpenChange: (open) => { + props.onOpenChange?.(open) if (!open) dismiss() },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/use-toast.ts` around lines 152 - 161, The ADD_TOAST dispatch is overwriting any caller-supplied props.onOpenChange; update the toast creation logic to compose the existing props.onOpenChange with the internal close behavior instead of replacing it: capture props.onOpenChange (if present), and in the dispatched toast.onOpenChange call first invoke the captured handler with the open value and then call dismiss() only when open becomes false. Modify the block that sets toast: { ...props, id, open: true, onOpenChange: (...) => { ... } } to call the original props.onOpenChange and then perform the internal dismiss when appropriate so consumers still receive open/close callbacks.kits/assistant/legal/components/ui/empty.tsx-46-59 (1)
46-59:⚠️ Potential issue | 🟡 MinorInconsistent
data-slotnaming.The component is named
EmptyMediabutdata-slot="empty-icon". For consistency with the component name and other patterns in this file, this should be"empty-media".🐛 Proposed fix
return ( <div - data-slot="empty-icon" + data-slot="empty-media" data-variant={variant} className={cn(emptyMediaVariants({ variant, className }))} {...props} /> )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/empty.tsx` around lines 46 - 59, Rename the data-slot on the EmptyMedia component for consistency: in the EmptyMedia function update the div attribute data-slot from "empty-icon" to "empty-media" so it matches the component name and the other patterns in this file; locate the EmptyMedia function and change the string value only (leave data-variant, className, props, and emptyMediaVariants usage intact).kits/assistant/legal/components/ui/chart.tsx-235-239 (1)
235-239:⚠️ Potential issue | 🟡 MinorFalsy check hides valid zero values.
item.value && (...)will not render whenitem.valueis0, which is a valid data point. Use an explicitundefined/nullcheck instead.🐛 Proposed fix
- {item.value && ( + {item.value !== undefined && item.value !== null && ( <span className="text-foreground font-mono font-medium tabular-nums"> {item.value.toLocaleString()} </span> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/chart.tsx` around lines 235 - 239, The conditional rendering currently uses "item.value && (...)" which skips valid zero values; update the check in the Chart UI where "item.value" is used (the span that displays {item.value.toLocaleString()}) to explicitly test for null/undefined (e.g., use "item.value !== null && item.value !== undefined" or "item.value != null") so 0 renders correctly while still hiding absent values; keep the rest of the span and formatting intact.kits/assistant/legal/components/ui/empty.tsx-71-82 (1)
71-82:⚠️ Potential issue | 🟡 MinorType/element mismatch: props typed as
<'p'>but renders<div>.
EmptyDescriptionacceptsReact.ComponentProps<'p'>but renders a<div>. This mismatch can cause subtle issues—consumers might expect paragraph semantics or pass<p>-specific attributes that won't validate correctly on a div.🐛 Proposed fix: Change to render a `
` or update the type
Option 1: Render a
<p>to match the type:function EmptyDescription({ className, ...props }: React.ComponentProps<'p'>) { return ( - <div + <p data-slot="empty-description" className={cn( 'text-muted-foreground [&>a:hover]:text-primary text-sm/relaxed [&>a]:underline [&>a]:underline-offset-4', className, )} {...props} /> + </p> ) }Option 2: Change the type to match the element:
-function EmptyDescription({ className, ...props }: React.ComponentProps<'p'>) { +function EmptyDescription({ className, ...props }: React.ComponentProps<'div'>) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/empty.tsx` around lines 71 - 82, EmptyDescription currently types its props as React.ComponentProps<'p'> but returns a <div>, causing a prop/element mismatch; fix by making the element and type consistent: either change the returned element to <p> inside the EmptyDescription component (so the signature React.ComponentProps<'p'> is correct) or update the prop type to React.ComponentProps<'div'> (and ensure any consumers expecting paragraph semantics are adjusted). Locate the EmptyDescription function and apply one of these two changes so the rendered element and the component prop type match.kits/assistant/legal/components/ui/pagination.tsx-107-115 (1)
107-115:⚠️ Potential issue | 🟡 MinorResolve the conflicting accessibility markup on the ellipsis.
aria-hiddenon the outer span also hides the innersr-onlytext, so assistive tech gets no cue that pages were skipped.💡 Suggested fix
- <span - aria-hidden - data-slot="pagination-ellipsis" - className={cn('flex size-9 items-center justify-center', className)} - {...props} - > - <MoreHorizontalIcon className="size-4" /> + <span + data-slot="pagination-ellipsis" + className={cn('flex size-9 items-center justify-center', className)} + {...props} + > + <MoreHorizontalIcon className="size-4" aria-hidden="true" /> <span className="sr-only">More pages</span> </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/pagination.tsx` around lines 107 - 115, The outer ellipsis element (data-slot="pagination-ellipsis") currently has aria-hidden which also hides the inner sr-only text, so remove aria-hidden from the outer <span> and instead mark only the decorative icon (<MoreHorizontalIcon>) as aria-hidden; keep the sr-only <span> inside the pagination-ellipsis element so screen readers receive the "More pages" cue and preserve existing className/props usage in the component.kits/assistant/legal/components/ui/command.tsx-46-58 (1)
46-58:⚠️ Potential issue | 🟡 MinorKeep the hidden dialog title inside
DialogContent.This header is mounted even while the dialog is closed because it sits outside the portaled content. That leaves stray screen-reader-only dialog copy in the page and separates the accessible name from the modal subtree.
💡 Suggested fix
return ( <Dialog {...props}> - <DialogHeader className="sr-only"> - <DialogTitle>{title}</DialogTitle> - <DialogDescription>{description}</DialogDescription> - </DialogHeader> <DialogContent className={cn('overflow-hidden p-0', className)} showCloseButton={showCloseButton} > + <DialogHeader className="sr-only"> + <DialogTitle>{title}</DialogTitle> + <DialogDescription>{description}</DialogDescription> + </DialogHeader> <Command className="[&_[cmdk-group-heading]]:text-muted-foreground **:data-[slot=command-input-wrapper]:h-12 [&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group]]:px-2 [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5"> {children}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/command.tsx` around lines 46 - 58, The hidden accessible header (DialogHeader with DialogTitle and DialogDescription) is rendered outside the portaled DialogContent causing stray screen-reader-only text and separating the dialog's accessible name from its modal subtree; move the DialogHeader (including DialogTitle and DialogDescription) inside DialogContent (before the Command) so the sr-only title/description live within the portaled modal subtree and remain associated with the dialog; update the JSX around DialogContent, DialogHeader, DialogTitle, and DialogDescription accordingly.kits/assistant/legal/.env.example-1-4 (1)
1-4:⚠️ Potential issue | 🟡 MinorRemove spaces around equal signs in .env format.
Most
.envparsers (includingdotenv) expectKEY=VALUEformat without spaces. Spaces around=may cause parsing issues or include literal spaces in values.🔧 Proposed fix
-ASSISTANT_LEGAL_ADVISOR = "ASSISTANT_LEGAL_ADVISOR Flow ID" -LAMATIC_API_URL = "LAMATIC_API_URL" -LAMATIC_PROJECT_ID = "LAMATIC_PROJECT_ID" -LAMATIC_API_KEY = "LAMATIC_API_KEY" +ASSISTANT_LEGAL_ADVISOR="your-flow-id-here" +LAMATIC_API_URL="https://your-lamatic-api-url" +LAMATIC_PROJECT_ID="your-project-id" +LAMATIC_API_KEY="your-api-key"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/.env.example` around lines 1 - 4, The .env example uses spaces around = which breaks typical dotenv parsing; update the four entries (ASSISTANT_LEGAL_ADVISOR, LAMATIC_API_URL, LAMATIC_PROJECT_ID, LAMATIC_API_KEY) to use KEY=VALUE without spaces (e.g., ASSISTANT_LEGAL_ADVISOR="ASSISTANT_LEGAL_ADVISOR Flow ID" or ASSISTANT_LEGAL_ADVISOR=ASSISTANT_LEGAL_ADVISOR_Flow_ID) so that dotenv and similar parsers read the keys and values correctly.kits/assistant/legal/app/layout.tsx-6-7 (1)
6-7:⚠️ Potential issue | 🟡 MinorApply the loaded
next/fontoutput to the document.The font loaders are initialized but never attached to
<html>, so the Geist fonts won't load. The body'sfont-sansclass references a Tailwind token that has no font family applied without the font variables present on the root element.Suggested fix
-const _geist = Geist({ subsets: ["latin"] }) -const _geistMono = Geist_Mono({ subsets: ["latin"] }) +const geist = Geist({ subsets: ["latin"], variable: "--font-geist-sans" }) +const geistMono = Geist_Mono({ subsets: ["latin"], variable: "--font-geist-mono" }) @@ - <html lang="en" suppressHydrationWarning> + <html + lang="en" + className={`${geist.variable} ${geistMono.variable}`} + suppressHydrationWarning + > <body className="font-sans antialiased" suppressHydrationWarning>Also applies to: 39-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/app/layout.tsx` around lines 6 - 7, The Geist fonts are initialized as _geist and _geistMono but their CSS variables aren't applied to the root element, so attach their generated className/variable outputs to the <html> element in the Layout component: locate the Layout function in layout.tsx and add the _geist.className and _geistMono.className (or their .variable names if using variable mode) to the html element's className/attributes so the font variables are present for the body’s font-sans Tailwind token to pick up.kits/assistant/legal/flows/assistant-legal-advisor/config.json-10-12 (1)
10-12:⚠️ Potential issue | 🟡 MinorWildcard domain allowlist may pose a security risk in production.
The
domains: ["*"]configuration allows the chat widget to be embedded on any website. For production deployments, consider restricting this to specific trusted domains to prevent unauthorized embedding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/flows/assistant-legal-advisor/config.json` around lines 10 - 12, The domains allowlist currently uses a wildcard ("domains": ["*"]); update the assistant-legal-advisor config.json so production uses a restricted list of trusted origins instead of "*": replace the "*" entry with an explicit array of allowed hostnames (e.g., ["https://app.example.com","https://admin.example.com"]) or wire the domains value to an environment-driven comma-separated variable (e.g., process/loader reads ALLOWED_WIDGET_DOMAINS and splits into the domains array), and keep "*" only behind a clear development flag so embedding is denied by default in production.kits/assistant/legal/components/ui/button-group.tsx-1-6 (1)
1-6:⚠️ Potential issue | 🟡 MinorMissing React import for type usage.
The file uses
React.ComponentPropson lines 28, 44, and 64, butReactis not imported. While the new JSX transform doesn't require React imports for JSX, theReactnamespace is still needed for type references.🐛 Proposed fix
+import * as React from 'react' import { Slot } from '@radix-ui/react-slot' import { cva, type VariantProps } from 'class-variance-authority' import { cn } from '@/lib/utils' import { Separator } from '@/components/ui/separator'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/button-group.tsx` around lines 1 - 6, The code uses the React namespace for types (React.ComponentProps) but never imports React; add a type-only import at the top such as "import type * as React from 'react'" so the React namespace is available for the type references used in this file (e.g., React.ComponentProps in the button group props/type definitions) without affecting the runtime bundle.kits/assistant/legal/hooks/use-toast.ts-174-182 (1)
174-182:⚠️ Potential issue | 🟡 MinorIncorrect useEffect dependency causes unnecessary re-subscriptions.
The dependency array includes
state, but the effect body only manages thesetStatelistener subscription. This causes the effect to re-run on every state change, repeatedly adding/removing the listener.🐛 Proposed fix
React.useEffect(() => { listeners.push(setState) return () => { const index = listeners.indexOf(setState) if (index > -1) { listeners.splice(index, 1) } } - }, [state]) + }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/hooks/use-toast.ts` around lines 174 - 182, The effect is re-subscribing on every state change because the dependency array includes state; change the useEffect dependency to an empty array so the listeners.push(setState) subscription runs only once and the cleanup removes the same setState; update the useEffect signature from useEffect(..., [state]) to useEffect(..., []) and keep the existing listener/cleanup logic referencing listeners and setState.kits/assistant/legal/components/ui/form.tsx-45-54 (1)
45-54:⚠️ Potential issue | 🟡 MinorContext guard is ineffective and checks too late.
The check
if (!fieldContext)on line 52 has two issues:
fieldContextis already destructured/used on lines 48-50 before this check- The context default is
{} as FormFieldContextValue, which is always truthy, so this condition never triggersThe error will never be thrown even when
useFormFieldis called outside a<FormField>.🐛 Proposed fix using null default
-const FormFieldContext = React.createContext<FormFieldContextValue>( - {} as FormFieldContextValue, -) +const FormFieldContext = React.createContext<FormFieldContextValue | null>(null) const useFormField = () => { const fieldContext = React.useContext(FormFieldContext) + + if (!fieldContext) { + throw new Error('useFormField should be used within <FormField>') + } + const itemContext = React.useContext(FormItemContext) const { getFieldState } = useFormContext() const formState = useFormState({ name: fieldContext.name }) const fieldState = getFieldState(fieldContext.name, formState) - if (!fieldContext) { - throw new Error('useFormField should be used within <FormField>') - } - const { id } = itemContext // ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/form.tsx` around lines 45 - 54, The guard in useFormField is ineffective because fieldContext is accessed (via fieldContext.name and passed to useFormState/getFieldState) before the null-check and the FormFieldContext default is an always-truthy {}. Update the provider/context type to allow null (make FormFieldContext default null) and change useFormField to read React.useContext(FormFieldContext) and immediately assert presence (throw if null) before using fieldContext; likewise ensure you check itemContext from FormItemContext before dereferencing; keep all subsequent calls to useFormState({ name: fieldContext.name }) and getFieldState(fieldContext.name, formState) after those null checks so the hook never dereferences an absent context.kits/assistant/legal/components/ui/sidebar.tsx-282-303 (1)
282-303:⚠️ Potential issue | 🟡 MinorDefault the button-based sidebar primitives to
type="button".These helpers render native buttons by default. If they ever sit inside a surrounding
<form>, clicks will submit it unless every call site remembers to overridetype.Suggested fix
-function SidebarRail({ className, ...props }: React.ComponentProps<'button'>) { +function SidebarRail({ + className, + type, + ...props +}: React.ComponentProps<'button'>) { @@ <button + type={type ?? 'button'} data-sidebar="rail" data-slot="sidebar-rail" @@ function SidebarGroupAction({ className, asChild = false, + type, ...props }: React.ComponentProps<'button'> & { asChild?: boolean }) { @@ <Comp + type={asChild ? undefined : type ?? 'button'} data-slot="sidebar-group-action" @@ function SidebarMenuButton({ asChild = false, + type, isActive = false, @@ }: React.ComponentProps<'button'> & { @@ <Comp + type={asChild ? undefined : type ?? 'button'} data-slot="sidebar-menu-button" @@ function SidebarMenuAction({ className, asChild = false, showOnHover = false, + type, ...props }: React.ComponentProps<'button'> & { @@ <Comp + type={asChild ? undefined : type ?? 'button'} data-slot="sidebar-menu-action"Apply the same pattern consistently to all four helpers.
Also applies to: 417-437, 498-523, 548-576
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/sidebar.tsx` around lines 282 - 303, The SidebarRail and the other three button-based sidebar helper components render native <button> elements but lack an explicit type, so when nested in a <form> they can trigger form submission; update SidebarRail and the other three helpers (the button primitives at the other ranges) to default the button to type="button" by supplying a default type prop on the rendered <button> (ensuring the explicit type is applied before spreading {...props} so callers can still override it) — locate the <button> in SidebarRail and the three analogous components and add type="button" as the default.kits/assistant/legal/components/ui/input-group.tsx-59-76 (1)
59-76:⚠️ Potential issue | 🟡 MinorCompose the addon click handler instead of replacing it.
The current handler only focuses
<input>, soInputGroupTextareanever gets focus, and a consumer-suppliedonClickwill override this logic entirely because{...props}comes afteronClick.Suggested fix
function InputGroupAddon({ className, align = 'inline-start', + onClick, ...props }: React.ComponentProps<'div'> & VariantProps<typeof inputGroupAddonVariants>) { return ( <div @@ - onClick={(e) => { - if ((e.target as HTMLElement).closest('button')) { + onClick={(e) => { + onClick?.(e) + if (e.defaultPrevented || (e.target as Element).closest('button')) { return } - e.currentTarget.parentElement?.querySelector('input')?.focus() + e.currentTarget.parentElement + ?.querySelector<HTMLElement>('[data-slot=input-group-control]') + ?.focus() }} {...props} /> ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/input-group.tsx` around lines 59 - 76, The click handler in InputGroupAddon currently overrides any consumer onClick and only focuses an <input>, preventing InputGroupTextarea focus; fix by destructuring onClick from props in the InputGroupAddon signature, then attach a composed onClick that first checks for a clicked button (same closest('button') guard), calls the original props.onClick if present, and finally focuses the nearest input or textarea (querySelector('input, textarea')); also ensure you still spread the remaining props (the rest) onto the div and do not let {...props} come after the composed onClick so the consumer handler is not overwritten.kits/assistant/legal/components/ui/sidebar.tsx-183-204 (1)
183-204:⚠️ Potential issue | 🟡 MinorThe mobile branch drops
classNameand div attributes.Desktop and non-collapsible modes honor
Sidebar's DOM props, but the mobile path sends...propsto<Sheet>and never appliesclassName. Styling hooks likeclassName,id, anddata-*disappear on mobile.Suggested fix
- return ( - <Sheet open={openMobile} onOpenChange={setOpenMobile} {...props}> + return ( + <Sheet open={openMobile} onOpenChange={setOpenMobile}> <SheetContent data-sidebar="sidebar" data-slot="sidebar" data-mobile="true" - className="bg-sidebar text-sidebar-foreground w-(--sidebar-width) p-0 [&>button]:hidden" + className={cn( + 'bg-sidebar text-sidebar-foreground w-(--sidebar-width) p-0 [&>button]:hidden', + className, + )} style={ { '--sidebar-width': SIDEBAR_WIDTH_MOBILE, } as React.CSSProperties } + {...props} side={side} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/sidebar.tsx` around lines 183 - 204, In the isMobile branch, the component spreads ...props onto <Sheet> but never forwards DOM props like className/id/data-* to the rendered sidebar, so Sidebar props are lost on mobile; update the mobile JSX to spread {...props} onto <SheetContent> (not just <Sheet>) and merge props.className with the existing SheetContent className (e.g., via a classNames utility or `${props.className ?? ''} bg-sidebar ...`) while preserving the current inline style and other attributes, ensuring children remain unchanged; reference the isMobile branch, openMobile, setOpenMobile, Sheet, and SheetContent symbols when locating where to forward and merge props.kits/assistant/legal/components/ui/sidebar.tsx-609-612 (1)
609-612:⚠️ Potential issue | 🟡 MinorAvoid
Math.random()during render in this client component.This
'use client'component can be pre-rendered on the server in Next.js. The random width will differ between server and client renders, causing hydration mismatch warnings and potential layout shifts.Use a deterministic seed instead:
Suggested fix
- const width = React.useMemo(() => { - return `${Math.floor(Math.random() * 40) + 50}%` - }, []) + const skeletonId = React.useId() + const width = React.useMemo(() => { + const seed = Array.from(skeletonId).reduce( + (total, char) => total + char.charCodeAt(0), + 0, + ) + return `${50 + (seed % 5) * 10}%` + }, [skeletonId])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/sidebar.tsx` around lines 609 - 612, The random width calculation using Math.random() inside the React.useMemo (the width constant) causes non-deterministic server vs client renders and hydration mismatches; replace it with a deterministic value derived from a stable seed (for example hash or modulo of a stable prop like an id, index, or other stable identifier passed into the component) or compute the random width only on the client after mount (e.g., set width inside a useEffect/useState once mounted). Update the React.useMemo("width") usage to use that deterministic calculation or client-only assignment so server and client renders match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e71beee0-5c27-4cf0-8d34-3e7b745663f2
⛔ Files ignored due to path filters (13)
kits/assistant/legal/package-lock.jsonis excluded by!**/package-lock.jsonkits/assistant/legal/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlkits/assistant/legal/public/apple-icon.pngis excluded by!**/*.pngkits/assistant/legal/public/icon-dark-32x32.pngis excluded by!**/*.pngkits/assistant/legal/public/icon-light-32x32.pngis excluded by!**/*.pngkits/assistant/legal/public/icon.svgis excluded by!**/*.svgkits/assistant/legal/public/lamatic-logo.pngis excluded by!**/*.pngkits/assistant/legal/public/legal-assistant-ui.pngis excluded by!**/*.pngkits/assistant/legal/public/placeholder-logo.pngis excluded by!**/*.pngkits/assistant/legal/public/placeholder-logo.svgis excluded by!**/*.svgkits/assistant/legal/public/placeholder-user.jpgis excluded by!**/*.jpgkits/assistant/legal/public/placeholder.jpgis excluded by!**/*.jpgkits/assistant/legal/public/placeholder.svgis excluded by!**/*.svg
📒 Files selected for processing (84)
README.mdkits/assistant/legal/.env.examplekits/assistant/legal/.gitignorekits/assistant/legal/README.mdkits/assistant/legal/actions/orchestrate.tskits/assistant/legal/app/globals.csskits/assistant/legal/app/layout.tsxkits/assistant/legal/app/page.tsxkits/assistant/legal/components.jsonkits/assistant/legal/components/header.tsxkits/assistant/legal/components/legal-ask-widget.tsxkits/assistant/legal/components/theme-provider.tsxkits/assistant/legal/components/ui/accordion.tsxkits/assistant/legal/components/ui/alert-dialog.tsxkits/assistant/legal/components/ui/alert.tsxkits/assistant/legal/components/ui/aspect-ratio.tsxkits/assistant/legal/components/ui/avatar.tsxkits/assistant/legal/components/ui/badge.tsxkits/assistant/legal/components/ui/breadcrumb.tsxkits/assistant/legal/components/ui/button-group.tsxkits/assistant/legal/components/ui/button.tsxkits/assistant/legal/components/ui/calendar.tsxkits/assistant/legal/components/ui/card.tsxkits/assistant/legal/components/ui/carousel.tsxkits/assistant/legal/components/ui/chart.tsxkits/assistant/legal/components/ui/checkbox.tsxkits/assistant/legal/components/ui/collapsible.tsxkits/assistant/legal/components/ui/command.tsxkits/assistant/legal/components/ui/context-menu.tsxkits/assistant/legal/components/ui/dialog.tsxkits/assistant/legal/components/ui/drawer.tsxkits/assistant/legal/components/ui/dropdown-menu.tsxkits/assistant/legal/components/ui/empty.tsxkits/assistant/legal/components/ui/field.tsxkits/assistant/legal/components/ui/form.tsxkits/assistant/legal/components/ui/hover-card.tsxkits/assistant/legal/components/ui/input-group.tsxkits/assistant/legal/components/ui/input-otp.tsxkits/assistant/legal/components/ui/input.tsxkits/assistant/legal/components/ui/item.tsxkits/assistant/legal/components/ui/kbd.tsxkits/assistant/legal/components/ui/label.tsxkits/assistant/legal/components/ui/menubar.tsxkits/assistant/legal/components/ui/navigation-menu.tsxkits/assistant/legal/components/ui/pagination.tsxkits/assistant/legal/components/ui/popover.tsxkits/assistant/legal/components/ui/progress.tsxkits/assistant/legal/components/ui/radio-group.tsxkits/assistant/legal/components/ui/resizable.tsxkits/assistant/legal/components/ui/scroll-area.tsxkits/assistant/legal/components/ui/select.tsxkits/assistant/legal/components/ui/separator.tsxkits/assistant/legal/components/ui/sheet.tsxkits/assistant/legal/components/ui/sidebar.tsxkits/assistant/legal/components/ui/skeleton.tsxkits/assistant/legal/components/ui/slider.tsxkits/assistant/legal/components/ui/sonner.tsxkits/assistant/legal/components/ui/spinner.tsxkits/assistant/legal/components/ui/switch.tsxkits/assistant/legal/components/ui/table.tsxkits/assistant/legal/components/ui/tabs.tsxkits/assistant/legal/components/ui/textarea.tsxkits/assistant/legal/components/ui/toast.tsxkits/assistant/legal/components/ui/toaster.tsxkits/assistant/legal/components/ui/toggle-group.tsxkits/assistant/legal/components/ui/toggle.tsxkits/assistant/legal/components/ui/tooltip.tsxkits/assistant/legal/components/ui/use-mobile.tsxkits/assistant/legal/components/ui/use-toast.tskits/assistant/legal/config.jsonkits/assistant/legal/flows/assistant-legal-advisor/README.mdkits/assistant/legal/flows/assistant-legal-advisor/config.jsonkits/assistant/legal/flows/assistant-legal-advisor/inputs.jsonkits/assistant/legal/flows/assistant-legal-advisor/meta.jsonkits/assistant/legal/hooks/use-mobile.tskits/assistant/legal/hooks/use-toast.tskits/assistant/legal/lib/lamatic-client.tskits/assistant/legal/lib/utils.tskits/assistant/legal/next.config.mjskits/assistant/legal/orchestrate.jskits/assistant/legal/package.jsonkits/assistant/legal/postcss.config.mjskits/assistant/legal/styles/globals.csskits/assistant/legal/tsconfig.json
Comment on lines
+9
to
+23
| function getTriggerType(): TriggerNodeId { | ||
| try { | ||
| const flowPath = join(process.cwd(), "flows", "assistant-legal-advisor", "config.json") | ||
| const parsed = JSON.parse(readFileSync(flowPath, "utf-8")) as { | ||
| nodes?: Array<{ data?: { nodeId?: string } }> | ||
| } | ||
| const nodeId = parsed.nodes?.find((node) => node?.data?.nodeId?.includes("TriggerNode"))?.data?.nodeId | ||
|
|
||
| if (nodeId === "askTriggerNode" || nodeId === "chatTriggerNode") { | ||
| return nodeId | ||
| } | ||
| return "unknown" | ||
| } catch { | ||
| return "unknown" | ||
| } |
Contributor
There was a problem hiding this comment.
Resolve the flow config relative to this file instead of process.cwd().
In workspace and Vercel builds, process.cwd() is usually the repo root, not kits/assistant/legal, so this lookup can miss the flow config, return "unknown", and silently configure the widget with the wrong trigger type.
Suggested fix
import { readFileSync } from "node:fs"
-import { join } from "node:path"
+import { fileURLToPath } from "node:url"
@@
function getTriggerType(): TriggerNodeId {
try {
- const flowPath = join(process.cwd(), "flows", "assistant-legal-advisor", "config.json")
+ const flowPath = fileURLToPath(
+ new URL("../flows/assistant-legal-advisor/config.json", import.meta.url)
+ )
const parsed = JSON.parse(readFileSync(flowPath, "utf-8")) as {
nodes?: Array<{ data?: { nodeId?: string } }>
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function getTriggerType(): TriggerNodeId { | |
| try { | |
| const flowPath = join(process.cwd(), "flows", "assistant-legal-advisor", "config.json") | |
| const parsed = JSON.parse(readFileSync(flowPath, "utf-8")) as { | |
| nodes?: Array<{ data?: { nodeId?: string } }> | |
| } | |
| const nodeId = parsed.nodes?.find((node) => node?.data?.nodeId?.includes("TriggerNode"))?.data?.nodeId | |
| if (nodeId === "askTriggerNode" || nodeId === "chatTriggerNode") { | |
| return nodeId | |
| } | |
| return "unknown" | |
| } catch { | |
| return "unknown" | |
| } | |
| import { readFileSync } from "node:fs" | |
| import { fileURLToPath } from "node:url" | |
| function getTriggerType(): TriggerNodeId { | |
| try { | |
| const flowPath = fileURLToPath( | |
| new URL("../flows/assistant-legal-advisor/config.json", import.meta.url) | |
| ) | |
| const parsed = JSON.parse(readFileSync(flowPath, "utf-8")) as { | |
| nodes?: Array<{ data?: { nodeId?: string } }> | |
| } | |
| const nodeId = parsed.nodes?.find((node) => node?.data?.nodeId?.includes("TriggerNode"))?.data?.nodeId | |
| if (nodeId === "askTriggerNode" || nodeId === "chatTriggerNode") { | |
| return nodeId | |
| } | |
| return "unknown" | |
| } catch { | |
| return "unknown" | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kits/assistant/legal/app/page.tsx` around lines 9 - 23, The code in
getTriggerType currently builds flowPath from process.cwd(), which can point to
the repo root; change it to resolve the flows/config.json relative to this
module file instead: compute the directory of this module (using
fileURLToPath(import.meta.url) and dirname from 'path' / 'url') and join that
base directory with "flows", "assistant-legal-advisor", "config.json" to form
flowPath; keep the rest of getTriggerType logic (parsing, checking nodeId for
"askTriggerNode" or "chatTriggerNode" and returning "unknown" in the catch)
intact so the function still returns a TriggerNodeId.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (10)
kits/assistant/legal/components/ui/input-group.tsx (1)
109-126: Consider documenting the size override strategy.This component omits
sizefrom Button's props and relies oninputGroupButtonVariantsclasses overriding Button's default size classes viaclassName. This works correctly withcn()using tailwind-merge, but the pattern is implicit.A brief comment noting the intentional override would improve maintainability if Button's implementation changes in the future.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/input-group.tsx` around lines 109 - 126, The InputGroupButton intentionally omits Button's "size" prop and forces size via data-size and className using inputGroupButtonVariants to override Button's default size classes; add a brief inline comment above the InputGroupButton function noting that "size" is intentionally omitted, that inputGroupButtonVariants combined with cn/tailwind-merge is used to override Button's size classes, and that this is deliberate to ensure consistent sizing for grouped inputs if Button's implementation changes.kits/assistant/legal/styles/globals.css (1)
1-124: Remove the unused second copy of the global stylesheet.
kits/assistant/legal/app/layout.tsxLine 4 imports./globals.css, andkits/assistant/legal/components.jsonLines 6-9 also points shadcn/ui atapp/globals.css. Keeping an identicalstyles/globals.cssleaves a second source of truth that can drift and doubles future theme/lint maintenance. Prefer deleting this file or turning it into a thin forwarder to../app/globals.css.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/styles/globals.css` around lines 1 - 124, There’s a duplicate global stylesheet; remove the extra copy (styles/globals.css) or turn it into a thin forwarder that simply imports the single canonical file (e.g. use an `@import` to the app globals) so there’s one source of truth; update/verify the import string './globals.css' used in layout.tsx and the components.json reference to 'app/globals.css' now point to the same canonical file.kits/assistant/legal/components/ui/chart.tsx (1)
187-189: Consider using a more robust React key.Using
item.dataKeyas the key could cause React warnings if multiple items share the samedataKeyor ifdataKeyis undefined. Using the loopindexas a fallback ensures uniqueness.Suggested fix
- <div - key={item.dataKey} + <div + key={item.dataKey ?? index}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/chart.tsx` around lines 187 - 189, The map rendering uses item.dataKey as the React key (see the return in the render loop that creates the <div key={item.dataKey}>), which can be undefined or duplicate; update the key to fall back to the loop index or a composite unique value (for example combine item.dataKey with the map index) to guarantee uniqueness and avoid React warnings—locate the map callback that returns the <div> and replace the single item.dataKey key with a fallback/composite key like `${item.dataKey ?? ''}-${index}`.kits/assistant/legal/components/legal-ask-widget.tsx (1)
145-153: Consider adding an accessible label for the Ask button.The button text "Ask" is brief. For better accessibility, consider adding an
aria-labelsuch asaria-label="Ask a legal question"to provide context for screen reader users.♿ Suggested improvement
<button id="lamatic-ask-widget-button" type="button" + aria-label="Ask a legal question" className="inline-flex h-12 items-center justify-center rounded-md bg-black px-6 text-sm font-medium text-white hover:bg-black/90" > Ask </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/legal-ask-widget.tsx` around lines 145 - 153, The Ask button rendered when triggerType === "askTriggerNode" (id "lamatic-ask-widget-button") lacks an accessible label; add an aria-label (for example aria-label="Ask a legal question") to the <button> element in the legal-ask-widget component so screen readers get context, and keep the visible text "Ask" unchanged; ensure the attribute is placed on the same button element guarded by the triggerType check (consider using a localized string if the app supports i18n).kits/assistant/legal/flows/assistant-legal-advisor/config.json (1)
17-17: External image URL may be unreliable.The
imageUrlreferences an external freepik.com asset. Consider hosting this image within the kit's assets or using a more reliable CDN to avoid broken images if the external resource becomes unavailable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/flows/assistant-legal-advisor/config.json` at line 17, The external image URL in the config.json's "imageUrl" property points to a Freepik asset which may break; replace it by adding the image to the kit's assets (or a reliable internal CDN) and update the "imageUrl" value to the new local/controlled asset URL, ensuring the asset is committed to the repository and any build/static-asset mapping (if used) will serve that path so the AssistantLegalAdvisor flow consistently loads the image.kits/assistant/legal/actions/orchestrate.ts (1)
59-76: Synchronous file I/O in server action may block the event loop.
existsSyncandreadFileSyncare blocking operations. While the files are small, consider using async alternatives (fs/promises) for consistency with server action best practices, especially if this code path is hit frequently.♻️ Suggested async alternative
+"use server" + +import { getLamaticClient } from "@/lib/lamatic-client" +import { config } from "../orchestrate.js" +import { access, readFile } from "node:fs/promises" +import { join } from "node:path" + -import { existsSync, readFileSync } from "node:fs" -import { join } from "node:path" - -function usesChatTriggerExport(): boolean { +async function usesChatTriggerExport(): Promise<boolean> { try { const candidates = [ join(process.cwd(), "flows", "assistant-legal-advisor", "config.json"), join(process.cwd(), "kits", "assistant", "legal", "flows", "assistant-legal-advisor", "config.json"), ] - const flowConfigPath = candidates.find((path) => existsSync(path)) + let flowConfigPath: string | undefined + for (const path of candidates) { + try { + await access(path) + flowConfigPath = path + break + } catch { + // continue to next candidate + } + } if (!flowConfigPath) { return false } - const raw = readFileSync(flowConfigPath, "utf-8") + const raw = await readFile(flowConfigPath, "utf-8") const parsed = JSON.parse(raw) as { nodes?: FlowNode[] } const nodes = Array.isArray(parsed.nodes) ? parsed.nodes : [] return nodes.some((node) => node?.data?.nodeId === "chatTriggerNode") } catch { return false } }Then update the call site at line 88 to
await usesChatTriggerExport().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/actions/orchestrate.ts` around lines 59 - 76, Convert the synchronous function usesChatTriggerExport to an async function that uses fs/promises (e.g., access/stat and readFile) instead of existsSync/readFileSync, return a Promise<boolean>, and preserve the same logic (parse JSON, check nodes for data.nodeId === "chatTriggerNode"); then update every call site that invokes usesChatTriggerExport() to await its result so the async behavior is handled correctly.kits/assistant/legal/hooks/use-toast.ts (1)
76-80: Replacing a toast should go through the dismiss path.Line 79 truncates older entries immediately, so an evicted toast never gets
open: false, itsonOpenChangecallback, or the normal removal flow. If two toasts can fire back-to-back, dismiss or replace the existing toast explicitly before slicing the array.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/hooks/use-toast.ts` around lines 76 - 80, The ADD_TOAST branch in the reducer (case actionTypes.ADD_TOAST) currently prepends the new toast then slices by TOAST_LIMIT, which can evict an existing toast without running its dismissal flow; change this so that when adding a toast would exceed TOAST_LIMIT you explicitly dismiss/replace the oldest toast (e.g., set its open to false or invoke its onOpenChange/dismiss handler) before inserting the new toast, using the existing dismiss logic (dismissToast or DISMISS_TOAST) instead of silently slicing state.toasts so the normal onOpenChange/onClose lifecycle always runs.kits/assistant/legal/components/ui/use-toast.ts (1)
1-193: Consolidate duplicate store implementation.Both
kits/assistant/legal/hooks/use-toast.tsandkits/assistant/legal/components/ui/use-toast.tsare identical. Currently,toaster.tsximports from the hooks path, but the duplicate in components/ui is dead code that should be replaced with a re-export to prevent future developers from accidentally importing from the wrong path and creating separate store instances.Suggested fix
'use client' export { reducer, toast, useToast } from '@/hooks/use-toast'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/use-toast.ts` around lines 1 - 193, The file duplicates the toast store implementation; replace its implementation with a simple re-export of the single source of truth: export the reducer, toast, and useToast symbols from the hooks implementation (keep the 'use client' directive at top) so consumers import the same store instance and avoid creating separate stores.kits/assistant/legal/components/ui/carousel.tsx (1)
14-21:opts.axisis part of the type, but this component always ignores it.
orientationis defaulted before the Embla options are built, so Lines 55-58 overwrite any incomingopts.axis. That also makes the fallback at Lines 114-115 unreachable, which is misleading for a public API.Proposed fix
-type CarouselOptions = UseCarouselParameters[0] +type CarouselOptions = NonNullable<UseCarouselParameters[0]> type CarouselProps = { - opts?: CarouselOptions + opts?: Omit<CarouselOptions, 'axis'> plugins?: CarouselPlugin orientation?: 'horizontal' | 'vertical' setApi?: (api: CarouselApi) => void } @@ - orientation: - orientation || (opts?.axis === 'y' ? 'vertical' : 'horizontal'), + orientation,Also applies to: 45-58, 109-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/carousel.tsx` around lines 14 - 21, The component currently ignores opts.axis by overriding it with the local orientation before building Embla options; update the code that constructs the Embla options to respect an explicitly passed opts.axis and only derive axis from the component prop orientation when opts.axis is undefined. Specifically, in the logic around CarouselProps, opts, and where Embla options are assembled (the code that reads orientation and sets opts.axis and the fallback at the end around the axis check), change the precedence so opts.axis (if present) wins, otherwise compute axis from orientation, and remove or rework the unreachable fallback so the public API behaves as documented.kits/assistant/legal/components/ui/sidebar.tsx (1)
35-43: Widen the exported sidebar setters to real state-setter types.
toggleSidebar()already uses updater callbacks on Lines 92-94, butSidebarContextPropsnarrowssetOpenandsetOpenMobileto boolean-only functions. That makesuseSidebar()misleading for consumers and forces them to readopeneagerly just to toggle state.Suggested fix
type SidebarContextProps = { state: 'expanded' | 'collapsed' open: boolean - setOpen: (open: boolean) => void + setOpen: React.Dispatch<React.SetStateAction<boolean>> openMobile: boolean - setOpenMobile: (open: boolean) => void + setOpenMobile: React.Dispatch<React.SetStateAction<boolean>> isMobile: boolean toggleSidebar: () => void }Also applies to: 92-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/sidebar.tsx` around lines 35 - 43, The SidebarContextProps type narrows setOpen and setOpenMobile to (open: boolean) => void which prevents consumers from using updater callbacks; change both setOpen and setOpenMobile to accept React-style state setters (e.g. React.Dispatch<React.SetStateAction<boolean>> or (value: boolean | (prev: boolean) => boolean) => void) so toggleSidebar and useSidebar callers can pass callback updaters; update the exported type and any related usage signatures to the widened setter type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@kits/assistant/legal/components/ui/button-group.tsx`:
- Around line 25-37: The ButtonGroup component renders
data-orientation={orientation} while orientation can be undefined, causing DOM
mismatch; set a default orientation before use (e.g., in the function signature
or via a local const) so orientation is always a valid value when passed to
data-orientation and to buttonGroupVariants; update the ButtonGroup parameter or
initialize a local variable so that orientation defaults to the horizontal
variant and then use that normalized value in both buttonGroupVariants(...) and
data-orientation.
In `@kits/assistant/legal/components/ui/carousel.tsx`:
- Around line 175-197: Carousel button components (CarouselPrevious and the
corresponding CarouselNext) spread {...props} after the internal onClick and
disabled, allowing consumer props to override and disable internal scrolling;
fix by destructuring onClick and disabled from props in the CarouselPrevious and
CarouselNext functions, call the internal handlers (scrollPrev/scrollNext) first
then invoke the consumer onClick if provided (e.g., const { onClick, disabled,
...rest } = props; onClickWrapper = (e) => { if (!internalCanScroll) return;
internalScroll(); onClick?.(e); }), and spread the remaining rest props onto
<Button> so consumer handlers are preserved without overriding the
component-controlled disabled state (ensure you use the controlled
disabled={!canScrollPrev/Next} rather than consumer disabled).
- Around line 78-89: The root key handler handleKeyDown currently always
intercepts arrow keys (wired on root in capture phase), which prevents nested
inputs from getting their native behavior and ignores vertical orientation;
update handleKeyDown to first guard by ignoring events whose target is an
editable element (input, textarea, select, contentEditable=true) or has
role="textbox"/isContentEditable, returning early so nested controls retain
native keys, and then map keys based on the orientation prop: if orientation ===
"vertical" respond to 'ArrowUp' -> scrollPrev and 'ArrowDown' -> scrollNext;
otherwise (horizontal) respond to 'ArrowLeft' -> scrollPrev and 'ArrowRight' ->
scrollNext; keep event.preventDefault() only when you actually handle the key
and rely on the existing scrollPrev/scrollNext callbacks.
In `@kits/assistant/legal/components/ui/form.tsx`:
- Around line 114-118: FormControl's aria-describedby currently includes
formMessageId only when error is truthy, which hides non-error FormMessage
content from assistive tech; update the aria-describedby computation in the
FormControl component (the JSX using formDescriptionId, formMessageId and error)
to always include formMessageId (e.g., `${formDescriptionId} ${formMessageId}`)
so helper text rendered by FormMessage is exposed to assistive technologies;
apply the same change to the other similar block that uses these IDs (lines
handling the second aria-describedby instance).
In `@kits/assistant/legal/components/ui/sidebar.tsx`:
- Around line 154-160: The Sidebar component currently doesn't extract the style
prop, so inline styles from callers overwrite the component's CSS-variable style
objects (including the mobile drawer width). Update the Sidebar function
signature to extract style from props and, in every render path (the left/right
sidebar branches, the default sidebar branch, and the mobile/offcanvas drawer
render), merge caller styles by spreading the extracted style into the
component-created style objects that set the CSS variables (so caller styles are
applied without dropping the CSS-variable declarations that control
width/behavior).
- Around line 286-312: The SidebarRail component lets a caller-provided onClick
in ...props override the internal toggleSidebar, so destructure onClick from
props and compose a new handler that calls toggleSidebar(event) and then calls
the original onClick(event) if present (matching the pattern used in
SidebarTrigger); pass that composed handler to the button as onClick and spread
the remaining props (without the original onClick) so caller handlers are
preserved but do not disable toggleSidebar.
- Around line 97-109: The global keydown handler (handleKeyDown inside the
React.useEffect) currently toggles the sidebar on Cmd/Ctrl+B without checking
focus, so it should early-return when the focused element is editable: inspect
event.target (or document.activeElement) and skip handling if it's an input,
textarea, select, or any element with isContentEditable true (check via (target
as HTMLElement)?.isContentEditable or tagName checks). Update handleKeyDown
(used with SIDEBAR_KEYBOARD_SHORTCUT and toggleSidebar) to perform this guard
before calling event.preventDefault()/toggleSidebar, and apply the same change
to the other duplicated sidebar handlers across the codebase.
In `@kits/assistant/legal/flows/assistant-legal-advisor/config.json`:
- Around line 10-13: The config.json contains a placeholder entry in the
"domains" array ("https://your-domain.vercel.app"); update the "domains" array
to include your actual deployment origin(s) or remove the placeholder, and
ensure any deployment documentation clearly instructs users to replace that
placeholder before going to production so the domains list used by the
assistant-legal-advisor config is accurate.
In `@kits/assistant/legal/lib/lamatic-client.ts`:
- Around line 4-22: The getLamaticClient function is passing null for projectId
which the Lamatic constructor rejects; update the object passed to new Lamatic
so projectId uses the same empty-string fallback as endpoint and apiKey (i.e.,
replace config.api.projectId ?? null with config.api.projectId ?? "") to ensure
a non-null string is always provided when constructing Lamatic.
---
Nitpick comments:
In `@kits/assistant/legal/actions/orchestrate.ts`:
- Around line 59-76: Convert the synchronous function usesChatTriggerExport to
an async function that uses fs/promises (e.g., access/stat and readFile) instead
of existsSync/readFileSync, return a Promise<boolean>, and preserve the same
logic (parse JSON, check nodes for data.nodeId === "chatTriggerNode"); then
update every call site that invokes usesChatTriggerExport() to await its result
so the async behavior is handled correctly.
In `@kits/assistant/legal/components/legal-ask-widget.tsx`:
- Around line 145-153: The Ask button rendered when triggerType ===
"askTriggerNode" (id "lamatic-ask-widget-button") lacks an accessible label; add
an aria-label (for example aria-label="Ask a legal question") to the <button>
element in the legal-ask-widget component so screen readers get context, and
keep the visible text "Ask" unchanged; ensure the attribute is placed on the
same button element guarded by the triggerType check (consider using a localized
string if the app supports i18n).
In `@kits/assistant/legal/components/ui/carousel.tsx`:
- Around line 14-21: The component currently ignores opts.axis by overriding it
with the local orientation before building Embla options; update the code that
constructs the Embla options to respect an explicitly passed opts.axis and only
derive axis from the component prop orientation when opts.axis is undefined.
Specifically, in the logic around CarouselProps, opts, and where Embla options
are assembled (the code that reads orientation and sets opts.axis and the
fallback at the end around the axis check), change the precedence so opts.axis
(if present) wins, otherwise compute axis from orientation, and remove or rework
the unreachable fallback so the public API behaves as documented.
In `@kits/assistant/legal/components/ui/chart.tsx`:
- Around line 187-189: The map rendering uses item.dataKey as the React key (see
the return in the render loop that creates the <div key={item.dataKey}>), which
can be undefined or duplicate; update the key to fall back to the loop index or
a composite unique value (for example combine item.dataKey with the map index)
to guarantee uniqueness and avoid React warnings—locate the map callback that
returns the <div> and replace the single item.dataKey key with a
fallback/composite key like `${item.dataKey ?? ''}-${index}`.
In `@kits/assistant/legal/components/ui/input-group.tsx`:
- Around line 109-126: The InputGroupButton intentionally omits Button's "size"
prop and forces size via data-size and className using inputGroupButtonVariants
to override Button's default size classes; add a brief inline comment above the
InputGroupButton function noting that "size" is intentionally omitted, that
inputGroupButtonVariants combined with cn/tailwind-merge is used to override
Button's size classes, and that this is deliberate to ensure consistent sizing
for grouped inputs if Button's implementation changes.
In `@kits/assistant/legal/components/ui/sidebar.tsx`:
- Around line 35-43: The SidebarContextProps type narrows setOpen and
setOpenMobile to (open: boolean) => void which prevents consumers from using
updater callbacks; change both setOpen and setOpenMobile to accept React-style
state setters (e.g. React.Dispatch<React.SetStateAction<boolean>> or (value:
boolean | (prev: boolean) => boolean) => void) so toggleSidebar and useSidebar
callers can pass callback updaters; update the exported type and any related
usage signatures to the widened setter type.
In `@kits/assistant/legal/components/ui/use-toast.ts`:
- Around line 1-193: The file duplicates the toast store implementation; replace
its implementation with a simple re-export of the single source of truth: export
the reducer, toast, and useToast symbols from the hooks implementation (keep the
'use client' directive at top) so consumers import the same store instance and
avoid creating separate stores.
In `@kits/assistant/legal/flows/assistant-legal-advisor/config.json`:
- Line 17: The external image URL in the config.json's "imageUrl" property
points to a Freepik asset which may break; replace it by adding the image to the
kit's assets (or a reliable internal CDN) and update the "imageUrl" value to the
new local/controlled asset URL, ensuring the asset is committed to the
repository and any build/static-asset mapping (if used) will serve that path so
the AssistantLegalAdvisor flow consistently loads the image.
In `@kits/assistant/legal/hooks/use-toast.ts`:
- Around line 76-80: The ADD_TOAST branch in the reducer (case
actionTypes.ADD_TOAST) currently prepends the new toast then slices by
TOAST_LIMIT, which can evict an existing toast without running its dismissal
flow; change this so that when adding a toast would exceed TOAST_LIMIT you
explicitly dismiss/replace the oldest toast (e.g., set its open to false or
invoke its onOpenChange/dismiss handler) before inserting the new toast, using
the existing dismiss logic (dismissToast or DISMISS_TOAST) instead of silently
slicing state.toasts so the normal onOpenChange/onClose lifecycle always runs.
In `@kits/assistant/legal/styles/globals.css`:
- Around line 1-124: There’s a duplicate global stylesheet; remove the extra
copy (styles/globals.css) or turn it into a thin forwarder that simply imports
the single canonical file (e.g. use an `@import` to the app globals) so there’s
one source of truth; update/verify the import string './globals.css' used in
layout.tsx and the components.json reference to 'app/globals.css' now point to
the same canonical file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc0e5eef-361d-4a63-98df-570acb245215
⛔ Files ignored due to path filters (1)
kits/assistant/legal/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
kits/assistant/legal/.env.examplekits/assistant/legal/.gitignorekits/assistant/legal/README.mdkits/assistant/legal/actions/orchestrate.tskits/assistant/legal/app/globals.csskits/assistant/legal/app/layout.tsxkits/assistant/legal/app/page.tsxkits/assistant/legal/components/legal-ask-widget.tsxkits/assistant/legal/components/ui/button-group.tsxkits/assistant/legal/components/ui/carousel.tsxkits/assistant/legal/components/ui/chart.tsxkits/assistant/legal/components/ui/command.tsxkits/assistant/legal/components/ui/empty.tsxkits/assistant/legal/components/ui/form.tsxkits/assistant/legal/components/ui/input-group.tsxkits/assistant/legal/components/ui/kbd.tsxkits/assistant/legal/components/ui/navigation-menu.tsxkits/assistant/legal/components/ui/pagination.tsxkits/assistant/legal/components/ui/progress.tsxkits/assistant/legal/components/ui/sidebar.tsxkits/assistant/legal/components/ui/tooltip.tsxkits/assistant/legal/components/ui/use-toast.tskits/assistant/legal/eslint.config.mjskits/assistant/legal/flows/assistant-legal-advisor/config.jsonkits/assistant/legal/hooks/use-toast.tskits/assistant/legal/lib/lamatic-client.tskits/assistant/legal/next.config.mjskits/assistant/legal/orchestrate.jskits/assistant/legal/package.jsonkits/assistant/legal/styles/globals.css
🚧 Files skipped from review as they are similar to previous changes (11)
- kits/assistant/legal/.gitignore
- kits/assistant/legal/app/page.tsx
- kits/assistant/legal/orchestrate.js
- kits/assistant/legal/app/layout.tsx
- kits/assistant/legal/components/ui/pagination.tsx
- kits/assistant/legal/components/ui/kbd.tsx
- kits/assistant/legal/package.json
- kits/assistant/legal/README.md
- kits/assistant/legal/components/ui/command.tsx
- kits/assistant/legal/next.config.mjs
- kits/assistant/legal/components/ui/empty.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
kits/assistant/legal/hooks/use-toast.ts (2)
56-72: Consider clearing timeout when toast is explicitly removed.When a toast is removed via
REMOVE_TOAST, the scheduled timeout intoastTimeoutsis not cleared. While this is harmless (the timeout will dispatch a no-op remove), it's slightly wasteful.♻️ Optional: Clear timeout on explicit removal
In the
REMOVE_TOASTcase of the reducer (or in a separate cleanup), consider clearing the timeout:// Helper to clear timeout const clearToastTimeout = (toastId: string) => { const timeout = toastTimeouts.get(toastId) if (timeout) { clearTimeout(timeout) toastTimeouts.delete(toastId) } }Then call
clearToastTimeout(action.toastId)in theREMOVE_TOASTcase before filtering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/hooks/use-toast.ts` around lines 56 - 72, The reducer never clears the scheduled timeout when handling REMOVE_TOAST, so add a helper function clearToastTimeout(toastId: string) that looks up toastTimeouts, calls clearTimeout on the stored timer and deletes the entry, then invoke clearToastTimeout(action.toastId) inside the REMOVE_TOAST case before you filter/remove the toast; this helper should reuse the existing toastTimeouts Map used by addToRemoveQueue and ensure timers are cleaned up when a toast is explicitly removed.
164-168: Minor:updatefunction type could be improved.The
updatefunction requiresToasterToast(which includesid), but then overwrites theidfrom the closure. Callers must provide anidthat gets ignored.♻️ Optional: Use partial type for update
- const update = (props: ToasterToast) => + const update = (props: Partial<Omit<ToasterToast, 'id'>>) => dispatch({ type: 'UPDATE_TOAST', toast: { ...props, id }, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/hooks/use-toast.ts` around lines 164 - 168, The update function currently accepts a full ToasterToast (which includes id) but then ignores the passed id and replaces it with the closure id; change the parameter type to avoid requiring/allowing an id — e.g., use Partial<ToasterToast> or Omit<ToasterToast, 'id'> for the update(props) signature — then keep the dispatch call using { ...props, id } (reference: update, ToasterToast, id, dispatch, action type 'UPDATE_TOAST'); update any callers to pass only the updatable fields (no id).kits/assistant/legal/components/ui/sidebar.tsx (1)
290-293: Consider checkingdefaultPreventedfor consistency withSidebarRail.
SidebarRail(lines 317-321) checksevent.defaultPreventedbefore callingtoggleSidebar(), butSidebarTriggerdoes not. This inconsistency means a caller'sonClickthat callsevent.preventDefault()would prevent toggle onSidebarRailbut not onSidebarTrigger.Suggested fix for consistency
onClick={(event) => { onClick?.(event) - toggleSidebar() + if (!event.defaultPrevented) { + toggleSidebar() + } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/components/ui/sidebar.tsx` around lines 290 - 293, The onClick handler in SidebarTrigger currently always calls toggleSidebar() after invoking onClick, causing inconsistency with SidebarRail which checks event.defaultPrevented; update the SidebarTrigger onClick handler (the anonymous function bound to onClick in sidebar.tsx) to call onClick?.(event) first and only call toggleSidebar() if !event.defaultPrevented, matching the behavior in SidebarRail so a caller that calls event.preventDefault() can suppress the toggle.kits/assistant/legal/actions/orchestrate.ts (1)
59-103: Clarify the relationship betweenorchestrate.tsand the shipped flow config.The
usesChatTriggerExport()guard (lines 59-87) detects the flow config'schatTriggerNodeand throws at lines 99-103, makinggetLegalGuidance()unusable with the providedflows/assistant-legal-advisor/config.json.This appears intentional—the kit offers two execution paths:
- Client-side widget (
legal-ask-widget.tsx): Works with the shippedchatTriggerNodeflow- Server-side orchestration (
orchestrate.ts): Requires a separate API Request trigger flowHowever, since
getLegalGuidance()is exported but will always fail with the bundled flow, consider either:
- Adding a brief code comment explaining this is for API Request trigger flows only
- Or documenting this distinction in the README under "Usage"
📝 Example comment addition
+/** + * Server-side legal guidance orchestration. + * Requires an API Request trigger flow (not Chat Widget trigger). + * The bundled flows/assistant-legal-advisor uses Chat Widget trigger + * and is designed for client-side widget usage via legal-ask-widget.tsx. + */ export async function getLegalGuidance(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kits/assistant/legal/actions/orchestrate.ts` around lines 59 - 103, Add a short clarifying comment above the usesChatTriggerExport() and getLegalGuidance() functions explaining that the bundled flows/assistant-legal-advisor/config.json uses a chatTriggerNode for client-side widget usage (legal-ask-widget.tsx) and therefore getLegalGuidance() is intended only for server-side/API Request trigger flows; also update the README "Usage" to document this distinction and instruct consumers to deploy an API Request trigger flow or replace the shipped config if they want to call getLegalGuidance() from the server.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@kits/assistant/legal/components/ui/carousel.tsx`:
- Around line 45-53: The Carousel component currently spreads {...props} after
setting onKeyDown={handleKeyDown}, which allows a consumer-provided onKeyDown to
override the built-in keyboard handling; fix this by destructuring onKeyDown
from the incoming props in the Carousel signature (e.g., const { onKeyDown:
userOnKeyDown, ...rest } = props) and then compose a root onKeyDown that calls
handleKeyDown(e) first (or vice versa if desired) and then calls
userOnKeyDown?.(e) if present, ensuring you respect e.defaultPrevented; apply
the same destructure-and-compose pattern for the other instance referenced by
handleKeyDown at the second occurrence (the 140-146 spot) so consumer handlers
do not replace internal arrow-key navigation.
In `@kits/assistant/legal/components/ui/chart.tsx`:
- Around line 82-100: The inline stylesheet built with dangerouslySetInnerHTML
in the ChartContainer component is vulnerable because unescaped id, theme
prefixes and colorConfig values are interpolated into an unquoted CSS attribute
selector and into CSS text; fix by removing raw HTML construction or sanitizing
inputs: use CSS.escape(id) (or wrap the attribute value in quotes and escape
quotes) when constructing the selector for THEME entries, ensure theme prefix
strings from THEMES are validated/whitelisted before use, and validate/normalize
colorConfig keys and itemConfig.color values against a strict whitelist/regex
(e.g., allow only safe hex, rgb/rgba or CSS variable formats) falling back to a
safe default if invalid; alternatively avoid dangerouslySetInnerHTML entirely by
generating className-based styles via a CSS-in-JS approach or applying styles
via inline style objects to the rendered elements (update the code paths around
ChartContainer, the dangerouslySetInnerHTML block, THEMES, and
colorConfig/itemConfig usages).
- Around line 326-343: The code uses the `in` operator which walks the prototype
chain; update all property checks to use own-property checks via
Object.prototype.hasOwnProperty.call() — replace the `in` checks on `payload`
and `payloadPayload` and the final `configLabelKey in config` check with
Object.prototype.hasOwnProperty.call(payload, key),
Object.prototype.hasOwnProperty.call(payloadPayload, key), and
Object.prototype.hasOwnProperty.call(config, configLabelKey) respectively
(retain the existing typeof guards and casts for `configLabelKey`, `payload`,
`payloadPayload`, and `config`).
- Around line 278-300: The legend rendering currently uses only
itemConfig?.label so when ChartConfig.label is omitted some legend entries show
no text; update the legend fallback in the payload.map render (where nameKey,
getPayloadConfigFromPayload, itemConfig are used) to display itemConfig?.label
|| item.name (or nameKey-derived value) instead of just itemConfig?.label so
series without a configured label fall back to item.name for consistency with
the tooltip behavior.
In `@kits/assistant/legal/components/ui/form.tsx`:
- Around line 134-149: The FormMessage component is always applying the
'text-destructive' class even when there is no error; update the className
composition in FormMessage so that 'text-destructive' is only included when the
local error variable is truthy (use the error value from useFormField()),
keeping other classes ('text-sm' and any incoming className) unchanged and still
passing id={formMessageId} and {...props}; locate the FormMessage function to
change the cn call where className is built so helper/assistive text uses normal
styling when error is falsy.
- Around line 107-116: The component spreads {...props} after computed
accessibility attributes so a consumer can override id/aria and break the ARIA
wiring; update FormControl so the consumer props cannot overwrite those
generated values by either moving {...props} before the explicit id/aria
attributes in the Slot JSX or by destructuring props to omit id,
aria-describedby, and aria-invalid before spreading, ensuring the computed
values from useFormField() (formItemId, formDescriptionId, formMessageId, error)
always take precedence.
---
Nitpick comments:
In `@kits/assistant/legal/actions/orchestrate.ts`:
- Around line 59-103: Add a short clarifying comment above the
usesChatTriggerExport() and getLegalGuidance() functions explaining that the
bundled flows/assistant-legal-advisor/config.json uses a chatTriggerNode for
client-side widget usage (legal-ask-widget.tsx) and therefore getLegalGuidance()
is intended only for server-side/API Request trigger flows; also update the
README "Usage" to document this distinction and instruct consumers to deploy an
API Request trigger flow or replace the shipped config if they want to call
getLegalGuidance() from the server.
In `@kits/assistant/legal/components/ui/sidebar.tsx`:
- Around line 290-293: The onClick handler in SidebarTrigger currently always
calls toggleSidebar() after invoking onClick, causing inconsistency with
SidebarRail which checks event.defaultPrevented; update the SidebarTrigger
onClick handler (the anonymous function bound to onClick in sidebar.tsx) to call
onClick?.(event) first and only call toggleSidebar() if !event.defaultPrevented,
matching the behavior in SidebarRail so a caller that calls
event.preventDefault() can suppress the toggle.
In `@kits/assistant/legal/hooks/use-toast.ts`:
- Around line 56-72: The reducer never clears the scheduled timeout when
handling REMOVE_TOAST, so add a helper function clearToastTimeout(toastId:
string) that looks up toastTimeouts, calls clearTimeout on the stored timer and
deletes the entry, then invoke clearToastTimeout(action.toastId) inside the
REMOVE_TOAST case before you filter/remove the toast; this helper should reuse
the existing toastTimeouts Map used by addToRemoveQueue and ensure timers are
cleaned up when a toast is explicitly removed.
- Around line 164-168: The update function currently accepts a full ToasterToast
(which includes id) but then ignores the passed id and replaces it with the
closure id; change the parameter type to avoid requiring/allowing an id — e.g.,
use Partial<ToasterToast> or Omit<ToasterToast, 'id'> for the update(props)
signature — then keep the dispatch call using { ...props, id } (reference:
update, ToasterToast, id, dispatch, action type 'UPDATE_TOAST'); update any
callers to pass only the updatable fields (no id).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10ade522-9514-46cb-9fc7-ccc479a6d1aa
📒 Files selected for processing (14)
kits/assistant/legal/README.mdkits/assistant/legal/actions/orchestrate.tskits/assistant/legal/components/legal-ask-widget.tsxkits/assistant/legal/components/ui/button-group.tsxkits/assistant/legal/components/ui/carousel.tsxkits/assistant/legal/components/ui/chart.tsxkits/assistant/legal/components/ui/form.tsxkits/assistant/legal/components/ui/input-group.tsxkits/assistant/legal/components/ui/sidebar.tsxkits/assistant/legal/components/ui/use-toast.tskits/assistant/legal/flows/assistant-legal-advisor/config.jsonkits/assistant/legal/hooks/use-toast.tskits/assistant/legal/lib/lamatic-client.tskits/assistant/legal/styles/globals.css
🚧 Files skipped from review as they are similar to previous changes (4)
- kits/assistant/legal/components/ui/button-group.tsx
- kits/assistant/legal/styles/globals.css
- kits/assistant/legal/components/ui/input-group.tsx
- kits/assistant/legal/lib/lamatic-client.ts
Comment on lines
+45
to
+53
| function Carousel({ | ||
| orientation = 'horizontal', | ||
| opts, | ||
| setApi, | ||
| plugins, | ||
| className, | ||
| children, | ||
| ...props | ||
| }: React.ComponentProps<'div'> & CarouselProps) { |
Contributor
There was a problem hiding this comment.
Compose the root onKeyDown instead of letting props replace it.
{...props} is spread after onKeyDown={handleKeyDown}, so any consumer-provided onKeyDown disables the carousel's built-in arrow-key navigation entirely. Destructure onKeyDown from props and invoke both handlers.
Proposed fix
function Carousel({
orientation = 'horizontal',
opts,
setApi,
plugins,
className,
children,
+ onKeyDown,
...props
}: React.ComponentProps<'div'> & CarouselProps) {
@@
<div
- onKeyDown={handleKeyDown}
+ onKeyDown={(event) => {
+ onKeyDown?.(event)
+ if (!event.defaultPrevented) {
+ handleKeyDown(event)
+ }
+ }}
className={cn('relative', className)}
role="region"
aria-roledescription="carousel"
data-slot="carousel"
{...props}Also applies to: 140-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kits/assistant/legal/components/ui/carousel.tsx` around lines 45 - 53, The
Carousel component currently spreads {...props} after setting
onKeyDown={handleKeyDown}, which allows a consumer-provided onKeyDown to
override the built-in keyboard handling; fix this by destructuring onKeyDown
from the incoming props in the Carousel signature (e.g., const { onKeyDown:
userOnKeyDown, ...rest } = props) and then compose a root onKeyDown that calls
handleKeyDown(e) first (or vice versa if desired) and then calls
userOnKeyDown?.(e) if present, ensuring you respect e.defaultPrevented; apply
the same destructure-and-compose pattern for the other instance referenced by
handleKeyDown at the second occurrence (the 140-146 spot) so consumer handlers
do not replace internal arrow-key navigation.
Comment on lines
+82
to
+100
| <style | ||
| dangerouslySetInnerHTML={{ | ||
| __html: Object.entries(THEMES) | ||
| .map( | ||
| ([theme, prefix]) => ` | ||
| ${prefix} [data-chart=${id}] { | ||
| ${colorConfig | ||
| .map(([key, itemConfig]) => { | ||
| const color = | ||
| itemConfig.theme?.[theme as keyof typeof itemConfig.theme] || | ||
| itemConfig.color | ||
| return color ? ` --color-${key}: ${color};` : null | ||
| }) | ||
| .join('\n')} | ||
| } | ||
| `, | ||
| ) | ||
| .join('\n'), | ||
| }} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the actual file
fd "chart.tsx" --type fRepository: Lamatic/AgentKit
Length of output: 483
🏁 Script executed:
# Read the specific file mentioned in the review
wc -l kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 108
🏁 Script executed:
# Read the target file to verify the XSS concern
cat -n kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 12373
XSS vulnerability: dangerouslySetInnerHTML with unescaped user input in CSS selectors and values.
The id parameter is user-controllable through the ChartContainer component props (line 50) and is interpolated unescaped into an unquoted CSS attribute selector at line 87. An id containing </style> breaks out of the stylesheet to inject script; special characters like spaces break the selector syntax. Config keys and color values from the user-provided config object are similarly interpolated without validation, allowing CSS injection through color values containing </style> or other dangerous sequences.
Recommended fix
- return (
- <style
- dangerouslySetInnerHTML={{
- __html: Object.entries(THEMES)
- .map(
- ([theme, prefix]) => `
-${prefix} [data-chart=${id}] {
-${colorConfig
- .map(([key, itemConfig]) => {
- const color =
- itemConfig.theme?.[theme as keyof typeof itemConfig.theme] ||
- itemConfig.color
- return color ? ` --color-${key}: ${color};` : null
- })
- .join('\n')}
-}
-`,
- )
- .join('\n'),
- }}
- />
- )
+ const safeId = CSS.escape(id)
+ const cssText = Object.entries(THEMES)
+ .map(([theme, prefix]) => {
+ const declarations = colorConfig
+ .map(([key, itemConfig]) => {
+ const safeKey = key.replace(/[^a-zA-Z0-9_-]/g, '')
+ const color =
+ itemConfig.theme?.[theme as keyof typeof THEMES] ?? itemConfig.color
+
+ if (!color || /[<>{}]/.test(color)) return null
+ return ` --color-${safeKey}: ${color};`
+ })
+ .filter(Boolean)
+ .join('\n')
+
+ return `${prefix} [data-chart="${safeId}"] {\n${declarations}\n}`
+ })
+ .join('\n')
+
+ return <style>{cssText}</style>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <style | |
| dangerouslySetInnerHTML={{ | |
| __html: Object.entries(THEMES) | |
| .map( | |
| ([theme, prefix]) => ` | |
| ${prefix} [data-chart=${id}] { | |
| ${colorConfig | |
| .map(([key, itemConfig]) => { | |
| const color = | |
| itemConfig.theme?.[theme as keyof typeof itemConfig.theme] || | |
| itemConfig.color | |
| return color ? ` --color-${key}: ${color};` : null | |
| }) | |
| .join('\n')} | |
| } | |
| `, | |
| ) | |
| .join('\n'), | |
| }} | |
| const safeId = CSS.escape(id) | |
| const cssText = Object.entries(THEMES) | |
| .map(([theme, prefix]) => { | |
| const declarations = colorConfig | |
| .map(([key, itemConfig]) => { | |
| const safeKey = key.replace(/[^a-zA-Z0-9_-]/g, '') | |
| const color = | |
| itemConfig.theme?.[theme as keyof typeof THEMES] ?? itemConfig.color | |
| if (!color || /[<>{}]/.test(color)) return null | |
| return ` --color-${safeKey}: ${color};` | |
| }) | |
| .filter(Boolean) | |
| .join('\n') | |
| return `${prefix} [data-chart="${safeId}"] {\n${declarations}\n}` | |
| }) | |
| .join('\n') | |
| return <style>{cssText}</style> |
🧰 Tools
🪛 ast-grep (0.41.0)
[warning] 82-82: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.4.4)
[error] 83-83: Avoid passing content using the dangerouslySetInnerHTML prop.
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kits/assistant/legal/components/ui/chart.tsx` around lines 82 - 100, The
inline stylesheet built with dangerouslySetInnerHTML in the ChartContainer
component is vulnerable because unescaped id, theme prefixes and colorConfig
values are interpolated into an unquoted CSS attribute selector and into CSS
text; fix by removing raw HTML construction or sanitizing inputs: use
CSS.escape(id) (or wrap the attribute value in quotes and escape quotes) when
constructing the selector for THEME entries, ensure theme prefix strings from
THEMES are validated/whitelisted before use, and validate/normalize colorConfig
keys and itemConfig.color values against a strict whitelist/regex (e.g., allow
only safe hex, rgb/rgba or CSS variable formats) falling back to a safe default
if invalid; alternatively avoid dangerouslySetInnerHTML entirely by generating
className-based styles via a CSS-in-JS approach or applying styles via inline
style objects to the rendered elements (update the code paths around
ChartContainer, the dangerouslySetInnerHTML block, THEMES, and
colorConfig/itemConfig usages).
Comment on lines
+278
to
+300
| {payload.map((item) => { | ||
| const key = `${nameKey || item.dataKey || 'value'}` | ||
| const itemConfig = getPayloadConfigFromPayload(config, item, key) | ||
|
|
||
| return ( | ||
| <div | ||
| key={item.value} | ||
| className={ | ||
| '[&>svg]:text-muted-foreground flex items-center gap-1.5 [&>svg]:h-3 [&>svg]:w-3' | ||
| } | ||
| > | ||
| {itemConfig?.icon && !hideIcon ? ( | ||
| <itemConfig.icon /> | ||
| ) : ( | ||
| <div | ||
| className="h-2 w-2 shrink-0 rounded-[2px]" | ||
| style={{ | ||
| backgroundColor: item.color, | ||
| }} | ||
| /> | ||
| )} | ||
| {itemConfig?.label} | ||
| </div> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd chart.tsxRepository: Lamatic/AgentKit
Length of output: 483
🏁 Script executed:
wc -l kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 108
🏁 Script executed:
sed -n '270,310p' kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 1125
🏁 Script executed:
sed -n '300,353p' kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 1185
🏁 Script executed:
head -n 100 kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 2955
🏁 Script executed:
rg -A 5 -B 5 "ChartLegendContent" kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 347
🏁 Script executed:
sed -n '250,280p' kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 795
🏁 Script executed:
rg -B 10 "ChartLegendContent" kits/assistant/legal/components/ui/chart.tsx | head -30Repository: Lamatic/AgentKit
Length of output: 422
🏁 Script executed:
rg "itemConfig\?.label" kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 176
🏁 Script executed:
sed -n '200,300p' kits/assistant/legal/components/ui/chart.tsx | grep -B 5 -A 5 "itemConfig?.label"Repository: Lamatic/AgentKit
Length of output: 781
🏁 Script executed:
rg "typeof payload\[" kits/assistant/legal/components/ui/chart.tsx -A 2Repository: Lamatic/AgentKit
Length of output: 194
🏁 Script executed:
rg "payload\[" kits/assistant/legal/components/ui/chart.tsx -B 3 -A 3 | head -40Repository: Lamatic/AgentKit
Length of output: 289
🏁 Script executed:
rg "item\.(name|value|dataKey)" kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 608
🏁 Script executed:
sed -n '130,180p' kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 1275
🏁 Script executed:
sed -n '170,230p' kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 2561
Legend labels disappear when config.label is omitted.
ChartConfig.label is optional, but the legend renders only itemConfig?.label. For series that define just color/theme, this produces a swatch with no text. Should fall back to item.name for consistency with the tooltip pattern.
Proposed fix
- {itemConfig?.label}
+ {itemConfig?.label || item.name}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {payload.map((item) => { | |
| const key = `${nameKey || item.dataKey || 'value'}` | |
| const itemConfig = getPayloadConfigFromPayload(config, item, key) | |
| return ( | |
| <div | |
| key={item.value} | |
| className={ | |
| '[&>svg]:text-muted-foreground flex items-center gap-1.5 [&>svg]:h-3 [&>svg]:w-3' | |
| } | |
| > | |
| {itemConfig?.icon && !hideIcon ? ( | |
| <itemConfig.icon /> | |
| ) : ( | |
| <div | |
| className="h-2 w-2 shrink-0 rounded-[2px]" | |
| style={{ | |
| backgroundColor: item.color, | |
| }} | |
| /> | |
| )} | |
| {itemConfig?.label} | |
| </div> | |
| {payload.map((item) => { | |
| const key = `${nameKey || item.dataKey || 'value'}` | |
| const itemConfig = getPayloadConfigFromPayload(config, item, key) | |
| return ( | |
| <div | |
| key={item.value} | |
| className={ | |
| '[&>svg]:text-muted-foreground flex items-center gap-1.5 [&>svg]:h-3 [&>svg]:w-3' | |
| } | |
| > | |
| {itemConfig?.icon && !hideIcon ? ( | |
| <itemConfig.icon /> | |
| ) : ( | |
| <div | |
| className="h-2 w-2 shrink-0 rounded-[2px]" | |
| style={{ | |
| backgroundColor: item.color, | |
| }} | |
| /> | |
| )} | |
| {itemConfig?.label || item.name} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kits/assistant/legal/components/ui/chart.tsx` around lines 278 - 300, The
legend rendering currently uses only itemConfig?.label so when ChartConfig.label
is omitted some legend entries show no text; update the legend fallback in the
payload.map render (where nameKey, getPayloadConfigFromPayload, itemConfig are
used) to display itemConfig?.label || item.name (or nameKey-derived value)
instead of just itemConfig?.label so series without a configured label fall back
to item.name for consistency with the tooltip behavior.
Comment on lines
+326
to
+343
| if ( | ||
| key in payload && | ||
| typeof payload[key as keyof typeof payload] === 'string' | ||
| ) { | ||
| configLabelKey = payload[key as keyof typeof payload] as string | ||
| } else if ( | ||
| payloadPayload && | ||
| key in payloadPayload && | ||
| typeof payloadPayload[key as keyof typeof payloadPayload] === 'string' | ||
| ) { | ||
| configLabelKey = payloadPayload[ | ||
| key as keyof typeof payloadPayload | ||
| ] as string | ||
| } | ||
|
|
||
| return configLabelKey in config | ||
| ? config[configLabelKey] | ||
| : config[key as keyof typeof config] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "chart.tsx" -type fRepository: Lamatic/AgentKit
Length of output: 501
🏁 Script executed:
wc -l ./kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 110
🏁 Script executed:
sed -n '300,353p' ./kits/assistant/legal/components/ui/chart.tsx | cat -nRepository: Lamatic/AgentKit
Length of output: 1563
🏁 Script executed:
rg -n "type ChartConfig|interface ChartConfig" ./kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 90
🏁 Script executed:
sed -n '1,50p' ./kits/assistant/legal/components/ui/chart.tsx | cat -nRepository: Lamatic/AgentKit
Length of output: 1506
🏁 Script executed:
rg -n "getPayloadConfigFromPayload" ./kits/assistant/legal/components/ui/chart.tsxRepository: Lamatic/AgentKit
Length of output: 333
🏁 Script executed:
rg -A 15 "function getPayloadConfigFromPayload" ./kits/assistant/legal/components/ui/chart.tsx | head -25Repository: Lamatic/AgentKit
Length of output: 410
🏁 Script executed:
for file in ./kits/*/*/components/ui/chart.tsx; do
echo "=== $file ==="
sed -n '308,345p' "$file" 2>/dev/null | head -20
doneRepository: Lamatic/AgentKit
Length of output: 4316
🏁 Script executed:
sed -n '308,345p' ./kits/assistant/legal/components/ui/chart.tsx | cat -nRepository: Lamatic/AgentKit
Length of output: 1228
Replace prototype chain walks with own-property checks.
The in operator walks the prototype chain, so keys like constructor or toString can resolve to built-in properties instead of chart config entries. The first two checks mitigate this with typeof guards, but the final return statement on line 342 lacks protection. Replace all in checks with Object.prototype.hasOwnProperty.call() for consistent own-property-only lookups.
Proposed fix
- if (
- key in payload &&
- typeof payload[key as keyof typeof payload] === 'string'
- ) {
+ if (
+ Object.prototype.hasOwnProperty.call(payload, key) &&
+ typeof payload[key as keyof typeof payload] === 'string'
+ ) {
configLabelKey = payload[key as keyof typeof payload] as string
} else if (
payloadPayload &&
- key in payloadPayload &&
+ Object.prototype.hasOwnProperty.call(payloadPayload, key) &&
typeof payloadPayload[key as keyof typeof payloadPayload] === 'string'
) {
configLabelKey = payloadPayload[
key as keyof typeof payloadPayload
] as string
}
- return configLabelKey in config
- ? config[configLabelKey]
- : config[key as keyof typeof config]
+ return Object.prototype.hasOwnProperty.call(config, configLabelKey)
+ ? config[configLabelKey]
+ : Object.prototype.hasOwnProperty.call(config, key)
+ ? config[key]
+ : undefined🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kits/assistant/legal/components/ui/chart.tsx` around lines 326 - 343, The
code uses the `in` operator which walks the prototype chain; update all property
checks to use own-property checks via Object.prototype.hasOwnProperty.call() —
replace the `in` checks on `payload` and `payloadPayload` and the final
`configLabelKey in config` check with
Object.prototype.hasOwnProperty.call(payload, key),
Object.prototype.hasOwnProperty.call(payloadPayload, key), and
Object.prototype.hasOwnProperty.call(config, configLabelKey) respectively
(retain the existing typeof guards and casts for `configLabelKey`, `payload`,
`payloadPayload`, and `config`).
Comment on lines
+107
to
+116
| function FormControl({ ...props }: React.ComponentProps<typeof Slot>) { | ||
| const { error, formItemId, formDescriptionId, formMessageId } = useFormField() | ||
|
|
||
| return ( | ||
| <Slot | ||
| data-slot="form-control" | ||
| id={formItemId} | ||
| aria-describedby={`${formDescriptionId} ${formMessageId}`} | ||
| aria-invalid={!!error} | ||
| {...props} |
Contributor
There was a problem hiding this comment.
Preserve the generated form ARIA wiring.
{...props} comes after the computed id/aria-describedby/aria-invalid, so a consumer can accidentally override the accessibility wiring and break the label, description, or error associations for this field.
♿ Proposed fix
-function FormControl({ ...props }: React.ComponentProps<typeof Slot>) {
+function FormControl({
+ 'aria-describedby': ariaDescribedBy,
+ 'aria-invalid': ariaInvalid,
+ ...props
+}: React.ComponentProps<typeof Slot>) {
const { error, formItemId, formDescriptionId, formMessageId } = useFormField()
+ const describedBy = [formDescriptionId, formMessageId, ariaDescribedBy]
+ .filter(Boolean)
+ .join(' ')
return (
<Slot
+ {...props}
data-slot="form-control"
id={formItemId}
- aria-describedby={`${formDescriptionId} ${formMessageId}`}
- aria-invalid={!!error}
- {...props}
+ aria-describedby={describedBy}
+ aria-invalid={ariaInvalid ?? !!error}
/>
)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kits/assistant/legal/components/ui/form.tsx` around lines 107 - 116, The
component spreads {...props} after computed accessibility attributes so a
consumer can override id/aria and break the ARIA wiring; update FormControl so
the consumer props cannot overwrite those generated values by either moving
{...props} before the explicit id/aria attributes in the Slot JSX or by
destructuring props to omit id, aria-describedby, and aria-invalid before
spreading, ensuring the computed values from useFormField() (formItemId,
formDescriptionId, formMessageId, error) always take precedence.
Comment on lines
+134
to
+149
| function FormMessage({ className, ...props }: React.ComponentProps<'p'>) { | ||
| const { error, formMessageId } = useFormField() | ||
| const body = error ? String(error?.message ?? '') : props.children | ||
|
|
||
| if (!body) { | ||
| return null | ||
| } | ||
|
|
||
| return ( | ||
| <p | ||
| data-slot="form-message" | ||
| id={formMessageId} | ||
| className={cn('text-destructive text-sm', className)} | ||
| {...props} | ||
| > | ||
| {body} |
Contributor
There was a problem hiding this comment.
Avoid error styling for non-error FormMessage content.
When error is falsy and props.children is rendered, the message still gets text-destructive, so helper text looks like a validation failure.
🎨 Proposed fix
return (
<p
data-slot="form-message"
id={formMessageId}
- className={cn('text-destructive text-sm', className)}
+ className={cn(
+ 'text-sm',
+ error ? 'text-destructive' : 'text-muted-foreground',
+ className,
+ )}
{...props}
>
{body}
</p>
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function FormMessage({ className, ...props }: React.ComponentProps<'p'>) { | |
| const { error, formMessageId } = useFormField() | |
| const body = error ? String(error?.message ?? '') : props.children | |
| if (!body) { | |
| return null | |
| } | |
| return ( | |
| <p | |
| data-slot="form-message" | |
| id={formMessageId} | |
| className={cn('text-destructive text-sm', className)} | |
| {...props} | |
| > | |
| {body} | |
| function FormMessage({ className, ...props }: React.ComponentProps<'p'>) { | |
| const { error, formMessageId } = useFormField() | |
| const body = error ? String(error?.message ?? '') : props.children | |
| if (!body) { | |
| return null | |
| } | |
| return ( | |
| <p | |
| data-slot="form-message" | |
| id={formMessageId} | |
| className={cn( | |
| 'text-sm', | |
| error ? 'text-destructive' : 'text-muted-foreground', | |
| className, | |
| )} | |
| {...props} | |
| > | |
| {body} | |
| </p> | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kits/assistant/legal/components/ui/form.tsx` around lines 134 - 149, The
FormMessage component is always applying the 'text-destructive' class even when
there is no error; update the className composition in FormMessage so that
'text-destructive' is only included when the local error variable is truthy (use
the error value from useFormField()), keeping other classes ('text-sm' and any
incoming className) unchanged and still passing id={formMessageId} and
{...props}; locate the FormMessage function to change the cn call where
className is built so helper/assistive text uses normal styling when error is
falsy.
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.
What This Kit Does
This kit adds a Legal Assistant under
kits/assistant/legalthat helps users ask legal questions, get informational summaries, receive references when available, and review practical next steps with a clear non-legal-advice disclaimer.It solves the problem of giving users a structured starting point for legal queries while explicitly avoiding legal representation claims.
Providers & Prerequisites
How to Run Locally
cd kits/assistant/legalnpm installcp .env.example .envand fill in valuesnpm run devRequired env vars:
ASSISTANT_LEGAL_ADVISORLAMATIC_API_URLLAMATIC_PROJECT_IDLAMATIC_API_KEYLive Preview
Add your deployed URL here (if available), e.g.:
https://<your-legal-assistant>.vercel.appLamatic Flow
Flow ID:
<your-flow-id>Flow folder:
kits/assistant/legal/flows/assistant-legal-advisorPR Checklist (Kits)
npm run dev.env.examplehas no secrets, only placeholdersREADME.mddocuments setup and usagekits/<category>/<kit-name>/config.jsonis present and validflows/folderSummary by CodeRabbit
New Features
Documentation