Keep NaN out of analytics aggregates when a measure is missing from some samples

[kriszyp]

Analytics aggregation now folds each numeric measure over only the samples that carry it, so a measure missing from a metric's first sample no longer stores NaN (or is dropped, for per-thread metrics), and measures present in every sample aggregate exactly as before.

⊙ Problem

Analytics aggregation stores NaN for a numeric measure that the first raw sample of a metric key did not carry. aggregation() builds each aggregate action from the key's first sample, then folds later samples into it; for a measure missing from that first sample both Math.max(undefined, v) (peak-named max* measures, added in fix(analytics): report aggregate maxDepth as the sum of per-thread peaks) and the count-weighted mean evaluate to NaN, and the NaN is written to hdb_analytics. For byThread metrics, the cross-thread combination summed only the measures of the key's very first sample, so a measure that sample lacked, or that only some threads carry, was dropped from the stored row. Sparse measures come from callback metrics (recordAction(fn)), analytics listeners and raw backlog from other versions.

A second, older defect sits in the same fold: the mean was weighted by the action's total sample count, so samples that lacked a measure still weighed into its mean (values 6 and 12, each count 2, with two samples lacking it between them, averaged to 8 instead of 9).

❓ Your call: Is fixing the mean's weighting (a per-measure count) warranted here, or only the NaN? I fixed both: seeding a late measure without a per-measure count trades NaN for a finite wrong number (the fixture's late would read 3 instead of 6). The change is confined to aggregation(); a measure present in every sample folds bit-identically to before.

💡 Solution

