fix(#866): per-observation children in expanded packet groups (#880)

## Problem
When a packet group is expanded in the Packets table, clicking any child
row pointed the side pane at the same aggregate packet — not the clicked
observation. URL would flip between `?obs=<packet_id>` values instead of
real observation ids.

## Root cause
The expand fetch used `/api/packets?hash=X&limit=20`, which returns ONE
aggregate row keyed by packet.id. Every child therefore carried
`data-value=<packet.id>`.

## Fix
Switch the expand fetch to `/api/packets/<hash>`, which includes the
full `observations[]` array. Build `_children` as `{...pkt, ...obs}` so
each child row gets a unique observation id and observation-level fields
(observer, path, timestamp, snr/rssi) override the aggregate.

## Verified live on staging
Tested on multiple packets:
- Click group-header → side pane shows observation 1 of N (first
observer)
- Click child row → pane updates to show THAT observer's details:
observer name, path, timestamp, obs counter (K of N), URL
`?obs=<observation_id>`

## Tests
592 frontend tests pass (no new ones — this is a wiring fix, live
E2E-verified instead).

Closes #866

---------

Co-authored-by: Kpa-clawbot <agent@corescope.local>
Co-authored-by: you <you@example.com>
This commit is contained in:
Kpa-clawbot
2026-04-21 13:36:45 -07:00
committed by GitHub
parent 1d449eabc7
commit 0ca559e348
2 changed files with 95 additions and 3 deletions
+15 -3
View File
@@ -519,7 +519,7 @@
if (p.decoded_json) existing.decoded_json = p.decoded_json;
// Update expanded children if this group is expanded
if (expandedHashes.has(h) && existing._children) {
existing._children.unshift(p);
existing._children.unshift(clearParsedCache({...p, _isObservation: true}));
if (existing._children.length > 200) existing._children.length = 200;
sortGroupChildren(existing);
// Invalidate row counts — child count changed, so virtual scroll
@@ -683,10 +683,14 @@
// Restore expanded group children (parallel fetch, Map lookup)
if (groupByHash && expandedHashes.size > 0) {
const expandedArr = [...expandedHashes];
// Fetch the full packet detail (which includes per-observation rows) for each expanded hash.
// Previously this used `/packets?hash=X&limit=20` which returned ONE aggregate row, causing
// every "child" row in the table to carry the parent packet.id instead of unique observation
// ids — so clicking any child pointed the side pane at the same aggregate. See #866.
const results = await Promise.all(expandedArr.map(hash => {
const group = hashIndex.get(hash);
if (!group) return { hash, group: null, data: null };
return api(`/packets?hash=${hash}&limit=20`)
return api(`/packets/${hash}`)
.then(data => ({ hash, group, data }))
.catch(() => ({ hash, group, data: null }));
}));
@@ -694,7 +698,15 @@
if (!group) {
expandedHashes.delete(hash);
} else if (data) {
group._children = data.packets || [];
const pkt = data.packet || group;
// Build per-observation children. Spread (pkt, obs) so obs-level fields
// (id, observer_id/name, path_json, snr/rssi, timestamp, raw_hex) override
// the aggregate. Each child's `id` is the observation id (unique per observer).
const obs = data.observations || [];
group._children = obs.length
? obs.map(o => clearParsedCache({...pkt, ...o, _isObservation: true}))
: [pkt];
group._fetchedData = { packet: pkt, observations: obs, breakdown: data.breakdown };
sortGroupChildren(group);
}
}
+80
View File
@@ -1778,6 +1778,86 @@ async function run() {
}
});
// Test: Expanded group children have unique observation ids (#866)
await test('Expanded group children update detail pane per-observation', async () => {
await page.goto(`${BASE}/#/packets`, { waitUntil: 'domcontentloaded' });
// Ensure grouped mode and wide time window
await page.evaluate(() => {
localStorage.setItem('meshcore-time-window', '525600');
localStorage.setItem('meshcore-groupbyhash', 'true');
});
await page.reload({ waitUntil: 'load' });
await page.waitForSelector('table tbody tr', { timeout: 15000 });
// Find a group row with observation_count > 1 (has expand button)
const expandBtn = await page.$('table tbody tr .expand-btn, table tbody tr [data-expand]');
if (!expandBtn) {
console.log(' ️ No expandable groups found — skipping child assertion');
return;
}
// Click expand and wait for the /packets/<hash> detail API call
const [detailResp] = await Promise.all([
page.waitForResponse(resp => {
const u = new URL(resp.url(), BASE);
// Match /api/packets/<hash> but not /api/packets?... or /api/packets/observations
return /\/api\/packets\/[A-Fa-f0-9]+$/.test(u.pathname) && resp.status() === 200;
}, { timeout: 15000 }),
expandBtn.click(),
]);
assert(detailResp, 'Expected /api/packets/<hash> response on expand');
// Wait for child rows to appear
await page.waitForSelector('table tbody tr.child-row, table tbody tr[class*="child"]', { timeout: 5000 });
const childRows = await page.$$('table tbody tr.child-row, table tbody tr[class*="child"]');
if (childRows.length < 2) {
console.log(' ️ Group has < 2 children — skipping per-observation assertion');
return;
}
// Click first child row
await childRows[0].click();
await page.waitForFunction(() => {
const panel = document.getElementById('pktRight');
return panel && !panel.classList.contains('empty') && panel.textContent.trim().length > 0;
}, { timeout: 10000 });
const content1 = await page.$eval('#pktRight', el => el.textContent.trim());
const url1 = page.url();
// Click second child row
await childRows[1].click();
await page.waitForTimeout(500);
const content2 = await page.$eval('#pktRight', el => el.textContent.trim());
const url2 = page.url();
// URL should contain ?obs= with a real observation id
assert(url1.includes('obs=') || url2.includes('obs='), `URL should contain obs= parameter, got: ${url1}`);
// The two children should show different detail pane content (different observers)
// At minimum, the URL obs= values should differ
if (url1.includes('obs=') && url2.includes('obs=')) {
const obs1 = new URL(url1).hash.match(/obs=(\d+)/)?.[1];
const obs2 = new URL(url2).hash.match(/obs=(\d+)/)?.[1];
if (obs1 && obs2) {
assert(obs1 !== obs2, `Two children should have different obs ids, both got obs=${obs1}`);
}
}
// Verify obs id is NOT the aggregate packet id (the bug from #866)
const obsMatch = url2.match(/obs=(\d+)/);
if (obsMatch) {
const detailJson = await detailResp.json().catch(() => null);
if (detailJson?.packet?.id) {
const aggId = String(detailJson.packet.id);
// At least one child obs id should differ from the aggregate packet id
const obs1 = url1.match(/obs=(\d+)/)?.[1];
const obs2 = url2.match(/obs=(\d+)/)?.[1];
const allSameAsAgg = obs1 === aggId && obs2 === aggId;
assert(!allSameAsAgg, `Child obs ids should not all equal aggregate packet.id (${aggId})`);
}
}
});
await browser.close();
// Summary