timeline-chart/src/layer/__tests__/time-graph-chart-rich-cursor.test.ts (1)

165-177: ⚡ Quick win

Consider asserting the tooltip position for edge-aware behavior.

The test verifies that a tooltip is drawn near the right edge, but doesn't assert that it's actually positioned to the left of the cursor as the test name suggests. Consider adding an assertion on bg.x to verify the edge-aware positioning logic.

📍 Suggested assertion to verify left-side positioning

             // Draw near right edge (canvas is 800px wide)
             (cursor as any).drawCursor(790);
             const bg = (cursor as any).tooltipContainer.children[0] as PIXI.Graphics;
-            // The tooltip box should be positioned to the left of cursor
-            expect(bg).toBeTruthy();
+            // The tooltip box should be positioned to the left of cursor
+            expect(bg).toBeTruthy();
+            expect(bg.x).toBeLessThan(790);

🤖 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 `@timeline-chart/src/layer/__tests__/time-graph-chart-rich-cursor.test.ts`
around lines 165 - 177, The test 'flips tooltip to left when near right edge'
currently only checks that the tooltip background exists; update it to assert
the tooltip is positioned left of the cursor by comparing the bg.x (from (cursor
as any).tooltipContainer.children[0]) to the cursor draw x (use the same 790
value or read a public/accessible cursor property like (cursor as any).lastX if
available) and expect bg.x to be less than the cursor x (or less than 790 minus
the tooltip width if you can read it) to verify edge-aware flipping logic in
drawCursor.

timeline-chart/src/layer/time-graph-chart-rich-cursor.ts (1)

93-118: ⚖️ Poor tradeoff

Consider object pooling for tooltip elements.

New PIXI.Text and PIXI.Graphics objects are created on every mousemove event. With many visible rows, this could cause performance issues or GC pressure. Consider reusing objects or implementing an object pool, especially if users frequently move the mouse across the chart.

🤖 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 `@timeline-chart/src/layer/time-graph-chart-rich-cursor.ts` around lines 93 -
118, This code creates new PIXI.Text (textObj) and PIXI.Graphics (bg) on every
mousemove and should instead reuse pooled/displayed tooltip objects to avoid GC
and allocation spikes: add persistent members (e.g., this.tooltipText and
this.tooltipBg) or an object pool used by the mousemove handler to update
text/geometry and reposition the existing PIXI.Text and PIXI.Graphics rather
than creating new ones, ensure tooltipContainer.addChild is called only once (or
check contains) and update textObj.text, textObj.x/y and
bg.clear()/bg.drawRoundedRect(...) as needed when showing/hiding the tooltip.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

timeline-chart/src/layer/time-graph-chart-cursors.ts (1)

110-111: 💤 Low value

Prefer reading modifier keys from the federated event directly.

The mousedown handler at lines 80–84 reads event.shiftKey, event.ctrlKey, and event.altKey directly from the FederatedPointerEvent. However, the globalmousemove and mouseup handlers cast to event.nativeEvent for the same reads—an unnecessary inconsistency. PixiJS v7's FederatedPointerEvent API exposes these modifier properties directly, eliminating the need for the cast.

♻️ Proposed simplification

                 if ((this.mouseButtons & 1) === 0) {
                     // handle missed button mouseup event
                     this.mouseSelecting = false;
-                    const orig = event.nativeEvent as PointerEvent;
-                    if (!orig.shiftKey || orig.ctrlKey || orig.altKey) {
+                    if (!event.shiftKey || event.ctrlKey || event.altKey) {
                         this.stage.cursor = 'default';
                     }
                     return;
                 }

             if (this.mouseSelecting && event.button === 0) {
                 this.mouseSelecting = false;
-                const orig = event.nativeEvent as PointerEvent;
-                if (!orig.shiftKey || orig.ctrlKey || orig.altKey) {
+                if (!event.shiftKey || event.ctrlKey || event.altKey) {
                     this.stage.cursor = 'default';
                 }
             }

Also applies to: 132-133

🤖 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 `@timeline-chart/src/layer/time-graph-chart-cursors.ts` around lines 110 - 111,
The globalmousemove and mouseup handlers currently cast event.nativeEvent to
PointerEvent to read modifier keys; change them to read shiftKey/ctrlKey/altKey
directly from the FederatedPointerEvent (the event parameter) — i.e., remove the
`const orig = event.nativeEvent as PointerEvent` pattern and use
`event.shiftKey`, `event.ctrlKey`, `event.altKey` in the globalmousemove and
mouseup handlers so they match the mousedown handler and PixiJS v7's
FederatedPointerEvent API.

timeline-chart/src/layer/time-graph-chart-rich-cursor.ts (1)

164-170: 💤 Low value

Tooltip Y position may overflow canvas bounds.

boxY is computed as y - boxHeight / 2 without clamping, which could position tooltips partially or fully outside the visible canvas area when the cursor is near the top or bottom edges.

🔧 Suggested clamping

         let boxY = y - boxHeight / 2;
+        const canvasHeight = this.stateController.canvasDisplayHeight;
+        if (boxY < 0) {
+            boxY = 0;
+        } else if (boxY + boxHeight > canvasHeight) {
+            boxY = canvasHeight - boxHeight;
+        }

🤖 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 `@timeline-chart/src/layer/time-graph-chart-rich-cursor.ts` around lines 164 -
170, Tooltip vertical position (boxY) can overflow the canvas when the cursor is
near the top or bottom; clamp the computed boxY (currently set as y - boxHeight
/ 2) to the visible area by ensuring boxY is no less than 0 and no greater than
canvasHeight - boxHeight (use y, boxHeight, canvasHeight and the existing boxY
calculation), so the tooltip is always fully inside the canvas while preserving
the horizontal flip logic that uses boxX and boxWidth.