• Per-measure weights. Each fold target (an action, or one thread's record for byThread metrics) keeps the count of the samples that carried each measure in a cycle-local measureCounts map, never on the action that storeMetric persists and listeners receive. A measure's first sample seeds it, max[A-Z] measures fold with Math.max, and other measures are the running mean weighted by their own count.
• Unchanged for complete measures. The counts are initialized lazily at a target's first fold from the target's own count, so a measure present in every sample has a count equal to the action's and the same running-mean arithmetic (not sum-then-divide, which rounds differently).

⚠️ Look hardest: the lazy initialization is what keeps complete measures bit-identical: it gives a top-level action's first sample the weight entry.count || 1 and a thread record's first sample weight 1, exactly as the old previousCount did.

• Cross-thread combination. The entry's numeric fields are zeroed and every thread record's numeric measures are added into it, in the same thread order as before, so a measure absent from the first sample or present on only some threads is summed over the threads that carry it.
• No total/ratio on thread records. A thread's record is built from a sample's measures alone and never holds a total, so folding total into it produced total/ratio NaN; the fold now keeps total/ratio on non-thread actions only. With that, everything on a thread record is a measure (plus its count, which the thread count replaces), and a caller measure named ratio is summed like any other.
• Thread fold keyed on byThread. Whether a sample folds into a thread record is now decided by the action's byThread flag (which creates the thread records), not by its threads field, which the first sample's own fields are spread into. On main, a non-byThread metric reporting its own threads field (for example a thread count, threads: 4) throws TypeError: Cannot create property '1' on number '4' on its second sample before the raw cursor advances, so every later cycle throws on the same record and aggregation stops.
• Design note. The weighting invariant is recorded in resources/analytics/DESIGN.md and indexed from the root DESIGN.md.

⚖️ Alternatives

• Planning review round 1 returned Framing-Verdict: better-alternative-exists; the alternative was adopted: accumulate the cross-thread sum directly into the entry instead of first building the union of thread measure names and summing per name. The union design as first written still gated each name on the entry's original field, so it dropped every newly discovered measure; the direct pass visits only present fields.
• Fix at the producers (fixed measure set, zero-filled): rejected — producers are open-ended callbacks and listeners, the raw backlog stays sparse, and a zero-filled mean is not the mean of the samples that measured it.
• Running weighted sums divided at the end: rejected — changes the last bits of every complete measure (0.7, 0.2, 0.1, 0.1, 0.1 → 0.24000000000000005 today vs 0.24).

❓ Your call: An absent measure averages over only the samples that carry it, rather than counting as 0. This concerns a measure missing inside a reported entry; a metric whose whole entry is skipped while zero (ws-connections, mqtt-connections) was already averaged over the samples that reported it and is unchanged. Reversing it later shifts stored values; existing rows are not recomputed.

❓ Your call: The per-measure counts stay cycle-local; the stored row exposes only the action's total count. Nothing in this repo re-weights stored rows by count, but a cross-period or cross-node rollup that did would mis-weight a sparse measure. Persisting per-measure counts later is additive.

❓ Your call: In the byThread sum, a measure carried by only some threads is summed over those threads (stalled: 3 in the fixture), so the row reads as a node-wide total although some threads never reported it; and every numeric field on a thread record is summed, so a non-measure field added to thread records later would be summed too. Easy to change if partial coverage should be flagged or omitted instead.

❓ Your call: byThread rows keep storing period: 0 and threadId as the sum of thread ids: the combination zeroes every numeric entry field and thread records carry no period. That is unchanged from main and left alone here (the task scoped this PR to sparse measures); excluding period/threadId from the sum is a one-line follow-up, and Record per-worker event loop delay, per-thread duration and whole-request time in analytics changes both while rewriting the same loop. Fold it in here, or leave it to that PR / a follow-up?

❓ Your call: The neighboring total field has the same NaN when a key's first sample in a window lacks it and a later one carries it (action.total += total, unchanged from main). I left it out: seeding it is one line, but ratio = total / count would then divide by samples that had no total, which needs its own decision. Fold it in here, or follow up?

❓ Your call: Fix forward only — aggregate rows already stored with NaN stay in hdb_analytics until retention removes them.

✅ Verification

• End-to-end route: the new unit cases drive the real runAggregationCycle over real hdb_raw_analytics / hdb_analytics tables and read the stored rows. The live sparse producers are callback/listener metrics; the seeded raw reports stand in for them.
• unitTests/resources/analytics/aggregationCycle.test.js — three new cases, on shared seed/drain/wait/assertNoNaN helpers:
  • mean/peak: maxLatency and late absent from the first sample, rare absent from the middle ones, a zero-valued first late, an unequal first count; asserts no NaN, per-measure weighted means, and the dense mean at its exact running-mean value 0.28571428571428575.
  • byThread: thread 0's first sample carries only depth; maxDepth/queued appear later; stalled only on thread 7; thread 7 also carries total and a ratio measure; asserts per-thread peaks and means summed, ratio 0.5, and no NaN.
  • threads field: a non-byThread metric whose samples carry threads: 4 folds to mean 3, count 2 (throws TypeError on main).
  • The existing peak-sum case now uses the shared drain/wait helpers, assertions unchanged.
• Fails on base: against origin/main (a4e217a) dist, the mean/peak case fails on maxLatency = NaN and the byThread case on maxDepth undefined !== 14; the total and ratio fixture fields each fail at the commit before their fix (total = NaN; ratio undefined !== 0.5). All 7 pass with the change, on RocksDB and HARPER_STORAGE_ENGINE=lmdb.
• Full gates at c81fb79: test:unit:resources 4135 passing / 0 failing; test:unit:main 6841 passing / 0 failing; test:integration:all 2338 pass / 0 fail / 6 cancelled (all six in the Ollama backend suite, which needs a local Ollama with default models; unrelated). Prettier, oxlint and check:design-docs clean.

Refs #3017

_{🤖 Generated by Claude Opus 5.5 (Claude Code dev-agent); posted via @kriszyp.}

🤖 Generated with Claude Code

https://claude.ai/code/session_01KfL5oZzJbTmj6ZjiVWM8dc

Related PRs: #3110 overlaps (rewrites the same aggregation fold and byThread combination; whichever lands second resolves the conflict)
Complexity: medium

_{Dispatch: task harper-analytics-sparse-measure-nan · queued by unknown · ran by claude/opus/xhigh · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=cursor-muse,codex,gemini; adjudicated=domain; declined=cursor-grok,cursor-composer,cursor-kimi; rounds=5; full=2 @ c81fb79}

_{Review-Attention: study ~15m (critical: write.ts; decisions: weight-not-persisted, absent-means-unreported, thread-sum-union, sparse-total-scope, ratio-denominator, absent-measure-semantics, partial-thread-sum, running-mean-kept, no-backfill) @ c81fb79}

[github-actions]

Release cherry-pick v5.3: tests ❌ failed

Cherry-picked PR #3139 onto v5.3 at branch cherry-pick/v5.3/pr-3139.
Integration tests: ❌ failed — https://github.com/HarperFast/harper/actions/runs/38067031677

On merge of this PR the cherry-pick branch will be fast-forwarded into v5.3.

[gemini-code-assist]

Code Review

This pull request updates the analytics aggregation logic to fold each measure over only the samples that carry it, tracking per-measure sample counts in a cycle-local map. It also updates the design documentation and adds comprehensive unit tests. The review feedback correctly identifies two critical issues: first, the period property is not destructured from entry in the thread-averaging loop, which incorrectly resets the aggregation period to 0 for thread-based metrics; second, a missing total property on the first sample can lead to NaN propagation. Both issues are accompanied by actionable code suggestions to resolve them, along with a recommendation to add a corresponding test assertion.