fix: packetsLastHour=0 for all observers — remove early break

The /api/observers handler assumed byObserver arrays were sorted
newest-first and used an early break when hitting an old timestamp.
In reality, byObserver is only roughly DESC from the initial DB load;
live-ingested observations are appended at the end (oldest-to-newest).
After ~1 hour of uptime, the first element is old, the break fires
immediately, and every observer returns packetsLastHour=0.

Fix: full scan without break — the array is not uniformly sorted.
The endpoint is cached so performance is unaffected.

fixes #182

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Kpa-clawbotandCopilot committed 2026-03-27 18:14:50 -07:00
1 parent 2435f2eaaf
commit 9b278d8e41
3 files changed
+68 -4

No files matched your search

+4 -4
View File
@@ -2286,11 +2286,11 @@ app.get('/api/observers', (req, res) => {
for (const n of allNodes) nodeMap.set(n.public_key?.toLowerCase(), n);
const result = observers.map(o => {
const obsPackets = pktStore.byObserver.get(o.id) || [];
// byObserver is sorted newest-first, so count from front until we pass the cutoff
// byObserver is NOT uniformly sorted — initial DB load is DESC but live
// ingestion appends newest at the end. Full scan required.
let count = 0;
for (let i = 0; i < obsPackets.length; i++) {
if (obsPackets[i].timestamp > oneHourAgo) count++;
else break;
for (const obs of obsPackets) {
if (obs.timestamp > oneHourAgo) count++;
}
const node = nodeMap.get(o.id?.toLowerCase());
return { ...o, packetsLastHour: count, lat: node?.lat || null, lon: node?.lon || null, nodeRole: node?.role || null };
+51
View File
@@ -496,6 +496,57 @@ test('memory estimate reflects deduplication savings', () => {
assert.strictEqual(obsWithRawHex, 0, 'no observation should have own raw_hex property');
});
// === Regression: packetsLastHour must count live-appended observations (#182) ===
console.log('\n=== packetsLastHour byObserver regression (#182) ===');
test('byObserver counts recent packets regardless of insertion order', () => {
const store = new PacketStore(createMockDb());
store.load();
const twoHoursAgo = new Date(Date.now() - 7200000).toISOString();
const thirtyMinAgo = new Date(Date.now() - 1800000).toISOString();
const fiveMinAgo = new Date(Date.now() - 300000).toISOString();
// Simulate initial DB load: oldest packets pushed first (as if loaded DESC then reversed)
store.insert(makePacketData({ hash: 'old1', timestamp: twoHoursAgo, observer_id: 'obs-hr' }));
// Simulate live-ingested packet (appended at end, most recent)
store.insert(makePacketData({ hash: 'new1', timestamp: thirtyMinAgo, observer_id: 'obs-hr' }));
store.insert(makePacketData({ hash: 'new2', timestamp: fiveMinAgo, observer_id: 'obs-hr' }));
const obsPackets = store.byObserver.get('obs-hr');
assert.strictEqual(obsPackets.length, 3, 'should have 3 observations');
// Count packets in the last hour — the same way the fixed /api/observers does
const oneHourAgo = new Date(Date.now() - 3600000).toISOString();
let count = 0;
for (const obs of obsPackets) {
if (obs.timestamp > oneHourAgo) count++;
}
assert.strictEqual(count, 2, 'should count 2 recent packets, not 0 (regression #182)');
});
test('byObserver early-break bug: old item at front must not abort count', () => {
const store = new PacketStore(createMockDb());
store.load();
const twoHoursAgo = new Date(Date.now() - 7200000).toISOString();
const tenMinAgo = new Date(Date.now() - 600000).toISOString();
// Old observation first, then recent — simulates the mixed-order array
store.insert(makePacketData({ hash: 'h1', timestamp: twoHoursAgo, observer_id: 'obs-bug' }));
store.insert(makePacketData({ hash: 'h2', timestamp: tenMinAgo, observer_id: 'obs-bug' }));
const obsPackets = store.byObserver.get('obs-bug');
const oneHourAgo = new Date(Date.now() - 3600000).toISOString();
// BUGGY code (break on first old item) would return 0 here
let count = 0;
for (const obs of obsPackets) {
if (obs.timestamp > oneHourAgo) count++;
}
assert.strictEqual(count, 1, 'must not skip recent packet after old one');
});
// === Summary ===
console.log(`\n${passed} passed, ${failed} failed`);
if (failed > 0) process.exit(1);
+13
View File
@@ -463,6 +463,19 @@ seedTestData();
assert(typeof r.body === 'object', 'should return observers');
});
await t('GET /api/observers — packetsLastHour regression #182', async () => {
// Clear cache so the handler re-computes counts
cache.clear();
const r = await request(app).get('/api/observers').expect(200);
const observers = r.body.observers || [];
const obs1 = observers.find(o => o.id === 'test-obs-1');
if (obs1) {
assert(typeof obs1.packetsLastHour === 'number', 'packetsLastHour must be a number');
// test-obs-1 has recent seeded packets — must not be zero
assert(obs1.packetsLastHour > 0, `packetsLastHour should be > 0, got ${obs1.packetsLastHour} (regression #182)`);
}
});
await t('GET /api/observers/:id — existing', async () => {
const r = await request(app).get('/api/observers/test-obs-1');
assert(r.status === 200 || r.status === 404, 'should handle');