[CHKM001]

PR #219 Per-venue heartbeat with stale-price exclusion

Reviewer action — please apply the tests-removed-intentionally label.
This PR intentionally renames one aquarius test case (details under CI notes
below). The "Dropped tests" check is designed to be cleared by that label, not
by a code change.

What this does

Adds a per-venue heartbeat keyed by (venue, network) that tracks last
successful read, last successful write, and consecutive read failures,
and excludes a venue from price aggregation once its last written price is older
than its staleness bound (default 5 min). The exclusion is surfaced, not silent.

The read/write split is what makes a dead adapter distinguishable from a
merely quiet one: a quiet venue keeps a recent lastSuccess with zero
failures while its lastWrite ages; a dead venue ages on both with
consecutiveFailures climbing. Staleness is measured on write age (the age
of the price), so a quiet venue's stale price is excluded either way.

Changes

• src/venueHeartbeat.ts — in-process heartbeat registry.
• Aggregation (src/aggregator/vwap.ts) — calculateVWAP/getAggregatedPrice
  take an excludedSources list and thread it through every query (source NOT IN (…)), including the AMM spot average, volume breakdown, source count and
  last-trade age. Exclusion applies to the exact decimal-string (raw) outputs
  too, not just the numeric ones.
• Every aggregate path honors it so a dead venue's price can't survive in a
  cached payload: GET /price (src/api/rest.ts), the refresh worker
  (src/jobs/aggregateRefresh.ts), and GraphQL (src/api/graphql.ts) — each
  returns excludedVenues.
