Address review: improve comments, naming, add edge case tests

- Rename tip -> cursor for consistency with backward walk
- Fix misleading test descriptions to reflect bidirectional behavior
- Add test for circular parent references (cycle protection)
- Add test for branching children (first-child-wins behavior)
- Document linear chain assumption in forward walk comment
This commit is contained in:
Vorflux AI 2026-03-27 02:21:03 +00:00
parent 47398cabc1
commit 5a4d610ed9
2 changed files with 51 additions and 8 deletions

View file

@ -37,15 +37,15 @@ function makeDoc(id: string, memories: GraphApiMemory[]): GraphApiDocument {
}
describe("VersionChainIndex", () => {
it("getChain returns null for version 1 memories (no chain)", () => {
it("getChain returns null for standalone memory (no parent, no children)", () => {
const idx = new VersionChainIndex()
const doc = makeDoc("d1", [makeMem({ id: "m1", version: 1 })])
idx.rebuild([doc])
// version <= 1 returns null per implementation
// Single memory with no parent and no children — not a chain
expect(idx.getChain("m1")).toBeNull()
})
it("builds chain by walking parentMemoryId backwards then reversing", () => {
it("getChain from latest node returns full chain in version order", () => {
const idx = new VersionChainIndex()
const doc = makeDoc("d1", [
makeMem({ id: "m1", version: 1 }),
@ -230,6 +230,46 @@ describe("VersionChainIndex", () => {
expect(chain2!.map((e) => e.id)).toEqual(["m3", "m4"])
})
it("handles circular parent references without infinite loop", () => {
const idx = new VersionChainIndex()
const doc = makeDoc("d1", [
makeMem({ id: "m1", version: 1, parentMemoryId: "m2" }),
makeMem({ id: "m2", version: 2, parentMemoryId: "m1" }),
])
idx.rebuild([doc])
// Cycle: m1->m2->m1. The visited set prevents infinite loops.
const chain = idx.getChain("m1")
expect(chain).not.toBeNull()
expect(chain!.length).toBe(2)
})
it("branching children: follows first child by document order", () => {
const idx = new VersionChainIndex()
const doc = makeDoc("d1", [
makeMem({ id: "m1", version: 1 }),
makeMem({
id: "m2a",
parentMemoryId: "m1",
rootMemoryId: "m1",
version: 2,
}),
makeMem({
id: "m2b",
parentMemoryId: "m1",
rootMemoryId: "m1",
version: 2,
}),
])
idx.rebuild([doc])
// m1 has two children; forward walk picks the first (m2a)
const chain = idx.getChain("m1")
expect(chain).not.toBeNull()
expect(chain!.length).toBe(2)
expect(chain!.map((e) => e.id)).toEqual(["m1", "m2a"])
})
it("chain entries have correct fields", () => {
const idx = new VersionChainIndex()
const doc = makeDoc("d1", [

View file

@ -56,17 +56,20 @@ export class VersionChainIndex {
}
backward.reverse()
// Walk forward from the selected node to find descendants
// Walk forward from the selected node to find descendants.
// Version chains are linear (each memory has one parent), so we
// follow the first child at each step. If branching occurs, only
// the first branch (by document order) is included.
const forward: GraphApiMemory[] = []
let tip: GraphApiMemory | undefined = mem
while (tip) {
const children = this.childrenMap.get(tip.id)
let cursor: GraphApiMemory | undefined = mem
while (cursor) {
const children = this.childrenMap.get(cursor.id)
if (!children || children.length === 0) break
const child = this.memoryMap.get(children[0])
if (!child || visited.has(child.id)) break
visited.add(child.id)
forward.push(child)
tip = child
cursor = child
}
// Combine: backward (root..selected) + forward (selected+1..latest)