feat(server): impl doc gc - #15282
Merged
Merged
Conversation
github-actions
Bot
added
app:server
test
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughDocument cleanup adds native reconciliation and deferred deletion for missing documents, versioned downstream effects, storage job orchestration, and validation across runtime, search, Copilot, synchronization, and editor reference behavior. ChangesDocument cleanup and storage reconciliation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant StorageCron
participant StorageBlobJob
participant StorageRuntime
participant SearchIndexer
participant CopilotEmbedding
StorageCron->>StorageBlobJob: enqueue document cleanup execution
StorageBlobJob->>StorageRuntime: execute cleanup candidates
StorageRuntime-->>StorageBlobJob: return pending search and copilot effects
StorageBlobJob->>SearchIndexer: enqueue search reconciliation
StorageBlobJob->>CopilotEmbedding: enqueue Copilot reconciliation
SearchIndexer->>StorageBlobJob: acknowledge search effect
CopilotEmbedding->>StorageBlobJob: acknowledge Copilot effect
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
darkskygit
force-pushed
the
darksky/impl-doc-gc
branch
from
July 19, 2026 13:54
66db2ba to
705da77
Compare
Deploying blocksuite-docs with
|
| Latest commit: |
2916d3d
|
| Status: | ⚡️ Build in progress... |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/backend/server/src/core/storage/blob-job.ts (1)
284-324: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCleanup execution has no continuation, so backlogs drain one batch per invocation.
Unlike the sid-based sweeps and
executeBlobCleanupCandidatesByMarkedRuns(which re-enqueue to continue), this handler runs a single batch oflimit(default 100) candidates and returns. With only the 4AM daily trigger, a backlog larger thanlimitdrains at ~100/day, and thedocument_cleanup_marked >= 10_000health warning inrecordDocumentCleanupHealthcould stay latched for a long time. Consider re-enqueueing when a full batch was executed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/server/src/core/storage/blob-job.ts` around lines 284 - 324, Update executeDocumentCleanupCandidates to enqueue a continuation job when the current invocation processes a full limit-sized batch, preserving the existing effect processing and health recording. Reuse the same workspaceId, gracePeriodDays, and limit parameters and avoid re-enqueuing when fewer than limit candidates were executed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/backend/native/src/runtime/storage_runtime/document_cleanup.rs`:
- Around line 660-674: The completion and deletion check must run after
`commentObjectsDone` is set in the `process_comment_objects` flow. Reuse the
existing ack-path logic that evaluates all completion flags and deletes the
`document_cleanup_candidates` row, ensuring it also handles cases where
search/copilot acknowledgements arrived first.
- Around line 821-837: Rename the query result variable `updated` to `_updated`
in the document cleanup effect acknowledgment flow, or inline it into
`debug_assert_eq!`, so release builds do not report an unused variable while
preserving the row-count assertion.
In `@packages/backend/server/src/plugins/indexer/job.ts`:
- Around line 76-103: Guard reconcileDocumentCleanup with the existing
config.indexer.enabled check before reading documents or calling
service.indexDoc/service.deleteDoc. Return early when indexing is disabled,
while keeping the queue.add ackDocumentCleanupEffect path outside the guard so
cleanup effects are always acknowledged.
---
Nitpick comments:
In `@packages/backend/server/src/core/storage/blob-job.ts`:
- Around line 284-324: Update executeDocumentCleanupCandidates to enqueue a
continuation job when the current invocation processes a full limit-sized batch,
preserving the existing effect processing and health recording. Reuse the same
workspaceId, gracePeriodDays, and limit parameters and avoid re-enqueuing when
fewer than limit candidates were executed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96a5363d-cf17-48e9-93f7-9cd1ed5a935a
📒 Files selected for processing (30)
blocksuite/integration-test/src/__tests__/main/editor-semantics.spec.tspackages/backend/native/index.d.tspackages/backend/native/src/runtime/backend_runtime/doc_storage.rspackages/backend/native/src/runtime/backend_runtime/tests.rspackages/backend/native/src/runtime/error.rspackages/backend/native/src/runtime/migrations.rspackages/backend/native/src/runtime/sql/runtime_migrations.sqlpackages/backend/native/src/runtime/storage_runtime/blob_cleanup.rspackages/backend/native/src/runtime/storage_runtime/blob_reconciliation.rspackages/backend/native/src/runtime/storage_runtime/doc_blob_refs.rspackages/backend/native/src/runtime/storage_runtime/document_cleanup.rspackages/backend/native/src/runtime/storage_runtime/mod.rspackages/backend/native/src/runtime/types.rspackages/backend/server/src/__tests__/copilot/copilot.spec.tspackages/backend/server/src/__tests__/sync/gateway.spec.tspackages/backend/server/src/core/doc/adapters/workspace.tspackages/backend/server/src/core/storage-runtime/provider.tspackages/backend/server/src/core/storage/__tests__/blob-job.spec.tspackages/backend/server/src/core/storage/blob-job.tspackages/backend/server/src/core/utils/__tests__/blocksute.spec.tspackages/backend/server/src/core/utils/blocksuite.tspackages/backend/server/src/models/copilot-context.tspackages/backend/server/src/plugins/copilot/embedding/job.tspackages/backend/server/src/plugins/copilot/embedding/types.tspackages/backend/server/src/plugins/copilot/session.tspackages/backend/server/src/plugins/indexer/__tests__/job.spec.tspackages/backend/server/src/plugins/indexer/__tests__/service.spec.tspackages/backend/server/src/plugins/indexer/index.tspackages/backend/server/src/plugins/indexer/job.tspackages/backend/server/src/plugins/indexer/service.ts
💤 Files with no reviewable changes (4)
- packages/backend/server/src/plugins/indexer/service.ts
- packages/backend/server/src/plugins/indexer/tests/service.spec.ts
- packages/backend/native/src/runtime/backend_runtime/doc_storage.rs
- packages/backend/server/src/plugins/copilot/session.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/backend/server/src/plugins/indexer/job.ts (1)
76-103: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard
reconcileDocumentCleanupwhen the indexer is disabled.The
executeDocumentCleanupCandidatesalways enqueues this job. Ifconfig.indexer.enabledis false, this handler should skip loading the root document and updating the search state, but it must still enqueue theackDocumentCleanupEffectjob to ensure the cleanup effect is acknowledged.💡 Proposed fix
`@OnJob`('indexer.reconcileDocumentCleanup') async reconcileDocumentCleanup({ workspaceId, docId, cleanupVersion, }: Jobs['indexer.reconcileDocumentCleanup']) { - const root = await this.doc.getDoc(workspaceId, workspaceId); - if (!root) { - throw new Error(`workspace root ${workspaceId} not found`); - } - const live = readAllDocIdsFromWorkspaceSnapshot(root.bin, true).includes( - docId - ); - if (live) { - if (!(await this.doc.getDoc(workspaceId, docId))) { - throw new Error(`restored document ${workspaceId}/${docId} not found`); - } - await this.service.indexDoc(workspaceId, docId); - } else { - await this.service.deleteDoc(workspaceId, docId); + if (this.config.indexer.enabled) { + const root = await this.doc.getDoc(workspaceId, workspaceId); + if (!root) { + throw new Error(`workspace root ${workspaceId} not found`); + } + const live = readAllDocIdsFromWorkspaceSnapshot(root.bin, true).includes( + docId + ); + if (live) { + if (!(await this.doc.getDoc(workspaceId, docId))) { + throw new Error(`restored document ${workspaceId}/${docId} not found`); + } + await this.service.indexDoc(workspaceId, docId); + } else { + await this.service.deleteDoc(workspaceId, docId); + } } await this.queue.add('backendRuntime.ackDocumentCleanupEffect', { workspaceId, docId, cleanupVersion, effect: 'search', }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/server/src/plugins/indexer/job.ts` around lines 76 - 103, Update reconcileDocumentCleanup to check config.indexer.enabled before loading the workspace root or calling indexDoc/deleteDoc; when disabled, skip those operations and still enqueue backendRuntime.ackDocumentCleanupEffect with the existing workspaceId, docId, cleanupVersion, and search effect payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/backend/server/src/plugins/indexer/job.ts`:
- Around line 76-103: Update reconcileDocumentCleanup to check
config.indexer.enabled before loading the workspace root or calling
indexDoc/deleteDoc; when disabled, skip those operations and still enqueue
backendRuntime.ackDocumentCleanupEffect with the existing workspaceId, docId,
cleanupVersion, and search effect payload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 838a7c0d-40da-4b1c-86d0-2e81454bd6c5
📒 Files selected for processing (30)
blocksuite/integration-test/src/__tests__/main/editor-semantics.spec.tspackages/backend/native/index.d.tspackages/backend/native/src/runtime/backend_runtime/doc_storage.rspackages/backend/native/src/runtime/backend_runtime/tests.rspackages/backend/native/src/runtime/error.rspackages/backend/native/src/runtime/migrations.rspackages/backend/native/src/runtime/sql/runtime_migrations.sqlpackages/backend/native/src/runtime/storage_runtime/blob_cleanup.rspackages/backend/native/src/runtime/storage_runtime/blob_reconciliation.rspackages/backend/native/src/runtime/storage_runtime/doc_blob_refs.rspackages/backend/native/src/runtime/storage_runtime/document_cleanup.rspackages/backend/native/src/runtime/storage_runtime/mod.rspackages/backend/native/src/runtime/types.rspackages/backend/server/src/__tests__/copilot/copilot.spec.tspackages/backend/server/src/__tests__/sync/gateway.spec.tspackages/backend/server/src/core/doc/adapters/workspace.tspackages/backend/server/src/core/storage-runtime/provider.tspackages/backend/server/src/core/storage/__tests__/blob-job.spec.tspackages/backend/server/src/core/storage/blob-job.tspackages/backend/server/src/core/utils/__tests__/blocksute.spec.tspackages/backend/server/src/core/utils/blocksuite.tspackages/backend/server/src/models/copilot-context.tspackages/backend/server/src/plugins/copilot/embedding/job.tspackages/backend/server/src/plugins/copilot/embedding/types.tspackages/backend/server/src/plugins/copilot/session.tspackages/backend/server/src/plugins/indexer/__tests__/job.spec.tspackages/backend/server/src/plugins/indexer/__tests__/service.spec.tspackages/backend/server/src/plugins/indexer/index.tspackages/backend/server/src/plugins/indexer/job.tspackages/backend/server/src/plugins/indexer/service.ts
💤 Files with no reviewable changes (4)
- packages/backend/native/src/runtime/backend_runtime/doc_storage.rs
- packages/backend/server/src/plugins/indexer/tests/service.spec.ts
- packages/backend/server/src/plugins/copilot/session.ts
- packages/backend/server/src/plugins/indexer/service.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/backend/server/src/core/utils/blocksuite.ts
- packages/backend/native/src/runtime/error.rs
- packages/backend/server/src/core/doc/adapters/workspace.ts
- packages/backend/server/src/plugins/indexer/index.ts
- packages/backend/native/src/runtime/types.rs
- packages/backend/server/src/tests/sync/gateway.spec.ts
- blocksuite/integration-test/src/tests/main/editor-semantics.spec.ts
- packages/backend/native/src/runtime/storage_runtime/blob_reconciliation.rs
- packages/backend/server/src/models/copilot-context.ts
- packages/backend/native/src/runtime/backend_runtime/tests.rs
- packages/backend/server/src/tests/copilot/copilot.spec.ts
- packages/backend/native/src/runtime/migrations.rs
- packages/backend/native/src/runtime/storage_runtime/doc_blob_refs.rs
- packages/backend/server/src/core/storage-runtime/provider.ts
- packages/backend/native/src/runtime/sql/runtime_migrations.sql
- packages/backend/native/index.d.ts
- packages/backend/native/src/runtime/storage_runtime/mod.rs
- packages/backend/server/src/plugins/indexer/tests/job.spec.ts
- packages/backend/native/src/runtime/storage_runtime/blob_cleanup.rs
- packages/backend/server/src/core/storage/tests/blob-job.spec.ts
- packages/backend/server/src/plugins/copilot/embedding/job.ts
- packages/backend/server/src/core/storage/blob-job.ts
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## canary #15282 +/- ##
==========================================
+ Coverage 60.70% 61.01% +0.31%
==========================================
Files 3311 3311
Lines 191719 191909 +190
Branches 28257 28319 +62
==========================================
+ Hits 116379 117092 +713
+ Misses 71539 71031 -508
+ Partials 3801 3786 -15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 tasks
This was referenced Jul 31, 2026
This was referenced Aug 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Dependency Tree
This tree was auto-generated by Charcoal
Summary by CodeRabbit