feat(api): enhance asset counting with fast count support - #203
Conversation
Added functionality to utilize an aggregate table for faster asset counting based on supported filters. Introduced a new method to determine if fast counting can be applied, and updated the `getRawAssetsAction` to prefer this method when applicable. Updated tests to ensure correct behavior when counting assets with various filters, ensuring that unnecessary joins are avoided for performance optimization.
|
Reviewed this against the release the v2 indexer fixes are cutting, to see whether it can ride along. The aggregate approach is sound and most of the filter parity holds, but three things block it and one needs a decision. Two count divergences from the same root cause. A comment narrates the change rather than the code. The line about the upstream commit having built it first and thrown the work away reads as history a year from now, and the git log already holds it. Test coverage does not reach the divergence. Ten of the fourteen allow-listed filters have no value-level test, and nothing compares the fast result against the raw result on one fixture, which is exactly the shape that would have caught both defects. Every fixture row is also an un-consolidated delta, so the consolidated shape production actually reads is never exercised. One decision, not a defect. The fast path makes the aggregate authoritative for a public count on every chain at once, with no runtime switch back. Drift is a known condition here, since Two smaller notes. The allow list includes Happy to take these on if you would rather not; say the word and I will push the fixes to this branch. |
- Add regression coverage for aggregate, fallback, and filtered counts - Document count=true listing responses
|
Updated to address consistency issues. SummaryValidated the PR-review fixes on branch
Validation results:
No asset-count regressions were found. Production-scale load and existing production aggregate accuracy were not tested. |
|
Reviewed against The eligibility gate is correctI enumerated every filter in The zero and null handling holds up too. Both paths read the same The integration suite is genuine parity rather than existence: it compares the fast count against the raw count and the listing length across roughly fifty filter combinations in the dirty, consolidated and mixed aggregate states, and CI runs it against a real database. Blocking: the default makes an unvalidated table authoritative
This repository already ships With the fast path on, drift stops being a stats problem and becomes a wrong number served to every caller of a public endpoint, with no error, no log line, and no way to notice. The remedy is a config edit plus a restart, which requires first suspecting the count. Gate on Blocking: no CHANGELOG entry
Worth fixing, not blocking
The comment at the top of UnmeasuredThe No migration is needed, and no response shape changes: both paths return a bigint as a decimal string. |
The fast path makes atomicassets_asset_counts authoritative for a public count, and that table has drifted before: recount-asset-counts.ts exists to repair it, and 1.3.27 added a unique index to stop a restore or an aggregation race duplicating its rows. The trigger and the aggregation job keep the sum exact for ordinary writes and for fork rollback, so drift enters through a restore, an import, a trigger disabled during maintenance, or a bug. Those paths leave no signal, and a total wrong that way is served to every caller with no error and no log line. Defaulting on meant an upgrade switched the source of that count with nobody choosing it, and the remedy, a config edit and a restart, needs the operator to suspect the count first. Defaulting off makes it a decision they can validate against the raw count on their own data. The tests that exercise the fast path now ask for it, and the case that covers the unset setting asserts it counts rows, so the default is pinned rather than assumed. The release carries a feature and a new configuration key, so it becomes a minor version. Signed-off-by: Rob Konsdorf <rob@facings.io>
|
I pushed the changes from my review above to this branch, since maintainer edits are enabled, so it can go into the 2.4.0 release. Four changes, and only the first is a behaviour change. The fast path is now opt-in. The CHANGELOG entry Two test changes from the review. Tests that exercise the fast path now ask for it rather than relying on the default, and the unset case asserts raw counting, so the default is pinned rather than assumed. Full suite is 1205 passing, 4 pending, 0 failing, against a database rebuilt from scratch on Postgres 14. Lint and types pass. To be clear about what I did not change: the eligibility gate itself. I enumerated every filter the endpoint accepts and each is either applied to the aggregate or forced to fall back, and because One thing still unmeasured: the |
There was a problem hiding this comment.
⚪ Unable to assess
Pull request overview
Adds aggregate-backed asset counting with configurable fallback to raw row counts.
Changes:
- Implements fast counts for supported asset filters.
- Adds namespace configuration and count-response handling.
- Adds unit/integration coverage and operator documentation.
File summaries
| File | Description |
|---|---|
src/api/namespaces/atomicmarket/index.ts |
Adds AtomicMarket configuration. |
src/api/namespaces/atomicassets/routes/assets.ts |
Returns count strings without asset hydration. |
src/api/namespaces/atomicassets/routes/assets.test.ts |
Tests listing count responses. |
src/api/namespaces/atomicassets/index.ts |
Adds AtomicAssets configuration. |
src/api/namespaces/atomicassets/handlers/assets.ts |
Implements aggregate counting and fallback logic. |
src/api/namespaces/atomicassets/handlers/assets.integration.test.ts |
Extends count integration coverage. |
src/api/namespaces/atomicassets/handlers/assets-join.test.ts |
Tests count source and join selection. |
src/api/namespaces/atomicassets/handlers/assets-count.integration.test.ts |
Tests aggregate/raw parity and edge cases. |
src/api/namespaces/atomicassets/fast-count-config.test.ts |
Tests configuration validation. |
README.md |
Documents fast-count operation. |
config/server.config.example.json |
Adds example configuration values. |
CHANGELOG.md |
Records the feature release. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // every caller with no error and no log line. Defaulting off makes taking | ||
| // the fast path a decision an operator can validate against the raw count | ||
| // on their own data first. | ||
| if (args.count && ctx.coreArgs.enable_fast_asset_counts === true && !options?.extraTables) { |
|
The main thing this aims to fix is https://wax.api.atomicassets.io/atomicassets/v1/assets/_count timeouts |
Eligible asset counts now sum the existing
atomicassets_asset_countstotals instead of counting asset rows. The fast path runs before listing-query construction and template preselection, and joins templates only for filters that read template columns.Supported filters cover collection/schema/template, burned state, authorized collection accounts, collection and template whitelists/blacklists, transferable/burnable flags, and template name match/search. Name searches read both immutable and mutable template data, preserving the behavior merged from main. Counts remain decimal strings, independent of pagination and sorting.
Numeric zero in a template ID or template whitelist selects raw counting after list expansion, including mixed lists, named lists, and alternate spellings such as
00and-0.template_id=nullkeeps selecting template-less assets. Owner, IDs, data filters, bounds, other unsupported keys, and non-empty market price joins also use raw counting.Both
atomicassetsandatomicmarketnamespace args acceptenable_fast_asset_counts, defaulting to true. Operators can set it to false and restart the API to restore raw count authority if aggregate totals have drifted, without rebuilding the image. Cached responses remain until their configured lifetime expires; raw counts can be slower. The change adds no migration or automatic recount.Validation on the working tree after merging main at
5c163f46:Local tests used Node 24.18.0 and disposable PostgreSQL instances. CI's Node 22 runtime was not exercised locally. No production load or deployment testing was performed.