Skip to content

chore(data): resolve five data-plane hygiene items from index audit (#178) - #185

Closed
sun-970 wants to merge 2 commits into
bytefolk:mainfrom
sun-970:chore/data-plane-hygiene-178
Closed

sun-970 wants to merge 2 commits into
bytefolk:mainfrom
sun-970:chore/data-plane-hygiene-178

Conversation

@sun-970

@sun-970 sun-970 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Resolves all five items from #178:

  1. embeddings_text uniqueness — migration 0026 adds UNIQUE (file_id, chunk_index) with defensive deduplication before the constraint. Integration test proves the constraint rejects duplicates.
  2. memories unindexed FKs — partial indexes on source_file_id and created_by_user_id (WHERE IS NOT NULL) to avoid seq scans on ON DELETE SET NULL.
  3. memory_relations cascade alignment — FKs now ON DELETE CASCADE to match memories.workspace_id, preventing orphan-edge failures on workspace delete.
  4. N+1 in indexgeneration.List — batch-fetches generations for all builds in one query (WHERE build_id = ANY($1)) instead of per-build.
  5. Keyset cursor for ListRelations — reuses encodeListCursor/decodeListCursor from memory/list.go with a relation-specific filter hash.

Changes

  • server/internal/db/migrations/0026_data_plane_hygiene.sql — schema fixes with Down blocks
  • server/internal/db/migrations_test.go — integration test for uniqueness constraint
  • server/internal/indexgeneration/service.go + store.go — N+1 fix
  • server/internal/memory/relation.go — cursor support
  • server/internal/api/ — handler, interface, and test stub updates
  • server/internal/memory/memory_integration_test.go — caller updates

Dependencies

This PR was previously stacked on #184. It has been rebased onto main and is now independent.

Test plan

  • Migration 0026 applies cleanly on a populated database (not only empty schema)
  • TestEmbeddingsTextUniqueConstraint passes with MEM_TEST_DB set
  • EXPLAIN for ON DELETE SET NULL path uses the new partial index (item 2)
  • ListRelations returns next_cursor when results exceed limit
  • indexgeneration.List returns correct generations per build (no N+1)
  • Existing integration tests pass

Closes #178

@PeterGuy326 PeterGuy326 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.

Gate decision

REQUEST CHANGES on current head 8f08cf6e57685fce5f21be8ac96ddbcfdbd8332f.

Blocking findings

  1. The branch is stacked on #184: git log origin/main..origin/pr-185 has 89bb12b as its parent, and the diff therefore includes the live recall producer files. The PR body presents this as the five-item data-plane hygiene change but does not declare the dependency. Rebase/split it, or explicitly document and enforce the merge order so the same files are not merged twice.
  2. The acceptance table leaves the migration, EXPLAIN, relation cursor, generation-list, and existing-integration checks unchecked; no checks are reported for this head. Please run and attach the exact-head CI/integration evidence.

Do not treat the unit-level changes as complete until the stacked scope and database acceptance are resolved.

…ytefolk#178)

1. Add UNIQUE (file_id, chunk_index) on embeddings_text with defensive
   deduplication before the constraint.
2. Add partial indexes on memories(source_file_id) and
   memories(created_by_user_id) to avoid seq scans on ON DELETE SET NULL.
3. Add ON DELETE CASCADE to memory_relations FKs to align with the
   memories cascade and prevent orphan-edge failures on workspace delete.
4. Fix N+1 in indexgeneration List: batch-fetch generations for all
   builds in one query instead of per-build.
5. Add keyset cursor to ListRelations reusing the existing
   encodeListCursor/decodeListCursor helpers from memory/list.go.

Includes migration 0025 with Down blocks and an integration test proving
the embeddings_text uniqueness constraint rejects duplicates.
- Remove bytefolk#184 stacking (recall benchmark is a separate PR)
- Rename migration 0025 → 0026 to avoid collision with bytefolk#180
@sun-970
sun-970 force-pushed the chore/data-plane-hygiene-178 branch from 8f08cf6 to 3104d35 Compare September 10, 2026 01:51
@PeterGuy326

Copy link
Copy Markdown
Collaborator

Current-head CI approval readback (2026-09-10)

Head: 3104d35ceaf877e9ddf27ca570b87c475645e50c. The original pending-approval runs on this head have been approved and real check-runs now exist (18 at this snapshot). No old-head run was approved. Auto-merge remains disabled; this is not a code approval or merge-ready declaration.

Failures at this snapshot:

@sun-970

sun-970 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by @PeterGuy326's #195 which covers the same changes on current main. Closing per the '本轮先不合并' directive.

@sun-970 sun-970 closed this Sep 10, 2026
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.

chore(data): five data-plane hygiene items from the index audit

3 participants