Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The updated freeze contract will break the existing transcript-native-scrollbar unit test expectations (freeze no longer applies to non-pending rows), so tests/callers must be updated accordingly before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes desktop transcript row misalignment during native scrollbar drag by narrowing the “frozen height” behavior so it only applies to rows whose async content is still pending geometry, while fully-rendered rows keep their real measured height.
Changes:
- Scope
measureTranscriptVirtuosoItem(..., freeze=true)to return cached/estimated heights only for pending-geometry rows (offsetHeight only). - Scope the
itemSizefrozen-heightstyle.heightshim to pending-geometry rows only, avoiding stale estimates on ready rows.
File summaries
| File | Description |
|---|---|
| desktop/frontend/src/lib/useTranscriptScrollArbiter.ts | Restricts the frozen-height shim to pending-geometry rows during native scrollbar drag. |
| desktop/frontend/src/lib/transcriptNativeScrollbar.ts | Restricts freeze-time measurement fallback to pending-geometry rows to prevent stale-height returns for already-rendered rows. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Freeze only rows whose async content is still pending geometry. Keeping | ||
| // the freeze narrowly scoped avoids returning stale estimates for already | ||
| // rendered rows, which would leave them visually misaligned after a manual | ||
| // scroll/selection gesture ends (#transcript-misalignment). | ||
| if (freeze && field === "offsetHeight" && hasPendingTranscriptGeometry(element)) { |
| const pendingGeometry = field === "offsetHeight" && hasPendingTranscriptGeometry(element); | ||
| if (field === "offsetHeight" && frozen) { | ||
| if (field === "offsetHeight" && frozen && pendingGeometry) { | ||
| const estimate = Number.parseFloat(element.dataset.transcriptEstimate ?? element.dataset.knownSize ?? element.dataset.staticEstimate ?? ""); |
|
功能已在上游 v1.31.4 被吸收,PR 过时,关闭以避免重复提交。 |
Summary
During transcript scrolling (especially when using the native scrollbar), already-rendered rows could become visibly misaligned. Holding the pointer made the layout correct; releasing it returned the stale estimate-based height, so rows overlapped or shifted.
Problem
measureTranscriptVirtuosoItemand theitemSizefrozen-height shim applied the freeze to every row while the native scrollbar was dragging:measureTranscriptVirtuosoItem(..., freeze=true)returned the oldtranscriptEstimate/knownSizefor all rows.itemSizealso forcedelement.style.height = estimatefor every frozen row.The freeze is intended to keep a row box stable only while its async content (Markdown, KaTeX, code highlighting) is still pending. For already-rendered rows, returning/forcing the old estimate leaves the DOM box different from the real content height, which is what produced the misalignment after the gesture ended.
Fix
transcriptNativeScrollbar.ts: only return the seed/estimate whenfreeze && offsetHeight && hasPendingTranscriptGeometry(element).useTranscriptScrollArbiter.ts: only apply the frozen-height shim whenfrozen && pendingGeometry.Already-rendered rows now keep their real measured height even during native scrollbar drag, while pending async rows still keep a stable seed to avoid reverse frames.
Verification
Cache impact / guards
Cache-impact: none- desktop host-only UI measurement change; no provider-visible prompt/tool changes.
Cache-guard: none- existing transcript scroll diagnostics / desktop tests; no new guard needed for host-only CSS/measurement path.
Documentation-impact: none- no user-facing docs or config changed.
System-prompt-review: none- desktop host-only change; no system-prompt/boot/config changes.
Chinese summary
修复会话滚动时已渲染行错排:原生滚动条拖动期间,冻结高度只应作用于尚未渲染完的异步内容行;已就绪行应保持真实测量高度。本次在测量函数与
itemSize两处收窄冻结范围。