fix(gui): stop the log panel from jittering as rows scroll in - #3290
Conversation
Roadmap unit for the September bug drawdown on dev: 000_plan.md locks a ten-work-phase map from live gh evidence, and 010-090 carry per-item root cause, file:line fix maps, and RED-before-fix assertions sourced from six parallel read-only investigators. Four decade docs cover the open bug-labelled PRs as adoption phases with immutable base/head anchors. Five cover the bug-labelled issues, three of which terminate as NEEDS_HUMAN with the ruled-out causes recorded rather than a speculative patch.
A completed response mutates the continuation cache and calls schedulePersist, and the process-level timer already coalesces those triggers. Under concurrent completions, though, the revision kept changing during the async write, so the bounded-retry loop rewrote the whole snapshot up to four times per background tick. The existing test codified that as the contract. Ordinary background persistence now gets a single attempt. When the snapshot is unstable it schedules the existing delayed follow-up instead of rewriting immediately, so an unstable revision costs one extra debounce interval rather than three extra full atomic rewrites. Graceful shutdown keeps the bounded retry: that path drains requests first and has to land. Closes #3141
The virtualized log table measured rows dynamically from a 44px estimate while its multiline cells are roughly twice that, and the table itself was on automatic layout. Every changed mounted-row subset therefore recomputed intrinsic column widths, model wrapping changed row heights, and that fed another virtualizer measurement. Fixed layout with an explicit ten-column schema removes the feedback loop: column widths no longer depend on which rows happen to be mounted. The estimate moves to the observed mean, getItemKey keys measurements to the request rather than the index so a prepend does not shift them, and the wrapper disables scroll anchoring and reserves the scrollbar gutter. Closes #3152
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05f3ca7998
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| .logs-table-wrap { | ||
| overflow-y: auto; | ||
| overflow-anchor: none; |
There was a problem hiding this comment.
When auto-refresh returns a new request while the user is scrolled below the newest rows, Logs.tsx reverses the source array so that request is inserted above the viewport, but it contains no corresponding scrollTop adjustment. Disabling scroll anchoring here also prevents the browser from keeping the previously visible row in place, so the displayed records shift on every two-second refresh during active traffic. Leave scroll anchoring enabled or explicitly compensate for the prepended row height, and add a scrolled-refresh regression test rather than only asserting that this declaration exists.
AGENTS.md reference: gui/AGENTS.md:L46-L50
Useful? React with 👍 / 👎.
리뷰 · 우선순위 62 / 80이 PR은 대시보드 로그 패널이 스크롤·폴링할 때마다 칸 너비가 흔들리던 문제(#3152)를 고칩니다. 지금 구체적으로는 세 갈래입니다. (1) 현재 라인 526 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
리뷰 · 우선순위 68 / 80이 PR은 대시보드 로그 패널이 스크롤하거나 2초 폴링으로 새 행이 붙을 때마다 칸 너비가 흔들리던 문제(#3152)를 고칩니다. 지금 막 갱신한 고치는 방법은 세 갈래입니다. 첫째, 이 리뷰 턴에서 CI는 게이트·hygiene·테스트 샤드 4개·macos·npm-global까지 이 리뷰 시점에 전부 통과입니다. 이슈 #3152는 아직 OPEN이고 트레이드오프는 작습니다. 고정 퍼센트 칸은 아주 긴 모델명에서 줄바꿈이 더 자주 생길 수 있지만, 그게 바로 흔들림을 없애는 대가입니다. 테스트는 CSS/DOM 스키마를 잠글 뿐 “스크롤 중 칸이 안 흔들린다”는 픽셀 테스트는 아닙니다. PR 본문도 라이브 대시보드로 확인했다고 했고, 그 한계는 차단 사유가 아닙니다. 참고로 #3289 본문의 Closes #3141은 기본 브랜치가 라인 526 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
1 participant
Summary
table-layout: fixedwith an explicit ten-column<colgroup>, so column widths no longer depend on which rows happen to be mounted.estimateSizemoves from 44px to the observed mean row height, andgetItemKeykeys measurements to the request rather than the row index, so a prepend does not shift them..logs-table-wrapdisables scroll anchoring and reserves the scrollbar gutter.Root cause: rows were measured dynamically from a 44px estimate while the multiline cells are roughly twice that, on a table left in automatic layout. Every changed mounted-row subset recomputed intrinsic column widths, model wrapping then changed row heights, and that fed another virtualizer measurement. Polling and scrollbar appearance amplified the loop rather than causing it.
UI change
Screenshot of the Logs & Debug panel, captured live against a running proxy with real request rows.
Before —
devat3d3c4fe, automatic table layout:After — this branch,
table-layout: fixedwith the ten-column schema:The rendered content is deliberately the same; what changes is that column boundaries stop moving as rows scroll in. Verified in the running dashboard rather than only in tests — with this branch built and served on port 10877, the live DOM reports:
On
devthe same query returns notable-layoutdeclaration and no<colgroup>at all.Verification
bun test ./gui/tests/viewport-scroll-caps.test.tsfailed withproperty not found: table.logs-table { table-layout }, and./gui/tests/logs-auto-refresh.test.tsxfailed withexpect(colgroup).not.toBeNull()receivingnull.bun run lint:gui— passed.bun run typecheck— passed. GUI production build — passed.Checklist
Closes #3152
Stacked on #3289; retarget to
devonce that lands.