Skip to content

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

Description

@waterbro-8

Summary

Five small data-plane hygiene items found while auditing storage and indexing.
Each is individually minor; they are filed together because each is one or two
lines and none of them justifies its own issue. Split them up if triage prefers.

All references are main @ 7a194f1eba4167d54bd46cf84cdbe86e00532319.

1. embeddings_text has no uniqueness on the chunk key

0001_init.sql:101-107 declares (id, file_id, chunk_index, chunk_text, embedding) with only idx_embeddings_text_file (file_id) as an index, and
0007 adds provider. There is no UNIQUE (file_id, chunk_index).

The write path depends on it being effectively unique: indexer.go:652-655
issues DELETE FROM embeddings_text WHERE file_id = $1 (guarded by status != "partial" || hasReplacementText) and then batch-inserts (indexer.go:657-677).
So correctness rests entirely on that delete succeeding in the same transaction,
with no database-level backstop — and the guard means a partial re-index
deliberately skips it. embeddings_visual (0001:114-117) does have a
one-per-file guarantee, via file_id as primary
key — this is the same invariant, enforced for one modality and not the other.

Wanted: the constraint, or a comment explaining why the delete is sufficient.

2. memories.source_file_id is an unindexed foreign key

0008_agent_memories.sql:45: source_file_id uuid REFERENCES files(id) ON DELETE SET NULL. Every index on memories covers workspace_id, path,
kind, search_tsv, lower(content) (0008:88-101, 0010:112-118). None
covers source_file_id.

Consequence: ON DELETE SET NULL means every single-file delete takes a
RowExclusiveLock on memories and performs a sequential scan to find the rows
to null out. That is a lock-scaled problem on the largest table in the memory
plane, triggered by an ordinary operation.

Wanted: CREATE INDEX … ON memories (source_file_id) WHERE source_file_id IS NOT NULL, and a note on whether the same applies to created_by_user_id
(0008:14, also ON DELETE SET NULL).

3. memory_relations foreign keys have no referential action

0022_memory_relations.sql:7-9 declares workspace_id REFERENCES workspaces(id), source_id REFERENCES memories(id), target_id REFERENCES memories(id) — all default NO ACTION. Meanwhile memories.workspace_id is
ON DELETE CASCADE (0008:13).

A cascading workspace or user delete therefore reaches memories, deletes rows,
and then fails on any memory_relations edge touching them. Latent, not
triggered
: I found no account-deletion or workspace-hard-delete path in the Go
code, which is precisely why this is cheap now and expensive later.

Wanted: decide the intended cascade and state it — relations are described as
append-only, erased only by Forget — then make the schema agree with the
memories cascade so the two cannot diverge during a real delete.

4. Listing generations is N+1

indexgeneration/service.go:282-299: List queries builds with a LIMIT, then calls
listGenerations(ctx, s.pool, workspaceID, build.ID) at :295, once per row
inside the loop.

Wanted: one query joining generations to the builds, or a documented reason the
per-build query is preferable.

5. ListRelations has no keyset cursor

memory/relation.go:302-309 builds WHERE … ORDER BY r.created_at DESC, r.id LIMIT $n with no continuation token, while memory/list.go already implements
exactly the right shape: the decodedListCursor type at :40, its predicates
bound into the query at :101-106, and encodeListCursor/decodeListCursor at
:330/:344.

Wanted: reuse the existing cursor pattern rather than inventing a second one.
Whether relations need it at all depends on realistic per-workspace edge counts;
if they do not, say so in the code so the next reader does not file this again.

Scope boundary

No behavior change is requested beyond what each item names, and no retrieval
quality change. Item 3 in particular should not be resolved by weakening the
memories cascade.

Acceptance

  • Each of the five is either implemented, or closed with a written reason in
    this issue.
  • 1 and 2 ship as migrations with Down blocks and are verified against a
    populated database, not only an empty schema.
  • 1 is proven to have teeth: a test that inserts a duplicate
    (file_id, chunk_index) and expects rejection.
  • 5 reuses memory/list.go's cursor helpers rather than duplicating encode
    and decode.
  • EXPLAIN output for the ON DELETE SET NULL path is recorded before and
    after 2, so the claim about the scan is measured rather than assumed.

Evidence level

E2 — every item is source-level and each cited line was read directly. Item 2's
performance consequence was inferred from the DDL and index list, not measured.

Proposed triage

Applied on filing, per docs/maintainers/triage.md and the precedent in #135:
type:maintenance, area:server, evidence:e2-source, status:needs-triage.
Left to a maintainer: priority, and any change to the set below.

Raised from the data/index audit on 2026-09-08.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:serverGo API, CLI, MCP, storage, or server runtimeevidence:e2-sourceSource or log evidence identifies the likely causestatus:readyScope and acceptance criteria are ready for developmenttype:maintenanceMaintenance, tooling, refactoring, or repository work

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions