Skip to content

fix(nodes): calculate analytics over available retained history - #494

Merged
dborup merged 2 commits into
masterfrom
codex/node-analytics-coverage-485
Oct 16, 2026
Merged

dborup merged 2 commits into
masterfrom
codex/node-analytics-coverage-485

Conversation

@dborup

@dborup dborup commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

Summary

Relates to #485.

Calculate node availability and packets/day over the available retained-history interval rather than diluting a short in-memory history across the entire requested 30/365 days. Full analytics and the lightweight summary share the same coverage snapshot and calculation.

  • Adds explicit coverage metadata while preserving existing response fields.
  • Tracks a conservative contiguous history floor through startup, background loading, retention, load limits/errors and memory eviction. Old re-heard FirstSeen values are not evidence of historical coverage.
  • Routine eviction advances coverage.from to the evicted transmission's FirstSeen (the eviction boundary), never to a late re-hearing. Late observations of evicted transmissions are counted in coverage.late_evicted_observations (known, bounded gap).
  • Applies the same inclusive UTC interval to packet selection and the denominator. First/last hour buckets are weighted by their intersecting duration; zero/unknown duration produces finite zero rates.
  • Shows an accessible partial-history notice on desktop/mobile and documents that retained history is not measured RF uptime.
  • No schema, configuration, ingestor or workflow changes, and no per-request SQL history scan. The only CI registration change is the new browser suite in the existing shard manifest.

Regression and review evidence

The first regression failed on unchanged production: 48 hours containing 48 packets were reported as 1.6 packets/day for a requested month rather than 24.

Deterministic tests cover full/summary parity, complete ranges, UTC and non-UTC clocks, exact boundaries, weighted partial hours, empty/unknown history, startup/background transitions, real retained-window loading, old re-heard transmissions, caps, eviction and SQLite count/cursor/scan failures.

Independent review found and resolved three edge cases before publication:

  1. Reaching a budget estimate is not itself evidence that rows were omitted; compare eligible population with actual load.
  2. Cursor/scan/count errors must not certify complete history.
  3. Live evidence accepted during an initially empty background load must survive its completion.

The final independent review is APPROVE with no remaining findings on tree 93e69468baabe9e750ab295df9c946548e759aa3. The reviewer's unchanged handoff reproduction and all coverage regressions pass with -race (6.051 s).

Verification

  • Full server suite on final source: pass (95.726 s).
  • Focused coverage/summary/characterization and injected-failure tests: pass normally and with -race; vet, gofmt and diff checks are clean.
  • Full npm suite: 257/257 test files pass, with existing platform/tool-specific skips reported by their harness.
  • New Chromium coverage-notice suite: 6/6 pass at 1440 and 375 px, including complete/partial/unknown states; registered in the existing shard manifest.
  • General Chromium regression: 154 passed, 3 fixture-dependent skips, matching baseline, no failures.
  • Final local API/UI check and desktop/mobile screenshots: pass against a fresh isolated fixture.
  • Full unchanged ingestor baseline: pass (114.377 s). No staging, production or measurement host was contacted.

GitHub/Linux CI is separate evidence and will be reported after the new PR run.

Performance and limits

The constant-sized coverage snapshot is O(1) (~133 ns/op, 64 B/op, 2 allocs). Evidence is reduced during existing load/ingest loops; there is no added per-request SQL or complete-store history scan. Endpoint aggregation remains O(packets indexed to the requested node).

Round 2 adds a canonical-UTC (YYYY-MM-DDTHH:MM:SSZ) fast path to parseNodeAnalyticsTime (fallback to time.Parse, parity-tested). Per timestamp: ~29.6 ns (fast) vs ~42.4 ns (time.Parse), 0 allocs both (BenchmarkParseNodeAnalyticsTime485).

Endpoint benchmark, same machine and flags, 1 s, 3 runs, medians (Linux amd64, 4 CPUs; run-to-run spread about 2-3%):

Packets / endpoint Before round 2 After round 2
1,000 / summary 0.789 ms 0.766 ms
1,000 / full 1.004 ms 0.999 ms
30,000 / summary 25.0 ms 24.4 ms
30,000 / full 33.0 ms 32.7 ms

