feat(ui): entities swimlanes 3 - #526
Conversation
f51babd to
09cb1f4
Compare
Auto-load a long-entities Gantt under each leaf resource (injected synthetic row in QueryResourceTree), filtered by the same getLongEntitiesThreshold cutoff the timelines use. Remove the entity marks overlay from the resource timelines (and the now-dead "Hide tasks" toggle / hideTasks atom), and stop requesting long_fsms in the timeline bulk/single fetches.
… and state to body
Use the shared three-argument builder API after moving elapsed-time normalization earlier in the stack.
Keep the tooltip and row files compliant with the repository copyright hook.
09cb1f4 to
bc5a38c
Compare
…olbar while no settings
…have same fsm states as timeline; portal tooltip to new re-usable component
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds multi-operator, paginated entity-list queries and infinite fetching. It adds long-entity rows, Gantt tooltips, timestamp lookup utilities, timeline request changes, reusable UI primitives, incremental interval packing, and resource-tree integration. ChangesEntity timeline stack
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (6)
ui/packages/@quent/components/src/lib/timeline.utils.ts-64-67 (1)
64-67: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the threshold documentation.
getLongEntitiesThresholdreturns two times the bin duration. It does not return a fraction of the bin duration. Describe the threshold as two bin durations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/packages/`@quent/components/src/lib/timeline.utils.ts around lines 64 - 67, Update the documentation above getLongEntitiesThreshold to state that the threshold is two bin durations, replacing the inaccurate description that calls it a fraction of the current bin duration. Leave the function implementation unchanged.ui/packages/@quent/client/src/entityList.test.ts-35-50 (1)
35-50: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd an empty-page termination test.
getNextPageParammust stop when the last page is empty, even if the reported total exceeds the accumulated item count. The test only covers the total-reached condition.As per path instructions,
ui/**/*.test.{ts,tsx}requires coverage for empty states.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/packages/`@quent/client/src/entityList.test.ts around lines 35 - 50, Add a test in the entityListInfiniteQueryOptions coverage that passes an empty final page to getNextPageParam while its reported total remains greater than the accumulated item count, and assert that it returns undefined. Keep the existing total-reached assertion unchanged.Sources: Coding guidelines, Path instructions
ui/packages/@quent/components/src/timeline/TimelineTooltip.test.tsx-73-91 (1)
73-91: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd coverage for the empty entity-tooltip state.
EntityTooltipContentaccepts an emptyactiveMarksarray.ActiveMarksSectionthen renders no entity rows. Add a test withactiveMarks={[]}. Assert that the timestamp remains visible and that no entity state row is rendered.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/packages/`@quent/components/src/timeline/TimelineTooltip.test.tsx around lines 73 - 91, Add a test covering EntityTooltipContent with activeMarks set to an empty array. Assert the timestamp remains visible and verify that no entity state row is rendered, reusing the existing tooltip test patterns and selectors.Sources: Coding guidelines, Path instructions
ui/packages/@quent/components/src/long-entities/LongEntitiesGantt.tsx-125-134 (1)
125-134: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRound each visible segment run boundary.
Resource filtering can remove intermediate segments in
buildSegments. Lines 125-134 only inspect array endpoints. A segment before a filtered gap keeps a square right edge, and a segment after the gap keeps a square left edge.Compare neighboring segment timestamps. Start or end a run when the adjacent segment is absent or not contiguous.
Proposed fix
- const isFirst = datum!.segmentIndex === 0; - const isLast = datum!.segmentIndex === entry.segments.length - 1; + const previousSegment = + datum!.segmentIndex > 0 ? entry.segments[datum!.segmentIndex - 1] : undefined; + const nextSegment = entry.segments[datum!.segmentIndex + 1]; + const isFirst = !previousSegment || previousSegment.endMs !== segment.startMs; + const isLast = !nextSegment || nextSegment.startMs !== segment.endMs;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/packages/`@quent/components/src/long-entities/LongEntitiesGantt.tsx around lines 125 - 134, Update the corner-radius logic in the Gantt segment rendering around isFirst, isLast, and r to detect run boundaries from neighboring segment timestamps rather than array endpoints. Treat a missing or non-contiguous previous segment as the run start and a missing or non-contiguous next segment as the run end, so each visible segment run rounds its outer corners while contiguous segments retain square inner seams.ui/packages/@quent/components/src/operator-timeline/utils.test.ts-173-184 (1)
173-184: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover empty timestamp lookups.
Both new lookup helpers need an assertion that an unmatched timestamp returns an empty array.
ui/packages/@quent/components/src/operator-timeline/utils.test.ts#L173-L184: add a timestamp before all spans or after all spans, and assertgetOperatorsAtTimestampreturns[].ui/packages/@quent/components/src/long-entities/utils.test.ts#L172-L203: add a timestamp outside every segment, and assertgetLongEntitySegmentsAtTimestampreturns[].As per coding guidelines, “Test observable behavior and meaningful boundaries, including fallback or unknown inputs, empty and error states.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/packages/`@quent/components/src/operator-timeline/utils.test.ts around lines 173 - 184, Extend the lookup tests to cover unmatched timestamps: in ui/packages/@quent/components/src/operator-timeline/utils.test.ts lines 173-184, assert getOperatorsAtTimestamp returns an empty array for a timestamp before or after all spans; in ui/packages/@quent/components/src/long-entities/utils.test.ts lines 172-203, add an assertion that getLongEntitySegmentsAtTimestamp returns an empty array for a timestamp outside every segment.Sources: Coding guidelines, Path instructions
ui/src/components/LongEntitiesRow.tsx-54-102 (1)
54-102: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRender
useInfiniteEntityListerrors. WithoutthrowOnError, a failed query with no data renders an emptyLongEntitiesGanttbecause the component ignoreserror. Add an error state atui/src/components/LongEntitiesRow.tsx:54-80.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/src/components/LongEntitiesRow.tsx` around lines 54 - 102, Handle the error returned by useInfiniteEntityList before rendering LongEntitiesGantt. Destructure the query error, and when it is present with no usable data, render the component’s established error state instead of an empty timeline; preserve the existing loading and successful-data rendering paths.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/src/components/QueryResourceTree.tsx`:
- Around line 325-332: Update the LongEntitiesRow invocation in
QueryResourceTree to pass fsmStateScope="resource", ensuring synthetic rows
render FSM states scoped to the current resourceId.
---
Other comments:
In `@ui/packages/`@quent/client/src/entityList.test.ts:
- Around line 35-50: Add a test in the entityListInfiniteQueryOptions coverage
that passes an empty final page to getNextPageParam while its reported total
remains greater than the accumulated item count, and assert that it returns
undefined. Keep the existing total-reached assertion unchanged.
In `@ui/packages/`@quent/components/src/lib/timeline.utils.ts:
- Around line 64-67: Update the documentation above getLongEntitiesThreshold to
state that the threshold is two bin durations, replacing the inaccurate
description that calls it a fraction of the current bin duration. Leave the
function implementation unchanged.
In `@ui/packages/`@quent/components/src/long-entities/LongEntitiesGantt.tsx:
- Around line 125-134: Update the corner-radius logic in the Gantt segment
rendering around isFirst, isLast, and r to detect run boundaries from
neighboring segment timestamps rather than array endpoints. Treat a missing or
non-contiguous previous segment as the run start and a missing or non-contiguous
next segment as the run end, so each visible segment run rounds its outer
corners while contiguous segments retain square inner seams.
In `@ui/packages/`@quent/components/src/operator-timeline/utils.test.ts:
- Around line 173-184: Extend the lookup tests to cover unmatched timestamps: in
ui/packages/@quent/components/src/operator-timeline/utils.test.ts lines 173-184,
assert getOperatorsAtTimestamp returns an empty array for a timestamp before or
after all spans; in
ui/packages/@quent/components/src/long-entities/utils.test.ts lines 172-203, add
an assertion that getLongEntitySegmentsAtTimestamp returns an empty array for a
timestamp outside every segment.
In `@ui/packages/`@quent/components/src/timeline/TimelineTooltip.test.tsx:
- Around line 73-91: Add a test covering EntityTooltipContent with activeMarks
set to an empty array. Assert the timestamp remains visible and verify that no
entity state row is rendered, reusing the existing tooltip test patterns and
selectors.
In `@ui/src/components/LongEntitiesRow.tsx`:
- Around line 54-102: Handle the error returned by useInfiniteEntityList before
rendering LongEntitiesGantt. Destructure the query error, and when it is present
with no usable data, render the component’s established error state instead of
an empty timeline; preserve the existing loading and successful-data rendering
paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: bddbcca3-5c02-4adf-808a-45d42c9c9246
📒 Files selected for processing (30)
ui/packages/@quent/client/src/entityList.test.tsui/packages/@quent/client/src/entityList.tsui/packages/@quent/client/src/index.tsui/packages/@quent/components/src/dag/ColorDot.tsxui/packages/@quent/components/src/dag/DataFlowMatrix.tsxui/packages/@quent/components/src/index.tsui/packages/@quent/components/src/lib/timeline.utils.test.tsui/packages/@quent/components/src/lib/timeline.utils.tsui/packages/@quent/components/src/long-entities/LongEntitiesGantt.tsxui/packages/@quent/components/src/long-entities/utils.test.tsui/packages/@quent/components/src/long-entities/utils.tsui/packages/@quent/components/src/operator-timeline/OperatorGanttChart.tsxui/packages/@quent/components/src/operator-timeline/types.tsui/packages/@quent/components/src/operator-timeline/utils.test.tsui/packages/@quent/components/src/operator-timeline/utils.tsui/packages/@quent/components/src/timeline/ResourceTimeline.tsxui/packages/@quent/components/src/timeline/TimelineSettingsPopover.tsxui/packages/@quent/components/src/timeline/TimelineToolbar.tsxui/packages/@quent/components/src/timeline/TimelineTooltip.test.tsxui/packages/@quent/components/src/timeline/TimelineTooltip.tsxui/packages/@quent/components/src/ui/button.tsxui/packages/@quent/components/src/ui/color-swatch.tsxui/packages/@quent/components/src/ui/gantt-tooltip.tsxui/packages/@quent/components/src/ui/pointer-tooltip-portal.tsxui/packages/@quent/hooks/src/atoms/timeline.tsui/packages/@quent/hooks/src/index.tsui/packages/@quent/hooks/src/timeline/useTimelineAtoms.tsui/src/components/LongEntitiesRow.test.tsxui/src/components/LongEntitiesRow.tsxui/src/components/QueryResourceTree.tsx
💤 Files with no reviewable changes (4)
- ui/packages/@quent/hooks/src/timeline/useTimelineAtoms.ts
- ui/packages/@quent/components/src/dag/ColorDot.tsx
- ui/packages/@quent/hooks/src/index.ts
- ui/packages/@quent/hooks/src/atoms/timeline.ts
| <LongEntitiesRow | ||
| engineId={engineId} | ||
| queryId={queryBundle.query_id} | ||
| resourceId={resourceId} | ||
| durationSeconds={durationSeconds} | ||
| fsmTypes={entities.fsm_types} | ||
| isDark={isDark} | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scope displayed FSM states to this resource.
LongEntitiesRow defaults fsmStateScope to 'all'. Each synthetic row therefore fetches entities for resourceId but renders segments that belong to other resources. Pass fsmStateScope="resource" here.
Proposed fix
<LongEntitiesRow
engineId={engineId}
queryId={queryBundle.query_id}
resourceId={resourceId}
durationSeconds={durationSeconds}
fsmTypes={entities.fsm_types}
isDark={isDark}
+ fsmStateScope="resource"
/>📝 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.
| <LongEntitiesRow | |
| engineId={engineId} | |
| queryId={queryBundle.query_id} | |
| resourceId={resourceId} | |
| durationSeconds={durationSeconds} | |
| fsmTypes={entities.fsm_types} | |
| isDark={isDark} | |
| /> | |
| <LongEntitiesRow | |
| engineId={engineId} | |
| queryId={queryBundle.query_id} | |
| resourceId={resourceId} | |
| durationSeconds={durationSeconds} | |
| fsmTypes={entities.fsm_types} | |
| isDark={isDark} | |
| fsmStateScope="resource" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ui/src/components/QueryResourceTree.tsx` around lines 325 - 332, Update the
LongEntitiesRow invocation in QueryResourceTree to pass
fsmStateScope="resource", ensuring synthetic rows render FSM states scoped to
the current resourceId.
…ss jumping around when zooming/panning
Description
Implements the entities gantt chart component as part of the resource timelines + pagination
Unit tests are like ~400 of these lines, sorry about the size regardless i'm happy to split if you want
Related Issues
relates to #215
Testing
cargo run -p quent-simulator -- --num-query-groups 1 --num-queries 1 --num-workers 2 --num-threads 4 --num-tasks 2048 --exporter collector --collector-address http://localhost:7836Screenshots
Screen.Recording.2026-08-05.at.1.56.43.PM.mov