refactor(platform): overhaul settings UI with grouped layouts and enhanced panels#1485
Merged
Conversation
…anced panels Restructure governance settings from flat tabs to grouped sections (content & models, policies & limits, security & monitoring). Add provider edit panel with description and default model selection. Enhance MCP server cards with inline edit/delete actions and detail panel menu. Add team detail dialog with member list. Add audit log export functionality. Extract reusable SelectTriggerButton component. Update Tabs component with actions slot and flex layout. Fix password input autocomplete attribute.
📝 WalkthroughWalkthroughThis PR modernizes the settings and governance UI across multiple features. It introduces a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
services/platform/app/features/settings/providers/components/provider-edit-panel.tsx (1)
219-223:⚠️ Potential issue | 🟡 MinorHonor reduced-motion on the new loader.
This spinner uses
animate-spinwithout amotion-reduce:fallback.As per coding guidelines "ENSURE all animations and transitions respect `prefers-reduced-motion: reduce`".💡 Suggested fix
- <Loader2 className="mr-2 size-4 animate-spin" /> + <Loader2 className="mr-2 size-4 animate-spin motion-reduce:animate-none" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/settings/providers/components/provider-edit-panel.tsx` around lines 219 - 223, The spinner shown when isPending renders Loader2 with the Tailwind animate-spin class but doesn't respect prefers-reduced-motion; update the provider-edit-panel component so the Loader2 respects reduced motion (e.g., add the Tailwind motion-reduce:animate-none utility or conditionally omit the spin when prefers-reduced-motion is set) so the animate-spin is suppressed for users who prefer reduced motion; ensure the change targets the isPending rendering path that includes Loader2 and the accompanying providers.saving text.services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx (1)
218-225:⚠️ Potential issue | 🟠 MajorUse the shared button affordances for these new text actions.
Because these are plain
<button>elements, they miss the shared min-size/focus styling the rest of the UI gets fromButton, and the resulting targets are only text-sized here. That falls below the settings page accessibility baseline for edit/remove actions.As per coding guidelines "ENSURE interactive elements are keyboard accessible and have visible focus states" and "ENSURE interactive elements have a minimum 24×24 CSS pixel touch target (WCAG 2.5.8). Prefer 44×44 for mobile".
Also applies to: 248-255, 864-890
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx around lines 218 - 225, Replace the plain <button> used to trigger setPanelOpen(true) (the one rendering <Pencil /> and t('providers.editGeneral')) with the shared Button component used across the app so it inherits min-size, focus, and keyboard affordances; ensure you pass the appropriate props (e.g., size/variant/aria-label) and keep the onClick calling setPanelOpen(true) and the icon+label composition, and apply the same replacement to the other plain buttons noted (the similar buttons rendering remove/edit actions referenced in the diff).services/platform/app/features/settings/mcp-servers/components/__tests__/mcp-server-panel.test.tsx (1)
138-150:⚠️ Potential issue | 🟠 MajorThis test still doesn't verify the new panel behaviors.
It only proves that the trigger renders. Please open the dropdown and assert the edit/delete entries, and add one
initialEditingrender so the new edit-mode footer path is covered too.As per coding guidelines "ALWAYS write tests for new features and bug fixes" and "Tests should cover happy paths, edge cases, and error conditions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/settings/mcp-servers/components/__tests__/mcp-server-panel.test.tsx` around lines 138 - 150, Update the test for McpServerPanel to actually exercise the dropdown and edit-mode footer: simulate opening the actions menu (use userEvent.click or fireEvent on the button found by role/name "actions menu"), then assert that the "Edit" and "Delete" menu items are present and accessible; additionally add a separate render of <McpServerPanel initialEditing={true} ... /> (using makeServer(), vi.fn() for callbacks) and assert the edit-mode footer elements appear to cover the new edit path. Use screen queries (getByRole/getByText) for assertions and keep the existing props on the component.services/platform/app/features/settings/mcp-servers/components/mcp-server-panel.tsx (1)
200-229:⚠️ Potential issue | 🟡 MinorGive the sheet an accessible description.
The panel tests are already warning that
DialogContentis missing a description. Please pass a shortSheetdescription here (visually hidden if you do not want visible copy) so screen readers get the extra context and the warning goes away.As per coding guidelines "ALL pages should be optimized for accessibility (Level AA)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/settings/mcp-servers/components/mcp-server-panel.tsx` around lines 200 - 229, The sheet/dialog in MCPServerPanel is missing an accessible description; update the component that renders Sheet/DialogContent in MCPServerPanel to provide a short descriptive string via the description prop or aria-describedby (e.g., description="Edit server settings for {server.displayName}") and ensure the description element is available to screen readers (use your VisuallyHidden/ScreenReaderOnly component or an element with sr-only class) so Screen Readers get context; locate the Sheet/DialogContent usage in MCPServerPanel (the same component that uses onOpenChange and renders the header with server.displayName) and pass the description there.services/platform/app/features/settings/governance/components/budget-editor.tsx (1)
334-336:⚠️ Potential issue | 🟡 MinorUse
formatCurrency()instead oftoLocaleString()per coding guidelines.The coding guidelines state: "DO NOT use
toLocaleDateString(),toLocaleTimeString(), ortoLocaleString()." This principle extends to currency formatting for consistency with the established utility patterns.Replace
toLocaleString()with the existingformatCurrency()utility fromlib/utils/format/number:🔧 Suggested fix
+import { formatCurrency } from '@/lib/utils/format/number'; + -function formatCost(cents: number): string { - return `$${(cents / 100).toLocaleString(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 })}`; -} +function formatCost(cents: number): string { + return formatCurrency(cents / 100, 'USD'); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/settings/governance/components/budget-editor.tsx` around lines 334 - 336, The formatCost function uses toLocaleString for currency which violates the guideline; replace its implementation to call the existing formatCurrency utility (imported from lib/utils/format/number) and pass cents/100 (or a number in dollars) so formatting uses formatCurrency instead of toLocaleString—update the import and change formatCost to delegate to formatCurrency while preserving two-decimal currency output.services/platform/messages/de.json (1)
3726-3734:⚠️ Potential issue | 🟠 MajorRemove the duplicate
governance.tabsmap.There is another
governance.tabsobject later in this same JSON object at Line 3950, so parsers will keep the later one and discard these entries. As written, the newretentionlabel never becomes available at runtime and the earlier tab labels here are ignored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/messages/de.json` around lines 3726 - 3734, Duplicate governance.tabs map (containing keys systemPrompt, budgets, featureControls, usage, pii, modelAccess, retention) appears earlier and is overridden by a later governance.tabs; delete this earlier governance.tabs block so the later definition (which includes the retention label) is the single source of truth and the retention label is available at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/providers/openrouter.secrets.json`:
- Around line 2-13: Add an explicit inline comment at the top of
examples/providers/openrouter.secrets.json stating it is a template
SOPS-encrypted example (not decryptable without the AGE private key) and not
intended for direct use, and also update docs/environment-reference.md to
include a short note referencing that example file and instructing users to
generate their own encrypted secrets via the tale init command (and never commit
private keys).
In
`@services/platform/app/features/settings/governance/components/budget-editor.tsx`:
- Around line 501-506: The opacity transition needs to respect reduced-motion
preferences: in the div that builds className via cn(...) in budget-editor.tsx
(the block starting with className={cn('transition-opacity duration-200',
...)}), add the Tailwind motion-reduce prefix to disable the transition for
users who prefer reduced motion (e.g., include "motion-reduce:transition-none"
alongside the existing transition classes, matching the approach used in
model-access-editor).
- Around line 569-582: Replace direct use of toLocaleString for numeric display
with the shared formatting utility: import formatNumber from
'lib/utils/format/number' (if not already imported) and use
formatNumber(rule.maxTokens) and formatNumber(rule.maxRequests) instead of
rule.maxTokens.toLocaleString() and rule.maxRequests.toLocaleString(); keep
existing null checks and leave formatCost(rule.maxCostCents) unchanged so
token/request cells use formatNumber while cost cell continues to use
formatCost.
In
`@services/platform/app/features/settings/governance/components/feature-flags-editor.tsx`:
- Around line 476-481: The transition on the wrapper div in FeatureFlagsEditor
uses 'transition-opacity duration-200' but lacks the motion-reduce override;
update the className in the div inside feature-flags-editor.tsx (the block using
cn(...) with 'transition-opacity duration-200' and the pointer-events/opacity
conditional) to include the Tailwind motion-reduce utility (e.g.,
'motion-reduce:transition-none') so users with reduced motion preferences won't
see the opacity transition.
- Around line 560-578: Replace the template-literal aria-labels on the
edit/delete Buttons in feature-flags-editor.tsx with i18n interpolation calls
(e.g., use t('featureFlags.editRuleAriaLabel', { index: index + 1 }) and
t('featureFlags.deleteRuleAriaLabel', { index: index + 1 })) instead of
`${t('featureFlags.editRule')} ${index + 1}`; update the Button props where
onClick handlers reference openEditDialog and removeRule to use these t(...)
calls, and add corresponding translation keys with a {{index}} placeholder in
the featureFlags namespace so RTL and other locales render correctly.
In
`@services/platform/app/features/settings/governance/components/model-access-editor.tsx`:
- Around line 458-463: The opacity transition on the JSX div that builds
className('transition-opacity duration-200', !enabled && 'pointer-events-none
opacity-50') should respect prefers-reduced-motion; update the class list used
in model-access-editor's component to include the Tailwind utility
motion-reduce:transition-none (e.g., add 'motion-reduce:transition-none'
alongside 'transition-opacity duration-200') so users who prefer reduced motion
will not see the transition while preserving the pointer-events/opacity behavior
tied to the enabled variable.
In
`@services/platform/app/features/settings/governance/components/pii-config.tsx`:
- Around line 64-74: The effect that syncs server policy into local state only
runs when policy is truthy, so when policy becomes undefined (e.g., switching
organization) the previous values remain; update the useEffect handling for
policy to include an else branch that resets local state to safe defaults by
calling setEnabled(false), setMode('mask'), setEnabledPatterns(new
Set<string>(PATTERN_NAMES)), and setCustomPatterns([]) so stale values aren't
carried into a new org. Reference: useEffect, policy, setEnabled, setMode,
setEnabledPatterns, setCustomPatterns, PATTERN_NAMES.
- Around line 230-235: The Select (and other controls) allow triggering
saveConfig while an upsertMutation is pending; update each control that calls
saveConfig directly (e.g., the Select using onValueChange={handleModeChange},
any controls between lines 245-253 and 274-345) to also disable when
upsertMutation.isPending (in addition to cannotManage) so rapid clicks cannot
submit overlapping writes; ensure you reference upsertMutation.isPending in the
disabled prop for those components that call saveConfig to prevent actions while
a write is in flight.
In
`@services/platform/app/features/settings/governance/components/retention-editor.tsx`:
- Around line 135-151: Replace all hardcoded user-facing strings in
retention-editor.tsx (e.g., the Input with label "Retention Days", placeholder
"e.g. 90", and the Text helper "Documents older than this will be deleted.")
with calls to the project's i18n translation function (e.g., t('...')) and
corresponding translation keys; update the onChange/onBlur logic
(setRetentionDays, saveConfig) to pass the translated label where necessary for
accessibility attributes, and do the same for the other hardcoded strings in the
same file (the UI around lines 177-195 and 221-239) so all labels, placeholders,
helper text and button text use translation keys instead of literal English
text.
- Around line 129-133: The fade wrapper divs in retention-editor.tsx (the div
using cn('transition-opacity duration-200', ...) and the two other identical
wrappers) do not respect prefers-reduced-motion; update their className strings
to include the motion-reduce utility (e.g., add 'motion-reduce:transition-none')
so transitions are disabled for users who request reduced motion, keeping the
existing conditional classes like 'pointer-events-none opacity-50' and using the
same cn(...) helper to compose classes.
In
`@services/platform/app/features/settings/governance/components/select-trigger-button.tsx`:
- Around line 1-16: The component SelectTriggerButton currently spreads
{...props} before a hardcoded className which prevents callers' className from
taking effect; destructure className from the component props (function
SelectTriggerButton({ hasValue, children, className, ...props }, ref)),
import/use the cn() helper to merge the default class string with the incoming
className (e.g., cn("ring-border bg-input ...", className)), and pass that
merged className on the <button> after spreading {...props} so the
merged/overridden classes are applied.
In
`@services/platform/app/features/settings/governance/components/upload-policy-editor.tsx`:
- Around line 168-172: The fade wrapper in upload-policy-editor.tsx (the div
with className built by cn in the UploadPolicyEditor component) doesn't respect
prefers-reduced-motion; update the class list to include the Tailwind utility
that disables transitions for reduced motion (motion-reduce:transition-none) so
the opacity transition is suppressed when users request reduced motion, e.g.,
add the utility to the cn call that builds the wrapper's className or
conditionally include it based on a reduced-motion check.
- Around line 168-245: The Save button is inside the wrapper that gets
"pointer-events-none opacity-50" when enabled is false, preventing mouse users
from clicking it; move the Button out of the surrounding div that uses cn(...,
!enabled && 'pointer-events-none opacity-50') so it is not covered by that CSS,
leaving its disabled prop as (cannotManage || upsertMutation.isPending) (i.e.,
keep handleSave and upsertMutation usage unchanged) so keyboard and mouse users
can persist the disabled policy; update the JSX to render the Button after the
wrapper/Stack block rather than inside it.
In
`@services/platform/app/features/settings/mcp-servers/components/__tests__/mcp-server-card.test.tsx`:
- Around line 82-93: Update the failing test in McpServerCard's test suite to
target the main card button by its accessible name instead of relying on the
first button found: in the test that uses McpServerCard, replace the generic
screen.getByRole('button') click with screen.getByRole('button', { name: /<card
button accessible label>/i }) (or the exact visible label text) to assert that
onClick is called, and add new tests that open the ellipsis/menu trigger (the
menu button) and assert that onEdit and onDelete callbacks passed to
McpServerCard are invoked when their respective menu items are selected; ensure
you reference the component McpServerCard and the props onClick, onEdit,
onDelete so the clicks target the correct interactive elements.
In
`@services/platform/app/features/settings/mcp-servers/components/mcp-server-card.tsx`:
- Around line 82-85: The Card component in mcp-server-card.tsx currently removes
the native focus outline but provides no replacement, so add a visible keyboard
focus style to the Card/button by updating the Card's className (or the
interactive element that wraps the card) to include focus-visible utilities such
as "focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary
focus-visible:ring-offset-2" (or a high-contrast outline class) so keyboard
users see a clear focus ring; ensure the same treatment is applied where the
native outline was removed (search for Card with className
"hover:border-primary/50 cursor-pointer transition-colors" and the interactive
button element) and test with keyboard tabbing to confirm the focus state is
visible.
- Around line 138-143: The UI currently hardcodes English pluralization for the
tool count in the MCP server card (see toolCount, Wrench, HStack, Text in
mcp-server-card.tsx); replace the inline "{toolCount} {toolCount === 1 ? 'tool'
: 'tools'}" with a call to the project's translation hook/function (e.g.,
useTranslation() / t) and an i18n key that accepts a count parameter (pass
toolCount) so pluralization/localization rules are applied; update the Text to
render the translated string and add the new i18n key (with singular/plural) to
the locale files.
In
`@services/platform/app/features/settings/mcp-servers/components/mcp-server-panel.tsx`:
- Around line 412-415: The spinner in mcp-server-panel uses Loader2 with
"animate-spin" which ignores users' prefers-reduced-motion setting; update the
Loader2 className (and any equivalent spinner instances using isTesting) to
include the Tailwind reduction override such as "motion-reduce:animate-none"
(e.g., "mr-2 size-4 animate-spin motion-reduce:animate-none") so the animation
is suppressed when the user requests reduced motion while preserving the spinner
for others.
In
`@services/platform/app/features/settings/mcp-servers/components/mcp-servers.tsx`:
- Around line 163-168: The header close IconButton (icon={X}) currently always
calls setAddDialogOpen(false) and can dismiss the sheet while a create is in
progress; change its behavior to respect the isCreating flag so it cannot close
during creation — e.g., make the IconButton disabled/aria-disabled when
isCreating or replace its onClick with a no-op while isCreating so
setAddDialogOpen(false) is not invoked; ensure accessibility attributes
(aria-disabled) mirror the disabled state to match the footer buttons’ behavior.
In
`@services/platform/app/features/settings/providers/components/provider-edit-panel.tsx`:
- Around line 94-102: The isDirty check is comparing
form.defaults.{chat,vision,embedding} (which can be undefined) directly to
NONE_VALUE causing true positives; update the logic to normalize the form
defaults before comparison—either create a normalizedDefaults object (e.g.,
const normalizedDefaults = { chat: form.defaults.chat ?? NONE_VALUE, vision:
form.defaults.vision ?? NONE_VALUE, embedding: form.defaults.embedding ??
NONE_VALUE }) and use normalizedDefaults.chat/vision/embedding in the isDirty
calculation, or change each comparison to use nullish coalescing
(form.defaults.chat ?? NONE_VALUE) so the comparisons against
data.config.defaults use consistent values; keep references to isDirty, form,
data.config, and NONE_VALUE.
In
`@services/platform/app/features/settings/teams/components/team-detail-dialog.tsx`:
- Around line 94-130: Wrap the members rendering with proper list semantics by
adding role="list" to the outer container div that currently wraps
teamMembers.map, and add role="listitem" to each mapped item div (the div with
key={member._id} inside teamMembers.map) so screen readers (Safari/VoiceOver
with Tailwind Preflight) correctly expose the list; no visual/CSS changes
required—just add those role attributes in team-detail-dialog.tsx where
teamMembers is mapped.
- Around line 125-127: The Badge currently renders the raw member.role string;
replace that with the translated label from the settings.roles translations (use
the app's i18n helper, e.g. t or useTranslations) so the role displays localized
text. Update the JSX where Badge is rendered (the Badge element that reads
{member.role}) to look up settings.roles[member.role] via the translation
hook/function and fall back to member.role if the translation is missing.
In `@services/platform/app/features/settings/teams/components/teams-table.tsx`:
- Around line 59-72: The TeamDetailDialog is only rendered when selectedTeam is
truthy, so the open={!!selectedTeam} prop is redundant; remove the open prop
from the TeamDetailDialog usage (leave the conditional render based on
selectedTeam intact) and keep the existing onRowClick/handleViewTeam wiring
(useTeamsTableConfig -> TeamRowActions) as-is so row click and action menu
continue to set selectedTeam and open the dialog.
In `@services/platform/app/routes/dashboard/`$id/settings/governance.tsx:
- Around line 137-150: The navigation buttons rendered in the map (button with
key={group.key}, onClick={() => handleGroupChange(group.key)}, and className
using cn) lack a visible keyboard focus style; update the className for that
button to include accessible focus-visible classes (e.g.,
focus-visible:outline-none or outline, focus-visible:ring and
focus-visible:ring-offset with appropriate colors) so the active/focused button
shows a clear focus ring for keyboard users while preserving the existing
activeGroup conditional styles.
In `@services/platform/app/routes/dashboard/`$id/settings/logs.tsx:
- Around line 96-126: The export buttons are placed in the shared Tabs actions
area so they appear on all tabs; restrict them to the Audit tab by making the
actions conditional on the current Tabs value instead of always rendering when
isAdminUser is true. Read the active tab value from the Tabs component (e.g.,
control Tabs value via state or access the Tabs selected value) and render the
export Button group only when that value === "audit" (keep references to
handleExport and exportAction as-is), leaving isAdminUser gating in place so
only admins on the "audit" tab see and use the export controls.
- Around line 63-70: The onSuccess handler for requestExport currently shows a
"complete" toast even when data.url is null; update the onSuccess in
services/platform/app/routes/dashboard/$id/settings/logs.tsx (the onSuccess
callback that uses toast, data.fileName, data.url and window.open) to first
check if data.url is non-null before opening the window and showing the
"complete" toast, and if data.url is null show a different toast/message (e.g.,
"export queued" or "no file available") so users aren't given a false success;
ensure you only call window.open when data.url is truthy and keep using
data.fileName in the appropriate toast.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx:
- Around line 801-839: The Inputs under the model form are using hardcoded
English labels and placeholders (e.g., "Input cost (USD / 1M tokens)", "Output
cost (USD / 1M tokens)", "e.g., 2.50", "e.g., 10.00", and "e.g., 1536") —
replace those literal strings with the i18n translation function (t(...)) so the
UI uses localized copy; update the Input components (the ones bound to
form.dimensions, form.inputCostPerMillion, and form.outputCostPerMillion and
their label/placeholder props) to call t with appropriate translation
keys/messages instead of hardcoded English.
- Around line 417-421: The spinner elements use Tailwind's animate-spin without
respecting prefers-reduced-motion; update the Loader2 usage(s) (the ones
rendered when saving and the other instance around lines 938-942) to include a
reduced-motion override by adding the motion-reduce:animate-none class alongside
animate-spin so the spinner stops for users with prefers-reduced-motion set, and
ensure any text/visual fallback remains readable when animation is disabled.
---
Outside diff comments:
In
`@services/platform/app/features/settings/governance/components/budget-editor.tsx`:
- Around line 334-336: The formatCost function uses toLocaleString for currency
which violates the guideline; replace its implementation to call the existing
formatCurrency utility (imported from lib/utils/format/number) and pass
cents/100 (or a number in dollars) so formatting uses formatCurrency instead of
toLocaleString—update the import and change formatCost to delegate to
formatCurrency while preserving two-decimal currency output.
In
`@services/platform/app/features/settings/mcp-servers/components/__tests__/mcp-server-panel.test.tsx`:
- Around line 138-150: Update the test for McpServerPanel to actually exercise
the dropdown and edit-mode footer: simulate opening the actions menu (use
userEvent.click or fireEvent on the button found by role/name "actions menu"),
then assert that the "Edit" and "Delete" menu items are present and accessible;
additionally add a separate render of <McpServerPanel initialEditing={true} ...
/> (using makeServer(), vi.fn() for callbacks) and assert the edit-mode footer
elements appear to cover the new edit path. Use screen queries
(getByRole/getByText) for assertions and keep the existing props on the
component.
In
`@services/platform/app/features/settings/mcp-servers/components/mcp-server-panel.tsx`:
- Around line 200-229: The sheet/dialog in MCPServerPanel is missing an
accessible description; update the component that renders Sheet/DialogContent in
MCPServerPanel to provide a short descriptive string via the description prop or
aria-describedby (e.g., description="Edit server settings for
{server.displayName}") and ensure the description element is available to screen
readers (use your VisuallyHidden/ScreenReaderOnly component or an element with
sr-only class) so Screen Readers get context; locate the Sheet/DialogContent
usage in MCPServerPanel (the same component that uses onOpenChange and renders
the header with server.displayName) and pass the description there.
In
`@services/platform/app/features/settings/providers/components/provider-edit-panel.tsx`:
- Around line 219-223: The spinner shown when isPending renders Loader2 with the
Tailwind animate-spin class but doesn't respect prefers-reduced-motion; update
the provider-edit-panel component so the Loader2 respects reduced motion (e.g.,
add the Tailwind motion-reduce:animate-none utility or conditionally omit the
spin when prefers-reduced-motion is set) so the animate-spin is suppressed for
users who prefer reduced motion; ensure the change targets the isPending
rendering path that includes Loader2 and the accompanying providers.saving text.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx:
- Around line 218-225: Replace the plain <button> used to trigger
setPanelOpen(true) (the one rendering <Pencil /> and t('providers.editGeneral'))
with the shared Button component used across the app so it inherits min-size,
focus, and keyboard affordances; ensure you pass the appropriate props (e.g.,
size/variant/aria-label) and keep the onClick calling setPanelOpen(true) and the
icon+label composition, and apply the same replacement to the other plain
buttons noted (the similar buttons rendering remove/edit actions referenced in
the diff).
In `@services/platform/messages/de.json`:
- Around line 3726-3734: Duplicate governance.tabs map (containing keys
systemPrompt, budgets, featureControls, usage, pii, modelAccess, retention)
appears earlier and is overridden by a later governance.tabs; delete this
earlier governance.tabs block so the later definition (which includes the
retention label) is the single source of truth and the retention label is
available at runtime.
🪄 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: 45024e3c-69fe-4a32-b3a4-12201be1e507
⛔ Files ignored due to path filters (1)
services/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (34)
examples/providers/openrouter.secrets.jsonservices/platform/app/components/ui/forms/input.test.tsxservices/platform/app/components/ui/forms/input.tsxservices/platform/app/components/ui/navigation/tabs.tsxservices/platform/app/features/chat/components/message-bubble.tsxservices/platform/app/features/settings/account/components/account-form.tsxservices/platform/app/features/settings/audit-logs/components/audit-log-table.tsxservices/platform/app/features/settings/governance/components/budget-editor.tsxservices/platform/app/features/settings/governance/components/default-model-editor.tsxservices/platform/app/features/settings/governance/components/feature-flags-editor.tsxservices/platform/app/features/settings/governance/components/model-access-editor.tsxservices/platform/app/features/settings/governance/components/pii-config.tsxservices/platform/app/features/settings/governance/components/retention-editor.tsxservices/platform/app/features/settings/governance/components/select-trigger-button.tsxservices/platform/app/features/settings/governance/components/system-prompt-editor.tsxservices/platform/app/features/settings/governance/components/upload-policy-editor.tsxservices/platform/app/features/settings/mcp-servers/components/__tests__/mcp-server-card.test.tsxservices/platform/app/features/settings/mcp-servers/components/__tests__/mcp-server-panel.test.tsxservices/platform/app/features/settings/mcp-servers/components/mcp-server-card.tsxservices/platform/app/features/settings/mcp-servers/components/mcp-server-form.tsxservices/platform/app/features/settings/mcp-servers/components/mcp-server-panel.tsxservices/platform/app/features/settings/mcp-servers/components/mcp-servers.tsxservices/platform/app/features/settings/organization/components/member-add-dialog.tsxservices/platform/app/features/settings/organization/components/member-edit-dialog.tsxservices/platform/app/features/settings/providers/components/provider-edit-panel.tsxservices/platform/app/features/settings/teams/components/team-detail-dialog.tsxservices/platform/app/features/settings/teams/components/team-row-actions.tsxservices/platform/app/features/settings/teams/components/teams-table.tsxservices/platform/app/features/settings/teams/hooks/use-teams-table-config.tsxservices/platform/app/routes/dashboard/$id/settings/governance.tsxservices/platform/app/routes/dashboard/$id/settings/logs.tsxservices/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsxservices/platform/messages/de.jsonservices/platform/messages/en.json
💤 Files with no reviewable changes (1)
- services/platform/app/features/settings/audit-logs/components/audit-log-table.tsx
Comment on lines
+2
to
+13
| "apiKey": "ENC[AES256_GCM,data:Os9IU5ZlUhe7QWMhqrhsVIFNKVD2FzszIkHGOb9l8Oa8z57AZUek+XK7qM//OyY60sMQQjVkbnygU7175lgO05lXlWRLqWiksA==,iv:2rpxkfK022BbwBCDwhu2YvlbF+HXv+KPaLPu6dQWX/k=,tag:gqMQcSCc9Q2wGJaer5V3jw==,type:str]", | ||
| "sops": { | ||
| "kms": null, | ||
| "gcp_kms": null, | ||
| "azure_kv": null, | ||
| "hc_vault": null, | ||
| "age": [ | ||
| { | ||
| "recipient": "age1xsc5y9x0dref9kd6fwv2356pw2zl5s7gp5v6jam9h4q7mv6fm9aqumvqhj", | ||
| "enc": "-----BEGIN AGE ENCRYPTED FILE-----\nYWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSBPYnV2WjVHR3ZFSGRlYmxR\ncnUybXVRcWNrVFkrMVlnTkZ1bFhNRmk3WjBVCmtSZ2RsbFZCdWY5dk94RlhhU1dN\nQnZnejFvYW4vVTBOUWVBUHJRaEovQmcKLS0tIENlVUJmTjFDcXZjWjV4RitnZHlv\nN09UeFo5Y2xnZXRwQ3RabEc2QVgyckUK7u0Avecl9B628T56Np6gGxQ1+yCSRjXa\n8HBlTjMa4g1OR8d4isfZ9VE+lETdNaVlultd9F1GWEvgv/p7WxpeUg==\n-----END AGE ENCRYPTED FILE-----\n" | ||
| "recipient": "age18ylfcfvf9we4rc5hpza6n9tvhwuw55jfun9wdpd5uhux5rs3qf4q95y2y4", | ||
| "enc": "-----BEGIN AGE ENCRYPTED FILE-----\nYWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSByUU1FMndnUDAzeTZySW1x\nMFJwNEhuYXF4UlZZWGRQaS85SjNkbmVpMG44CjVhTEh4c1BNZ1VXZXN3R2cxeFBt\nVjB6Sm5BOXc1NFdCcGMzS1BEWDJHeWMKLS0tIEFUekUrOHFmMSsraFdMOVU4d3FJ\nTzdwWWtiNytmTVNRQW5MTExRVEk3MmcKHmSVxvF3Nz/yMGZ3lFY+oDcryUdoOT7/\nmxnY/U4ZX3sCkLphYdUVuAA6nqstfzVn57X1ND8vqaFXPn2lhVc00Q==\n-----END AGE ENCRYPTED FILE-----\n" | ||
| } | ||
| ], | ||
| "lastmodified": "2026-04-05T11:39:10Z", | ||
| "mac": "ENC[AES256_GCM,data:2OYyLhDFsCM/spi9pp6bUXwuShtuqumEv7brGMrcRMdziaX8tQTnEL8Kcd774ne3BFDg+sfAnX2SH7e66qUxXd1qMXJWNN96lkjStc422rilpXWvq29RDKjatuAf7qSZoF0rWFNK9sAeQ0J+6bHytQ5FT3DXnS0SoXozMxCvz3A=,iv:8fx48QGvx8Nudqo1ByyWnPsebKn6BpAHQzw5rvj7zdg=,tag:21dyPzSJgJNpnStSvxs+yA==,type:str]", | ||
| "pgp": null, | ||
| "lastmodified": "2026-04-12T18:34:34Z", | ||
| "mac": "ENC[AES256_GCM,data:dTQzeY02n+I8wUXpfhlHBAQ8EZgMmPo1lMKZFtvbyEl9VWgYc9Q9+Bde0g6hmpXJNKqNx4VNhiQV6NFl0oyQQNpaN6yAu9Ka1gs64eymLUwuiBigSvSoiBOoICGaHx9rNhdgu55vU3xCGpp1C4lwGdj6lpKYJ7Pefdc0ZS1rnLo=,iv:ZBoTH20q8nPOp5H11WQiiH/VrKMao2RcaBiu9TeSVx4=,tag:HiqFbmIQMpO5Id0S7ls6Gw==,type:str]", | ||
| "unencrypted_suffix": "_unencrypted", | ||
| "version": "3.9.4" | ||
| "version": "3.12.2" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only checks for potential key-material coupling and accidental secret-key commits.
set -euo pipefail
echo "1) Locate AGE private key patterns in repository (should be none):"
rg -n --hidden --no-ignore -P 'AGE-SECRET-KEY-1[0-9A-Z]+' || true
echo
echo "2) Locate references to this specific AGE recipient:"
rg -n --hidden --no-ignore 'age18ylfcfvf9we4rc5hpza6n9tvhwuw55jfun9wdpd5uhux5rs3qf4q95y2y4' || true
echo
echo "3) List committed provider secrets example files for manual provenance audit:"
fd -H '.*\.secrets\.json$' examples providers services || trueRepository: tale-project/tale
Length of output: 504
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Check git history for this file (last 5 commits) ==="
git log --oneline -5 -- examples/providers/openrouter.secrets.json 2>/dev/null || echo "No git history found"
echo
echo "=== 2) Check for README or documentation about secrets management ==="
fd -H -i 'readme|doc.*secret|secret.*doc' examples/ || true
echo
echo "=== 3) Search for comments in the examples directory about test/dummy secrets ==="
rg -n -i 'dummy|placeholder|test.*secret|example.*key' examples/ --type json --type md || true
echo
echo "=== 4) Check if sops/decryption is tested or documented ==="
fd -H -i 'sops\|secret' . --path '*/test*' --path '*/spec*' -o --path '*/*.test.*' --path '*/*.spec.*' | head -20 || true
echo
echo "=== 5) Look for any .env.example or pattern files ==="
fd -H -e 'example' -e 'template' examples/ || trueRepository: tale-project/tale
Length of output: 4746
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Full content of openrouter.secrets.json ==="
cat examples/providers/openrouter.secrets.json
echo
echo "=== 2) Check for any README or documentation in examples/providers ==="
ls -la examples/providers/ | grep -i readme || true
echo
echo "=== 3) Check for SOPS keys directory or configuration ==="
find . -type f -name '.sops.yaml' -o -name 'sops.yaml' -o -name '.sops.*' 2>/dev/null | head -20 || true
echo
echo "=== 4) Git show full commit details for the last change ==="
git show 62053de7 --stat | head -40 || true
echo
echo "=== 5) Search for test/development AGE keys or SOPS documentation ==="
rg -n -i 'age.*key\|sops.*test\|test.*sops' . --type md --type txt --type yaml 2>/dev/null | head -20 || trueRepository: tale-project/tale
Length of output: 3882
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Check how provider secrets are loaded/used ==="
rg -n -A 5 -B 5 'openrouter.secrets.json\|provider\.secrets\.apiKey' . --type ts --type tsx --type js --type jsx 2>/dev/null | head -40 || true
echo
echo "=== 2) Search for test fixtures or test setup with AGE keys ==="
rg -n -i 'test.*age\|fixture.*secret\|mock.*secret' . --type ts --type tsx --type js --type json 2>/dev/null | head -30 || true
echo
echo "=== 3) Look for file_actions.ts and how it loads secrets ==="
fd -H 'file_actions.ts' . -exec head -60 {} \; 2>/dev/null || true
echo
echo "=== 4) Check if there are integration tests that use this provider ==="
fd -H '.test.ts\|.spec.ts' . --path '*openrouter*' -o --path '*provider*' -exec wc -l {} \; 2>/dev/null | head -10 || true
echo
echo "=== 5) Search for any unencrypted example of what the API key looks like ==="
rg -n -i 'sk-\|openrouter\|api.key' examples/ --type json --type md 2>/dev/null | head -20 || trueRepository: tale-project/tale
Length of output: 391
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Verify repository structure ==="
find . -type d -name 'services' -o -type d -name 'convex' -o -type d -name 'providers' | head -20
echo
echo "=== 2) Search for ANY file that references .secrets.json ==="
rg -l 'secrets\.json' . --type ts --type tsx --type js --type json 2>/dev/null | head -20 || true
echo
echo "=== 3) List all files in examples/providers ==="
find examples/providers -type f 2>/dev/null || true
echo
echo "=== 4) Search for any provider-related code ==="
rg -l 'provider' . --type ts --type tsx --type js 2>/dev/null | head -20 || true
echo
echo "=== 5) Check .gitignore for secrets handling ==="
grep -i 'secret\|\.env' .gitignore 2>/dev/null || trueRepository: tale-project/tale
Length of output: 961
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Read openrouter.json (non-encrypted counterpart) ==="
cat examples/providers/openrouter.json
echo
echo "=== 2) Search for any documentation about example files ==="
rg -n -B 2 -A 2 'example\|openrouter' examples/providers/ --type json --type md 2>/dev/null || true
echo
echo "=== 3) Check if .secrets.json should be in .gitignore ==="
rg -n '\.secrets|secrets\.json' .gitignore || echo "Not found in .gitignore"
echo
echo "=== 4) Check services/platform/convex/providers for actual usage ==="
ls -la services/platform/convex/providers/ 2>/dev/null | head -20 || true
echo
echo "=== 5) Search for SOPS or encryption-related code ==="
rg -n 'sops\|decrypt\|encrypt' services/platform/convex/ --type ts --type tsx --type js 2>/dev/null | head -30 || trueRepository: tale-project/tale
Length of output: 7228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Search for secret or apiKey references in file_actions.ts ==="
rg -n -i 'secret\|apikey\|credential' services/platform/convex/providers/file_actions.ts | head -20 || true
echo
echo "=== 2) Read sections of file_actions.ts that mention provider ==="
rg -n -B 3 -A 10 'provider\|secret' services/platform/convex/providers/file_actions.ts 2>/dev/null | head -80 || true
echo
echo "=== 3) Check if there's documentation about how to set up providers ==="
find . -name '*README*' -o -name '*SETUP*' -o -name '*GUIDE*' 2>/dev/null | xargs grep -l 'provider\|secret\|openrouter' 2>/dev/null | head -10 || true
echo
echo "=== 4) Check if tests use or mock the secrets file ==="
rg -n 'openrouter\|\.secrets\|provider.*test' . --type ts --type tsx 2>/dev/null | head -20 || trueRepository: tale-project/tale
Length of output: 337
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Check README for provider/secret setup documentation ==="
rg -n -B 2 -A 5 'provider\|secret\|openrouter' ./README.md 2>/dev/null | head -40 || true
echo
echo "=== 2) Read first 100 lines of file_actions.ts to understand its purpose ==="
head -100 services/platform/convex/providers/file_actions.ts
echo
echo "=== 3) Look for imports in file_actions.ts ==="
head -30 services/platform/convex/providers/file_actions.ts | grep -E 'import|require'
echo
echo "=== 4) Search for any reference to json files in provider loading ==="
rg -n '\.json\|load.*file\|readFile' services/platform/convex/providers/ --type ts --type tsx 2>/dev/null | head -20 || trueRepository: tale-project/tale
Length of output: 4025
🏁 Script executed:
#!/bin/bash
set -euco pipefail
echo "=== 1) Read more of file_actions.ts to see secrets loading ==="
sed -n '80,150p' services/platform/convex/providers/file_actions.ts
echo
echo "=== 2) Look at the decryptSecretsFile function ==="
cat services/platform/convex/lib/sops.ts 2>/dev/null | head -80
echo
echo "=== 3) Check resolveProviderSecretsPath implementation ==="
rg -n -A 5 'resolveProviderSecretsPath' services/platform/convex/providers/file_utils.ts 2>/dev/null | head -20
echo
echo "=== 4) Search for how the example files in examples/providers are used ==="
rg -n 'examples/providers' . --type ts --type tsx --type md 2>/dev/null | head -20 || true
echo
echo "=== 5) Check if there's a test AGE key or setup instructions ==="
rg -n -i 'age.*test\|test.*key\|setup.*provider' . --type md --type txt --type sh 2>/dev/null | head -20 || trueRepository: tale-project/tale
Length of output: 4612
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Verify examples/providers is NOT the actual config directory ==="
rg -n 'TALE_CONFIG_DIR\|providers.*dir' . --type ts --type tsx --type md 2>/dev/null | head -20 || true
echo
echo "=== 2) Check what happens if decryption fails - is example used at all? ==="
sed -n '100,140p' services/platform/convex/providers/file_actions.ts
echo
echo "=== 3) Verify the example file cannot be decrypted without the AGE private key ==="
echo "Checking: Does the repo contain any AGE private keys?"
rg -n 'AGE-SECRET-KEY\|age.*private' . --type txt --type sh --type env 2>/dev/null || echo "No AGE private keys found"
echo
echo "=== 4) Search for documentation on setting up local development with providers ==="
find . -name '*.md' -o -name '*.txt' 2>/dev/null | xargs grep -l 'provider.*setup\|TALE_CONFIG_DIR\|local.*develop' 2>/dev/null | head -5 || trueRepository: tale-project/tale
Length of output: 1927
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Check docs/environment-reference.md for TALE_CONFIG_DIR ==="
grep -A 10 -B 2 'TALE_CONFIG_DIR\|providers' ./docs/environment-reference.md 2>/dev/null | head -40 || true
echo
echo "=== 2) Check README for provider setup instructions ==="
grep -A 10 -B 2 'provider\|TALE_CONFIG_DIR' ./README.md 2>/dev/null | head -50 || true
echo
echo "=== 3) Verify examples/ directory is documentation only ==="
ls -la examples/
echo
echo "=== 4) Confirm: the code loads from TALE_CONFIG_DIR, not examples/ ==="
rg -n 'TALE_CONFIG_DIR' services/platform/convex/providers/file_utils.ts 2>/dev/null | head -10 || trueRepository: tale-project/tale
Length of output: 2333
Add documentation clarifying that openrouter.secrets.json is a template example.
This file cannot be decrypted or used by the application without a matching AGE private key (which is not stored in the repository), so it poses no immediate security risk. However, since any encrypted secrets committed to a repository can be a source of confusion, consider adding an inline comment or updating docs/environment-reference.md to explicitly state that examples/providers/openrouter.secrets.json is a template showing the expected SOPS-encrypted structure, and that users should generate their own via tale init.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/providers/openrouter.secrets.json` around lines 2 - 13, Add an
explicit inline comment at the top of examples/providers/openrouter.secrets.json
stating it is a template SOPS-encrypted example (not decryptable without the AGE
private key) and not intended for direct use, and also update
docs/environment-reference.md to include a short note referencing that example
file and instructing users to generate their own encrypted secrets via the tale
init command (and never commit private keys).
Comment on lines
+501
to
+506
| <div | ||
| className={cn( | ||
| 'transition-opacity duration-200', | ||
| !enabled && 'pointer-events-none opacity-50', | ||
| )} | ||
| > |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add motion-reduce prefix for transition.
Same as in model-access-editor, the opacity transition should respect user preferences.
♻️ Suggested change
<div
className={cn(
- 'transition-opacity duration-200',
+ 'transition-opacity duration-200 motion-reduce:transition-none',
!enabled && 'pointer-events-none opacity-50',
)}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/governance/components/budget-editor.tsx`
around lines 501 - 506, The opacity transition needs to respect reduced-motion
preferences: in the div that builds className via cn(...) in budget-editor.tsx
(the block starting with className={cn('transition-opacity duration-200',
...)}), add the Tailwind motion-reduce prefix to disable the transition for
users who prefer reduced motion (e.g., include "motion-reduce:transition-none"
alongside the existing transition classes, matching the approach used in
model-access-editor).
Comment on lines
+569
to
+582
| <td className="px-3 py-2 text-right"> | ||
| {rule.maxTokens != null | ||
| ? rule.maxTokens.toLocaleString() | ||
| : '\u2014'} | ||
| </td> | ||
| <td className="px-3 py-2 text-right"> | ||
| {rule.maxCostCents != null | ||
| ? formatCost(rule.maxCostCents) | ||
| : '\u2014'} | ||
| </td> | ||
| <td className="px-3 py-2 text-right"> | ||
| {rule.maxRequests != null | ||
| ? rule.maxRequests.toLocaleString() | ||
| : '\u2014'} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Number formatting also uses toLocaleString().
Lines 571 and 581 use toLocaleString() for token and request counts. For consistency with the codebase patterns, consider using formatNumber from lib/utils/format/number (which is already imported in feature-flags-editor.tsx at line 27).
♻️ Use existing formatNumber utility
+import { formatNumber } from '@/lib/utils/format/number';
+
<td className="px-3 py-2 text-right">
{rule.maxTokens != null
- ? rule.maxTokens.toLocaleString()
+ ? formatNumber(rule.maxTokens)
: '\u2014'}
</td>
...
<td className="px-3 py-2 text-right">
{rule.maxRequests != null
- ? rule.maxRequests.toLocaleString()
+ ? formatNumber(rule.maxRequests)
: '\u2014'}
</td>📝 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
| <td className="px-3 py-2 text-right"> | |
| {rule.maxTokens != null | |
| ? rule.maxTokens.toLocaleString() | |
| : '\u2014'} | |
| </td> | |
| <td className="px-3 py-2 text-right"> | |
| {rule.maxCostCents != null | |
| ? formatCost(rule.maxCostCents) | |
| : '\u2014'} | |
| </td> | |
| <td className="px-3 py-2 text-right"> | |
| {rule.maxRequests != null | |
| ? rule.maxRequests.toLocaleString() | |
| : '\u2014'} | |
| <td className="px-3 py-2 text-right"> | |
| {rule.maxTokens != null | |
| ? formatNumber(rule.maxTokens) | |
| : '\u2014'} | |
| </td> | |
| <td className="px-3 py-2 text-right"> | |
| {rule.maxCostCents != null | |
| ? formatCost(rule.maxCostCents) | |
| : '\u2014'} | |
| </td> | |
| <td className="px-3 py-2 text-right"> | |
| {rule.maxRequests != null | |
| ? formatNumber(rule.maxRequests) | |
| : '\u2014'} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/governance/components/budget-editor.tsx`
around lines 569 - 582, Replace direct use of toLocaleString for numeric display
with the shared formatting utility: import formatNumber from
'lib/utils/format/number' (if not already imported) and use
formatNumber(rule.maxTokens) and formatNumber(rule.maxRequests) instead of
rule.maxTokens.toLocaleString() and rule.maxRequests.toLocaleString(); keep
existing null checks and leave formatCost(rule.maxCostCents) unchanged so
token/request cells use formatNumber while cost cell continues to use
formatCost.
Comment on lines
+476
to
+481
| <div | ||
| className={cn( | ||
| 'transition-opacity duration-200', | ||
| !enabled && 'pointer-events-none opacity-50', | ||
| )} | ||
| > |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add motion-reduce prefix for transition.
Consistent with other governance editors, add the motion-reduce override.
♻️ Suggested change
<div
className={cn(
- 'transition-opacity duration-200',
+ 'transition-opacity duration-200 motion-reduce:transition-none',
!enabled && 'pointer-events-none opacity-50',
)}
>📝 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
| <div | |
| className={cn( | |
| 'transition-opacity duration-200', | |
| !enabled && 'pointer-events-none opacity-50', | |
| )} | |
| > | |
| <div | |
| className={cn( | |
| 'transition-opacity duration-200 motion-reduce:transition-none', | |
| !enabled && 'pointer-events-none opacity-50', | |
| )} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/governance/components/feature-flags-editor.tsx`
around lines 476 - 481, The transition on the wrapper div in FeatureFlagsEditor
uses 'transition-opacity duration-200' but lacks the motion-reduce override;
update the className in the div inside feature-flags-editor.tsx (the block using
cn(...) with 'transition-opacity duration-200' and the pointer-events/opacity
conditional) to include the Tailwind motion-reduce utility (e.g.,
'motion-reduce:transition-none') so users with reduced motion preferences won't
see the opacity transition.
Comment on lines
+560
to
+578
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={() => openEditDialog(index)} | ||
| disabled={cannotManage} | ||
| aria-label={`${t('featureFlags.editRule')} ${index + 1}`} | ||
| > | ||
| <Pencil className="size-4" /> | ||
| </Button> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={() => removeRule(index)} | ||
| disabled={cannotManage} | ||
| aria-label={`${t('featureFlags.deleteRule')} ${index + 1}`} | ||
| > | ||
| <Trash2 className="size-4" /> | ||
| </Button> | ||
| </HStack> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use i18n interpolation for aria-labels instead of template literals.
The aria-labels concatenate translated text with the index using template literals. This can cause issues in RTL languages and doesn't follow i18n best practices. The budget-editor uses the proper pattern with interpolation:
// budget-editor.tsx (correct pattern)
aria-label={t('budgets.editRuleAriaLabel', { index: index + 1 })}
// feature-flags-editor.tsx (current - less flexible)
aria-label={`${t('featureFlags.editRule')} ${index + 1}`}♻️ Use i18n interpolation
<Button
variant="ghost"
size="icon"
onClick={() => openEditDialog(index)}
disabled={cannotManage}
- aria-label={`${t('featureFlags.editRule')} ${index + 1}`}
+ aria-label={t('featureFlags.editRuleAriaLabel', { index: index + 1 })}
>
<Pencil className="size-4" />
</Button>
<Button
variant="ghost"
size="icon"
onClick={() => removeRule(index)}
disabled={cannotManage}
- aria-label={`${t('featureFlags.deleteRule')} ${index + 1}`}
+ aria-label={t('featureFlags.deleteRuleAriaLabel', { index: index + 1 })}
>This requires adding the corresponding translation keys with {{index}} placeholders.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/governance/components/feature-flags-editor.tsx`
around lines 560 - 578, Replace the template-literal aria-labels on the
edit/delete Buttons in feature-flags-editor.tsx with i18n interpolation calls
(e.g., use t('featureFlags.editRuleAriaLabel', { index: index + 1 }) and
t('featureFlags.deleteRuleAriaLabel', { index: index + 1 })) instead of
`${t('featureFlags.editRule')} ${index + 1}`; update the Button props where
onClick handlers reference openEditDialog and removeRule to use these t(...)
calls, and add corresponding translation keys with a {{index}} placeholder in
the featureFlags namespace so RTL and other locales render correctly.
Comment on lines
+137
to
+150
| <button | ||
| key={group.key} | ||
| type="button" | ||
| onClick={() => handleGroupChange(group.key)} | ||
| className={cn( | ||
| 'rounded-md px-3 py-2 text-left text-sm font-medium transition-colors', | ||
| activeGroup === group.key | ||
| ? 'bg-muted text-foreground' | ||
| : 'text-muted-foreground hover:bg-muted/50 hover:text-foreground', | ||
| )} | ||
| > | ||
| {group.label} | ||
| </button> | ||
| ))} |
There was a problem hiding this comment.
Add visible focus state to navigation buttons.
Per coding guidelines, all interactive elements must have visible focus states. The current styling includes hover states but no focus-visible styling for keyboard navigation.
🔧 Add focus ring styling
<button
key={group.key}
type="button"
onClick={() => handleGroupChange(group.key)}
className={cn(
- 'rounded-md px-3 py-2 text-left text-sm font-medium transition-colors',
+ 'rounded-md px-3 py-2 text-left text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2',
activeGroup === group.key
? 'bg-muted text-foreground'
: 'text-muted-foreground hover:bg-muted/50 hover:text-foreground',
)}
>📝 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
| <button | |
| key={group.key} | |
| type="button" | |
| onClick={() => handleGroupChange(group.key)} | |
| className={cn( | |
| 'rounded-md px-3 py-2 text-left text-sm font-medium transition-colors', | |
| activeGroup === group.key | |
| ? 'bg-muted text-foreground' | |
| : 'text-muted-foreground hover:bg-muted/50 hover:text-foreground', | |
| )} | |
| > | |
| {group.label} | |
| </button> | |
| ))} | |
| <button | |
| key={group.key} | |
| type="button" | |
| onClick={() => handleGroupChange(group.key)} | |
| className={cn( | |
| 'rounded-md px-3 py-2 text-left text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2', | |
| activeGroup === group.key | |
| ? 'bg-muted text-foreground' | |
| : 'text-muted-foreground hover:bg-muted/50 hover:text-foreground', | |
| )} | |
| > | |
| {group.label} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/routes/dashboard/`$id/settings/governance.tsx around
lines 137 - 150, The navigation buttons rendered in the map (button with
key={group.key}, onClick={() => handleGroupChange(group.key)}, and className
using cn) lack a visible keyboard focus style; update the className for that
button to include accessible focus-visible classes (e.g.,
focus-visible:outline-none or outline, focus-visible:ring and
focus-visible:ring-offset with appropriate colors) so the active/focused button
shows a clear focus ring for keyboard users while preserving the existing
activeGroup conditional styles.
Comment on lines
+63
to
+70
| onSuccess: (data) => { | ||
| if (data.url) { | ||
| window.open(data.url, '_blank', 'noopener,noreferrer'); | ||
| } | ||
| toast({ | ||
| title: t('logs.audit.export.complete'), | ||
| description: data.fileName, | ||
| }); |
There was a problem hiding this comment.
Handle the nullable export URL before showing success.
requestExport can legally return url: null (services/platform/convex/audit_logs/actions.ts, Lines 13-43), but this branch still shows the “complete” toast. That leaves the user with a success message and no downloadable file.
Suggested guard
const exportAction = useConvexAction(api.audit_logs.actions.requestExport, {
onSuccess: (data) => {
- if (data.url) {
- window.open(data.url, '_blank', 'noopener,noreferrer');
- }
+ if (!data.url) {
+ toast({
+ title: t('logs.audit.export.error'),
+ variant: 'destructive',
+ });
+ return;
+ }
+ window.open(data.url, '_blank', 'noopener,noreferrer');
toast({
title: t('logs.audit.export.complete'),
description: data.fileName,
});📝 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
| onSuccess: (data) => { | |
| if (data.url) { | |
| window.open(data.url, '_blank', 'noopener,noreferrer'); | |
| } | |
| toast({ | |
| title: t('logs.audit.export.complete'), | |
| description: data.fileName, | |
| }); | |
| onSuccess: (data) => { | |
| if (!data.url) { | |
| toast({ | |
| title: t('logs.audit.export.error'), | |
| variant: 'destructive', | |
| }); | |
| return; | |
| } | |
| window.open(data.url, '_blank', 'noopener,noreferrer'); | |
| toast({ | |
| title: t('logs.audit.export.complete'), | |
| description: data.fileName, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/routes/dashboard/`$id/settings/logs.tsx around lines 63
- 70, The onSuccess handler for requestExport currently shows a "complete" toast
even when data.url is null; update the onSuccess in
services/platform/app/routes/dashboard/$id/settings/logs.tsx (the onSuccess
callback that uses toast, data.fileName, data.url and window.open) to first
check if data.url is non-null before opening the window and showing the
"complete" toast, and if data.url is null show a different toast/message (e.g.,
"export queued" or "no file available") so users aren't given a false success;
ensure you only call window.open when data.url is truthy and keep using
data.fileName in the appropriate toast.
Comment on lines
+96
to
+126
| <Tabs | ||
| defaultValue="audit" | ||
| className="flex min-h-0 flex-1 flex-col" | ||
| actions={ | ||
| isAdminUser ? ( | ||
| <div className="flex items-center gap-2"> | ||
| <Button | ||
| variant="secondary" | ||
| size="sm" | ||
| onClick={() => handleExport('csv')} | ||
| disabled={exportAction.isPending} | ||
| aria-label={t('logs.audit.export.csvLabel')} | ||
| > | ||
| {exportAction.isPending | ||
| ? t('logs.audit.export.inProgress') | ||
| : t('logs.audit.export.csv')} | ||
| </Button> | ||
| <Button | ||
| variant="secondary" | ||
| size="sm" | ||
| onClick={() => handleExport('json')} | ||
| disabled={exportAction.isPending} | ||
| aria-label={t('logs.audit.export.jsonLabel')} | ||
| > | ||
| {exportAction.isPending | ||
| ? t('logs.audit.export.inProgress') | ||
| : t('logs.audit.export.json')} | ||
| </Button> | ||
| </div> | ||
| ) : undefined | ||
| } |
There was a problem hiding this comment.
Keep the export actions scoped to the Audit tab.
These buttons live in the shared Tabs.actions area, so they remain visible on Activity and Errors while handleExport() still exports audit logs only. From those tabs the UI looks like it exports the current view, but it downloads a different dataset.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/routes/dashboard/`$id/settings/logs.tsx around lines 96
- 126, The export buttons are placed in the shared Tabs actions area so they
appear on all tabs; restrict them to the Audit tab by making the actions
conditional on the current Tabs value instead of always rendering when
isAdminUser is true. Read the active tab value from the Tabs component (e.g.,
control Tabs value via state or access the Tabs selected value) and render the
export Button group only when that value === "audit" (keep references to
handleExport and exportAction as-is), leaving isAdminUser gating in place so
only admins on the "audit" tab see and use the export controls.
Comment on lines
+417
to
+421
| {saving ? ( | ||
| <> | ||
| <Loader2 className="mr-2 size-4 animate-spin" /> | ||
| {t('providers.saving')} | ||
| </> |
There was a problem hiding this comment.
Add reduced-motion fallbacks to both new loaders.
Both spinners use animate-spin without a motion-reduce: override.
💡 Suggested fix
- <Loader2 className="mr-2 size-4 animate-spin" />
+ <Loader2 className="mr-2 size-4 animate-spin motion-reduce:animate-none" />Also applies to: 938-942
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx
around lines 417 - 421, The spinner elements use Tailwind's animate-spin without
respecting prefers-reduced-motion; update the Loader2 usage(s) (the ones
rendered when saving and the other instance around lines 938-942) to include a
reduced-motion override by adding the motion-reduce:animate-none class alongside
animate-spin so the spinner stops for users with prefers-reduced-motion set, and
ensure any text/visual fallback remains readable when animation is disabled.
Comment on lines
+801
to
839
| <Input | ||
| label={t('providers.dimensions')} | ||
| type="number" | ||
| value={form.dimensions} | ||
| onChange={(e) => | ||
| setForm((f) => ({ ...f, dimensions: e.target.value })) | ||
| } | ||
| placeholder="e.g., 1536" | ||
| /> | ||
| )} | ||
| <HStack gap={3}> | ||
| <Input | ||
| label="Input cost (USD / 1M tokens)" | ||
| type="number" | ||
| value={form.inputCostPerMillion} | ||
| onChange={(e) => | ||
| setForm((f) => ({ | ||
| ...f, | ||
| tags: checked | ||
| ? [...f.tags, tag] | ||
| : f.tags.filter((v) => v !== tag), | ||
| })); | ||
| }} | ||
| inputCostPerMillion: e.target.value, | ||
| })) | ||
| } | ||
| placeholder="e.g., 2.50" | ||
| min={0} | ||
| step={0.01} | ||
| /> | ||
| <Input | ||
| label="Output cost (USD / 1M tokens)" | ||
| type="number" | ||
| value={form.outputCostPerMillion} | ||
| onChange={(e) => | ||
| setForm((f) => ({ | ||
| ...f, | ||
| outputCostPerMillion: e.target.value, | ||
| })) | ||
| } | ||
| placeholder="e.g., 10.00" | ||
| min={0} | ||
| step={0.01} | ||
| /> |
There was a problem hiding this comment.
Localize the new model-form copy.
Lines 808, 813, 822, 827, and 836 hardcode user-facing English strings, so this sheet will partially fall back to English in translated locales.
As per coding guidelines "Do NOT hardcode text, use the translation hooks/functions instead for user-facing UI".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx
around lines 801 - 839, The Inputs under the model form are using hardcoded
English labels and placeholders (e.g., "Input cost (USD / 1M tokens)", "Output
cost (USD / 1M tokens)", "e.g., 2.50", "e.g., 10.00", and "e.g., 1536") —
replace those literal strings with the i18n translation function (t(...)) so the
UI uses localized copy; update the Input components (the ones bound to
form.dimensions, form.inputCostPerMillion, and form.outputCostPerMillion and
their label/placeholder props) to call t with appropriate translation
keys/messages instead of hardcoded English.
This was referenced Apr 18, 2026
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
SelectTriggerButtonfor consistent select-like triggers across governance editors; addedactionsslot toTabscomponent; fixed password input autocomplete attributeTest plan
?group=param)Summary by CodeRabbit
New Features
Improvements