Improve session History on mobile and add message minimap - #55
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed runtime/performance issues in the new minimap/tooltip logic (optional-chaining bug and potential selector exception, plus an avoidable O(n) per-frame scroll cost) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This pull request improves the mobile session History experience in the web/ React UI by adding a scroll-synced message minimap for long timelines, simplifying message visibility (user+assistant always together), and tightening mobile layout/overlay behavior so dialogs and controls remain usable with mobile viewport quirks.
Changes:
- Add a message minimap to the History drawer (scroll wave + tap/drag scrubbing + card highlight) and remove the assistant-only filter.
- Render message/transcript dialogs via
document.bodyportals and adjust modal sizing/safe-area handling for mobile. - Restrict focus-triggered tooltips to keyboard-style focus (
:focus-visible) to avoid “stuck” tooltips after touch taps.
File summaries
| File | Description |
|---|---|
| web/src/styles.css | Adds minimap styling, mobile layout refinements, modal sizing tweaks, reduced-motion handling, and safe-area padding. |
| web/src/components/GlobalTooltip.tsx | Gates focus-triggered tooltips behind :focus-visible to prevent touch-tap tooltip stickiness. |
| web/src/components/AgentSessionPreviewDialog.tsx | Ports the transcript dialog to document.body to avoid mobile stacking/viewport issues. |
| web/src/components/agentSession.ts | Removes message-filtering helper no longer needed after dropping the assistant filter switch. |
| web/src/components/agentSession.test.ts | Removes tests for the deleted filtering helper. |
| web/src/components/AgentMessageDialog.tsx | Ports the full-message dialog to document.body for consistent mobile overlay behavior. |
| web/src/components/AgentHistoryDrawer.tsx | Implements the minimap, scroll/viewport tracking, highlight behavior, and removes assistant filtering UI/state. |
| CHANGELOG.md | Documents the mobile History improvements, minimap, tooltip behavior change, and safe-area fixes. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
The minimap currently triggers duplicate selection on tap/click (pointer-down scrubbing plus per-bar click), causing redundant scroll/highlight behavior.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
web/src/components/AgentHistoryDrawer.tsx:280
- Clicking/tapping a minimap bar currently triggers
onSelecttwice: once via the strip'sonPointerDown(bubbled from the button) and again via the bar button'sonClick. This can restart smooth scrolling and reset the highlight timer unnecessarily. Consider removing the per-baronClickand handling keyboard activation explicitly, since pointer interactions are already covered by the strip-level pointer handlers.
}
title={`#${sequence} ${roleLabel}`}
aria-label={`Go to message ${sequence} (${roleLabel})`}
onClick={() => onSelect(sequence)}
/>
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The minimap bars currently trigger selection twice on pointer interactions (parent onPointerDown + child onClick), which can cause redundant smooth-scroll/highlight behavior and should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
web/src/components/AgentHistoryDrawer.tsx:308
- Each minimap bar is a with its own
onClick, but the parent.agent-history-minimapalso callsonSelect()inonPointerDown. A pointer tap/click on a bar will therefore invokeonSelecttwice (once on pointerdown, once on click), which can restartscrollIntoView({behavior:"smooth"})and reset the highlight timer unnecessarily.
}
title={`#${sequence} ${roleLabel}`}
aria-label={`Go to message ${sequence} (${roleLabel})`}
onClick={() => onSelect(sequence)}
/>
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of confirmed runtime/behavior issues in the new minimap implementation (pointer selection double-fire and unguarded ResizeObserver usage) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
web/src/components/AgentHistoryDrawer.tsx:280
- Tapping a minimap bar can trigger
onSelecttwice: once via the strip’sonPointerDown(which callsonSelect) and again via the bar button’sonClick. This can cause redundant scroll/highlight work and can make scrubbing feel glitchy. Consider only selecting on pointer-down when the event originated from the strip itself (padding/gap), and let bar buttons handle taps/clicks.
const handlePointerDown = (event: ReactPointerEvent<HTMLDivElement>) => {
// Capture so a drag keeps scrubbing even off the strip.
event.currentTarget.setPointerCapture(event.pointerId);
scrubbingRef.current = true;
const sequence = sequenceAtClientX(event.clientX);
if (sequence !== null) onSelect(sequence);
};
web/src/components/AgentHistoryDrawer.tsx:317
ResizeObserveris used here without a feature check. In environments where it’s unavailable, this will throw at effect setup time and break the minimap (the repo already guardsResizeObserverincontextMenuPosition.ts). Consider guarding it and using optional chaining in cleanup.
This issue also appears on line 616 of the same file.
const resizeObserver = new ResizeObserver(update);
resizeObserver.observe(strip);
strip.addEventListener("scroll", update, { passive: true });
return () => {
resizeObserver.disconnect();
strip.removeEventListener("scroll", update);
};
web/src/components/AgentHistoryDrawer.tsx:631
- The visible-range tracking effect also constructs a
ResizeObserverwithout checking for support. IfResizeObserveris undefined, this effect will throw and the drawer won’t render. Guarding it (as done elsewhere in the repo) avoids that hard failure.
const resizeObserver = new ResizeObserver(schedule);
resizeObserver.observe(root);
// Scroll events do not bubble, but capture listeners on window fire for
// scrolls on any descendant, so this covers the content container and
// any ancestor that ends up scrolling instead.
window.addEventListener("scroll", schedule, {
passive: true,
capture: true,
});
window.addEventListener("resize", schedule);
return () => {
if (frame !== 0) window.cancelAnimationFrame(frame);
resizeObserver.disconnect();
window.removeEventListener("scroll", schedule, true);
window.removeEventListener("resize", schedule);
};
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
There are a couple of small but concrete correctness/maintainability issues in the new minimap implementation (debug flag behavior vs comment, and hard-coded gap math for scrubbing) that should be addressed before approval.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
web/src/components/AgentHistoryDrawer.tsx:81
MINIMAP_DEBUG_ENABLEDis documented as being enabled with?debug-minimap=1, but the current.has("debug-minimap")check will also enable it for?debug-minimap=0(or any value). Either update the comment or make the check match the documented=1behavior so the flag is unambiguous.
// Temporary diagnostics for the minimap wave, enabled with ?debug-minimap=1.
const MINIMAP_DEBUG_ENABLED =
typeof window !== "undefined" &&
new URLSearchParams(window.location.search).has("debug-minimap");
web/src/components/AgentHistoryDrawer.tsx:258
sequenceAtClientXhard-codes the minimap bar gap (+ 2) to match the CSSgap: 2px. This will silently break scrubbing math if the gap changes (e.g. responsive tweaks). Derive the gap from computed styles so the mapping stays correct when the CSS changes.
// Keep the gap in sync with .agent-history-minimap in styles.css.
const stride = firstBar.offsetWidth + 2;
if (stride <= 0) return null;
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The new minimap viewport tracking uses ResizeObserver without a guard, which can crash in environments where ResizeObserver is unavailable.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
web/src/components/AgentHistoryDrawer.tsx:578
ResizeObserveris used unguarded here; on platforms/environments where it’s undefined, constructing it will throw and break the History drawer. Consider the same defensive check used elsewhere (e.g. create it only when available) and null-check in cleanup.
const resizeObserver = new ResizeObserver(schedule);
resizeObserver.observe(root);
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
Improves the agent session History view on phones and adds a message minimap for navigating long histories.
Changes:
transform: scaleY, so it stays smooth on mobile), the strip auto-scrolls to keep the wave visible in long histories with a slim position indicator (the heavy global overlay scrollbar is excluded there), and tapping or dragging anywhere on the strip scrubs the timeline to the matching position with a brief card highlight.:focus-visible) focus, so tapping shortcut buttons on touch devices no longer leaves a tooltip stuck.Verification
bun run format:check && bun run lint && bun run test— 715 tests pass.cd web && bun run typecheck && bun run build;cd server && bun run typecheck.