fix(core): compute taxonomy counts on demand#2219
Conversation
🦋 Changeset detectedLatest commit: ea5df32 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Query-count snapshot changes4 routes changed, total Δ -4 queries. SQLite
D1
Comparing snapshot files between base and head. Updated automatically on each push. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
This is the right change for #2210: layout prefetch was burning D1 reads on full taxonomy-count aggregates that most pages never render. The split between term-list caching (CacheNamespace.TAXONOMIES) and count caching (termCountNamespaces(collections)) is clean, includeCounts defaults to true so direct getTaxonomyTerms callers keep the old behavior, and the Tags/Categories widgets now only pay for counts when showCount is on. Query-count snapshots move in the right direction.
I checked:
- The diff and the changed implementation in
packages/core/src/taxonomies/index.ts. - Callers of
getTaxonomyTermsandgetTermacross core, templates, and tests. - The prefetch path and widget components.
- The new regression test that asserts the aggregate query is skipped during prefetch and runs exactly once when counts are requested with no object-cache backend.
- AGENTS.md conventions: SQL is still builder-only, no new locale filters are needed, query counts only decrease, the changeset is present, and comments explain non-obvious reasons rather than narrating the PR.
No blocking or correctness issues found. The separation of term-list and count caches is sound, and the public API contract for existing callers is preserved.
MA2153
left a comment
There was a problem hiding this comment.
Looks good. If this has the chance of breaking existing flows, the changelog should begin with Breaking Change.
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
|
@logelog hi, does this PR includes AI generated code? If yes, could you edit the description please? Cheers! |
khoinguyenpham04
left a comment
There was a problem hiding this comment.
Thanks for fixing this @logelog ! Everything looks good but 1 tiny nit, so could we use a new/versioned cache key here, such as termList:? This avoids briefly returning legacy cached count values after an upgrade for taxonomies with no collections.
There was a problem hiding this comment.
This is the right fix for #2210: the layout prefetch was running expensive aggregate queries to compute visible taxonomy term counts on every render, even though most pages never display them. The split between a term-list cache (CacheNamespace.TAXONOMIES) and a separate count cache (termCountNamespaces(collections)), plus the new includeCounts option defaulting to true, makes the optimization transparent to existing callers.
I re-read the diff and re-traced the caching/invalidation paths:
getTermListandgetVisibleTermCountsare now independent: the prefetch warms only the term list, and widgets (Categories/Tags) request counts only whenshowCountis on.withCountsrebuilds a fresh term tree instead of mutating the cached count-free list, so both callers safely share the same term lookup.- Object-cache invalidation is consistent: term-only data depends on the
taxonomiesnamespace; count entries additionally depend on each collection's content namespace, which existing write paths already bump. - The new regression test asserts the aggregate is skipped during prefetch and runs exactly once when counts are requested without an object-cache backend.
- Query-count snapshots decrease as expected, and the changeset/docs/template updates are in scope.
No prior emdashbot[bot] findings were left open. I didn't find any new blocking correctness issues or AGENTS.md violations. The public API contract is preserved, and the logged-out query-count trend is downward.
What does this PR do?
Stops layout prefetch from recomputing visible taxonomy term counts when no consumer displays them.
Before this change, every HTML render called
getTaxonomyTerms()for every taxonomy, which aggregated the full content-term assignment table even when only term labels were being prefetched. Without an object-cache backend, those scans went directly to D1 on every request.This separates term-list caching from visible-count caching and adds an
includeCountsoption, defaulting totrueso existing consumers keep their behavior. Layout prefetch opts out, while widgets request counts only whenshowCountis enabled. Query-count snapshots and documentation are updated.A regression test reproduces two aggregate queries on current
main; this branch performs zero during layout prefetch and computes counts only for explicit consumers.Closes #2210
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (targeted taxonomy tests)pnpm formathas been runScreenshots / test output
pnpm --filter emdash exec vitest run tests/unit/taxonomies— 8 files, 68 tests passedpnpm --filter emdash typecheckpnpm lintpnpm formatpnpm --filter emdash buildpnpm query-counts --target sqlite --skip-buildpnpm query-counts --target d1