SSE event with pinned older windows leaves a one-seq hole #227

New issue

Closed

opened 2026-09-09 04:19:26 +00:00 by crueber · 3 comments

crueber commented

2026-09-09 04:19:26 +00:00

Owner

Found during the #225 browser pass (not caused by it). With older windows loaded (pinned extras), a remote SSE event slides the newest-50 view while pinned extras stay put, leaving a one-seq gap (e.g. rendered 0..64 missing seq 14); more() then reads false so it persists until reload. Mechanism: own mutations call reload() (clears extras, Issue.jsx:93) but the SSE path (useCollabStream to invalidateCollab) refetches only the view. Fix: SSE path should reconcile pinned extras against the slid window (or reload like own mutations). Acceptance: pinned older windows + remote SSE event leaves no gap; node --test green; browser proof.

crueber referenced this issue from a commit

2026-09-09 04:37:24 +00:00

Fix #227: SSE reconcile pinned windows (02 §11, 02 Decisions)

crueber referenced this issue from a pull request that will close it,

2026-09-09 04:37:26 +00:00

Fix #227: SSE reconcile pinned windows #228

crueber commented

2026-09-09 04:37:29 +00:00

Author

Owner

Fix open as PR #228 (fix/issue-227): SSE path now reconciles pinned extras against the slid newest-50 window via reconcilePinnedWindow (carry the evicted tail, zero round trips) instead of holing the assembly. node --test 442/442 green. Browser proof still open — no runnable browser daemon in this env.

crueber commented

2026-09-09 12:45:55 +00:00

Author

Owner

Re-review of PR #228 (fix/issue-227) after the interrupted attempt.

BRANCH STATE: origin/fix/issue-227 is a single commit f35d9a9 on top of main (67055c1). No review fix commits landed — the branch is unchanged since the PR opened, so the interrupted prior attempt pushed nothing. No PR comments exist on #228.

REVIEW (all clear, no fixes pushed):

web/src/lib/thread-order.js:74-103 reconcilePinnedWindow — correct. floor = min seq of the slid view; carry = prev-view rows strictly below floor and absent from both next and extras (extraSeqs updated as it goes, so dupes within prevView are also excluded). Prev-view iteration preserves newest-first order for the carried head; inputs never mutated; same-reference no-op when nothing pinned/evicted. Growth bounded by thread length (each real seq carried at most once via the extraSeqs guard). more-flag claim holds: the extras tail never moves, so getExtraMore() keeps its meaning after the prepend.
web/src/pages/Issue.jsx:350-357 view-tracking effect — no self-trigger (tracks getView().events only; extras written via setter-updater form which never subscribes). Navigation ordering sound: declared after the line-75 reset effect, and temporally the sync reset on num() always precedes the async refetch that delivers the new view, so a cross-issue change meets already-cleared extras (empty-extras early return = no-op even with overlapping seq spaces). reload() (lines 93-97) untouched — own mutations still drop extras.
Laws: no new deps (AGENTS.md law 1); no core-package/seam impact (law 8); doc entry in docs/features/02_issues.md updated in the same commit and accurate (law 12). Dark/light N/A (pure logic).
Tests pin the one-seq hole (15 carried onto extras head, assembly 0..65 contiguous, length == distinct count), multi-event slides, no-op/reference-identity cases, and input immutability.

VERIFY (scratch worktree /tmp/pr228b, since removed; other agents' /tmp dirs untouched): node --test web/test/unit/*.test.js → 442 pass / 0 fail. vite build (direct binary, web/ dir) → 131 modules, success in 1.64s. (pnpm wrapper refused due to the read-only node_modules symlink tripping its install check — vite itself is the substantive check and it passed.) No browser drive: logic-only change, DOM untouched — noted explicitly per instructions.

RECOMMENDATION: ready to merge.

Re-review of PR #228 (fix/issue-227) after the interrupted attempt. BRANCH STATE: origin/fix/issue-227 is a single commit f35d9a9 on top of main (67055c1). No review fix commits landed — the branch is unchanged since the PR opened, so the interrupted prior attempt pushed nothing. No PR comments exist on #228. REVIEW (all clear, no fixes pushed): - web/src/lib/thread-order.js:74-103 reconcilePinnedWindow — correct. floor = min seq of the slid view; carry = prev-view rows strictly below floor and absent from both next and extras (extraSeqs updated as it goes, so dupes within prevView are also excluded). Prev-view iteration preserves newest-first order for the carried head; inputs never mutated; same-reference no-op when nothing pinned/evicted. Growth bounded by thread length (each real seq carried at most once via the extraSeqs guard). more-flag claim holds: the extras tail never moves, so getExtraMore() keeps its meaning after the prepend. - web/src/pages/Issue.jsx:350-357 view-tracking effect — no self-trigger (tracks getView().events only; extras written via setter-updater form which never subscribes). Navigation ordering sound: declared after the line-75 reset effect, and temporally the sync reset on num() always precedes the async refetch that delivers the new view, so a cross-issue change meets already-cleared extras (empty-extras early return = no-op even with overlapping seq spaces). reload() (lines 93-97) untouched — own mutations still drop extras. - Laws: no new deps (AGENTS.md law 1); no core-package/seam impact (law 8); doc entry in docs/features/02_issues.md updated in the same commit and accurate (law 12). Dark/light N/A (pure logic). - Tests pin the one-seq hole (15 carried onto extras head, assembly 0..65 contiguous, length == distinct count), multi-event slides, no-op/reference-identity cases, and input immutability. VERIFY (scratch worktree /tmp/pr228b, since removed; other agents' /tmp dirs untouched): node --test web/test/unit/*.test.js → 442 pass / 0 fail. vite build (direct binary, web/ dir) → 131 modules, success in 1.64s. (pnpm wrapper refused due to the read-only node_modules symlink tripping its install check — vite itself is the substantive check and it passed.) No browser drive: logic-only change, DOM untouched — noted explicitly per instructions. RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-09 12:52:25 +00:00

crueber referenced this issue from a commit

2026-09-09 12:52:25 +00:00

Merge pull request 'Fix #227: SSE reconcile pinned windows' (#228) from fix/issue-227 into main

crueber commented

2026-09-09 12:52:26 +00:00

Author

Owner

Fixed by PR #228 (re-review clean after interrupted attempt; 442/442 node tests), merged. Closing.

crueber added the

git storage