fix(data): publish migration and cursor corrections (successor to #185)

[PeterGuy326]

Refs #178.

After #197 closed

#197 was superseded by #219 (0025 HNSW on main). This PR no longer stacks on codex/fix-pr-180. It is rebased onto current main and keeps only the #178 data-plane hygiene slice:

• migration 0026: text-chunk uniqueness, partial indexes on nullable memory FKs, cascade memory_relations with workspace deletion
• batched generation listings
• opaque cursor pagination for relation listings

Head: ac4a3c9 (3 cherry-picks: 5cbd05b → rename 0026 → cursor/populated verification). Lexical/HNSW files that were already on main were dropped.

Not claimed: live production latency, or that #197 merged.

[PeterGuy326]

Independent automated preflight: HOLD on deployment order at 746501415f7e7f56f09bcd21f349b5adb9a9cf24.

The source tree has migrations 1–23 and 26, without 24/25. The runner uses Goose without WithAllowMissing, so deploying this candidate before the lower numbered migrations makes their later introduction invalid. This is source-confirmed, not a newly executed PostgreSQL upgrade test. An enforced deployment sequence or coordinated, verified-unreleased numbering is required before this PR can be treated as independently deployable.

The actual cursor, populated uniqueness, and bounded-query fixes passed independent source/compile checks; the implementation worker's real PostgreSQL evidence remains separately attributed. Latest 20/20 successful checks do not prove the cross-PR upgrade sequence. Follow-up is assigned, with no migration-policy waiver or deployed-history rewrite authorized.

[sun-970]

Code Review

Overall code quality is good and test coverage is solid. Here are a few specific issues and suggestions:

---

1. [Medium] Migration 0026's DELETE deduplication may be slow on large datasets

DELETE FROM embeddings_text
 WHERE id NOT IN (
    SELECT DISTINCT ON (file_id, chunk_index) id
      FROM embeddings_text
     ORDER BY file_id, chunk_index, id
 );

NOT IN (SELECT ...) requires materializing the IDs of all rows to keep. If the embeddings_text table is large, this holds a table lock for an extended period. The PR description also acknowledges that "write path DELETEs all chunks before re-inserting, so duplicates should never exist in practice."

Suggestion: Add a pre-check to skip the DELETE if there are no duplicates:

DO $$
BEGIN
  IF EXISTS (
    SELECT 1 FROM embeddings_text
    GROUP BY file_id, chunk_index
    HAVING count(*) > 1
    LIMIT 1
  ) THEN
    DELETE FROM embeddings_text
     WHERE id NOT IN (
      SELECT DISTINCT ON (file_id, chunk_index) id
        FROM embeddings_text
       ORDER BY file_id, chunk_index, id
     );
  END IF;
END $$;

This way, on the vast majority of production tables that have no duplicates, the migration only needs a lightweight GROUP BY check to pass.

---

2. [Low] Two valuable comments were removed

These two comments were removed from relation.go:

// The anchor memory must exist, be visible under the caller's allowed
// paths, and not be forgotten — mirroring Get — so listing relations never
// leaks the existence of a memory the caller cannot read.

// BFS from target following supersedes/corrects edges in their forward
// direction: if we ever reach sourceID, there is a cycle.

The first explains the existence-leakage guard for the security check — this is critical context for understanding why the anchor check is necessary. The second explains the BFS strategy of wouldCycle. Both are "WHY is non-obvious" comments and should be preserved.

---

3. [Low] Three unrelated fixes bundled in one migration

The PR description says "each is a single DDL statement and none warrants its own schema version." However, Item 3 (the FK cascade change) is semantically the heaviest — it drops and recreates three foreign key constraints. If a deployed environment has manually adjusted these FKs (e.g., temporarily changed to DEFERRABLE), this migration would overwrite them.

Consider documenting this assumption in the PR description or as a migration comment.

---

4. [Nit] relationFilterHash JSON + SHA-256 approach

payload := relationFilterFingerprint{...}
encoded, _ := json.Marshal(payload)
sum := sha256.Sum256(encoded)

JSON serialization + SHA-256 for generating the filter fingerprint has the benefit of staying consistent with the cursor format in list.go, but JSON marshaling introduces some fragility (field ordering, zero-value handling, etc.). Since this is only for preventing cursor reuse across queries and not for security purposes, a simpler approach (e.g., direct string concatenation before hashing) could work. That said, the current approach is acceptable for consistency.

---

What's done well

• N+1 fix (service.go): Clean and straightforward — collects buildIDs first, then batch-queries in one go. The test verifies exactly 2 SQL statements.
• Migration tests (migrations_test.go): Drops the constraint inside a rollback-only transaction → inserts duplicates → replays the migration, verifying both deduplication and the unique constraint. The _test suffix check on the database name is also a good safety practice.
• Cursor pagination: The filter hash binding prevents cursor reuse across filters. The limit+1 pattern for determining whether there's a next page is the standard keyset pagination approach. Using id > as a tiebreaker for tied timestamps is correct (paired with ORDER BY created_at DESC, id ASC).
• Down migration: Correctly reverses all changes in proper order.

[waterbro-8]

Code review（draft，给作者的清单，非批准）

一、keyset 分页：形状是对的，而且做对了一件容易漏的事

我特意核了排序与游标谓词是否自洽，因为这是这类改动最常错的地方：

• 谓词 (r.created_at < $t OR (r.created_at = $t AND r.id > $id))，而 SQL 的排序是 ORDER BY r.created_at DESC, r.id（id 升序）。两者一致，不会出现"同一时间戳的行被跳过或重复"。这条我对照了 main 上的原实现与这条分支的实现，排序没有被改动，新增的谓词是配得上它的。
• 取 q.Limit+1 行来探测是否还有下一页，len(out) <= q.Limit 时不出 next_cursor。✓
• 游标取自裁剪后那一页的最后一行（result.Relations[len-1]），不是 out[q.Limit]——这点很容易写错，这里是对的。
• 游标绑定了过滤条件的哈希（workspace + memory + direction + relationType 四项参与 sha256）。这是我认为最值得记一笔的地方：它让一个游标无法被拿到另一组过滤条件下重放。很多 keyset 实现会漏掉这个约束，导致跨查询的游标串味。
• 非法游标走 ErrInvalidCursor → handler 映射成 400 invalid_cursor，不是 500。✓

二、0026 迁移：三条都是能站住的，而且写清楚了取舍

• Item 1（唯一性）：先按 DISTINCT ON (file_id, chunk_index) ... ORDER BY file_id, chunk_index, id 去重，再加 UNIQUE (file_id, chunk_index)。注释明说"保留 UUID 最小的那一行是为了确定性选择，UUID 顺序不等于插入顺序"——这句话很关键，它承认了去重规则是人为约定而非"保留原始那行"。我不认为这是缺陷（写入路径本来就是先删后插，理论上不会有重复），但这个诚实度值得保持。
• Item 2（部分索引）：idx_memories_source_file_id 与 idx_memories_created_by_user_id 都是 WHERE ... IS NOT NULL 的部分索引，正是 ON DELETE SET NULL 反查需要的形状，而且部分索引不会为空值浪费空间。顺带一提：source_file_id 这个索引正好也是我在 #210 上提出的那条守卫缺陷（只查 memories.path、不查 source_file_id）如果要修，所需要的底层支撑——两条 PR 在这点上是有协同的。
• Item 3（级联对齐）：memory_relations 的三个 FK 改为与 memories.workspace_id 一致的 ON DELETE CASCADE。理由写得很实在：memories 会被 workspace 删除连带清掉，而 memory_relations 不级联的话，删 workspace 会因为还残留引用它的边而失败。这是真的会咬人的不一致。
• Down 把三处 FK 还原成无级联、删掉索引、删掉约束，顺序与 Up 相反。✓

三、非阻塞，但建议补一句的两点

• 这次迁移在生产库上不是无感的，而且它碰的是最大的那张表。 Item 1 的 DELETE ... WHERE id NOT IN (...) 加 ADD CONSTRAINT UNIQUE，都要全表扫描 embeddings_text 并加锁（约束建立期间对写入是排他的）。这不是缺陷，但 docs/MIGRATION_SEQUENCE.md 目前只讲了顺序与严格性，没讲迁移窗口。考虑到 0024（重写 files + 建索引）、0025（在三个 embeddings_* 上建 HNSW）、0026（又是 embeddings_text）是连着跑的，建议在序列文档里补一句"这三次迁移会依次对最大的几张表加锁，请预留窗口"。
• Item 1 的 Down 是不保数据的（被去重删掉的行不会回来），迁移注释里没提。这是 down 迁移的常态，我不反对；只是既然注释对其他取舍都写得很细，这里补一句"down 不恢复被去重的行"会更完整。

四、接口变更的处理是完整的

ListRelations 的 Go 签名从 ([]Relation, error) 改成 (*ListRelationsResult, error)，涉及三处：api.go 的 MemoryService 接口、handlers_memory.go 的实际调用、以及 handlers_memory_test.go 里的 stub——三处都改了，没有留下编译不过的悬空实现。HTTP 层面是向后兼容的：relations 字段仍在，只是新增了可选的 next_cursor 与可选的 cursor 查询参数。

五、我没有声称验证的事

本机没有 PostgreSQL，我没有跑过 migrations_test.go、也没有跑过分页的集成测试。以上结论来自逐行读迁移、SQL 与分页逻辑，不是运行时观测。

另外需要点明：这条 PR 的 CI 绿不等于它可以先合。docs/MIGRATION_SEQUENCE.md 已经写明它是链上第 3 环（#195 HOLD behind #197），我同意这个约束——0026 声明的前驱是 schema 25，跳序部署会让 Goose 正确地在启动时失败。

（本次为独立评审：我不是本 PR 作者，也不是其 head 的推送者。draft 状态下我不投批准票。）

[waterbro-8]

Draft + dirty/conflicting. In the recorded order this waits on #197. Not merging drafts. Please rebase after #197 or close if superseded.

[xiaocui-big]

平台组 Review（崔泽生，assigned by @冯浩然）

一、总体评价

结构清晰、文档充分的 PR，解决 #178 五项数据面卫生中的三项（migration 0026），同时修复关系分页游标方向 bug、index generation listing N+1 查询。CI 22/22 全绿。PR body 明确标注"非独立可部署"和 HOLD behind #197。

二、Migration 0026 正确性

Item 1：embeddings_text 唯一性约束

• DISTINCT ON (file_id, chunk_index) ... ORDER BY file_id, chunk_index, id 保留 UUID 最小行，注释诚实说明"UUID 顺序不等于插入顺序，是确定性选择而非保留原始行" ✅
• DELETE + ADD CONSTRAINT 在同一事务内，约束失败则全回滚 ✅
• 关注：Down 不恢复被去重的行，建议补一句注释说明

Item 2：memories 部分索引

• WHERE source_file_id IS NOT NULL / WHERE created_by_user_id IS NOT NULL 正是 ON DELETE SET NULL 反查所需形状 ✅
• 无问题，三项中最干净

Item 3：memory_relations FK 级联对齐

• workspace 删除 ON DELETE CASCADE 到 memories，如果 relations 不级联会因残留边失败——修复正确 ✅
• Down 把三个 FK 还原为 NO ACTION，顺序正确 ✅

Up/Down 对称性：

Up	Down	对称
DELETE duplicates + ADD UNIQUE	DROP CONSTRAINT	部分（被删行不恢复）
CREATE INDEX x2 (partial)	DROP INDEX x2	✓
DROP + ADD FK (CASCADE) x3	DROP + ADD FK (NO ACTION) x3	✓

三、memory/relation.go 游标分页——核心 bug 修复正确

排序 ORDER BY r.created_at DESC, r.id（id 升序），游标谓词 (r.created_at < $t OR (r.created_at = $t AND r.id > $id))——DESC 排序需要"更小时间戳"或"同时间戳更大 UUID"，谓词一致 ✅

relationFilterFingerprint 将四个过滤维度 sha256 后嵌入游标，防止跨查询条件重放——少见的安全意识 ✅

集成测试验证了游标跨 filter 失效、同时间戳分页不重复不遗漏 ✅

被删除的注释：同意 sun-970 的判断——anchor memory 安全不变量说明和 wouldCycle BFS 策略说明是"WHY is non-obvious"注释，建议恢复。

四、indexgeneration/service.go N+1 修复——干净

listGenerationsForBuilds 用 build_id = ANY($2::uuid[]) 批量查询，集成测试通过 pgx Tracer 精确验证 List 只用 2 条 SQL（1 builds + 1 generations）✅

五、词法搜索路由（附加变更）

RouteLexical 使用与 Recall 相同的三层结构（精确短语 → FTS → trigram），不需要 worker。managed_embeddings.go 正确跳过 lexical 路由的 embedding provider 调用 ✅

strpos(lower(f.name), lower($N)) 无法利用索引，当前阶段可接受。

六、依赖链

main (schema 23) → #194 (0024) → #197 (0025) → #195 (0026)

Goose 严格模式，跳序部署会正确失败。三个 PR 连续对大表加锁，生产部署需预留维护窗口。

七、建议级问题

项	级别
恢复 relation.go 被删的安全注释	Medium
MIGRATION_SEQUENCE.md 补充迁移窗口提示	Medium
Item 1 去重加 IF EXISTS (duplicates) 前置检查	Low
Item 1 Down 注释补"不恢复被删行"	Low
Item 3 迁移注释加 FK 假设声明	Low

八、核心判断

代码质量高，变更正确，测试覆盖充分。依赖链管理严格，文档诚实。Draft 状态下不投批准票。等待 #197 先行解决、conflict 解决后，建议合并。

[waterbro-8]

崔泽生：质量过关，等 #197。同意部署顺序 #194(已合) → #197 → #195。

当前 DIRTY/CONFLICTING，rebase 需要作者 @PeterGuy326，且必须在 #197 的 0025 落地之后。

[waterbro-8]

#197 已关（被 #219 取代）。本 PR 已换基到 main，只保留 #178 的 0026 hygiene + relation cursor。

Head ac4a3c9。请审这一条独立切片，不要再当 HOLD behind #197。