Skip to content

fix(services): readers that one bad answer could break, and the tests that should have caught it - #27

Merged
mikim merged 1 commit into
mainfrom
claude/service-correctness-tests
Sep 22, 2026
Merged

mikim merged 1 commit into
mainfrom
claude/service-correctness-tests

Conversation

@mikim

@mikim mikim commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Stacked on #26 — merge that first. This PR's diff is only its own commit.

The service layer turns every upstream answer into a dot, a count or a LIVE badge, and most of it had no test. Two earlier commits describe tests that were never committed: 71eb398 ("adds the invariant as a test") touched only src/, and 9ee7d72 was "verified with mocked fetch and timers" that are nowhere in the repo. Mutating the grading, the health merge, the connection state machine or the polling chain left the old 18 tests green.

So the tests come first, written against the existing behaviour. Each fix below then comes with a test that fails without it.

New tests (18 → 91 across the stack; 59 in this PR)

  • ecosystem-feed-merge.test.ts stubs fetch, not the client module, so the real readers run. It uses an inline registry with one of each case: streamed, direct ok, direct 503 degraded that city calls ok, CORS wall with and without aggregate cover, off-contract running, city, an artifact city names, a link, and a hidden entry. It asserts each node's kind, instrumentation and status, that links and files are never fetched, that an empty sweep keeps the snapshot, and that a hung aggregate is cut off at 10 s without costing the first-hand readings.
  • data-bridge.test.ts runs on fake timers. It covers connecting until signals and stats settle, and live while the bulk requests still hang. It checks reachability from stats alone, from signals alone, and from neither. Polls start at 0, 25 s and 50 s: chained, never overlapping, with the 10 s cap. destroy() mid-poll leaves no timers.
  • The 18 known mutants from the review (M1–M16 style) each fail at least one test.

Fixes

What happened Now
A null element in city's aggregate threw out of the health sweep. The next sweep is armed only after a job settles, so health polling stopped for the life of the tab and the last verdicts stayed on screen. Entries are validated at ingest. Both pollers re-arm whatever a cycle does.
metadata: null on one Algora row threw mid-ingest (JSON.parse(null) returns null) and dropped all three origins' signals for that cycle. Rows convert one at a time; a bad row skips only itself.
An abort or dropped connection while reading a 5xx body was caught together with the JSON parse and became down. The body is read outside the try, so the failure propagates and stays unmeasured.
The dedupe prune rebuilt its memory from the drained queue, forgot ids still inside the upstream windows, and counted ~66 old signals as new. An LRU per origin that forgets only ids that stopped being served.
city was fetched twice per sweep (its statusUrl is the aggregate), doubling load on the most expensive health endpoint. Once. Matched on the URL, so a separate city endpoint would still be read first-hand.
A cached aggregate could answer while offline (it is served with stale-while-revalidate). Health reads use cache: "no-store".
Cross-service values went into innerHTML unchecked, and HubMap's tooltip esc threw on non-strings. Type-checked at ingest, escaped and coerced at the sinks (defence in depth). The tooltip markup moves to a Phaser-free src/ui/hub-tooltip.ts with tests.
AO's debate card rotated on time.now % 5000 < 50, which a 60 Hz frame hits about three times per window and a slow frame can miss. An elapsed-time timer; the card fills as soon as debates arrive.
AlgoraZone latched botBusy before code that could throw, stopping the belt for good. Latched after, released in finally.

Also removed: the signal normalizers nothing read (they were wrong on live data, e.g. moc_blockchain classified as ai), fetchAODebateDetail, isConnected, and a dead CSS rule.

Visible differences: a sidebar figure a service sends as something other than a number now shows — instead of 0 or undefined. An off-contract Algora severity draws as medium.

What is verified, and what is not

  • ✅ typecheck, 91 tests, build, and check-health-json
  • ✅ Headless Chrome with real data at 1440×900 and 375×812: map, all tabs, tooltips and sidebar figures match main, and the console is clean
  • ✅ A/B in the browser, with injected responses, for the aggregate freeze (main never sweeps again; this sweeps at the next tick) and for sink escaping
  • Out of scope, and coming in the next PRs: polling cadence and payload size, hidden-tab behaviour, and new UI states (stale, offline, tallies)

🤖 Generated with Claude Code

… that should have caught it

Two commits described tests that were never committed (71eb398's grading
invariant, 9ee7d72's mocked-timer lifecycle checks), so the code that turns
every answer into a dot or a LIVE badge ran unguarded: mutants of the grading,
the health merge, the connection state machine and the polling chain all
passed the old 18 tests. They are written here first, against the existing
behaviour, then each fix below comes with a test that fails without it.

- A `null` element in city's aggregate threw out of the health sweep, and the
  schedule re-armed only after a job settled, so health polling stopped for
  the life of the tab. Entries are now validated at ingest, and both pollers
  re-arm whatever a cycle does.
- `metadata: null` on one Algora row threw mid-ingest and dropped all three
  origins' signals for the cycle. Rows are converted one at a time.
- An abort or dropped connection while reading a 5xx body was caught with the
  JSON parse and became "down". It now propagates, so it stays unmeasured.
- The dedupe prune rebuilt its memory from the drained queue, forgetting ids
  still in the upstream windows and re-counting ~66 signals as new. It is now
  an LRU per origin that only forgets ids that stopped being served.
- city was fetched twice per sweep (its statusUrl is the aggregate). Once now.
- Health reads bypass the HTTP cache: a cached aggregate could answer offline.
- Cross-service values are type-checked at ingest and escaped/coerced at the
  innerHTML sinks (defence in depth); HubMap's tooltip markup moves to a
  Phaser-free module so it can be tested.
- AO's debate card rotated on `time.now % 5000 < 50`; AlgoraZone could latch
  botBusy before code that throws. Both fixed.
- Dead code removed: the signal normalizers nothing read, and unused exports.

Co-Authored-By: Claude Opus 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