From 202bafea28b2bb7e65f8e3147666faa2cc3cc609 Mon Sep 17 00:00:00 2001 From: n30nex Date: Thu, 29 Oct 2026 13:34:12 -0400 Subject: [PATCH] fix(observers): show metadata while analytics loads (#2339) Partial follow-up to #1778, limited to observer details; other pages remain separate. Observer identity, status and radio information now render when the basic request finishes. Analytics and clock calibration load into their own regions, so either can finish first. An analytics failure preserves usable metadata. Identity failures and obsolete responses cannot overwrite the current view, and failed chart construction is cleaned up. The same three requests start together. Generation ownership, request deduplication, date-window behavior and disposal remain covered. No new fetches, settings, dependencies or customizer options; secondary replies preserve the actual metadata DOM node. [RED CI](https://github.com/Kpa-clawbot/CoreScope/actions/runs/40426343054/job/121932999182) proves 14 new assertion failures while all 11 existing unit cases pass. Local RED browsers reproduce 20 new failures while all 32 old cases pass. GREEN `52eabad7` passes 221 standalone suites, 28 ownership unit checks and 56 browser cases across desktop/mobile and both themes; the parent independently repeated the 56 browser cases. All three independent review roles passed. Held-response assertions prove the rendering behavior without claiming a server-latency speedup. [Final CI](https://github.com/Kpa-clawbot/CoreScope/actions/runs/40427087948) passes on this exact head, including all three browser shards, the repeated slide-over stability gate and the architecture build. --------- Co-authored-by: n30nex <6686206+n30nex@users.noreply.github.com> --- public/observer-detail.js | 67 ++++++-- .../e2e/test-observer-detail-ownership-e2e.js | 150 +++++++++++++++--- tests/unit/test-observer-detail-ownership.js | 121 +++++++++++++- 3 files changed, 293 insertions(+), 45 deletions(-) diff --git a/public/observer-detail.js b/public/observer-detail.js index 6d2d9c7f..5c6e9435 100644 --- a/public/observer-detail.js +++ b/public/observer-detail.js @@ -181,20 +181,42 @@ window.ObserverDetailNaiveBanner = { const days = currentDays; try { destroyCharts(); - chartDefaults(); - const [obs, analytics, obsSkewArr] = await Promise.all([ - api('/observers/' + encodeURIComponent(observerId)), - api('/observers/' + encodeURIComponent(observerId) + '/analytics?days=' + days), - api('/observers/clock-skew', { ttl: 30000 }).catch(function() { return []; }), - ]); + // Start all requests together, but only identity gates the first render. + // Capture analytics errors immediately, even if identity is still pending. + const observerRequest = api('/observers/' + encodeURIComponent(observerId)); + const analyticsRequest = api('/observers/' + encodeURIComponent(observerId) + '/analytics?days=' + days) + .then(data => ({ data }), error => ({ error })); + const skewRequest = api('/observers/clock-skew', { ttl: 30000 }).catch(function() { return []; }); + const obs = await observerRequest; // Only the latest load of the current visit owns the detail view. if (generation !== detailGeneration) return; - // Find this observer's calibration data. - var obsSkew = null; - (Array.isArray(obsSkewArr) ? obsSkewArr : []).forEach(function(s) { - if (s && s.observerID === observerId) obsSkew = s; + renderDetail(obs); + analyticsRequest.then(result => { + if (generation !== detailGeneration) return; + const el = document.getElementById('obsAnalytics'); + if (!el) return; + try { + if (result.error) throw result.error; + renderAnalytics(result.data); + } catch (error) { + destroyCharts(); + el.innerHTML = '
'; + el.firstChild.textContent = 'Error: ' + error.message; + } + el.setAttribute('aria-busy', 'false'); + }); + skewRequest.then(obsSkewArr => { + if (generation !== detailGeneration) return; + // Preserve the last matching calibration from the bulk response. + var obsSkew = null; + (Array.isArray(obsSkewArr) ? obsSkewArr : []).forEach(function(s) { + if (s && s.observerID === observerId) obsSkew = s; + }); + renderClockOffset(obsSkew); + }).catch(() => { + if (generation !== detailGeneration) return; + renderClockOffset(null); }); - renderDetail(obs, analytics, obsSkew); } catch (e) { if (generation !== detailGeneration) return; // SECURITY (OBS-2, PR #1539): use textContent for error messages. @@ -209,7 +231,7 @@ window.ObserverDetailNaiveBanner = { } } - function renderDetail(obs, analytics, obsSkew) { + function renderDetail(obs) { const el = document.getElementById('obsDetailContent'); if (!el) return; @@ -317,7 +339,15 @@ window.ObserverDetailNaiveBanner = {
ID: ${escapeHtml(obs.id)}
- ${obsSkew && obsSkew.samples > 0 ? ` +
Loading clock calibration…
+ ${window.ObserverDetailRadioHistory ? window.ObserverDetailRadioHistory.render(obs.radio_history) : ''} +
Loading analytics…
`; + } + + function renderClockOffset(obsSkew) { + const el = document.getElementById('obsClockOffset'); + if (!el) return; + el.innerHTML = obsSkew && obsSkew.samples > 0 ? `

Clock Offset

@@ -328,8 +358,15 @@ window.ObserverDetailNaiveBanner = {
How this is computed: when this observer and another observer see the same packet, we compare their receive timestamps. The median deviation across all multi-observer packets is this observer's offset.
-
` : ''} - ${window.ObserverDetailRadioHistory ? window.ObserverDetailRadioHistory.render(obs.radio_history) : ''} +
` : ''; + el.setAttribute('aria-busy', 'false'); + } + + function renderAnalytics(analytics) { + const el = document.getElementById('obsAnalytics'); + if (!el) return; + chartDefaults(); + el.innerHTML = `

Packets Over Time

diff --git a/tests/e2e/test-observer-detail-ownership-e2e.js b/tests/e2e/test-observer-detail-ownership-e2e.js index 5f939c66..c76113e1 100644 --- a/tests/e2e/test-observer-detail-ownership-e2e.js +++ b/tests/e2e/test-observer-detail-ownership-e2e.js @@ -10,9 +10,9 @@ const IDS = { A: 'AA'.repeat(32), B: 'BB'.repeat(32) }; const ARTIFACTS = process.env.ARTIFACT_DIR; if (ARTIFACTS) fs.mkdirSync(ARTIFACTS, { recursive: true }); const cases = []; -function test(name, run) { cases.push({ name, run }); } +function test(name, run, options = {}) { cases.push({ name, run, options }); } -async function setup(browser, viewport, theme) { +async function setup(browser, viewport, theme, options) { const context = await browser.newContext({ viewport, colorScheme: theme }); const page = await context.newPage(); page.setDefaultTimeout(10000); @@ -22,9 +22,11 @@ async function setup(browser, viewport, theme) { const ready = new Map(); const visits = { A: 0, B: 0 }; const network = []; + let holdClock = false; function observer(letter, visit = 0) { return { id: IDS[letter], name: 'Observer ' + letter + (visit ? ' visit ' + visit : ''), - last_seen: letter === 'B' ? new Date().toISOString() : null, packet_count: 5 }; + last_seen: letter === 'B' ? new Date().toISOString() : null, packet_count: 5, + model: 'Test radio', radio: '910.0,250,10,5' }; } await page.route('**/api/observers**', async route => { const url = new URL(route.request().url()); @@ -34,6 +36,11 @@ async function setup(browser, viewport, theme) { return route.fulfill({ json: { observers: ['A', 'B'].map(letter => observer(letter)), server_time: new Date().toISOString() } }); } if (url.pathname === '/api/observers/clock-skew') { + if (holdClock) { + pending.set('clock', route); + if (ready.has('clock')) ready.get('clock')(); + return; + } return route.fulfill({ json: [ { observerID: IDS.A, offsetSec: 1, samples: 11 }, { observerID: IDS.B, offsetSec: 2, samples: 22 }, @@ -42,6 +49,7 @@ async function setup(browser, viewport, theme) { } for (const letter of ['A', 'B']) { if (url.pathname === '/api/observers/' + IDS[letter]) { + if (options.identityFailure) return route.fulfill({ status: 404, json: { error: 'fixture identity failure' } }); return route.fulfill({ json: observer(letter, ++visits[letter]) }); } if (url.pathname === '/api/observers/' + IDS[letter] + '/analytics') { @@ -55,6 +63,10 @@ async function setup(browser, viewport, theme) { }); await page.goto(BASE + '/?theme=' + theme + '#/observers', { waitUntil: 'domcontentloaded' }); await page.waitForSelector('#obsTable tbody tr[data-value]'); + // The list also requests calibration. Hold only the detail request, after + // clearing that completed list cache entry (preserve real in-flight dedup). + holdClock = !!options.holdClock; + if (holdClock) await page.evaluate(() => invalidateApiCache('/observers/clock-skew')); // Observe the real api() promises (including its in-flight deduplication), // so assertions wait for loadDetail's continuations, never a guessed sleep. await page.evaluate(() => { @@ -67,43 +79,63 @@ async function setup(browser, viewport, theme) { }; }); async function open(letter) { - if (await page.locator('#obsTitle').count()) { - await page.locator('a[title="Back to Observers"]').click(); - } - await page.locator('#obsTable tbody tr[data-value="#/observers/' + IDS[letter] + '"]').click(); - if (viewport.width < 768) { - await page.locator('.slide-over-panel a').filter({ hasText: 'Open full detail' }).click(); - await page.keyboard.press('Escape'); - await page.locator('.slide-over-panel[data-state="open"]').waitFor({ state: 'hidden' }); + if (holdClock && await page.locator('#obsTitle').count()) { + // Navigate through the real router without making the list wait for + // this deliberately held, shared calibration request. + await page.evaluate(id => { location.hash = '#/observers/' + id; }, IDS[letter]); + } else { + if (await page.locator('#obsTitle').count()) { + await page.locator('a[title="Back to Observers"]').click(); + } + await page.locator('#obsTable tbody tr[data-value="#/observers/' + IDS[letter] + '"]').click(); + if (viewport.width < 768) { + await page.locator('.slide-over-panel a').filter({ hasText: 'Open full detail' }).click(); + await page.keyboard.press('Escape'); + await page.locator('.slide-over-panel[data-state="open"]').waitFor({ state: 'hidden' }); + } } await page.waitForSelector('#obsDaysSelect'); await page.waitForFunction(id => window.__observerRequests.some(r => r.path.startsWith('/observers/' + id + '/analytics')), IDS[letter]); await page.evaluate(id => Promise.allSettled(window.__observerRequests.filter(r => r.path === '/observers/' + id).map(r => r.promise)), IDS[letter]); } - async function release(letter, days, status = 200) { - const key = letter + ':' + days; + async function held(key) { // Wait until this exact route is intercepted, without a timing assumption. if (!pending.has(key)) { let timeout; try { await new Promise((resolve, reject) => { ready.set(key, resolve); - timeout = setTimeout(() => reject(new Error('Missing held analytics request: ' + key)), 10000); + timeout = setTimeout(() => reject(new Error('Missing held request: ' + key)), 10000); }); } finally { clearTimeout(timeout); ready.delete(key); } } const route = pending.get(key); - assert.ok(route, 'analytics request is held: ' + key); + assert.ok(route, 'request is held: ' + key); pending.delete(key); + return route; + } + async function settled(path) { + await page.evaluate(async path => { + await Promise.allSettled(window.__observerRequests.filter(r => r.path === path).map(r => r.promise)); + await new Promise(requestAnimationFrame); + }, path); + } + async function releaseClock(status = 200) { + const route = await held('clock'); + await route.fulfill({ status, json: status === 200 ? [ + { observerID: IDS.A, offsetSec: 3, samples: 33 }, + { observerID: IDS.B, offsetSec: 2, samples: 22 }, + ] : { error: 'fixture calibration failure' } }); + await settled('/observers/clock-skew'); + } + async function release(letter, days, status = 200) { + const route = await held(letter + ':' + days); const label = letter + ' ' + days + ' days'; await route.fulfill({ status, json: status === 200 ? { timeline: [{ label, count: days }], packetTypes: { 4: days }, nodesTimeline: [{ label, count: 1 }], snrDistribution: [{ range: '0 to 5', count: days }], recentPackets: [], } : { error: 'fixture failure' } }); - await page.evaluate(async suffix => { - await Promise.allSettled(window.__observerRequests.filter(r => r.path.endsWith(suffix) || !r.path.includes('/analytics?')).map(r => r.promise)); - await new Promise(requestAnimationFrame); - }, '/observers/' + IDS[letter] + '/analytics?days=' + days); + await settled('/observers/' + IDS[letter] + '/analytics?days=' + days); } async function snapshot() { return page.evaluate(() => ({ @@ -122,9 +154,82 @@ async function setup(browser, viewport, theme) { assert.match(state.content, letter === 'B' ? /22 samples/ : /33 samples/); assert.strictEqual(await page.locator('#obsDaysSelect').inputValue(), String(days)); } - return { page, context, errors, open, release, snapshot, current, network }; + return { page, context, errors, open, release, releaseClock, snapshot, current, network }; } +for (const first of ['analytics', 'clock']) test('progressive-' + first + '-first', async h => { + await h.open('B'); + assert.strictEqual(await h.page.locator('#obsTitle').textContent(), 'Observer B visit 1', + 'identity must be visible with both secondary responses held'); + const info = h.page.locator('.obs-info-grid'); + assert.match(await info.innerText(), /Online/); + assert.match(await info.innerText(), /Test radio/); + assert.match(await info.innerText(), /910.0 MHz/); + assert.strictEqual((await h.snapshot()).charts.length, 0); + assert.strictEqual(await h.page.locator('#obsAnalytics').getAttribute('aria-busy'), 'true'); + await h.page.evaluate(() => { window.__observerMetadata = document.querySelector('.obs-info-grid'); }); + if (ARTIFACTS) await h.page.screenshot({ path: path.join(ARTIFACTS, h.label + '-pending.png'), fullPage: true }); + if (first === 'analytics') { + await h.release('B', 7); + assert.strictEqual((await h.snapshot()).charts.length, 4, 'analytics does not wait for calibration'); + await h.releaseClock(); + } else { + await h.releaseClock(); + assert.match((await h.snapshot()).content, /22 samples/, 'calibration does not wait for analytics'); + assert.strictEqual((await h.snapshot()).charts.length, 0); + await h.release('B', 7); + } + await h.current('B', 7); + assert.strictEqual(await h.page.locator('#obsAnalytics').getAttribute('aria-busy'), 'false'); + assert.strictEqual(await h.page.evaluate(() => window.__observerMetadata === document.querySelector('.obs-info-grid')), true, + 'secondary replies preserve the actual metadata DOM node'); + assert.strictEqual(h.network.filter(url => url.includes('/analytics?')).length, 1); + assert.strictEqual(h.network.filter(url => url === '/api/observers/clock-skew').length, 2, + 'one list calibration request plus one detail request'); + if (ARTIFACTS) await h.page.screenshot({ path: path.join(ARTIFACTS, h.label + '-complete.png'), fullPage: true }); +}, { holdClock: true }); + +test('analytics-error-keeps-identity', async h => { + await h.open('B'); + await h.release('B', 7, 500); + assert.strictEqual(await h.page.locator('#obsTitle').textContent(), 'Observer B visit 1'); + assert.match(await h.page.locator('.obs-info-grid').innerText(), /Online/); + assert.match(await h.page.locator('#obsAnalytics').innerText(), /Error: API 500:/); + await h.releaseClock(); + assert.match((await h.snapshot()).content, /22 samples/); + assert.strictEqual((await h.snapshot()).charts.length, 0); +}, { holdClock: true }); + +test('calibration-error-keeps-identity', async h => { + await h.open('B'); + await h.releaseClock(500); + assert.match(await h.page.locator('.obs-info-grid').innerText(), /Test radio/); + await h.release('B', 7); + assert.strictEqual((await h.snapshot()).charts.length, 4); + assert.strictEqual(await h.page.locator('.skew-detail-section').count(), 0); +}, { holdClock: true }); + +test('identity-error-with-late-secondary-results', async h => { + await h.open('B'); + const before = await h.snapshot(); + assert.match(before.content, /Error: API 404:/); + await h.release('B', 7); + await h.releaseClock(); + assert.deepStrictEqual(await h.snapshot(), before, 'secondary success cannot hide a failed identity'); +}, { holdClock: true, identityFailure: true }); + +test('early-identity-route-with-held-calibration', async h => { + await h.open('A'); + assert.strictEqual(await h.page.locator('#obsTitle').textContent(), 'Observer A visit 1'); + await h.open('B'); + await h.release('B', 7); + await h.releaseClock(); + await h.current('B', 7); + const before = await h.snapshot(); + await h.release('A', 7); + assert.deepStrictEqual(await h.snapshot(), before, 'old observer cannot mutate independently completed sections'); +}, { holdClock: true }); + test('late-success', async h => { await h.open('A'); await h.open('B'); @@ -229,9 +334,10 @@ test('dispose-and-controls', async h => { try { for (const [device, viewport] of Object.entries({ desktop: { width: 1400, height: 900 }, mobile: { width: 390, height: 844 } })) { for (const theme of ['light', 'dark']) { - for (const { name, run } of cases) { + for (const { name, run, options } of cases) { const label = device + '-' + theme + '-' + name; - const h = await setup(browser, viewport, theme); + const h = await setup(browser, viewport, theme, options); + h.label = label; try { await run(h); assert.deepStrictEqual(h.errors, [], 'no uncaught browser errors'); diff --git a/tests/unit/test-observer-detail-ownership.js b/tests/unit/test-observer-detail-ownership.js index ea2747ef..f091c847 100644 --- a/tests/unit/test-observer-detail-ownership.js +++ b/tests/unit/test-observer-detail-ownership.js @@ -11,15 +11,24 @@ function harness() { const charts = []; let page; function element(id) { - const el = { textContent: '', firstChild: { textContent: '' }, listeners: {}, + const children = []; + const el = { textContent: '', firstChild: { textContent: '' }, listeners: {}, htmlWrites: 0, + remove() { children.forEach(child => child.remove()); elements.delete(id); }, addEventListener(name, fn) { this.listeners[name] = fn; }, setAttribute() {} }; let html = ''; Object.defineProperty(el, 'innerHTML', { get() { return html; }, set(value) { + el.htmlWrites++; + children.forEach(child => child.remove()); + children.length = 0; html = value; el.firstChild = { textContent: '' }; - for (const match of value.matchAll(/id="([^"]+)"/g)) elements.set(match[1], element(match[1])); + for (const match of value.matchAll(/id="([^"]+)"/g)) { + const child = element(match[1]); + children.push(child); + elements.set(match[1], child); + } }, }); return el; @@ -55,19 +64,29 @@ function harness() { } function snapshot() { return { title: elements.get('obsTitle').textContent, - html: elements.get('obsDetailContent').innerHTML, + // Include independently updated child regions, as a real DOM does. + html: Array.from(elements.values()).map(el => el.innerHTML).join('\n'), error: elements.get('obsDetailContent').firstChild.textContent, charts: charts.filter(c => !c.destroyed) }; } return { page, app, elements, requests, charts, visit, days, snapshot }; } +function identity(id, label = id) { + return { id, name: label, last_seen: null, model: 'Test radio', radio: '910.0,250,10,5' }; +} +function analytics(label) { + return { timeline: [{ label, count: 1 }], packetTypes: { 4: 1 }, + nodesTimeline: [{ label, count: 1 }], snrDistribution: [{ range: '0 to 5', count: 1 }], recentPackets: [] }; +} +function calibration(id) { + return [{ observerID: 'other', offsetSec: 99, samples: 99 }, + { observerID: id, offsetSec: 1, samples: 1 }, { observerID: id, offsetSec: 2, samples: 2 }]; +} function complete(requests, id, label = id) { - requests[0].resolve({ id, name: label, last_seen: null }); - requests[1].resolve({ timeline: [{ label, count: 1 }], packetTypes: { 4: 1 }, - nodesTimeline: [{ label, count: 1 }], snrDistribution: [{ range: '0 to 5', count: 1 }], recentPackets: [] }); - requests[2].resolve([{ observerID: 'other', offsetSec: 99, samples: 99 }, - { observerID: id, offsetSec: 1, samples: 1 }, { observerID: id, offsetSec: 2, samples: 2 }]); + requests[0].resolve(identity(id, label)); + requests[1].resolve(analytics(label)); + requests[2].resolve(calibration(id)); } // Yield one event-loop turn after resolving promises, rather than guessing a delay. const settled = () => new Promise(resolve => setImmediate(resolve)); @@ -78,6 +97,92 @@ async function test(name, fn) { } (async () => { + for (const order of [[0, 1, 2], [0, 2, 1], [1, 2, 0], [2, 1, 0], [1, 0, 2], [2, 0, 1]]) { + await test('progressive detail handles completion order ' + order.join(','), async () => { + const h = harness(); + const pending = h.visit('A'); + assert.strictEqual(pending.length, 3, 'all requests start before any resolves'); + let identityWrites; + const done = new Set(); + for (const index of order) { + pending[index].resolve([identity('A'), analytics('current'), calibration('A')][index]); + done.add(index); + await settled(); + const state = h.snapshot(); + if (done.has(0)) { + assert.strictEqual(state.title, 'A', 'identity does not wait for secondary responses'); + assert.match(state.html, /Offline/); + assert.match(state.html, /Test radio/); + assert.match(state.html, /910.0 MHz/); + if (identityWrites === undefined) identityWrites = h.elements.get('obsDetailContent').htmlWrites; + assert.strictEqual(h.elements.get('obsDetailContent').htmlWrites, identityWrites, + 'secondary responses never rebuild the metadata DOM'); + assert.strictEqual(state.charts.length, done.has(1) ? 4 : 0); + assert.strictEqual(/2 seconds/.test(state.html), done.has(2), 'calibration renders independently'); + } else { + assert.strictEqual(state.charts.length, 0, 'secondary results wait for a valid identity'); + } + } + assert.strictEqual(h.charts.length, 4, 'charts are only constructed once'); + assert.strictEqual(h.requests.length, 3, 'no extra requests'); + }); + } + + for (const failBeforeIdentity of [false, true]) { + await test('analytics failure preserves identity, including early failure=' + failBeforeIdentity, async () => { + const h = harness(); + const pending = h.visit('A'); + if (failBeforeIdentity) pending[1].reject(new Error('')); + pending[0].resolve(identity('A')); + await settled(); + if (!failBeforeIdentity) pending[1].reject(new Error('')); + pending[2].resolve(calibration('A')); + await settled(); + assert.strictEqual(h.snapshot().title, 'A'); + assert.match(h.snapshot().html, /Test radio/); + assert.match(h.snapshot().html, /2 seconds/); + assert.strictEqual(h.elements.get('obsAnalytics')?.firstChild.textContent, 'Error: '); + assert.doesNotMatch(h.snapshot().html, //, 'error uses textContent'); + assert.strictEqual(h.snapshot().charts.length, 0); + }); + } + + await test('identity failure stays visible when secondary responses settle later', async () => { + const h = harness(); + const pending = h.visit('A'); + pending[0].reject(new Error('')); + await settled(); + const before = h.snapshot(); + assert.strictEqual(before.error, 'Error: '); + assert.doesNotMatch(before.html, //); + pending[1].resolve(analytics('unused')); + pending[2].reject(new Error('unused calibration failure')); + await settled(); + assert.deepStrictEqual(h.snapshot(), before); + }); + + for (const leave of ['route', 'days', 'revisit', 'destroy']) { + for (const failure of [false, true]) { + await test('early identity then ' + leave + ' rejects stale secondary ' + (failure ? 'errors' : 'results'), async () => { + const h = harness(); + const old = h.visit('A'); + old[0].resolve(identity('A', 'old identity')); + await settled(); + assert.strictEqual(h.snapshot().title, 'old identity'); + if (leave === 'route') complete(h.visit('B'), 'B'); + if (leave === 'days') complete(h.days(1), 'A', 'current window'); + if (leave === 'revisit') { complete(h.visit('B'), 'B'); complete(h.visit('A'), 'A', 'new visit'); } + if (leave === 'destroy') h.page.destroy(); + await settled(); + const before = h.snapshot(); + if (failure) { old[1].reject(new Error('stale analytics')); old[2].reject(new Error('stale clock')); } + else { old[1].resolve(analytics('stale')); old[2].resolve(calibration('A')); } + await settled(); + assert.deepStrictEqual(h.snapshot(), before); + }); + } + } + await test('current success renders four charts and the last matching calibration without extra calls', async () => { const h = harness(); complete(h.visit('A /?'), 'A /?', 'Current observer');