From cc96747b6886568a959ddba0147df807a3cc9fbe Mon Sep 17 00:00:00 2001 From: witbrock Date: Thu, 17 Sep 2026 12:17:19 -0400 Subject: [PATCH] Seek directly to unread messages without loading entire exchange --- .../unread_message_latency_2026_09_17.md | 66 +++++++++++++ src/backend/server/routes/message_routes.py | 2 + .../services/message_catalogue_service.py | 92 +++++++++++++++---- .../static/js/components/messagePanel.js | 72 ++++++++++----- .../test_message_exchange_catalogue.py | 50 ++++++++++ tests/backend/test_message_routes.py | 31 +++++++ tests/browser/messageUnreadLatency.cjs | 84 +++++++++++++++++ tests/browser/messageUnreadNavigation.cjs | 16 ++-- tests/frontend/messageExchangeView.test.js | 58 ++++++------ 9 files changed, 386 insertions(+), 85 deletions(-) create mode 100644 docs/engineering/unread_message_latency_2026_09_17.md create mode 100644 tests/browser/messageUnreadLatency.cjs diff --git a/docs/engineering/unread_message_latency_2026_09_17.md b/docs/engineering/unread_message_latency_2026_09_17.md new file mode 100644 index 00000000..d36288d5 --- /dev/null +++ b/docs/engineering/unread_message_latency_2026_09_17.md @@ -0,0 +1,66 @@ +# Unread-message navigation latency evidence — 17 September 2026 + +Evidence record for native task +`#V#task_agent_43536ba9d1dd211b359a0cf4709f8707`. +The task remains the delivery/decision surface. + +Source screenshot: `#V#computer_file_copy_ce8757decd8044169fdb4ae3fd5f74d5`, +SHA-256 `3034e3c2c05f2ef86940c8277c5b351af4a45a537960675ee89c9d02a2f7b199`. +The staged original was inspected; it shows the disabled “Loading unread +messages…” action. It provides no measured duration or server trace. + +## Diagnosis and matched measurement + +At baseline `9fdc8b623`, `jumpToFirstUnread` exhausts every earlier page, +reconciling/rendering the growing message list on each response. It does this +even when an unread message is already visible, because older unread gaps may +exist. This makes client requests, transport and rendering scale with complete +history, rather than the target window. + +`tests/browser/messageUnreadLatency.cjs` exercises the production pane/CSS in +Chromium with 1,000 synthetic messages, 50 messages per page and a fixed 50 ms +HTTP response delay. Unread messages are at positions 700 and 980; the catalogue +count deliberately understates them. IntersectionObserver is disabled for this +latency comparison so read acknowledgements cannot change the target. + +| Viewport width | Baseline | Candidate | Requests before/after | Rendered messages before/after | +| --- | --- | --- | --- | --- | +| 390 px | 2,083.5 ms | 66.3 ms | 19 / 1 | 1,000 / 50 | +| 1,280 px | 2,616.1 ms | 77.4 ms | 19 / 1 | 1,000 / 50 | + +These are matched local fixture observations (final candidate rerun), not production percentiles +or measured Atlas query latency. The candidate asks the existing scoped exchange +service to locate the earliest unread, then returns a bounded chronological +window. Both reads retain actor visibility, exact participants, organisation and +deleted-message filtering. Earlier/later cursors retain history access. A window +with later pages pauses latest-page polling until the reader loads through it or +uses Latest message, avoiding disconnected ranges in the displayed transcript. +No database migration, index change, represented write or model request is used. + +## Validation and limitations + +- Backend exchange and route tests: scope/group/deletion/recipient filtering, + authenticated actor selection, unread gaps, equal-time ordering, bidirectional + pagination and empty results. +- Frontend exchange tests: one-request seek, stale counts, retry, pending + position preservation, conversation switching, later pagination and return to + latest; existing read acknowledgement and draft coverage. +- `tests/browser/messageUnreadNavigation.cjs`: desktop/phone navigation, draft + preservation, visible-read receipts and catalogue reconciliation, using real + IntersectionObserver in its read-state cases. +- `tests/browser/messageUnreadLatency.cjs`: timed click-to-focused-target with + the production frontend; the candidate asserts the fixture's one-second target. + +Local evidence is retained under `.run/unread-latency-baseline`, +`.run/unread-latency-candidate` and `.run/unread-navigation` in the task worktree. +The baseline module copy is `.run/unread-baseline/messagePanel.js`. + +There is no prepared authenticated browser profile bound to this task checkout. +The host browser README was inspected; its retained profiles belong to other +checkouts/tasks and were not reused. Private database access and live mutations +are outside this worker request. Thus the production bottleneck's exact share, +actual database query plan, and public under-one-second target remain unverified. +The observed history traversal is independently sufficient to explain delay and +its removal is a bounded improvement. A future authorised live measurement can +use a task-bound, read-only operator-prepared profile without asking Michael to +log in. No deployment is requested by this task. diff --git a/src/backend/server/routes/message_routes.py b/src/backend/server/routes/message_routes.py index 228494e0..1467ba93 100644 --- a/src/backend/server/routes/message_routes.py +++ b/src/backend/server/routes/message_routes.py @@ -1047,6 +1047,8 @@ def message_exchange(): ), limit=50, before=payload.get("before"), + after=payload.get("after"), + first_unread=payload.get("first_unread") is True, ) for message in result["messages"]: message["_id"] = str(message.get("_id", "")) diff --git a/src/backend/services/message_catalogue_service.py b/src/backend/services/message_catalogue_service.py index e2d3196a..ed633521 100644 --- a/src/backend/services/message_catalogue_service.py +++ b/src/backend/services/message_catalogue_service.py @@ -267,7 +267,16 @@ def list_exchanges( } -def get_exchange(actor, participants, *, organisation=None, limit=50, before=None): +def get_exchange( + actor, + participants, + *, + organisation=None, + limit=50, + before=None, + after=None, + first_unread=False, +): if ( not actor or not isinstance(participants, list) @@ -275,47 +284,90 @@ def get_exchange(actor, participants, *, organisation=None, limit=50, before=Non or len(participants) > 50 ): raise PermissionError("Conversation unavailable.") + if sum(bool(value) for value in (before, after, first_unread)) > 1: + raise ValueError("Choose one message navigation direction.") + limit = min(50, max(1, limit)) query = _query(actor, organisation) query["$expr"] = {"$setEquals": [participant_expression(), participants]} - if before: + collection = get_concepts_collection() + if collection is None: + raise RuntimeError("Messages unavailable.") + + def cursor(row): + return { + "created_at": row["created_at"].isoformat(), + "concept_id": row["concept_id"], + } + + anchor = before or after + if first_unread: + # Locate the earliest visible unread without transporting/rendering the + # entire exchange. Keep the exact same actor, organisation and group ACL. + unread_query = { + "$and": [ + query, + { + f"relationships.{PREDICATE_RECIPIENT}": actor, + "concept_data.read_by": {"$ne": actor}, + }, + ] + } + unread = next( + iter( + collection.find(unread_query, {"created_at": 1, "concept_id": 1}) + .sort([("created_at", 1), ("concept_id", 1)]) + .limit(1) + ), + None, + ) + if unread is None: + return { + "messages": [], + "has_more": False, + "before": None, + "after": None, + "current_user_id": actor, + } + anchor = cursor(unread) + if anchor: from .chat_history_service import _coerce_datetime - timestamp = _coerce_datetime(before.get("created_at")) - if not timestamp or not isinstance(before.get("concept_id"), str): + if not isinstance(anchor, dict): + raise ValueError("Invalid message cursor.") + timestamp = _coerce_datetime(anchor.get("created_at")) + if not timestamp or not isinstance(anchor.get("concept_id"), str): raise ValueError("Invalid message cursor.") + comparison = "$lt" if before else "$gt" query.setdefault("$and", []).append( { "$or": [ - {"created_at": {"$lt": timestamp}}, + {"created_at": {comparison: timestamp}}, { "created_at": timestamp, - "concept_id": {"$lt": before["concept_id"]}, + "concept_id": { + "$gte" if first_unread else comparison: anchor["concept_id"] + }, }, ] } ) - collection = get_concepts_collection() - if collection is None: - raise RuntimeError("Messages unavailable.") + forward = bool(after or first_unread) + direction = 1 if forward else -1 rows = list( collection.find(query) - .sort([("created_at", -1), ("concept_id", -1)]) + .sort([("created_at", direction), ("concept_id", direction)]) .limit(limit + 1) ) more = len(rows) > limit rows = rows[:limit] - cursor = ( - { - "created_at": rows[-1]["created_at"].isoformat(), - "concept_id": rows[-1]["concept_id"], - } - if more and rows - else None - ) + if not forward: + rows.reverse() return { - "messages": list(reversed(rows)), + "messages": rows, "has_more": more, - "before": cursor, + # A seek window may have earlier history; one empty page is harmless. + "before": cursor(rows[0]) if rows and (forward or more) else None, + "after": cursor(rows[-1]) if rows and forward and more else None, "current_user_id": actor, } 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 18ad0aab..02cd668c 100644 --- a/src/frontend/web/von_interface/static/js/components/messagePanel.js +++ b/src/frontend/web/von_interface/static/js/components/messagePanel.js @@ -62,6 +62,7 @@ let _deliveryKeySequence = 0; let _exchange = null; let _exchangeGeneration = 0; let _olderCursor = null; +let _newerCursor = null; let _unreadJumpGeneration = null; let _readObserver = null; const _exchangeDrafts = new Map(); @@ -113,6 +114,7 @@ export async function openMessageExchange(row) { _exchange = { ...row, browser_scope: getSessionScopedOrgId() }; resetMessagesViewportPosition(); _olderCursor = null; + _newerCursor = null; if (!_messagesContainer) initializeMessagePanel(); if (!_messagesContainer?.querySelector('#messageViewContent')) renderMessagesTabContent(); _currentUserId = row.viewer_id; @@ -162,7 +164,7 @@ export async function showMessageComposer() { } export async function refreshOpenMessageExchange() { - if (!_exchange || document.hidden || !document.getElementById('conversationWorkspace')?.classList.contains('show-message-exchange')) return; + if (!_exchange || _newerCursor || _unreadJumpGeneration !== null || document.hidden || !document.getElementById('conversationWorkspace')?.classList.contains('show-message-exchange')) return; await loadConversation(_currentConversationUserId, { silent: true }); } @@ -199,27 +201,21 @@ async function jumpToFirstUnread() { const stillCurrent = () => generation === _exchangeGeneration && org === getSessionScopedOrgId(); updateMessageNavigation(); try { - while (stillCurrent()) { - // Read markers can have gaps, and the catalogue count can be stale. - // Exhaust earlier pages before choosing the chronological first unread. - if (!_olderCursor) { - const target = content?.querySelector('.is-unread[data-contribution-id]'); - if (target) { - target.scrollIntoView({ block: 'start' }); - focusConversationTarget(target); - } else showUnreadNavigationStatus('No unread messages remain in this conversation.'); - return; - } - const cursor = _olderCursor; + if (_olderCursor || _newerCursor) { const loaded = await loadConversation(_currentConversationUserId, { - silent: true, before: cursor, observeReads: false + silent: true, firstUnread: true, observeReads: false }); if (!stillCurrent()) return; - if (!loaded || _olderCursor === cursor) { + if (!loaded) { showUnreadNavigationStatus('Could not load earlier unread messages. Try jumping again.'); return; } } + const target = content?.querySelector('.is-unread[data-contribution-id]'); + if (target) { + target.scrollIntoView({ block: 'start' }); + focusConversationTarget(target); + } else showUnreadNavigationStatus('No unread messages remain in this conversation.'); } finally { if (_unreadJumpGeneration === generation) _unreadJumpGeneration = null; if (stillCurrent()) { @@ -249,7 +245,16 @@ function updateMessageNavigation() { unread.type = 'button'; unread.className = 'message-jump-unread'; unread.onclick = () => void jumpToFirstUnread(); - const latest = createLatestMessageButton(() => { + const latest = createLatestMessageButton(async () => { + if (_isLoading) return; + const generation = _exchangeGeneration; + const org = getSessionScopedOrgId(); + const loaded = !_newerCursor || await loadConversation(_currentConversationUserId, { latest: true }); + if (generation !== _exchangeGeneration || org !== getSessionScopedOrgId()) return; + if (!loaded) { + showUnreadNavigationStatus('Could not load the latest messages. Try again.'); + return; + } content.scrollTop = content.scrollHeight; focusConversationTarget(content.querySelector('.message-list')?.lastElementChild || content); updateMessageNavigation(); @@ -264,9 +269,9 @@ function updateMessageNavigation() { unread.textContent = jumping ? 'Loading unread messages…' : 'Jump to first unread'; unread.disabled = jumping || _isLoading; unread.hidden = !jumping && !content.querySelector('.is-unread') - && !(_olderCursor && _exchange?.shared_unread_count > 0); + && !((_olderCursor || _newerCursor) && _exchange?.shared_unread_count > 0); setNavigationVisible(navigation.querySelector('.chat-scroll-to-end-btn'), - content.scrollHeight - content.scrollTop - content.clientHeight > 60); + Boolean(_newerCursor) || content.scrollHeight - content.scrollTop - content.clientHeight > 60); } function observeDisplayedMessages() { @@ -865,7 +870,7 @@ function bindMessageStreamMenu(element, getOtherUserId) { /** * Load messages for a conversation with a specific user. */ -async function loadConversation(userId, { silent = false, before = null, observeReads = true } = {}) { +async function loadConversation(userId, { silent = false, before = null, after = null, firstUnread = false, latest = false, observeReads = true } = {}) { if (_isLoading) return; _isLoading = true; const generation = _exchangeGeneration; @@ -874,14 +879,24 @@ async function loadConversation(userId, { silent = false, before = null, observe if (contentEl && !silent && !_currentMessages.length) contentEl.innerHTML = '
Loading conversation…
'; try { const response = _exchange - ? await postJson('/api/messages/exchange', { participant_ids: _exchange.participant_ids, organisation_concept_id: _exchange.organisation_concept_id || null, before }) + ? await postJson('/api/messages/exchange', { participant_ids: _exchange.participant_ids, organisation_concept_id: _exchange.organisation_concept_id || null, before, ...(after ? { after } : {}), ...(firstUnread ? { first_unread: true } : {}) }) : await getJson(`/api/messages/conversation/${encodeURIComponent(userId)}?limit=50`); if (generation !== _exchangeGeneration || org !== getSessionScopedOrgId() || userId !== _currentConversationUserId) return; const incoming = response.messages || []; + if (firstUnread && !incoming.length) { + // A successful scoped lookup supersedes stale catalogue/read markers. + _exchange.shared_unread_count = 0; + _currentMessages.forEach(message => { + const data = message.concept_data ||= {}; + data.read_by = [...new Set([...(data.read_by || []), _currentUserId])]; + }); + updateUnreadMarkers(); + return true; + } const compare = (a, b) => new Date(a.created_at) - new Date(b.created_at) || String(a.concept_id).localeCompare(String(b.concept_id)); let nextMessages = incoming; - if (before) nextMessages = [...new Map([..._currentMessages, ...incoming].map(m => [m.concept_id, m])).values()].sort(compare); - else if (silent && incoming.length) { + 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. const overlap = incoming.some(m => _currentMessages.some(old => old.concept_id === m.concept_id)); @@ -890,12 +905,16 @@ async function loadConversation(userId, { silent = false, before = null, observe } const sameContent = JSON.stringify(nextMessages.map(messageRenderKey)) === JSON.stringify(_currentMessages.map(messageRenderKey)); _currentMessages = nextMessages; - if (silent && !before && sameContent) { + if (silent && !before && !after && !firstUnread && !latest && sameContent) { updateUnreadMarkers(); if (observeReads) observeDisplayedMessages(); return; } - if (before || !silent) _olderCursor = response.before || null; + if (firstUnread || latest || !silent) { + _olderCursor = response.before || null; + _newerCursor = response.after || null; + } else if (before) _olderCursor = response.before || null; + else if (after) _newerCursor = response.after || null; _currentUserId = response.current_user_id || null; _readObserver?.disconnect(); _readObserver = null; @@ -916,6 +935,11 @@ async function loadConversation(userId, { silent = false, before = null, observe earlier.onclick = () => void loadConversation(userId, { silent: true, before: _olderCursor }); contentEl.prepend(earlier); } + if (_newerCursor && contentEl) { + const later = document.createElement('button'); later.type = 'button'; later.textContent = 'Load later messages'; + later.onclick = () => void loadConversation(userId, { silent: true, after: _newerCursor }); + contentEl.append(later); + } updateUnreadMarkers(); if (contentEl) { const restoredAnchor = [...contentEl.querySelectorAll('[data-contribution-id]')] diff --git a/tests/backend/test_message_exchange_catalogue.py b/tests/backend/test_message_exchange_catalogue.py index 92c57986..6df6d233 100644 --- a/tests/backend/test_message_exchange_catalogue.py +++ b/tests/backend/test_message_exchange_catalogue.py @@ -166,3 +166,53 @@ def test_reference_denies_other_actor_group_and_organisation( actor, participants, organisation=org ) assert not reference_store[1] + + +def test_unread_seek_skips_read_history_and_pages_both_directions(collection): + rows = [message(i) for i in range(160)] + for row in rows: + row['concept_data']['read_by'] = ['#V#alice'] + # Read gaps and a stale catalogue count must not change the first target. + for i in [70, 140]: + rows[i]['concept_data']['read_by'] = [] + collection.insert_many(rows) + args = ('#V#alice', ['#V#alice', '#V#bob']) + window = catalogue.get_exchange(*args, organisation='#V#lab', first_unread=True) + assert [m['concept_id'] for m in window['messages']] == [ + f'#V#message_{i:03}' for i in range(70, 120) + ] + later = catalogue.get_exchange(*args, organisation='#V#lab', after=window['after']) + assert [m['concept_id'] for m in later['messages']] == [ + f'#V#message_{i:03}' for i in range(120, 160) + ] + assert later['after'] is None + earlier = catalogue.get_exchange(*args, organisation='#V#lab', before=window['before']) + assert [m['concept_id'] for m in earlier['messages']] == [ + f'#V#message_{i:03}' for i in range(20, 70) + ] + + +def test_unread_seek_preserves_scope_recipients_deletion_and_equal_time(collection, monkeypatch): + rows = [message(i) for i in range(8)] + rows[0]['concept_data']['read_by'] = ['#V#alice'] + rows[1]['relationships']['#V#has_recipient'].append('#V#carol') + rows[2]['concept_data']['organisation_concept_id'] = '#V#other' + rows[3]['concept_data']['deleted'] = True + rows[4]['relationships']['#V#has_sender'] = ['#V#alice'] + rows[4]['relationships']['#V#has_recipient'] = ['#V#bob'] + rows[5]['hidden'] = True + rows[7]['created_at'] = rows[6]['created_at'] + monkeypatch.setattr(catalogue, 'apply_concept_query_filter', + lambda query: {**query, 'hidden': {'$ne': True}}) + collection.insert_many(rows) + args = ('#V#alice', ['#V#alice', '#V#bob']) + window = catalogue.get_exchange(*args, organisation='#V#lab', first_unread=True, limit=1) + assert window['messages'][0]['concept_id'] == '#V#message_006' + later = catalogue.get_exchange(*args, organisation='#V#lab', after=window['after']) + assert [m['concept_id'] for m in later['messages']] == ['#V#message_007'] + collection.update_many({}, {'$set': {'concept_data.read_by': ['#V#alice']}}) + assert catalogue.get_exchange(*args, organisation='#V#lab', first_unread=True)['messages'] == [] + with pytest.raises(PermissionError): + catalogue.get_exchange('#V#eve', args[1], first_unread=True) + with pytest.raises(ValueError): + catalogue.get_exchange(*args, first_unread=True, after=window['after']) diff --git a/tests/backend/test_message_routes.py b/tests/backend/test_message_routes.py index b01833bb..7f2d78a9 100644 --- a/tests/backend/test_message_routes.py +++ b/tests/backend/test_message_routes.py @@ -683,3 +683,34 @@ def denied(*args, **kwargs): ) assert response.status_code == 403 assert "concept_id" not in response.json + + +def test_exchange_unread_seek_and_forward_cursor_use_authenticated_actor(monkeypatch, app_client): + from src.backend.services import message_catalogue_service + + _, client = app_client + _authorise_test_direct_message(monkeypatch) + calls = [] + + def exchange(actor, participants, **kwargs): + calls.append((actor, participants, kwargs)) + return {'messages': [], 'current_user_id': actor, 'before': None, 'after': None} + + monkeypatch.setattr(message_catalogue_service, 'get_exchange', exchange) + participants = ['#V#user_alice', '#V#user_bob'] + cursor = {'created_at': '2026-09-01T12:00:00Z', 'concept_id': '#V#message'} + for navigation in [{'first_unread': True}, {'after': cursor}]: + response = client.post('/api/messages/exchange', json={ + 'actor': '#V#spoofed', 'participant_ids': participants, **navigation, + }) + assert response.status_code == 200 + actor, actual_participants, kwargs = calls[-1] + assert actor == '#V#user_alice' + assert actual_participants == participants + assert kwargs['organisation'] == '#V#org_test' + assert kwargs['limit'] == 50 + for key, value in navigation.items(): + assert kwargs[key] == value + monkeypatch.setattr(message_routes, '_get_current_user_concept_id', lambda: None) + assert client.post('/api/messages/exchange', json={'first_unread': True}).status_code == 401 + assert len(calls) == 2 diff --git a/tests/browser/messageUnreadLatency.cjs b/tests/browser/messageUnreadLatency.cjs new file mode 100644 index 00000000..d4b20c2d --- /dev/null +++ b/tests/browser/messageUnreadLatency.cjs @@ -0,0 +1,84 @@ +// Production pane with a deterministic 1,000-message HTTP fixture. No live data. +// Optional baseline module path lets the same workload measure an older revision. +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const http = require('node:http'); +const path = require('node:path'); +const { chromium } = require('@playwright/test'); +const root = path.resolve(__dirname, '../../src/frontend/web/von_interface'); +const evidence = process.argv[2] || '.run/unread-latency'; +const baseline = process.argv[3]; +fs.mkdirSync(evidence, { recursive: true }); +const markup = fs.readFileSync(path.join(root, 'templates/chat_tab.html'), 'utf8').replace(/{%[\s\S]*?%}|{{[\s\S]*?}}/g, ''); +const messages = Array.from({ length: 1000 }, (_, i) => ({ + concept_id: `message-${String(i).padStart(4, '0')}`, + created_at: new Date(Date.UTC(2026, 8, 1) + i * 1000).toISOString(), + relationships: { '#V#has_sender': ['#V#bob'], '#V#has_recipient': ['#V#alice'] }, + concept_data: { content_fallback: `Message ${i}. ${'Research progress and follow-up. '.repeat(8)}`, read_by: [700, 980].includes(i) ? [] : ['#V#alice'] } +})); +let requests = 0; +const server = http.createServer(async (req, res) => { + if (req.url === '/') { + res.setHeader('Content-Type', 'text/html'); + return res.end(`${markup}`); + } + if (req.url.startsWith('/static/')) { + const file = path.resolve(root, `.${req.url}`); + if (file.startsWith(root + '/static/') && fs.existsSync(file)) { + res.setHeader('Content-Type', file.endsWith('.css') ? 'text/css' : 'text/javascript'); + return res.end(fs.readFileSync(baseline && req.url.endsWith('/components/messagePanel.js') ? baseline : file)); + } + } + res.setHeader('Content-Type', 'application/json'); + if (req.url === '/api/messages/exchange') { + let body = ''; for await (const chunk of req) body += chunk; + const { before, after, first_unread } = JSON.parse(body); + requests++; + const start = first_unread ? 700 : after ? Number(after) : Math.max(0, (before ? Number(before) : 1000) - 50); + const end = Math.min(start + 50, 1000); + await new Promise(resolve => setTimeout(resolve, 50)); + return res.end(JSON.stringify({ messages: messages.slice(start, end), before: start || null, + after: (first_unread || after) && end < 1000 ? end : null, current_user_id: '#V#alice' })); + } + res.end(JSON.stringify({ success: true, profiles: [], messages: [] })); +}); +(async () => { + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const browser = await chromium.launch(); + try { + const results = []; + for (const width of [390, 1280]) { + const page = await browser.newPage({ viewport: { width, height: 850 } }); + const errors = []; page.on('pageerror', error => errors.push(error.message)); + await page.goto(`http://127.0.0.1:${server.address().port}`); + await page.evaluate(async () => { + window.IntersectionObserver = undefined; + document.getElementById('conversationWorkspace').classList.add('show-message-exchange'); + document.body.classList.add('viewing-message-exchange'); + const panel = await import('/static/js/components/messagePanel.js'); + await panel.openMessageExchange({ session_id: 'latency', viewer_id: '#V#alice', + participant_ids: ['#V#alice', '#V#bob'], other_participant_ids: ['#V#bob'], + shared_unread_count: 1 }); // Deliberately stale; earlier unread must win. + }); + requests = 0; + const ms = await page.evaluate(async () => { + const started = performance.now(); + document.querySelector('.message-jump-unread').click(); + while (document.activeElement?.dataset.contributionId !== 'message-0700') { + if (performance.now() - started > 60000) throw new Error('Unread navigation did not finish'); + await new Promise(resolve => requestAnimationFrame(resolve)); + } + return performance.now() - started; + }); + const rendered = await page.locator('[data-contribution-id]').count(); + assert.deepEqual(errors, []); + if (!baseline) { assert.equal(requests, 1); assert.equal(rendered, 50); assert.ok(ms < 1000, `${ms}ms exceeds fixture target`); } + results.push({ width, milliseconds: ms, exchangeRequests: requests, renderedMessages: rendered }); + await page.screenshot({ path: path.join(evidence, `unread-${width}.png`) }); + await page.close(); + } + const result = { fixture: true, messageCount: 1000, responseDelayMs: 50, baseline: Boolean(baseline), results }; + fs.writeFileSync(path.join(evidence, 'result.json'), JSON.stringify(result, null, 2)); + console.log(JSON.stringify(result)); + } finally { await browser.close(); server.close(); } +})().catch(error => { console.error(error); server.close(); process.exitCode = 1; }); diff --git a/tests/browser/messageUnreadNavigation.cjs b/tests/browser/messageUnreadNavigation.cjs index 0545ded7..7322fcd4 100644 --- a/tests/browser/messageUnreadNavigation.cjs +++ b/tests/browser/messageUnreadNavigation.cjs @@ -55,18 +55,18 @@ const server = http.createServer(async (req, res) => { } if (req.url === '/api/messages/exchange') { let body = ''; for await (const chunk of req) body += chunk; - const { before, participant_ids } = JSON.parse(body); cursors.push(before); + const { before, first_unread, participant_ids } = JSON.parse(body); cursors.push(first_unread ? "first_unread" : before); if (zeroTransition) { const id = participant_ids.includes('#V#carol') ? 'carol-final' : 'bob-final'; return res.end(JSON.stringify({ current_user_id: '#V#alice', before: 'read-history', messages: [message(id, !readMessages.has(id), '15')] })); } const initial = Array.from({ length: 20 }, (_, index) => message(`latest-${String(index).padStart(2, '0')}`, false, '13')); - const result = !before ? { messages: background ? [...initial, message('Background unread', true, '14')] : initial, before: 'middle' } + const result = first_unread ? { messages: [message('Earlier unread', true, '10'), message('Most recent unread', true, '11')], before: 'history', after: 'later' } : !before ? { messages: background ? [...initial, message('Background unread', true, '14')] : initial, before: 'middle' } : before === 'middle' ? { messages: Array.from({ length: 10 }, (_, index) => message(`middle-${index}`, index === 5, '12')), before: 'older' } : { messages: [message('Earlier unread', true, '10'), message('Most recent unread', true, '11')], before: null }; const finish = () => res.end(JSON.stringify({ ...result, current_user_id: '#V#alice' })); - if (before || background) pendingPage = finish; + if (first_unread || before || background) pendingPage = finish; else finish(); return; } @@ -125,7 +125,7 @@ const server = http.createServer(async (req, res) => { pendingPage(); pendingPage = null; await page.evaluate(() => window.refreshDone); assert.equal(await content.evaluate(el => el.scrollTop), bottomPosition, 'background unread must not follow the bottom'); - // Reopen the initial fixture, then traverse delayed earlier pages. + // Reopen the initial fixture, then seek through a delayed unread response. background = false; await page.evaluate(async () => { const panel = await import('/static/js/components/messagePanel.js'); @@ -145,24 +145,20 @@ const server = http.createServer(async (req, res) => { await page.getByRole('button', { name: 'Jump to first unread', exact: true }).click(); await expect.poll(() => Boolean(pendingPage)).toBe(true); assert.equal(await offset(), anchor.offset, 'pending go-to-unread preserves the visible message'); - pendingPage(); pendingPage = null; - await expect.poll(() => Boolean(pendingPage)).toBe(true); - await page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve)))); - assert.ok(Math.abs(await offset() - anchor.offset) <= 1, 'intermediate page preserves the visible message within one CSS pixel'); await expect(page.getByRole('button', { name: 'Loading unread messages…', exact: true })).toBeVisible(); await page.screenshot({ path: path.join(evidence, `loading-${width}.png`) }); pendingPage(); pendingPage = null; const target = page.locator('[data-contribution-id="Earlier unread"]'); await expect(target).toBeFocused(); await expect(target).toBeInViewport(); - assert.deepEqual(cursors, ['middle', 'older']); + assert.deepEqual(cursors, ['first_unread']); await page.screenshot({ path: path.join(evidence, `unread-${width}.png`) }); 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, intermediateAnchorOffset: anchor.offset, firstUnreadFocused: true, targetInViewport: true }); + 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 }); 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 49b43b78..51c5839e 100644 --- a/tests/frontend/messageExchangeView.test.js +++ b/tests/frontend/messageExchangeView.test.js @@ -225,21 +225,20 @@ function page(ids, before = null, unread = [], firstDay = 1) { })) }; } -test('unloaded unread count exposes navigation that pages past loaded unread messages to the first unread', async () => { +test('unloaded unread count uses one seek request regardless of history length', async () => { HTMLElement.prototype.scrollIntoView = jest.fn(); const api = require(base + 'apiService.js'); api.postJson.mockResolvedValueOnce(page(['latest'], 'page2', [], 15)) - .mockResolvedValueOnce(page(['middle'], 'page3', [], 12)) - .mockResolvedValueOnce(page(['old-unread', 'recent-unread'], 'page4', ['old-unread', 'recent-unread'], 9)) - .mockResolvedValueOnce(page(['first-unread'], null, ['first-unread'])); + .mockResolvedValueOnce({ ...page(['first-unread'], 'earlier', ['first-unread']), after: 'later' }); await require(base + 'components/messagePanel.js').openMessageExchange({ ...row('#V#bob'), shared_unread_count: 2 }); const jump = document.querySelector('.message-jump-unread'); expect(jump.hidden).toBe(false); expect(jump.textContent).toBe('Jump to first unread'); jump.click(); await flush(); expect(document.activeElement.dataset.contributionId).toBe('first-unread'); - expect(api.postJson.mock.calls.map(([, body]) => body.before)).toEqual([null, 'page2', 'page3', 'page4']); - expect(document.querySelectorAll('.is-unread')).toHaveLength(3); + expect(api.postJson).toHaveBeenCalledTimes(2); + expect(api.postJson.mock.calls[1][1].first_unread).toBe(true); + expect(document.querySelectorAll('.is-unread')).toHaveLength(1); expect(jump.disabled).toBe(false); }); @@ -249,16 +248,15 @@ test('already loaded unread does not skip an older unread separated by a read pa const api = require(base + 'apiService.js'); const latest = page(['recent-unread'], 'read-gap', ['recent-unread'], 15); api.postJson.mockResolvedValueOnce(latest) - .mockResolvedValueOnce(page(['read-gap'], 'oldest', [], 10)) .mockResolvedValueOnce(page(['first-unread'], null, ['first-unread'])); // A stale low count must not cause an early stop. await require(base + 'components/messagePanel.js').openMessageExchange({ ...row('#V#bob'), shared_unread_count: 1 }); document.querySelector('.message-jump-unread').click(); await flush(); expect(document.activeElement.dataset.contributionId).toBe('first-unread'); - expect(api.postJson.mock.calls.map(([, body]) => body.before)).toEqual([null, 'read-gap', 'oldest']); + expect(api.postJson.mock.calls[1][1].first_unread).toBe(true); expect(observers[0].disconnect).toHaveBeenCalled(); expect(observers).toHaveLength(2); // Re-arm only after the final jump. - expect(document.querySelectorAll('.is-unread')).toHaveLength(2); + expect(document.querySelectorAll('.is-unread')).toHaveLength(1); }); test('a fully loaded conversation focuses its first unread without a request', async () => { @@ -302,9 +300,9 @@ test('switching conversation cancels a pending unread jump without moving focus' expect(document.activeElement.dataset.contributionId).toBeUndefined(); }); -test('exhausting earlier pages reports stale unread counts and hides the action', async () => { +test('empty unread lookup reports stale counts without removing loaded history', async () => { const api = require(base + 'apiService.js'); - api.postJson.mockResolvedValueOnce(page(['latest'], 'older')).mockResolvedValueOnce(page(['old'])); + api.postJson.mockResolvedValueOnce(page(['latest'], 'older')).mockResolvedValueOnce(page([])); 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('.message-unread-navigation-status').textContent).toContain('No unread messages remain'); @@ -350,36 +348,34 @@ test('background unread loading preserves even a near-bottom reader and scrollin expect(HTMLElement.prototype.scrollIntoView).not.toHaveBeenCalled(); }); -test('deferred unread navigation preserves the visible contribution across intermediate pages before jumping', async () => { +test('unread seek preserves position while pending and retains both pagination directions', async () => { HTMLElement.prototype.scrollIntoView = jest.fn(); const api = require(base + 'apiService.js'); api.postJson.mockResolvedValueOnce(page(['latest'], 'page2')); - await require(base + 'components/messagePanel.js').openMessageExchange({ ...row('#V#bob'), shared_unread_count: 1 }); + const panel = require(base + 'components/messagePanel.js'); + await panel.openMessageExchange({ ...row('#V#bob'), shared_unread_count: 1 }); const content = document.getElementById('messageViewContent'); content.scrollTop = 25; - const geometry = jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function () { - if (this.dataset.contributionId === 'latest') { - const top = 100 + (content.querySelector('[data-contribution-id="middle"]') ? 200 : 0) - content.scrollTop; - return { top, bottom: top + 100 }; - } - return { top: 0, bottom: 0 }; - }); - let finishMiddle, finishUnread; - api.postJson.mockImplementationOnce(() => new Promise(resolve => { finishMiddle = resolve; })) - .mockImplementationOnce(() => new Promise(resolve => { finishUnread = resolve; })); + let finish; + api.postJson.mockImplementationOnce(() => new Promise(resolve => { finish = resolve; })); const jump = document.querySelector('.message-jump-unread'); jump.click(); expect(jump.textContent).toBe('Loading unread messages…'); expect(content.scrollTop).toBe(25); - content.scrollTop = 40; - finishMiddle(page(['middle'], 'page3')); await flush(); - expect(content.scrollTop).toBe(240); - expect(content.querySelector('[data-contribution-id="latest"]').getBoundingClientRect().top).toBe(60); - expect(HTMLElement.prototype.scrollIntoView).not.toHaveBeenCalled(); - finishUnread(page(['unread'], null, ['unread'])); await flush(); - expect(HTMLElement.prototype.scrollIntoView).toHaveBeenCalledTimes(1); + expect(content.querySelector('[data-contribution-id="latest"]')).not.toBeNull(); + finish({ ...page(['unread'], 'earlier', ['unread']), after: 'later' }); await flush(); expect(document.activeElement.dataset.contributionId).toBe('unread'); - geometry.mockRestore(); + const count = api.postJson.mock.calls.length; + await panel.refreshOpenMessageExchange(); + expect(api.postJson).toHaveBeenCalledTimes(count); // No disconnected latest page from polling. + api.postJson.mockResolvedValueOnce({ ...page(['next'], null), after: 'more' }); + [...content.querySelectorAll('button')].find(el => el.textContent === 'Load later messages').click(); await flush(); + expect(api.postJson.mock.calls.at(-1)[1].after).toBe('later'); + expect(content.querySelector('[data-contribution-id="unread"]')).not.toBeNull(); + api.postJson.mockResolvedValueOnce(page(['latest'], 'previous')); + document.querySelector('.chat-scroll-to-end-btn').click(); await flush(); + expect(content.querySelector('[data-contribution-id="latest"]')).not.toBeNull(); + expect(content.querySelector('[data-contribution-id="unread"]')).toBeNull(); }); test('confirmed reads hide navigation at zero even with older history and ignore stale catalogue replies', async () => {