feat(platform): prompt library redesign and chat sidebar integration#1572
Merged
Conversation
- Extend `Tabs` with `variant` ('pill' | 'underline') and `triggerClassName`
- Remove `LocaleTabs` in favor of the underline `Tabs` variant
- Adopt `Tabs` in notification bell, onedrive picker, conversation starters
- Rename dialog and dropdown background token from bg-muted to bg-card
- Introduce `--tab` CSS var; tune dark-mode border/muted tokens
- Replace locale/theme tabs in user-button with inline button group
- Minor cosmetic polish for arena selector, dictation button, upload dialogs
- Guard Tabs onValueChange handlers with string unions instead of unsafe type assertions (user-button, notification-bell, onedrive-picker-stage) - Rename inner `items` variable in conversation-starters to avoid shadow - Reorder tailwind classes in tabs.tsx header row
Israeltheminer
force-pushed
the
feat/prompt-library-and-chat-sidebar
branch
from
c408754 to
1989710
Compare
Israeltheminer
changed the base branch from
refactor/ui-tabs-variant-and-tokens
to
main
- Migrate Tabs variants to cva (project standard) - Use !== undefined checks for tab content to respect falsy ReactNodes - Add ariaLabel to TabItem and wire up for icon-only theme tabs in user-button - Bump dark --border to 24% lightness for clearer component boundaries
Prompts backend: - Make `title` optional on `createPrompt`; auto-generate `PROMPT-XXXXX` when absent - Track `sourceMessageId` to link prompts back to the message they were saved from - Add `by_org_sourceMessageId` index and new `prompts/actions.ts` + `prompts/generate_title.ts` Threads: - Fire-and-forget AI title generation on the first message of a new thread (new internal action `threads/generate_thread_title`) Prompt library UI: - Replace card grid with a compact list row layout and new empty state - Rewrite `save-as-prompt-dialog` as `save-prompt-dialog` with scope + content edit - New `add-category-popover`, `category-filter-popover`, `sidebar-prompt-section` - Delete `prompt-card` in favor of `prompt-list-row` Chat surface: - History sidebar gains a tabbed History / Prompt Library switcher - Messages can be bookmarked as prompts via a new toolbar action on user bubbles - Chat header adopts PanelLeft toggle and `SquarePen` for new-chat - ChatLayoutContext exposes `insertedPrompt` so the sidebar can inject into the composer - Locale strings updated across de/en/fr for new copy
Israeltheminer
force-pushed
the
feat/prompt-library-and-chat-sidebar
branch
from
1989710 to
cd70eeb
Compare
…work - prompt-list-row: replace div[role=button] with nested <button>, remove stray console.log from DropdownMenu onOpenChange - Guard Tabs onValueChange handlers in prompt-form-dialog and chat-history-sidebar against invalid values - Move thread title generation to run after the file-upload guard so blocked requests do not schedule unnecessary work - Add internal mock for threads.generate_thread_title in agent_chat tests - Mock usePrompts in prompt-form-dialog test so it no longer needs a QueryClient provider
- Run oxfmt on chat-header, chat-input, prompt-form-dialog test - Extend save-prompt-dialog test mocks with useSavePrompt and usePrompts
📝 WalkthroughWalkthroughThis PR restructures the prompt management and chat history UI layers. It removes the Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/components/ui/navigation/tabs.stories.tsx`:
- Around line 326-343: The HeadlessNoContent story currently renders
TabsPrimitive.Trigger elements without matching TabsPrimitive.Content panels
because hasContent is false; fix by updating the Tabs component so that when
hasContent is false it either (A) switches to non-tab semantics (render triggers
as plain buttons or a radiogroup) by detecting headless mode and using
button/radiogroup roles for the Trigger elements, or (B) ensures a matching
TabsPrimitive.Content is always rendered for each item (render empty/fallback
Content panels when items lack content) so the ARIA tab/tabpanel relationship
remains valid; locate logic around hasContent, TabsPrimitive.Trigger and
TabsPrimitive.Content and implement one consistent headless strategy.
In `@services/platform/app/components/ui/overlays/dropdown-menu.tsx`:
- Line 257: The dropdown overlay components are inconsistent: the Content
component uses bg-card while SubContent still uses bg-muted; update the
SubContent component to use the same background token (replace bg-muted with
bg-card in the SubContent JSX/CSS class list) so Content and SubContent share
the unified background; locate the class string on the SubContent definition
(matching the Content class that includes 'bg-card') and make the token change
there for visual consistency.
In `@services/platform/app/components/user-button.tsx`:
- Around line 227-242: The locale tab triggers in the Tabs component (see Tabs
prop usage and items array in user-button.tsx) currently use terse labels
('EN','DE','FR') that screen readers may spell out; update the items passed to
Tabs to include ariaLabel fields that use translation keys (e.g.
t('language.english'), t('language.german'), t('language.french')) instead of
hardcoded English strings, so each item becomes { value: 'en', label: 'EN',
ariaLabel: t('...') } and similarly for 'de' and 'fr'; keep existing logic that
reads locale and setLocale/onValueChange unchanged.
In `@services/platform/app/features/chat/components/chat-history-sidebar.tsx`:
- Around line 280-283: The onValueChange handler on the Tabs component currently
uses an `as` cast to setActiveTab which bypasses the union type; remove the `as`
cast and validate the incoming value first (e.g., check if v === 'history' || v
=== 'archived') and only call setActiveTab(v) when it matches one of those
allowed strings; update the Tabs onValueChange implementation in
chat-history-sidebar.tsx where `Tabs`, `activeTab`, and `setActiveTab` are used
so the runtime guard narrows the type instead of using `as`.
In `@services/platform/app/features/chat/components/chat-interface.tsx`:
- Around line 205-217: The chat is currently subscribing to the entire prompt
library via usePrompts which causes unrelated prompt changes to re-render
savedMessageMap; replace this with a narrower data subscription that only
fetches prompts for the currently visible message IDs (e.g. create/use a hook
like usePromptsBySourceIds or usePromptsForMessageIds and pass the array of
visible message ids), then build savedMessageMap from that limited result;
update references to usePrompts, prompts, and savedMessageMap accordingly and
remove useDeletePrompt coupling if it forces a full-library refetch.
In `@services/platform/app/features/chat/components/message-bubble.tsx`:
- Around line 569-587: The bookmark icon button lacks an accessible name and
state; update the Button inside the (onSavePrompt || isSavedPrompt) &&
!!displayContent block to include a translated aria-label from tChat (use the
same keys as the Tooltip: tChat('unsavePrompt') when isSavedPrompt is true,
otherwise tChat('savePrompt')), and add aria-pressed={isSavedPrompt} to expose
the toggled state; keep existing onClick={handleBookmarkClick} and icon children
(BookmarkCheck / Bookmark) unchanged so screen-readers get both name and state.
In `@services/platform/app/features/prompts/components/add-category-popover.tsx`:
- Around line 85-91: The cancel button in add-category-popover.tsx (the button
that calls handleCancel) has a small text-only touch target; update its styling
so the interactive area meets the WCAG 2.5.8 minimum (at least 24×24 CSS pixels,
preferably 44×44 for mobile) by adding sufficient padding or explicit
min-width/min-height to the button (e.g., padding classes or min-h/min-w), keep
visual appearance (text styling) unchanged, and ensure the clickable area
expands without shifting layout or breaking alignment with surrounding elements.
In
`@services/platform/app/features/prompts/components/category-filter-popover.tsx`:
- Around line 52-59: Remove the debug console.log call inside the CategoryFilter
component's onOpenChange handler; keep the state update (setOpen(next)) and any
necessary props but delete the console.log('[DEBUG] CategoryFilter
onOpenChange:', next, 'current:', open) line so the popover no longer logs on
every open/close event.
In `@services/platform/app/features/prompts/components/prompt-form-dialog.tsx`:
- Around line 201-225: The visible label element rendering
t('form.categoryLabel') is not programmatically associated with the active
control (Select or Input); update the prompt-form-dialog to generate a stable id
(e.g., categoryInputId) and apply it to the label via htmlFor and to the active
control via id (or use aria-labelledby on Select if it doesn't accept id),
ensuring both branches (when categoryOptions.length > 0 rendering Select and the
else rendering Input) use that same id and keep existing handlers
(value={category}, onValueChange={setCategory} /
onChange={(e)=>setCategory(e.target.value)}); verify AddCategoryPopover remains
unaffected and that the id is unique within the component.
- Around line 183-190: Wrap the scope selector in a semantic fieldset/legend
(replace the standalone <label>) to associate the label with the Tabs control;
make scopeTabItems a const assertion (e.g., const scopeTabItems = [...] as
const) so its values are literal types, and remove the unsafe cast in
onValueChange — implement a type guard (e.g., isValidScope(v): v is PromptScope)
that checks the allowed values and only call setScope(v) when the guard returns
true; update the Tabs usage (Tabs, scope, setScope, scopeTabItems, PromptScope)
accordingly.
In `@services/platform/app/features/prompts/components/prompt-list-row.tsx`:
- Around line 44-47: The handleCopy callback currently shows the success toast
immediately even if navigator.clipboard.writeText(prompt.content) fails; update
handleCopy (the useCallback declared as handleCopy in prompt-list-row.tsx) to
await the writeText promise and only call toast({ title: t('actions.copied'),
variant: 'success' }) after it resolves, and catch any rejection to show an
error toast (e.g., toast with variant 'error' and a localized message) or
silently handle unsupported/denied clipboard cases; ensure you keep the
dependency array [prompt.content, toast, t] and handle async errors without
unhandled promise rejections.
- Around line 79-88: The outer clickable row (the div with role="button" that
calls handleUse) is receiving bubbled keyboard events from the nested menu
trigger; update the onKeyDown handler on that div to ignore bubbled events by
adding an early check like if (e.target !== e.currentTarget) return; so only
direct key presses on the row run handleUse; reference the onKeyDown handler and
handleUse function in prompt-list-row.tsx and ensure the row remains keyboard
accessible (or alternatively convert the row body into a semantic <button>
sibling of the menu trigger to prevent event propagation).
In `@services/platform/app/features/prompts/components/save-prompt-dialog.tsx`:
- Around line 79-80: The test mock for the mutations hook is missing
useSavePrompt which the component imports; update the mocked module export (the
vi.mock of '../../hooks/mutations') to include both useCreatePrompt and
useSavePrompt returning the same shape ({ mutateAsync: vi.fn(), isPending: false
}) so components using useSavePrompt won't throw at render time; ensure the mock
defines useSavePrompt alongside useCreatePrompt with the same stubbed API.
- Around line 29-68: Replace the custom ScopeRadio/button-based radio UI with
the project's accessible RadioGroup primitive: import RadioGroup and
RadioGroupItem from the radio-group component and swap the <div
role="radiogroup"> and ScopeRadio usages to a <RadioGroup value={scope}
onValueChange={setScope}> containing three <RadioGroupItem
value="personal|team|global"> entries that render the same labels; remove the
ScopeRadio component (or keep it for visual pieces only) and ensure
RadioGroupItem receives the same styling and focus-visible classes (e.g.,
focus-visible:ring-2 focus-visible:ring-ring) so keyboard arrow navigation,
Enter/Space activation, and visible focus are provided by the Radix-based
primitive.
In `@services/platform/app/globals.css`:
- Line 155: The light-mode contrast between --muted (95.9% L) and
--muted-foreground (46.1% L) falls below WCAG (4.39:1 < 4.5:1); fix by either
increasing the light-mode --muted-foreground L value in
services/platform/app/globals.css to achieve ≥4.5:1 against --muted, or update
components that use the pairing (e.g., conversation-starters.tsx at the bg-muted
+ text-muted-foreground usage) to use text-foreground instead of
text-muted-foreground in light mode; ensure you only change the CSS variable
value or the component class pairing so dark-mode tokens and focus ring (--ring)
behavior remain unchanged.
In `@services/platform/convex/lib/agent_chat/start_agent_chat.ts`:
- Around line 197-198: The current isFirstMessage calculation (isFirstMessage =
!messageAlreadyExists && existingMessages.page.length === 0) can race when two
requests query an empty thread concurrently; to fix, make title-generation
conditional on a post-insert, authoritative check: after persisting the new
message use a single-source check (e.g., re-query existingMessages for the
thread or check a thread-level flag/column) or use an atomic DB mechanism
(transaction/optimistic lock or unique constraint on a “threadHasTitle” marker)
before calling the title generator so only one request proceeds; refer to
isFirstMessage, existingMessages, and messageAlreadyExists to locate where to
move/add this re-check or locking logic.
- Around line 214-222: Tests fail because the mock for ctx.internal lacks the
threads path used when isFirstMessage is true; update the test mock to include
internal.threads.generate_thread_title.generateThreadTitle so
ctx.scheduler.runAfter(0,
internal.threads.generate_thread_title.generateThreadTitle, ...) resolves, e.g.
add an internal.threads object with generate_thread_title.generateThreadTitle (a
mock value) alongside the existing internal.lib.agent_chat.internal_actions
mock; this ensures the call in start_agent_chat.ts to ctx.scheduler.runAfter and
the referenced generateThreadTitle symbol is defined during tests.
In `@services/platform/convex/prompts/actions.ts`:
- Around line 27-45: The savePrompt action currently calls
internal.prompts.generatePromptTitle before any authentication, allowing
unauthenticated callers to consume AI tokens; add an explicit top-level guard at
the start of the savePrompt handler by calling const authUser = await
authComponent.getAuthUser(ctx); and throwing (e.g., throw new
Error('Unauthenticated')) if falsy, before invoking ctx.runAction or any other
token-using logic (referencing savePrompt, internal.prompts.generatePromptTitle,
and authComponent.getAuthUser(ctx)).
In `@services/platform/convex/prompts/generate_title.ts`:
- Around line 29-36: The providerOptions in createTitleGenerator currently
hardcodes openai-specific settings which may be ignored for non-OpenAI models
resolved by resolveLanguageModelWithFallback; update createTitleGenerator (or
the Agent construction path) to detect the resolved model's provider and only
set providerOptions.openai when the model is OpenAI-compatible, or replace it
with a provider-agnostic maxOutputTokens if supported by the Agent/LanguageModel
API, or add a short doc comment near createTitleGenerator explaining that all
resolved models are expected to be OpenAI-compatible and why; reference
createTitleGenerator, Agent, providerOptions and
resolveLanguageModelWithFallback when making the conditional/provider-agnostic
change.
In `@services/platform/convex/prompts/mutations.ts`:
- Line 19: The insert path for prompt rows must enforce a deterministic policy
for the (organizationId, sourceMessageId) key: before inserting a row with
sourceMessageId, query for an existing row matching organizationId and
sourceMessageId and then either reject or perform an upsert; implement the
upsert policy here (lookup existing row and call update on the found document,
otherwise insert) so duplicate submissions update the existing bookmark/link
instead of creating a second row. Locate the mutations that create prompt rows
(the insert mutation handling sourceMessageId) and change the flow to use a
findOne({ organizationId, sourceMessageId }) then update(...) if found, else
insert(...); also ensure the insert/update uses the same normalized
sourceMessageId value to avoid false duplicates.
In `@services/platform/messages/en.json`:
- Around line 4178-4179: The translation value for the key
"sidebar.sectionTitle" is using title case ("Prompt Library"); update it to
sentence case ("Prompt library") to comply with the JSON translations casing
rule; locate the "sidebar" object and change the string assigned to sectionTitle
from "Prompt Library" to "Prompt library".
In `@services/platform/messages/fr.json`:
- Around line 4154-4159: The placeholder example in the French translation (key
"namePlaceholder") uses a lowercase prefix "ex." which violates the
sentence-case rule; update the value for "namePlaceholder" in
services/platform/messages/fr.json to use sentence case (capitalize the prefix
to "Ex.") so the string starts with a capital letter while keeping the rest of
the placeholder text unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5c15ffb3-0b27-47b2-8142-02829f6da410
⛔ Files ignored due to path filters (1)
services/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (48)
services/platform/app/components/ui/dialog/dialog.tsxservices/platform/app/components/ui/navigation/locale-tabs.stories.tsxservices/platform/app/components/ui/navigation/locale-tabs.test.tsxservices/platform/app/components/ui/navigation/locale-tabs.tsxservices/platform/app/components/ui/navigation/tabs.stories.tsxservices/platform/app/components/ui/navigation/tabs.tsxservices/platform/app/components/ui/overlays/dropdown-menu.tsxservices/platform/app/components/user-button.tsxservices/platform/app/features/automations/components/automation-create-dialog.tsxservices/platform/app/features/chat/components/arena/arena-model-selector.tsxservices/platform/app/features/chat/components/chat-header.tsxservices/platform/app/features/chat/components/chat-history-sidebar.tsxservices/platform/app/features/chat/components/chat-input.tsxservices/platform/app/features/chat/components/chat-interface.tsxservices/platform/app/features/chat/components/chat-messages.tsxservices/platform/app/features/chat/components/dictation-button.tsxservices/platform/app/features/chat/components/message-bubble.tsxservices/platform/app/features/chat/context/chat-layout-context.tsxservices/platform/app/features/documents/components/document-comparison/comparison-file-selector.tsxservices/platform/app/features/documents/components/document-upload-dialog.tsxservices/platform/app/features/documents/components/onedrive-import/onedrive-picker-stage.tsxservices/platform/app/features/notifications/components/notification-bell.tsxservices/platform/app/features/prompts/components/__tests__/save-prompt-dialog.test.tsxservices/platform/app/features/prompts/components/add-category-popover.tsxservices/platform/app/features/prompts/components/category-filter-popover.tsxservices/platform/app/features/prompts/components/prompt-card.test.tsxservices/platform/app/features/prompts/components/prompt-card.tsxservices/platform/app/features/prompts/components/prompt-form-dialog.tsxservices/platform/app/features/prompts/components/prompt-library-dialog.tsxservices/platform/app/features/prompts/components/prompt-list-row.tsxservices/platform/app/features/prompts/components/save-as-prompt-dialog.tsxservices/platform/app/features/prompts/components/save-prompt-dialog.tsxservices/platform/app/features/prompts/components/sidebar-prompt-section.tsxservices/platform/app/features/prompts/hooks/mutations.tsservices/platform/app/features/prompts/hooks/queries.tsservices/platform/app/globals.cssservices/platform/app/routes/dashboard/$id/agents/$agentId/conversation-starters.tsxservices/platform/app/routes/dashboard/$id/chat.tsxservices/platform/convex/lib/agent_chat/start_agent_chat.tsservices/platform/convex/prompts/actions.tsservices/platform/convex/prompts/generate_title.tsservices/platform/convex/prompts/mutations.tsservices/platform/convex/prompts/schema.tsservices/platform/convex/prompts/validators.tsservices/platform/convex/threads/generate_thread_title.tsservices/platform/messages/de.jsonservices/platform/messages/en.jsonservices/platform/messages/fr.json
💤 Files with no reviewable changes (6)
- services/platform/app/components/ui/navigation/locale-tabs.test.tsx
- services/platform/app/features/prompts/components/prompt-card.test.tsx
- services/platform/app/features/prompts/components/prompt-card.tsx
- services/platform/app/features/prompts/components/save-as-prompt-dialog.tsx
- services/platform/app/components/ui/navigation/locale-tabs.tsx
- services/platform/app/components/ui/navigation/locale-tabs.stories.tsx
Comment on lines
+326
to
+343
| export const HeadlessNoContent: Story = { | ||
| args: { | ||
| defaultValue: 'en', | ||
| items: [ | ||
| { value: 'en', label: 'EN' }, | ||
| { value: 'de', label: 'DE' }, | ||
| { value: 'fr', label: 'FR' }, | ||
| ], | ||
| }, | ||
| parameters: { | ||
| docs: { | ||
| description: { | ||
| story: | ||
| 'Tabs without content panels act as a segmented control. Useful for theme or locale selectors.', | ||
| }, | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tabs_file="$(fd -p 'tabs.tsx' services/platform/app/components/ui/navigation)"
echo "Inspecting: $tabs_file"
echo
rg -n "TabsPrimitive\\.(Root|List|Trigger|Content)|aria-label|aria-labelledby|content|items\\.some" "$tabs_file"
echo
sed -n '1,260p' "$tabs_file"Repository: tale-project/tale
Length of output: 4009
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get full tabs.tsx to see if there's any additional logic beyond line 260
tabs_file="services/platform/app/components/ui/navigation/tabs.tsx"
echo "=== Full tabs.tsx file ==="
wc -l "$tabs_file"
echo
cat -n "$tabs_file"Repository: tale-project/tale
Length of output: 4300
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the story file to see how HeadlessNoContent is set up and if any a11y overrides exist
story_file="services/platform/app/components/ui/navigation/tabs.stories.tsx"
echo "=== Checking full story file ==="
wc -l "$story_file"
echo
# Show lines around the HeadlessNoContent story (lines 326-343 and surrounding context)
sed -n '310,360p' "$story_file"Repository: tale-project/tale
Length of output: 862
Refactor Tabs component to support headless mode without invalid ARIA.
The HeadlessNoContent story renders tab elements (via TabsPrimitive.Trigger) without matching tabpanel elements. When all items have no content property, hasContent is false (line 70) and TabsPrimitive.Content elements are not rendered (lines 97–109). This creates an invalid ARIA tab structure that violates the tabs pattern and will fail the addon-a11y audit.
Either refactor the component to use button or radiogroup semantics when headless, or ensure TabsPrimitive.Content is always rendered with matching items. The story itself documents this anti-pattern and should not pass accessibility checks as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/navigation/tabs.stories.tsx` around lines
326 - 343, The HeadlessNoContent story currently renders TabsPrimitive.Trigger
elements without matching TabsPrimitive.Content panels because hasContent is
false; fix by updating the Tabs component so that when hasContent is false it
either (A) switches to non-tab semantics (render triggers as plain buttons or a
radiogroup) by detecting headless mode and using button/radiogroup roles for the
Trigger elements, or (B) ensures a matching TabsPrimitive.Content is always
rendered for each item (render empty/fallback Content panels when items lack
content) so the ARIA tab/tabpanel relationship remains valid; locate logic
around hasContent, TabsPrimitive.Trigger and TabsPrimitive.Content and implement
one consistent headless strategy.
| onClick={(e) => e.stopPropagation()} | ||
| className={cn( | ||
| 'z-50 max-h-(--radix-dropdown-menu-content-available-height) min-w-[max(10rem,var(--radix-dropdown-menu-trigger-width))] overflow-y-auto overflow-x-hidden rounded-lg border bg-muted p-1 text-popover-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2', | ||
| 'z-50 max-h-(--radix-dropdown-menu-content-available-height) min-w-[max(10rem,var(--radix-dropdown-menu-trigger-width))] overflow-y-auto overflow-x-hidden rounded-lg border bg-card p-1 text-popover-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2', |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent background token between Content and SubContent.
The main Content now uses bg-card, but SubContent (line 156) still uses bg-muted. If the intent is to unify overlay backgrounds, consider updating the SubContent as well for visual consistency.
♻️ Proposed fix for consistency
- <DropdownMenuPrimitive.SubContent className="bg-muted text-popover-foreground data-[state=open]:animate-in ...
+ <DropdownMenuPrimitive.SubContent className="bg-card text-popover-foreground data-[state=open]:animate-in ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/overlays/dropdown-menu.tsx` at line 257,
The dropdown overlay components are inconsistent: the Content component uses
bg-card while SubContent still uses bg-muted; update the SubContent component to
use the same background token (replace bg-muted with bg-card in the SubContent
JSX/CSS class list) so Content and SubContent share the unified background;
locate the class string on the SubContent definition (matching the Content class
that includes 'bg-card') and make the token change there for visual consistency.
Comment on lines
+227
to
+242
| <Tabs | ||
| value={ | ||
| locale === 'en' || locale.startsWith('en-') | ||
| ? 'en' | ||
| : locale.startsWith('de') | ||
| ? 'de' | ||
| : 'fr' | ||
| } | ||
| onValueChange={(v) => setLocale(v)} | ||
| listClassName="w-full" | ||
| triggerClassName="flex-1" | ||
| items={[ | ||
| { value: 'en', label: 'EN' }, | ||
| { value: 'de', label: 'DE' }, | ||
| { value: 'fr', label: 'FR' }, | ||
| ]} |
There was a problem hiding this comment.
Add full-language ARIA labels to the locale tabs.
EN / DE / FR work visually, but they are terse accessible names and some screen readers will spell them out letter-by-letter. Since Tabs already supports ariaLabel, give these triggers translated full language names too.
As per coding guidelines, "ALWAYS use translation keys for aria-label values. Never hardcode English in ARIA attributes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/user-button.tsx` around lines 227 - 242, The
locale tab triggers in the Tabs component (see Tabs prop usage and items array
in user-button.tsx) currently use terse labels ('EN','DE','FR') that screen
readers may spell out; update the items passed to Tabs to include ariaLabel
fields that use translation keys (e.g. t('language.english'),
t('language.german'), t('language.french')) instead of hardcoded English
strings, so each item becomes { value: 'en', label: 'EN', ariaLabel: t('...') }
and similarly for 'de' and 'fr'; keep existing logic that reads locale and
setLocale/onValueChange unchanged.
Comment on lines
+280
to
+283
| <Tabs | ||
| value={activeTab} | ||
| onValueChange={(v) => setActiveTab(v as 'history' | 'archived')} | ||
| triggerClassName="flex-1" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 "setActiveTab\\(v as 'history' \\| 'archived'\\)" services/platform/app/features/chat/components/chat-history-sidebar.tsx
sed -n '1,80p' services/platform/app/components/ui/navigation/tabs.tsxRepository: tale-project/tale
Length of output: 2538
🏁 Script executed:
#!/bin/bash
# Check setActiveTab function signature
rg -n "setActiveTab|useState" services/platform/app/features/chat/components/chat-history-sidebar.tsx | head -20
# Look for similar Tab onValueChange patterns in the codebase
rg -n "onValueChange.*=>" services/platform/app --type tsx --type ts -A2 | head -40Repository: tale-project/tale
Length of output: 532
🏁 Script executed:
#!/bin/bash
# Search for Tab onValueChange patterns without file type issues
rg -n "onValueChange" services/platform/app/features --type ts -A2 | grep -A2 "onValueChange"Repository: tale-project/tale
Length of output: 34243
Remove the as cast from the tab change handler.
The cast sidesteps the union instead of narrowing it. Guard the value before calling setActiveTab to align with the repo's no-as rule.
Suggested change
- onValueChange={(v) => setActiveTab(v as 'history' | 'archived')}
+ onValueChange={(v) => {
+ if (v === 'history' || v === 'archived') {
+ setActiveTab(v);
+ }
+ }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/components/chat-history-sidebar.tsx`
around lines 280 - 283, The onValueChange handler on the Tabs component
currently uses an `as` cast to setActiveTab which bypasses the union type;
remove the `as` cast and validate the incoming value first (e.g., check if v ===
'history' || v === 'archived') and only call setActiveTab(v) when it matches one
of those allowed strings; update the Tabs onValueChange implementation in
chat-history-sidebar.tsx where `Tabs`, `activeTab`, and `setActiveTab` are used
so the runtime guard narrows the type instead of using `as`.
Comment on lines
+205
to
+217
| // Build a lookup of messageId → promptId for saved prompts | ||
| const { prompts } = usePrompts(organizationId); | ||
| const deletePrompt = useDeletePrompt(); | ||
|
|
||
| const savedMessageMap = useMemo(() => { | ||
| const map = new Map<string, (typeof prompts)[number]['_id']>(); | ||
| for (const prompt of prompts) { | ||
| if (prompt.sourceMessageId) { | ||
| map.set(prompt.sourceMessageId, prompt._id); | ||
| } | ||
| } | ||
| return map; | ||
| }, [prompts]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid subscribing the chat surface to the entire prompt library.
This makes every prompt create/edit/delete invalidate the chat view even though this screen only needs saved-state for the currently rendered message ids. A narrower query keyed by sourceMessageId or the visible message ids would scale better and avoid unrelated rerenders as prompt counts grow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/components/chat-interface.tsx` around
lines 205 - 217, The chat is currently subscribing to the entire prompt library
via usePrompts which causes unrelated prompt changes to re-render
savedMessageMap; replace this with a narrower data subscription that only
fetches prompts for the currently visible message IDs (e.g. create/use a hook
like usePromptsBySourceIds or usePromptsForMessageIds and pass the array of
visible message ids), then build savedMessageMap from that limited result;
update references to usePrompts, prompts, and savedMessageMap accordingly and
remove useDeletePrompt coupling if it forces a full-library refetch.
Comment on lines
+27
to
+45
| export const savePrompt = action({ | ||
| args: { | ||
| organizationId: v.string(), | ||
| content: v.string(), | ||
| description: v.optional(v.string()), | ||
| scope: promptScopeValidator, | ||
| teamId: v.optional(v.string()), | ||
| category: v.optional(v.string()), | ||
| tags: v.optional(v.array(v.string())), | ||
| isPublished: v.optional(v.boolean()), | ||
| sourceMessageId: v.optional(v.string()), | ||
| }, | ||
| returns: promptTemplateValidator, | ||
| handler: async (ctx, args): Promise<Doc<'promptTemplates'>> => { | ||
| // Try AI-generated title first (10s timeout enforced in the action) | ||
| const aiTitle = await ctx.runAction( | ||
| internal.prompts.generate_title.generatePromptTitle, | ||
| { content: args.content }, | ||
| ); |
There was a problem hiding this comment.
Authenticate before spending model tokens.
savePrompt calls generatePromptTitle before any auth check. If createPrompt rejects later, unauthenticated callers can still burn AI usage and probe the endpoint. Add a top-level authComponent.getAuthUser(ctx) guard and fail fast before runAction. Based on learnings: "For integrations actions, enforce explicit authentication at the action boundary using const authUser = await authComponent.getAuthUser(ctx); if (!authUser) throw new Error('Unauthenticated'); rather than relying on downstream RLS via queries..."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/prompts/actions.ts` around lines 27 - 45, The
savePrompt action currently calls internal.prompts.generatePromptTitle before
any authentication, allowing unauthenticated callers to consume AI tokens; add
an explicit top-level guard at the start of the savePrompt handler by calling
const authUser = await authComponent.getAuthUser(ctx); and throwing (e.g., throw
new Error('Unauthenticated')) if falsy, before invoking ctx.runAction or any
other token-using logic (referencing savePrompt,
internal.prompts.generatePromptTitle, and authComponent.getAuthUser(ctx)).
Comment on lines
+29
to
+36
| function createTitleGenerator(languageModel: LanguageModelV3): Agent { | ||
| return new Agent(components.agent, { | ||
| name: 'prompt-title-generator', | ||
| languageModel, | ||
| instructions: TITLE_INSTRUCTIONS, | ||
| providerOptions: { openai: { maxOutputTokens: 64 } }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what providers resolveLanguageModelWithFallback might return
echo "=== resolveLanguageModelWithFallback implementation ==="
ast-grep --pattern $'async function resolveLanguageModelWithFallback($$$) {
$$$
}'
echo ""
echo "=== Provider types and fallback chain ==="
rg -n -A 10 'resolveLanguageModelWithFallback' services/platform/convex/providers/Repository: tale-project/tale
Length of output: 980
🏁 Script executed:
#!/bin/bash
# Read the full failover.ts implementation
echo "=== Full failover.ts implementation ==="
cat services/platform/convex/providers/failover.tsRepository: tale-project/tale
Length of output: 3423
🏁 Script executed:
#!/bin/bash
# Search for other uses of providerOptions to understand the pattern
echo "=== Usage of providerOptions in codebase ==="
rg -n 'providerOptions' services/platform/convex/ -A 3 -B 3Repository: tale-project/tale
Length of output: 7138
🏁 Script executed:
#!/bin/bash
# Check the Agent constructor and what it expects
echo "=== Agent constructor usage ==="
rg -n 'new Agent' services/platform/convex/ -A 5 -B 2Repository: tale-project/tale
Length of output: 9917
Clarify whether OpenAI-specific provider options are intentional for all resolved models.
The providerOptions: { openai: { maxOutputTokens: 64 } } pattern is used consistently across the codebase to prevent OpenRouter from applying its own low defaults. However, since resolveLanguageModelWithFallback can return models from potentially different providers (primary model → fallback model → tag-based fallback), these options may be silently ignored if a non-OpenAI provider is resolved.
Review the resolved provider sources in resolve_model.ts and either:
- Document that all resolved models are expected to be OpenAI-compatible, or
- Use provider-agnostic
maxOutputTokensif the underlying API supports it, or - Add provider-detection logic to apply provider-specific options conditionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/prompts/generate_title.ts` around lines 29 - 36, The
providerOptions in createTitleGenerator currently hardcodes openai-specific
settings which may be ignored for non-OpenAI models resolved by
resolveLanguageModelWithFallback; update createTitleGenerator (or the Agent
construction path) to detect the resolved model's provider and only set
providerOptions.openai when the model is OpenAI-compatible, or replace it with a
provider-agnostic maxOutputTokens if supported by the Agent/LanguageModel API,
or add a short doc comment near createTitleGenerator explaining that all
resolved models are expected to be OpenAI-compatible and why; reference
createTitleGenerator, Agent, providerOptions and
resolveLanguageModelWithFallback when making the conditional/provider-agnostic
change.
| category: v.optional(v.string()), | ||
| tags: v.optional(v.array(v.string())), | ||
| isPublished: v.optional(v.boolean()), | ||
| sourceMessageId: v.optional(v.string()), |
There was a problem hiding this comment.
Enforce a deterministic policy for sourceMessageId before insert.
sourceMessageId is now the linkage key for the message bookmark flow, but this path allows multiple prompt rows for the same (organizationId, sourceMessageId). A double-submit or saving the same message twice leaves later “saved / unsave” lookups ambiguous, because nothing here decides whether to reject, update, or upsert the existing link.
Also applies to: 34-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/prompts/mutations.ts` at line 19, The insert path
for prompt rows must enforce a deterministic policy for the (organizationId,
sourceMessageId) key: before inserting a row with sourceMessageId, query for an
existing row matching organizationId and sourceMessageId and then either reject
or perform an upsert; implement the upsert policy here (lookup existing row and
call update on the found document, otherwise insert) so duplicate submissions
update the existing bookmark/link instead of creating a second row. Locate the
mutations that create prompt rows (the insert mutation handling sourceMessageId)
and change the flow to use a findOne({ organizationId, sourceMessageId }) then
update(...) if found, else insert(...); also ensure the insert/update uses the
same normalized sourceMessageId value to avoid false duplicates.
Comment on lines
+4178
to
+4179
| "sidebar": { | ||
| "sectionTitle": "Prompt Library" |
There was a problem hiding this comment.
Keep the new sidebar label in sentence case.
"Prompt Library" breaks the repo’s JSON translation casing rule; this should match the rest of the copy as "Prompt library". As per coding guidelines: "**/*.json: ALWAYS use sentence case in translations"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` around lines 4178 - 4179, The translation
value for the key "sidebar.sectionTitle" is using title case ("Prompt Library");
update it to sentence case ("Prompt library") to comply with the JSON
translations casing rule; locate the "sidebar" object and change the string
assigned to sectionTitle from "Prompt Library" to "Prompt library".
Comment on lines
+4154
to
+4159
| "title": "Enregistrer le prompt", | ||
| "description": "Nomme ton prompt et modifie le contenu avant de l'enregistrer.", | ||
| "namePlaceholder": "ex. Bonnes pratiques de limitation de débit API", | ||
| "saveTo": "Enregistrer sous", | ||
| "cancel": "Annuler" | ||
| }, |
There was a problem hiding this comment.
Use sentence case for the placeholder example prefix.
Line 4156 starts with lowercase ex., which breaks the JSON translation sentence-case rule.
✏️ Proposed fix
- "namePlaceholder": "ex. Bonnes pratiques de limitation de débit API",
+ "namePlaceholder": "Ex. Bonnes pratiques de limitation de débit API",As per coding guidelines **/*.json: ALWAYS use sentence case in translations.
📝 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
| "title": "Enregistrer le prompt", | |
| "description": "Nomme ton prompt et modifie le contenu avant de l'enregistrer.", | |
| "namePlaceholder": "ex. Bonnes pratiques de limitation de débit API", | |
| "saveTo": "Enregistrer sous", | |
| "cancel": "Annuler" | |
| }, | |
| "title": "Enregistrer le prompt", | |
| "description": "Nomme ton prompt et modifie le contenu avant de l'enregistrer.", | |
| "namePlaceholder": "Ex. Bonnes pratiques de limitation de débit API", | |
| "saveTo": "Enregistrer sous", | |
| "cancel": "Annuler" | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/fr.json` around lines 4154 - 4159, The placeholder
example in the French translation (key "namePlaceholder") uses a lowercase
prefix "ex." which violates the sentence-case rule; update the value for
"namePlaceholder" in services/platform/messages/fr.json to use sentence case
(capitalize the prefix to "Ex.") so the string starts with a capital letter
while keeping the rest of the placeholder text unchanged.
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createPromptaccepts an optional title (auto-generatesPROMPT-XXXXXwhen absent), trackssourceMessageIdto link prompts back to the originating message, and gains aby_org_sourceMessageIdindex. New Convex action modules for prompts and AI title generation.save-prompt-dialog, add-category popover, category filter popover, sidebar prompt section.PanelLeft+SquarePen,ChatLayoutContextexposesinsertedPromptso the sidebar can inject into the composer.Test plan
PROMPT-XXXXXfallback appears.ChatInput.Summary by CodeRabbit
New Features
Improvements
Chores