fix(web): preserve virtual transcript reading and focus order - #552
Conversation
(cherry picked from commit 0bbc13dce8da0f642a98c5c15726847a368b6ead)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL 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 |
…activity-dom-order
…activity-dom-order
…activity-dom-order
…activity-dom-order
…dom-order Cascade TMPISO-007b repair (a7bed43) from the chain bottom.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e2a7b4138
ℹ️ 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".
|
|
||
| Canonical terminal previews and public answers can arrive in either order. A later run-bound native-present or print answer corrects only its own earlier canonical row, without another completion/unread notification. Native-absent diagnostics are notices, not Activity answers. Virtual scroll uses stable message IDs and additive live remount/recycle hooks; cache correction updates only existing assistant rows in the captured browser cache scope and run. The full answer is never read back from the bounded reducer. | ||
|
|
||
| Virtualized rows are reconciled into geometric order in the DOM before lazy rendering and post-render hooks. Backward scrolling inserts older rows before retained newer rows, so reading and Tab order agree without replacing message or Activity hosts. Retained focus survives reordering without scrolling; evicted rows and focus outside the transcript are not restored. |
There was a problem hiding this comment.
Refresh the structure counts for the changed trees
This change adds a test file and 15 lines under public/, but leaves structure/str_func.md and its count verifier untouched. The repository sync guide explicitly requires updating both whenever tests/ or public/ changes, so refresh the derived structure metadata and its verification coverage in this commit.
AGENTS.md reference: structure/AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
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. |
Summary
Ordinary dependent layer above #551. Draft; merge bottom-up. This PR contains only three files /121 added lines, not the parent's cumulative runtime changes.
Source commit:
0bbc13dce8da0f642a98c5c15726847a368b6ead; cherry-picked without modification onto the exact #551 head. All three file hashes match the independently reviewed source.Original DOM layer validation (
d1f14269b)npm run typecheck:frontendnpm run build:frontendbash structure/verify-counts.sh:538 entries passed.The source owner's independent source/test review passed9 tests across2 files. Source-owner browser keyboard checks were reported separately; they were not rerun by the publisher. Final integrated Electron QA and complete phase acceptance remain pending. Existing theme-boot and bundle-size warnings are unchanged. Hosted CI is tracked on this exact head after publication; queued checks are not passing evidence.
Dependency
The baseline refresh at
39f8c61b9normally merged #551 (d15b1c037), preserving both branch histories. The parent-relative diff remains exactly the same three DOM-order files. Updated-head verification and CI are reported separately; the earlier 39-test result is not relabeled as a new execution.On new head
39f8c61b9, 85 tests /9 named files passed: all seven baseline-repair files listed in #551 plus virtual-scroll-dom-order and web-activity-virtual-host. Existing EG007a/b reentry coverage passed again. Hosted CI is pending on this new head; unrelated remaining parent CI failures are not waived.The following FS refresh at
70fc46c35normally merges #551106ab9f8d. 34 tests /4 files freshly passed (the three FS/elicitation files plus virtual-scroll-dom-order). Parent-relative scope is still exactly three DOM-order files; new-head CI is tracked separately.No native-stack registration, auto-merge, donor closure, release or deployment.
Rendering and bootstrap repair update (c4cd273)
Includes the reviewed Node A repair from
916653397: behavioral badge/final-render proofs, actual Mermaid callback dispatch, settings-failure/location ordering and AST-verified API counts. These repairs are present in lower #551 and propagated normally; the required existing replacement tests are retained there. Each layer freshly passed 60 tests across six named files after that update.Also includes the reviewed Node B repair from
be4f5b3b: observable reconnect/history/sidebar readiness, declared HTTP ledgers and an actually-started stale-A request. Model-free/cache/request-bridge repairs start in #551; the newer redirect fixture is repaired only in its existing #554 layer and descendants. No new test is introduced into an earlier layer merely to apply a patch.Fresh bootstrap verification at this head: 61 tests /6 files passed. The test-only repairs preserve production code, process policy and Windows selection. Exact-head hosted Node/Windows results remain mandatory; previous Windows successes are not relabeled as new-head CI. No whole-suite local run, force-push, rebase, skip or approval bypass.
Shared disconnect repair (b9b764a)
This head carries the shared lowest-layer repair through normal parent merges: disconnect cleanup uses the existing synchronous UI import, and a same-stack exact-effect regression preserves every prior oracle. Fresh Node22 focused verification at this head: 80 tests across 5 files, all passed with closed processes and removed owned roots. Historical CI failures remain anchored to their original heads; fresh hosted CI, other runtime reviews and applicable QA are still required. This update is not final merge approval.
Claude cleanup and steer repair (1cddbde)
The shared repair is applied at #551 and normally propagated here. Main acquisition cleanup stays tracked after logical completion; late fulfillment finishes only its captured control and worker directory cleanup remains worker-only. Main, slash fallback and queued steer wait for main accounting without waiting for intentionally surviving workers; inclusive shutdown and exit-settle/salvage order remain intact. Fresh Node22 focused verification at this exact head: 168 tests across 13 files, all passed with observed process closure and owned-root cleanup. No SDK, protocol, permission or delivery-contract expansion. Other runtime/platform reviews, fresh hosted CI and applicable QA remain required; this is not final merge approval.
Pi preparation and cleanup repair (60a22a4)
Carries the lowest-layer Pi repair through normal parent merges: one asynchronous completed version observation per RPC gates prompt dispatch while the actual RPC child is returned immediately. Both pool capability getters remain live. Direct Stop and RPC exit start the same bounded paired cleanup; first failure claims its result before draining, and temporary worker deletion requires both immutable physical evidence and captured directory identity. Fresh Node22 focused verification at this exact head passed223 tests across11 files with closed processes and removed owned roots. Typed finality is preserved. The older3s command-availability resolver, opaque/escaped wrapper limitations, aggregate shutdown and actual Windows/provider/native-GUI proof remain distinct limits. Fresh hosted CI and applicable review/QA are still required; no final merge approval.