Don't show queue time in the run timeline by default #1909
Conversation
|
WalkthroughThis pull request updates the run lifecycle data across several components. In the presenter, a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RunPresenter
participant Database
Client->>RunPresenter: call()
RunPresenter->>Database: Query run (including `startedAt`)
Database-->>RunPresenter: Return run data
RunPresenter->>Client: Return runData (with `startedAt` & `queuedDuration`)
sequenceDiagram
participant TraceView
participant TimelineView
participant TasksTreeView
participant CurrentTimeIndicator
TraceView->>TimelineView: Pass `trace` (includes `queuedDuration`)
TimelineView->>TimelineView: Compute duration using `queueAdjustedNs(totalDuration, queuedDuration)`
TimelineView->>CurrentTimeIndicator: Update current time display with `queuedDuration`
TimelineView->>TasksTreeView: Pass down `queuedDuration` and `showQueueTime` flag
sequenceDiagram
participant Handler as runEngineHandlers
participant EventRepo
Handler->>EventRepo: recordEvent({ startTime, ... })
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)
516-516: Variable naming could be more descriptiveThe variable
queuedTimemight be confusing since it represents whether to use or ignore the queue duration rather than the actual queue time value.- const queuedTime = showQueueTime ? undefined : queuedDuration; + const effectiveQueuedDuration = showQueueTime ? undefined : queuedDuration;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
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(21 hunks)apps/webapp/app/v3/runEngineHandlers.server.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (4)
apps/webapp/app/components/primitives/Switch.tsx (1)
Switch(44-95)packages/core/src/v3/index.ts (3)
millisecondsToNanoseconds(35-35)nanosecondsToMilliseconds(36-36)formatDurationMilliseconds(33-33)apps/webapp/app/v3/taskEventStore.server.ts (1)
TraceEvent(7-24)apps/webapp/app/components/runs/v3/SpanTitle.tsx (1)
eventBackgroundClassName(120-148)
⏰ 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: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
apps/webapp/app/v3/runEngineHandlers.server.ts (1)
336-336: Replace spanIdSeed with startTime for retry spansThe change replaces the previous
spanIdSeedproperty with a precisestartTimein nanoseconds. This timestamp will ensure that retry attempt spans are properly ordered in the timeline tree, addressing the issue mentioned in the PR objectives.apps/webapp/app/presenters/v3/RunPresenter.server.ts (3)
53-53: Include startedAt in the run data selectionAdding the
startedAtproperty to the database selection is necessary to support the new queue duration functionality.
108-108: Expose startedAt in the returned run dataThis change makes the
startedAttimestamp available to the UI components.
206-209: Add queue duration calculation to trace dataThis change calculates the time a task spent in the queue before execution started, converting from milliseconds to nanoseconds for consistent time precision. This is essential for implementing the "hide queue time" feature in the timeline visualization.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (6)
509-509: Default to hiding queue timeSetting
showQueueTimetofalseby default implements the main objective of the PR - hiding queue times in the run timeline by default to improve readability for users.
565-571: Add UI toggle for queue time with keyboard shortcutThis switch implementation with the "Q" keyboard shortcut gives users control to toggle queue time visibility as needed, which matches the PR objective.
1075-1081: Well-implemented queue time adjustment utility functionThe
queueAdjustedNsutility function cleanly handles the adjustment of timeline points and spans based on whether queue time should be displayed. This centralized approach ensures consistency across all time calculations in the timeline.
1243-1246: Add gradient effect to indicate hidden queue timeWhen queue time is hidden, this visual indicator (gradient from dark to transparent) helps users understand that some time has been omitted at the beginning of the span, providing good UI feedback.
1299-1303: Update time indicator to account for queue timeThe current time indicator now correctly adds the queue duration when displaying the absolute timestamp. This ensures the displayed timestamp is accurate regardless of whether queue time is visible.
1368-1368: Document keyboard shortcut for queue time toggleThis change adds the "Q" shortcut to the keyboard shortcuts guide, making the feature discoverable for users.
If you have concurrency limits set you can end up with queue times greater than a few seconds if you add many items to your queue.
This makes the run timeline really annoying because the queue time dominates the horizontal space, making it hard to tell what happened when your code started executing.
This PR makes it so this queue time is hidden by default, and can be turned back on using a toggle (or pressing "Q" on the keyboard).
Fixes