Query improvements #2905
Query improvements #2905
Conversation
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/code/TSQLResultsTable.tsx`:
- Around line 956-1049: The empty-state check currently uses rows.length which
ignores active table filters; change the condition to check the table's current
row model instead (e.g. replace rows.length === 0 with
table.getRowModel().rows.length === 0 or the existing tableRows variable if
present) so the "No results" UI is shown when filters reduce the visible rows to
zero; update the if condition where rows is checked in the component (around the
top of the empty-state JSX block) to use table.getRowModel().rows.length (or
tableRows.length) and keep the rest of the rendering logic unchanged.
- Around line 105-121: The datetime formatting branch in TSQLResultsTable uses
toLocaleDateString but passes time options (hour, minute, second) which are
ignored; update the code in the isDateTimeType handling (where value is parsed
into const date = new Date(value)) to call date.toLocaleString("en-GB", { day:
"numeric", month: "short", year: "numeric", hour: "2-digit", minute: "2-digit",
second: "2-digit" }) instead of toLocaleDateString, preserving the try/catch and
the fallback return String(value).
In `@apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx`:
- Around line 74-91: The currentData computed in the useMemo (referencing
highlight.activePayload and totals) stores raw hover values which can be
strings; coerce hovered values to numbers when building hoverData so legend
rendering matches currentTotal/AnimatedNumber expectations—replace use of
item.value with Number(item.value) || 0 inside the reduce that constructs
hoverData (in the currentData useMemo) so the returned merged object contains
numeric values only.
In `@apps/webapp/app/components/primitives/charts/ChartLine.tsx`:
- Around line 235-306: The LegacyChartLineProps and ChartLine function currently
declare a useGlobalDateRange prop that is never used; remove useGlobalDateRange
from the LegacyChartLineProps type, delete the parameter and its default value
from the ChartLine signature, and remove any mention in the JSDoc comment so the
API is not misleading; also search for and update any callers/tests that pass
useGlobalDateRange (or forward it to ChartRoot if you instead choose to
implement the behavior) so the codebase remains consistent.
In `@apps/webapp/app/components/primitives/charts/ChartLoading.tsx`:
- Around line 116-146: ChartBarLoadingBackground (and the other loading
component that uses Math.random between lines 183–197) is producing
non‑deterministic DOM during render causing SSR/client hydration mismatch; fix
by moving any Math.random() calls out of the render path — generate the bar/line
heights after mount (e.g., compute the array of random heights inside a
useEffect/useLayoutEffect and store in state or useRef seeded on mount) and
render deterministically from that state/ref, ensuring initial server render
uses a stable placeholder and updates on the client.
In `@apps/webapp/app/components/primitives/charts/ChartRoot.tsx`:
- Around line 166-178: useSeriesTotal currently sums every property except
dataKey, which accidentally includes numeric metadata; update it to only sum
known series keys from the chart context. Retrieve dataKeys from useChartContext
and in the useMemo reduce iterate over dataKeys (for each seriesKey do
acc[seriesKey] = (acc[seriesKey]||0) + Number(item[seriesKey]||0)), remove
Object.entries usage, and include dataKeys in the useMemo dependency array so
totals correctly reflect only series fields.
In `@apps/webapp/app/components/primitives/charts/DateRangeContext.tsx`:
- Around line 41-50: The helpers toISODateString and parseISODateString are
using mismatched UTC vs local behavior causing day shifts across timezones;
update toISODateString to build the YYYY-MM-DD string from local components
(date.getFullYear(), date.getMonth()+1, date.getDate()) with zero-padding rather
than date.toISOString(), and update parseISODateString to parse the YYYY-MM-DD
parts and construct a local Date via new Date(year, month-1, day) so both
functions use local dates consistently; modify the implementations in
DateRangeContext.tsx for the functions toISODateString and parseISODateString
accordingly.
In `@apps/webapp/app/components/primitives/Tooltip.tsx`:
- Line 126: The className on InformationCircleIcon contains an invalid Tailwind
utility `flex-0`; update the JSX in Tooltip.tsx where InformationCircleIcon is
rendered (look for the className using buttonClassName) and replace `flex-0`
with a valid utility such as `flex-none` (or `shrink-0` if you only want to
prevent shrinking) so the icon behaves correctly in a flex container.
In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx`:
- Around line 315-333: The useEffect in SharedFilters (using previousSearch and
onApply) and TimeDropdown.applySelection both trigger onApply, causing duplicate
applies; keep the location-driven path and stop the duplicate by removing the
direct prop-through from TimeDropdown or preventing TimeDropdown.applySelection
from calling onApply immediately. Specifically, stop passing the onApply prop
into the TimeDropdown instance (or alter TimeDropdown.applySelection to only
emit a local state change and not call onApply when the URL/location will drive
applies), ensuring SharedFilters.useEffect remains the single source that
invokes onApply based on location.search changes (references: SharedFilters
useEffect, previousSearch, onApply; TimeDropdown and its applySelection method).
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHelpSidebar.tsx:
- Around line 14-18: The AITimeFilter interface is duplicated across server and
client files; extract it into a single exported type alias (change from
"interface AITimeFilter" to "export type AITimeFilter = { period?: string;
from?: string; to?: string }") in a new shared types file colocated with the
query route (so both server and client code can import it), then update all
usages/imports in aiQueryService.server.ts, ai-generate.tsx, route.tsx,
AITabContent.tsx, QueryHelpSidebar.tsx, and AIQueryInput.tsx to import the named
type from that new file (and remove any imports from .server files in client
components). Ensure the type is exported so server and client modules can
reference it.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx:
- Around line 368-371: The current detection assigned to queryHasTriggeredAt
uses the regex /\bWHERE\b[\s\S]*\btriggered_at\b/i which can match occurrences
inside string literals or comments; update the logic to first remove/strip SQL
string literals and comments (single-quoted, double-quoted, backtick, -- line
comments and /* */ block comments) from the raw query, then run the existing
/\bWHERE\b[\s\S]*\btriggered_at\b/i test against the cleaned query; implement
this as a small helper (e.g., stripSqlLiteralsOrComments) and use it where
queryHasTriggeredAt is computed so the UI only disables the time filter for real
column references.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts:
- Around line 22-28: formatBytes computes a negative unit index for 0 < bytes <
1 (e.g. byte_seconds) because Math.log(bytes) < 0; update the unit index
calculation in formatBytes to clamp the computed index to a minimum of 0 and
maximum of sizes.length - 1 (e.g. use Math.max(0, Math.floor(Math.log(bytes) /
Math.log(k))) then Math.min(..., sizes.length - 1)) so sub‑1 values resolve to
the "B" unit and avoid sizes[-1].
- Around line 7-19: The parsing code in utils.ts that computes readRows,
readBytes, elapsedNs, and byteSeconds should guard against non-numeric inputs
and potential precision loss: validate parsed values with Number.isFinite and
fall back to a safe default (e.g., 0 or a placeholder) when parseInt/parseFloat
yields NaN, and when counters may exceed Number.MAX_SAFE_INTEGER, parse as
BigInt and branch to use BigInt-safe formatting (or coerce to string) for
readRows/readBytes/byteSeconds before calling formatBytes; ensure formattedTime
calculation checks for finite elapsedNs and handles BigInt or very large values
safely so the UI never displays "NaN" or loses precision.
In `@apps/webapp/package.json`:
- Line 194: Bump the dependency "recharts" in package.json to "^2.15.2" (replace
the existing "recharts" entry) and then run the project's Prettier formatting
before committing; ensure the package.json line containing the "recharts" key is
updated and formatted to match the repo's Prettier rules.
🧹 Nitpick comments (32)
apps/webapp/app/assets/icons/ArrowTopRightBottomLeftIcon.tsx (1)
4-19: Duplicate overlapping paths detected.Paths 1 and 3 share the same starting point (
M14.8258 10.5L20.125 5.20083) and draw similar geometry for the top-right arrow. Similarly, paths 2 and 4 both start atM2 21.0625Vfor the bottom-left arrow. This results in redundant overlapping SVG content that increases file size without visual benefit.Consider consolidating to just 2 paths (one per arrow) or verifying the intended design from the source.
apps/webapp/app/assets/icons/AbacusIcon.tsx (1)
4-4: OrphanedclipPathreference.The
<g>element referencesclipPath="url(#clip0_16909_120578)", but no<clipPath>definition with that ID exists in the SVG. The browser ignores invalid references, so the icon renders correctly, but this is dead code likely left over from a design tool export.Either add the missing
<clipPath>definition or remove the unused attribute:🧹 Suggested cleanup
- <g clipPath="url(`#clip0_16909_120578`)"> + <g>apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)
274-288: Prefer type aliases over interfaces for TS exports.Convert
TimeFilterApplyValuesandTimeFilterPropstotypeto align with the TS style guideline.♻️ Proposed refactor
-export interface TimeFilterApplyValues { - period?: string; - from?: string; - to?: string; -} +export type TimeFilterApplyValues = { + period?: string; + from?: string; + to?: string; +}; -export interface TimeFilterProps { - defaultPeriod?: string; - /** Label name used in the filter display, defaults to "Created" */ - labelName?: string; - applyShortcut?: ShortcutDefinition | undefined; - /** Callback when the user applies a time filter selection, receives the applied values */ - onApply?: (values: TimeFilterApplyValues) => void; -} +export type TimeFilterProps = { + defaultPeriod?: string; + /** Label name used in the filter display, defaults to "Created" */ + labelName?: string; + applyShortcut?: ShortcutDefinition | undefined; + /** Callback when the user applies a time filter selection, receives the applied values */ + onApply?: (values: TimeFilterApplyValues) => void; +};As per coding guidelines, please prefer types over interfaces in TS.
apps/webapp/app/components/primitives/SegmentedControl.tsx (1)
80-86: Prefer named export over default export.This file still uses a default export, which conflicts with the repo guideline for TS/TSX. Consider switching to a named export and updating imports accordingly.
♻️ Proposed refactor
-export default function SegmentedControl({ +export function SegmentedControl({ name, value, defaultValue, options, variant = "secondary/medium", fullWidth, onChange, }: SegmentedControlProps) {apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/TRQLGuideContent.tsx (1)
71-138: Consider extracting duplicated query strings.The query strings are duplicated between the
codeprop and theonTrycallback. Consider defining them as constants to avoid maintenance issues.Example refactor for basic queries section
+const BASIC_QUERY_EXAMPLE = `SELECT run_id, task_identifier, status +FROM runs +LIMIT 10`; + <TryableCodeBlock - code={`SELECT run_id, task_identifier, status -FROM runs -LIMIT 10`} - onTry={() => - onTryExample( - `SELECT run_id, task_identifier, status -FROM runs -LIMIT 10`, - "environment" - ) - } + code={BASIC_QUERY_EXAMPLE} + onTry={() => onTryExample(BASIC_QUERY_EXAMPLE, "environment")} />apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (2)
467-471: Consider using zod schema forAITimeFiltertype consistency.Since
ActionSchemaalready defines the same fields with zod, you could derive this type from the schema for consistency and to avoid duplication. However, this is optional since the current approach is also valid.Alternative using zod inference
// If you want to derive from ActionSchema: type AITimeFilter = Pick<z.infer<typeof ActionSchema>, 'period' | 'from' | 'to'>;
513-513: Simplify the loading state check.The condition can be simplified since
navigation.formMethodis only set when submitting, so checkingstate === "loading"with a POST method is redundant.Simplified check
- const isLoading = (navigation.state === "submitting" || navigation.state === "loading") && navigation.formMethod === "POST"; + const isLoading = navigation.state === "submitting" && navigation.formMethod === "POST";Note: If you need to show loading during the redirect/revalidation phase after submission, the current approach is correct. Verify the intended behavior.
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsx (1)
127-139: Consider adding runtime validation for tool args.The type assertion on line 130 assumes
part.argsmatches the expected shape. While this is reasonable given the tool name check, consider using Zod validation for safer handling of AI-generated arguments.♻️ Optional: Add runtime validation
+const TimeFilterArgsSchema = z.object({ + period: z.string().optional(), + from: z.string().optional(), + to: z.string().optional(), +}); + // If it's a setTimeFilter call, emit the time_filter event immediately if (part.toolName === "setTimeFilter") { - const args = part.args as { period?: string; from?: string; to?: string }; + const parsed = TimeFilterArgsSchema.safeParse(part.args); + if (parsed.success) { + const args = parsed.data; sendEvent({ type: "time_filter", filter: { period: args.period, from: args.from, to: args.to, }, }); + } }apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/AITabContent.tsx (1)
5-9: DuplicateAITimeFilterdefinition.Same duplication issue as noted in
QueryHelpSidebar.tsx. Import from a shared location instead.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsx (1)
38-77: Remove dead code inhighlightSQL.The
suffixvariable is always an empty string (line 41), making the conditional block on lines 72-74 unreachable. This appears to be leftover from a truncation feature that was removed.🧹 Remove dead code
function highlightSQL(query: string): React.ReactNode[] { // Normalize whitespace for display (let CSS line-clamp handle truncation) const normalized = query.replace(/\s+/g, " ").slice(0, 200); - const suffix = ""; // Create a regex pattern that matches keywords as whole words (case insensitive) const keywordPattern = new RegExp( `\\b(${SQL_KEYWORDS.map((k) => k.replace(/\s+/g, "\\s+")).join("|")})\\b`, "gi" ); const parts: React.ReactNode[] = []; let lastIndex = 0; let match; while ((match = keywordPattern.exec(normalized)) !== null) { // Add text before the match if (match.index > lastIndex) { parts.push(normalized.slice(lastIndex, match.index)); } // Add the highlighted keyword parts.push( <span key={match.index} className="text-[`#c678dd`]"> {match[0]} </span> ); lastIndex = keywordPattern.lastIndex; } // Add remaining text if (lastIndex < normalized.length) { parts.push(normalized.slice(lastIndex)); } - if (suffix) { - parts.push(suffix); - } - return parts; }apps/webapp/app/v3/services/aiQueryService.server.ts (4)
14-18: Usetypeinstead ofinterfaceper coding guidelines.The coding guidelines specify using types over interfaces for TypeScript.
Suggested fix
-export interface AITimeFilter { - period?: string; - from?: string; - to?: string; -} +export type AITimeFilter = { + period?: string; + from?: string; + to?: string; +};
49-53: Usetypeinstead ofinterfaceper coding guidelines.Suggested fix
-interface QueryValidationResult { - valid: boolean; - syntaxError?: string; - issues: ValidationIssue[]; -} +type QueryValidationResult = { + valid: boolean; + syntaxError?: string; + issues: ValidationIssue[]; +};
88-145: Significant code duplication in tool definitions.The
validateTSQLQuery,getTableSchema, andsetTimeFiltertools are defined identically in bothstreamQueryandcallmethods. Consider extracting the tools object into a private method to reduce duplication and maintenance burden.Suggested refactor
private buildTools() { return { validateTSQLQuery: tool({ description: "Validate a TSQL query...", parameters: z.object({ query: z.string().describe("The TSQL query to validate") }), execute: async ({ query }) => this.validateQuery(query), }), getTableSchema: tool({ description: "Get detailed schema information...", parameters: z.object({ tableName: z.string().optional().describe("...") }), execute: async ({ tableName }) => this.getSchemaInfo(tableName), }), setTimeFilter: tool({ description: "Set the time filter for the query page UI...", parameters: z.object({ /* ... */ }), execute: async ({ period, from, to }) => { this.pendingTimeFilter = { period, from, to }; return { success: true, message: /* ... */ }; }, }), }; }Also applies to: 187-244
112-145: Consider validating that at least one time filter parameter is provided.All parameters (
period,from,to) are optional. If the AI callssetTimeFilterwith no arguments,pendingTimeFilterwill be set to{ period: undefined, from: undefined, to: undefined }, which is effectively empty. Consider adding validation or using zod's.refine()to ensure at least one parameter is provided.apps/webapp/app/components/code/AIQueryInput.tsx (1)
23-27: Duplicated type definition.
AITimeFilteris defined here and also inapps/webapp/app/v3/services/aiQueryService.server.ts. Consider importing from a shared location to avoid drift between the definitions.Additionally, per coding guidelines, use
typeinstead ofinterface.Suggested fix
-interface AITimeFilter { - period?: string; - from?: string; - to?: string; -} +type AITimeFilter = { + period?: string; + from?: string; + to?: string; +};internal-packages/tsql/src/index.ts (1)
440-457: Usetypeinstead ofinterfaceper coding guidelines.Suggested fix
-export interface SimpleComparisonFallback { - /** The comparison operator */ - op: "eq" | "neq" | "gt" | "gte" | "lt" | "lte"; - /** The value to compare against */ - value: Date | string | number; -} +export type SimpleComparisonFallback = { + /** The comparison operator */ + op: "eq" | "neq" | "gt" | "gte" | "lt" | "lte"; + /** The value to compare against */ + value: Date | string | number; +}; -export interface BetweenFallback { - /** The between operator */ - op: "between"; - /** The low bound of the range */ - low: Date | string | number; - /** The high bound of the range */ - high: Date | string | number; -} +export type BetweenFallback = { + /** The between operator */ + op: "between"; + /** The low bound of the range */ + low: Date | string | number; + /** The high bound of the range */ + high: Date | string | number; +};apps/webapp/app/components/code/ChartConfigPanel.tsx (1)
15-41: Prefer type aliases over interfaces in TS files.
Per repo guidelines, please convertChartConfigurationandChartConfigPanelPropstotypealiases.As per coding guidelines, please use types over interfaces.♻️ Proposed refactor
-export interface ChartConfiguration { +export type ChartConfiguration = { chartType: ChartType; xAxisColumn: string | null; yAxisColumns: string[]; groupByColumn: string | null; stacked: boolean; sortByColumn: string | null; sortDirection: SortDirection; aggregation: AggregationType; -} +}; -interface ChartConfigPanelProps { +type ChartConfigPanelProps = { columns: OutputColumnMetadata[]; config: ChartConfiguration; onChange: (config: ChartConfiguration) => void; className?: string; -} +};apps/webapp/app/components/primitives/AnimatedNumber.tsx (1)
8-13: MissingmotionValuein useEffect dependency array.The
motionValueis used inside the effect but not listed in the dependency array. WhileuseMotionValuereturns a stable reference, ESLint's exhaustive-deps rule would flag this. Consider adding it for completeness.Suggested fix
useEffect(() => { animate(motionValue, value, { duration, ease: "easeInOut", }); - }, [value, duration]); + }, [motionValue, value, duration]);apps/webapp/app/components/primitives/charts/Card.tsx (1)
5-16: Consider using function declaration instead of arrow function.Per coding guidelines, function declarations are preferred over arrow function exports. This applies to the main
Cardcomponent.Suggested refactor
-export const Card = ({ children, className }: { children: ReactNode; className?: string }) => { - return ( +export function Card({ children, className }: { children: ReactNode; className?: string }) { + return ( <div className={cn( "flex flex-col rounded-lg border border-grid-bright bg-background-bright pb-2 pt-4", className )} > {children} </div> ); -}; +}apps/webapp/app/components/primitives/charts/hooks/useHighlightState.ts (1)
3-12: Consider stronger typing foractivePayload.The
activePayload: any[] | nullcould benefit from a more specific type. If the payload structure is known (e.g., from Recharts), consider defining a type for it to improve type safety and IDE support.// Example: If payload items have a known structure type PayloadItem = { dataKey: string; value: number; payload: Record<string, unknown>; // ... other Recharts payload fields }; export type HighlightState = { activeBarKey: string | null; activeDataPointIndex: number | null; activePayload: PayloadItem[] | null; tooltipActive: boolean; };apps/webapp/app/components/primitives/charts/ChartBar.tsx (1)
221-228: Variable shadowing in event handlers.The
indexparameter inonClick(line 221) andonMouseEnter(line 222) shadows the outer loop variable. While the code works correctly (using the parameterindexwhich represents the data point index), this could be confusing.Suggested rename for clarity
- onClick={(data, index, e) => handleBarClick(data, e)} - onMouseEnter={(entry, index) => { + onClick={(data, _dataIndex, e) => handleBarClick(data, e)} + onMouseEnter={(entry, dataIndex) => { if (entry.tooltipPayload?.[0]) { const { dataKey: hoveredKey } = entry.tooltipPayload[0]; - highlight.setHoveredBar(hoveredKey, index); + highlight.setHoveredBar(hoveredKey, dataIndex); } }}apps/webapp/app/components/code/TSQLResultsTable.tsx (2)
216-220: Prefer a type alias forColumnMetaThe codebase guideline is to use
typeoverinterfacein TS/TSX.♻️ Suggested change
-interface ColumnMeta { - outputColumn: OutputColumnMetadata; - alignment: "left" | "right"; -} +type ColumnMeta = { + outputColumn: OutputColumnMetadata; + alignment: "left" | "right"; +};As per coding guidelines, please align this with the project’s TS style.
722-748: Use a button element for the copy affordanceThe clickable icon is a
spanwith an onClick handler, which isn’t keyboard-accessible. Abuttonwith an aria-label improves accessibility.♿ Suggested change
- <span + <button + type="button" + aria-label="Copy cell value" onClick={(e) => { e.stopPropagation(); e.preventDefault(); copy(); }} - className="absolute right-1 top-1/2 z-10 flex -translate-y-1/2 cursor-pointer" + className="absolute right-1 top-1/2 z-10 flex -translate-y-1/2 cursor-pointer" > <SimpleTooltip button={ <span className={cn( "flex size-6 items-center justify-center rounded border border-charcoal-650 bg-charcoal-750", copied ? "text-green-500" : "text-text-dimmed hover:border-charcoal-600 hover:bg-charcoal-700 hover:text-text-bright" )} > {copied ? ( <ClipboardCheckIcon className="size-3.5" /> ) : ( <ClipboardIcon className="size-3.5" /> )} </span> } content={copied ? "Copied!" : "Copy"} disableHoverableContent /> - </span> + </button>apps/webapp/app/components/primitives/charts/Chart.tsx (2)
244-248: Consider strengthening the type forExtendedLegendPayload.The
payload?: { remainingCount?: number }type is loosely defined. Since this extends from Recharts' formatter parameter, you might want to be more explicit about what additional properties are expected beyondremainingCount.
303-320: Avoid usinganyfor event handler parameters.The
onMouseEnterandonMouseLeaveprops are typed as(e: any) => void. Consider using a more specific type based on the legend item payload structure.- onMouseEnter?: (e: any) => void; - onMouseLeave?: (e: any) => void; + onMouseEnter?: (item: ExtendedLegendPayload) => void; + onMouseLeave?: (item: ExtendedLegendPayload) => void;apps/webapp/app/components/code/QueryResultsChart.tsx (1)
63-79: Consider extracting time constants to avoid duplication.The time constants (
SECOND,MINUTE,HOUR,DAY,WEEK,MONTH,YEAR) are duplicated acrossdetectTimeGranularity,detectDataInterval,fillTimeGaps, andgenerateTimeTicks. Consider extracting them to module-level constants.// At module level const SECOND = 1000; const MINUTE = 60 * SECOND; const HOUR = 60 * MINUTE; const DAY = 24 * HOUR; const WEEK = 7 * DAY; const MONTH = 30 * DAY; const YEAR = 365 * DAY;apps/webapp/app/routes/storybook.charts/route.tsx (2)
32-47:filterDataByDateRangeuses exact string matching which may be fragile.The function uses
findIndexwith exact string equality (item[dataKey] === startDate). This works only if the date strings in the data exactly match the format fromDateRangeContext. If formats differ (e.g., ISO with/without time components), filtering will fail silently and return all data.Consider using date comparison instead:
function filterDataByDateRange<T extends Record<string, any>>( data: T[], dataKey: string, startDate: string | undefined, endDate: string | undefined ): T[] { if (!startDate || !endDate) return data; const start = new Date(startDate).getTime(); const end = new Date(endDate).getTime(); return data.filter((item) => { const itemDate = new Date(item[dataKey]).getTime(); return itemDate >= start && itemDate <= end; }); }
55-61: Remove or implement the commented-out zoom sync logic.The
handleZoomChangefunction has commented-out code for syncing withDateRangeContext. Either implement it or remove the comment to avoid confusion. Currently it only logs to console.const handleZoomChange = (range: ZoomRange) => { console.log("Zoom changed:", range); - // Update the shared date range context so all charts sync - // dateRange?.setDateRange(range.start, range.end); - // In a real app, you would fetch new data here based on the range: - // fetchChartData(range.start, range.end).then(setData); + // TODO: Implement server-side data fetching when zooming + dateRange?.setDateRange(range.start, range.end); };apps/webapp/app/components/primitives/charts/ChartZoom.tsx (3)
38-45: Unused props and variables inChartZoomcomponent.The
ChartZoomcomponent acceptssyncWithDateRangeandminDataPointsprops but doesn't use them. Additionally,globalDateRangeis fetched but never used. These are only used inuseZoomHandlers.Either:
- Remove unused props from
ChartZoomPropsif they're only foruseZoomHandlers- Or document that these props are for composing with
useZoomHandlers-export function ChartZoom({ syncWithDateRange = false, minDataPoints = 3 }: ChartZoomProps) { - const { zoom, data, dataKey, onZoomChange } = useChartContext(); - const globalDateRange = useDateRange(); +export function ChartZoom(_props: ChartZoomProps) { + const { zoom } = useChartContext();
58-61: Avoid usinganyfor event parameter.The
onClickhandler usesanytype. Consider using a more specific type or at leastReact.MouseEvent.- onClick={(e: any) => { - e?.stopPropagation?.(); + onClick={(e: React.MouseEvent) => { + e.stopPropagation();
86-103: Use more specific types instead ofanyfor tooltip props.The
ZoomTooltipPropsusesany[]forpayload. Consider using a proper type based on what Recharts provides or creating a specific type for your use case.export type ZoomTooltipProps = { active?: boolean; - payload?: any[]; + payload?: Array<{ value?: number; name?: string; payload?: Record<string, unknown> }>; label?: string;apps/webapp/app/components/primitives/charts/ChartContext.tsx (1)
13-36: Consider a generic type for chart data.The
data: any[]type is very loose. Consider using a generic or a more specific base type to improve type safety:export type ChartContextValue<T extends Record<string, unknown> = Record<string, unknown>> = { // Core data config: ChartConfig; data: T[]; dataKey: string; // ... };However, this might require propagating the generic through multiple components, so
any[]may be acceptable for flexibility.
.../_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHelpSidebar.tsx
Outdated
Show resolved
Hide resolved
| // Check if the query contains triggered_at in a WHERE clause | ||
| // This disables the time filter UI since the user is filtering in their query | ||
| const queryHasTriggeredAt = /\bWHERE\b[\s\S]*\btriggered_at\b/i.test(query); | ||
|
|
There was a problem hiding this comment.
Regex for triggered_at detection has limitations.
The regex /\bWHERE\b[\s\S]*\btriggered_at\b/i will match triggered_at appearing anywhere after WHERE, including in string literals or comments. While this is only used for UI hints (disabling the time filter), it could cause minor UX confusion.
For example, a query like SELECT * FROM runs WHERE status = 'triggered_at test' would incorrectly disable the time filter UI.
🤖 Prompt for AI Agents
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
around lines 368 - 371, The current detection assigned to queryHasTriggeredAt
uses the regex /\bWHERE\b[\s\S]*\btriggered_at\b/i which can match occurrences
inside string literals or comments; update the logic to first remove/strip SQL
string literals and comments (single-quoted, double-quoted, backtick, -- line
comments and /* */ block comments) from the raw query, then run the existing
/\bWHERE\b[\s\S]*\btriggered_at\b/i test against the cleaned query; implement
this as a small helper (e.g., stripSqlLiteralsOrComments) and use it where
queryHasTriggeredAt is computed so the UI only disables the time filter for real
column references.
| const readRows = parseInt(stats.read_rows, 10); | ||
| const readBytes = parseInt(stats.read_bytes, 10); | ||
| const elapsedNs = parseInt(stats.elapsed_ns, 10); | ||
| const byteSeconds = parseFloat(stats.byte_seconds); | ||
|
|
||
| const elapsedMs = elapsedNs / 1_000_000; | ||
| const formattedTime = | ||
| elapsedMs < 1000 ? `${elapsedMs.toFixed(1)}ms` : `${(elapsedMs / 1000).toFixed(2)}s`; | ||
| const formattedBytes = formatBytes(readBytes); | ||
|
|
||
| return `${readRows.toLocaleString()} rows read · ${formattedBytes} · ${formattedTime} · ${formatBytes( | ||
| byteSeconds | ||
| )}s`; |
There was a problem hiding this comment.
Guard against NaN/precision loss in stats parsing.
If any stat is non‑numeric, the UI will show NaN values; large counters can also lose precision. Consider finite checks (and BigInt if values can exceed Number.MAX_SAFE_INTEGER) with a safe fallback.
🛠️ Suggested hardening
- const readRows = parseInt(stats.read_rows, 10);
- const readBytes = parseInt(stats.read_bytes, 10);
- const elapsedNs = parseInt(stats.elapsed_ns, 10);
- const byteSeconds = parseFloat(stats.byte_seconds);
+ const readRows = Number(stats.read_rows);
+ const readBytes = Number(stats.read_bytes);
+ const elapsedNs = Number(stats.elapsed_ns);
+ const byteSeconds = Number(stats.byte_seconds);
+
+ if (![readRows, readBytes, elapsedNs, byteSeconds].every(Number.isFinite)) {
+ return "—";
+ }🤖 Prompt for AI Agents
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts
around lines 7 - 19, The parsing code in utils.ts that computes readRows,
readBytes, elapsedNs, and byteSeconds should guard against non-numeric inputs
and potential precision loss: validate parsed values with Number.isFinite and
fall back to a safe default (e.g., 0 or a placeholder) when parseInt/parseFloat
yields NaN, and when counters may exceed Number.MAX_SAFE_INTEGER, parse as
BigInt and branch to use BigInt-safe formatting (or coerce to string) for
readRows/readBytes/byteSeconds before calling formatBytes; ensure formattedTime
calculation checks for finite elapsedNs and handles BigInt or very large values
safely so the UI never displays "NaN" or loses precision.
...p/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/webapp/app/routes/storybook.charts/route.tsx`:
- Around line 694-711: Remove the unused constant barChartConfig (and its
trailing type assertion "satisfies ChartConfig") from the file; locate the
declaration of barChartConfig and delete the entire object literal and the
"satisfies ChartConfig" suffix, and if ChartConfig is no longer referenced
anywhere in this module, also remove its import/usage to avoid unused symbol
warnings.
🧹 Nitpick comments (5)
apps/webapp/app/components/primitives/charts/ChartBar.tsx (2)
40-55: Consider using a more specific type forpayloadin tooltip formatter.The
tooltipLabelFormatterusesany[]for the payload parameter, which loses type safety.♻️ Suggested improvement
+import type { Payload } from "recharts/types/component/DefaultTooltipContent"; + export type ChartBarRendererProps = { // ... - tooltipLabelFormatter?: (label: string, payload: any[]) => string; + tooltipLabelFormatter?: (label: string, payload: Payload<number, string>[]) => string; // ... };
194-218: Parameter shadowing in onClick handler.The
indexparameter in theonClickcallback (line 211) shadows the outerindexfrom themapcallback (line 194). While functionally correct, this can be confusing. Consider renaming one of them for clarity.♻️ Suggested fix
- {dataKeys.map((key, index, array) => { + {dataKeys.map((key, keyIndex, array) => { return ( <Bar key={key} dataKey={key} stackId={stackId} fill={config[key]?.color} radius={ [ - index === array.length - 1 ? 2 : 0, - index === array.length - 1 ? 2 : 0, - index === 0 ? 2 : 0, - index === 0 ? 2 : 0, + keyIndex === array.length - 1 ? 2 : 0, + keyIndex === array.length - 1 ? 2 : 0, + keyIndex === 0 ? 2 : 0, + keyIndex === 0 ? 2 : 0, ] as [number, number, number, number] } activeBar={false} fillOpacity={1} - onClick={(data, index, e) => handleBarClick(data, e)} + onClick={(data, _cellIndex, e) => handleBarClick(data, e)}apps/webapp/app/components/primitives/charts/ChartLine.tsx (3)
25-37: Consider exportingCurveTypefor external use.The
CurveTypeunion is a useful type that consumers might want to use for type-safe curve specification. Currently it's not exported.♻️ Suggested improvement
-type CurveType = +export type CurveType = | "basis" | "basisClosed" // ...
126-130: Consider memoizinghandleMouseLeavefor performance.Unlike
ChartBarRendererwhich usesuseCallbackfor itshandleMouseLeave, this implementation creates a new function on every render. For consistency and to prevent unnecessary re-renders, consider memoizing it.♻️ Suggested improvement
+import React, { useCallback } from "react"; // ... - const handleMouseLeave = () => { + const handleMouseLeave = useCallback(() => { highlight.setTooltipActive(false); highlight.reset(); - }; + }, [highlight]);
132-179: Stacked area chart uses hardcodedstackId.The
stackId="stack"is hardcoded on line 173. If multiple stacked area charts need different stacking groups, this could cause issues. Consider making it configurable via props, similar to howChartBarRendereraccepts astackIdprop.♻️ Suggested improvement
export type ChartLineRendererProps = { // ... stacked?: boolean; + /** Stack ID for stacked area charts (default: "stack") */ + stackId?: string; // ... }; export function ChartLineRenderer({ // ... stacked = false, + stackId = "stack", // ... }: ChartLineRendererProps) { // ... <Area key={key} type={lineType} dataKey={key} stroke={config[key]?.color} fill={config[key]?.color} fillOpacity={0.6} strokeWidth={2} - stackId="stack" + stackId={stackId} isAnimationActive={false} />
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/storybook.charts/route.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/routes/storybook.charts/route.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/storybook.charts/route.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/routes/storybook.charts/route.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/storybook.charts/route.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/routes/storybook.charts/route.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/routes/storybook.charts/route.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/primitives/charts/ChartBar.tsx (6)
apps/webapp/app/components/primitives/charts/ChartContext.tsx (1)
useChartContext(40-46)apps/webapp/app/components/primitives/charts/ChartRoot.tsx (1)
useHasNoData(147-160)apps/webapp/app/components/primitives/charts/ChartZoom.tsx (2)
useZoomHandlers(173-244)ZoomTooltip(105-167)apps/webapp/app/components/primitives/charts/ChartLoading.tsx (3)
ChartBarLoading(8-18)ChartBarNoData(20-39)ChartBarInvalid(41-60)apps/webapp/app/components/primitives/charts/hooks/useHighlightState.ts (1)
getBarOpacity(95-115)apps/webapp/app/components/primitives/charts/hooks/useZoomSelection.ts (1)
ZoomRange(3-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
apps/webapp/app/components/primitives/charts/ChartBar.tsx (2)
1-27: LGTM - Imports and type definitions are well-structured.The imports are properly organized, separating external dependencies (recharts, React) from internal modules. The type import for
ZoomRangecorrectly usesimport typesyntax.
165-181: Tooltip content logic may be inverted.The tooltip renders
ChartTooltipContentwhentooltipLabelFormatteris provided, but usesZoomTooltipwhen it's not. However,ChartTooltipContentis typically the default content component, whileZoomTooltipis specialized for zoom interactions. This logic seems inverted - you'd expect the custom formatter to work with the zoom tooltip, not the standard one.Verify this is the intended behavior. If
tooltipLabelFormattercustomizes label formatting, it should likely apply to theZoomTooltipas well, or the condition should be checking for zoom enablement instead.apps/webapp/app/components/primitives/charts/ChartLine.tsx (2)
1-24: LGTM - Clean imports with proper separation.Imports are well-organized. The comment on line 22 clarifies the legend rendering architecture. Note that
ChartRootis imported but onlyuseHasNoDataappears to be used from it.
181-224: LGTM - LineChart implementation is clean and consistent.The
LineChartrendering follows the same patterns as theAreaChartbranch. Good use ofaccessibilityLayerprop for a11y, disabled animations for performance, and consistent axis/tooltip configuration.apps/webapp/app/routes/storybook.charts/route.tsx (5)
1-18: LGTM - Imports are well-organized.Good separation of UI primitives, chart components, and context providers. The imports follow the project conventions.
28-43: Edge case:filterDataByDateRangemay return incorrect results with duplicate keys.If the data array contains duplicate values for
dataKey,findIndexreturns only the first match, which could lead to unexpected slicing behavior. This is likely fine for date-keyed data where each date is unique, but worth noting.
51-57: Zoom handler is a no-op (intentional for demo).The
handleZoomChangefunction only logs and doesn't update the date range context. The commented-outdateRange?.setDateRangecall and the explanatory comments make it clear this is intentional scaffolding for the demo. Just confirming this is expected behavior for a storybook route.
713-786: LGTM - Rich mock data with JSX labels.The
barChartBigDatasetConfigdemonstrates the chart system's support for JSX labels (React nodes), which is a nice capability showcase. The data structure is consistent and comprehensive.
280-284: The current implementation is correct; no type mismatch exists.The
DateRangeProvideracceptsDateobjects as props (not strings), and internally converts them to ISO strings viatoISODateString()before storing in context. The code at lines 280-284 is correctly typed and requires no changes.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const barChartConfig = { | ||
| completed: { | ||
| label: "Completed", | ||
| color: "#28BF5C", | ||
| }, | ||
| "in-progress": { | ||
| label: "In Progress", | ||
| color: "#3B82F6", | ||
| }, | ||
| canceled: { | ||
| label: "Canceled", | ||
| color: "#878C99", | ||
| }, | ||
| failed: { | ||
| label: "Failed", | ||
| color: "#E11D48", | ||
| }, | ||
| } satisfies ChartConfig; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if barChartConfig is used anywhere in the file
rg -n "barChartConfig[^B]" apps/webapp/app/routes/storybook.charts/route.tsxRepository: triggerdotdev/trigger.dev
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Search for all occurrences of barChartConfig in the file
rg -n "barChartConfig" apps/webapp/app/routes/storybook.charts/route.tsxRepository: triggerdotdev/trigger.dev
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Search for all references to barChartConfig across the entire webapp
rg "barChartConfig" apps/webapp/Repository: triggerdotdev/trigger.dev
Length of output: 143
Remove the unused barChartConfig constant.
The barChartConfig constant defined at lines 694-711 is not used anywhere in the file or codebase. Remove it unless there's a planned use case for it.
🤖 Prompt for AI Agents
In `@apps/webapp/app/routes/storybook.charts/route.tsx` around lines 694 - 711,
Remove the unused constant barChartConfig (and its trailing type assertion
"satisfies ChartConfig") from the file; locate the declaration of barChartConfig
and delete the entire object literal and the "satisfies ChartConfig" suffix, and
if ChartConfig is no longer referenced anywhere in this module, also remove its
import/usage to avoid unused symbol warnings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/webapp/app/v3/services/aiQueryService.server.ts`:
- Around line 107-140: The setTimeFilter tool is duplicated in streamQuery() and
call(); extract the shared tool definition into a single private helper (e.g., a
method or getter like buildSetTimeFilterTool or this.setTimeFilterTool) and have
both streamQuery() and call() reference that helper so behavior stays consistent
and DRY; ensure the helper returns the tool object with description, parameters,
and execute which updates this.pendingTimeFilter and returns the same
success/message shape.
♻️ Duplicate comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts (1)
7-19: Guard against non‑numeric stats to avoid NaN in the UI.This is the same issue as previously flagged:
parseInt/parseFloatcan yieldNaN, which then propagates intotoLocaleString()/formatBytes(). Please add finite checks (and consider large‑number safety) before formatting.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
369-371: Regex fortriggered_atdetection has limitations.The regex
/\bWHERE\b[\s\S]*\btriggered_at\b/iwill matchtriggered_atanywhere afterWHERE, including in string literals or comments. This could cause minor UX confusion (e.g.,WHERE status = 'triggered_at test'would incorrectly disable the time filter UI).Consider this a known limitation for now, or strip SQL string literals/comments before testing.
🧹 Nitpick comments (2)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsx (1)
128-140: Unsafe type assertion onpart.args.The cast
part.args as { period?: string; from?: string; to?: string }bypasses type safety. While the tool's Zod schema should ensure valid args, the SDK typespart.argsasunknown. Consider using Zod parsing for runtime validation to maintain type safety.♻️ Suggested fix using Zod validation
+ // If it's a setTimeFilter call, emit the time_filter event immediately if (part.toolName === "setTimeFilter") { - const args = part.args as { period?: string; from?: string; to?: string }; + const argsSchema = z.object({ + period: z.string().optional(), + from: z.string().optional(), + to: z.string().optional(), + }); + const args = argsSchema.parse(part.args); sendEvent({ type: "time_filter", filter: {apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
403-409: Empty string handling may cause unexpected behavior.The hidden inputs pass empty strings (
value={period ?? ""}) when values are null. However, on the server side (lines 256-258), the code usesperiod ?? undefined, which only convertsnulltoundefined, not empty strings. An empty string passed toparse("")could returnnullor cause unexpected behavior.Consider converting empty strings to undefined explicitly:
♻️ Suggested fix
const timeFilter = timeFilters({ - period: period ?? undefined, - from: from ?? undefined, - to: to ?? undefined, + period: period || undefined, + from: from || undefined, + to: to || undefined, defaultPeriod: DEFAULT_PERIOD, });This uses
||instead of??to treat empty strings as falsy and convert them toundefined.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/AITabContent.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHelpSidebar.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsxapps/webapp/app/v3/services/aiQueryService.server.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHelpSidebar.tsx
- apps/webapp/app/components/code/AIQueryInput.tsx
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/AITabContent.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
package.jsonapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/v3/services/aiQueryService.server.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/v3/services/aiQueryService.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/v3/services/aiQueryService.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/v3/services/aiQueryService.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/v3/services/aiQueryService.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/v3/services/aiQueryService.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/v3/services/aiQueryService.server.ts
apps/webapp/app/v3/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Organize services in the webapp following the pattern
app/v3/services/*/*.server.ts
Files:
apps/webapp/app/v3/services/aiQueryService.server.ts
🧠 Learnings (11)
📚 Learning: 2026-01-15T10:48:02.673Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T10:48:02.673Z
Learning: Use pnpm as the package manager (version 10.23.0 or later) and Node.js 20.20.0
Applied to files:
package.json
📚 Learning: 2025-11-26T14:40:07.146Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2710
File: packages/schema-to-json/package.json:0-0
Timestamp: 2025-11-26T14:40:07.146Z
Learning: Node.js 24+ has native TypeScript support and can execute .ts files directly without tsx or ts-node for scripts that use only erasable TypeScript syntax (type annotations, interfaces, etc.). The trigger.dev repository uses Node.js 24.11.1+ and scripts like updateVersion.ts can be run with `node` instead of `tsx`.
Applied to files:
package.json
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsx
📚 Learning: 2026-01-15T11:50:06.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.044Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Access environment variables via `env` export from `apps/webapp/app/env.server.ts`, never use `process.env` directly
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsx
📚 Learning: 2026-01-15T11:50:06.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.044Z
Learning: Applies to apps/webapp/**/*.test.{ts,tsx} : For testable code in the webapp, never import env.server.ts in test files - pass configuration as options instead
Applied to files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsx
📚 Learning: 2026-01-08T15:57:09.323Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/otel-metrics.mdc:0-0
Timestamp: 2026-01-08T15:57:09.323Z
Learning: Applies to **/*.ts : Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts
📚 Learning: 2026-01-08T15:57:09.323Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/otel-metrics.mdc:0-0
Timestamp: 2026-01-08T15:57:09.323Z
Learning: Applies to **/*.ts : When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2025-06-10T09:31:01.040Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2158
File: internal-packages/clickhouse/src/client/queryBuilder.ts:77-92
Timestamp: 2025-06-10T09:31:01.040Z
Learning: ClickHouse SQL syntax requires GROUP BY clause to come before ORDER BY clause, following standard SQL clause ordering: SELECT, FROM, WHERE, GROUP BY, HAVING, ORDER BY, LIMIT.
Applied to files:
apps/webapp/app/v3/services/aiQueryService.server.ts
📚 Learning: 2026-01-15T11:50:06.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.044Z
Learning: Applies to internal-packages/clickhouse/schema/**/*.sql : Follow ClickHouse naming conventions: `raw_` prefix for input tables, `_v1`, `_v2` suffixes for versioning, `_mv_v1` suffix for materialized views
Applied to files:
apps/webapp/app/v3/services/aiQueryService.server.ts
🧬 Code graph analysis (3)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsx (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.ts (1)
AITimeFilter(5-9)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.ts (1)
apps/webapp/app/v3/services/aiQueryService.server.ts (1)
AITimeFilter(13-13)
apps/webapp/app/v3/services/aiQueryService.server.ts (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.ts (1)
AITimeFilter(5-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts (1)
22-31: LGTM.Clamping the unit index and handling negatives keeps formatting safe and predictable.
package.json (1)
87-87: Format package.json with Prettier before committing.The package.json file has code style issues that violate the formatting guideline. Run
npx prettier --write package.jsonto fix.⛔ Skipped due to learnings
Learnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-15T10:48:02.673Z Learning: Applies to **/*.{js,ts,jsx,tsx,json,md,yaml,yml} : Format code using Prettier before committingLearnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-15T10:48:02.673Z Learning: Use pnpm as the package manager (version 10.23.0 or later) and Node.js 20.20.0Learnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-15T11:50:06.044Z Learning: Applies to {packages,integrations}/**/* : Add a changeset when modifying any public package in `packages/*` or `integrations/*` using `pnpm run changeset:add`Learnt from: ericallam Repo: triggerdotdev/trigger.dev PR: 2710 File: packages/schema-to-json/package.json:0-0 Timestamp: 2025-11-26T14:40:07.146Z Learning: Node.js 24+ has native TypeScript support and can execute .ts files directly without tsx or ts-node for scripts that use only erasable TypeScript syntax (type annotations, interfaces, etc.). The trigger.dev repository uses Node.js 24.11.1+ and scripts like updateVersion.ts can be run with `node` instead of `tsx`.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.ts (1)
1-9: LGTM!The type definition is clean, well-documented, and follows the coding guideline to use
typeoverinterface. The optional fields appropriately model the flexible time filter configuration.apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsx (1)
151-163: LGTM!The integration of
timeFilterinto the final result correctly retrieves the pending time filter from the service and includes it when a query is successfully extracted.apps/webapp/app/v3/services/aiQueryService.server.ts (2)
54-68: LGTM!The
pendingTimeFilterstate management is well-designed. Resetting the state at the start of eachstreamQuery/callensures clean state for each request. This works correctly since a newAIQueryServiceinstance is created per request in the route handler.
153-158: LGTM!The
getPendingTimeFilter()method provides clean access to the time filter set during query generation, enabling the caller to include it in the response.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (3)
254-276: LGTM!The time filter fallback logic correctly handles all cases: absolute ranges (both
fromandto), partial ranges (only one bound), and relative periods. The double null-coalescing on line 274 properly handles both undefined periods and invalid parse results.
441-452: LGTM!The conditional rendering provides good UX - showing "Set in query" when the user has manually added
triggered_atto their query prevents confusion about which filter takes precedence. TheTimeFiltercomponent'sonApplycallback correctly triggers form submission.
493-506: LGTM!The
handleTimeFilterChangecallback correctly propagates AI-generated time filters to the URL search params and appropriately clears pagination state (cursor,direction) when the time filter changes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| setTimeFilter: tool({ | ||
| description: | ||
| "Set the time filter for the query page UI instead of adding triggered_at conditions to the query. ALWAYS use this tool when the user wants to filter by time (e.g., 'last 7 days', 'past hour', 'yesterday'). The UI will apply this filter automatically. Do NOT add triggered_at to the WHERE clause - use this tool instead.", | ||
| parameters: z.object({ | ||
| period: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| "Relative time period like '1m', '5m', '30m', '1h', '6h', '12h', '1d', '3d', '7d', '14d', '30d', '90d'. Use this for 'last X days/hours/minutes' requests." | ||
| ), | ||
| from: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| "ISO 8601 timestamp for the start of an absolute date range. Use with 'to' for specific date ranges." | ||
| ), | ||
| to: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| "ISO 8601 timestamp for the end of an absolute date range. Use with 'from' for specific date ranges." | ||
| ), | ||
| }), | ||
| execute: async ({ period, from, to }) => { | ||
| // Store the time filter so we can include it in the result | ||
| this.pendingTimeFilter = { period, from, to }; | ||
| return { | ||
| success: true, | ||
| message: period | ||
| ? `Time filter set to: last ${period}` | ||
| : `Time filter set to: ${from ?? "start"} - ${to ?? "now"}`, | ||
| }; | ||
| }, | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate tool definition violates DRY principle.
The setTimeFilter tool is defined identically in both streamQuery() (lines 107-140) and call() (lines 206-239). Extract this to a shared private method or property to avoid duplication and ensure consistent behavior.
♻️ Suggested refactor to extract shared tool definition
export class AIQueryService {
private pendingTimeFilter: AITimeFilter | undefined;
constructor(
private readonly tableSchema: TableSchema[],
private readonly model: LanguageModelV1 = openai("gpt-4o-mini")
) {}
+ /**
+ * Build the shared tools for query generation
+ */
+ private buildTools() {
+ return {
+ validateTSQLQuery: tool({
+ description:
+ "Validate a TSQL query for syntax errors and schema compliance. Always use this tool to verify your query before returning it to the user.",
+ parameters: z.object({
+ query: z.string().describe("The TSQL query to validate"),
+ }),
+ execute: async ({ query }) => {
+ return this.validateQuery(query);
+ },
+ }),
+ getTableSchema: tool({
+ description:
+ "Get detailed schema information about available tables and columns. Use this to understand what data is available and how to query it.",
+ parameters: z.object({
+ tableName: z
+ .string()
+ .optional()
+ .describe("Optional: specific table name to get details for"),
+ }),
+ execute: async ({ tableName }) => {
+ return this.getSchemaInfo(tableName);
+ },
+ }),
+ setTimeFilter: tool({
+ description:
+ "Set the time filter for the query page UI instead of adding triggered_at conditions to the query. ALWAYS use this tool when the user wants to filter by time (e.g., 'last 7 days', 'past hour', 'yesterday'). The UI will apply this filter automatically. Do NOT add triggered_at to the WHERE clause - use this tool instead.",
+ parameters: z.object({
+ period: z
+ .string()
+ .optional()
+ .describe(
+ "Relative time period like '1m', '5m', '30m', '1h', '6h', '12h', '1d', '3d', '7d', '14d', '30d', '90d'. Use this for 'last X days/hours/minutes' requests."
+ ),
+ from: z
+ .string()
+ .optional()
+ .describe(
+ "ISO 8601 timestamp for the start of an absolute date range. Use with 'to' for specific date ranges."
+ ),
+ to: z
+ .string()
+ .optional()
+ .describe(
+ "ISO 8601 timestamp for the end of an absolute date range. Use with 'from' for specific date ranges."
+ ),
+ }),
+ execute: async ({ period, from, to }) => {
+ this.pendingTimeFilter = { period, from, to };
+ return {
+ success: true,
+ message: period
+ ? `Time filter set to: last ${period}`
+ : `Time filter set to: ${from ?? "start"} - ${to ?? "now"}`,
+ };
+ },
+ }),
+ };
+ }
streamQuery(prompt: string, options: AIQueryOptions = {}) {
// ... existing code ...
return streamText({
model: this.model,
system: systemPrompt,
prompt: userPrompt,
- tools: {
- validateTSQLQuery: tool({ ... }),
- getTableSchema: tool({ ... }),
- setTimeFilter: tool({ ... }),
- },
+ tools: this.buildTools(),
maxSteps: 5,
// ...
});
}
async call(prompt: string, options: AIQueryOptions = {}): Promise<AIQueryResult> {
// ... existing code ...
const result = await streamText({
model: this.model,
system: systemPrompt,
prompt: userPrompt,
- tools: {
- validateTSQLQuery: tool({ ... }),
- getTableSchema: tool({ ... }),
- setTimeFilter: tool({ ... }),
- },
+ tools: this.buildTools(),
maxSteps: 5,
// ...
});
// ...
}🤖 Prompt for AI Agents
In `@apps/webapp/app/v3/services/aiQueryService.server.ts` around lines 107 - 140,
The setTimeFilter tool is duplicated in streamQuery() and call(); extract the
shared tool definition into a single private helper (e.g., a method or getter
like buildSetTimeFilterTool or this.setTimeFilterTool) and have both
streamQuery() and call() reference that helper so behavior stays consistent and
DRY; ensure the helper returns the tool object with description, parameters, and
execute which updates this.pendingTimeFilter and returns the same
success/message shape.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/code/TSQLResultsTable.tsx`:
- Around line 63-69: Remove the orphaned doc comment block "Fuzzy filter
function using match-sorter ranking" that is sitting above the CellValue
formatting comment; it is a leftover and should be deleted so docs match code,
leaving the actual fuzzyFilter implementation (the function named fuzzyFilter
further down) as the single source of truth. Locate the stray comment near the
top of the file and delete it, ensuring the real fuzzyFilter function remains
unchanged.
♻️ Duplicate comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
370-372: Regex can disable the time filter whentriggered_atappears in strings/comments.The
/\bWHERE\b[\s\S]*\btriggered_at\b/itest will matchtriggered_atinside string literals or comments and hide the filter UI. Consider stripping SQL strings/comments before the regex.
🧹 Nitpick comments (3)
apps/webapp/app/components/code/TSQLResultsTable.tsx (2)
206-212: Consider stabilizingonChangereference to prevent debounce timer resets.Including
onChangein the dependency array means the debounce timer resets whenever the parent re-renders with a new callback reference. SinceFilterCellpasses an inline arrow function(value) => column.setFilterValue(value), this could reset the timer on unrelated re-renders.A common fix is to use a ref to capture the latest callback without triggering the effect:
Suggested fix
const DebouncedInput = forwardRef< HTMLInputElement, { value: string; onChange: (value: string) => void; debounce?: number; } & Omit<React.InputHTMLAttributes<HTMLInputElement>, "onChange"> >(function DebouncedInput({ value: initialValue, onChange, debounce = 300, ...props }, ref) { const [value, setValue] = useState(initialValue); + const onChangeRef = useRef(onChange); + onChangeRef.current = onChange; useEffect(() => { setValue(initialValue); }, [initialValue]); useEffect(() => { const timeout = setTimeout(() => { - onChange(value); + onChangeRef.current(value); }, debounce); return () => clearTimeout(timeout); - }, [value, debounce, onChange]); + }, [value, debounce]); return <input ref={ref} {...props} value={value} onChange={(e) => setValue(e.target.value)} />; });
218-221: Usetypeinstead ofinterfaceper coding guidelines.The coding guidelines specify to use types over interfaces for TypeScript.
Suggested fix
-interface ColumnMeta { - outputColumn: OutputColumnMetadata; - alignment: "left" | "right"; -} +type ColumnMeta = { + outputColumn: OutputColumnMetadata; + alignment: "left" | "right"; +};apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
255-261: Normalize empty-string time filter inputs beforetimeFilters.Hidden inputs submit
"", andperiod ?? undefinedwon’t convert that, sotimeFiltersmay see empty strings as explicit values. Consider trimming and normalizing toundefinedfirst.Proposed normalization
- const timeFilter = timeFilters({ - period: period ?? undefined, - from: from ?? undefined, - to: to ?? undefined, - defaultPeriod: DEFAULT_PERIOD, - }); + const normalizedPeriod = period?.trim() ? period : undefined; + const normalizedFrom = from?.trim() ? from : undefined; + const normalizedTo = to?.trim() ? to : undefined; + const timeFilter = timeFilters({ + period: normalizedPeriod, + from: normalizedFrom, + to: normalizedTo, + defaultPeriod: DEFAULT_PERIOD, + });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/code/TSQLResultsTable.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/components/code/TSQLResultsTable.tsx (3)
106-124: DateTime formatting correctly usestoLocaleString.The previous review flagged that
toLocaleDateStringignores time options per ECMAScript spec. This has been correctly updated to usetoLocaleString, which includes time components as intended.
1089-1095: Verify: hiding filter row clears all column filters.When toggling off the filter row visibility,
setColumnFilters([])clears all active filters. This means users lose their filter state when they hide the filter row. If this is intentional (to reset filters on toggle), this is fine. If users expect to hide the UI but keep filters active, consider removing thesetColumnFilters([])call.
886-957: Solid virtualized table implementation.The table setup with TanStack Table and TanStack Virtual is well-structured:
- Memoized column definitions with dynamic width calculation
- Proper row virtualization with reasonable overscan
- Filtering and sorting integration
- Column resizing support
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
291-293: ConfirmwhereClauseFallbackis ignored when the query already filterstriggered_at.
whereClauseFallbackis always passed. IfexecuteQuerydoesn’t guard against existingtriggered_atpredicates, users will get an extra constraint. Please verify that behavior (or add a server-side check before passing the fallback).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /** | ||
| * Fuzzy filter function using match-sorter ranking | ||
| */ | ||
| /** | ||
| * Get the formatted display string for a value based on its column type | ||
| * This mirrors the formatting logic in CellValue component | ||
| */ |
There was a problem hiding this comment.
Remove orphaned doc comment.
Lines 63-65 contain a doc comment for "Fuzzy filter function using match-sorter ranking" but no function follows it. The actual fuzzyFilter function is defined later at line 149. This appears to be a leftover from refactoring.
Suggested fix
-/**
- * Fuzzy filter function using match-sorter ranking
- */
/**
* Get the formatted display string for a value based on its column type📝 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.
| /** | |
| * Fuzzy filter function using match-sorter ranking | |
| */ | |
| /** | |
| * Get the formatted display string for a value based on its column type | |
| * This mirrors the formatting logic in CellValue component | |
| */ | |
| /** | |
| * Get the formatted display string for a value based on its column type | |
| * This mirrors the formatting logic in CellValue component | |
| */ |
🤖 Prompt for AI Agents
In `@apps/webapp/app/components/code/TSQLResultsTable.tsx` around lines 63 - 69,
Remove the orphaned doc comment block "Fuzzy filter function using match-sorter
ranking" that is sitting above the CellValue formatting comment; it is a
leftover and should be deleted so docs match code, leaving the actual
fuzzyFilter implementation (the function named fuzzyFilter further down) as the
single source of truth. Locate the stray comment near the top of the file and
delete it, ensuring the real fuzzyFilter function remains unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)
19-33: Remove unused imports.
useRef(line 19),SpanPresenter(line 32), anduseLocation(line 33) are imported but not used in this file. These should be removed to avoid confusion.🧹 Suggested fix
-import { startTransition, useCallback, useEffect, useRef, useState, type ReactNode } from "react"; +import { startTransition, useCallback, useEffect, useState, type ReactNode } from "react";import { ShortcutDefinition } from "~/hooks/useShortcutKeys"; -import { SpanPresenter } from "~/presenters/v3/SpanPresenter.server"; -import { useLocation } from "@remix-run/react";
🤖 Fix all issues with AI agents
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx:
- Around line 507-520: handleTimeFilterChange currently updates URL search
params via replaceSearchParams (AITimeFilter) but QueryEditorForm never reads
those params or auto-submits, so AI updates don't apply; fix by wiring
QueryEditorForm to react to search param changes: either have QueryEditorForm
read the period/from/to params from the URL (useSearchParams) and trigger its
submit handler when they change, or change the parent to pass the AITimeFilter
values as props to QueryEditorForm and call its onSubmit programmatically when
handleTimeFilterChange runs; update references: handleTimeFilterChange,
replaceSearchParams, QueryEditorForm, and any submit handler (e.g., onSubmit or
submitQuery) to ensure form re-submission occurs after the params update.
- Around line 366-368: The time filter state variables period, from and to are
never initialized from or synchronized with URL search params, so changes made
by handleTimeFilterChange update the URL but not the component state (and
refresh loses the values); fix by reading search params (e.g., via
useSearchParams or equivalent) on mount and whenever params change and
initializing/setting period, setPeriod, from, setFrom, to, setTo accordingly,
and ensure handleTimeFilterChange also updates these state setters (or
alternatively lift the time filter state to the parent and pass it down so the
URL and form stay in sync).
♻️ Duplicate comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
370-372: Regex fortriggered_atdetection has known limitations.The regex can match
triggered_atappearing in string literals or comments, which could incorrectly disable the time filter UI. This was noted in a previous review.apps/webapp/app/components/code/TSQLResultsTable.tsx (1)
63-66: Remove the stray doc comment abovegetFormattedValue.
It references the fuzzy filter but sits before the formatter docs, which is confusing.Suggested change
-/** - * Fuzzy filter function using match-sorter ranking - */
🧹 Nitpick comments (7)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (4)
274-279: Usetypeinstead ofinterface.Per coding guidelines, prefer type aliases over interfaces in TypeScript.
♻️ Suggested fix
-/** Values passed to onApply callback when a time filter is applied */ -export interface TimeFilterApplyValues { - period?: string; - from?: string; - to?: string; -} +/** Values passed to onApply callback when a time filter is applied */ +export type TimeFilterApplyValues = { + period?: string; + from?: string; + to?: string; +};
281-291: Usetypeinstead ofinterface.Same as above—per coding guidelines, prefer type aliases.
♻️ Suggested fix
-export interface TimeFilterProps { +export type TimeFilterProps = { defaultPeriod?: string; period?: string; from?: string; to?: string; /** Label name used in the filter display, defaults to "Created" */ labelName?: string; applyShortcut?: ShortcutDefinition | undefined; /** Callback when the user applies a time filter selection, receives the applied values */ onValueChange?: (values: TimeFilterApplyValues) => void; -} +};
416-496: Consider clarifying callback semantics.The relationship between
onApplyandonValueChangemay be confusing to consumers:
onValueChangeis checked first to determine controlled vs URL modeonApplyfires unconditionally after either pathIf both callbacks are provided, both will be invoked. Consider documenting this behavior or consolidating to a single callback to reduce ambiguity.
827-831: Simplify with nullish coalescing.Minor readability improvement using
??instead of a ternary.♻️ Suggested fix
- shortcut={applyShortcut ? applyShortcut : { + shortcut={applyShortcut ?? { modifiers: ["mod"], key: "Enter", enabledOnInputElements: true, }}apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
757-774: Consider adding error handling for clipboard operations.The
navigator.clipboard.writeTextcalls can fail if clipboard permissions are denied. While not critical, adding a try-catch with user feedback would improve UX.♻️ Suggested improvement
const handleCopyCSV = () => { const csv = rowsToCSV(rows, columns); - navigator.clipboard.writeText(csv); - setIsOpen(false); + navigator.clipboard.writeText(csv) + .then(() => setIsOpen(false)) + .catch((err) => console.error("Failed to copy:", err)); };apps/webapp/app/components/code/TSQLResultsTable.tsx (2)
217-221: Prefer a type alias forColumnMeta.
This aligns with the project’s TS style guidance.As per coding guidelines, prefer `type` over `interface` in TS.Suggested change
-interface ColumnMeta { - outputColumn: OutputColumnMetadata; - alignment: "left" | "right"; -} +type ColumnMeta = { + outputColumn: OutputColumnMetadata; + alignment: "left" | "right"; +};
699-758: Use a button for the copy affordance (keyboard access).
The clickable<span>isn’t focusable; a<button>improves accessibility without changing layout.Suggested change
- <span + <button + type="button" + aria-label="Copy cell value" onClick={(e) => { e.stopPropagation(); e.preventDefault(); copy(); }} - className="absolute right-1 top-1/2 z-10 flex -translate-y-1/2 cursor-pointer" + className="absolute right-1 top-1/2 z-10 flex -translate-y-1/2 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-indigo-500/50" > <SimpleTooltip button={ <span className={cn( "flex size-6 items-center justify-center rounded border border-charcoal-650 bg-charcoal-750", copied ? "text-green-500" : "text-text-dimmed hover:border-charcoal-600 hover:bg-charcoal-700 hover:text-text-bright" )} > {copied ? ( <ClipboardCheckIcon className="size-3.5" /> ) : ( <ClipboardIcon className="size-3.5" /> )} </span> } content={copied ? "Copied!" : "Copy"} disableHoverableContent /> - </span> + </button>
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/components/code/TSQLResultsTable.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/code/TSQLResultsTable.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
🧬 Code graph analysis (2)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (2)
apps/webapp/app/hooks/useShortcutKeys.tsx (1)
ShortcutDefinition(14-19)apps/webapp/app/components/runs/v3/RunFilters.tsx (1)
filterIcon(227-263)
apps/webapp/app/components/code/TSQLResultsTable.tsx (7)
internal-packages/tsql/src/query/schema.ts (2)
column(318-330)OutputColumnMetadata(242-257)internal-packages/clickhouse/src/index.ts (1)
OutputColumnMetadata(61-61)packages/core/src/v3/index.ts (1)
formatDurationMilliseconds(36-36)apps/webapp/app/utils/numberFormatter.ts (2)
formatCurrencyAccurate(48-50)formatNumber(15-22)apps/webapp/app/components/environments/EnvironmentLabel.tsx (1)
EnvironmentSlug(141-143)apps/webapp/app/hooks/useCopy.ts (1)
useCopy(3-22)apps/webapp/app/utils/cn.ts (1)
cn(77-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (19)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (2)
141-206: LGTM!The
labelNameparameterization is cleanly threaded through the function, allowing customizable labels while maintaining backward compatibility with the default value.
293-340: LGTM!The component cleanly supports both controlled mode (via props) and URL-driven mode (via
useSearchParams). OnlyonValueChangeis passed down, avoiding the double-calling issue mentioned in past reviews.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (7)
1-70: LGTM!Imports are well-organized and appropriately scoped.
72-104: LGTM!The access control logic properly handles admin/impersonation bypass and feature flag validation.
112-155: LGTM!The loader properly validates access, handles missing resources with 404s, and returns the expected data shape.
157-166: LGTM!The schema correctly uses zod for validation with nullable/optional time filter fields.
255-277: Time filter fallback logic is well-structured.The handling of different time filter combinations (between, gte, lte, period-based) is comprehensive. The
parse-durationlibrary returnsnullfor invalid inputs, so the?? 7 * 24 * 60 * 60 * 1000fallback correctly handles malformed period strings.
560-714: LGTM!The results display logic properly handles all states (loading, error, explain, data) with appropriate conditional rendering and user-friendly error recovery.
804-823: LGTM!The ScopeItem component cleanly handles all scope types with appropriate labels.
apps/webapp/app/components/code/TSQLResultsTable.tsx (10)
1-61: Import set + sizing constants look solid.
Clear TanStack integration and well-named sizing constants make the later layout logic easy to follow.
70-187: Formatting + fuzzy filter logic is well-aligned.
Combining raw and formatted values is a nice touch for search relevance.
189-215: Debounced input implementation looks good.
State sync + timeout cleanup are handled cleanly.
223-332: Column width heuristics are clear and bounded.
Sampling + min/max constraints should keep sizing stable.
390-403: Right-alignment helper is straightforward.
Nice centralized rule for numeric-like columns.
666-675: EnvironmentSlug swap looks correct.
Keeps display consistent with the updated label component.
677-697: JSON cell truncation/tooltip handling looks good.
Clear and consistent with other truncation patterns.
764-843: Header cell UX (tooltip/sort/filter) looks solid.
Good separation of concerns and readable interactions.
848-884: Filter row wiring is clean.
Focus handling and debounced updates are well-implemented.
886-1183: Virtualized table integration looks solid.
Row model + virtualizer hookup is clean and should scale well.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const [period, setPeriod] = useState<string | undefined>(); | ||
| const [from, setFrom] = useState<string | undefined>(); | ||
| const [to, setTo] = useState<string | undefined>(); |
There was a problem hiding this comment.
Time filter state is not synchronized with URL search params.
The period, from, and to state variables are initialized as undefined, but handleTimeFilterChange in the parent component updates URL search params. This means:
- If the AI sidebar changes the time filter, the URL updates but the form state doesn't reflect it.
- On page refresh, URL params are lost since they're never read back into the component.
Consider initializing from search params and keeping them in sync:
♻️ Suggested approach
+ import { useSearchParams as useRemixSearchParams } from "@remix-run/react";
// Inside QueryEditorForm, or pass as props:
+ const [searchParams] = useRemixSearchParams();
- const [period, setPeriod] = useState<string | undefined>();
- const [from, setFrom] = useState<string | undefined>();
- const [to, setTo] = useState<string | undefined>();
+ const [period, setPeriod] = useState<string | undefined>(searchParams.get("period") ?? undefined);
+ const [from, setFrom] = useState<string | undefined>(searchParams.get("from") ?? undefined);
+ const [to, setTo] = useState<string | undefined>(searchParams.get("to") ?? undefined);Alternatively, lift the time filter state to the parent and pass it down as props.
🤖 Prompt for AI Agents
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
around lines 366 - 368, The time filter state variables period, from and to are
never initialized from or synchronized with URL search params, so changes made
by handleTimeFilterChange update the URL but not the component state (and
refresh loses the values); fix by reading search params (e.g., via
useSearchParams or equivalent) on mount and whenever params change and
initializing/setting period, setPeriod, from, setFrom, to, setTo accordingly,
and ensure handleTimeFilterChange also updates these state setters (or
alternatively lift the time filter state to the parent and pass it down so the
URL and form stay in sync).
.../app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/components/primitives/DateTime.tsx (1)
104-123: Update callers offormatDateTimethat are missing theincludeDateparameter.The
includeDateparameter is now required (no default value). At least two locations need updating:
apps/webapp/app/v3/services/bulk/BulkActionV2.server.tslines 339-340: Calls with only 5 arguments (missingincludeDate)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam._index/route.tsxline 574: Call with only 5 arguments (missingincludeDate)These calls will fail at runtime without providing the required 6th parameter.
🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx`:
- Around line 248-257: The same-day check in the "range" case of
SharedFilters.tsx currently uses from.getDate() === to.getDate(), which falsely
treats different months/years as the same day; replace this with a proper
full-date comparison by importing and using isSameDay from date-fns to compute
the isSameDay boolean, then keep the DateTime rendering logic (DateTime
date={from!} ... and DateTime date={to!} includeDate={!isSameDay} ...) so the
`to` label only hides the date when the two timestamps are truly the same
calendar day.
♻️ Duplicate comments (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts (1)
1-19: Guard against NaN/precision loss in stats parsing.Non‑numeric stat values will propagate
NaNinto the UI (e.g.,"NaN rows read"). Consider finite checks with a safe fallback.🛠️ Suggested hardening
- const readRows = parseInt(stats.read_rows, 10); - const readBytes = parseInt(stats.read_bytes, 10); - const elapsedNs = parseInt(stats.elapsed_ns, 10); - const byteSeconds = parseFloat(stats.byte_seconds); + const readRows = Number(stats.read_rows); + const readBytes = Number(stats.read_bytes); + const elapsedNs = Number(stats.elapsed_ns); + const byteSeconds = Number(stats.byte_seconds); + + if (![readRows, readBytes, elapsedNs, byteSeconds].every(Number.isFinite)) { + return "—"; + }apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (2)
387-390: Regex-basedtriggered_atdetection still matches literals/comments.
542-552: Ensure AI time‑filter updates propagate to form state and execution.After
replaceSearchParams, confirm the editor picks upperiod/from/toand re-submits (or otherwise applies the new bounds), or the AI-suggested time filter won’t take effect until manual submit.
🧹 Nitpick comments (5)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts (1)
22-31: Consider extending units beyond GB for large datasets.For TB+ values, the formatter caps at GB and produces large GB numbers, which is harder to read. Extending units improves UX.
♻️ Example adjustment
- const sizes = ["B", "KB", "MB", "GB"]; + const sizes = ["B", "KB", "MB", "GB", "TB", "PB"];apps/webapp/app/components/code/AIQueryInput.tsx (1)
33-43: Prefer atypealias instead of aninterfacefor props.Repository guidelines ask for
typein TS/TSX.
As per coding guidelines, please consider the change below.♻️ Proposed change
-interface AIQueryInputProps { +type AIQueryInputProps = { onQueryGenerated: (query: string) => void; /** Called when the AI sets a time filter - updates URL search params */ onTimeFilterChange?: (filter: AITimeFilter) => void; /** Set this to a prompt to auto-populate and immediately submit */ autoSubmitPrompt?: string; /** Change this to force re-submission even if prompt is the same */ autoSubmitKey?: number; /** Get the current query in the editor (used for edit mode) */ getCurrentQuery?: () => string; -} +};apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)
279-296: Prefertypealiases overinterfacein TSX here.Project guidelines call for
typealiases in.tsxfiles.As per coding guidelines, prefer `type` aliases here.♻️ Proposed refactor
-export interface TimeFilterApplyValues { +export type TimeFilterApplyValues = { period?: string; from?: string; to?: string; -} +}; -export interface TimeFilterProps { +export type TimeFilterProps = { defaultPeriod?: string; period?: string; from?: string; to?: string; /** Label name used in the filter display, defaults to "Created" */ labelName?: string; applyShortcut?: ShortcutDefinition | undefined; /** Callback when the user applies a time filter selection, receives the applied values */ onValueChange?: (values: TimeFilterApplyValues) => void; -} +};apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
358-363: Use a type alias instead of an interface forQueryEditorFormHandle.♻️ Suggested change
-interface QueryEditorFormHandle { - setQuery: (query: string) => void; - setScope: (scope: QueryScope) => void; - getQuery: () => string; - setTimeFilter: (filter: { period?: string; from?: string; to?: string }) => void; -} +type QueryEditorFormHandle = { + setQuery: (query: string) => void; + setScope: (scope: QueryScope) => void; + getQuery: () => string; + setTimeFilter: (filter: { period?: string; from?: string; to?: string }) => void; +};As per coding guidelines, prefer type aliases over interfaces.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsx (1)
8-46: Prefer matching multi‑word SQL keywords before single‑word ones.This avoids highlighting only the
JOINportion ofLEFT JOIN,RIGHT JOIN, etc.♻️ Suggested tweak
- const keywordPattern = new RegExp( - `\\b(${SQL_KEYWORDS.map((k) => k.replace(/\s+/g, "\\s+")).join("|")})\\b`, - "gi" - ); + const keywordPattern = new RegExp( + `\\b(${[...SQL_KEYWORDS] + .sort((a, b) => b.length - a.length) + .map((k) => k.replace(/\s+/g, "\\s+")) + .join("|")})\\b`, + "gi" + );
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/presenters/v3/QueryPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.tsapps/webapp/app/services/queryService.server.tsinternal-packages/database/prisma/migrations/20260120115732_add_time_filter_to_customer_query/migration.sqlinternal-packages/database/prisma/schema.prisma
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/services/queryService.server.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/presenters/v3/QueryPresenter.server.tsapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/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.query/utils.tsapps/webapp/app/components/runs/v3/SharedFilters.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/presenters/v3/QueryPresenter.server.tsapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/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.query/utils.tsapps/webapp/app/components/runs/v3/SharedFilters.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/presenters/v3/QueryPresenter.server.tsapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/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.query/utils.tsapps/webapp/app/components/runs/v3/SharedFilters.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/presenters/v3/QueryPresenter.server.tsapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/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.query/utils.tsapps/webapp/app/components/runs/v3/SharedFilters.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/presenters/v3/QueryPresenter.server.tsapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/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.query/utils.tsapps/webapp/app/components/runs/v3/SharedFilters.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/presenters/v3/QueryPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/presenters/v3/QueryPresenter.server.tsapps/webapp/app/components/primitives/DateTime.tsxapps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/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.query/utils.tsapps/webapp/app/components/runs/v3/SharedFilters.tsx
internal-packages/database/prisma/migrations/**/*.sql
📄 CodeRabbit inference engine (CLAUDE.md)
internal-packages/database/prisma/migrations/**/*.sql: When editing the Prisma schema, remove extraneous migration lines related to specific tables:_BackgroundWorkerToBackgroundWorkerFile,_BackgroundWorkerToTaskQueue,_TaskRunToTaskRunTag,_WaitpointRunConnections,_completedWaitpoints,SecretStore_key_idx, and unrelatedTaskRunindexes
Database indexes must use CONCURRENTLY to avoid table locks and must be in their own separate migration file
Files:
internal-packages/database/prisma/migrations/20260120115732_add_time_filter_to_customer_query/migration.sql
🧠 Learnings (6)
📚 Learning: 2026-01-15T11:50:06.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.044Z
Learning: Applies to internal-packages/database/prisma/migrations/**/*.sql : When editing the Prisma schema, remove extraneous migration lines related to specific tables: `_BackgroundWorkerToBackgroundWorkerFile`, `_BackgroundWorkerToTaskQueue`, `_TaskRunToTaskRunTag`, `_WaitpointRunConnections`, `_completedWaitpoints`, `SecretStore_key_idx`, and unrelated `TaskRun` indexes
Applied to files:
internal-packages/database/prisma/schema.prismainternal-packages/database/prisma/migrations/20260120115732_add_time_filter_to_customer_query/migration.sql
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2026-01-08T15:57:09.323Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/otel-metrics.mdc:0-0
Timestamp: 2026-01-08T15:57:09.323Z
Learning: Applies to **/*.ts : Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts
📚 Learning: 2026-01-08T15:57:09.323Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/otel-metrics.mdc:0-0
Timestamp: 2026-01-08T15:57:09.323Z
Learning: Applies to **/*.ts : When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts
📚 Learning: 2026-01-15T11:50:06.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.044Z
Learning: Applies to internal-packages/database/prisma/migrations/**/*.sql : Database indexes must use CONCURRENTLY to avoid table locks and must be in their own separate migration file
Applied to files:
internal-packages/database/prisma/migrations/20260120115732_add_time_filter_to_customer_query/migration.sql
🧬 Code graph analysis (2)
apps/webapp/app/components/code/AIQueryInput.tsx (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.ts (1)
AITimeFilter(5-9)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsx (4)
apps/webapp/app/presenters/v3/QueryPresenter.server.ts (1)
QueryHistoryItem(5-15)apps/webapp/app/components/primitives/Buttons.tsx (1)
Button(296-329)apps/webapp/app/assets/icons/ClockRotateLeftIcon.tsx (1)
ClockRotateLeftIcon(1-15)apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)
timeFilterRenderValues(208-277)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (23)
internal-packages/database/prisma/schema.prisma (1)
2472-2478: LGTM!The new time-filter fields (
filterPeriod,filterFrom,filterTo) are correctly defined as optional (String?,DateTime?) since time filters are not always applied. The field placement and documentation comments are appropriate.internal-packages/database/prisma/migrations/20260120115732_add_time_filter_to_customer_query/migration.sql (1)
1-5: LGTM!The migration is clean and focused, adding only the three new nullable columns. Since this only adds nullable columns without default values, it's a non-blocking DDL operation safe for production tables. No extraneous migration lines present. Based on learnings, the CONCURRENTLY guideline only applies to index creation, which is not applicable here.
apps/webapp/app/presenters/v3/QueryPresenter.server.ts (2)
5-15: LGTM!The
QueryHistoryItemtype correctly extends with the new time-filter fields. Usingtypeinstead ofinterfacefollows the coding guidelines. The nullable types (string | null,Date | null) correctly correspond to the optional Prisma fields (String?,DateTime?).
28-30: LGTM!The select projection and mapping correctly include the new time-filter fields. Direct pass-through is appropriate since Prisma handles the
DateTimeto JavaScriptDateconversion automatically.Also applies to: 46-48
apps/webapp/app/components/code/AIQueryInput.tsx (2)
21-29: Time-filter wiring looks solid.Immediate application on
time_filterplus the result fallback makes the flow resilient if an event is missed.Also applies to: 45-48, 182-207, 219-219
389-392: Status copy updates read well.apps/webapp/app/components/primitives/DateTime.tsx (4)
1-40: LGTM!The import cleanup and SSR-compatible timezone handling with
useSyncExternalStoreare well-implemented. The caching strategy for the local timezone is efficient.
55-102: LGTM - Good SSR hydration fix.The
suppressHydrationWarningwrapper is the correct approach for date/time content that may render differently between server (UTC) and client (local timezone). The newincludeDateprop is backward-compatible with its default value oftrue.
168-336: LGTM - Consistent hydration fix across components.The
suppressHydrationWarningpattern is consistently applied toSmartDateTime,DateTimeAccurateInner, andDateTimeShortcomponents, ensuring SSR compatibility across the entire date/time rendering surface.
399-436: LGTM!The tooltip content correctly passes
includeDate: trueexplicitly to allformatDateTimecalls, ensuring tooltips always display the complete date information regardless of the parent component'sincludeDatesetting.apps/webapp/app/components/runs/v3/SharedFilters.tsx (4)
141-205: Label-aware time filter rendering looks good.Nice propagation of
labelNameinto the computed filter display.
298-342: TimeFilter controlled props wiring looks solid.Nice integration of
labelNameandonValueChangewith existing URL-based defaults.
421-501: Controlled vs URL mode branching reads clean.The apply path is easy to follow, and the state resets are consistent.
867-870: Good guard on numeric timestamps.Restricting numeric parsing to fully numeric strings prevents accidental timestamp parsing of ISO values.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (8)
1-71: No concerns with the updated imports.
72-78: Helper keeps Date/string normalization straightforward.
263-301: Confirm fallback doesn’t double‑filter when SQL already includestriggered_at.The UI disables the time filter when the query already filters
triggered_at, but the action always passeswhereClauseFallback. Please verifyexecuteQueryignores this fallback in that case, or add a server-side guard to skip it to avoid unexpected extra filtering.
475-491: Good: TimeFilter changes sync state before fetcher submit.
617-707: Results header + table card layout look solid.
721-743: Chart/config split layout is clean and contained.
821-823: Popover trigger swap looks fine.
845-855: Environment label rendering reads well in the scope list.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsx (1)
79-138: History popover UX and selection flow look good.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| case "range": | ||
| valueLabel = ( | ||
| <span> | ||
| <DateTime date={from!} includeTime includeSeconds /> –{" "} | ||
| <DateTime date={to!} includeTime includeSeconds /> | ||
| </span> | ||
| ); | ||
| { | ||
| //If the day is the same, only show the time for the `to` date | ||
| const isSameDay = from && to && from.getDate() === to.getDate(); | ||
|
|
||
| valueLabel = ( | ||
| <span> | ||
| <DateTime date={from!} includeTime includeSeconds /> –{" "} | ||
| <DateTime date={to!} includeTime includeSeconds includeDate={!isSameDay} /> | ||
| </span> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/webapp/app/components/runs/v3/SharedFilters.tsx | sed -n '240,270p'Repository: triggerdotdev/trigger.dev
Length of output: 1114
🏁 Script executed:
cat -n apps/webapp/app/components/runs/v3/SharedFilters.tsx | head -50 | grep -A 50 "import"Repository: triggerdotdev/trigger.dev
Length of output: 2066
Fix same‑day detection to avoid hiding dates across months/years.
Using getDate() alone treats Jan 5 and Feb 5 as the "same day," which will omit the date in the to label incorrectly. Use isSameDay from date-fns for full date comparison.
🐛 Proposed fix
- const isSameDay = from && to && from.getDate() === to.getDate();
+ const sameDay = from && to && isSameDay(from, to);- <DateTime date={to!} includeTime includeSeconds includeDate={!isSameDay} />
+ <DateTime date={to!} includeTime includeSeconds includeDate={!sameDay} /> import {
+ isSameDay,
startOfDay,
endOfDay,
startOfWeek,
endOfWeek,
startOfMonth,
endOfMonth,
startOfYear,
subDays,
subWeeks,
subMonths,
previousSaturday,
isSaturday,
isSunday,
} from "date-fns";📝 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.
| case "range": | |
| valueLabel = ( | |
| <span> | |
| <DateTime date={from!} includeTime includeSeconds /> –{" "} | |
| <DateTime date={to!} includeTime includeSeconds /> | |
| </span> | |
| ); | |
| { | |
| //If the day is the same, only show the time for the `to` date | |
| const isSameDay = from && to && from.getDate() === to.getDate(); | |
| valueLabel = ( | |
| <span> | |
| <DateTime date={from!} includeTime includeSeconds /> –{" "} | |
| <DateTime date={to!} includeTime includeSeconds includeDate={!isSameDay} /> | |
| </span> | |
| case "range": | |
| { | |
| //If the day is the same, only show the time for the `to` date | |
| const sameDay = from && to && isSameDay(from, to); | |
| valueLabel = ( | |
| <span> | |
| <DateTime date={from!} includeTime includeSeconds /> –{" "} | |
| <DateTime date={to!} includeTime includeSeconds includeDate={!sameDay} /> | |
| </span> |
🤖 Prompt for AI Agents
In `@apps/webapp/app/components/runs/v3/SharedFilters.tsx` around lines 248 - 257,
The same-day check in the "range" case of SharedFilters.tsx currently uses
from.getDate() === to.getDate(), which falsely treats different months/years as
the same day; replace this with a proper full-date comparison by importing and
using isSameDay from date-fns to compute the isSameDay boolean, then keep the
DateTime rendering logic (DateTime date={from!} ... and DateTime date={to!}
includeDate={!isSameDay} ...) so the `to` label only hides the date when the two
timestamps are truly the same calendar day.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/primitives/AppliedFilter.tsx`:
- Around line 54-56: In AppliedFilter.tsx, the JSX condition currently uses
"label !== undefined" which still renders the colon when label is null or empty;
change the conditional around the label block (the JSX that renders <div
className="text-text-bright"><span>{label}</span>:) to use a truthy check (e.g.,
"if (label)" or "Boolean(label)") so the whole label block is omitted for
null/empty/other falsy values — update the conditional in the component's
render/return where "label !== undefined" is used.
♻️ Duplicate comments (4)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (2)
247-257: Fix same-day detection to avoid hiding dates across months/years.Using
getDate()alone compares only the day-of-month (1-31), so Jan 5 and Feb 5 would incorrectly be treated as the same day, omitting the date in thetolabel.🐛 Proposed fix
case "range": { - //If the day is the same, only show the time for the `to` date - const isSameDay = from && to && from.getDate() === to.getDate(); + //If the day is the same, only show the time for the `to` date + const sameDay = from && to && isSameDay(from, to); valueLabel = ( <span> <DateTime date={from!} includeTime includeSeconds /> –{" "} - <DateTime date={to!} includeTime includeSeconds includeDate={!isSameDay} /> + <DateTime date={to!} includeTime includeSeconds includeDate={!sameDay} /> </span> ); }
3-17: MissingisSameDayimport from date-fns.The same-day comparison at line 249 uses
from.getDate() === to.getDate(), which only compares the day-of-month and will incorrectly treat dates like Jan 5 and Feb 5 as the same day. ImportisSameDayfrom date-fns.🐛 Proposed fix
import { endOfDay, endOfMonth, endOfWeek, + isSameDay, isSaturday, isSunday, previousSaturday, startOfDay, startOfMonth, startOfWeek, startOfYear, subDays, subMonths, subWeeks, } from "date-fns";apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (2)
387-389: Regex may mis-detecttriggered_atinside strings/comments.This is the same limitation noted previously; it can disable the UI even when
triggered_atis only in a literal.
543-556: AI time-filter updates only the URL, not the editor state.
handleTimeFilterChangedoesn’t use the newsetTimeFilter, so AI-selected ranges won’t show up in the TimeFilter UI or affect the next submit unless the user manually changes it.🐛 Minimal fix (sync UI state)
const handleTimeFilterChange = useCallback( (filter: AITimeFilter) => { replaceSearchParams({ period: filter.period, from: filter.from, to: filter.to, cursor: undefined, direction: undefined, }); + editorRef.current?.setTimeFilter(filter); }, [replaceSearchParams] );
🧹 Nitpick comments (2)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)
277-295: Prefertypeoverinterfaceper coding guidelines.The coding guidelines specify using types instead of interfaces for TypeScript definitions.
♻️ Proposed refactor
-/** Values passed to onApply callback when a time filter is applied */ -export interface TimeFilterApplyValues { - period?: string; - from?: string; - to?: string; -} +/** Values passed to onApply callback when a time filter is applied */ +export type TimeFilterApplyValues = { + period?: string; + from?: string; + to?: string; +}; -export interface TimeFilterProps { +export type TimeFilterProps = { defaultPeriod?: string; period?: string; from?: string; to?: string; /** Label name used in the filter display, defaults to "Created" */ labelName?: string; hideLabel?: boolean; applyShortcut?: ShortcutDefinition | undefined; /** Callback when the user applies a time filter selection, receives the applied values */ onValueChange?: (values: TimeFilterApplyValues) => void; -} +};apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
357-363: Prefer a type alias over an interface here.This file is TS-only and the project guideline is to use
typealiases.As per coding guidelines, prefer `type` over `interface` in TS files.♻️ Suggested refactor
-interface QueryEditorFormHandle { - setQuery: (query: string) => void; - setScope: (scope: QueryScope) => void; - getQuery: () => string; - setTimeFilter: (filter: { period?: string; from?: string; to?: string }) => void; -} +type QueryEditorFormHandle = { + setQuery: (query: string) => void; + setScope: (scope: QueryScope) => void; + getQuery: () => string; + setTimeFilter: (filter: { period?: string; from?: string; to?: string }) => void; +};
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/webapp/app/components/primitives/AppliedFilter.tsxapps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/AppliedFilter.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/AppliedFilter.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/AppliedFilter.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/AppliedFilter.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/AppliedFilter.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/runs/v3/SharedFilters.tsxapps/webapp/app/components/primitives/AppliedFilter.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
🧬 Code graph analysis (3)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (3)
apps/webapp/app/components/primitives/DateTime.tsx (1)
DateTime(55-102)apps/webapp/app/hooks/useShortcutKeys.tsx (1)
ShortcutDefinition(14-19)apps/webapp/app/hooks/useSearchParam.ts (1)
useSearchParams(7-64)
apps/webapp/app/components/primitives/AppliedFilter.tsx (1)
apps/webapp/app/utils/cn.ts (1)
cn(77-79)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (14)
apps/webapp/app/components/runs/v3/SharedFilters.tsx (2)
timeFilters(139-204)TimeFilter(297-346)internal-packages/clickhouse/src/index.ts (1)
WhereClauseFallback(59-59)internal-packages/clickhouse/src/client/tsql.ts (1)
WhereClauseFallback(28-28)apps/webapp/app/presenters/v3/QueryPresenter.server.ts (1)
QueryHistoryItem(5-15)apps/webapp/app/v3/querySchemas.ts (1)
defaultQuery(430-433)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsx (1)
QueryHistoryPopover(79-139)apps/webapp/app/components/primitives/Tooltip.tsx (1)
SimpleTooltip(141-141)apps/webapp/app/routes/runs.$runParam.ts (1)
loader(12-68)apps/webapp/app/hooks/useSearchParam.ts (1)
useSearchParams(7-64)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/types.ts (1)
AITimeFilter(5-9)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/utils.ts (1)
formatQueryStats(1-20)apps/webapp/app/components/code/TSQLResultsTable.tsx (1)
TSQLResultsTable(886-1184)apps/webapp/app/components/code/ChartConfigPanel.tsx (1)
ChartConfigPanel(80-522)apps/webapp/app/components/environments/EnvironmentLabel.tsx (1)
EnvironmentLabel(78-139)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
🔇 Additional comments (15)
apps/webapp/app/components/primitives/AppliedFilter.tsx (2)
22-30: LGTM on the type definition.Making
labeloptional is a clean approach to support the use case of showing filters without a label.
51-51: Good use of conditional spacing.The dynamic gap adjustment based on label presence maintains proper visual alignment.
apps/webapp/app/components/runs/v3/SharedFilters.tsx (5)
139-204: LGTM!The
labelNameparameter is properly integrated with a sensible default, and all branches correctly propagate it totimeFilterRenderValues.
297-346: LGTM!The controlled mode integration is clean - prop values correctly override URL params, and only
onValueChangeis passed toTimeDropdown, avoiding the previously flagged double-apply issue.
421-501: LGTM!The controlled mode implementation is well-structured:
onValueChangeskips URL navigation for controlled consumersonApplyserves as a post-apply callback for both modes (useful for analytics/side effects)- Dependency array is complete
830-844: LGTM!The shortcut customization with a sensible default (mod+Enter) is a good UX pattern.
864-873: LGTM!The refined logic correctly distinguishes between numeric timestamps and date strings. The regex approach prevents ISO date strings from being incorrectly parsed as timestamps.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (8)
1-70: No review needed for the import adjustments.
72-78: Helper for Date/string normalization looks good.
165-174: Action schema now accepts time-filter inputs—nice.
258-312: Time-filter fallback + history wiring looks solid.
429-493: Time-filter hidden inputs + auto-submit are well wired.
583-749: Results/graph layout refactor looks good.
822-824: Popover trigger change is fine.
846-856: Environment label usage is clear and consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| {label !== undefined && <div className="text-text-bright"> | ||
| <span>{label}</span>: | ||
| </div> | ||
| </div>} |
There was a problem hiding this comment.
Consider handling other falsy values for label.
The label !== undefined check will render just a colon if label is null or an empty string. If the intent is to hide the label block for all falsy values, consider using a truthy check instead.
Suggested fix if hiding all falsy labels is intended
- {label !== undefined && <div className="text-text-bright">
+ {label && <div className="text-text-bright">
<span>{label}</span>:
</div>}And on line 51:
- <div className={cn("flex items-start leading-4", label === undefined ? "gap-1.5" : "gap-0.5")}>
+ <div className={cn("flex items-start leading-4", label ? "gap-0.5" : "gap-1.5")}>If the strict undefined check is intentional (to allow explicit null to still show the colon), please disregard.
🤖 Prompt for AI Agents
In `@apps/webapp/app/components/primitives/AppliedFilter.tsx` around lines 54 -
56, In AppliedFilter.tsx, the JSX condition currently uses "label !== undefined"
which still renders the colon when label is null or empty; change the
conditional around the label block (the JSX that renders <div
className="text-text-bright"><span>{label}</span>:) to use a truthy check (e.g.,
"if (label)" or "Boolean(label)") so the whole label block is omitted for
null/empty/other falsy values — update the conditional in the component's
render/return where "label !== undefined" is used.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/components/code/QueryResultsChart.tsx (1)
538-551: Avoid padding when filling time gaps.
timeDomainincludes padding for axis rendering; using it forfillTimeGapsadds artificial zero buckets at the edges, which can misrepresent the series (e.g., lines dropping to zero outside the real data range). Use the raw min/max timestamps for gap filling and keep padding only for the axis domain.🛠️ Suggested fix
- if (isDateBased && timeDomain) { - const timestamps = dateValues.map((d) => d.getTime()); - const dataInterval = detectDataInterval(timestamps); - data = fillTimeGaps( - data, - xDataKey, - yAxisColumns, - timeDomain[0], - timeDomain[1], - dataInterval, - granularity, - aggregation - ); - } + if (isDateBased && timeDomain) { + const timestamps = dateValues.map((d) => d.getTime()); + const dataInterval = detectDataInterval(timestamps); + const minTime = Math.min(...timestamps); + const maxTime = Math.max(...timestamps); + data = fillTimeGaps( + data, + xDataKey, + yAxisColumns, + minTime, + maxTime, + dataInterval, + granularity, + aggregation + ); + } @@ - if (isDateBased && timeDomain) { - const timestamps = dateValues.map((d) => d.getTime()); - const dataInterval = detectDataInterval(timestamps); - data = fillTimeGaps( - data, - xDataKey, - series, - timeDomain[0], - timeDomain[1], - dataInterval, - granularity, - aggregation - ); - } + if (isDateBased && timeDomain) { + const timestamps = dateValues.map((d) => d.getTime()); + const dataInterval = detectDataInterval(timestamps); + const minTime = Math.min(...timestamps); + const maxTime = Math.max(...timestamps); + data = fillTimeGaps( + data, + xDataKey, + series, + minTime, + maxTime, + dataInterval, + granularity, + aggregation + ); + }Also applies to: 607-620
🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/primitives/charts/ChartBar.tsx`:
- Around line 187-195: The conditional rendering for the ReferenceArea is using
truthy checks (enableZoom && zoom?.refAreaLeft && zoom?.refAreaRight) which
prevents zero-valued bounds from rendering; change the condition in ChartBar.tsx
to use nullish checks so 0 is allowed (e.g., check enableZoom and that
zoom.refAreaLeft and zoom.refAreaRight are not null/undefined via != null or !==
null/undefined). Apply the same fix to the other similar block that renders
inspection lines/zoom (the second ReferenceArea rendering block later in the
file using zoom?.refAreaLeft/Right).
♻️ Duplicate comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (2)
389-391: Regex can still false-positive on string literals/comments.The raw query regex will match
triggered_atinside quoted strings or comments. Consider stripping SQL literals/comments before testing.
545-557: AI time-filter updates don’t reach the form state/submission.
handleTimeFilterChangeonly updates URL search params; the form’s hidden inputs stay stale, so the query results don’t reflect the AI-selected range until a manual change/submit. Please sync the editor state and re-submit.✅ Suggested fix
const handleTimeFilterChange = useCallback( (filter: AITimeFilter) => { + editorRef.current?.setTimeFilter({ + period: filter.period, + from: filter.from, + to: filter.to, + }); + editorRef.current?.submit?.(); replaceSearchParams({ period: filter.period, from: filter.from, to: filter.to, // Clear cursor/direction when time filter changes cursor: undefined, direction: undefined, }); }, [replaceSearchParams] );Also add a submit method on the handle to let the parent trigger a submit after state updates:
-type QueryEditorFormHandle = { +type QueryEditorFormHandle = { setQuery: (query: string) => void; setScope: (scope: QueryScope) => void; getQuery: () => string; setTimeFilter: (filter: { period?: string; from?: string; to?: string }) => void; + submit: () => void; };useImperativeHandle( ref, () => ({ setQuery, setScope, getQuery: () => query, setTimeFilter: (filter: { period?: string; from?: string; to?: string }) => { - setPeriod(filter.period); - setFrom(filter.from); - setTo(filter.to); + flushSync(() => { + setPeriod(filter.period); + setFrom(filter.from); + setTo(filter.to); + }); }, + submit: () => { + if (formRef.current) { + fetcher.submit(formRef.current); + } + }, }), [query] );
🧹 Nitpick comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
360-365: Prefer atypealias forQueryEditorFormHandle.This file is TSX and the guideline requires
typeoverinterface. Please convert to a type alias. As per coding guidelines.♻️ Proposed change
-interface QueryEditorFormHandle { - setQuery: (query: string) => void; - setScope: (scope: QueryScope) => void; - getQuery: () => string; - setTimeFilter: (filter: { period?: string; from?: string; to?: string }) => void; -} +type QueryEditorFormHandle = { + setQuery: (query: string) => void; + setScope: (scope: QueryScope) => void; + getQuery: () => string; + setTimeFilter: (filter: { period?: string; from?: string; to?: string }) => void; +};apps/webapp/app/components/code/QueryResultsChart.tsx (1)
26-33: Prefer a type alias for props.Use a
typealias instead of aninterfaceforQueryResultsChartPropsto match the codebase TS guideline.As per coding guidelines, use types over interfaces.♻️ Suggested refactor
-interface QueryResultsChartProps { +type QueryResultsChartProps = { rows: Record<string, unknown>[]; columns: OutputColumnMetadata[]; config: ChartConfiguration; fullLegend?: boolean; /** Callback when "View all" legend button is clicked */ onViewAllLegendItems?: () => void; -} +};
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/Dialog.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/primitives/charts/ChartContext.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/app/components/primitives/charts/ChartRoot.tsx
- apps/webapp/app/components/primitives/charts/ChartContext.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/Dialog.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/Dialog.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/Dialog.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/Dialog.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/Dialog.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartBar.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/Dialog.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
🧬 Code graph analysis (5)
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx (6)
apps/webapp/app/components/primitives/charts/ChartCompound.tsx (4)
ChartLegendCompoundProps(77-77)ChartLegendCompound(101-101)useChartContext(104-104)useSeriesTotal(105-105)apps/webapp/app/components/primitives/charts/ChartContext.tsx (1)
useChartContext(43-49)apps/webapp/app/components/primitives/charts/ChartRoot.tsx (1)
useSeriesTotal(177-190)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/components/primitives/AnimatedNumber.tsx (1)
AnimatedNumber(4-16)apps/webapp/app/components/primitives/Paragraph.tsx (1)
Paragraph(88-107)
apps/webapp/app/components/code/QueryResultsChart.tsx (1)
apps/webapp/app/components/primitives/charts/ChartCompound.tsx (1)
Chart(92-98)
apps/webapp/app/components/primitives/charts/ChartBar.tsx (6)
apps/webapp/app/components/primitives/charts/ChartCompound.tsx (6)
ChartBarRendererProps(75-75)ChartBarRenderer(101-101)useChartContext(104-104)useHasNoData(105-105)useZoomHandlers(106-106)ZoomTooltip(106-106)apps/webapp/app/components/primitives/charts/ChartContext.tsx (1)
useChartContext(43-49)apps/webapp/app/components/primitives/charts/ChartRoot.tsx (1)
useHasNoData(158-171)apps/webapp/app/components/primitives/charts/ChartZoom.tsx (2)
useZoomHandlers(173-244)ZoomTooltip(105-167)apps/webapp/app/components/primitives/charts/Chart.tsx (2)
ChartTooltip(450-450)ChartTooltipContent(451-451)apps/webapp/app/components/primitives/charts/hooks/useHighlightState.ts (1)
getBarOpacity(95-115)
apps/webapp/app/components/primitives/charts/ChartLine.tsx (5)
apps/webapp/app/components/primitives/charts/ChartCompound.tsx (4)
ChartLineRendererProps(76-76)ChartLineRenderer(101-101)useChartContext(104-104)useHasNoData(105-105)apps/webapp/app/components/primitives/charts/ChartContext.tsx (1)
useChartContext(43-49)apps/webapp/app/components/primitives/charts/ChartRoot.tsx (1)
useHasNoData(158-171)apps/webapp/app/components/primitives/charts/ChartLoading.tsx (3)
ChartLineLoading(63-73)ChartLineNoData(75-94)ChartLineInvalid(96-115)apps/webapp/app/components/primitives/charts/Chart.tsx (2)
ChartTooltip(450-450)ChartTooltipContent(451-451)
apps/webapp/app/components/primitives/Dialog.tsx (1)
apps/webapp/app/utils/cn.ts (1)
cn(77-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/components/primitives/Dialog.tsx (4)
13-19: DialogPortal wrapper looks good.
Nice encapsulation of the portal container styling.
22-35: DialogOverlay export + styling is solid.
Clear, reusable overlay with consistent animation utilities.
37-52: Fullscreen prop wiring is correct.
Conditional class split is clean and keeps the non-fullscreen behavior intact.
116-125: Export updates are consistent.
Public API now exposes the new primitives as expected.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| {enableZoom && zoom?.refAreaLeft && zoom?.refAreaRight && ( | ||
| <ReferenceArea | ||
| x1={zoom.refAreaLeft} | ||
| x2={zoom.refAreaRight} | ||
| strokeOpacity={0.4} | ||
| fill="#3B82F6" | ||
| fillOpacity={0.3} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Allow zero-valued zoom bounds/inspection lines.
Truthy checks skip rendering when the value is 0, which can happen with numeric x-axes. Use nullish checks so zero values still render.
🛠️ Suggested fix
- {enableZoom && zoom?.refAreaLeft && zoom?.refAreaRight && (
+ {enableZoom && zoom?.refAreaLeft != null && zoom?.refAreaRight != null && (
<ReferenceArea
x1={zoom.refAreaLeft}
x2={zoom.refAreaRight}
strokeOpacity={0.4}
fill="#3B82F6"
fillOpacity={0.3}
/>
)}
@@
- {enableZoom && zoom?.inspectionLine && (
+ {enableZoom && zoom?.inspectionLine != null && (
<ReferenceLine
x={zoom.inspectionLine}
stroke="#D7D9DD"
strokeWidth={2}
isFront={true}Also applies to: 258-264
🤖 Prompt for AI Agents
In `@apps/webapp/app/components/primitives/charts/ChartBar.tsx` around lines 187 -
195, The conditional rendering for the ReferenceArea is using truthy checks
(enableZoom && zoom?.refAreaLeft && zoom?.refAreaRight) which prevents
zero-valued bounds from rendering; change the condition in ChartBar.tsx to use
nullish checks so 0 is allowed (e.g., check enableZoom and that zoom.refAreaLeft
and zoom.refAreaRight are not null/undefined via != null or !== null/undefined).
Apply the same fix to the other similar block that renders inspection lines/zoom
(the second ReferenceArea rendering block later in the file using
zoom?.refAreaLeft/Right).
What changed