From 1fbaf82d3c9208e0392cf2889f46c08f8adc0bd9 Mon Sep 17 00:00:00 2001 From: witbrock Date: Thu, 17 Sep 2026 12:35:23 -0400 Subject: [PATCH] Keep refreshed message history contiguous after unread seek --- .../unread_refresh_repair_2026_09_17.md | 57 +++++++++++++++++ .../static/js/components/messagePanel.js | 14 +++-- tests/browser/messageUnreadNavigation.cjs | 24 ++++++- tests/frontend/messageExchangeView.test.js | 62 +++++++++++++++++++ 4 files changed, 152 insertions(+), 5 deletions(-) create mode 100644 docs/engineering/unread_refresh_repair_2026_09_17.md diff --git a/docs/engineering/unread_refresh_repair_2026_09_17.md b/docs/engineering/unread_refresh_repair_2026_09_17.md new file mode 100644 index 00000000..82a7e7c8 --- /dev/null +++ b/docs/engineering/unread_refresh_repair_2026_09_17.md @@ -0,0 +1,57 @@ +# Unread-window refresh repair — 17 September 2026 + +Evidence for native task `#V#task_agent_5497a7092c696239e79ced69414ac9a1`. +The native task remains the decision surface. This repairs the navigation +regression following PR #740 without requesting production deployment. + +## Reproduction and scope + +The supplied manager regression fails on `29327080be6ac7235cf2223148d98216176179f4`: +after seeking to an older unread window, manual Refresh renders both that window +and a disconnected latest page (`Expected: false; Received: true`). The test was +run before changing production code and is retained with stronger assertions. + +Source evidence, staged and checksum-verified by the controller: + +- Patch: `#V#computer_file_copy_7e3e5575afea4192a547e7f076ea9ea5`, SHA-256 + `30a206a193417e211c5ad86b14cdd30ac5a74d93bd0914b5a58f70021692178c`. +- Manager proof: `#V#computer_file_copy_e233925b936642d59fd57885b03e8b69`, SHA-256 + `5f1475d34a8bc0518b5ff7fd34904872b9f02c47e96cc6676787ad8ddf8e5aa8`. + +Manual Refresh now replaces an earlier bounded window with the latest page and +resets both cursors. Automatic refresh, including partial read-ack fallback, +cannot join or navigate away from an earlier window. An unconfirmed read retains +its unread label and existing Retry action; explicit Refresh/Latest remains +available. Ordinary latest-page reconciliation also replaces a disjoint result +instead of displaying a gap as continuous history. + +Direct callers were inspected: initial open and post-send loading already replace +the window; unread seek and Latest explicitly replace it; earlier/later controls +extend it through chronological cursors; polling has its existing guard. The +repair leaves exchange/actor/organisation selection and composer logic intact. +PR #692 was open during validation; its composer changes were inspected and are +outside the changed production hunks. + +## Validation + +Tier 1 evidence on the candidate based on the above merge: + +- 29 exchange-view tests pass, including all previous 26 and three regression + cases: manual Refresh, partial read receipt/retry, and disconnected new arrivals. +- 58 tests pass across six exchange/message-panel suites, covering draft, reply, + send-failure, read and navigation behaviour. +- All 24 existing backend exchange-catalogue/message-route tests pass through + PDM; no backend code or schema changed. +- Static frontend lint and `git diff --check` pass. +- `tests/browser/messageUnreadNavigation.cjs` passes at 390×850 and 1280×850 in + Chromium. The actual Conversation actions → Refresh messages control replaces + the older window, clears the later cursor, preserves the unsent draft, resumes + polling, and retrieves earlier history with the latest page's cursor. Existing + seek, visible-read receipt, catalogue and send checks remain passing. + +Browser results and inspected screenshots are retained in +`.run/refresh-navigation/` in the task worktree. These use production frontend +modules/CSS with synthetic HTTP responses, not authenticated production data or +public deployment. A partial read receipt while viewing older history remains +uncertain until retry or explicit navigation; labels are never optimistically +cleared. No broader production-health or latency claim is made. diff --git a/src/frontend/web/von_interface/static/js/components/messagePanel.js b/src/frontend/web/von_interface/static/js/components/messagePanel.js index 02cd668c..ff664ac0 100644 --- a/src/frontend/web/von_interface/static/js/components/messagePanel.js +++ b/src/frontend/web/von_interface/static/js/components/messagePanel.js @@ -571,7 +571,9 @@ function attachMessagesEventListeners() { if (refreshBtn) { refreshBtn.addEventListener('click', () => { if (_currentConversationUserId) { - loadConversation(_currentConversationUserId, { silent: true }); + // Refresh is explicit navigation when reading an earlier window. + // Replace it with the latest page and reset both cursors. + loadConversation(_currentConversationUserId, { silent: true, latest: Boolean(_newerCursor) }); } }); } @@ -872,6 +874,10 @@ function bindMessageStreamMenu(element, getOtherUserId) { */ async function loadConversation(userId, { silent = false, before = null, after = null, firstUnread = false, latest = false, observeReads = true } = {}) { if (_isLoading) return; + // Automatic refreshes (including partial read-ack recovery) must not join + // the latest page to an earlier window or move the reader away from it. + // Explicit Refresh/Latest and directional pagination remain available. + if (silent && _newerCursor && !before && !after && !firstUnread && !latest) return false; _isLoading = true; const generation = _exchangeGeneration; const org = getSessionScopedOrgId(); @@ -898,10 +904,10 @@ async function loadConversation(userId, { silent = false, before = null, after = if (before || after) nextMessages = [...new Map([..._currentMessages, ...incoming].map(m => [m.concept_id, m])).values()].sort(compare); else if (silent && !firstUnread && !latest && incoming.length) { // Reconcile the returned range, including deletions and edits, while - // retaining already loaded earlier pages. A reconnect gap remains pageable. + // retaining earlier pages only when the returned page overlaps them. const overlap = incoming.some(m => _currentMessages.some(old => old.concept_id === m.concept_id)); - if (!overlap && response.before) _olderCursor = response.before; - nextMessages = [..._currentMessages.filter(m => compare(m, incoming[0]) < 0), ...incoming]; + if (!overlap && response.before) latest = true; + else nextMessages = [..._currentMessages.filter(m => compare(m, incoming[0]) < 0), ...incoming]; } const sameContent = JSON.stringify(nextMessages.map(messageRenderKey)) === JSON.stringify(_currentMessages.map(messageRenderKey)); _currentMessages = nextMessages; diff --git a/tests/browser/messageUnreadNavigation.cjs b/tests/browser/messageUnreadNavigation.cjs index 7322fcd4..eafbc995 100644 --- a/tests/browser/messageUnreadNavigation.cjs +++ b/tests/browser/messageUnreadNavigation.cjs @@ -153,12 +153,34 @@ const server = http.createServer(async (req, res) => { await expect(target).toBeInViewport(); assert.deepEqual(cursors, ['first_unread']); await page.screenshot({ path: path.join(evidence, `unread-${width}.png`) }); + // Exercise the visible Refresh action, not the background helper. + await page.locator('#messageViewHeader summary').click(); + await page.getByRole('button', { name: 'Refresh messages', exact: true }).click(); + await expect(page.locator('[data-contribution-id="latest-00"]')).toBeVisible(); + await expect(target).toHaveCount(0); + await expect(page.getByRole('button', { name: 'Load later messages', exact: true })).toHaveCount(0); + await expect(input).toHaveValue('A reply from the up-arrow control.'); + assert.equal(sent.length, 0, 'refresh must preserve the unsent draft'); + await page.screenshot({ path: path.join(evidence, `refreshed-${width}.png`) }); + // Polling resumes once Refresh reaches latest, and the older cursor + // belongs to the latest window rather than the abandoned unread one. + const requestsBeforePoll = cursors.length; + await page.evaluate(async () => { + const panel = await import('/static/js/components/messagePanel.js'); + await panel.refreshOpenMessageExchange(); + }); + assert.equal(cursors.length, requestsBeforePoll + 1); + await page.getByRole('button', { name: 'Load earlier messages', exact: true }).click(); + await expect.poll(() => Boolean(pendingPage)).toBe(true); + assert.equal(cursors.at(-1), 'middle'); + pendingPage(); pendingPage = null; + await expect(page.locator('[data-contribution-id="middle-0"]')).toBeVisible(); await send.click(); await expect(input).toHaveValue(''); assert.equal(sent.length, 1); assert.equal(sent[0].content, 'A reply from the up-arrow control.'); assert.deepEqual(errors, []); - results.push({ width, cursors: [...cursors], sendArrow: true, sendTarget: box, sends: sent.length, labelledLatestNavigation: true, defaultTopPreserved: true, backgroundPosition: bottomPosition, pendingAnchorOffset: anchor.offset, firstUnreadFocused: true, targetInViewport: true }); + results.push({ width, cursors: [...cursors], manualRefreshReplacesWindow: true, manualRefreshPreservesDraft: true, pollingResumes: true, refreshedHistoryCursor: 'middle', sendArrow: true, sendTarget: box, sends: sent.length, labelledLatestNavigation: true, defaultTopPreserved: true, backgroundPosition: bottomPosition, pendingAnchorOffset: anchor.offset, firstUnreadFocused: true, targetInViewport: true }); await page.close(); // Real IntersectionObserver and confirmed HTTP receipt: the last diff --git a/tests/frontend/messageExchangeView.test.js b/tests/frontend/messageExchangeView.test.js index 51c5839e..4f7597c3 100644 --- a/tests/frontend/messageExchangeView.test.js +++ b/tests/frontend/messageExchangeView.test.js @@ -463,3 +463,65 @@ test.each([0, 1])('post-read catalogue refresh reconciles stale counts after a r staleRefresh([exchange]); expect(document.querySelector('.message-jump-unread').hidden).toBe(true); }); + + +test('manager regression: manual refresh does not join disconnected unread and latest windows', async () => { + HTMLElement.prototype.scrollIntoView = jest.fn(); + const api = require(base + 'apiService.js'); + api.postJson.mockResolvedValueOnce(page(['latest'], 'previous', [], 15)) + .mockResolvedValueOnce({ ...page(['unread'], 'earlier', ['unread'], 1), after: 'later' }); + await require(base + 'components/messagePanel.js').openMessageExchange({ ...row('#V#bob'), shared_unread_count: 1 }); + document.querySelector('.message-jump-unread').click(); await flush(); + expect(document.querySelector('[data-contribution-id="unread"]')).not.toBeNull(); + api.postJson.mockResolvedValueOnce(page(['latest'], 'previous', [], 15)); + document.getElementById('refreshMessagesBtn').click(); await flush(); + const oldWindow = document.querySelector('[data-contribution-id="unread"]'); + const newWindow = document.querySelector('[data-contribution-id="latest"]'); + expect(Boolean(oldWindow && newWindow)).toBe(false); + expect(oldWindow).toBeNull(); + expect(newWindow).not.toBeNull(); + expect([...document.querySelectorAll('button')].some(el => el.textContent === 'Load later messages')).toBe(false); + // Normal refresh resumes, and history uses the new page's cursor. + const panel = require(base + 'components/messagePanel.js'); + await panel.refreshOpenMessageExchange(); + expect(api.postJson).toHaveBeenCalledTimes(4); + [...document.querySelectorAll('button')].find(el => el.textContent === 'Load earlier messages').click(); await flush(); + expect(api.postJson.mock.calls.at(-1)[1].before).toBe('previous'); +}); + +test('partial read receipt in an earlier window retains unread state and offers retry without joining latest', async () => { + HTMLElement.prototype.scrollIntoView = jest.fn(); + const observers = mockReadObserver(); + const api = require(base + 'apiService.js'); + api.postJson.mockResolvedValueOnce(page(['latest'], 'previous', [], 15)) + .mockResolvedValueOnce({ ...page(['unread'], 'earlier', ['unread']), after: 'later' }); + await require(base + 'components/messagePanel.js').openMessageExchange({ ...row('#V#bob'), shared_unread_count: 1 }); + document.getElementById('messageInput').value = 'Keep this draft'; + document.querySelector('.message-jump-unread').click(); await flush(); + api.postJson.mockResolvedValueOnce({ success: true, updated_count: 0 }); + observers.at(-1).callback([visibleEntry()]); await flush(); + expect(api.postJson).toHaveBeenCalledTimes(3); + expect(document.querySelector('[data-contribution-id="latest"]')).toBeNull(); + expect(document.querySelector('[data-contribution-id="unread"]').classList.contains('is-unread')).toBe(true); + const retry = document.querySelector('.message-read-status button'); + expect(retry.textContent).toBe('Retry'); + retry.click(); + api.postJson.mockResolvedValueOnce({ success: true, updated_count: 1 }); + observers.at(-1).callback([visibleEntry()]); await flush(); + expect(document.querySelector('.is-unread')).toBeNull(); + expect(document.getElementById('messageInput').value).toBe('Keep this draft'); + expect([...document.querySelectorAll('button')].some(el => el.textContent === 'Load later messages')).toBe(true); +}); + +test('a refresh after a disconnected new arrival replaces the window and keeps history pageable', async () => { + const api = require(base + 'apiService.js'); + api.postJson.mockResolvedValueOnce(page(['old'], 'old-history')); + const panel = require(base + 'components/messagePanel.js'); + await panel.openMessageExchange(row('#V#bob')); + api.postJson.mockResolvedValueOnce(page(['new'], 'new-history', [], 15)); + await panel.refreshOpenMessageExchange(); + expect(document.querySelector('[data-contribution-id="old"]')).toBeNull(); + expect(document.querySelector('[data-contribution-id="new"]')).not.toBeNull(); + [...document.querySelectorAll('button')].find(el => el.textContent === 'Load earlier messages').click(); await flush(); + expect(api.postJson.mock.calls.at(-1)[1].before).toBe('new-history'); +});