The end-to-end gain is within noise (about 1-2%). Profiling shows timestamp parsing is now ~4.5% of the summary request; the dominant cost is per-packet JSON decoding in the accumulator, which this PR does not touch. The earlier round-1 +12-14% (about 2.3 ms at 30K) therefore largely remains and is not claimed as recovered. It is the price of correct, validated UTC handling and should be weighed at merge review. This is a correctness change, not a performance optimization.

Coverage is deliberately conservative. A capped, failed or unverifiably incomplete startup discards uncertified historical intervals and uses subsequent supported evidence instead of guessing. The standard E2E fixture exposes an existing loader limitation: seeded negative transmission IDs are counted as eligible but omitted by its legacy cursor. The new metadata therefore reports partial/unverified history and can show zero historical rates on that fixture. This PR does not hide that mismatch, change node-membership semantics, or silently alter the loader's ID behavior. Repairing that loader limitation is separate work.

Status: draft, auto-merge disabled. Round 2 review fixes are pushed; readiness, merge and staging validation (check that coverage.from follows the oldest in-memory packet across eviction ticks) are left to the maintainers.

Attribution

Reference adapted to the fork's full/summary API and loader lifecycle:

Repository: Kpa-clawbot/CoreScope
PR: 2211
Head: 78f82a6122af4e512d04702093158a6dfce3f08e
Merge: 927a8eec8a430b8813e8afa1668d536ca62ed3a0

@dborup

dborup commented Oct 16, 2026

Copy link
Copy Markdown
Owner Author

Review — CS-Coordinator PR#494 — head e1fd75d

Verdict: REQUEST CHANGES. One medium finding, and the rest is optional or informational.

The fix addresses a real defect. Rates were divided by the requested 30 or 365 days even when only a few days are held in memory. The approach is sound:

Findings

# Severity Finding
1 Medium (blocking) Routine eviction can advance the coverage edge far past the real history. invalidateNodeAnalyticsEvictionLocked is called from evictStaleInternal, which runs about every minute on both of our production instances. Their logs show [store] Evicted N packets (… obs …) about 125–180 times in 24 h. For each evicted tx it moves from to the latest observation timestamp + 1 ns. Measured on production data, one day (2026-10-12, 17,999 transmissions): 27 were re-heard more than 1 h after first observation, and the longest gap was ~2 days (172,428 s). When such a tx is evicted at the window edge, coverage jumps forward by up to ~2 days, so availability and packets/day fluctuate with whichever tx was just evicted. Suggestion: advance the edge to the eviction boundary (the evicted tx's FirstSeen/cutoff). Account for late observations of evicted txs as a known, bounded gap, for example a counter in coverage, instead of moving from. Or cap the advancement. Add a test in which a re-heard old tx is evicted and assert that from does not jump to its late observation.
2 Low (optional) Per-request parsing cost. The PR's own measurement is +12–14% at 30K packets (~2.3 ms), mostly from parseNodeAnalyticsTime per packet in filterNodePacketsLocked and the bucket loop. The old code compared ISO strings. Suggestion: use a fast path with string comparison when the timestamp is canonical UTC …Z, and parse only as a fallback, or pre-parse at ingest. The trade-off is documented honestly, so this is not blocking.
3 Info The standard E2E fixture reports partial or unverified history with zero rates, because the legacy cursor skips the seeded negative IDs. Thanks for not hiding it. It means the "complete" state is not exercised on realistic data in CI, so it should be checked on staging (see below).
4 Process CI was still running when this review was done, and the PR is a draft.

Before merge

After fix 1, run on staging for a few hours and check that coverage.from follows the oldest in-memory packet and does not jump on eviction ticks.

…late observations (#485)

Routine eviction moved coverage.from to the latest observation of each
evicted tx, so a re-heard old tx (up to ~2 days late in production) made
availability and packets/day jump on every eviction tick. Packet
membership is by FirstSeen, so the edge is now the evicted tx's
FirstSeen (+1 ns). Late observations of evicted txs are counted in
coverage.late_evicted_observations as a known, bounded gap and never move
from. Unparseable FirstSeen and capped/failed/loading paths stay
conservative.

Also add a canonical-UTC (YYYY-MM-DDTHH:MM:SSZ) fast path to
parseNodeAnalyticsTime with time.Parse fallback, guarded by a parity test.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant