Skip to content

fix(web): close hover tooltips when the chat timeline scrolls - #7860

Closed
Exotic209093 wants to merge 1 commit into
pingdotgg:mainfrom
Exotic209093:fix/tooltip-dismiss-on-timeline-scroll
Closed

Exotic209093 wants to merge 1 commit into
pingdotgg:mainfrom
Exotic209093:fix/tooltip-dismiss-on-timeline-scroll

fix(web): scope tooltip scroll dismiss to user scrolls

64b0366
Select commit
Loading
Failed to load commit list.
MacroscopeApp / Macroscope - UI Consistency failed Aug 22, 2026 in 22m 31s

UI Consistency: 2 findings in shared tooltip primitive

Findings

  • apps/web/src/components/ui/tooltip.tsx (line 64): the new Tooltip wrapper is non-generic (TooltipPrimitive.Root.Props), so the primitive loses the Payload type parameter it had when Tooltip was TooltipPrimitive.Root. handle degrades to TooltipHandle<unknown>, making the exported TooltipCreateHandle (createTooltipHandle<Payload>()) unusable through the wrapper. Fix: function Tooltip<Payload>(props: TooltipPrimitive.Root.Props<Payload>).
  • apps/web/src/components/ui/tooltip.tsx (lines 96-105): behavior of an app-wide primitive changes (prop forwarding of actionsRef/onOpenChange, hover-only registration, gesture-only dismissal) with no focused test, despite existing conventions in components/ui/*.test.tsx and chat/MessagesTimeline.test.tsx.

Verified as correct (no finding)

  • The scope/listener split fixes the earlier context bug: TooltipScrollDismissListener is a descendant of TooltipScrollDismissScope, so useTooltipScrollDismiss() is non-null and the gesture listeners are attached.
  • details.reason !== "trigger-hover" correctly exempts focus/keyboard opens (Base UI emits "trigger-focus"), and actionsRef.current.close() exists on Tooltip.Root.Actions in the pinned Base UI 1.4.x.
  • Gesture-only gating (wheel/touchmove/pointerdown/scroll keys on the timeline viewport) does keep maintainScrollAtEnd and minimap scrollToIndex from dismissing tooltips; events bubble from the LegendList scroller to the wrapper div, so the listener placement works.
  • Tooltips outside a scope are unchanged: scrollDismiss is null and handleOpenChange only forwards.

Details

Note

Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.

Methodology

  • Compared merge base 549201f and head 64b0366 for apps/web/src/components/ui/tooltip.tsx and apps/web/src/components/chat/MessagesTimeline.tsx.
  • Checked Base UI 1.4.1 semantics used by the patch: TooltipRootActions (unmount + close), change reasons (trigger-hover, trigger-focus, trigger-press, imperative-action), onOpenChange firing on imperative close, and useHover (move: false, blockMouseMove) so an imperative close does not immediately re-open under a stationary pointer.
  • Enumerated Tooltip call sites (ChangedFilesTree, ComposerPendingReviewComments, PullRequest*, settings, ThemePreviewCircles, routes/_chat.pull-requests.tsx): none currently pass handle, payload, actionsRef, open, or defaultOpen, so the generic narrowing is a contract regression rather than an immediate break.
  • Checked whether pointerdown dismissal breaks state-flipping tooltip labels (ChatMarkdown copy/wrap toolbars, ChangedFilesTree expand-all): Base UI already closes on trigger press (closeOnClick defaults to true, referencePressEvent: "sloppy"), so this is pre-existing and not introduced here.
  • Confirmed no keyboard timeline scrolling is implemented app-side; ChatView.tsx keeps its own gesture detector on the LegendList scroll node for live-follow cancellation with an intentionally asymmetric key set, so the new key list is not a drift finding.
  • No CSS/Tailwind, mask, fade, or scrollbar-lane changes are present in this diff, so those rule sections did not apply.

🔍 Agent activity — 26 tool calls

💭 Good, it looks like the previous issue with useTooltipScrollDismiss being called directly in MessagesTimeline was fixed by extracting it into a separate TooltipScrollDismissListener component. Let me check the current files to confirm.

1. browse_code — apps/web/src/components/ui/tooltip.tsx

2. browse_code — apps/web/src/components/chat/MessagesTimeline.tsx

💭 I'm checking whether the tooltip's open-change handler only registers on trigger-hover, and tracing whether closing the tooltip properly triggers unregistration through that same callback path. I'm also weighing whether pointerdown is the right trigger for dismissal — it needs to fire for scrollbar drags, but I want to make sure it doesn't accidentally close tooltips that should persist after a click, like a "Copied!" label. I should also check whether the keydown listener actually captures events when focus isn't inside the div. Unmount should be handled by the cleanup effect, so that's fine. I'm now questioning whether the Tooltip's actionsRef actually exposes a close() method in Base UI, or whether it's something like unmount() instead — I should check how actionsRef is used elsewhere in the codebase to confirm the correct API.

3. git_tools — actionsRef

4. git_tools — Actions

💭 That's odd — the grep result shows a TrackedTooltip<Payload> reference that wasn't in the file I viewed at that revision. I'm wondering if the rev parameter for git_grep isn't being applied the same way as the tree reference I used for the view, which could explain the mismatch. Let me try grepping again with an explicit rev to confirm which content is actually correct.

5. git_tools — actionsRef

💭 I notice the default branch's tooltip.tsx actually has TrackedTooltip<Payload> using a generic Root.Props<Payload>, meaning Base UI's Tooltip Root.Props is generic over Payload. This suggests the PR's version, which drops that generic parameter, could break consumers relying on typed payloads. But I should check whether this is actually relevant to the PR or just a later evolution on main — what matters is the merge base at 549201f, so I need to look at tooltip.tsx there instead.

6. browse_code — apps/web/src/components/ui/tooltip.tsx

💭 I'm noticing the merge base has Tooltip as a generic Root component, and checking whether consumers rely on that generic payload typing with the handle prop. The default-branch version seems to preserve generics through TrackedTooltip, so I want to verify that's intentional before deciding how to reconcile the two.

7. git_tools — TooltipCreateHandle|createHandle

💭 I'm searching for where TooltipCreateHandle is used with a payload, looking for Tooltip usage with a handle parameter.

8. browse_code — In apps/web/src, find all usages of the Tooltip component exported from ~/components/ui/tooltip that pass a handle prop, a payload prop, or use TooltipCreateHandle. Show the code and the generic payload types involved. Also list any usages that pass actionsRef, onOpenChange, open, or defaultOpen to Tooltip.

9. git_tools — handle={|payload={|<Tooltip$|<Tooltip |useTooltipScrollDismiss|TooltipScrollDismissScope

💭 I notice the default branch already has a TrackedTooltip<Payload> pattern, which makes me wonder if main already solved this problem differently and this PR might be diverging from or duplicating that work. I should check main's tooltip.tsx to see what the canonical approach looks like before proceeding.

10. browse_code — apps/web/src/components/ui/tooltip.tsx

💭 The tooltip.tsx discrepancy seems like a red herring, probably just a different checked-out revision, so I'll move past it. Now I want to confirm whether Base UI's Tooltip Root Actions actually exposes a close method by checking the package's type definitions and version. I recall Base UI's Actions API has varied across versions—older ones only had unmount, while newer ones may add close/open. Since the repo has a .repos folder with vendored dependencies, I should check there for the actual base-ui source to confirm the exact API shape.

11. browse_code — .repos/*

12. browse_code — .repos/**/tooltip/root/.ts

