Skip to content

fix(compaction): keep memory clusters within one scope - #1020

Merged
rwmjhb merged 1 commit into
CortexReach:masterfrom
igs-rogenlo:fix/compaction-scope-clusters
Nov 7, 2026
Merged

rwmjhb merged 1 commit into
CortexReach:masterfrom
igs-rogenlo:fix/compaction-scope-clusters

Conversation

@igs-rogenlo

Copy link
Copy Markdown
Contributor

Problem and behavior

An all-scope compaction pass clusters rows by vector similarity before checking their scopes. Two identical memories belonging to different scopes can therefore become one merged row under the first member's scope, after which both original rows are deleted. The registered compaction caller passes undefined for the scope filter, so this is a reachable maintenance path.

Group candidates by scope before calling buildClusters, and retain each plan's group-local members. Same-scope compaction, bounded cluster/delete concurrency, failure handling, and dry-run behavior are preserved. This does not migrate or repair previously compacted rows.

Validation

  • Baseline: upstream c0e2c7a3a8b5780b6133ddfd9e2f426bf0ea1b6c. The four added regressions produce 1 pass / 3 failures without the fix: cross-scope separation and the two-cluster real/dry-run expectations fail, while ordinary same-scope compaction passes.
  • Fixed: node --test test/compaction-scope-isolation.test.mjs test/memory-compactor.test.mjs — 34 passed, 0 failed.
  • npm run build and git diff --check pass; generated dist/src/memory-compactor.js is included.
  • Independent source review through P2 found no actionable issue. Test results are separate from that review.

The regressions execute the real compaction runner with a synthetic in-memory store and embedder. No production database, Gateway, or real model was used, and no historical-data repair or hosted-CI success is claimed.

Related: #923 handles different legacy scope/reflection ownership read paths; its changes do not group compaction clusters by scope. This patch addresses the cluster/write/delete boundary only.

@rwmjhb rwmjhb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 95e7d2e through the orchestrator. Approved: partitioning candidates by exact scope before clustering, and retaining each group's concrete members, prevents cross-scope merging and deletion.

Independent follow-up verification: all 34 tests in compaction-scope-isolation.test.mjs and memory-compactor.test.mjs passed; npm run build passed; regenerated dist/src/memory-compactor.js matched the committed artifact.

Please register the new scope-isolation regression in package.json and scripts/ci-test-manifest.mjs: the normal automated entry points currently omit it. Broad regression verification remains incomplete: npm test timed out after 600 seconds in the real LanceDB update rollback suite, with no compaction assertion failure shown. This approval is not a claim that the full suite passed.

@rwmjhb
rwmjhb merged commit b2493de into CortexReach:master Nov 7, 2026
1 check 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.

2 participants