From 9b278d8e416cc07c6f032e8af2047b72afafc408 Mon Sep 17 00:00:00 2001
From: Kpa-clawbot <259247574+Kpa-clawbot@users.noreply.github.com>
Date: Fri, 27 Mar 2026 18:14:50 -0700
Subject: [PATCH] =?UTF-8?q?fix:=20packetsLastHour=3D0=20for=20all=20observ?=
 =?UTF-8?q?ers=20=E2=80=94=20remove=20early=20break?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 server.js             |  8 +++----
 test-packet-store.js  | 51 +++++++++++++++++++++++++++++++++++++++++++
 test-server-routes.js | 13 +++++++++++
 3 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/server.js b/server.js
index fb17f187..9775dbde 100644
--- a/server.js
+++ b/server.js
@@ -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 };
diff --git a/test-packet-store.js b/test-packet-store.js
index 3441319d..9a25fe03 100644
--- a/test-packet-store.js
+++ b/test-packet-store.js
@@ -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);
diff --git a/test-server-routes.js b/test-server-routes.js
index 9dc08996..d3becc09 100644
--- a/test-server-routes.js
+++ b/test-server-routes.js
@@ -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');