💭 I recall code_research mentioned verifying against the Base UI source for TooltipRoot earlier, suggesting it may have repo access. I should ask it specifically about the Actions type for Tooltip.Root in the pinned version—whether close exists and what reasons appear in onOpenChange details, like "trigger-hover".

13. browse_code — In the Base UI (@base-ui/react v1.4.x) Tooltip implementation: (1) What is the exact shape of Tooltip.Root.Actions (the object assigned to actionsRef)? Does it have a close() method, or only unmount()? (2) What are the possible reason values on Tooltip.Root.ChangeEventDetails passed to onOpenChange — is "trigger-hover" a valid reason string for hover-opened tooltips, and what reason is used for focus/keyboard opens? (3) Is onOpenChange invoked when the tooltip is closed programmatically via the actions ref? Quote the relevant source.

💭 I'm checking for re-entrancy issues when dismissAll iterates and closes tooltips, since each close synchronously triggers its own unregister callback that deletes itself from the set mid-iteration — but deleting the current element during iteration is safe, so this should be fine. Now I'm weighing broader concerns: every tooltip instance now carries extra render overhead from the hook-based interception, which could add up given how many tooltips exist across the timeline. More importantly, I want to check whether pointerdown-based dismissal could break tooltips that convey post-click feedback, like a "Copied" tooltip after pressing a copy button, and whether keyboard dismissal on arrow keys or Home/End could interfere with other keyboard-driven interactions in the timeline. I'm also wondering whether Base UI already handles scroll-dismissal natively, which would make this whole interception redundant. And there's a subtler edge case: if a hover-opened tooltip gets force-closed while the pointer is still resting on the trigger, Base UI's hover state machine might not realize the pointer hasn't left, potentially causing odd reopen behavior. I'm trying to recall whether floating-ui's useHover reopens on mousemove over the trigger or strictly requires a fresh pointerenter event — I believe it only listens for mouseenter/mouseleave, so a stationary pointer after imperative close wouldn't trigger reopening until it actually leaves and re-enters. Stepping back, I'm weighing whether this is actually a concrete bug versus expected tooltip behavior, since most UIs close tooltips on scroll anyway. I'm also checking whether dismissal registration correctly scopes to "trigger-hover" reasons for grouped tooltips, and confirming the unregister-on-unmount cleanup looks correct to avoid leaks. Now I'm tracing how TooltipScrollDismissListener receives its node prop and whether timelineViewportElement is properly tracked as state versus a ref, and verifying whether the scroll-dismiss hook usage was actually moved out of MessagesTimeline into a subcomponent as intended, checking the imports to confirm.

