Repository navigation
feat(web-shell): select a time range on the trajectory overview to narrow the table - #12543
Conversation
|
Thanks for the review. On the four non-blocking notes:
The five browser specs have now run on |
chiga0
left a comment
There was a problem hiding this comment.
Tier: Standard — web-shell UI only; no SDK / daemon / persisted-format changes.
rowKeysInRange — closed-interval overlap span.start ≤ range.end && span.end ≥ range.start is correct; handles zero-length spans and boundary-touching spans as confirmed by the unit test suite. ✓
Range–trajectory binding — rangeState.of === trajectory (reference equality) makes the old range expire in the same render as a new trajectory load, preventing the filtered table from persisting after a session reload. ✓
Gesture state machine — gestureRef (mutable ref, no render) for in-flight state, draft (React state) only for the visible band; DRAG_THRESHOLD_PX=4 guard against accidental drag; setPointerCapture keeps drag events alive past element edge; pointerId matching handles multi-touch safely; onContextMenu prevents the browser menu and clears the selection. ✓
pendingRevealRef + useLayoutEffect — clicking a dimmed span clears the range filter (expanding the table) and then scrolls to the newly visible row after React re-renders the unfiltered list. Correct two-phase deferred scroll. ✓
timingAbsent fix — now uses trajectory.rows.length > 0 instead of visualRows.length > 0, so the "no timing" banner is not suppressed when the range filter empties the table. ✓
rangeCounts — shown counts kind === 'row' entries only (excludes turn headers), which matches the test expectation of "2 of 5". ✓
No blockers. 146 unit tests + E2E smoke (160 passed) + 14 mutation kills. Bundle at 1,939,315 B against the 2,030,000 B limit.
yiliang114
left a comment
There was a problem hiding this comment.
Verified the pointer geometry and the filtering model — clean:
- The gesture state machine is right: press records anchor + the span under it (because pointer capture retargets the release to the track — the comment calls out exactly why the span must be remembered at pointerdown); 4px threshold keeps a wobbling click from becoming a range; drag commits on release, click selects the span, click on empty track / right-click / pointer-cancel each do their documented thing (cancel commits nothing). Ends are clamped to the track.
rowKeysInRangeis closed on both sides on purpose — a zero-length span on an edge still counts — and the comment correctly notes the invariant ("any range finds a row") only holds while idle time stays cut from the domain; the follow-up real-clock mode will have to revisit it. Turns keep their header/prompt rows so surviving rows still say which ask they answered.- The follow-up fix (3cd178f) is the right call: a pending reveal now gets exactly one attempt on the first render after the selection drops, rather than waiting open-endedly and scrolling the table at some unrelated later change.
- Evidence ran on this head: E2E Smoke and the visuals job are both completed/success at 3cd178f, so the five real-mouse-input specs (including the 1.5px band-edge assertion and the table-stays-put narrowing check) genuinely executed; the 14 mutation kills cover the nasty corners (open interval, no threshold, draft surviving release, range surviving refresh).
- The header's "Showing n of m" swap and the clear button are wired; refreshing or switching sessions drops the selection in the same render, which is the only safe answer since the compressed axis is relaid out.
CI green on this head (review-pr lane queued — bot infrastructure, not a gate); 0 open threads.
What this PR does
Press on the trajectory overview strip and drag sideways to select a stretch of time. The table then keeps only the requests and tool calls that were running at any point in that stretch, ends included, plus the header and prompt of each turn they belong to, so every surviving row still says which turn and which ask it answered. On the strip, the selection is drawn as a light wash with an edge line on each side, and spans outside it fade. The header swaps its totals for "Showing n of m rows in the selected time" and grows a clear button.
The selection can be dropped four ways: the clear button, Escape on the table, a right click on the strip, or a plain click on empty track. A press that travels less than 4 px is still a click. On a span it selects that row exactly as it does today; on a span that is outside the selection, it drops the selection first and then reveals the row. Refreshing the window, or opening another session, drops the selection in the same render, because the compressed time axis is laid out again and the old numbers would name a different stretch.
Why it's needed
Part of #12293. The overview shows where a run spent its time, but it could not answer the obvious next question, "what happened in these two minutes?", without scrolling the table by hand. This follows the range focus in deepseek-harness's trajectory overview: an inclusive range, where a record active at any point in it counts.
This is the first half of the overview's interaction step. Wheel zoom, right-drag panning and a real-clock mode are left to a follow-up, because they are a second, separate piece of pointer geometry, and each such piece has needed real-browser checks of its own.
Reviewer Test Plan
How to verify
web-shell.trajectory.spec.tsdrive all of this with real mouse input.Evidence (Before & After)
Captured by CI's
Capture web-shell visualsjob on head02dfeab: the same session as the existing trajectory capture, with a stretch dragged across the middle of the overview using real mouse input. The band sits between two edge lines, spans outside it fade, the table keeps the one turn that ran in that time, and the header reads "Showing 5 of 10 rows in the selected time" beside the clear button.web-shell E2E Smokeon the same head: 160 passed, and the log shows all five new cases below executing, not skipped. The export renderer bundle measured 1,942,157 bytes in that run, under the 1,970,000 warning line.Browser specs added, all
@smoke, using realpage.mouseinput:Unit tests: 146 pass across the trajectory hook, loader, projection, range helper, panel and overview. Fourteen mutations were each killed by at least one test, including an open interval instead of a closed one, no drag threshold, a click on a span committing a range, the drag band left drawn after release, turns kept regardless of the range, the prompt row dropped, the range surviving a refresh, no fading, no clamping at the strip's ends, Escape claimed with no selection, the selected span faded, a cancelled press committing, a press outside the range ignored, and the browser menu left open on right click.
Tested on
Environment (optional)
Unit tests locally on Linux. The browser specs run in CI's
web-shell E2E Smoke; Chromium cannot run on the machine these changes were written on.Risk & Scope
Linked Issues
Part of #12293
中文说明
在轨迹面板的时间概览条上按住左键横向拖动,即可选出一段时间。表格只保留在这段时间内运行过的请求和工具调用(闭区间,端点相接也算),并保留它们所在轮次的轮次头和用户提示。概览条上选区显示为浅色底加两侧边线,区间外的条变淡;表头改为显示「区间内 n / m 行」并出现清除按钮。
清除方式有四种:清除按钮、在表格上按 Esc、在概览条上右键、在空白处单击。位移不足 4px 的按下仍算单击:点在条上与原来一样选中并滚到该行;点在区间外的条上,会先清除区间再显示该行。刷新或切换会话会在同一次渲染里清除区间,因为压缩后的时间轴会重排。
这是概览交互的前半部分。滚轮缩放、右键拖动平移、真实时刻模式是另一套指针几何,留给下一个 PR。键盘目前只能清除区间,不能划区间。