Conversation
…update # Conflicts: # apps/webapp/app/components/logs/LogsTaskFilter.tsx # apps/webapp/app/components/runs/v3/RunFilters.tsx
|
WalkthroughThis pull request updates the web application's filter UI and UX across the dashboard, logs, metrics, runs, schedules, and other pages. Changes include adding keyboard shortcuts and tooltips to filter controls, refactoring status and task filters from on-demand dropdowns to always-visible "permanent" controls, extracting reusable filter dropdown patterns (notably Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const { value, replace } = useSearchParams(); | ||
| const currentValue = value(paramKey); | ||
|
|
||
| const [inputValue, setInputValue] = useState(currentValue); |
There was a problem hiding this comment.
🟡 IdFilterDropdown input retains stale typed value when popover is reopened without unmounting
The IdFilterDropdown component initializes inputValue with useState(currentValue) at SharedFilters.tsx:1033, but useState only uses its initializer on mount. When the component stays mounted (as in the new PermanentBatchIdFilter, PermanentWaitpointIdFilter, and PermanentIdempotencyKeyFilter), closing and reopening the popover does NOT reset inputValue to the current URL param. So if a user opens the popover, types a value, clicks Cancel, then reopens — the stale typed value persists instead of showing the current param value.
The old code avoided this because the dropdown components were inside a FilterMenu switch and unmounted when the popover closed (which re-ran the initializer on next mount). The new always-mounted pattern means state goes stale.
Affected components
PermanentBatchIdFilterinBatchFilters.tsx:251PermanentWaitpointIdFilterinWaitpointTokenFilters.tsx:418PermanentIdempotencyKeyFilterinWaitpointTokenFilters.tsx:502
A fix would be to sync inputValue when open transitions to true, e.g.:
useEffect(() => { if (open) setInputValue(currentValue); }, [open]);| const [inputValue, setInputValue] = useState(currentValue); | |
| const [inputValue, setInputValue] = useState(currentValue); | |
| // Reset input to the current URL value each time the popover opens | |
| useEffect(() => { | |
| if (open) setInputValue(currentValue); | |
| }, [open, currentValue]); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const { filteredItems } = useFuzzyFilter<TaskListItem>({ | ||
| items: tasks, | ||
| keys: ["slug", "filePath", "triggerSource"], | ||
| filterText: value("search") ?? "", | ||
| }); |
There was a problem hiding this comment.
🚩 Tasks page search changed from instant client-side to Enter-to-search with server round-trips
The tasks page (_index/route.tsx:178-182) replaced the local-only AnimatedSearchField (which filtered on every keystroke via useFuzzyFilter's internal state) with SearchInput (which writes to URL params only on Enter). The useFuzzyFilter hook now reads from value("search") which only updates after URL navigation. This has two effects: (1) filtering requires pressing Enter instead of being instant as-you-type, and (2) the URL param change triggers the Remix loader to re-run, re-fetching all task data from the server even though the loader at _index/route.tsx:130-145 does not use the search param. The same pattern applies to the environment variables page at environment-variables/route.tsx:268-277. This appears to be an intentional choice for UI consistency, but reviewers should confirm the UX tradeoff is acceptable, especially the unnecessary server round-trips.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/webapp/app/components/primitives/Select.tsx (1)
358-374:⚠️ Potential issue | 🟠 MajorTooltip is disabled when only
tooltipTitleis provided (no shortcut).
showTooltipbecomes truthy fortooltipTitle || shortcut, so theAriakit.Tooltiprenders, butdisabled={shortcut === undefined}keeps it suppressed whenever there's no shortcut. Since many call sites passshortcut={shortcut}(where shortcut is a variable that can be undefined), this will break tooltip display. Updatedisabledto enable the tooltip when eithertooltipTitleorshortcutis present.🛠️ Suggested fix
{showTooltip && ( <Ariakit.Tooltip - disabled={shortcut === undefined} + disabled={!tooltipTitle && shortcut === undefined} className="z-40 cursor-default rounded border border-charcoal-700 bg-background-bright px-2 py-1.5 text-xs" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/primitives/Select.tsx` around lines 358 - 374, The Tooltip is being rendered when showTooltip (tooltipTitle || shortcut) is truthy but remains disabled whenever shortcut is undefined; update the disabled prop on Ariakit.Tooltip so it is enabled when either tooltipTitle or shortcut is present (e.g., make disabled true only when both tooltipTitle and shortcut are absent). Locate the Ariakit.Tooltip usage and change disabled={shortcut === undefined} to a condition that reflects both tooltipTitle and shortcut (referencing the tooltipTitle, shortcut and showTooltip variables and the ShortcutKey usage to verify behavior).apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx (1)
65-89:⚠️ Potential issue | 🟡 MinorKeep the clear-all visibility check aligned with the active filters.
This local
hasFiltersonly looks atstatuses,tags,id, andidempotencyKey, so a time-only state (period/from/to) won't show the clear button even though submitting this form clears that time filter too. Reuse the incominghasFiltersprop here, or include the time params in the local check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx` around lines 65 - 89, The local hasFilters in WaitpointTokenFilters only checks "statuses", "tags", "id", and "idempotencyKey", so the clear-all button is hidden when only time filters are active; update the component to either use the incoming props.hasFilters instead of the local searchParams check, or extend the local check to include "period", "from", and "to" (i.e., inspect location.search for those keys). Modify the hasFilters reference used in the JSX (the conditional rendering that shows the Form and Button) to use the corrected boolean so time-only filters make the clear button visible; refer to the WaitpointTokenFilters function and the hasFilters variable to locate the change.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx (1)
447-489:⚠️ Potential issue | 🟠 MajorRe-use the shared create controls in the empty state.
When
totalWidgetCount === 0, this branch renders only a plain “Add chart” button and never mountsfilterAccessories. That drops the new “Add title” action entirely and also skips the shortcut/tooltip wiring you added to the shared controls, so empty dashboards now behave differently from populated ones.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx around lines 447 - 489, The empty-state branch (totalWidgetCount === 0) currently renders a lone Add chart Button and never mounts the shared controls (filterAccessories), which removes the "Add title" action and shortcut/tooltip wiring; fix this by reusing the same shared control set used by MetricDashboard: render filterAccessories (or the shared DashboardControls component) inside the empty-state InfoPanel accessory (or directly render the shared accessory node alongside the Add chart button) so that actions.openAddEditor, filterAccessories, and the shortcut/tooltip wiring are mounted for the empty dashboard just like for MetricDashboard.
🧹 Nitpick comments (8)
apps/webapp/app/components/metrics/ModelsFilter.tsx (1)
155-155: Droptext-text-brighton the disabled empty-state item.The other filter dropdowns in this PR (
QueuesFilter,ProvidersFilter,OperationsFilter) leave the "No X found"SelectItemwithouttext-text-bright, so the disabled placeholder reads as muted helper text. Applyingtext-text-brighthere makes ModelsFilter's empty state visually inconsistent.♻️ Proposed fix
- {filtered.length === 0 && <SelectItem disabled className="text-text-bright">No models found</SelectItem>} + {filtered.length === 0 && <SelectItem disabled>No models found</SelectItem>}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/metrics/ModelsFilter.tsx` at line 155, In ModelsFilter, remove the explicit "text-text-bright" class from the disabled empty-state SelectItem so it matches the other filters; locate the JSX that renders "{filtered.length === 0 && <SelectItem disabled className="text-text-bright">No models found</SelectItem>}" and change it to render the disabled SelectItem without the text-text-bright class (keep the disabled prop and inner text "No models found" intact).apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)
1035-1044: Drop the unnecessaryuseCallbackand a stale-closure dep.
applyis just an inline click handler used once at the JSX site (line 1090) — no dep array, no provider, no expensive derivation. Wrapping it inuseCallbackadds no benefit and the dep list is also missingclearSearchValue(the captured closure may become stale if the parent re-creates it).♻️ Suggested simplification
- const apply = useCallback(() => { + const apply = () => { clearSearchValue(); replace({ cursor: undefined, direction: undefined, [paramKey]: inputValue === "" ? undefined : inputValue?.toString(), }); setOpen(false); - }, [inputValue, replace, paramKey]); + };As per coding guidelines: "Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx` around lines 1035 - 1044, The apply function is unnecessarily wrapped in useCallback and captures a stale closure (missing clearSearchValue in deps); remove the useCallback wrapper and replace it with a plain inline function (or const apply = () => { ... } defined without useCallback) used at the JSX click site, ensuring it still calls clearSearchValue(), replace({ cursor: undefined, direction: undefined, [paramKey]: inputValue === "" ? undefined : inputValue?.toString() }), and setOpen(false); this eliminates the incorrect dependency array and stale-closure risk around clearSearchValue, inputValue, replace, paramKey, and setOpen.apps/webapp/app/hooks/useFuzzyFilter.ts (1)
36-61: Minor: clarify thatsetFilterTextis uncontrolled-only.In controlled mode (
filterTextprop is provided), the returnedsetFilterTextonly mutatesinternalFilterText, which is then shadowed by the controlled value and effectively a no-op. Consumers in controlled mode that wire this setter to anonChangewill see no updates unless they also propagate to the source offilterText. The current top-level JSDoc still describessetFilterTextas "Function to update the filter text", which could mislead future callers.📝 Suggested doc tweak
* `@returns` An object containing: * - filterText: The current filter text - * - setFilterText: Function to update the filter text + * - setFilterText: Updates the internal filter text. Ignored when the controlled + * `filterText` prop is provided — manage that state in the parent instead. * - filteredItems: The filtered array of items based on the current filter text🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/hooks/useFuzzyFilter.ts` around lines 36 - 61, The returned setter setFilterText currently only updates internalFilterText and is a no-op when controlledFilterText is provided; update useFuzzyFilter's API docs and behavior to avoid misleading callers: in the JSDoc for useFuzzyFilter (and the exported return shape) explicitly state that setFilterText only updates internal state when uncontrolled (i.e., when controlledFilterText is undefined) and will not override a provided controlledFilterText, and optionally replace the raw setInternalFilterText return with a conditional setter that either calls setInternalFilterText or, when controlledFilterText is present, logs a clear warning/returns a no-op; reference the symbols filterText, setFilterText, internalFilterText, controlledFilterText, and function useFuzzyFilter so reviewers can locate and apply the change.apps/webapp/app/components/runs/v3/ScheduleFilters.tsx (1)
75-85: Extract the"ALL"select value into a shared constant.This sentinel is duplicated across both permanent filters in comparisons, defaults, and option values. A single constant will keep the UI and URL handling from drifting the next time one of these filters changes.
As per coding guidelines, use named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons.Also applies to: 97-127, 166-176, 186-217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/ScheduleFilters.tsx` around lines 75 - 85, Define a single named constant for the sentinel select value (e.g., export const UNSET_VALUE = '__unset__') and replace all raw "ALL" string literals in ScheduleFilters (including inside handleChange, other change handlers, defaults, and option value attributes) with this constant so comparisons (selected === UNSET_VALUE), default values, and option value props stay in sync; update URL handling logic that sets/deletes the "type" param to treat UNSET_VALUE as meaning unset and ensure any tests or consumers reference the new constant as well.apps/webapp/app/components/runs/v3/RunFilters.tsx (2)
589-657: LGTM on the permanent status/task filter pattern.The
Ariakit.TooltipProvider+TooltipAnchor(render={<Ariakit.Select … />})+ programmatictriggerRef.current?.click()shortcut wiring is consistent acrossPermanentStatusFilter,PermanentTasksFilter,RootOnlyToggle, and the newScopeFilter— good consolidation. One small nit: theuseShortcutKeys({ shortcut: statusShortcut, ... })calls here don’t passdisabled, whileScopeFilterpassesdisabled: !shortcut; sincestatusShortcut/tasksShortcutare always-defined module-level constants, that’s fine, but consider extracting a tinyuseTriggerShortcut(triggerRef, shortcut)helper to remove the duplicated 7-line block in three places.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/RunFilters.tsx` around lines 589 - 657, The shortcut wiring in PermanentStatusFilter (and the similar blocks in PermanentTasksFilter, RootOnlyToggle, and ScopeFilter) is duplicated; extract a small helper useTriggerShortcut(triggerRef, shortcut) that calls useShortcutKeys with the same action (preventDefault/stopPropagation then triggerRef.current?.click()) and passes disabled: !shortcut, then replace the 7-line inline useShortcutKeys blocks in PermanentStatusFilter, PermanentTasksFilter, RootOnlyToggle, and ScopeFilter with calls to useTriggerShortcut using their trigger refs and shortcut constants to remove duplication.
59-61: Minor: import grouping.
ShortcutKey(a primitive component) is wedged between two hook imports (useShortcutKeysandtagsLoader). Move it up next to the other~/components/primitives/*imports for consistency with the rest of the file. (Skip if Prettier/import-sort isn’t enforced here.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/runs/v3/RunFilters.tsx` around lines 59 - 61, The import for the primitive component ShortcutKey is currently between two hook imports; move the ShortcutKey import so it sits with the other primitive/component imports (e.g., alongside other imports from ~/components/primitives/*) instead of between useShortcutKeys and tagsLoader—adjust the import order in RunFilters.tsx so useShortcutKeys and tagsLoader remain together and ShortcutKey is grouped with components.apps/webapp/app/components/metrics/ScopeFilter.tsx (2)
56-82: Minor: missingeslint-disable-next-lineon theas anycast.Line 62 uses
ref={triggerRef as any}but, unlike the equivalent pattern inRunFilters.tsx(e.g. lines 614-616 and 784-786), there is no// eslint-disable-next-line@typescript-eslint/no-explicit-any`` comment. If the repo's ESLint rules forbid explicitany, this will emit a lint error. Consider adding the same disable comment for consistency, or typing the ref as `HTMLDivElement` (since `Ariakit.Select` renders a `` here and `HTMLDivElement.click()` exists on `HTMLElement`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/metrics/ScopeFilter.tsx` around lines 56 - 82, The ref passed to Ariakit.Select uses an explicit any cast (ref={triggerRef as any}) which triggers the no-explicit-any lint rule; fix by either adding the same eslint disable comment used in RunFilters (// eslint-disable-next-line `@typescript-eslint/no-explicit-any`) directly above the ref usage, or better, change the triggerRef typing to a proper element type (e.g., HTMLDivElement | null) and remove the as any cast so Ariakit.Select receives a correctly typed ref; locate the usage of triggerRef and the Ariakit.Select render prop to apply the change.
22-44: Optional: enforceonValueChangeat the type level when controlled.The JSDoc says
onValueChangeis "Required whenvalueis provided", but the type allowsvaluewithoutonValueChange, andhandleChangethen silently no-ops viaonValueChange?.(...). Consider a discriminated union so misuse is caught at compile time:♻️ Proposed refactor
-export type ScopeFilterProps = { - shortcut?: ShortcutDefinition; - /** Controlled value. If provided, the filter uses controlled mode and ignores search params. */ - value?: QueryScope; - /** Called when the user selects a new scope. Required when `value` is provided. */ - onValueChange?: (scope: QueryScope) => void; -}; +export type ScopeFilterProps = + | { + shortcut?: ShortcutDefinition; + value?: undefined; + onValueChange?: undefined; + } + | { + shortcut?: ShortcutDefinition; + /** Controlled value; the filter ignores search params in this mode. */ + value: QueryScope; + /** Called when the user selects a new scope. */ + onValueChange: (scope: QueryScope) => void; + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/metrics/ScopeFilter.tsx` around lines 22 - 44, ScopeFilterProps should be a discriminated union so providing value requires onValueChange at the type level; replace the current interface with a union like Controlled (has value: QueryScope and onValueChange: (scope: QueryScope) => void) | Uncontrolled (no value and optional/absent onValueChange) and update the ScopeFilter signature to accept that union (keep shortcut optional), then simplify handleChange to call onValueChange(newScope) without optional chaining when in the controlled branch and only call replace(...) in the uncontrolled branch; refer to the ScopeFilterProps type, the ScopeFilter function parameters, and the handleChange implementation when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/webapp/app/components/metrics/ModelsFilter.tsx`:
- Line 19: The disabled "No models found" SelectItem in the ModelsFilter
component is using the bright utility class `text-text-bright`; remove that
class so the empty-state disabled item is muted like in QueuesFilter and
ProvidersFilter. Locate the SelectItem rendering for the "No models found"
option inside the ModelsFilter component and delete the `text-text-bright` class
(or replace it with the same muted class used by the other filters) to restore
visual consistency.
In `@apps/webapp/app/components/navigation/DashboardDialogs.tsx`:
- Around line 6-7: Remove the unused imports useRef and useShortcutKeys from the
top of DashboardDialogs.tsx and keep only the used symbols; replace the current
import so it imports useEffect and useState plus the ShortcutDefinition type
(preferably as a type-only import: import type { ShortcutDefinition } ...) since
ShortcutDefinition is only used as a type for the shortcut parameter in the
function referenced by ShortcutDefinition; ensure no other references to useRef
or useShortcutKeys remain in the file.
In `@apps/webapp/app/components/primitives/SearchInput.tsx`:
- Around line 100-110: The clear button currently uses onPointerDown with
preventDefault which preserves input focus for mouse but breaks keyboard
activation; change the button to use onMouseDown={(e) => e.preventDefault()} to
keep focus on mouse and add an onClick={() => handleClear()} so keyboard
Enter/Space triggers the action; update the button that renders <XMarkIcon />
and the handler reference handleClear accordingly, and apply the same
onMouseDown + onClick pattern to the analogous control in AIFilterInput (the
same clear button logic) to restore keyboard accessibility.
In `@apps/webapp/app/components/runs/v3/BatchFilters.tsx`:
- Around line 230-233: The validateBatchId function currently checks for lengths
27 or 31 but returns the incorrect message "27/32 characters long"; update the
error string in validateBatchId to read "27 or 31 characters long" so it matches
the allowed lengths and aligns with the similar RunFilters.tsx validator; ensure
the two conditions remain the same (startsWith "batch_" and length check) and
only the returned message is changed.
- Around line 184-187: The Ariakit.Select usage in BatchFilters.tsx is casting
triggerRef to any (ref={triggerRef as any}) which bypasses type safety; remove
the unnecessary casts and any related eslint-disable comments and use
ref={triggerRef} directly since triggerRef is a useRef<HTMLButtonElement> (same
pattern as TimeFilter in SharedFilters.tsx). Apply this change to both the
Status filter and the batch-id filter Ariakit.Select instances (and consider
doing the same cleanup in RunFilters.tsx for its triggerRef usages). Ensure
imports/types remain unchanged and the components compile without the casts.
In `@apps/webapp/app/components/runs/v3/RunFilters.tsx`:
- Around line 1640-1643: The validator validateScheduleId currently uses
startsWith("sched") which permits IDs without the required underscore; update
the check to startsWith("sched_") so it enforces the placeholder/error text,
keeping the existing length check and return messages (i.e., ensure
validateScheduleId uses startsWith("sched_") and still returns "Schedule IDs
start with 'sched_'" on failure).
- Around line 1497-1532: The RootOnlyToggle currently only updates the rootOnly
param and must also clear pagination so old cursors don't point past the
filtered results: in RootOnlyToggle (useSearchParams, replace) update the
replace call to remove/reset the "cursor" and "direction" params whenever
rootOnly changes (i.e., when onCheckedChange runs), matching the behavior of
other handlers like TasksDropdown.handleChange and StatusDropdown.handleChange
by deleting or omitting "cursor" and "direction" from the new search params you
pass to replace.
- Around line 1534-1537: The validateRunId function currently checks for lengths
25 or 29 but returns an error message saying "25/30", causing inconsistency;
update the user-facing error text in validateRunId to match the actual allowed
lengths (e.g., return "Run IDs are 25/29 characters long") so the message
matches the check (function name: validateRunId).
In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx`:
- Around line 1029-1075: The inputValue state is initialized from currentValue
but never synced when currentValue changes externally, causing stale values in
the Input inside the SelectPopover; update the component to synchronize
inputValue with currentValue by adding an effect (watching currentValue) that
calls setInputValue(currentValue) and/or reset inputValue when the popover opens
(watch open) so Input shows the latest URL/search param state; reference the
inputValue/currentValue state variables, setInputValue setter, open/setOpen from
SelectProvider, and the SelectPopover/IdFilterDropdown wrapping this input to
locate where to add the useEffect.
In `@apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx`:
- Around line 303-313: The ComboBox is being unmounted when filtered.length ===
0 which hides the input; update the rendering condition around the ComboBox (the
JSX that currently uses !(filtered.length === 0 && fetcher.state !== "loading"))
to also keep the component mounted whenever searchValue is non-empty (e.g., only
hide when filtered is empty, fetcher is not loading, AND searchValue is empty).
Locate the ComboBox block in WaitpointTokenFilters.tsx and change the condition
so the input remains rendered while searchValue has content.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardKey/route.tsx:
- Around line 248-254: hasAppliedFilters currently only checks
tasks/queues/models/prompts/operations/providers and ignores time and scope
changes; update the logic in MetricDashboard where hasAppliedFilters is defined
(the const hasAppliedFilters) to also include checks for scope, period, from,
and to (e.g., scope !== defaultScope OR period !== defaultPeriod OR from/to
timestamps present or different from defaults) so that changing dashboard scope
or time range counts as an applied filter; make the same change to the other
occurrence of hasAppliedFilters in this file to ensure the “Clear all filters”
button appears when only scope/time range changes.
---
Outside diff comments:
In `@apps/webapp/app/components/primitives/Select.tsx`:
- Around line 358-374: The Tooltip is being rendered when showTooltip
(tooltipTitle || shortcut) is truthy but remains disabled whenever shortcut is
undefined; update the disabled prop on Ariakit.Tooltip so it is enabled when
either tooltipTitle or shortcut is present (e.g., make disabled true only when
both tooltipTitle and shortcut are absent). Locate the Ariakit.Tooltip usage and
change disabled={shortcut === undefined} to a condition that reflects both
tooltipTitle and shortcut (referencing the tooltipTitle, shortcut and
showTooltip variables and the ShortcutKey usage to verify behavior).
In `@apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx`:
- Around line 65-89: The local hasFilters in WaitpointTokenFilters only checks
"statuses", "tags", "id", and "idempotencyKey", so the clear-all button is
hidden when only time filters are active; update the component to either use the
incoming props.hasFilters instead of the local searchParams check, or extend the
local check to include "period", "from", and "to" (i.e., inspect location.search
for those keys). Modify the hasFilters reference used in the JSX (the
conditional rendering that shows the Form and Button) to use the corrected
boolean so time-only filters make the clear button visible; refer to the
WaitpointTokenFilters function and the hasFilters variable to locate the change.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsx:
- Around line 447-489: The empty-state branch (totalWidgetCount === 0) currently
renders a lone Add chart Button and never mounts the shared controls
(filterAccessories), which removes the "Add title" action and shortcut/tooltip
wiring; fix this by reusing the same shared control set used by MetricDashboard:
render filterAccessories (or the shared DashboardControls component) inside the
empty-state InfoPanel accessory (or directly render the shared accessory node
alongside the Add chart button) so that actions.openAddEditor,
filterAccessories, and the shortcut/tooltip wiring are mounted for the empty
dashboard just like for MetricDashboard.
---
Nitpick comments:
In `@apps/webapp/app/components/metrics/ModelsFilter.tsx`:
- Line 155: In ModelsFilter, remove the explicit "text-text-bright" class from
the disabled empty-state SelectItem so it matches the other filters; locate the
JSX that renders "{filtered.length === 0 && <SelectItem disabled
className="text-text-bright">No models found</SelectItem>}" and change it to
render the disabled SelectItem without the text-text-bright class (keep the
disabled prop and inner text "No models found" intact).
In `@apps/webapp/app/components/metrics/ScopeFilter.tsx`:
- Around line 56-82: The ref passed to Ariakit.Select uses an explicit any cast
(ref={triggerRef as any}) which triggers the no-explicit-any lint rule; fix by
either adding the same eslint disable comment used in RunFilters (//
eslint-disable-next-line `@typescript-eslint/no-explicit-any`) directly above the
ref usage, or better, change the triggerRef typing to a proper element type
(e.g., HTMLDivElement | null) and remove the as any cast so Ariakit.Select
receives a correctly typed ref; locate the usage of triggerRef and the
Ariakit.Select render prop to apply the change.
- Around line 22-44: ScopeFilterProps should be a discriminated union so
providing value requires onValueChange at the type level; replace the current
interface with a union like Controlled (has value: QueryScope and onValueChange:
(scope: QueryScope) => void) | Uncontrolled (no value and optional/absent
onValueChange) and update the ScopeFilter signature to accept that union (keep
shortcut optional), then simplify handleChange to call onValueChange(newScope)
without optional chaining when in the controlled branch and only call
replace(...) in the uncontrolled branch; refer to the ScopeFilterProps type, the
ScopeFilter function parameters, and the handleChange implementation when making
the change.
In `@apps/webapp/app/components/runs/v3/RunFilters.tsx`:
- Around line 589-657: The shortcut wiring in PermanentStatusFilter (and the
similar blocks in PermanentTasksFilter, RootOnlyToggle, and ScopeFilter) is
duplicated; extract a small helper useTriggerShortcut(triggerRef, shortcut) that
calls useShortcutKeys with the same action (preventDefault/stopPropagation then
triggerRef.current?.click()) and passes disabled: !shortcut, then replace the
7-line inline useShortcutKeys blocks in PermanentStatusFilter,
PermanentTasksFilter, RootOnlyToggle, and ScopeFilter with calls to
useTriggerShortcut using their trigger refs and shortcut constants to remove
duplication.
- Around line 59-61: The import for the primitive component ShortcutKey is
currently between two hook imports; move the ShortcutKey import so it sits with
the other primitive/component imports (e.g., alongside other imports from
~/components/primitives/*) instead of between useShortcutKeys and
tagsLoader—adjust the import order in RunFilters.tsx so useShortcutKeys and
tagsLoader remain together and ShortcutKey is grouped with components.
In `@apps/webapp/app/components/runs/v3/ScheduleFilters.tsx`:
- Around line 75-85: Define a single named constant for the sentinel select
value (e.g., export const UNSET_VALUE = '__unset__') and replace all raw "ALL"
string literals in ScheduleFilters (including inside handleChange, other change
handlers, defaults, and option value attributes) with this constant so
comparisons (selected === UNSET_VALUE), default values, and option value props
stay in sync; update URL handling logic that sets/deletes the "type" param to
treat UNSET_VALUE as meaning unset and ensure any tests or consumers reference
the new constant as well.
In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx`:
- Around line 1035-1044: The apply function is unnecessarily wrapped in
useCallback and captures a stale closure (missing clearSearchValue in deps);
remove the useCallback wrapper and replace it with a plain inline function (or
const apply = () => { ... } defined without useCallback) used at the JSX click
site, ensuring it still calls clearSearchValue(), replace({ cursor: undefined,
direction: undefined, [paramKey]: inputValue === "" ? undefined :
inputValue?.toString() }), and setOpen(false); this eliminates the incorrect
dependency array and stale-closure risk around clearSearchValue, inputValue,
replace, paramKey, and setOpen.
In `@apps/webapp/app/hooks/useFuzzyFilter.ts`:
- Around line 36-61: The returned setter setFilterText currently only updates
internalFilterText and is a no-op when controlledFilterText is provided; update
useFuzzyFilter's API docs and behavior to avoid misleading callers: in the JSDoc
for useFuzzyFilter (and the exported return shape) explicitly state that
setFilterText only updates internal state when uncontrolled (i.e., when
controlledFilterText is undefined) and will not override a provided
controlledFilterText, and optionally replace the raw setInternalFilterText
return with a conditional setter that either calls setInternalFilterText or,
when controlledFilterText is present, logs a clear warning/returns a no-op;
reference the symbols filterText, setFilterText, internalFilterText,
controlledFilterText, and function useFuzzyFilter so reviewers can locate and
apply the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9454039f-f83a-4d5c-8fbb-14476a32d9cc
📒 Files selected for processing (45)
apps/webapp/app/components/MachineLabelCombo.tsxapps/webapp/app/components/logs/LogsLevelFilter.tsxapps/webapp/app/components/logs/LogsRunIdFilter.tsxapps/webapp/app/components/logs/LogsTaskFilter.tsxapps/webapp/app/components/logs/LogsVersionFilter.tsxapps/webapp/app/components/metrics/ModelsFilter.tsxapps/webapp/app/components/metrics/OperationsFilter.tsxapps/webapp/app/components/metrics/PromptsFilter.tsxapps/webapp/app/components/metrics/ProvidersFilter.tsxapps/webapp/app/components/metrics/QueuesFilter.tsxapps/webapp/app/components/metrics/ScopeFilter.tsxapps/webapp/app/components/navigation/DashboardDialogs.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/components/primitives/Buttons.tsxapps/webapp/app/components/primitives/Input.tsxapps/webapp/app/components/primitives/SearchInput.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/Table.tsxapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/components/runs/v3/AIFilterInput.tsxapps/webapp/app/components/runs/v3/BatchFilters.tsxapps/webapp/app/components/runs/v3/RunFilters.tsxapps/webapp/app/components/runs/v3/ScheduleFilters.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/runs/v3/TaskRunsTable.tsxapps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsxapps/webapp/app/hooks/useFuzzyFilter.tsapps/webapp/app/presenters/v3/BuiltInDashboards.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.batches/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardKey/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.custom.$dashboardId/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.models._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.test/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules.new/route.tsxapps/webapp/app/services/runsRepository/runsRepository.server.ts
💤 Files with no reviewable changes (1)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens/route.tsx
| import { AnthropicLogoIcon } from "~/assets/icons/AnthropicLogoIcon"; | ||
|
|
||
| const shortcut = { key: "l" }; | ||
| const shortcut = { key: "m" }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all uses of `key: "m"` shortcut bindings to assess collision surface area.
rg -nP --type=ts --type=tsx -C2 $'\\bkey:\\s*[\"\\\']m[\"\\\']'
# Locate the Metadata shortcut definition to understand its scope (global vs route-scoped).
rg -nP -C5 'Metadata' apps/webapp/app/components/Shortcuts.tsxRepository: triggerdotdev/trigger.dev
Length of output: 751
🏁 Script executed:
# Search for where the Metadata shortcut is actually handled/triggered
rg -n "Metadata" apps/webapp/app/components --type ts --type tsx -A5 -B5 | head -100
# Check if there's a shortcut handler/listener for the metrics dashboard route
rg -n "shortcut" apps/webapp/app/routes --type ts --type tsx | grep -i metric | head -20
# Find the ModelsFilter component and check the styling of disabled "No models found" item
cat apps/webapp/app/components/metrics/ModelsFilter.tsx | head -160Repository: triggerdotdev/trigger.dev
Length of output: 4921
🏁 Script executed:
# Search for how shortcuts are implemented/handled in this codebase
fd "Shortcuts" apps/webapp/app/components --type f
rg -n "key.*m" apps/webapp/app/components/Shortcuts.tsx -B3 -A3
# Check how shortcut handlers work - look for event listeners
rg -n "useShortcut\|onKeyDown\|addEventListener" apps/webapp/app/components --type ts --type tsx | head -30Repository: triggerdotdev/trigger.dev
Length of output: 7750
🏁 Script executed:
# Compare styling in QueuesFilter, ProvidersFilter, OperationsFilter for disabled/empty state
fd "QueuesFilter|ProvidersFilter|OperationsFilter" apps/webapp/app/components --type f
rg -n "text-text-bright\|disabled" apps/webapp/app/components/metrics/QueuesFilter.tsx -B2 -A2 | head -40
rg -n "text-text-bright\|disabled" apps/webapp/app/components/metrics/ProvidersFilter.tsx -B2 -A2 | head -40Repository: triggerdotdev/trigger.dev
Length of output: 232
🏁 Script executed:
# Search for shortcut handler implementations - look for key event listeners or shortcut handlers
rg -n "useShortcut\|handleShortcut\|onKeyDown.*m" apps/webapp/app --type ts --type tsx | head -40
# Check if shortcut handlers are route-scoped or global
rg -n "key.*m\|Metadata" apps/webapp/app/routes --type ts --type tsx | grep -i metric | head -20
# Look at the actual QueuesFilter and ProvidersFilter to compare styling on disabled items
head -250 apps/webapp/app/components/metrics/QueuesFilter.tsx | tail -100
head -250 apps/webapp/app/components/metrics/ProvidersFilter.tsx | tail -100Repository: triggerdotdev/trigger.dev
Length of output: 6128
🏁 Script executed:
# Search for where the run page "Metadata" shortcut with "m" is actually implemented/handled
rg -n "\"m\"" apps/webapp/app/routes --type ts --type tsx -B5 -A5 | grep -i "metadata\|shortcut" | head -40Repository: triggerdotdev/trigger.dev
Length of output: 97
Remove text-text-bright from the disabled "No models found" SelectItem (L155).
QueuesFilter and ProvidersFilter leave their disabled empty-state items unstyled (muted appearance), but ModelsFilter adds text-text-bright here, creating visual inconsistency. Disabled helper text should be dimmed, not bright.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/metrics/ModelsFilter.tsx` at line 19, The disabled
"No models found" SelectItem in the ModelsFilter component is using the bright
utility class `text-text-bright`; remove that class so the empty-state disabled
item is muted like in QueuesFilter and ProvidersFilter. Locate the SelectItem
rendering for the "No models found" option inside the ModelsFilter component and
delete the `text-text-bright` class (or replace it with the same muted class
used by the other filters) to restore visual consistency.
| import { useEffect, useRef, useState } from "react"; | ||
| import { type ShortcutDefinition, useShortcutKeys } from "~/hooks/useShortcutKeys"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether useRef and useShortcutKeys are referenced in DashboardDialogs.tsx
fd -t f 'DashboardDialogs.tsx' --exec rg -n -C1 'useRef|useShortcutKeys' {}Repository: triggerdotdev/trigger.dev
Length of output: 357
🏁 Script executed:
#!/bin/bash
# Get the full content of DashboardDialogs.tsx and search for usage of useRef, useShortcutKeys, ShortcutDefinition
fd -t f 'DashboardDialogs.tsx' | head -1 | xargs wc -lRepository: triggerdotdev/trigger.dev
Length of output: 132
🏁 Script executed:
#!/bin/bash
# Read the full file to check for usage
fd -t f 'DashboardDialogs.tsx' | head -1 | xargs cat -n | head -200Repository: triggerdotdev/trigger.dev
Length of output: 8202
🏁 Script executed:
#!/bin/bash
# Search for usage of useRef and useShortcutKeys in the file
fd -t f 'DashboardDialogs.tsx' | head -1 | xargs rg -n 'useRef|useShortcutKeys' -A 2 -B 2Repository: triggerdotdev/trigger.dev
Length of output: 503
🏁 Script executed:
#!/bin/bash
# Also check if ShortcutDefinition type is used beyond the import
fd -t f 'DashboardDialogs.tsx' | head -1 | xargs rg -n 'ShortcutDefinition' -A 1 -B 1Repository: triggerdotdev/trigger.dev
Length of output: 387
🏁 Script executed:
#!/bin/bash
# Verify that shortcut parameter is actually used in CreateDashboardPageButton
fd -t f 'DashboardDialogs.tsx' | head -1 | xargs sed -n '118,150p'Repository: triggerdotdev/trigger.dev
Length of output: 1064
Remove unused imports useRef and useShortcutKeys.
Line 6 imports useRef and line 7 imports useShortcutKeys, but neither is referenced anywhere in the file. Only ShortcutDefinition (line 7) is actually used—as a type annotation for the shortcut parameter on line 127. Remove the unused imports and keep only ShortcutDefinition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/navigation/DashboardDialogs.tsx` around lines 6 -
7, Remove the unused imports useRef and useShortcutKeys from the top of
DashboardDialogs.tsx and keep only the used symbols; replace the current import
so it imports useEffect and useState plus the ShortcutDefinition type
(preferably as a type-only import: import type { ShortcutDefinition } ...) since
ShortcutDefinition is only used as a type for the shortcut parameter in the
function referenced by ShortcutDefinition; ensure no other references to useRef
or useShortcutKeys remain in the file.
| button={ | ||
| <button | ||
| type="button" | ||
| onPointerDown={(e) => { | ||
| e.preventDefault(); | ||
| handleClear(); | ||
| }} | ||
| className="flex size-4.5 items-center justify-center rounded-[2px] border border-text-dimmed/40 text-text-dimmed transition hover:bg-charcoal-600 hover:text-text-bright" | ||
| > | ||
| <XMarkIcon className="size-3" /> | ||
| </button> |
There was a problem hiding this comment.
Clear button is not keyboard-activatable.
Replacing onClick with onPointerDown (+ preventDefault) keeps focus in the input on mouse use, but it removes keyboard activation entirely — Enter/Space on a focused <button> only synthesizes pointer events for click, not pointerdown. Tabbing to this button and pressing Enter/Space will do nothing.
You also can't reliably reach this button: the clear icon only renders when text.length > 0, and Esc inside the input already clears (handled at lines 82–89), so most keyboard users never need it. Still, an interactive control that is unreachable/inert by keyboard is an a11y regression.
♿ Suggested fix — keep focus while still supporting keyboard activation
<button
type="button"
+ onMouseDown={(e) => e.preventDefault()}
- onPointerDown={(e) => {
- e.preventDefault();
- handleClear();
- }}
+ onClick={handleClear}
className="flex size-4.5 items-center justify-center rounded-[2px] border border-text-dimmed/40 text-text-dimmed transition hover:bg-charcoal-600 hover:text-text-bright"
>
<XMarkIcon className="size-3" />
</button>onMouseDown + preventDefault preserves the "don't steal focus from the input" behavior on mouse, while onClick covers both mouse clicks and keyboard activation. The same pattern applies in AIFilterInput.tsx (lines 144–153).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/primitives/SearchInput.tsx` around lines 100 -
110, The clear button currently uses onPointerDown with preventDefault which
preserves input focus for mouse but breaks keyboard activation; change the
button to use onMouseDown={(e) => e.preventDefault()} to keep focus on mouse and
add an onClick={() => handleClear()} so keyboard Enter/Space triggers the
action; update the button that renders <XMarkIcon /> and the handler reference
handleClear accordingly, and apply the same onMouseDown + onClick pattern to the
analogous control in AIFilterInput (the same clear button logic) to restore
keyboard accessibility.
| <Ariakit.Select | ||
| ref={triggerRef as any} | ||
| render={<div className="group cursor-pointer focus-custom" />} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look at how Ariakit.Select is rendered/typed in similar usages in the repo
rg -nP --type=tsx -C2 'Ariakit\.Select\b' apps/webapp/app | head -80Repository: triggerdotdev/trigger.dev
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Corrected search for Ariakit.Select usage (without explicit file type)
rg -nP -C2 'Ariakit\.Select\b' apps/webapp/app | head -100Repository: triggerdotdev/trigger.dev
Length of output: 10628
🏁 Script executed:
#!/bin/bash
# Read the BatchFilters.tsx file to see the actual ref definition and usages
wc -l apps/webapp/app/components/runs/v3/BatchFilters.tsxRepository: triggerdotdev/trigger.dev
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Read relevant sections of BatchFilters.tsx
cat -n apps/webapp/app/components/runs/v3/BatchFilters.tsx | sed -n '1,50p'Repository: triggerdotdev/trigger.dev
Length of output: 1910
🏁 Script executed:
#!/bin/bash
# Read more of BatchFilters to find the ref definitions and both instances
cat -n apps/webapp/app/components/runs/v3/BatchFilters.tsx | sed -n '170,200p'Repository: triggerdotdev/trigger.dev
Length of output: 1382
🏁 Script executed:
#!/bin/bash
# Check around line 276 for the second trigger ref usage
cat -n apps/webapp/app/components/runs/v3/BatchFilters.tsx | sed -n '260,290p'Repository: triggerdotdev/trigger.dev
Length of output: 1315
🏁 Script executed:
#!/bin/bash
# Find where triggerRef is defined in BatchFilters
rg -n 'triggerRef.*useRef' apps/webapp/app/components/runs/v3/BatchFilters.tsxRepository: triggerdotdev/trigger.dev
Length of output: 185
🏁 Script executed:
#!/bin/bash
# Read TimeFilter in SharedFilters to see its ref type
rg -nP -B5 -A10 'const TimeFilter' apps/webapp/app/components/runs/v3/SharedFilters.tsx | head -40Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Look for TimeFilter definition in SharedFilters.tsx
cat -n apps/webapp/app/components/runs/v3/SharedFilters.tsx | grep -A20 "^[^:]*:.*TimeFilter"Repository: triggerdotdev/trigger.dev
Length of output: 4074
🏁 Script executed:
#!/bin/bash
# Better search for TimeFilter function/export in SharedFilters
rg -n 'export.*TimeFilter|function TimeFilter' apps/webapp/app/components/runs/v3/SharedFilters.tsxRepository: triggerdotdev/trigger.dev
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Search for useRef in SharedFilters to find the TimeFilter ref
rg -n 'useRef' apps/webapp/app/components/runs/v3/SharedFilters.tsxRepository: triggerdotdev/trigger.dev
Length of output: 229
🏁 Script executed:
#!/bin/bash
# Find the actual Ariakit.Select usage in SharedFilters at line 408
cat -n apps/webapp/app/components/runs/v3/SharedFilters.tsx | sed -n '400,420p'Repository: triggerdotdev/trigger.dev
Length of output: 956
🏁 Script executed:
#!/bin/bash
# Check if there are TypeScript errors or if TimeFilter's ref works without as any
# Look at the build config or tsconfig to understand strictness
head -50 apps/webapp/tsconfig.jsonRepository: triggerdotdev/trigger.dev
Length of output: 869
🏁 Script executed:
#!/bin/bash
# Search for the Ariakit Select component definition to understand what ref type it expects
rg -n "Select.*forwardRef\|ref.*Select" node_modules/@ariakit/react 2>/dev/null | head -20Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Check if there's a type definition file for Ariakit Select
find apps/webapp -name "*.d.ts" -o -name "types.ts" | xargs rg -l "Ariakit.*Select\|Select.*ref" 2>/dev/null | head -5Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Verify the exact Ariakit.Select signature and what element it actually renders
# Check if there's a way to understand the ref type requirement
rg -n 'render=.*<div' apps/webapp/app/components/runs/v3/SharedFilters.tsx | head -10Repository: triggerdotdev/trigger.dev
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Check if TypeScript compilation passes for SharedFilters.tsx with the current code
# Look for any type errors in CI config or build scripts
cat apps/webapp/package.json | grep -A10 '"scripts"'Repository: triggerdotdev/trigger.dev
Length of output: 862
🏁 Script executed:
#!/bin/bash
# Examine what element type Ariakit.Select actually exposes when render is a div
# Search for any type narrowing or ref handling documentation
rg -n 'Ariakit\.Select' apps/webapp/app/components/primitives/Select.tsx -A5 | head -30Repository: triggerdotdev/trigger.dev
Length of output: 1301
🏁 Script executed:
#!/bin/bash
# Verify what the actual compiled/rendered element type is
# Check if there are any type stubs or definitions that explain Ariakit.Select's ref behavior
rg -n 'Select.*ref.*HTMLElement\|HTMLElement.*ref' apps/webapp --type=tsx | head -20Repository: triggerdotdev/trigger.dev
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Look at other filter components to see if any work without the as any cast
rg -n 'Ariakit\.Select' apps/webapp/app/components/runs/v3/RunFilters.tsx -B2 -A2 | head -50Repository: triggerdotdev/trigger.dev
Length of output: 1853
🏁 Script executed:
#!/bin/bash
# Double-check the specific lines in BatchFilters where the as any appears
sed -n '183,187p' apps/webapp/app/components/runs/v3/BatchFilters.tsx
sed -n '274,278p' apps/webapp/app/components/runs/v3/BatchFilters.tsxRepository: triggerdotdev/trigger.dev
Length of output: 595
Remove the unnecessary as any cast; useRef<HTMLButtonElement> works without it.
triggerRef as any defeats type safety. Ariakit's Select component accepts the HTMLButtonElement ref type when the underlying element is a <div> via render=. TimeFilter in SharedFilters.tsx demonstrates this pattern works without the cast using identical setup: useRef<HTMLButtonElement> with ref={triggerRef} (no cast). Remove the as any casts at lines 185 and 276 along with their eslint-disable comments.
♻️ Suggested fix
- const triggerRef = useRef<HTMLButtonElement>(null);
+ const triggerRef = useRef<HTMLDivElement>(null);
...
- // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
- <Ariakit.Select
- ref={triggerRef as any}
- render={<div className="group cursor-pointer focus-custom" />}
- />
+ <Ariakit.Select
+ ref={triggerRef}
+ render={<div className="group cursor-pointer focus-custom" />}
+ />Apply to both Status filter (line 185) and batch-id filter (line 276). Note: RunFilters.tsx has the same pattern at lines 616 and 786 and would benefit from the same cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/runs/v3/BatchFilters.tsx` around lines 184 - 187,
The Ariakit.Select usage in BatchFilters.tsx is casting triggerRef to any
(ref={triggerRef as any}) which bypasses type safety; remove the unnecessary
casts and any related eslint-disable comments and use ref={triggerRef} directly
since triggerRef is a useRef<HTMLButtonElement> (same pattern as TimeFilter in
SharedFilters.tsx). Apply this change to both the Status filter and the batch-id
filter Ariakit.Select instances (and consider doing the same cleanup in
RunFilters.tsx for its triggerRef usages). Ensure imports/types remain unchanged
and the components compile without the casts.
| function validateBatchId(value: string): string | undefined { | ||
| if (!value.startsWith("batch_")) return "Batch IDs start with 'batch_'"; | ||
| if (value.length !== 27 && value.length !== 31) return "Batch IDs are 27/32 characters long"; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm canonical batch id lengths produced by the codebase
rg -nP --type=ts -C2 "batch_" apps/webapp/app | rg -nP "length|toFriendlyId|fromFriendlyId" | head -50
ast-grep --pattern $'function $_($$$): $_ {
$$$
batch_$$$
$$$
}'Repository: triggerdotdev/trigger.dev
Length of output: 626
Fix error message mismatch in validateBatchId:
The validation check allows lengths 27 or 31, but the error message says "27/32 characters long". A similar validation function in RunFilters.tsx (line 1591) already has the correct message format. Align this to say "27 or 31 characters long".
Suggested fix
function validateBatchId(value: string): string | undefined {
if (!value.startsWith("batch_")) return "Batch IDs start with 'batch_'";
- if (value.length !== 27 && value.length !== 31) return "Batch IDs are 27/32 characters long";
+ if (value.length !== 27 && value.length !== 31) return "Batch IDs are 27 or 31 characters long";
}📝 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.
| function validateBatchId(value: string): string | undefined { | |
| if (!value.startsWith("batch_")) return "Batch IDs start with 'batch_'"; | |
| if (value.length !== 27 && value.length !== 31) return "Batch IDs are 27/32 characters long"; | |
| } | |
| function validateBatchId(value: string): string | undefined { | |
| if (!value.startsWith("batch_")) return "Batch IDs start with 'batch_'"; | |
| if (value.length !== 27 && value.length !== 31) return "Batch IDs are 27 or 31 characters long"; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/runs/v3/BatchFilters.tsx` around lines 230 - 233,
The validateBatchId function currently checks for lengths 27 or 31 but returns
the incorrect message "27/32 characters long"; update the error string in
validateBatchId to read "27 or 31 characters long" so it matches the allowed
lengths and aligns with the similar RunFilters.tsx validator; ensure the two
conditions remain the same (startsWith "batch_" and length check) and only the
returned message is changed.
| function validateRunId(value: string): string | undefined { | ||
| if (!value.startsWith("run_")) return "Run IDs start with 'run_'"; | ||
| if (value.length !== 25 && value.length !== 29) return "Run IDs are 25/30 characters long"; | ||
| } |
There was a problem hiding this comment.
Inconsistent run-ID length in error message.
The check accepts 25 or 29 chars, but the user-facing error says "25/30". This will mislead users when validation rejects a valid-looking ID.
🐛 Proposed fix
function validateRunId(value: string): string | undefined {
if (!value.startsWith("run_")) return "Run IDs start with 'run_'";
- if (value.length !== 25 && value.length !== 29) return "Run IDs are 25/30 characters long";
+ if (value.length !== 25 && value.length !== 29) return "Run IDs are 25 or 29 characters long";
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/runs/v3/RunFilters.tsx` around lines 1534 - 1537,
The validateRunId function currently checks for lengths 25 or 29 but returns an
error message saying "25/30", causing inconsistency; update the user-facing
error text in validateRunId to match the actual allowed lengths (e.g., return
"Run IDs are 25/29 characters long") so the message matches the check (function
name: validateRunId).
| function validateScheduleId(value: string): string | undefined { | ||
| if (!value.startsWith("sched")) return "Schedule IDs start with 'sched_'"; | ||
| if (value.length !== 27) return "Schedule IDs are 27 characters long"; | ||
| } |
There was a problem hiding this comment.
validateScheduleId accepts IDs without the _ separator.
startsWith("sched") will allow inputs like schedXYZ... that don’t start with sched_, even though the error message and placeholder="sched_" (line 1652) communicate that the underscore is required. Tighten the check to match the placeholder/error string:
🐛 Proposed fix
function validateScheduleId(value: string): string | undefined {
- if (!value.startsWith("sched")) return "Schedule IDs start with 'sched_'";
+ if (!value.startsWith("sched_")) return "Schedule IDs start with 'sched_'";
if (value.length !== 27) return "Schedule IDs are 27 characters long";
}📝 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.
| function validateScheduleId(value: string): string | undefined { | |
| if (!value.startsWith("sched")) return "Schedule IDs start with 'sched_'"; | |
| if (value.length !== 27) return "Schedule IDs are 27 characters long"; | |
| } | |
| function validateScheduleId(value: string): string | undefined { | |
| if (!value.startsWith("sched_")) return "Schedule IDs start with 'sched_'"; | |
| if (value.length !== 27) return "Schedule IDs are 27 characters long"; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/runs/v3/RunFilters.tsx` around lines 1640 - 1643,
The validator validateScheduleId currently uses startsWith("sched") which
permits IDs without the required underscore; update the check to
startsWith("sched_") so it enforces the placeholder/error text, keeping the
existing length check and return messages (i.e., ensure validateScheduleId uses
startsWith("sched_") and still returns "Schedule IDs start with 'sched_'" on
failure).
| const [open, setOpen] = useState<boolean | undefined>(); | ||
| const { value, replace } = useSearchParams(); | ||
| const currentValue = value(paramKey); | ||
|
|
||
| const [inputValue, setInputValue] = useState(currentValue); | ||
|
|
||
| const apply = useCallback(() => { | ||
| clearSearchValue(); | ||
| replace({ | ||
| cursor: undefined, | ||
| direction: undefined, | ||
| [paramKey]: inputValue === "" ? undefined : inputValue?.toString(), | ||
| }); | ||
|
|
||
| setOpen(false); | ||
| }, [inputValue, replace, paramKey]); | ||
|
|
||
| const error = inputValue ? validate?.(inputValue) : undefined; | ||
|
|
||
| return ( | ||
| <SelectProvider virtualFocus={true} open={open} setOpen={setOpen}> | ||
| {trigger} | ||
| <SelectPopover | ||
| hideOnEnter={false} | ||
| hideOnEscape={() => { | ||
| if (onClose) { | ||
| onClose(); | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| }} | ||
| className="max-w-[min(32ch,var(--popover-available-width))]" | ||
| > | ||
| <div className="flex flex-col gap-3 p-3 pt-2"> | ||
| <div className="flex flex-col gap-1"> | ||
| <Paragraph variant="extra-small/bright" className="mb-0.5"> | ||
| {label} | ||
| </Paragraph> | ||
| <Input | ||
| placeholder={placeholder} | ||
| value={inputValue ?? ""} | ||
| onChange={(e) => setInputValue(e.target.value)} | ||
| variant="small" | ||
| className={cn(inputWidth, "font-mono")} | ||
| spellCheck={false} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether SelectPopover unmounts contents on close (Ariakit)
rg -nP --type=ts -C5 '\bSelectPopover\b' apps/webapp/app/components/primitives/Select
rg -nP --type=tsx -C2 '\bIdFilterDropdown\b'Repository: triggerdotdev/trigger.dev
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Find Select component files
git ls-files | grep -i select | head -20Repository: triggerdotdev/trigger.dev
Length of output: 910
🏁 Script executed:
#!/bin/bash
# Find SharedFilters.tsx and examine the context
git ls-files | grep SharedFiltersRepository: triggerdotdev/trigger.dev
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Look for IdFilterDropdown or similar filter dropdown components
git ls-files | grep -i filter | grep -i dropdownRepository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Read Select.tsx to understand SelectPopover behavior
cat -n apps/webapp/app/components/primitives/Select.tsx | head -100Repository: triggerdotdev/trigger.dev
Length of output: 3945
🏁 Script executed:
#!/bin/bash
# Look for SelectPopover definition in Select.tsx
rg -n "SelectPopover" apps/webapp/app/components/primitives/Select.tsxRepository: triggerdotdev/trigger.dev
Length of output: 296
🏁 Script executed:
#!/bin/bash
# Read SharedFilters.tsx lines around 1029-1075 to see the full context
sed -n '1020,1080p' apps/webapp/app/components/runs/v3/SharedFilters.tsx | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 2308
🏁 Script executed:
#!/bin/bash
# Read SelectPopover implementation in Select.tsx
sed -n '631,680p' apps/webapp/app/components/primitives/Select.tsx | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 1992
🏁 Script executed:
#!/bin/bash
# Check if there's any useEffect or syncing logic in the IdFilterDropdown or around it
sed -n '1000,1100p' apps/webapp/app/components/runs/v3/SharedFilters.tsx | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 3509
🏁 Script executed:
#!/bin/bash
# Check Ariakit's SelectPopover props/behavior to understand unmounting
rg -A 10 "SelectPopover" apps/webapp/app/components/primitives/Select.tsx | head -30Repository: triggerdotdev/trigger.dev
Length of output: 949
🏁 Script executed:
#!/bin/bash
# Check where IdFilterDropdown is used and if popover closure behavior matters
rg -n "IdFilterDropdown" apps/webapp/app/components/runs/v3/SharedFilters.tsxRepository: triggerdotdev/trigger.dev
Length of output: 185
🏁 Script executed:
#!/bin/bash
# Check if useSearchParams updates reactively when URL changes
rg -n "useSearchParams" apps/webapp/app/components/runs/v3/SharedFilters.tsx | head -10Repository: triggerdotdev/trigger.dev
Length of output: 271
🏁 Script executed:
#!/bin/bash
# Look for FilterMenuProvider to understand the filtering pattern and lifecycle
sed -n '1,100p' apps/webapp/app/components/runs/v3/SharedFilters.tsx | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 3358
🏁 Script executed:
#!/bin/bash
# Check if there's a "Clear filters" or similar feature that changes URL while component is mounted
rg -n "clear.*filter|Clear.*Filter" apps/webapp/app/components/runs/v3/ -i | head -20Repository: triggerdotdev/trigger.dev
Length of output: 987
🏁 Script executed:
#!/bin/bash
# Check the parent component that uses IdFilterDropdown to understand lifecycle
rg -B 10 "IdFilterDropdown" apps/webapp/app/components/runs/v3/SharedFilters.tsx | head -40Repository: triggerdotdev/trigger.dev
Length of output: 720
🏁 Script executed:
#!/bin/bash
# Verify if useSearchParams provides reactive updates
rg -n "useSearchParam" apps/webapp/app/hooks/ -lRepository: triggerdotdev/trigger.dev
Length of output: 203
🏁 Script executed:
#!/bin/bash
# Read useSearchParam hook implementation
cat -n apps/webapp/app/hooks/useSearchParam.tsRepository: triggerdotdev/trigger.dev
Length of output: 2676
🏁 Script executed:
#!/bin/bash
# Check the ClearFiltersButton implementation to see if it changes URL while component is mounted
sed -n '236,260p' apps/webapp/app/components/runs/v3/ScheduleFilters.tsx | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 988
🏁 Script executed:
#!/bin/bash
# Check if RunFilters or ScheduleFilters use IdFilterDropdown
rg -l "IdFilterDropdown" apps/webapp/app/components/runs/v3/Repository: triggerdotdev/trigger.dev
Length of output: 285
🏁 Script executed:
#!/bin/bash
# Look at RunFilters.tsx to see the clear filters context
sed -n '370,400p' apps/webapp/app/components/runs/v3/RunFilters.tsx | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 1655
🏁 Script executed:
#!/bin/bash
# Find how IdFilterDropdown is used in RunFilters.tsx
rg -n "IdFilterDropdown" apps/webapp/app/components/runs/v3/RunFilters.tsx -A 8Repository: triggerdotdev/trigger.dev
Length of output: 1663
🏁 Script executed:
#!/bin/bash
# Check if RunFilters itself unmounts/remounts when "Clear all filters" is clicked
rg -n "Form" apps/webapp/app/components/runs/v3/RunFilters.tsx | grep -i clear -A 2 -B 2Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Check if IdFilterDropdown has a dependency on currentValue that would cause re-initialization
sed -n '1019,1100p' apps/webapp/app/components/runs/v3/SharedFilters.tsx | cat -nRepository: triggerdotdev/trigger.dev
Length of output: 2958
Sync inputValue with external currentValue changes while the dropdown is mounted.
inputValue initializes from currentValue on first render only. When the user clears filters elsewhere (e.g., "Clear all filters" button, URL changes via del(), browser back), the component remains mounted, currentValue updates reactively, but inputValue stays stale. Reopening the popover will show the old captured value instead of the current URL state.
Although SelectPopover unmounts its contents on close (unmountOnHide=true), the parent IdFilterDropdown component doesn't remount, so the Input component re-renders with the stale inputValue state.
Consider syncing with a useEffect:
useEffect(() => {
setInputValue(currentValue);
}, [currentValue]);Or reset on popover open if the popover remounting alone should handle this.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx` around lines 1029 -
1075, The inputValue state is initialized from currentValue but never synced
when currentValue changes externally, causing stale values in the Input inside
the SelectPopover; update the component to synchronize inputValue with
currentValue by adding an effect (watching currentValue) that calls
setInputValue(currentValue) and/or reset inputValue when the popover opens
(watch open) so Input shows the latest URL/search param state; reference the
inputValue/currentValue state variables, setInputValue setter, open/setOpen from
SelectProvider, and the SelectPopover/IdFilterDropdown wrapping this input to
locate where to add the useEffect.
| {!(filtered.length === 0 && fetcher.state !== "loading") && ( | ||
| <ComboBox | ||
| value={searchValue} | ||
| render={(props) => ( | ||
| <div className="flex items-center justify-stretch"> | ||
| <input {...props} placeholder={"Filter by tags..."} /> | ||
| {fetcher.state === "loading" && <Spinner color="muted" />} | ||
| </div> | ||
| )} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Don't remove the tag search input when there are no matches.
Once filtered.length hits 0, the ComboBox disappears, so the user can't refine or clear the current query without closing and reopening the popover. Keep the input mounted while searchValue is non-empty.
💡 Suggested adjustment
- {!(filtered.length === 0 && fetcher.state !== "loading") && (
+ {(searchValue !== "" || filtered.length > 0 || fetcher.state === "loading") && (
<ComboBox
value={searchValue}
render={(props) => (
<div className="flex items-center justify-stretch">
<input {...props} placeholder={"Filter by tags..."} />📝 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.
| {!(filtered.length === 0 && fetcher.state !== "loading") && ( | |
| <ComboBox | |
| value={searchValue} | |
| render={(props) => ( | |
| <div className="flex items-center justify-stretch"> | |
| <input {...props} placeholder={"Filter by tags..."} /> | |
| {fetcher.state === "loading" && <Spinner color="muted" />} | |
| </div> | |
| )} | |
| /> | |
| )} | |
| {(searchValue !== "" || filtered.length > 0 || fetcher.state === "loading") && ( | |
| <ComboBox | |
| value={searchValue} | |
| render={(props) => ( | |
| <div className="flex items-center justify-stretch"> | |
| <input {...props} placeholder={"Filter by tags..."} /> | |
| {fetcher.state === "loading" && <Spinner color="muted" />} | |
| </div> | |
| )} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/runs/v3/WaitpointTokenFilters.tsx` around lines
303 - 313, The ComboBox is being unmounted when filtered.length === 0 which
hides the input; update the rendering condition around the ComboBox (the JSX
that currently uses !(filtered.length === 0 && fetcher.state !== "loading")) to
also keep the component mounted whenever searchValue is non-empty (e.g., only
hide when filtered is empty, fetcher is not loading, AND searchValue is empty).
Locate the ComboBox block in WaitpointTokenFilters.tsx and change the condition
so the input remains rendered while searchValue has content.
| const hasAppliedFilters = | ||
| tasks.length > 0 || | ||
| queues.length > 0 || | ||
| models.length > 0 || | ||
| prompts.length > 0 || | ||
| operations.length > 0 || | ||
| providers.length > 0; |
There was a problem hiding this comment.
Count scope and time changes as applied filters.
hasAppliedFilters ignores scope, period, from, and to, so the new “Clear all filters” button never appears when a user changes only the dashboard scope or time range. Because this lives in MetricDashboard, the same gap affects both built-in and custom dashboards.
Possible fix
const hasAppliedFilters =
tasks.length > 0 ||
queues.length > 0 ||
models.length > 0 ||
prompts.length > 0 ||
operations.length > 0 ||
- providers.length > 0;
+ providers.length > 0 ||
+ scope !== "environment" ||
+ from !== undefined ||
+ to !== undefined ||
+ (period !== undefined && period !== defaultPeriod);Also applies to: 306-316
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardKey/route.tsx
around lines 248 - 254, hasAppliedFilters currently only checks
tasks/queues/models/prompts/operations/providers and ignores time and scope
changes; update the logic in MetricDashboard where hasAppliedFilters is defined
(the const hasAppliedFilters) to also include checks for scope, period, from,
and to (e.g., scope !== defaultScope OR period !== defaultPeriod OR from/to
timestamps present or different from defaults) so that changing dashboard scope
or time range counts as an applied filter; make the same change to the other
occurrence of hasAppliedFilters in this file to ensure the “Clear all filters”
button appears when only scope/time range changes.
1 participant
Lots of filter UX improvements across lots of routes
General
Route updates