Cover the changes tab and stop it clobbering the saved scroll position - #974
Merged
Merged
Conversation
The Changes tab's orchestration layer had no coverage: `changes-tab.tsx` wires the diff query, the persisted per-agent view/review state, the file tree and the diff renderer together, and `changes-diff-section.tsx` routes each file's own drafts, feedback and selection down to it. Both of its children (`unified-diff-view`, `use-diff-widgets`) are covered, so only the wiring between them was unguarded. `changes-tab.test.tsx` mounts the real tab with the renderer stubbed (it has its own suite) and pins: the `active` gate on both queries, the path sort and per-file routing, hide-test-files with the deep-link exception, the loading and two empty states, deep-link navigation by file, line and feedback id, the review draft lifecycle, collapse persistence, the scroll debounce, and the truncated-file load path. Writing the scroll test surfaced a real bug. The flush-on-unmount effect measured an element captured when the effect first ran — null, because the loading placeholder was still up — and React nulls the ref before cleanups run anyway, so unmounting the tab inside the 300ms debounce wrote `scrollTop: 0` over the position the user had scrolled to. The handler now records the offset it measured and the cleanup flushes that, disarming both refs so an agent switch mid-debounce cannot replay one agent's offset onto the next. Also gives the review bar's cancel button an accessible name, which it was missing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Recurring Test Enforcer run. Local suite was green on the first pass (30th consecutive), so the run went to the queued
next_focus: the diff/review orchestration layer.Coverage
apps/web/src/components/app/changes-tab.tsx(367 lines) andchanges-diff-section.tsx(369) had no test sibling and no covering caller — their only caller,agents-view.tsx, is itself uncovered. Both of their children (unified-diff-view.tsx,use-diff-widgets.tsx) are covered by #931, so the untested part was purely the wiring between them.changes-tab.test.tsx(18 tests) mounts the real tab —DiffPane,FileDiffSection,FileDiffContent,FileTreeandReviewModeBarall render for real — with onlyUnifiedDiffViewstubbed, since it has its own suite. The stub reproduces the one piece of react-diff-view DOM this file reaches into (td[data-change-key]rows, keyed with the library's owngetChangeKey) so the deep-link line scroll is exercised end to end rather than mocked away.Pinned behaviors: the
activegate on both queries · path sort and per-file routing of drafts, feedback and line selection · hide-test-files with the deep-link exception · loading and the two distinct empty states · deep-link navigation by file, line and feedback id (history replace, uncollapse, row scroll, stale-focus guard) · the review draft lifecycle including "removing the last draft leaves review mode" · collapse persistence for files and directories · the scroll debounce, unmount flush and restore · the truncated-file on-demand load (force=true).Mutation battery: 37 mutants, 35 killed. The two survivors are proven redundancies, not gaps — each dies when mutated together with the layer that duplicates it.
Product fixes
Scroll position clobbered on unmount. The flush-on-unmount effect measured an element captured when the effect first ran —
null, because the loading placeholder was still up — and React nulls the ref before cleanups run in any case. So leaving the tab inside the 300 ms scroll debounce wrotescrollTop: 0over the position the user had scrolled to. The handler now records the offset it measured and the cleanup flushes that value, disarming both refs so an agent switch mid-debounce cannot replay one agent's offset onto the next (a second defect the review pass caught in the first version of this fix, now covered by its own regression test).Missing accessible name on the review bar's icon-only cancel button.
Validation
pnpm run check·pnpm run format·pnpm run lint:web·pnpm run finalize:web·pnpm run test(server 163 files/2791, web 932 → 950, extension 60, scripts 4) ·pnpm run test:e2e180 passed/12 skipped ·pnpm run test:e2e:live11 passed in 22 s with no leakede2e-*tmux sessions. The new file and the full web suite were each re-run repeatedly to check for order dependence and flakiness.🤖 Generated with Claude Code