14. git_tools — timelineViewportElement|setTimelineViewportElement

💭 I'm checking whether tooltip triggers inside the timeline rows and minimap are properly scoped within the provider context, and starting to identify a potential concrete regression worth flagging. A pointerdown-based dismissal could break copy buttons whose tooltip shows "Copied!" feedback after a click — if the tooltip hides immediately on pointerdown, that feedback would never be visible. I should search for "Copied" tooltip usages in the chat components to confirm this case exists.

15. git_tools — Copied

💭 I'm checking that the copy button's "Copied!" feedback uses a toast rather than the tooltip, so pointerdown dismissal wouldn't affect it, and confirming the minimap's hover tooltip closing on click is expected behavior too. I'm now considering whether this pointerdown-dismissal design could cause issues elsewhere, like with a disclosure-style tooltip. Looking more carefully, I'm weighing whether the Tooltip wrapper dropping the generic payload type parameter from TooltipPrimitive.Root is a real regression — since TooltipCreateHandle is exported for handle-based consumers, this narrowing could silently degrade their typing to unknown even though nothing currently uses that pattern.

16. github_api_read_only — repos/{owner}/{repo}/pulls/{pull_number}/commits?per_page=100

💭 I'm realizing the grep result showing TrackedTooltip might have come from a different location in the workspace, possibly a mirror repo, rather than the actual PR branch I'm examining—since this is a single squashed commit, I need to figure out where that earlier result actually originated from.

17. git_tools — TrackedTooltip

💭 That earlier result seems stale, maybe from a different cache or revision. I should search the git history for TrackedTooltip using a pickaxe search to see if it appears in another commit or branch, possibly as a canonical upstream fix.

18. git_tools

19. git_tools — apps/web/src/components/ui/tooltip.tsx

