Skip to content

feat(api): enhance asset counting with fast count support - #203

Merged
robrigo merged 6 commits into
atomicassets:mainfrom
includenull:fix-asset-count-timeouts
Sep 10, 2026
Merged

feat(api): enhance asset counting with fast count support#203
robrigo merged 6 commits into
atomicassets:mainfrom
includenull:fix-asset-count-timeouts

Conversation

@includenull

@includenull includenull commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Eligible asset counts now sum the existing atomicassets_asset_counts totals 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 00 and -0. template_id=null keeps selecting template-less assets. Owner, IDs, data filters, bounds, other unsupported keys, and non-empty market price joins also use raw counting.

Both atomicassets and atomicmarket namespace args accept enable_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:

  • Lint, TypeScript checking, and SWC build pass.
  • Unit suite: 553 passed; 3 full-config metrics tests skipped.
  • Integration suite on PostgreSQL 14/15: 615 passed, 1 expected version-gated role test skipped per version. PostgreSQL 16/17/18: 616 passed per version.
  • 123 focused tests pass. New tests first reproduced 30 failures before the fixes, including numeric zero, mutable template names, and operator-switch behavior.
  • 240 healthy-fixture comparisons against current main show no count or listing differences, across dirty, consolidated, and mixed aggregate states. Additional drift tests confirm that disabling fast counts restores raw results through both namespaces.
  • Release-note script tests: 12/12 pass.

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.

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.
@includenull
includenull marked this pull request as draft August 31, 2026 04:58
@robrigo

robrigo commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

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. atomicassets_assets.template_id is NULL for a template-less asset, while the aggregate stores 0 through COALESCE(NEW.template_id, 0). template_blacklist maps that sentinel with a CASE; template_id and template_whitelist do not. So ?template_id=0&count=true counts every template-less asset on the chain while the listing for the same parameter returns an empty array, and template_whitelist=0 diverges the same way. Pagination built on that count walks empty pages. The fix is either to refuse the fast path when the value contains 0, or to map a supplied 0 to the sentinel the way the blacklist branch already does.

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 src/scripts/recount-asset-counts.ts exists because stale counts were baked in once already. A drifted chain would answer wrong with no error and no log, and an operator would have to build and roll an image to get the raw count back. A config flag defaulting to on would make that a setting rather than a deploy.

Two smaller notes. The allow list includes template_blacklist and template_whitelist, which the description does not mention and which are the least covered by tests. And there is no CHANGELOG entry, which the release script needs at tag time.

Happy to take these on if you would rather not; say the word and I will push the fixes to this branch.

@includenull

Copy link
Copy Markdown
Contributor Author

Updated to address consistency issues.

Summary

Validated the PR-review fixes on branch 6fa8babb against main 5c163f46.

Area Coverage and result
Template ID 0 Zero, alternate spellings, mixed lists, and named whitelists now preserve main’s count behavior.
Filter parity Counts matched main for collection/schema/template filters, whitelists/blacklists, burned state, permissions, names, asset filters, sorting, and pagination.
Aggregate states Empty, dirty, consolidated, and mixed records remained correct after mint, transfer, burn, and delete operations.
Operator fallback Default/enabled/disabled settings were tested in both namespaces. Disabling fast counts restored raw results after deliberately injected drift.
Listing count=true Reproduced main’s failure and confirmed the branch returns the correct decimal count. Normal formatted listings remained unchanged.

Validation results:

  • 42,240 comparison checks passed across PostgreSQL 14–18, using Node 22.
  • 131 focused tests passed per PostgreSQL version.
  • Full suites on PostgreSQL 16: 1,117 passed on main; 1,181 passed on the branch. Both skipped the same three metrics tests requiring full configuration.
  • Lint, TypeScript checking, and build passed.
  • README/configuration examples were updated, and the historical implementation comment was rewritten.

No asset-count regressions were found. Production-scale load and existing production aggregate accuracy were not tested.

@robrigo

robrigo commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Reviewed against main while assembling the 2.3.4 release. This change is not riding that release. Two things block it, and neither is the part that was hardest to get right.

The eligibility gate is correct

I enumerated every filter in openapi.ts, every boundary parameter in buildBoundaryFilter, and every filter in buildAssetFilter, buildAssetQueryCondition, buildGreylistFilter and buildHideOffersFilter. Each one is either applied to the aggregate or absent from fastCountSupportedKeys and therefore forced to raw counting. Because canUseFastCount is a closed allow-list over Object.keys(values), a filter added later falls back by construction rather than returning a silently wrong total. That is the right shape.

