fix(project-metrics): drop the five-minute buckets the cache can never answer again

changelog.d/2806.fixed.md

@@ -0,0 +1 @@
+The board's metrics cache no longer holds every five-minute window it has ever answered: a window that is over is dropped on the next write, and the table is capped, so a host that runs for weeks keeps a handful of entries instead of one per window per repository and zone. A hit inside the same window, the five-minute lifetime and every number on the board are unchanged. See [#2806](https://github.com/ThinkFlowLab/FlyBot/issues/2806).

core/src/host/project-metrics.js

@@ -140,6 +140,22 @@ export function runningOf(task, now) {
 // with it. A hit answers the same object and spends no source call, which is what the issue's
 // "cache hit reads the source once" case is about.
 const cache = new Map()
+// #2806: the key carries the five-minute window, so a bucket that is over can never be answered
+// again — a new key arrives every five minutes for each repository and zone, and nothing ever
+// removed the old ones. Every write now drops what it can no longer answer, and the cap catches a
+// table whose writes outpace its reads, because nothing sweeps in between.
+const CACHE_MAX = 512
+const pruneCache = (cacheStore, now) => {
+  // The default store is the Map above. An injected store that keeps only `get`/`set` (a test seam)
+  // is left as it is rather than given a `delete` it never promised.
+  if (typeof cacheStore.delete !== 'function') return
+  for (const [key, entry] of cacheStore) if (entry.expiresAt <= now) cacheStore.delete(key)
+  while (cacheStore.size >= CACHE_MAX) {
+    const oldest = cacheStore.keys().next()
+    if (oldest.done) break
+    cacheStore.delete(oldest.value)
+  }
+}
 
 const emptyMetrics = () => Object.fromEntries(METRIC_KEYS.map(key => [key, null]))
 
@@ -243,6 +259,7 @@ export async function readMetrics({ repo, token, fetchImpl, timeZone = 'UTC', no
     generatedAt: value.generatedAt,
     metrics: { prsOpenedToday, prsMergedToday, issuesOpenedToday, issuesClosedToday, prMergeMedianMs, issueCloseMedianMs },
   }
+  pruneCache(cacheStore, now)
   cacheStore.set(key, { expiresAt: now + ttlMs, value: repositoryValue })
   return { ...withLive(value), cache: 'miss', ...(source.truncated ? { truncated: true } : {}) }
 }

core/test/project-metrics-cache-bound.test.js

@@ -0,0 +1,22 @@
+// #2806: the shared metrics cache is keyed by a five-minute window, so an entry can never be
+// answered again once its window is over — a bucket that is done is dead weight, and the host kept
+// every one of them for the life of the process (288 a day per repository and zone).
+import test from 'node:test'
+import assert from 'node:assert/strict'
+import { METRICS_TTL_MS, readMetrics } from '../src/host/project-metrics.js'
+
+const NOW = Date.parse('2026-10-30T12:00:00Z')
+const fetcher = async () => ({ ok: true, json: async () => [] })
+
+test('a five-minute bucket that is over is dropped, not kept for the life of the host', async () => {
+  const store = new Map()
+  const read = now => readMetrics({ repo: 'o/r', token: 'tok', fetchImpl: fetcher, now, cacheStore: store })
+  // Bucket 0 fills the cache, then 200 more windows pass: each one is a new key, and the read that
+  // fills it is the only chance this table has to drop the windows it can no longer answer.
+  assert.equal((await read(NOW)).cache, 'miss')
+  for (let bucket = 1; bucket <= 200; bucket += 1) await read(NOW + bucket * METRICS_TTL_MS)
+  // A bucket that is over is a miss, not a hit served out of a dead entry. Before #2806 this read
+  // answered `hit` from bucket 0's entry, which had been carried across 200 later windows.
+  assert.equal((await read(NOW)).cache, 'miss')
+  assert.ok(store.size <= 2, `the store keeps the live window, not 201 dead ones (kept ${store.size})`)
+})

patches/ds-bot/files/src/host/project-metrics.js

@@ -140,6 +140,22 @@ export function runningOf(task, now) {
 // with it. A hit answers the same object and spends no source call, which is what the issue's
 // "cache hit reads the source once" case is about.
 const cache = new Map()
+// #2806: the key carries the five-minute window, so a bucket that is over can never be answered
+// again — a new key arrives every five minutes for each repository and zone, and nothing ever
+// removed the old ones. Every write now drops what it can no longer answer, and the cap catches a
+// table whose writes outpace its reads, because nothing sweeps in between.
+const CACHE_MAX = 512
+const pruneCache = (cacheStore, now) => {
+  // The default store is the Map above. An injected store that keeps only `get`/`set` (a test seam)
+  // is left as it is rather than given a `delete` it never promised.
+  if (typeof cacheStore.delete !== 'function') return
+  for (const [key, entry] of cacheStore) if (entry.expiresAt <= now) cacheStore.delete(key)
+  while (cacheStore.size >= CACHE_MAX) {
+    const oldest = cacheStore.keys().next()
+    if (oldest.done) break
+    cacheStore.delete(oldest.value)
+  }
+}
 
 const emptyMetrics = () => Object.fromEntries(METRIC_KEYS.map(key => [key, null]))
 
@@ -243,6 +259,7 @@ export async function readMetrics({ repo, token, fetchImpl, timeZone = 'UTC', no
     generatedAt: value.generatedAt,
     metrics: { prsOpenedToday, prsMergedToday, issuesOpenedToday, issuesClosedToday, prMergeMedianMs, issueCloseMedianMs },
   }
+  pruneCache(cacheStore, now)
   cacheStore.set(key, { expiresAt: now + ttlMs, value: repositoryValue })
   return { ...withLive(value), cache: 'miss', ...(source.truncated ? { truncated: true } : {}) }
 }

patches/ds-bot/files/test/project-metrics-cache-bound.test.js

@@ -0,0 +1,22 @@
+// #2806: the shared metrics cache is keyed by a five-minute window, so an entry can never be
+// answered again once its window is over — a bucket that is done is dead weight, and the host kept
+// every one of them for the life of the process (288 a day per repository and zone).
+import test from 'node:test'
+import assert from 'node:assert/strict'
+import { METRICS_TTL_MS, readMetrics } from '../src/host/project-metrics.js'
+
+const NOW = Date.parse('2026-10-30T12:00:00Z')
+const fetcher = async () => ({ ok: true, json: async () => [] })
+
+test('a five-minute bucket that is over is dropped, not kept for the life of the host', async () => {
+  const store = new Map()
+  const read = now => readMetrics({ repo: 'o/r', token: 'tok', fetchImpl: fetcher, now, cacheStore: store })
+  // Bucket 0 fills the cache, then 200 more windows pass: each one is a new key, and the read that
+  // fills it is the only chance this table has to drop the windows it can no longer answer.
+  assert.equal((await read(NOW)).cache, 'miss')
+  for (let bucket = 1; bucket <= 200; bucket += 1) await read(NOW + bucket * METRICS_TTL_MS)
+  // A bucket that is over is a miss, not a hit served out of a dead entry. Before #2806 this read
+  // answered `hit` from bucket 0's entry, which had been carried across 200 later windows.
+  assert.equal((await read(NOW)).cache, 'miss')
+  assert.ok(store.size <= 2, `the store keeps the live window, not 201 dead ones (kept ${store.size})`)
+})