💭 That grep was a dead end, so I'm moving past it. The real question is whether dismissal actually fires for the intended scenario—since programmatic scroll is now excluded, I need to check whether auto-follow scrolling content under a stationary pointer during streaming still triggers the gating correctly. Wheel events over the portaled tooltip popup should still bubble to the timeline node since the positioner has pointer-events disabled, so that path looks fine. The earlier null dismissTooltips issue is resolved by the listener component. I'm also checking whether the passive pointerdown listener causes any issues (it shouldn't, since nothing calls preventDefault), and whether the keydown listener actually works given that focus likely sits in the composer rather than inside the timeline container, which would mean keyboard-driven scrolling wouldn't even reach this listener in typical usage. This adds a global context and wrapper into the shared tooltip primitive for just one consumer, essentially reimplementing something Base UI doesn't natively support—there's no closeOnScroll feature there, though positioner options exist for collision handling, not dismissal. I'm weighing whether this counts as a reasonable durable contract versus an unnecessary abstraction, but I'd rather pin down a concrete bug, like stale registration when a tooltip reopens without proper cleanup. Tracing through the hover sequence between triggers, the close/register flow seems fine, and the onOpenChange closure looks stable too. I'm now considering whether passing actionsRef and onOpenChange unconditionally to the Root component could cause an issue, though on the surface it seems harmless. Now I'm walking through the actual dismissal scenario step by step: user hovers a trigger, tooltip opens, and I need to verify the pointer stays stationary while checking if the close logic correctly triggers. Actually, I realize there's a subtlety with Floating UI's useHover hook — it has a move option (default true) that listens for mousemove on the reference element and can reopen the tooltip even without a fresh pointerenter, which could undermine the dismissal behavior if the pointer never actually leaves the trigger. That's acceptable as standard tooltip-on-scroll behavior. I should check whether Base UI's positioner already tracks the anchor during scroll via trackAnchor, which keeps the tooltip following the trigger rather than leaving it orphaned — so the real issue the PR describes is likely about the tooltip following the trigger out of the viewport while still clipped or visible. I'm also considering whether dismissAll exempts keyboard-focus-opened tooltips from the hover-registry closing behavior, which seems like intentional, documented scoping rather than a bug. On touch devices, since Base UI tooltips don't open via touch press, pointerdown dismissal shouldn't cause issues there either — so I want to shift focus toward a different category of potential problem, particularly around shared primitives. Since the diff has no CSS or class changes, any findings need to center on interaction semantics and prop forwarding contracts rather than styling. The PR clearly changes primitive composition, prop forwarding like actionsRef merging and onOpenChange interception, state transitions, and keyboard handling — yet it adds no tests for any of this. I want to check whether the repo has an existing testing convention for ui primitives, since if so, the missing focused test would be a legitimate, reportable finding per the check's rules.

20. browse_code — apps/web/src/components/**/.test.

💭 Given the established test conventions elsewhere in the codebase, the lack of any tests for this new tooltip behavior is a legitimate gap worth flagging. I want to dig deeper though and look at the keydown handler logic to see if there's an actual behavioral bug I can point to, not just a missing-test complaint. Checking the event listener typing, it's loose but TypeScript's structural typing allows it through, so that's just a minor style nit rather than a real bug. I'm more interested in whether nested scrollable regions inside the timeline—like a horizontally scrollable code block—would cause wheel events to bubble up and dismiss an open tooltip even though the timeline itself didn't scroll. That feels like a plausible, if minor, regression worth noting. A more concerning case is pointerdown dismissal interacting with toggle controls that have tooltips, like a disclosure button labeled "Expand"—clicking it closes the tooltip and it won't reopen while the pointer stays still, so the user loses feedback when the label flips to "Collapse." But I'm now second-guessing whether Base UI already closes tooltips on trigger press by default (its reason list includes something like "trigger-press"), which would make this pointerdown listener mostly redundant for that case, just adding extra closes when clicking elsewhere in the timeline, which seems harmless enough. I also want to check whether a consumer canceling an open-change event via details.cancel() creates a mismatch: the wrapper would still update its registration bookkeeping as if the close happened, even though the tooltip stays open, meaning it could become unreachable by future scroll-dismissal. This seems like a minor edge case since no current consumer cancels opens. I'm now looking at whether registration actually fires correctly in practice. Checking whether TooltipProvider's onOpenChange fires properly on the Root, and whether all timeline tooltip instances genuinely live within the intended scope — rows rendered through renderItem inside LegendList appear to be inside it, and portal-rendered tooltips elsewhere seem fine too. I also consider whether MessagesTimeline renders in multiple places like the chat view and an inline preview panel, but since scope is per-instance, that should be fine. The strongest remaining concern worth weighing is whether keyboard scroll keys could dismiss hover tooltips while a user is typing in an inline editor within the timeline. I'm thinking through whether editable inputs exist inside the timeline — inline message editing, review comment composers, or terminal/diff comment fields — since pressing arrow keys or Home/End in those would dismiss tooltips without any actual scroll occurring, though the impact is minor since it just closes the tooltip. All the behavioral findings so far are minor, but since the review defaults to failure unless concrete violations are found, I should only report real issues or conclude "all clear." I also want to double check that the previously flagged null-context bug fix actually works by confirming the listener component renders inside the scope, which it does at line 613 inside the relevant wrapper. I'm verifying that the scope component is rendered by the timeline with the listener as a child so context reads correctly, and that the scope's internal callback set is properly instantiated per scope instance. Now I'm examining a potential correctness issue with how the merged actions ref handles imperative handle assignment — checking whether React's ref-setting behavior works correctly when the ref is a plain object with accessor properties rather than a standard mutable ref. This should typecheck fine. I'm now considering a subtler issue: mutating a ref during render to mirror the latest actions is a known "latest ref" pattern, but it's technically unsafe under concurrent rendering since writes during render can be discarded or duplicated in StrictMode double-renders. Turning to the virtualized timeline, I'm checking whether the scroll listener correctly targets the wrapper rather than the inner scroller, and whether LegendList's root element is properly nested so gesture-based dismissal still works. I'm also weighing edge cases like scrollbar-drag (handled since dismissal triggers on pointerdown) and keyboard-driven scrolling via space or page keys, which might need separate handling. Given the check's guidance to flag only clear regressions, I'm leaning toward the strongest finding being the lack of focused test coverage for this shared-primitive behavior change—prop forwarding, hover-only registration, and gesture-only dismissal—especially since the repo already has established test conventions for similar components. I'm also checking whether there's a harder bug lurking around tooltips whose triggers sit inside the timeline but whose Root lives outside that scope, particularly for portaled overlays anchored to timeline content. Ruling out memoization concerns since dismiss is stable with an empty dependency array, and checking whether dismissAll's use of close() with an imperative-action reason avoids triggering flushSync during hover-driven updates. I'm now wondering whether calling dismissAll repeatedly on every wheel event during a scroll could cause performance issues from the frequent iteration. One concrete issue worth flagging: pointerdown dismissal isn't scoped to scrolling, so it closes hover tooltips on any press inside the timeline, including text selection or clicking a control whose tooltip reflects state — and keydown arrow handling fires even while typing in inline editors. Both feel like interaction-semantics changes beyond the PR's stated scroll-dismissal scope, since they live in a shared primitive. I should check whether timeline rows actually contain text inputs, like an inline message-edit field, to see if this is a real violation.

21. browse_code — In apps/web/src/components/chat, does anything rendered INSIDE the MessagesTimeline list rows (i.e. descendants of the timeline viewport div in MessagesTimeline.tsx, including TimelineRow components, user message editing, inline diff/review comment editors, terminal context chips) render a focusable text input, textarea, or contenteditable editor? Also: are there controls inside timeline rows wrapped in Tooltip whose tooltip label changes based on state after a click (e.g. expand/collapse toggles labelled "Expand"/"Collapse", pin/unpin, mute)? Give concrete file paths and code snippets.

💭 I'm noticing that pointerdown dismissal closes tooltips on click for toggles whose labels flip state, like "Copy code" becoming "Copied" — since that feedback is delivered through the tooltip itself and the tooltip is already open on hover when clicked, dismissing it on pointerdown would hide that confirmation. But I should check whether Base UI's tooltip already closes on trigger press by default, since there's a "trigger-press" close reason mentioned in its API. If that's already the default behavior, this wouldn't be a regression introduced by the PR at all — I need to verify this against the actual Base UI source before concluding it's a real issue.

22. browse_code — In Base UI v1.4.1's Tooltip implementation: when the user CLICKS (mouse press) on a Tooltip.Trigger while the tooltip is already open from hover, does the tooltip close? Which hooks does TooltipRoot use (useHover, useFocus, useClientPoint, useDismiss, useClick?) and what generates the "trigger-press" reason for tooltips? Quote the relevant source from TooltipRoot.tsx / TooltipTrigger.tsx / useHover.ts. Also: after an imperative close() while the pointer remains stationary over the trigger, does Base UI's useHover reopen the tooltip on subsequent pointer movement within the trigger (is the move option enabled)?

💭 I realize Base UI already closes tooltips on trigger pointerdown by default, so that listener's effect on trigger clicks isn't actually a new regression—the "Copied" label issue predates this PR. But the pointerdown listener could still cause a minor side effect: closing tooltips elsewhere when someone clicks to select text within a message. There's also a wheel-scroll edge case: scrolling inside a nested scroller like an expanded tool-call body or code block would close an open tooltip even though the timeline itself didn't move. Checking the reopen behavior, since the pointer needs to leave and re-enter the trigger after an imperative close, this stays consistent with existing press-to-close behavior, so it's acceptable. With the 600ms hover delay making tooltips fairly deliberate already, I'm not finding a hard functional regression—just noting the lack of focused tests for this shared behavior as a remaining gap. On the pointerdown listener, I initially thought it was redundant with Base UI's built-in trigger-press dismissal, but it actually serves a purpose: it also catches scrollbar-drag interactions, which look identical to programmatic scroll events otherwise. So that's not a real finding. Now I'm looking at whether the keydown listener only fires under certain focus conditions. This is still an unverified claim though, since it depends on whether keyboard scrolling happens at the document level rather than through the node itself. I should check whether the LegendList scroller is actually focusable and whether there's a document-level PageUp/PageDown handler controlling the timeline.

23. browse_code — In apps/web/src, how does keyboard scrolling of the chat message timeline work? Search for keydown handlers involving "PageUp", "PageDown", "Home", "End", "ArrowUp", "ArrowDown" that scroll the timeline / LegendList (e.g. in ChatView.tsx, MessagesTimeline.tsx, keybinding registries, or document/window-level listeners). Also: does the LegendList scroller element or the timeline viewport div receive tabIndex/autoFocus so that keydown events would bubble to the timeline viewport div? Give file paths and snippets.

💭 I notice ChatView.tsx already has its own wheel/touchmove/pointerdown/keydown listeners on the LegendList's actual scroller node for detecting user scroll gestures to cancel live-follow — this means the PR is introducing a duplicate gesture-detection layer with a slightly different key set and targeting a different element (the wrapper div instead of the real scroller). This feels like a legitimate consistency issue worth flagging: rather than adding a parallel listener set with divergent keys, the new tooltip-dismissal logic should probably reuse the existing user-gesture signal that ChatView already computes, since having two separate sources of truth for "did the user scroll the timeline" is fragile. The wrapper-div attachment still works functionally since these events bubble, so that part isn't a real bug — but I want to check whether this duplication is significant enough to call out as a concrete violation introduced by the changed lines. I'm double-checking the listener wiring is correct across render states — the wrapper div exists for all timeline states, touch and pointer events bubble properly to it, and the null-ref-then-attach sequence on first render works fine. Now I'm considering whether tooltips rendered outside MessagesTimeline by ChatView are also covered by this dismissal logic. My conclusion: I'll flag the duplicated gesture-detection logic between the two components and recommend reusing ChatView's existing detector, along with noting the lack of focused test coverage — though since the two detectors operate on different scopes and nodes, this is more of a consistency nit than a hard regression risk. This is a legitimate but modest finding since the instructions do authorize requiring focused tests for interaction contracts on shared primitives. I'll draft one comment asking for a test covering hover-open dismissal on wheel scroll, exemption for programmatic scroll and focus-opened tooltips, and confirmation that actionsRef still passes through correctly — then decide whether this alone warrants a failing state. Let me do one more pass checking for controlled tooltips inside the timeline that might interact badly with the dismiss-on-scroll logic, since that would be a more concrete bug than a missing test. Actually that case does work fine since PageDown scrolls the nearest scrollable ancestor and the keydown bubbles up to trigger dismissal, so there's no dead code there. I'm also noting ChatView already wires up the same four listeners to its scroll node, so MessagesTimeline could potentially reuse that pattern. Thinking about the strongest behavioral issue now — the orphan-tooltip problem likely isn't unique to this timeline but affects other virtualized or scrollable lists using the same primitives, like the sidebar thread list, changed files panel, or PR lists. Given the fix is scoped to just one consumer, which the check tolerates under "prefer the smallest durable contract," I'm weighing whether to report "all clear" since there's no concrete violation — but the output format requires valid JSON per the schema, so I need to encode that "all clear" state within the JSON structure itself, using success status and an appropriate summary, rather than literally outputting the plain text phrase. Now I'm deciding whether requiring a focused test here is justified enough to flag as a failure, checking against the rule about requiring tests when primitives are composed together. This PR changes behavior, prop forwarding, state transitions, and keyboard handling all within a shared primitive, with zero test coverage — that's exactly the scenario the test-coverage rule targets, so I'll flag it. I also noticed the code mutates a ref's current value during render, but since that write is idempotent it's safe under StrictMode's double-rendering and doesn't rise to a real finding. So my plan is to post one inline comment on the new Tooltip wrapper requesting focused test coverage for the new contract, pointing to existing test conventions in the codebase as examples to follow. I'm also weighing whether to flag that ChatView already implements an equivalent gesture detector on its scroll node, which would make the new gesture key list in this PR duplicative — I'll fold that observation into a second, separate inline comment on the MessagesTimeline file rather than cluttering the tooltip comment, keeping things to two targeted comments instead of risking noise with more. I second-guess whether suggesting reuse of onManualNavigation is actually correct, since that's only triggered by specific gestures, not any scroll interaction—so I shouldn't overstate the equivalence. Given the rule explicitly endorses requesting tests, I'll settle on posting one focused comment about that rather than pushing the detector-duplication point too hard. Before finalizing, I want to check if there's a repo-local lint rule (like from oxlint-plugin-t3code) around addEventListener or context usage that the new code might actually violate—that would be a more concrete, verifiable finding than my speculative ones. I also want to check whether MessagesTimeline.test.tsx could break due to the new scope and pointerdown dismissal logic.

24. browse_code — oxlint-plugin-t3code/rules/*.ts

💭 No listeners mentioned there. I should check AGENTS.md or contributing docs for conventions like avoiding inline event listeners or requiring tests for UI components, since that could reinforce the testing gap I found.

25. git_tools — test

💭 I'm weighing whether the missing test is really a repo-convention violation, since AGENTS.md's testing rule is framed around backend behavior, though the check instructions do seem to authorize requiring focused tests for this kind of frontend change given existing test coverage patterns. I'm also considering whether the "hit every surface" guidance about consistent behavior across views applies here, though it doesn't seem directly relevant to scoping the fix to one consumer. Checking whether mobile or desktop clients have their own chat timeline that shares this tooltip bug — but MessagesTimeline appears to be the single implementation, so that's settled. I'm now leaning toward posting one inline review comment on the tooltip primitive asking for focused test coverage of the new shared contract, while also considering whether to flag the duplicated gesture-detection logic between the primitive and MessagesTimeline as a separate, more substantive issue. Actually, I've found that ChatView already has its own nearly identical gesture detector attached to the same scroller, with overlapping listener types but a different key set than MessagesTimeline's — that's a real "two owners, one signal" inconsistency introduced by this diff, and it's concrete enough to cite specific line ranges in both files. The cleanest fix would be having one of them reuse the other's detector rather than maintaining two parallel listener sets, so I'll write this up as a suggestion-style comment alongside the missing-tests comment on tooltip.tsx, making sure both land on valid diff lines. scroll-gesture detector already exists in ChatView that cancels live-follow on wheel/touch/pointer/keydown events against the same scrollable node, but this new code adds a second, differently-scoped listener set on the wrapper div, risking drift between the two definitions of "user scrolled." I'm weighing whether to suggest deriving the dismissal from the existing detector via a callback prop, though I recognize MessagesTimeline owning its own viewport interactions is also a reasonable design. I'm leaning toward proposing a shared hook like useTimelineScrollGesture that both components could use, since that's more actionable than just pointing at ChatView's ownership. There's also a second, smaller comment about missing focused tests for the new tooltip contract around line 104. Neither issue is a hard defect, so I'm deciding whether to mark this as a failure or just flag these as suggestions. The instructions say the check defaults to failure unless there are no findings, in which case I should respond with exactly "All clear." Since I'm considering posting these two comments, that would mean failing the check for consistency reasons — but I want to weigh whether these really rise to the level of "concrete violations" versus optional preferences before committing to that. I'm weighing whether a missing test actually counts as a "concrete violation introduced by changed lines" per the reporting rules, versus just a best-practice gap. Given the stated requirement to verify typing/interaction contracts with focused tests, I'll go ahead and post the coverage finding and mark the result as a failure. Before finalizing, I want to double-check whether there's a genuine behavioral defect I can actually prove, which would make this decision moot. I'm tracing through the tooltip scope wrapping again — checking markdown code block toolbars, the minimap hover preview, and the terminal context chip — all seem correctly scoped, so I'm now looking for whether there's a second instance I might have missed. Other surfaces like the PR timeline tab, sidebar thread list, or changed files tree could have the same orphaned tooltip issue, but I'll stick to the narrower scope rather than demanding a broader fix. I'll settle on leaving one focused comment on tooltip.tsx near the handleOpenChange function, requesting targeted tests, and work out the exact wording now. I'm also noticing that Tooltip lost its generic Payload type parameter compared to the merge base, which means the handle is now typed as unknown and the exported TooltipCreateHandle helper can no longer be passed in without a type error — that's a real typing contract narrowing worth flagging even without current consumers. Confirming that Root.Props is indeed generic over Payload in v1.4.1, with createHandle producing a typed TooltipHandle. Since TooltipHandle likely uses Payload in both argument and return positions, passing a typed handle into the non-generic Tooltip wrapper (which expects TooltipHandle) would fail type assignability, confirming the bug is real and worth flagging. I'm proposing a minimal one-line fix: making the Tooltip wrapper function itself generic over Payload so it forwards the type parameter through to TooltipPrimitive.Root.Props, while leaving actionsRef and other non-generic parts untouched since Actions isn't payload-dependent. I'll note that TooltipTrigger has the same pre-existing generic gap but decide not to flag it since it's out of scope for this review. I'll post this as an inline suggestion on line 64, with a caveat that TS inference might need verification, plus a second comment around line 104 asking for more focused test coverage.

26. modify_pr — Reviewed the tooltip primitive change and its timeline call site. The scope/listener split now resolves correctly (the listener is a descendant of the provider, so useTooltipScrollDismiss() is non-null), and gating registration on details.reason === "trigger-hover" does exempt focus/keyboard opens ("trigger-focus") as documented. Two findings on the shared-primitive contract below.


Agent Credits: 921 credits