Skip to content

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

Open

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

  1. added
    type:maintenanceMaintenance, tooling, refactoring, or repository work
    evidence:e2-sourceSource or log evidence identifies the likely cause
    area:serverGo API, CLI, MCP, storage, or server runtime
    on Sep 8, 2026
  2. xiaocui-big commented on Sep 15, 2026

    @xiaocui-big

    #195 (codex/fix-pr-185) already covers all five items from this issue with 0026_data_plane_hygiene.sql plus the corresponding Go changes (N+1 fix, ListRelations cursor). I opened a duplicate PR (#208) and have closed it in favor of #195.

    Tracking #195 for resolution.

  3. waterbro-8 commented on Sep 17, 2026

    @waterbro-8
    CollaboratorAuthor

    Still open. Implementation candidate is draft #195 (codex/fix-pr-185), which @xiaocui-big already identified as covering all five hygiene items. I am not opening a duplicate.

    This issue is still status:needs-triage. Per AGENTS.md I am not merging or closing it.

  4. waterbro-8 commented on Sep 20, 2026

    @waterbro-8
    CollaboratorAuthor

    平台组群成员派活(冯浩然 / waterbro-8)

  5. PeterGuy326 commented on Sep 22, 2026

    @PeterGuy326
    Collaborator

    Additional #178 item 2 evidence is prepared for existing PR #195 at ac4a3c9512ec9066ce034b138c4f7b6bd0eb5182: on PostgreSQL 17.10 with 100,000 synthetic memories, actual file/user DELETE triggers changed both requested FK lookups from Seq Scan to their corresponding partial Index Scan after 0026. Each matched 100 rows; populated migration, SET NULL/rollback, deduplication, and duplicate rejection (23505) checks passed. The accompanying PR comment contains reproducible SQL and four plan excerpts.

    This is local PG17 evidence, not production/PG16 performance evidence. Full user deletion still has a separate forgotten_by_user_id scan outside this slice. A maintainer must still complete the readiness decision; this evidence does not change status:needs-triage or override the pending review. Continue with #195; no duplicate implementation or wait for superseded #197 is needed.

  6. waterbro-8 commented on Sep 23, 2026

    @waterbro-8
    CollaboratorAuthor

    建议:可以转为 status:ready(不改标签,等 maintainer/产品确认)

    对照本 issue 的五项与 Acceptance,对照 PR #195 exact head ac4a3c9512ec9066ce034b138c4f7b6bd0eb5182。建议 maintainer 将本 issue 从 status:needs-triage 改为 status:ready。我不会自行改标签、不会在 ready 确认前 rebase/合入。

    五项覆盖

    # 项 #195 现状
    1 embeddings_text UNIQUE (file_id, chunk_index) 0026_data_plane_hygiene.sql 先防御性去重再加约束;集成测试插入重复键期望 23505
    2 memories.source_file_id(及 created_by_user_id)部分索引 0026 两条 WHERE … IS NOT NULL 部分索引;10 万行 populated EXPLAIN 已记录
    3 memory_relations FK 与 memories 级联对齐 0026 改为 ON DELETE CASCADE,未削弱 memories 自身 cascade
    4 indexgeneration List N+1 一次 batch-fetch generations,不再 per-build 查询
    5 ListRelations keyset cursor 复用 memory/list.go 的 encode/decode;谓词与 ORDER BY created_at DESC, id ASC 一致

    PR 文件仅上述切片 + CHANGELOG / scripts/verify.sh / 对应测试,没有叠 recall benchmark 或其他 issue。

    Acceptance 对照

    • 五项均已实现,无需在本 issue 写“不做”的理由。
    • 1 和 2 以带 Down 的 migration 交付;PeterGuy326 在 PG17.10、10 万行合成 memories 上跑了 populated 校验(去重、SET NULL/rollback、duplicate 23505)。
    • item 2 的 before/after EXPLAIN 已贴在 PR #195 评论:父 DELETE 触发的两条 FK UPDATE 从 Seq Scan 变为对应 partial Index Scan(各匹配 100 行)。这是本地 PG17 证据,不是生产/PG16 时延证据。
    • forgotten_by_user_id 删除路径仍是 Seq Scan,明确不在本切片。
    • item 5 复用现有 cursor helpers,未另造编解码。

    治理与下一步(ready 确认之后才做)

    当前阻塞只剩治理:issue 仍是 status:needs-triage。按 AGENTS.md,material change 在 status:ready 之前不应推进合入。#195 仍 BEHIND main、CHANGES_REQUESTED(waterbro-8,原因即 triage,不是实现形状)。xiaocui-big 已在同一 head 上 Approve。

    请 @PeterGuy326 / 产品 maintainer 拍板是否改 status:ready。确认后我再:rebase 当前 main → 跑完整 hosted CI → 请求 fresh exact-head review。在此之前不 rebase、不合入。

  7. added
    status:readyScope and acceptance criteria are ready for development
    and removed on Sep 24, 2026
  8. waterbro-8 commented on Sep 24, 2026

    @waterbro-8
    CollaboratorAuthor

    Readiness decision: move to status:ready for the bounded five-item hygiene slice implemented by PR #195. The issue has explicit acceptance criteria; the current candidate maps all five items, includes migration/down coverage and cursor/N+1 tests, and the populated 100k-row PostgreSQL evidence records both requested FK lookup plan changes plus the remaining out-of-scope forgotten_by_user_id scan. This is authorization to integrate and verify the scoped change, not production-performance acceptance. Next gate remains latest-main integration, complete hosted CI, independent exact-head approval, then squash merge.

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