Skip to content

Added smarterCounts labs flag for optimized pagination count queries#26709

Merged
kevinansfield merged 1 commit intomainfrom
worktree-count-performance
Mar 12, 2026
Merged

Added smarterCounts labs flag for optimized pagination count queries#26709
kevinansfield merged 1 commit intomainfrom
worktree-count-performance

Conversation

@kevinansfield
Copy link
Copy Markdown
Member

@kevinansfield kevinansfield commented Mar 4, 2026

Summary

  • Adds a smarterCounts private labs flag that automatically optimizes pagination count queries
  • When enabled, inspects the query builder for JOINs and uses COUNT(*) when safe (no JOINs), falling back to COUNT(DISTINCT table.id) when JOINs are present
  • The only model with JOINs is products (LEFT JOIN on stripe_prices) — all other models benefit from the faster COUNT(*)
  • During e2e API tests (1178 tests, all passing), 313 out of 727 count queries (43%) switched from COUNT(DISTINCT) to COUNT(*)

Tables that benefit (based on queries made during e2e-api tests)

Table Queries improved
posts 79
comments 71
newsletters 58
members 42
tags 13
users 10
actions, snippets, email_recipient_failures, invites, and others 40

Changes

  • ghost/core/core/shared/labs.js — Added smarterCounts to private features
  • ghost/core/core/server/models/base/plugins/crud.js — Sets useSmartCount option when labs flag is enabled
  • apps/admin-x-settings/.../private-features.tsx — Added labs UI toggle
  • ghost/core/test/e2e-api/content/posts.test.js — Fixed overly broad select * assertion that was catching count(*)
  • Snapshot updated for new labs flag

Companion PR

Framework: TryGhost/framework#434 (adds useSmartCount option to @tryghost/bookshelf-pagination)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 28e406d7-4ff2-4201-b680-0b9a4438fd28

📥 Commits

Reviewing files that changed from the base of the PR and between d871177 and b860e8e.

⛔ Files ignored due to path filters (3)
  • ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap is excluded by !**/*.snap
  • ghost/core/test/e2e-api/content/__snapshots__/posts.test.js.snap is excluded by !**/*.snap
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
  • ghost/core/core/server/models/base/plugins/crud.js
  • ghost/core/core/shared/labs.js
  • ghost/core/package.json
  • ghost/core/test/e2e-api/content/posts.test.js
🚧 Files skipped from review as they are similar to previous changes (4)
  • apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
  • ghost/core/core/server/models/base/plugins/crud.js
  • ghost/core/test/e2e-api/content/posts.test.js
  • ghost/core/core/shared/labs.js

Walkthrough

Adds a private lab feature smarterCounts (admin UI entry and inclusion in PRIVATE_FEATURES). The CRUD layer (findPage) reads labs.smarterCounts and sets options.useSmartCount = true when enabled. Updates ghost/core/package.json to bump @tryghost/bookshelf-plugins to 2.0.2. E2E tests relax assertions to permit queries that include count(*) alongside previous checks.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: adding a smarterCounts labs flag for optimized pagination count queries, which directly matches the primary objective of the pull request.
Description check ✅ Passed The description is directly related to the changeset, providing clear context about the smarterCounts feature, its benefits, affected tables, specific file changes, and companion PR details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch worktree-count-performance

Comment @coderabbitai help to get the list of available commands and usage tips.

@9larsons 9larsons self-requested a review March 5, 2026 13:39
@9larsons
Copy link
Copy Markdown
Contributor

@kevinansfield I've merged the framework PR and bumped the packages so they can be included here.

@kevinansfield kevinansfield force-pushed the worktree-count-performance branch from e53fc54 to 69b38a8 Compare March 11, 2026 19:23
@ErisDS
Copy link
Copy Markdown
Member

ErisDS commented Mar 11, 2026

🤖 Velo CI Failure Analysis

Classification: 🟠 SOFT FAIL

  • Workflow: CI
  • Failed Step: E2E tests
  • Run: View failed run
    What failed: Test assertion failed
    Why: The failure is caused by a code issue that needs to be fixed in this PR.
    Action:
    Check the error message and fix the issue in your code.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.18%. Comparing base (3ad6d52) to head (b860e8e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26709      +/-   ##
==========================================
- Coverage   73.19%   73.18%   -0.01%     
==========================================
  Files        1534     1534              
  Lines      121048   121054       +6     
  Branches    14636    14634       -2     
==========================================
- Hits        88601    88597       -4     
- Misses      31415    31425      +10     
  Partials     1032     1032              
Flag Coverage Δ
admin-tests 54.31% <ø> (ø)
e2e-tests 73.18% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ErisDS
Copy link
Copy Markdown
Member

ErisDS commented Mar 11, 2026

🤖 Velo CI Failure Analysis

Classification: 🟠 SOFT FAIL

  • Workflow: CI
  • Failed Step: Run yarn nx run @tryghost/admin-x-settings:test:acceptance
  • Run: View failed run
    What failed: CI failure - likely code issue
    Why: The failure appears to be related to code changes. Check the error output for details.
    Action:
    Review the error logs and fix the issue in your code.

@kevinansfield kevinansfield force-pushed the worktree-count-performance branch from 924df3c to 80f3323 Compare March 12, 2026 09:42
@kevinansfield kevinansfield marked this pull request as ready for review March 12, 2026 10:02
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ghost/core/core/server/models/base/plugins/crud.js`:
- Around line 147-150: Move the inline require of labs out of the findPage
function into module scope: add a top-level const labs =
require('../../../../shared/labs'); and remove the inline require inside
findPage; then keep the existing conditional that sets options.useSmartCount =
true when labs.isSet('smarterCounts') is true. Update any references in the file
to use the module-scoped labs variable (e.g., inside findPage) to avoid repeated
requires and match the pattern used in settings.js.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f98f02f5-9155-49a9-903b-4bcf82bde6b4

📥 Commits

Reviewing files that changed from the base of the PR and between 3369a6c and 80f3323.

⛔ Files ignored due to path filters (3)
  • ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap is excluded by !**/*.snap
  • ghost/core/test/e2e-api/content/__snapshots__/posts.test.js.snap is excluded by !**/*.snap
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx
  • ghost/core/core/server/models/base/plugins/crud.js
  • ghost/core/core/shared/labs.js
  • ghost/core/package.json
  • ghost/core/test/e2e-api/content/posts.test.js

Comment thread ghost/core/core/server/models/base/plugins/crud.js Outdated
@kevinansfield kevinansfield force-pushed the worktree-count-performance branch 2 times, most recently from d871177 to d1be93a Compare March 12, 2026 10:51
refs https://linear.app/ghost/issue/ENG-000
- Added `smarterCounts` private labs flag
- Updated `@tryghost/bookshelf-plugins` to 2.0.2 (includes bookshelf-pagination
  with useSmartCount support for count(*) on single-table queries)
- When enabled, uses count(*) instead of count(distinct id) for simple queries
@kevinansfield kevinansfield force-pushed the worktree-count-performance branch from d1be93a to b860e8e Compare March 12, 2026 10:54
@kevinansfield kevinansfield merged commit 1a31cdb into main Mar 12, 2026
32 checks passed
@kevinansfield kevinansfield deleted the worktree-count-performance branch March 12, 2026 13:55
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