refactor(platform): toast positioning, automations created column, UI polish#1678
Merged
Israeltheminer merged 7 commits intoMay 6, 2026
Conversation
… polish - Add `position` prop to toast (`top-right` default, `top-center` for auth flows) - Replace automations table version column with localized `created` date sourced from history snapshots, falling back to file birthtime/mtime - Wire automations table through DataTable's built-in loading/empty/infinite-scroll props instead of bespoke branches; introduce `icon-text` skeleton variant - Replace forgot-password helper text with a click-to-toast hint - Reorder integrations panel actions: test/delete in body, disconnect in footer - Migrate raw `<select>` and `<input>` usages in providers, agents, and prompts to shared Select/Input/SearchableSelect components - Drop `size="lg"` from auth pages, brighten default toast info icon - Add `searchPlaceholder` keys for products/customers/vendors/websites and `automations.entityLabel`; translate forgot-password toast in de/fr
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis PR introduces toast positioning support, expands skeleton types for data tables, refactors the automations workflow table with creation date tracking and infinite scroll, simplifies agent model selection, removes size constraints from authentication form inputs, and adds internationalization strings for search placeholders and entity labels across multiple languages. Backend changes include retrieving workflow creation timestamps via file metadata. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
services/platform/app/features/prompts/components/add-category-popover.tsx (1)
68-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd an explicit label for the category input.
This field is still placeholder-only after the
Inputswap, so it has no stable accessible name once the user starts typing. Please wire a translatedlabel(preferred) oraria-labelhere.As per coding guidelines "Forms must have labels paired with inputs via
htmlForor wrapping."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/platform/app/features/prompts/components/add-category-popover.tsx` around lines 68 - 83, The Input is missing an accessible label so once the placeholder disappears the field has no stable name; update the add-category popover to pair the input with a translated label (preferred) or add a translated aria-label via the same i18n function `t` used for the placeholder (e.g., `t('addCategory.inputLabel')`), ensuring the label is associated using htmlFor/wrapping the `Input` (or set `aria-label` on the `Input` if you must) while keeping existing handlers (`onChange` -> `setValue`, `onKeyDown` -> `handleAdd`/`handleCancel`) and props (`value`, `autoFocus`, `placeholder`) intact.services/platform/app/components/ui/data-table/data-table.tsx (1)
487-507:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
icon-textskeleton can be shadowed by first-column avatar fallbackOn Line 487, the first-column avatar fallback runs before the new
icon-textbranch, soskeleton.type = 'icon-text'won’t render in first columns unlesshasAvatar: falseis also set.Proposed fix
- } else if ( - hasAvatar === true || - skeletonType === 'avatar-text' || - (isFirstColumn && hasAvatar !== false) - ) { + } else if (skeletonType === 'icon-text') { + cellContent = ( + <HStack gap={3}> + <Skeleton className="size-4 shrink-0 rounded" /> + <Skeleton className="h-3.5 w-full max-w-48" /> + </HStack> + ); + } else if ( + hasAvatar === true || + skeletonType === 'avatar-text' || + (isFirstColumn && hasAvatar !== false) + ) { cellContent = ( <HStack gap={3}> <Skeleton className="size-8 shrink-0 rounded-md" /> <Stack gap={1} className="flex-1"> <Skeleton className="h-3.5 w-full max-w-48" /> <Skeleton className="h-3 w-2/3 max-w-24" /> </Stack> </HStack> ); - } else if (skeletonType === 'icon-text') { - cellContent = ( - <HStack gap={3}> - <Skeleton className="size-4 shrink-0 rounded" /> - <Skeleton className="h-3.5 w-full max-w-48" /> - </HStack> - ); } else if (align === 'right') {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/platform/app/components/ui/data-table/data-table.tsx` around lines 487 - 507, The first-column avatar fallback condition (the else if that checks hasAvatar, skeletonType === 'avatar-text', or (isFirstColumn && hasAvatar !== false)) runs before the skeletonType === 'icon-text' branch, preventing 'icon-text' from rendering for first-column cells; update that avatar/fallback condition in data-table.tsx so it excludes the icon-text case (for example, change (isFirstColumn && hasAvatar !== false) to (isFirstColumn && hasAvatar !== false && skeletonType !== 'icon-text')) or move the skeletonType === 'icon-text' branch above the avatar fallback so cellContent renders the icon-text HStack/Skeleton when skeletonType === 'icon-text'.services/platform/app/features/automations/components/automations-table.test.tsx (1)
34-75: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd assertions for the new created-date and table-state paths.
This spec only updates the fixture shape, so the PR’s functional changes—rendering
createdAtMsand delegating loading/empty behavior toDataTable—still go unverified. Please add at least one happy-path assertion for the created column plus one empty/loading case here.As per coding guidelines, "
**/*.test.{ts,tsx,js,jsx}: Every new feature and bug fix carries its test. Happy path, one edge case, one error condition at minimum.`"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/platform/app/features/automations/components/automations-table.test.tsx` around lines 34 - 75, Add assertions to the AutomationsTable test to verify the new created-date column and DataTable-driven loading/empty behavior: update the happy-path test (render(<AutomationsTable ... />)) to assert that the mocked workflow's createdAtMs value is rendered in the table (reference createdAtMs in the useListWorkflows mock and the AutomationsTable output) and add two small cases that render AutomationsTable while useListWorkflows returns isLoading: true (assert a loading state/message from DataTable) and while workflows: [] (assert the empty state/message from DataTable) — use the existing render/cleanup helpers and the mocked hooks (useListWorkflows, DataTable behavior) to drive these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/platform/app/components/ui/data-table/data-table-filters.tsx`:
- Around line 164-167: The search wrapper in DataTableFilters uses
wrapperClassName with default search.className 'w-full sm:max-w-[16rem]' which
does not match the skeleton's reserved width (sm:w-[18.75rem]) and causes a
header jump; update the default to use the same breakpoint width as the skeleton
(e.g., change 'w-full sm:max-w-[16rem]' to 'w-full sm:max-w-[18.75rem]') or
alternatively update the skeleton to sm:max-w-[16rem] so both wrapperClassName
(and search.className) and the skeleton use the exact same sm width to keep the
loading width in sync.
In
`@services/platform/app/features/automations/hooks/use-automations-table-config.tsx`:
- Around line 62-66: The current rendering uses a truthy check on
row.original.createdAtMs (variable ms) which treats 0 as missing; update the
condition in the cell renderer that calls formatDate(new Date(ms), 'medium') to
use a nullish check (e.g., ms !== null && ms !== undefined or the nullish
coalescing approach) so that createdAtMs = 0 is treated as a valid timestamp and
formatted instead of rendering '—'; locate the ms variable in
use-automations-table-config.tsx (the Text caption cell) and replace the truthy
check with a nullish check.
In
`@services/platform/app/features/settings/integrations/components/integration-panel.tsx`:
- Around line 264-268: The disconnect loading UI (where manage.isSubmitting is
true in integration-panel.tsx) is only visual; wrap or annotate the HStack that
contains Loader2 and the text so it provides screen-reader semantics by adding
role="status", aria-live="polite", and a descriptive aria-label (e.g.,
"Disconnecting integration") to that container element; ensure the visible text
remains for sighted users while the new attributes allow assistive tech to
announce the state.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx:
- Around line 1060-1067: The Select options in the providers settings are using
hardcoded labels; update the options array in
routes/dashboard/$id/settings/providers/$providerName.tsx (the options passed to
the Select) to use the translation hook (t) instead of string literals — e.g.,
replace `label: 'images-api'`, `label: 'chat-multimodal'` and the template label
using `t('providers.default')` with appropriate translation keys like
`t('providers.imagesApi')`, `t('providers.chatMultimodal')` and keep
`t('providers.default')` for the default variant; ensure the translation hook
(t) is in scope where the options array is defined.
- Around line 1050-1057: The onValueChange handler currently uses a type
assertion to coerce value into 'images-api' | 'chat-multimodal'; replace that
with an explicit guard: inside the onValueChange passed to setForm check if
value === 'images-api' or value === 'chat-multimodal' and set
imageGenerationMode to the matching string, otherwise set it to '' so unexpected
option values can't be assigned; update the handler where setForm and
imageGenerationMode are referenced to perform this explicit runtime check
instead of using "as".
In `@services/platform/convex/workflows/file_actions.ts`:
- Around line 75-106: The current resolveCreatedAtMs function (used by
listWorkflows) derives createdAt from prunable history in resolveHistoryDir
which drifts after MAX_HISTORY_ENTRIES; instead persist a stable creation
timestamp on first write and have resolveCreatedAtMs read that persisted value
first. Add logic in the workflow creation/save path to write a durable creation
timestamp (e.g., in workflow metadata or a .meta file alongside the workflow)
when the workflow is first created, update the code that saves snapshots to
never overwrite that field, and change resolveCreatedAtMs to check the persisted
metadata (and return it if present) before falling back to the existing
history/readdir/stat logic; reference resolveCreatedAtMs, resolveHistoryDir,
listWorkflows, and the MAX_HISTORY_ENTRIES pruning behavior when implementing
this change.
---
Outside diff comments:
In `@services/platform/app/components/ui/data-table/data-table.tsx`:
- Around line 487-507: The first-column avatar fallback condition (the else if
that checks hasAvatar, skeletonType === 'avatar-text', or (isFirstColumn &&
hasAvatar !== false)) runs before the skeletonType === 'icon-text' branch,
preventing 'icon-text' from rendering for first-column cells; update that
avatar/fallback condition in data-table.tsx so it excludes the icon-text case
(for example, change (isFirstColumn && hasAvatar !== false) to (isFirstColumn &&
hasAvatar !== false && skeletonType !== 'icon-text')) or move the skeletonType
=== 'icon-text' branch above the avatar fallback so cellContent renders the
icon-text HStack/Skeleton when skeletonType === 'icon-text'.
In
`@services/platform/app/features/automations/components/automations-table.test.tsx`:
- Around line 34-75: Add assertions to the AutomationsTable test to verify the
new created-date column and DataTable-driven loading/empty behavior: update the
happy-path test (render(<AutomationsTable ... />)) to assert that the mocked
workflow's createdAtMs value is rendered in the table (reference createdAtMs in
the useListWorkflows mock and the AutomationsTable output) and add two small
cases that render AutomationsTable while useListWorkflows returns isLoading:
true (assert a loading state/message from DataTable) and while workflows: []
(assert the empty state/message from DataTable) — use the existing
render/cleanup helpers and the mocked hooks (useListWorkflows, DataTable
behavior) to drive these assertions.
In `@services/platform/app/features/prompts/components/add-category-popover.tsx`:
- Around line 68-83: The Input is missing an accessible label so once the
placeholder disappears the field has no stable name; update the add-category
popover to pair the input with a translated label (preferred) or add a
translated aria-label via the same i18n function `t` used for the placeholder
(e.g., `t('addCategory.inputLabel')`), ensuring the label is associated using
htmlFor/wrapping the `Input` (or set `aria-label` on the `Input` if you must)
while keeping existing handlers (`onChange` -> `setValue`, `onKeyDown` ->
`handleAdd`/`handleCancel`) and props (`value`, `autoFocus`, `placeholder`)
intact.
🪄 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: c3bccd0a-0578-415a-9ff3-4382383487ff
📒 Files selected for processing (21)
packages/ui/src/components/feedback/toast.tsxservices/platform/app/components/ui/data-table/data-table-filters.tsxservices/platform/app/components/ui/data-table/data-table-skeleton.tsxservices/platform/app/components/ui/data-table/data-table.tsxservices/platform/app/components/ui/feedback/toaster.tsxservices/platform/app/features/agents/components/agent-create-dialog.tsxservices/platform/app/features/automations/components/automations-table.test.tsxservices/platform/app/features/automations/components/automations-table.tsxservices/platform/app/features/automations/hooks/use-automations-table-config.tsxservices/platform/app/features/chat/components/chat-history-sidebar.tsxservices/platform/app/features/prompts/components/add-category-popover.tsxservices/platform/app/features/settings/integrations/components/integration-panel.tsxservices/platform/app/routes/2fa-enroll.tsxservices/platform/app/routes/_auth/2fa.tsxservices/platform/app/routes/_auth/log-in.tsxservices/platform/app/routes/_auth/sign-up.tsxservices/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsxservices/platform/convex/workflows/file_actions.tsservices/platform/messages/de.jsonservices/platform/messages/en.jsonservices/platform/messages/fr.json
Comment on lines
164
to
167
| wrapperClassName={cn( | ||
| 'flex-1 sm:flex-none', | ||
| search.className ?? 'w-full sm:w-[18.75rem]', | ||
| search.className ?? 'w-full sm:max-w-[16rem]', | ||
| )} |
There was a problem hiding this comment.
Keep the loading width in sync with the live search field.
DataTableFilters now caps the search wrapper at sm:max-w-[16rem], but services/platform/app/components/ui/data-table/data-table-skeleton.tsx:270-272 still reserves sm:w-[18.75rem]. That brings back the header width jump the skeleton is meant to avoid.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/app/components/ui/data-table/data-table-filters.tsx` around
lines 164 - 167, The search wrapper in DataTableFilters uses wrapperClassName
with default search.className 'w-full sm:max-w-[16rem]' which does not match the
skeleton's reserved width (sm:w-[18.75rem]) and causes a header jump; update the
default to use the same breakpoint width as the skeleton (e.g., change 'w-full
sm:max-w-[16rem]' to 'w-full sm:max-w-[18.75rem]') or alternatively update the
skeleton to sm:max-w-[16rem] so both wrapperClassName (and search.className) and
the skeleton use the exact same sm width to keep the loading width in sync.
Comment on lines
+62
to
66
| const ms = row.original.createdAtMs; | ||
| return ( | ||
| <Text as="span" variant="caption" className="block text-right"> | ||
| {row.original.version} | ||
| <Text as="span" variant="caption"> | ||
| {ms ? formatDate(new Date(ms), 'medium') : '—'} | ||
| </Text> |
There was a problem hiding this comment.
Handle createdAtMs = 0 explicitly
Line 65 uses a truthy check (ms ? ...), which treats 0 as missing and renders —. Use a nullish check instead.
Proposed fix
- const ms = row.original.createdAtMs;
+ const ms = row.original.createdAtMs;
return (
<Text as="span" variant="caption">
- {ms ? formatDate(new Date(ms), 'medium') : '—'}
+ {ms !== undefined ? formatDate(new Date(ms), 'medium') : '—'}
</Text>
);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/features/automations/hooks/use-automations-table-config.tsx`
around lines 62 - 66, The current rendering uses a truthy check on
row.original.createdAtMs (variable ms) which treats 0 as missing; update the
condition in the cell renderer that calls formatDate(new Date(ms), 'medium') to
use a nullish check (e.g., ms !== null && ms !== undefined or the nullish
coalescing approach) so that createdAtMs = 0 is treated as a valid timestamp and
formatted instead of rendering '—'; locate the ms variable in
use-automations-table-config.tsx (the Text caption cell) and replace the truthy
check with a nullish check.
Comment on lines
+264
to
+268
| {manage.isSubmitting ? ( | ||
| <HStack gap={2} align="center"> | ||
| <Loader2 className="size-3.5 animate-spin" /> | ||
| {t('integrations.disconnecting')} | ||
| </HStack> |
There was a problem hiding this comment.
Add accessible status semantics for disconnect loading state.
The submitting state currently relies on visual spinner/text only. Add role="status" + aria-live="polite" (and aria-label) so screen readers reliably announce the change.
Suggested patch
<button
type="button"
onClick={() => setConfirmDisconnect(true)}
disabled={manage.busy}
+ aria-busy={manage.isSubmitting}
className="text-muted-foreground hover:text-foreground text-sm font-medium transition-colors disabled:opacity-50"
>
{manage.isSubmitting ? (
- <HStack gap={2} align="center">
- <Loader2 className="size-3.5 animate-spin" />
- {t('integrations.disconnecting')}
+ <HStack
+ gap={2}
+ align="center"
+ role="status"
+ aria-live="polite"
+ aria-label={t('integrations.disconnecting')}
+ >
+ <Loader2 className="size-3.5 animate-spin" aria-hidden="true" />
+ <span>{t('integrations.disconnecting')}</span>
</HStack>
) : (
t('integrations.disconnect')
)}
</button>As per coding guidelines, "Dynamic content must use aria-live='polite' for non-urgent updates... spinners are role='status' with an aria-label."
📝 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
| {manage.isSubmitting ? ( | |
| <HStack gap={2} align="center"> | |
| <Loader2 className="size-3.5 animate-spin" /> | |
| {t('integrations.disconnecting')} | |
| </HStack> | |
| <button | |
| type="button" | |
| onClick={() => setConfirmDisconnect(true)} | |
| disabled={manage.busy} | |
| aria-busy={manage.isSubmitting} | |
| className="text-muted-foreground hover:text-foreground text-sm font-medium transition-colors disabled:opacity-50" | |
| > | |
| {manage.isSubmitting ? ( | |
| <HStack | |
| gap={2} | |
| align="center" | |
| role="status" | |
| aria-live="polite" | |
| aria-label={t('integrations.disconnecting')} | |
| > | |
| <Loader2 className="size-3.5 animate-spin" aria-hidden="true" /> | |
| <span>{t('integrations.disconnecting')}</span> | |
| </HStack> | |
| ) : ( | |
| t('integrations.disconnect') | |
| )} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/features/settings/integrations/components/integration-panel.tsx`
around lines 264 - 268, The disconnect loading UI (where manage.isSubmitting is
true in integration-panel.tsx) is only visual; wrap or annotate the HStack that
contains Loader2 and the text so it provides screen-reader semantics by adding
role="status", aria-live="polite", and a descriptive aria-label (e.g.,
"Disconnecting integration") to that container element; ensure the visible text
remains for sighted users while the new attributes allow assistive tech to
announce the state.
Comment on lines
+1050
to
+1057
| onValueChange={(value) => | ||
| setForm((f) => ({ | ||
| ...f, | ||
| imageGenerationMode: | ||
| value === 'default' | ||
| ? '' | ||
| : // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- value is one of the option values defined below | ||
| (value as 'images-api' | 'chat-multimodal'), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate and check the file
find . -type f -name "*$providerName.tsx" 2>/dev/null | head -20Repository: tale-project/tale
Length of output: 1105
🏁 Script executed:
# Read the specific lines from the file
if [ -f "services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx" ]; then
echo "File found using literal path"
sed -n '1045,1060p' "services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx"
else
# Search for the file
fd -t f '$providerName.tsx' services/platform 2>/dev/null
fiRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Search for the pattern in provider files
rg -A 10 "imageGenerationMode" services/platform --type tsx --type ts -B 2Repository: tale-project/tale
Length of output: 89
🏁 Script executed:
# Search for imageGenerationMode pattern in tsx/ts files
rg "imageGenerationMode" services/platform --type-add 'tsx:*.{tsx}' --type-add 'ts:*.{ts}' -B 2 -A 10Repository: tale-project/tale
Length of output: 25526
🏁 Script executed:
# Simpler approach - just search in tsx files
rg "imageGenerationMode" services/platform -B 2 -A 10Repository: tale-project/tale
Length of output: 25526
🏁 Script executed:
# Get the exact lines 1045-1065 to verify the exact location
sed -n '1045,1065p' services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx | cat -nRepository: tale-project/tale
Length of output: 168
🏁 Script executed:
# Also check the Select component definition to understand what values it passes to onValueChange
fd -t f "Select" packages/ui/src/components/forms -type f | head -5Repository: tale-project/tale
Length of output: 232
🏁 Script executed:
# Find the actual file by searching for the pattern
find services/platform -name "*providerName.tsx" -type fRepository: tale-project/tale
Length of output: 141
🏁 Script executed:
# Also search for files in the dashboard routes
ls -la services/platform/app/routes/dashboard/*/settings/providers/ 2>/dev/null | head -20Repository: tale-project/tale
Length of output: 303
🏁 Script executed:
# Search more broadly
find services/platform/app/routes -name "*.tsx" | grep -i provider | head -10Repository: tale-project/tale
Length of output: 279
🏁 Script executed:
# Read lines 1045-1065 from the actual file
sed -n '1045,1065p' 'services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx' | cat -nRepository: tale-project/tale
Length of output: 1226
🏁 Script executed:
# Also search for the Select component
find packages/ui -name "*.tsx" -path "*/forms/*" -exec grep -l "Select" {} \;Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Read more lines to see all options in the Select
sed -n '1060,1080p' 'services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx' | cat -nRepository: tale-project/tale
Length of output: 988
🏁 Script executed:
# Check the Select component implementation to understand its type signature
rg -A 20 "export.*function.*Select" packages/ui/src/components/forms/Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Check Select component in packages/ui
find packages/ui -type f -name "select.tsx" -o -name "select.ts"Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Get more context around the type definition
sed -n '1,50p' 'services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx' | grep -A 10 "ModelFormState"Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Search for Select component definition to understand its prop types
rg "Select" packages/ui/src/components/forms --type ts --type tsx -B 2 -A 15 | head -100Repository: tale-project/tale
Length of output: 89
🏁 Script executed:
# Check if there are type definitions for the Select component
find packages/ui -name "*.ts" -o -name "*.tsx" | xargs grep -l "onValueChange" 2>/dev/null | head -5Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Search for Select component implementation
rg "export.*Select" packages/ui/src/components/forms -B 2 -A 20 | head -150Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Check the Select component file directly
find packages/ui -name "select*" 2>/dev/nullRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Also check the type of the form field
sed -n '30,100p' 'services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx' | cat -nRepository: tale-project/tale
Length of output: 3871
🏁 Script executed:
# Verify the exact lines and get context
sed -n '1048,1071p' 'services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx' | cat -nRepository: tale-project/tale
Length of output: 1290
🏁 Script executed:
# Check what the Select component's onValueChange handler receives
rg "onValueChange" packages/ui/src -B 3 -A 3 | head -50Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Verify the options array is static and check for other uses of imageGenerationMode in this file
rg "imageGenerationMode" 'services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx' -B 2 -A 2Repository: tale-project/tale
Length of output: 1324
Replace as assertion with explicit type guards.
Line 1057 uses as to narrow the type. Use an explicit guard so invalid values cannot be assigned if options change.
Proposed fix
onValueChange={(value) =>
setForm((f) => ({
...f,
imageGenerationMode:
value === 'default'
? ''
- : // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- value is one of the option values defined below
- (value as 'images-api' | 'chat-multimodal'),
+ : value === 'images-api' || value === 'chat-multimodal'
+ ? value
+ : '',
}))
}Per the coding guidelines, TypeScript code must never use as assertions. This fix eliminates the assertion while maintaining type safety through explicit guards, defaulting to empty string if the value unexpectedly differs from the defined options.
📝 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
| onValueChange={(value) => | |
| setForm((f) => ({ | |
| ...f, | |
| imageGenerationMode: | |
| value === 'default' | |
| ? '' | |
| : // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- value is one of the option values defined below | |
| (value as 'images-api' | 'chat-multimodal'), | |
| onValueChange={(value) => | |
| setForm((f) => ({ | |
| ...f, | |
| imageGenerationMode: | |
| value === 'default' | |
| ? '' | |
| : value === 'images-api' || value === 'chat-multimodal' | |
| ? value | |
| : '', | |
| })) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx
around lines 1050 - 1057, The onValueChange handler currently uses a type
assertion to coerce value into 'images-api' | 'chat-multimodal'; replace that
with an explicit guard: inside the onValueChange passed to setForm check if
value === 'images-api' or value === 'chat-multimodal' and set
imageGenerationMode to the matching string, otherwise set it to '' so unexpected
option values can't be assigned; update the handler where setForm and
imageGenerationMode are referenced to perform this explicit runtime check
instead of using "as".
Comment on lines
+1060
to
+1067
| options={[ | ||
| { | ||
| value: 'default', | ||
| label: `images-api (${t('providers.default')})`, | ||
| }, | ||
| { value: 'images-api', label: 'images-api' }, | ||
| { value: 'chat-multimodal', label: 'chat-multimodal' }, | ||
| ]} |
There was a problem hiding this comment.
Localize the Select option labels (currently hardcoded).
Lines 1063-1066 render user-facing text literals directly. These should come from translation keys to avoid locale drift.
Proposed fix
options={[
{
value: 'default',
- label: `images-api (${t('providers.default')})`,
+ label: t('providers.imageGenerationModeOptions.default'),
},
- { value: 'images-api', label: 'images-api' },
- { value: 'chat-multimodal', label: 'chat-multimodal' },
+ {
+ value: 'images-api',
+ label: t('providers.imageGenerationModeOptions.imagesApi'),
+ },
+ {
+ value: 'chat-multimodal',
+ label: t(
+ 'providers.imageGenerationModeOptions.chatMultimodal',
+ ),
+ },
]}As per coding guidelines: "No hardcoded user-facing strings. Always use the translation hook."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx
around lines 1060 - 1067, The Select options in the providers settings are using
hardcoded labels; update the options array in
routes/dashboard/$id/settings/providers/$providerName.tsx (the options passed to
the Select) to use the translation hook (t) instead of string literals — e.g.,
replace `label: 'images-api'`, `label: 'chat-multimodal'` and the template label
using `t('providers.default')` with appropriate translation keys like
`t('providers.imagesApi')`, `t('providers.chatMultimodal')` and keep
`t('providers.default')` for the default variant; ensure the translation hook
(t) is in scope where the options array is defined.
Comment on lines
+75
to
+106
| async function resolveCreatedAtMs( | ||
| orgSlug: string, | ||
| workflowSlug: string, | ||
| filePath: string, | ||
| ): Promise<number | undefined> { | ||
| const historyDir = resolveHistoryDir(orgSlug, workflowSlug); | ||
| const entries = await readdir(historyDir).catch((err: unknown) => { | ||
| if (err instanceof Error && 'code' in err && err.code === 'ENOENT') { | ||
| return [] as string[]; | ||
| } | ||
| console.warn( | ||
| `[listWorkflows] readdir history failed for ${workflowSlug}: ${err instanceof Error ? err.message : String(err)}`, | ||
| ); | ||
| return [] as string[]; | ||
| }); | ||
| const earliest = entries | ||
| .filter((e) => e.endsWith('.json')) | ||
| .map((e) => Number(e.replace('.json', '').split('-')[0])) | ||
| .filter((n) => Number.isFinite(n)) | ||
| .sort((a, b) => a - b)[0]; | ||
| if (earliest !== undefined) return earliest; | ||
|
|
||
| try { | ||
| const s = await stat(filePath); | ||
| const birth = s.birthtimeMs; | ||
| return birth && birth > 0 ? birth : s.mtimeMs; | ||
| } catch (err) { | ||
| console.warn( | ||
| `[listWorkflows] stat failed for ${workflowSlug}: ${err instanceof Error ? err.message : String(err)}`, | ||
| ); | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
createdAtMs will drift forward after history pruning.
This helper treats the oldest retained snapshot as the workflow's creation time, but history is pruned to MAX_HISTORY_ENTRIES. Once a workflow has been edited more than that, the "created" date becomes the timestamp of a later save, not the first save. Since listWorkflows now returns this value directly to the automations UI, the new created column will become incorrect over time. Persist a stable creation timestamp on first write instead of deriving it from prunable history.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/convex/workflows/file_actions.ts` around lines 75 - 106,
The current resolveCreatedAtMs function (used by listWorkflows) derives
createdAt from prunable history in resolveHistoryDir which drifts after
MAX_HISTORY_ENTRIES; instead persist a stable creation timestamp on first write
and have resolveCreatedAtMs read that persisted value first. Add logic in the
workflow creation/save path to write a durable creation timestamp (e.g., in
workflow metadata or a .meta file alongside the workflow) when the workflow is
first created, update the code that saves snapshots to never overwrite that
field, and change resolveCreatedAtMs to check the persisted metadata (and return
it if present) before falling back to the existing history/readdir/stat logic;
reference resolveCreatedAtMs, resolveHistoryDir, listWorkflows, and the
MAX_HISTORY_ENTRIES pruning behavior when implementing this change.
- use nullish check for createdAtMs so epoch 0 still renders - replace 'as' assertion in image-generation-mode select with explicit guard
Collaborator
Author
|
Addressing CodeRabbit feedback in 4e27f4f: Applied:
Skipped (with reason):
|
…toast-position-and-automations-created-column # Conflicts: # packages/ui/src/lib/format.test.ts
The CI runner takes >5s on the 80KB regex pass, well beyond the default vitest timeout. The test asserts correctness, not speed — giving it room avoids spurious failures on loaded runners.
Israeltheminer
deleted the
refactor/platform-ui-toast-position-and-automations-created-column
branch
5 tasks
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
positionprop to toast (top-rightdefault,top-centerfor auth flows: log in, sign up, 2FA, forgot-password hint)versioncolumn with a localizedcreateddate — derived from the earliest history snapshot epoch, falling back to file birthtime/mtime when no history exists yetDataTable's built-in loading / empty / infinite-scroll props instead of bespoke branches; introduce anicon-textskeleton variant<select>and<input>to sharedSelect/Input/SearchableSelectcomponents in providers, agents, promptssize="lg"overrides on auth pages, brighten the default toast info iconsearchPlaceholderkeys for products / customers / vendors / websites, plusautomations.entityLabel; translate the new forgot-password toast indeandfrPre-PR checklist
bun run check(format, lint, typecheck, all tests). Lint + typecheck pass. Test suite has 5 pre-existing unrelated failures (PII regex ReDoS perf budget + ahtml-encoding-snifferESM/CJS pool issue blocking 1 dashboard layout test). The touchedautomations-table.test.tsxcould not run locally due to the same pool issue — CI runs in a clean environment.services/platform/messages/{en,de,fr}.jsonfor new keys (forgotPasswordToast,entityLabel,searchPlaceholder× 4).bun run --filter @tale/docs lint— no docs changed.Test plan
SelectSummary by CodeRabbit
New Features
Improvements