-
-
Notifications
You must be signed in to change notification settings - Fork 811
New jump to parent or root run buttons #2067
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
|
WalkthroughAdded two SVG icon components: Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
🧠 Learnings (1)📚 Learning: 2025-08-18T10:07:17.368Z
Applied to files:
🧬 Code graph analysis (1)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (8)
⏰ 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)
🔇 Additional comments (5)
✨ 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
🧹 Nitpick comments (1)
apps/webapp/app/components/primitives/TextLink.tsx (1)
60-66
: Minor: repeated shortcut rendering logic can be simplified
renderShortcutKey()
already checksshortcut
andhideShortcutKey
.
The extrashortcut && !tooltip
guard duplicates that logic.
Consider relying on the helper’s own guard to reduce condition nesting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
apps/webapp/app/assets/icons/MoveToTopIcon.tsx
(1 hunks)apps/webapp/app/assets/icons/MoveUpIcon.tsx
(1 hunks)apps/webapp/app/components/Shortcuts.tsx
(1 hunks)apps/webapp/app/components/primitives/TextLink.tsx
(3 hunks)apps/webapp/app/presenters/v3/RunPresenter.server.ts
(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
(9 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/webapp/app/components/Shortcuts.tsx
- apps/webapp/app/assets/icons/MoveUpIcon.tsx
- apps/webapp/app/assets/icons/MoveToTopIcon.tsx
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)
1198-1244
: VerifyLinkButton
API accepts the new props
LinkButton
is now passedhideShortcutKey
,shortcut
,LeadingIcon
, and a newvariant="minimal/small"
.
WhileLeadingIcon
andvariant
existed before,hideShortcutKey
&shortcut
are freshly added forTextLink
.If
LinkButton
still forwards unknown props with...rest
things will work, otherwise TypeScript will error or the props will be silently ignored (no shortcut registered).Please double-check the
LinkButton
definition or add the props there as well.apps/webapp/app/presenters/v3/RunPresenter.server.ts (2)
64-71
: Looks good – consistent selection forparentTaskRun
The additional select block mirrors
rootTaskRun
and keeps the query lean.
No objections.
120-121
: Expose nullableparentTaskRun
inrunData
Nice pass-through; downstream consumers can safely rely on the same shape as
rootTaskRun
.
if (shortcut) { | ||
useShortcutKeys({ | ||
shortcut: shortcut, | ||
action: () => { | ||
if (innerRef.current) { | ||
innerRef.current.click(); | ||
} | ||
}, | ||
}); | ||
} |
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.
Avoid conditional hook calls – they break the Rules of Hooks
useShortcutKeys
is invoked only when shortcut
is truthy.
If the component first renders with shortcut={undefined}
and later re-renders with a shortcut the hook order will change, throwing Invariant Violation: Rendered fewer hooks than expected
.
Unconditionally call the hook and gate the feature inside the hook’s own options instead:
- if (shortcut) {
- useShortcutKeys({
- shortcut: shortcut,
- action: () => {
- if (innerRef.current) {
- innerRef.current.click();
- }
- },
- });
- }
+ useShortcutKeys(
+ shortcut
+ ? {
+ shortcut,
+ action: () => innerRef.current?.click(),
+ }
+ : undefined // hook still called, but no-op when undefined
+ );
🤖 Prompt for AI Agents
In apps/webapp/app/components/primitives/TextLink.tsx around lines 45 to 54, the
hook useShortcutKeys is called conditionally based on the shortcut prop, which
violates the Rules of Hooks and can cause runtime errors. To fix this, call
useShortcutKeys unconditionally every render and move the conditional logic
inside the hook's options or implementation so that the shortcut feature is
enabled or disabled internally without changing the hook call order.
{rootRun || parentRun ? ( | ||
<ShowParentOrRootLinks | ||
relationships={{ | ||
root: rootRun | ||
? { | ||
friendlyId: rootRun.friendlyId, | ||
taskIdentifier: rootRun.taskIdentifier, | ||
spanId: rootRun.spanId, | ||
isParent: parentRun ? rootRun.friendlyId === parentRun.friendlyId : true, | ||
} | ||
: undefined, | ||
parent: | ||
parentRun && rootRun?.friendlyId !== parentRun.friendlyId | ||
? { | ||
friendlyId: parentRun.friendlyId, | ||
taskIdentifier: "", | ||
spanId: "", | ||
} | ||
: undefined, | ||
}} | ||
/> |
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.
Parent-run link loses its spanId (and taskIdentifier) – navigation will break
relationships.parent
is intentionally created with empty strings:
taskIdentifier: "",
spanId: "",
Yet those empty values are later forwarded to v3RunSpanPath
, resulting in
URLs like /runs/ABC?span=
which 404s and breaks keyboard shortcuts.
Pass the real values that are already present on parentRun
:
- parentRun && rootRun?.friendlyId !== parentRun.friendlyId
- ? { friendlyId: parentRun.friendlyId, taskIdentifier: "", spanId: "" }
+ parentRun && rootRun?.friendlyId !== parentRun.friendlyId
+ ? {
+ friendlyId: parentRun.friendlyId,
+ taskIdentifier: parentRun.taskIdentifier,
+ spanId: parentRun.spanId,
+ }
🤖 Prompt for AI Agents
In
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
around lines 592 to 612, the parentRun object is passed with empty strings for
taskIdentifier and spanId, causing broken navigation links. Update the
relationships.parent object to use the actual taskIdentifier and spanId values
from parentRun instead of empty strings to ensure URLs are correctly formed and
navigation works properly.
…tyle # Conflicts: # apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
relationships: { | ||
root?: { | ||
friendlyId: string; | ||
taskIdentifier: string; |
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.
I don't think you need taskIdentifier here or on parent, but it's hard to tell. Remove if you can.
Also, you should pass the spanId for the parent, not an empty string. When you click the parent button it should go to the parent but keep the correct side panel open.
# Conflicts: # apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
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: 0
♻️ Duplicate comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (2)
1158-1174
: Trim relationships type: taskIdentifier is unused in this component.
It’s not read anywhere here; removing it simplifies props and call sites.If you choose to simplify, apply:
- root?: { - friendlyId: string; - taskIdentifier: string; - spanId: string; - isParent?: boolean; - }; - parent?: { - friendlyId: string; - taskIdentifier: string; - spanId: string; - }; + root?: { + friendlyId: string; + spanId: string; + isParent?: boolean; + }; + parent?: { + friendlyId: string; + spanId: string; + };And remove taskIdentifier at the call site (lines 610-612 and 619-619).
604-625
: Parent navigation built with empty spanId (and taskIdentifier) — links and shortcuts will 404.
This re-introduces the previously flagged bug: passing empty strings yields?span=
and breaks navigation to parent and its side panel.Apply this diff to pass real values and to avoid marking “root is parent” when there is no parent:
- isParent: parentRun ? rootRun.friendlyId === parentRun.friendlyId : true, + isParent: !!parentRun && rootRun.friendlyId === parentRun.friendlyId, ... - ? { - friendlyId: parentRun.friendlyId, - taskIdentifier: "", - spanId: "", - } + ? { + friendlyId: parentRun.friendlyId, + taskIdentifier: parentRun.taskIdentifier, + spanId: parentRun.spanId, + }Run to catch similar issues elsewhere:
#!/bin/bash # Look for any empty spanId/taskIdentifier assignments and bare ?span= links rg -nP --type=ts --type=tsx '(spanId:\s*""|taskIdentifier:\s*""|\?span=($|[^0-9a-fA-F-]))' -C2
🧹 Nitpick comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (2)
1210-1261
: Separate Root/Parent buttons: minor spacing and class nits.
- Add gap between buttons for readability.
- Remove
leadingIconClassName="gap-x-2"
; use container gap instead.- return ( - <div className="flex items-center"> + return ( + <div className="flex items-center gap-1.5"> {relationships.root && ( <LinkButton variant="minimal/small" to={v3RunSpanPath( ... - LeadingIcon={MoveToTopIcon} - leadingIconClassName="gap-x-2" + LeadingIcon={MoveToTopIcon} ... )} {relationships.parent && ( <LinkButton variant="minimal/small" to={v3RunSpanPath( ... - LeadingIcon={MoveUpIcon} - leadingIconClassName="gap-x-2" + LeadingIcon={MoveUpIcon}
1179-1206
: Update copy and remove unused leadingIconClassName
- Align label & tooltip to “Root & Parent” for consistency.
- Drop
leadingIconClassName="gap-x-2"
(gap utility on the icon is no-op; spacing is handled by ButtonContent).- leadingIconClassName="gap-x-2" ... - tooltip={ + tooltip={ <div className="-mr-1 flex items-center gap-1"> - <Paragraph variant="extra-small">Jump to root and parent run</Paragraph> + <Paragraph variant="extra-small">Jump to root & parent run</Paragraph> <ShortcutKey shortcut={{ key: "p" }} variant="small" /> </div> } ... - Root/parent + Root & Parent
📜 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 (2)
apps/webapp/app/components/Shortcuts.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
(9 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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:
apps/webapp/app/components/Shortcuts.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/components/Shortcuts.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}
: In the webapp, all environment variables must be accessed through theenv
export ofenv.server.ts
, instead of directly accessingprocess.env
.
When importing from@trigger.dev/core
in the webapp, never import from the root@trigger.dev/core
path; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/components/Shortcuts.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
🧬 Code graph analysis (2)
apps/webapp/app/components/Shortcuts.tsx (2)
apps/webapp/app/hooks/useShortcutKeys.tsx (1)
Shortcut
(7-12)apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
ShortcutKey
(38-55)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (8)
apps/webapp/app/hooks/useOrganizations.ts (1)
useOrganization
(39-43)apps/webapp/app/hooks/useProject.tsx (1)
useProject
(20-24)apps/webapp/app/hooks/useEnvironment.tsx (1)
useEnvironment
(19-23)apps/webapp/app/components/primitives/Buttons.tsx (1)
LinkButton
(337-403)apps/webapp/app/utils/pathBuilder.ts (1)
v3RunSpanPath
(298-306)apps/webapp/app/assets/icons/MoveToTopIcon.tsx (1)
MoveToTopIcon
(1-34)apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
ShortcutKey
(38-55)apps/webapp/app/assets/icons/MoveUpIcon.tsx (1)
MoveUpIcon
(1-41)
⏰ 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 (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 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 (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 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 (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- 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: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
apps/webapp/app/components/Shortcuts.tsx (1)
150-155
: Verify matching useShortcutKeys handlers and disable on inputs
- Ensure the Run page registers “t” (jump to root) and “p” (jump to parent) via
useShortcutKeys
.- Confirm each binding includes
enabledOnInputElements: false
to prevent firing while typing.- Verify the Root/Parent link components receive the
shortcut
prop so tooltips display the correct keys.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (8)
14-14
: Importing useRevalidator for live updates looks correct.
28-29
: New icon imports are appropriate and scoped to app assets.
305-305
: Trace destructuring update is sound.
Parent-friendlyId fields were removed and only the needed values are kept. No issues.
359-359
: Passing parentRun down to TasksTreeView is correct.
Assuming RunPresenter returns parentTaskRun with friendlyId/spanId, this wires through cleanly.
491-495
: Prop type for parentRun matches usage.
No functional issues spotted with the nullability or shape.
511-513
: TasksTreeView signature addition LGTM.
627-627
: Root placeholder copy reads well.
646-646
: Adding parentClassName="pl-3" improves alignment; safe change.
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: 0
♻️ Duplicate comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)
604-623
: Parent link drops spanId — results in broken navigation and shortcuts
relationships.parent.spanId
is set to an empty string, producing URLs like?span=
that 404 and break the “Jump to parent run” action. Pass the realparentRun.spanId
.Apply:
- parent: - parentRun && rootRun?.friendlyId !== parentRun.friendlyId - ? { - friendlyId: parentRun.friendlyId, - spanId: "", - } - : undefined, + parent: + parentRun && rootRun?.friendlyId !== parentRun.friendlyId + ? { + friendlyId: parentRun.friendlyId, + spanId: parentRun.spanId, + } + : undefined,Run to ensure no other empty spanIds are passed to paths:
#!/bin/bash rg -nP '\bspanId:\s*""' -C2 rg -nP 'v3RunSpanPath\([^)]*\{[^}]*spanId:\s*""' -n -C2
🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)
1156-1169
: Tighten relationships typing (optional)Consider a discriminated union for the two render modes to avoid the optional
isParent
flag and reduce conditional complexity.Example:
- Mode "combined":
{ mode: "combined"; root: { friendlyId; spanId } }
- Mode "split":
{ mode: "split"; root: {...}; parent: {...} }
📜 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 (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
(9 hunks)apps/webapp/app/v3/services/completeAttempt.server.ts
(1 hunks)internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts
- apps/webapp/app/v3/services/completeAttempt.server.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
🧬 Code graph analysis (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (8)
apps/webapp/app/hooks/useOrganizations.ts (1)
useOrganization
(39-43)apps/webapp/app/hooks/useProject.tsx (1)
useProject
(20-24)apps/webapp/app/hooks/useEnvironment.tsx (1)
useEnvironment
(19-23)apps/webapp/app/components/primitives/Buttons.tsx (1)
LinkButton
(337-403)apps/webapp/app/utils/pathBuilder.ts (1)
v3RunSpanPath
(298-306)apps/webapp/app/assets/icons/MoveToTopIcon.tsx (1)
MoveToTopIcon
(1-34)apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
ShortcutKey
(38-55)apps/webapp/app/assets/icons/MoveUpIcon.tsx (1)
MoveUpIcon
(1-41)
⏰ 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 (2, 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 (8, 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 (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (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 (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- 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 - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (7)
359-361
: Prop wiring LGTMPassing
parentRun={run.parentTaskRun}
intoTasksTreeView
aligns with the new presenter shape.
491-496
: Type shape looks correct (minor note on unused field)
parentRun
includestaskIdentifier
, which isn’t consumed in this file. Keep if used elsewhere; otherwise prune later.
625-628
: Copy text clarity is good“This is the root task” fallback reads clearly when no root/parent links are applicable.
645-645
: Padding tweak LGTM
parentClassName="pl-3"
improves visual alignment for the tree list.
1175-1202
: Combined Root/Parent link UX is solidGood tooltip + shortcut behavior for the “Root/parent” case.
1208-1241
: Root link implementation LGTMCorrectly targets the root span and advertises the “t” shortcut via tooltip.
1233-1255
: Parent link will work once spanId is fixed aboveWith
spanId: parentRun.spanId
, this link and the “p” shortcut should navigate and keep the correct side panel open.
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
♻️ Duplicate comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)
614-619
: Parent-run link uses empty spanId — navigation breaks
v3RunSpanPath
will generate?span=
and 404. Pass the real parentspanId
.Apply:
- parent: - parentRun && rootRun?.friendlyId !== parentRun.friendlyId - ? { - friendlyId: parentRun.friendlyId, - spanId: "", - } - : undefined, + parent: + parentRun && rootRun?.friendlyId !== parentRun.friendlyId + ? { + friendlyId: parentRun.friendlyId, + spanId: parentRun.spanId, + } + : undefined,
🧹 Nitpick comments (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (2)
623-626
: Copy tweak: say “root run” for consistencyElsewhere we use “run” (e.g., “Jump to root run”).
- <Paragraph variant="extra-small" className="flex-1 pl-3 text-charcoal-500"> - This is the root task - </Paragraph> + <Paragraph variant="extra-small" className="flex-1 pl-3 text-charcoal-500"> + This is the root run + </Paragraph>
1197-1199
: Optional: clarify combined label“Root/parent” can read as a path. Consider “Root & parent” or “Root and parent”.
📜 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 (2)
apps/webapp/app/presenters/v3/RunPresenter.server.ts
(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
(9 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
apps/webapp/app/presenters/v3/RunPresenter.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
apps/webapp/app/presenters/v3/RunPresenter.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
apps/webapp/app/presenters/v3/RunPresenter.server.ts
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧠 Learnings (3)
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Define tasks using task({ id, run, ... }) with a unique id per project
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
apps/webapp/app/presenters/v3/RunPresenter.server.ts
📚 Learning: 2025-07-18T17:49:24.468Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:49:24.468Z
Learning: Applies to internal-packages/database/**/*.{ts,tsx} : We use prisma in internal-packages/database for our database interactions using PostgreSQL
Applied to files:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
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:
apps/webapp/app/presenters/v3/RunPresenter.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (9)
packages/core/src/v3/logger/taskLogger.ts (2)
trace
(103-113)trace
(138-140)apps/webapp/app/hooks/useOrganizations.ts (1)
useOrganization
(39-43)apps/webapp/app/hooks/useProject.tsx (1)
useProject
(20-24)apps/webapp/app/hooks/useEnvironment.tsx (1)
useEnvironment
(19-23)apps/webapp/app/components/primitives/Buttons.tsx (1)
LinkButton
(337-403)apps/webapp/app/utils/pathBuilder.ts (1)
v3RunSpanPath
(298-306)apps/webapp/app/assets/icons/MoveToTopIcon.tsx (1)
MoveToTopIcon
(1-34)apps/webapp/app/components/primitives/ShortcutKey.tsx (1)
ShortcutKey
(38-55)apps/webapp/app/assets/icons/MoveUpIcon.tsx (1)
MoveUpIcon
(1-41)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (1)
apps/webapp/app/presenters/v3/SpanPresenter.server.ts (3)
run
(521-527)run
(529-594)run
(596-598)
⏰ 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 (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 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 (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 (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- 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 (2, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
apps/webapp/app/presenters/v3/RunPresenter.server.ts (1)
3-3
: Type-only PrismaClient import looks goodReduces bundle size and avoids accidental runtime imports.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (4)
28-29
: New icons wired correctlyImports for MoveToTopIcon/MoveUpIcon are fine.
486-493
: Prop types aligned with presenter changes
rootRun
now includesspanId
;parentRun
added. This matches the data shape returned by the presenter.Also applies to: 509-510
359-360
: Passing parentRun through to TasksTreeView is correctKeeps the component API coherent with the new navigation controls.
1215-1219
: Remove unnecessary verification—leadingIconClassName
is supported
ButtonContentPropsType
declaresleadingIconClassName
and applies it in the icon’sclassName
, andLinkButton
’s props extend and spread allButtonContent
props (includingleadingIconClassName
), so these classes will be passed through.
spanId: true, | ||
createdAt: true, | ||
}, | ||
}, | ||
parentTaskRun: { |
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
Root/parent selections updated; confirm no lingering uses of removed taskIdentifier/parentRunFriendlyId
Selections and returned shape look correct for new consumers. Please verify no remaining code paths still read rootTaskRun.taskIdentifier
or parentRunFriendlyId
.
Run from repo root:
Also applies to: 66-71, 120-121
🏁 Script executed:
#!/bin/bash
# Check for removed fields usage
rg -nP -C2 '\b(rootTaskRun|parentTaskRun)\.taskIdentifier\b|parentRunFriendlyId\b'
Length of output: 460
Remove lingering parentRunFriendlyId usage
apps/webapp/app/utils/taskEvent.ts:143 remove or replace the parentRunFriendlyId
reference to match the updated parentTaskRun
shape.
🤖 Prompt for AI Agents
In apps/webapp/app/presenters/v3/RunPresenter.server.ts around lines 61-65, the
presenter still expects a legacy parentRunFriendlyId; update the data shape so
callers no longer use parentRunFriendlyId — remove that reference in
apps/webapp/app/utils/taskEvent.ts line 143 and instead access the new
parentTaskRun field (use parentTaskRun.friendlyId if the parentTaskRun now
exposes a friendly identifier, otherwise use parentTaskRun.id). Ensure the
presenter either includes friendlyId on parentTaskRun or change the consumer to
read the canonical id field so no legacy parentRunFriendlyId is referenced.
root: rootRun | ||
? { | ||
friendlyId: rootRun.friendlyId, | ||
spanId: rootRun.spanId, | ||
isParent: parentRun ? rootRun.friendlyId === parentRun.friendlyId : true, | ||
} | ||
: undefined, | ||
parent: |
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.
🛠️ Refactor suggestion
“Root = Parent” detection misfires when there is no parent
isParent
defaults to true
when parentRun
is null, causing a combined “Root/parent” link to render on true root runs. Only mark isParent
when a parent exists and guard the combined case on both root and parent.
- isParent: parentRun ? rootRun.friendlyId === parentRun.friendlyId : true,
+ isParent: !!parentRun && rootRun.friendlyId === parentRun.friendlyId,
- if (relationships.root?.isParent === true) {
+ if (relationships.root && relationships.parent && relationships.root.isParent) {
Also applies to: 1173-1199
🤖 Prompt for AI Agents
In
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
around lines 606-613 (and similarly at 1173-1199), the isParent flag is being
set to true when parentRun is null which causes root runs to be incorrectly
treated as combined "Root/parent"; change the isParent assignment so it is false
when parentRun is null (e.g., isParent = parentRun ? rootRun.friendlyId ===
parentRun.friendlyId : false) and update any combined-root/parent rendering
logic to require both rootRun and parentRun to be present and equal before
rendering the combined link; apply the same fix to the second occurrence at
lines 1173-1199.
Adds a new options to jump to parent or root from the run page
Normal "Root" state

"Root & Parent" state

"Root & Parent" state tooltip

Separate "Root" and "Parent" buttons

Jump to "Root" button tooltip

Jump to "Parent" button tooltip

Demo
https://github.com/user-attachments/assets/933f5288-a99f-4458-b343-6f55c8c8ea0e