Repository navigation
fix(search): skip row-count scans on cold search requests - #4760
emdash-autofix[bot] wants to merge 2 commits into
Conversation
Replaced per-isolate search row-count verification with missing/empty-index checks while preserving explicit full verification and interrupted-rebuild recovery. Added real SQLite regression coverage that reproduced both reported COUNT queries before the fix. Verified search integration tests, root build with sandbox resource overrides, core typecheck, formatting, and lint:quick (zero diagnostics). Independent follow-up review found all findings addressed. EmDash-Bot-Case: 1198246700-issue-6226228056 EmDash-Bot-Candidate: cand-1-34991f9790
emdash-autofix
Bot
added
the
bot:fix
EmDash bot: Ready for human reviewPlan Clarified Admin and token requirements, added canonical recovery documentation, and made the query-capture assertion non-vacuous. Docs build, 7 repair tests, formatting, and quick lint pass; independent review found no blockers.
Status
Next action: A maintainer will review and approve the pull request. Details
Required checks on the current head:
This status is generated from recorded evidence. It is not a review and contains no instructions. |
🦋 Changeset detectedLatest commit: b3dc7a5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
github-actions
Bot
added
the
query-count changed
Query-count snapshot changes40 routes changed, total Δ +41 queries. SQLite
D1
Comparing snapshot files between base and head. Updated automatically on each push. |
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://emdash-bot-issue-4675.try.emdashcms.com, https://emdash-bot-issue-4675-emdash-playground.emdash-cms.workers.dev (commit b3dc7a5)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
The approach is sound. Cold-request COUNT(*) scans are replaced by bounded existence probes (a missing table, an empty _docsize, and a short-circuiting EXISTS for live content). Explicit verification keeps its previous default behavior. Both reviewers found no implementation bug in the changed paths. Both flagged that the manual-repair guidance for partial indexes is incomplete: it does not say that POST /_emdash/api/search/rebuild requires the Admin-only search:manage permission. One reviewer also wants the recovery procedure added to the canonical docs. One test can also pass without capturing any queries.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
| "emdash": patch | ||
| --- | ||
|
|
||
| Reduces D1 rows read by search and autocomplete on cold Workers isolates by skipping full row-count verification of existing search indexes. Missing indexes and indexes left empty by an interrupted rebuild are still repaired automatically. Partially populated indexes are no longer detected automatically; to repair one, send an authenticated `POST /_emdash/api/search/rebuild` request with `X-EmDash-Request: 1` and a JSON body such as `{"collection":"articles"}`. |
There was a problem hiding this comment.
[needs fixing] The recovery steps say to send "an authenticated POST /_emdash/api/search/rebuild request". That route requires search:manage, which packages/auth/src/rbac.ts maps to Admin only. An Editor, or a token without that permission, gets a 403. Name the prerequisite, for example: "...as an Admin (or with a token that has search:manage), with the X-EmDash-Request: 1 header and a JSON body such as {"collection":"articles"}."
The other reviewer adds that this paragraph is the only documentation of the change to partial-index recovery. The canonical search sections in docs/src/content/docs/reference/rest-api.mdx do not explain the missing/empty-only health check or how to recover partial indexes. .changeset/README.md expects useful explanations and examples in canonical feature or upgrade docs too. Add the recovery procedure there and link it from this entry.
There was a problem hiding this comment.
Updated the changeset to require the Admin-only search:manage permission and, for Bearer tokens, the admin scope. Added a linked “Search index recovery” section to the REST API reference with the health-check limits, a complete rebuild request, and verification steps. The docs build passes.
| expect( | ||
| statements.filter((statement) => /CREATE|DROP|INSERT|count\s*\(/i.test(statement)), | ||
| ).toEqual([]); |
There was a problem hiding this comment.
[suggestion] This test only checks that no CREATE/DROP/INSERT/COUNT statements were captured. If the spy captured nothing, the filter would return [] and the test would still pass. The cold-runtime test above guards against this with expect(statements.length).toBeGreaterThan(0). Add the same assertion here.
| expect( | |
| statements.filter((statement) => /CREATE|DROP|INSERT|count\s*\(/i.test(statement)), | |
| ).toEqual([]); | |
| expect(statements.length).toBeGreaterThan(0); | |
| expect( | |
| statements.filter((statement) => /CREATE|DROP|INSERT|count\s*\(/i.test(statement)), | |
| ).toEqual([]); |
There was a problem hiding this comment.
Added the non-empty query-capture assertion. All 7 FTS repair tests pass.
@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-test
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-loader
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
upgrade-emdash
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-ai-search
@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: |
Addressed review feedback: documented Admin-only search:manage permission and Bearer admin scope in the changeset and canonical REST API recovery procedure; added a non-empty query-capture assertion. Verified docs build, all 7 FTS repair tests, formatting, and quick lint with zero diagnostics. Independent review found no blockers. EmDash-Bot-Case: 1198246700-issue-6226228056 EmDash-Bot-Candidate: cand-2-6f9fa078d4
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | b3dc7a5 | Nov 08 2026, 12:47 AM |
There was a problem hiding this comment.
The approach is sound. Cold-isolate health checks now use bounded existence probes (missing table, empty _docsize, short-circuiting EXISTS for live content) instead of two full COUNT(*) scans per collection. Explicit verifyAndRepairAll() and verifyAndRepairIndex() calls keep their default row-count checks. The trade-off is that partially populated indexes are no longer repaired automatically, and the PR states this openly. Both reviewers approved, and neither found any remaining issues. The earlier findings are resolved: the changeset now names the Admin-only search:manage permission and the Bearer-token admin scope, a linked "Search index recovery" section was added to the REST API reference, and the empty-collection test now asserts that queries were captured.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
emdash-autofix
Bot
added
the
triage/awaiting-reporter
What does this PR do?
Removes content and FTS
_docsizeCOUNT scans from the lazy health check on each new Workers isolate. Both search and autocomplete, including authenticated editors, now check for missing or empty indexes instead. The emptiness probe stops at the first FTS row; only if the index is empty does it probe live content using the existing deleted_at index. This keeps automatic recovery when an interrupted rebuild leaves a newly created table empty.Full row-count verification remains the default for explicit
FTSManager.verifyAndRepairAll()/verifyAndRepairIndex()calls. Non-empty, partially populated indexes are no longer automatically detected on requests: use the authenticatedPOST /_emdash/api/search/rebuildendpoint withX-EmDash-Request: 1and JSON{"collection":"articles"}to repair them. This trade-off removes the unbounded healthy-index scans without introducing persistent health-marker state. The changeset documents recovery.Adds a backwards-compatible
checkRowCountsoption and preserves singleton DB routing and single-flight error handling. Real SQLite tests cover repeated cold runtimes without COUNTs, empty-table recovery after an interrupted rebuild, truly empty collections without repeated rebuilds, missing-table recovery through both runtime and explicit verification, and explicit repair of non-empty drift. The cold-runtime regression failed before the fix with both reported COUNT statements.Type of change
Your connection to this change
Assigned by danielmlr.
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI assistance disclosure
Screenshots / test output
Screenshots: Not applicable; no UI changes.
pnpm build: passed with raised file-descriptor and Node heap limits (ulimit -n 65536,NODE_OPTIONS=--max-old-space-size=8192). Initial attempts hit sandbox resource limits.pnpm --filter emdash typecheck: passed.pnpm lint:quick: zero diagnostics.pnpm format: passed.pnpm --filter emdash exec vitest run tests/integration/search: passed (16 files; 70 tests passed, 4 skipped).Full workspace typecheck and type-aware lint are left to CI.
Closes #4675
A Maintainer (@danielmlr) assigned this issue to the EmDash bot.