• Adapters record heartbeats (SDEX, AMM, aquarius_amm, soroswap_amm):
  a successful read with no new trades records a read success (quiet, not dead);
  a successful write records both; a read error records a failure.
  fetchAquariusPools now throws on a transport/non-2xx error and returns []
  only for a genuinely empty result, so a dead API is no longer conflated with a
  pair that has no pool. A cycle abandoned on its own deadline (feat(ingest): bound every ingester cycle with a deadline, abandoned-cycle metric, and overlap-proof abandonment (#351) #471) is not
  recorded as a venue failure.
• Exposure — GET /status → venueHeartbeats (filtered to the requested
  network); GET /metrics → venue_last_success_timestamp,
  venue_last_write_timestamp, venue_consecutive_failures (labels venue,
  network; cardinality bounded by config).
• Contract & docs — src/api/schemas.ts, openapi.yaml/openapi.json,
  schema.graphql, README.md, docs/ingest-metrics.md.

Acceptance criteria

• Every venue reports last-success, last-write and consecutive-failure count.
• Stale venue prices are excluded from aggregates, and responses say a venue
  was excluded (live /price, refresh worker, and GraphQL).
• /status and /metrics both expose it.
• A test drives a venue to stale and asserts it leaves the aggregate.

CI notes

• Dropped tests → needs the tests-removed-intentionally label. This PR
  removes aquariusIngester.test.ts :: "returns an empty array on a non-ok response" and replaces it with "throws on a non-ok response so the caller can record a read failure". The throw-vs-[] change is the core of the dead-vs-
  quiet distinction, so the old assertion can no longer be true — the removal is
  deliberate.
• Typecheck & build → this branch also includes one change unrelated to
  A venue that goes quiet is indistinguishable from a venue with no trades #219: it defines components/responses/BadRequest in openapi.yaml (and
  regenerates openapi.json + clients/typescript/src/schema.d.ts). That $ref
  was dangling on main (from the /rates endpoint in feat: ingest Blend lending pool state, rates, and derived utilisation #453) and broke
  client:gen for every open PR. Included only to get CI green — happy to split
  it into its own commit/PR if you'd prefer it separate.

Testing

• tsc --noEmit, client:gen + client:typecheck clean; committed client is in
  sync (git diff --exit-code -- clients/typescript).
• Full suite green (0 failures). New coverage: heartbeat unit tests (read vs
  write, quiet-vs-dead staleness, per-network isolation), the
  venue_last_write_timestamp gauge, /status exposure, stale-driven exclusion
  from /price (asserting excludedVenues and the NOT IN SQL), and the
  refresh worker dropping a stale venue from both the cached payload and the
  precomputed buckets.

Closes #219

[drips-wave]

@CHKM001 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[Miracle656]

This has gone conflicting — #250 landed a couple of hours ago and moved the ingest layer onto a single VenueAdapter contract, with scheduling and error handling owned by a runner in src/ingest/venues/. That is the ground your branch is standing on, so a rebase is needed before I can review it properly.

Worth knowing before you start, so the rebase is quick rather than a surprise: src/ingesters/amm.ts, sdex.ts and soroswap.ts now have counterparts under src/ingest/venues/, and the per-adapter polling and retry code has moved into runner.ts. If your change duplicates any of that, it should get smaller.

No rush and nothing wrong with the work — rebase on main and push, and I will pick it straight up.

[CHKM001]

DONE!

[CHKM001]

done

[Miracle656]

This needs a rebase, and that is the only thing wrong with it — so I want to be clear up front that I am not asking you to redesign anything.

The work itself is complete and wired

I checked the thing I check first on a feature like this, which is whether it is actually switched on. It is, everywhere it needs to be:

src/ingest/venues/sdex.ts       recordVenueSuccess / recordVenueFailure / recordVenueReadSuccess
src/ingest/venues/amm.ts        same
src/ingest/venues/soroswap.ts   same
src/ingest/venues/aquarius.ts   same
src/api/rest.ts:116             venueHeartbeats: getAllHeartbeats().filter(h => h.network === network)
src/api/graphql.ts:10           getStaleVenues
src/jobs/aggregateRefresh.ts:8  getStaleVenues

All four adapters report, /status exposes the heartbeats per network, and both the GraphQL layer and the aggregate refresh consult getStaleVenues() so a quiet venue is excluded from the price rather than silently contributing a stale number. That last part is what #219 is really about — "indistinguishable from a venue with no trades" is only fixed if something downstream acts on the distinction — and you did it. 495 lines of tests on a 226-line module is the right ratio for something that decides whether a venue counts toward a published price.

Why I am not hand-merging it

Nine of the twenty-one files conflict, and one of them is src/aggregator/vwap.ts. main has changed the shape of exactly the functions you extend, in 3d69182 and the feat/248-decimal-strings-in-api work behind it:

// your base
async function calculateVWAP(…, source?: 'SDEX' | 'AMM'): Promise<number>

// main now
async function calculateVWAP(…, source?: 'SDEX' | 'AMM'): Promise<{ num: number; raw: string }>

and getAggregatedPrice's single return type has been split into AggregatedPriceNumeric and AggregatedPriceString, because the API now serves exact decimal strings alongside numbers — prices were being rounded through IEEE-754 doubles on the way out.

Your change threads an excludedSources: string[] = [] parameter through those same functions. Combining the two is not a merge, it is re-threading your parameter through signatures that no longer exist, in the code that computes published prices. I am not willing to do that by hand on your behalf: a mistake there is a silently wrong price, not a failing test, and you know what you intended far better than I can infer it from a conflict.

The same applies to src/api/schemas.ts and src/api/rest.ts, where /status has grown status, health, process, data and adminConfigured blocks since you branched (#273, #508), and your venueHeartbeats addition lands in the middle of that.

What a rebase needs to account for

• calculateVWAP returns { num, raw }. Your excludedSources filter goes into the same SQL condition list, but every caller now destructures. Decide whether excluding a venue affects the raw string too (it should — same rows, same filter).
• getAggregatedPrice is split into numeric and string variants. excludedVenues belongs on both, or on neither.
• /status has new required fields. statusResponseSchema has additionalProperties: false, so venueHeartbeats has to be declared there (you do declare it — just re-apply against the current list).
• openapi.json, schema.graphql and the README tables are all CI-guarded generated artifacts now. After rebasing, run npm run openapi:gen, npm run readme:gen, and npx --yes openapi-typescript@7.13.0 openapi.yaml -o clients/typescript/src/schema.d.ts. Do not hand-edit openapi.json — the guard compares it byte-for-byte against the generator.
• #471 landed a per-cycle ingest deadline. ingestPair now takes an AbortSignal and adapters check it at write-unit boundaries. A cycle abandoned on its deadline should probably not count as a venue failure — it is our own timeout, not the venue going quiet — so recordVenueFailure on that path is worth a thought.
• console.* is rejected in non-test source by src/__tests__/logger.test.ts; use createLogger.

Ping me when it is rebased and I will review it properly — on the merits this is one of the better PRs open against the repo, and I would rather have it in than have it rot. If the rebase turns out to be painful, splitting the heartbeat module and its adapter wiring from the excludedSources plumbing into vwap.ts would give you two PRs that each land cleanly, and the first one has no conflicts at all.

[Miracle656]

Status update, and an apology for the moving target.

When I reviewed this a couple of days ago I asked for a rebase because the branch conflicted heavily with src/aggregator/vwap.ts. You rebased, and it merged cleanly this morning — I verified it: clean merge, tsc clean, 1,628 tests passing. It was ready.

It is not any more, and that is on the volume of work landing around it rather than on anything you did. Since this morning main has absorbed nine PRs, several of them in exactly the files you touch:

• feat: separate observation and source time (#240) #394 moved the ingest windows from timestamp to COALESCE(observed_at, timestamp), including inside the VWAP query
• fix: classify venues and report net vs gross volume totals #455 added the venue classification and the net/gross volume split
• feat: add volumes and spreads GraphQL queries with REST parity #613 extracted computeVolumes/computeSpreads for the GraphQL resolvers
• fix: distinguish failed venue fetches from empty venues in aggregate #612 touched the Aquarius adapter
• Refactor/ingest consolidation 225 #255 consolidated the ingesters under src/ingest/venues

Merging main now gives nine conflicts, four of them inside vwap.ts and 81 lines deep. The shape is this: your branch builds a dynamic conditions array so excluded venues drop out of the query, while main now has a fixed WHERE carrying the observed_at predicate:

-- yours
WHERE ${conditions.join('\n       AND ')}

-- main
WHERE pair_key = $1
  AND network = $2
  AND COALESCE(observed_at, timestamp) > NOW() - ($3 || ' minutes')::interval
  ${sourceFilter}

Both are right; reconciling them means deciding how venue exclusion composes with the observation-time predicate inside the published-price query. I am not going to guess at that on your behalf — it is the query behind every price Lens serves, and you designed the exclusion mechanism.

The work itself still looks good. I said so last time and nothing has changed: the heartbeat is recorded by all four adapters, /status exposes it per network, and both the GraphQL layer and the aggregate refresh consult getStaleVenues() so a quiet venue is excluded from the price rather than silently contributing a stale one. That last part is what actually closes #219.

A rebase now should be smaller than it sounds — most of the nine conflicts are generated artifacts (openapi.json, clients/typescript/src/schema.d.ts) that you should not hand-merge at all. Regenerate them instead:

npm run openapi:gen
npx --yes openapi-typescript@7.13.0 openapi.yaml -o clients/typescript/src/schema.d.ts
npm run readme:gen

That leaves vwap.ts and src/__tests__/aquariusIngester.test.ts as the only real work.

Two things to watch for while you are in there, both of which bit other PRs today:

• openapi.yaml duplicate keys. Two PRs today ended up with components/responses/BadRequest defined twice after merging. js-yaml rejects the whole file, openapi.json will not generate, and three unrelated-looking suites fail without naming the cause. Worth a grep -c '^ BadRequest:' openapi.yaml before you push.
• The observed_at predicate. If a query you touch still filters on bare timestamp, it is comparing chain time to our wall clock for some venues. feat: separate observation and source time (#240) #394 has the reasoning.

Ping me when it is up and I will prioritise it — this has been ready once already and I would rather it landed than kept losing races.

[Miracle656]

Scanned clean. A per-venue heartbeat with stale-price exclusion is worth having — a venue that stops updating but keeps returning its last price is worse than one that errors, because the staleness is invisible to every consumer downstream. Excluding on a heartbeat rather than trusting the row is the right shape.

The branch is 79 commits behind main, and merging it as-is would revert shipped work.

Against current main the diff removes roughly 6,000 lines from src/ that are not yours — files that landed after you branched, which git removes because your branch predates them. The largest are src/reconciliation/dispersion.ts (312, #448), src/__tests__/venueReconciliation.test.ts (385) and src/__tests__/observationAndSourceTime.test.ts (335, #240). Among the rest is src/routes/facilitator.ts, whose x402 spend ceiling landed this morning — reverting that one would be a silent regression on a money path.

None of that is a judgement on your code. Please rebase on current main and re-request.

Two things that moved and will matter: every documented path in openapi.yaml now carries a /v1 prefix (routes are still registered bare; rewriteUrl resolves the segment), and npx tsc --noEmit plus npx vitest run both pass on main right now — so anything they report after your rebase is genuinely yours.

One thing to check while rebasing: main now distinguishes observed_at from the legacy timestamp column (#240), and they are deliberately different clocks. A staleness check has to be explicit about which one it means — observed_at is when we saw the price, which is almost certainly what a heartbeat wants. There is a guard test, src/__tests__/windowBoundaries.test.ts, that reads the operator out of the SQL a route sends; worth a look as a model for asserting this kind of thing.

https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB