From d6256c4f94feee34f10254d64a18d039193f5ec7 Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Thu, 7 May 2026 08:29:22 -0700 Subject: [PATCH] fix(#1151): drop orphan separators from side-panel Heard By rows (#1161) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1151 ## Problem The side-panel "Heard By" row template in `public/nodes.js` (line 1337) built its stats suffix with inline ternaries: ```js ${o.packetCount} pkts · ${o.avgSnr != null ? '...' : ''}${o.avgRssi != null ? ' · RSSI ...' : ''} ``` When `avgSnr` and/or `avgRssi` were `null` (very common in prod — many CJS observers have both null), this produced orphan separators: - both null → `"110 pkts · "` (trailing dot) - snr null only → `"55 pkts · · RSSI -50"` (double dot) ## Fix Build a filtered parts array, then `.join(' · ')`. Only present fields contribute, so the separator can never appear next to nothing. ```js const stats = [`${o.packetCount} pkts`]; if (o.avgSnr != null) stats.push('SNR ' + Number(o.avgSnr).toFixed(1) + 'dB'); if (o.avgRssi != null) stats.push('RSSI ' + Number(o.avgRssi).toFixed(0)); // → stats.join(' · ') ``` Full-page table (line 1337's neighbor) was already null-safe (separate `` cells), so only the side-panel template needed the change. ## TDD Red commit: `1c02ff9a7889aadd16f87f4e673287f9742d4ad0` — adds `test-issue-1151-orphan-separators-e2e.js` to the deploy.yml E2E job. The test stubs `/api/nodes/:pubkey/health` via Playwright `page.route()` with four observer permutations (both null, snr-only-null, rssi-only-null, both set), opens the side panel, and asserts no `.observer-row` stat suffix matches `· ·`, leading `·`, or trailing `·`. E2E assertion added: `test-issue-1151-orphan-separators-e2e.js:96` ## Preflight All hard gates pass — see preflight output in the implementation log. --------- Co-authored-by: CoreScope Bot --- .github/workflows/deploy.yml | 1 + public/nodes.js | 11 +- test-issue-1151-orphan-separators-e2e.js | 128 +++++++++++++++++++++++ 3 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 test-issue-1151-orphan-separators-e2e.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 1a6c1b79..599d8831 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -238,6 +238,7 @@ jobs: BASE_URL=http://localhost:13581 node test-issue-1150-404-state-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1146-path-link-contrast-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1147-section-order-e2e.js 2>&1 | tee -a e2e-output.txt + BASE_URL=http://localhost:13581 node test-issue-1151-orphan-separators-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-rebrand-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-theme-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-default-sage-teal-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/public/nodes.js b/public/nodes.js index d2336099..a7916b73 100644 --- a/public/nodes.js +++ b/public/nodes.js @@ -1389,10 +1389,15 @@ ${(() => { const regions = [...new Set(observers.map(o => o.iata).filter(Boolean))]; return regions.length ? `
Regions: ${regions.join(', ')}
` : ''; })()}

Heard By (${observers.length} observer${observers.length > 1 ? 's' : ''})

- ${observers.map(o => `
+ ${observers.map(o => { + const stats = [`${o.packetCount} pkts`]; + if (o.avgSnr != null) stats.push('SNR ' + Number(o.avgSnr).toFixed(1) + 'dB'); + if (o.avgRssi != null) stats.push('RSSI ' + Number(o.avgRssi).toFixed(0)); + return `
${escapeHtml(o.observer_name || o.observer_id)}${o.iata ? ' ' + escapeHtml(o.iata) + '' : ''} - ${o.packetCount} pkts · ${o.avgSnr != null ? 'SNR ' + Number(o.avgSnr).toFixed(1) + 'dB' : ''}${o.avgRssi != null ? ' · RSSI ' + Number(o.avgRssi).toFixed(0) : ''} -
`).join('')} + ${stats.join(' · ')} +
`; + }).join('')}
` : ''} diff --git a/test-issue-1151-orphan-separators-e2e.js b/test-issue-1151-orphan-separators-e2e.js new file mode 100644 index 00000000..55dfefab --- /dev/null +++ b/test-issue-1151-orphan-separators-e2e.js @@ -0,0 +1,128 @@ +/** + * E2E (#1151): Side-panel "Heard By" rows must not render orphan separators + * when an observer's SNR and/or RSSI are null. + * + * Bug template (public/nodes.js): + * `${o.packetCount} pkts · ${snr ?: ''}${rssi ?: ''}` + * → "110 pkts · " (trailing dot when both null) + * → "110 pkts · · RSSI -52" (double dot when only SNR null) + * + * Fix: build a filtered parts array, then `.join(' · ')`. + * + * This test stubs /api/nodes/:pubkey/health via page.route() so we get + * deterministic observer rows with all three null/non-null permutations + * (the fixture DB has no real observers attached to a single node). + * + * Usage: BASE_URL=http://localhost:13581 node test-issue-1151-orphan-separators-e2e.js + */ +'use strict'; +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; + +let passed = 0, failed = 0; +async function step(name, fn) { + try { await fn(); passed++; console.log(' \u2713 ' + name); } + catch (e) { failed++; console.error(' \u2717 ' + name + ': ' + e.message); } +} +function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } + +(async () => { + const browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'], + }); + const ctx = await browser.newContext({ viewport: { width: 1400, height: 900 } }); + const page = await ctx.newPage(); + page.setDefaultTimeout(15000); + page.on('pageerror', (e) => console.error('[pageerror]', e.message)); + + console.log('\n=== #1151 orphan-separator E2E against ' + BASE + ' ==='); + + // Pick any node from the API to drive the test. + let pubkey = null; + await step('fetch a real node pubkey from /api/nodes', async () => { + const res = await page.request.get(BASE + '/api/nodes'); + assert(res.ok(), '/api/nodes returned ' + res.status()); + const body = await res.json(); + const arr = Array.isArray(body) ? body : (body.nodes || []); + assert(arr.length > 0, 'fixture must contain at least one node'); + pubkey = arr[0].public_key || arr[0].pubkey || arr[0].id; + assert(pubkey, 'first node must expose a public_key'); + }); + + // Stub the health endpoint for this pubkey with three observer permutations: + // 1. both null → pre-fix: "110 pkts · · " (trailing orphan after pkts) + // 2. snr null only → pre-fix: "55 pkts · · RSSI -50" (orphan between pkts and RSSI) + // 3. rssi null only→ pre-fix: "22 pkts · SNR 5.5dB" (clean — control) + // 4. both present → pre-fix: "11 pkts · SNR 7.0dB · RSSI -42" (clean — control) + await page.route('**/api/nodes/' + encodeURIComponent(pubkey) + '/health', async (route) => { + const stubBody = { + node: { public_key: pubkey, name: 'TEST-NODE', role: 'repeater' }, + stats: { totalPackets: 200, packetsToday: 10, avgSnr: null, avgHops: 2, lastHeard: new Date().toISOString() }, + observers: [ + { observer_id: 'obs-both-null', observer_name: 'BothNull', iata: 'SJC', avgSnr: null, avgRssi: null, packetCount: 110 }, + { observer_id: 'obs-snr-null', observer_name: 'SnrNull', iata: 'SJC', avgSnr: null, avgRssi: -50, packetCount: 55 }, + { observer_id: 'obs-rssi-null', observer_name: 'RssiNull', iata: 'OAK', avgSnr: 5.5, avgRssi: null, packetCount: 22 }, + { observer_id: 'obs-both-set', observer_name: 'BothSet', iata: 'OAK', avgSnr: 7.0, avgRssi: -42, packetCount: 11 }, + ], + recentPackets: [], + }; + await route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(stubBody) }); + }); + + // Navigate to the nodes LIST page (not /#/nodes/ — that opens the + // full-screen view, which uses a different, already null-safe template + // with separate cells). The bug lives in the side-panel template + // that renders when you click a row on the list page. + await step('navigate /#/nodes (list view) and wait for rows', async () => { + await page.goto(BASE + '/#/nodes', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#nodesBody tr[data-action="select"]', { timeout: 15000 }); + }); + + await step('click target row to open side-panel detail', async () => { + const sel = '#nodesBody tr[data-action="select"][data-value="' + pubkey + '"]'; + await page.waitForSelector(sel, { timeout: 8000 }); + await page.evaluate((s) => document.querySelector(s).scrollIntoView(), sel); + await page.click(sel); + await page.waitForSelector('#nodesRight .observer-row', { timeout: 15000 }); + }); + + await step('side-panel "Heard By" rows render exactly 4 observers', async () => { + const count = await page.$$eval('#nodesRight .observer-row', els => els.length); + assert(count === 4, 'expected 4 observer rows, got ' + count); + }); + + await step('NO observer row contains an orphan separator (no "· ·" or trailing/leading " · ")', async () => { + const rows = await page.$$eval('#nodesRight .observer-row', els => els.map(el => { + // Read just the right-hand "stats" span (the suffix after the name). + // Two children per row; second one is the stats. + const spans = el.querySelectorAll('span'); + const last = spans[spans.length - 1]; + // Normalize whitespace. + return (last.textContent || '').replace(/\s+/g, ' ').trim(); + })); + + const offences = []; + for (const text of rows) { + // Adjacent middle-dot separators with optional spaces between them. + if (/·\s*·/.test(text)) offences.push(['adjacent-dots', text]); + // Trailing separator (e.g. "110 pkts ·"). + if (/·\s*$/.test(text)) offences.push(['trailing-dot', text]); + // Leading separator (e.g. "· SNR ..."). + if (/^\s*·/.test(text)) offences.push(['leading-dot', text]); + } + + if (offences.length) { + const detail = offences.map(o => `[${o[0]}] "${o[1]}"`).join('\n '); + throw new Error('Found ' + offences.length + ' orphan-separator row(s):\n ' + detail); + } + }); + + await page.unroute('**/api/nodes/' + encodeURIComponent(pubkey) + '/health'); + await browser.close(); + + console.log('\n=== #1151: ' + passed + ' passed, ' + failed + ' failed ==='); + process.exit(failed ? 1 : 0); +})().catch((e) => { console.error('FATAL', e); process.exit(2); });