-
-
Notifications
You must be signed in to change notification settings - Fork 787
Expand useRealtimeRunsWithTag hook to support date range & run status filtering #2413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
WalkthroughClient-side filtering is introduced for realtime runs using a new RealtimeFilterOptions (startDate, endDate, statuses). The hook useRealtimeRunsWithTag now accepts filters via options and recomputes a filtersKey to restart streaming when filters change. Runs are post-filtered locally with useMemo. Server-side skipColumns-based filtering is removed from all subscriptions and processing paths. Ordering for inserted runs changes from createdAt to number. Exported/public API changes: added RealtimeFilterOptions, removed UseRealtimeRunsWithTagOptions, removed skipColumns from UseRealtimeSingleRunOptions, and updated the useRealtimeRunsWithTag signature accordingly. Additional utilities handle date/status extraction, normalization, and stable filter keys. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/react-hooks/src/hooks/useRealtime.ts (1)
654-671
: Critical:run.number
isn’t part ofRealtimeRun
and will always beundefined
TheinsertRunShapeInOrder
helper sorts byrun.number
, but neither the raw Zod schema nor theRunShape
/RealtimeRun
type includes anumber
field. This means every new run will fall through to the end of the list, and updates won’t re-order correctly.Please address by one of the following:
- Add and populate a numeric sequence field (e.g. parse from
friendlyId
or have the API supplynumber
) intransformRunShape
, update theRunShape
/TaskRunShape
types, and ensureRealtimeRun
includesnumber
.- Or switch the ordering logic to use an existing timestamp property (
createdAt
orupdatedAt
) instead ofnumber
.
🧹 Nitpick comments (2)
packages/react-hooks/src/hooks/useRealtime.ts (2)
381-396
: Consider adding type-safe field access for run date extractionWhile the current implementation works, accessing fields through type casting could be improved for better type safety and maintainability.
Consider using a discriminated union or type guard approach:
-function getRunDate<TTask extends AnyTask>(run: RealtimeRun<TTask>): Date | null { - // Common timestamp fields used by different APIs - const candidates = ["startedAt", "createdAt", "started_at", "created_at"] as const; - - for (const key of candidates) { - const val = (run as unknown as Record<string, unknown>)[key]; - if (val instanceof Date) return val; - if (typeof val === "string" || typeof val === "number") { - const d = normalizeToDate(val as string | number); - if (d) return d; - } - } - - return null; +function getRunDate<TTask extends AnyTask>(run: RealtimeRun<TTask>): Date | null { + // Check standard fields first (these should be properly typed on RealtimeRun) + if ('startedAt' in run && run.startedAt) { + return normalizeToDate(run.startedAt as Date | string | number); + } + if ('createdAt' in run && run.createdAt) { + return normalizeToDate(run.createdAt as Date | string | number); + } + + // Fallback to underscore variants for backward compatibility + const runAny = run as any; + if (runAny.started_at) return normalizeToDate(runAny.started_at); + if (runAny.created_at) return normalizeToDate(runAny.created_at); + + return null; }
513-528
: Add explicit type casting for better type safety in filter functionThe filter function uses
any
type which bypasses TypeScript's type checking. Consider using proper type assertions or guards.- // small, readable filter function - return list.filter((run: any) => { + // Apply date and status filters + return list.filter((run) => { const runDate = getRunDate(run); if (start && runDate) { if (runDate < start) return false; } if (end && runDate) { if (runDate > end) return false; } if (allowedStatuses) { const status = getRunStatus(run); if (!allowedStatuses.includes(status)) return false; } return true; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/react-hooks/src/hooks/useRealtime.ts
(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/react-hooks/src/hooks/useRealtime.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
📚 Learning: 2025-08-18T10:07:17.345Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.345Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Import Trigger.dev APIs from "trigger.dev/sdk/v3" when writing tasks or related utilities
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
📚 Learning: 2025-08-18T10:07:17.345Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.345Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Define tasks using task({ id, run, ... }) with a unique id per project
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
📚 Learning: 2025-08-18T10:07:17.345Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.345Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use triggerAndWait() only from within a task context (not from generic app code) and handle result.ok or use unwrap() with error handling
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
📚 Learning: 2025-08-18T10:07:17.345Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.345Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Do not use client.defineJob or any deprecated v2 patterns (e.g., eventTrigger) when defining tasks
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
📚 Learning: 2025-08-18T10:07:17.345Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.345Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use metadata API (metadata.current/get/set/append/stream, etc.) only inside run functions or lifecycle hooks
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
📚 Learning: 2025-07-12T18:06:04.133Z
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
Applied to files:
packages/react-hooks/src/hooks/useRealtime.ts
🧬 Code Graph Analysis (1)
packages/react-hooks/src/hooks/useRealtime.ts (2)
packages/core/src/v3/types/tasks.ts (1)
AnyTask
(645-645)packages/core/src/v3/apiClient/runStream.ts (1)
RealtimeRun
(61-61)
🔇 Additional comments (2)
packages/react-hooks/src/hooks/useRealtime.ts (2)
354-367
: LGTM! Well-structured filter options interfaceThe
RealtimeFilterOptions
interface is well-designed with clear JSDoc comments. The use of union types for date inputs provides good flexibility for consumers.
369-379
: LGTM! Robust date normalization utilityThe
normalizeToDate
function properly handles all expected input types and includes proper null checks and NaN validation for invalid date strings.
// Including filtersKey here to restart the streaming request when filters change | ||
// This ensures we get fresh data when filter criteria are modified | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, [tag, stop, options?.enabled, createFiltersKey(options?.filters)]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix useEffect dependencies to use stable filter key
The current implementation has inconsistent dependency handling - createFiltersKey
is called inline in the dependency array but filtersKey
is computed with useMemo
below. This could cause unnecessary re-renders.
Apply this fix to use the memoized filtersKey
:
+ // Stable key for useMemo deps
+ const filtersKey = useMemo(
+ () => createFiltersKey(options?.filters),
+ [options?.filters]
+ );
+
useEffect(() => {
if (typeof options?.enabled === "boolean" && !options.enabled) {
return;
}
triggerRequest().finally(() => {});
return () => {
stop();
};
- // Including filtersKey here to restart the streaming request when filters change
- // This ensures we get fresh data when filter criteria are modified
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [tag, stop, options?.enabled, createFiltersKey(options?.filters)]);
- // Stable key for useMemo deps
- const filtersKey = useMemo(
- () => createFiltersKey(options?.filters),
- [options?.filters]
- );
+ }, [tag, stop, options?.enabled, filtersKey]);
📝 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.
// Including filtersKey here to restart the streaming request when filters change | |
// This ensures we get fresh data when filter criteria are modified | |
// eslint-disable-next-line react-hooks/exhaustive-deps | |
}, [tag, stop, options?.enabled, createFiltersKey(options?.filters)]); | |
// Stable key for useMemo deps | |
const filtersKey = useMemo( | |
() => createFiltersKey(options?.filters), | |
[options?.filters] | |
); | |
useEffect(() => { | |
if (typeof options?.enabled === "boolean" && !options.enabled) { | |
return; | |
} | |
triggerRequest().finally(() => {}); | |
return () => { | |
stop(); | |
}; | |
// Including filtersKey here to restart the streaming request when filters change | |
// This ensures we get fresh data when filter criteria are modified | |
// eslint-disable-next-line react-hooks/exhaustive-deps | |
}, [tag, stop, options?.enabled, filtersKey]); | |
// …rest of the hook implementation… | |
// (Remove the original `useMemo` call that was below the effect.) |
🤖 Prompt for AI Agents
In packages/react-hooks/src/hooks/useRealtime.ts around lines 492 to 495, the
useEffect dependency array calls createFiltersKey inline which is inconsistent
with the memoized filtersKey computed below and can trigger unnecessary
re-renders; change the dependency array to reference the memoized filtersKey
(i.e., replace createFiltersKey(options?.filters) with filtersKey), ensure
filtersKey is declared with useMemo before this useEffect, and remove the inline
eslint-disable comment if it becomes unnecessary.
const subscription = apiClient.subscribeToRunsWithTag<InferRunTypes<TTask>>(tag, { | ||
signal: abortControllerRef.current?.signal, | ||
onFetchError: onError, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify server-side filtering removal is intentional
The subscription no longer passes filter parameters to the server. Confirm that client-side filtering alone meets performance requirements for the expected data volumes.
Based on the retrieved learning about in-memory filtering being acceptable for paginated results (max 100 runs), this approach should be fine. However, let's verify if there are any pagination limits in place:
🏁 Script executed:
#!/bin/bash
# Check if subscribeToRunsWithTag has any built-in limits or pagination
ast-grep --pattern 'subscribeToRunsWithTag<$_>($$$) {
$$$
}'
# Look for any limit/pagination parameters in the API client
rg -n "subscribeToRunsWithTag" -B3 -A10 --type=ts
Length of output: 6012
🏁 Script executed:
#!/bin/bash
# Show context around the subscribeToRunsWithTag call in useRealtime.ts
sed -n '650,720p' packages/react-hooks/src/hooks/useRealtime.ts
Length of output: 2351
🏁 Script executed:
#!/bin/bash
# Locate the definition of createSearchQueryForSubscribeToRuns
rg -n "createSearchQueryForSubscribeToRuns" -n packages/core/src/v3/apiClient
Length of output: 329
🏁 Script executed:
#!/bin/bash
# Show the createSearchQueryForSubscribeToRuns function definition
sed -n '1040,1120p' packages/core/src/v3/apiClient/index.ts
Length of output: 2048
🏁 Script executed:
#!/bin/bash
# Search for listRuns usage in useRealtime.ts
rg -n "listRuns" -C3 packages/react-hooks/src/hooks/useRealtime.ts
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Show full implementation of subscribeToRunsWithTag in the API client
sed -n '900,1000p' packages/core/src/v3/apiClient/index.ts
Length of output: 3144
Fix parameter usage for subscribeToRunsWithTag and confirm filter removal is intentional
It looks like you’re accidentally passing the signal
/onFetchError
object as the filters param, so:
- No
createdAt
-based server filter is applied (all runs with this tag will be streamed), and - The
signal
andonFetchError
options never reach the SSE subscription.
Please update the call in
packages/react-hooks/src/hooks/useRealtime.ts (around lines 681–684):
- const subscription = apiClient.subscribeToRunsWithTag<InferRunTypes<TTask>>(tag, {
- signal: abortControllerRef.current?.signal,
- onFetchError: onError,
- });
+ const subscription = apiClient.subscribeToRunsWithTag<InferRunTypes<TTask>>(
+ tag,
+ /* filters */ undefined,
+ {
+ signal: abortControllerRef.current?.signal,
+ onFetchError: onError,
+ }
+ );
Then, if you intentionally removed any createdAt
or pagination filters, please verify that your in-memory ordering/duplication logic (insertRunShape) will perform acceptably under the expected stream volume. Consider re-introducing a createdAt
filter or using skipColumns
to limit payload size if needed.
📝 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.
const subscription = apiClient.subscribeToRunsWithTag<InferRunTypes<TTask>>(tag, { | |
signal: abortControllerRef.current?.signal, | |
onFetchError: onError, | |
}); | |
const subscription = apiClient.subscribeToRunsWithTag<InferRunTypes<TTask>>( | |
tag, | |
/* filters */ undefined, | |
{ | |
signal: abortControllerRef.current?.signal, | |
onFetchError: onError, | |
} | |
); |
Closes #1487
✅ Checklist
Testing
filters
option withstatus
,startDate
, andendDate
.Changelog
Extended
useRealtimeRunsWithTag
to accept optionalfilters
.Supported filters:
status
: filter runs by one or multiple statuses.startDate
&endDate
: filter runs within a specific date range.Pls check and tell if needed some changes