The zero and null handling holds up too. Both paths read the same filterQueryArgs output after $list: expansion, validateId normalizes NULL to null while preserving 0, 00 and -0 as strings that /^-?0+$/ catches, and isWeakInt rejects +0 and whitespace on both paths. template_blacklist needs no zero test, because CASE WHEN ac.template_id = 0 THEN 9223372036854775807 and COALESCE(asset.template_id, 9223372036854775807) agree. template_id=null matches because the 1.3.15 trigger writes COALESCE(template_id, 0).

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

enable_fast_asset_counts defaults to true, so upgrading switches every operator's /v1/assets/_count from a live count to a sum over atomicassets_asset_counts without anyone choosing it.

This repository already ships src/scripts/recount-asset-counts.ts, whose own header says stale counts were baked into that table, and migration 1.3.27 adds a unique index to stop duplicate stats from a restore or an aggregation race. So drift has happened at least twice. The trigger and the aggregation job keep the sum exact for ordinary writes and for fork rollback, which replays inverse row operations through the same trigger. Drift enters through the paths outside that loop: a restore, an import, a trigger disabled during maintenance, a bug. Those paths leave no signal.

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 enable_fast_asset_counts === true and say in the README and the config example that the fast path is opt-in. Then an operator can compare the two counts on their own data before taking it.

Blocking: no CHANGELOG entry

RELEASING.md puts the entry in the feature PR, and scripts/release-notes.sh composes the Release body from it. The change adds a config key and moves the source of a public count, so it needs a bullet naming enable_fast_asset_counts, its default, the supported filters and the fallback.

Worth fixing, not blocking

assets-count.integration.test.ts pins the fallback for many filters but not for match_immutable_name and match_mutable_name. Those read asset data the aggregate cannot express at all, and their names differ from the supported match and search only by a suffix, so a later change that adds them to the allow-list by analogy would count every asset in the selected groups with no test going red.

The comment at the top of assets-join.test.ts names the wrong file and script. The parity suite is in assets-count.integration.test.ts under pnpm test:integration:ci, not routes/assets.test.ts under pnpm test:e2e:ci.

Unmeasured

The match and search fast path joins the aggregate table to templates rather than preselecting templates in a CTE, which is what addTemplateFilter does deliberately for the trigram index. The plan numbers in the comment cover only the no-join case. An EXPLAIN of a match count on staging would close that.

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>
@robrigo
robrigo marked this pull request as ready for review September 10, 2026 21:19
@robrigo
robrigo requested a balanced review from Copilot September 10, 2026 21:19
@robrigo

robrigo commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

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. enable_fast_asset_counts defaults to false and the gate tests === true. The reasoning is in the commit and in a comment above the gate: the fast path makes atomicassets_asset_counts authoritative for a public count, and that table has drifted before, which is why recount-asset-counts.ts exists and why 1.3.27 added a unique index. Drift enters through a restore, an import, a trigger disabled during maintenance or a bug, none of which leave a signal, and a total wrong that way is served to every caller with no error. Defaulting on meant an upgrade switched the source of that count with nobody choosing it. The README and the config example now describe it as opt-in and say to compare the two counts before enabling.

The CHANGELOG entry RELEASING.md asks for is added, under ### Features.

Two test changes from the review. match_immutable_name and match_mutable_name are pinned to the raw-count fallback list, because they read asset data the aggregate cannot express and sit a suffix away from the supported match and search. And the header comment in assets-join.test.ts named the wrong file and script for the parity suite.

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 canUseFastCount is a closed allow-list a filter added later falls back by construction. The zero and null handling matches the raw path. That part was right.

One thing still unmeasured: the match and search fast path joins the aggregate to templates rather than preselecting templates in a CTE the way addTemplateFilter does for the trigram index, and the plan numbers in the comment cover only the no-join case. An EXPLAIN of a match count on staging would close it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ 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) {
@includenull

Copy link
Copy Markdown
Contributor Author

The main thing this aims to fix is https://wax.api.atomicassets.io/atomicassets/v1/assets/_count timeouts

@robrigo
robrigo merged commit 27d36e4 into atomicassets:main Sep 10, 2026
7 checks passed
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.

3 participants