-
Notifications
You must be signed in to change notification settings - Fork 76
feat(memory-v3): compose node index from children + routing hints #31978
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
velissa-ai
merged 1 commit into
velissa-ai/memory-v3-build
from
run-plan/memory-v3/pr-3
May 25, 2026
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
233 changes: 233 additions & 0 deletions
233
assistant/src/memory/v3/__tests__/index-composition.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,233 @@ | ||
| /** | ||
| * Tests for `assistant/src/memory/v3/index-composition.ts`. | ||
| * | ||
| * `composeNodeIndex` is a pure function over an already-built `TreeIndex` and | ||
| * `PageIndex`, so these tests hand-build both fixtures (no filesystem / no I/O) | ||
| * and assert on the rendered string. | ||
| * | ||
| * Coverage matrix: | ||
| * - mixed node:/page: children render one summary line each, in authored | ||
| * order, with the node's routing hints appended as a trailer. | ||
| * - a `page:` ref whose slug is absent from the index is silently omitted. | ||
| * - a `node:` ref whose id is absent from the tree is silently omitted. | ||
| * - empty / missing children → just the routing hints, or the empty string | ||
| * when there are none either. | ||
| * - a `node:` child with no summary falls back to the first non-empty body | ||
| * line; with neither, only its header is emitted. | ||
| */ | ||
|
|
||
| import { describe, expect, test } from "bun:test"; | ||
|
|
||
| import type { PageIndex, PageIndexEntry } from "../../v2/page-index.js"; | ||
| import { composeNodeIndex } from "../index-composition.js"; | ||
| import type { ChildRef, TreeIndex } from "../tree-index.js"; | ||
| import type { TreeNode } from "../types.js"; | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Fixture builders | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| function treeNode( | ||
| id: string, | ||
| opts: { summary?: string; routing_hints?: string; body?: string } = {}, | ||
| ): TreeNode { | ||
| return { | ||
| id, | ||
| frontmatter: { | ||
| children: [], | ||
| summary: opts.summary, | ||
| routing_hints: opts.routing_hints, | ||
| }, | ||
| body: opts.body ?? "", | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Build a `TreeIndex` from a list of nodes and an explicit child-ref list for | ||
| * the node under test. Only the fields `composeNodeIndex` reads (`nodes`, | ||
| * `childrenByNode`) are populated; the reverse-adjacency maps are left empty. | ||
| */ | ||
| function treeIndex( | ||
| nodes: TreeNode[], | ||
| childrenByNode: Record<string, ChildRef[]>, | ||
| ): TreeIndex { | ||
| return { | ||
| nodes: new Map(nodes.map((n) => [n.id, n])), | ||
| childrenByNode: new Map(Object.entries(childrenByNode)), | ||
| parentsByNode: new Map(), | ||
| pageParents: new Map(), | ||
| root: "_root", | ||
| }; | ||
| } | ||
|
|
||
| function pageEntry(slug: string, summary: string): PageIndexEntry { | ||
| return { id: 1, slug, summary, edges: [], modifiedAt: 0 }; | ||
| } | ||
|
|
||
| function pageIndex(entries: PageIndexEntry[]): PageIndex { | ||
| return { | ||
| entries, | ||
| bySlug: new Map(entries.map((e) => [e.slug, e])), | ||
| byId: new Map(entries.map((e) => [e.id, e])), | ||
| rendered: "", | ||
| }; | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Tests | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| describe("composeNodeIndex", () => { | ||
| test("composes mixed node:/page: children in authored order with routing hints", () => { | ||
| const tree = treeIndex( | ||
| [ | ||
| treeNode("people", { | ||
| summary: "People you know", | ||
| routing_hints: "for work contacts see node:colleagues", | ||
| }), | ||
| treeNode("colleagues", { summary: "Work relationships" }), | ||
| ], | ||
| { | ||
| people: [ | ||
| { kind: "node", ref: "colleagues" }, | ||
| { kind: "page", ref: "alice" }, | ||
| ], | ||
| }, | ||
| ); | ||
| const pages = pageIndex([ | ||
| pageEntry("alice", "Alice — neighbor and friend"), | ||
| ]); | ||
|
|
||
| const block = composeNodeIndex("people", tree, pages); | ||
|
|
||
| expect(block).toBe( | ||
| [ | ||
| "[node:colleagues] Work relationships", | ||
| "[page:alice] Alice — neighbor and friend", | ||
| "Routing hints: for work contacts see node:colleagues", | ||
| ].join("\n"), | ||
| ); | ||
| }); | ||
|
|
||
| test("emits children in authored order regardless of map insertion", () => { | ||
| const tree = treeIndex( | ||
| [treeNode("a", { summary: "Node A" }), treeNode("root", {})], | ||
| { | ||
| root: [ | ||
| { kind: "page", ref: "zeta" }, | ||
| { kind: "node", ref: "a" }, | ||
| { kind: "page", ref: "beta" }, | ||
| ], | ||
| }, | ||
| ); | ||
| const pages = pageIndex([ | ||
| pageEntry("beta", "Beta page"), | ||
| pageEntry("zeta", "Zeta page"), | ||
| ]); | ||
|
|
||
| const block = composeNodeIndex("root", tree, pages); | ||
|
|
||
| expect(block).toBe( | ||
| [ | ||
| "[page:zeta] Zeta page", | ||
| "[node:a] Node A", | ||
| "[page:beta] Beta page", | ||
| ].join("\n"), | ||
| ); | ||
| }); | ||
|
|
||
| test("silently omits a page ref absent from the index", () => { | ||
| const tree = treeIndex([treeNode("root", {})], { | ||
| root: [ | ||
| { kind: "page", ref: "present" }, | ||
| { kind: "page", ref: "missing" }, | ||
| ], | ||
| }); | ||
| const pages = pageIndex([pageEntry("present", "I exist")]); | ||
|
|
||
| const block = composeNodeIndex("root", tree, pages); | ||
|
|
||
| expect(block).toBe("[page:present] I exist"); | ||
| }); | ||
|
|
||
| test("silently omits a node ref absent from the tree", () => { | ||
| const tree = treeIndex([treeNode("present", { summary: "Here" })], { | ||
| root: [ | ||
| { kind: "node", ref: "present" }, | ||
| { kind: "node", ref: "ghost" }, | ||
| ], | ||
| }); | ||
| const pages = pageIndex([]); | ||
|
|
||
| const block = composeNodeIndex("root", tree, pages); | ||
|
|
||
| expect(block).toBe("[node:present] Here"); | ||
| }); | ||
|
|
||
| test("empty children → just the routing hints", () => { | ||
| const tree = treeIndex( | ||
| [treeNode("leaf", { routing_hints: "this is a leaf branch" })], | ||
| { leaf: [] }, | ||
| ); | ||
|
|
||
| const block = composeNodeIndex("leaf", tree, pageIndex([])); | ||
|
|
||
| expect(block).toBe("Routing hints: this is a leaf branch"); | ||
| }); | ||
|
|
||
| test("no children and no routing hints → empty string", () => { | ||
| const tree = treeIndex([treeNode("bare", {})], { bare: [] }); | ||
|
|
||
| expect(composeNodeIndex("bare", tree, pageIndex([]))).toBe(""); | ||
| }); | ||
|
|
||
| test("node with no childrenByNode entry composes from routing hints alone", () => { | ||
| const tree = treeIndex( | ||
| [treeNode("orphan", { routing_hints: "hint only" })], | ||
| {}, | ||
| ); | ||
|
|
||
| expect(composeNodeIndex("orphan", tree, pageIndex([]))).toBe( | ||
| "Routing hints: hint only", | ||
| ); | ||
| }); | ||
|
|
||
| test("node child with no summary falls back to first non-empty body line", () => { | ||
| const tree = treeIndex( | ||
| [ | ||
| treeNode("root", {}), | ||
| treeNode("bodyonly", { | ||
| body: "\n \nFirst real line\nSecond line", | ||
| }), | ||
| ], | ||
| { root: [{ kind: "node", ref: "bodyonly" }] }, | ||
| ); | ||
|
|
||
| const block = composeNodeIndex("root", tree, pageIndex([])); | ||
|
|
||
| expect(block).toBe("[node:bodyonly] First real line"); | ||
| }); | ||
|
|
||
| test("node child with empty summary string falls back to body line", () => { | ||
| const tree = treeIndex( | ||
| [ | ||
| treeNode("root", {}), | ||
| treeNode("blank", { summary: " ", body: "fallback line" }), | ||
| ], | ||
| { root: [{ kind: "node", ref: "blank" }] }, | ||
| ); | ||
|
|
||
| expect(composeNodeIndex("root", tree, pageIndex([]))).toBe( | ||
| "[node:blank] fallback line", | ||
| ); | ||
| }); | ||
|
|
||
| test("node child with neither summary nor body emits only its header", () => { | ||
| const tree = treeIndex( | ||
| [treeNode("root", {}), treeNode("empty", { body: " \n\t" })], | ||
| { root: [{ kind: "node", ref: "empty" }] }, | ||
| ); | ||
|
|
||
| expect(composeNodeIndex("root", tree, pageIndex([]))).toBe("[node:empty]"); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| /** | ||
| * Memory v3 — Compositional index rendering. | ||
| * | ||
| * A v3 tree node has no stored "index" of its own. Instead, a parent node's | ||
| * index is *composed at read time* by concatenating one description line per | ||
| * child (a `node:` sub-node's summary or a `page:` leaf's summary) plus a thin | ||
| * `Routing hints:` trailer drawn from the node's own frontmatter. Nothing here | ||
| * is persisted — the block is generated fresh every time a descent prompt needs | ||
| * it, so it always reflects the current state of the children. | ||
| * | ||
| * {@link composeNodeIndex} is a **pure function** over an already-built | ||
| * {@link TreeIndex} (from `tree-index.ts`) and {@link PageIndex} (from | ||
| * `../v2/page-index.ts`). It does no I/O: the tree walk / driver PR is | ||
| * responsible for building those indices and feeding them in. | ||
| * | ||
| * Resolution rules, per child ref of `nodeId` (in authored order): | ||
| * - `kind:"node"` → look up the child in `tree.nodes`; emit | ||
| * `"[node:<id>] <summary>"` where summary is the child's | ||
| * `frontmatter.summary` if non-empty, else the first non-empty line of its | ||
| * body. A node with neither still emits its header (`"[node:<id>]"`). | ||
| * - `kind:"page"` → look up `pages.bySlug.get(ref)`; emit | ||
| * `"[page:<slug>] <entry.summary>"`. | ||
| * - Either lookup missing → emit nothing for that ref. Reporting dangling | ||
| * refs is validation's job, not this renderer's. | ||
| * | ||
| * The node's own `routing_hints` (when present) are appended last under a | ||
| * `Routing hints:` trailer. A node with no resolvable children and no routing | ||
| * hints composes to the empty string. | ||
| */ | ||
|
|
||
| import type { PageIndex } from "../v2/page-index.js"; | ||
| import type { TreeIndex } from "./tree-index.js"; | ||
| import type { TreeNode } from "./types.js"; | ||
|
|
||
| /** Trailer label introducing a node's own routing hints. */ | ||
| const ROUTING_HINTS_LABEL = "Routing hints:"; | ||
|
|
||
| /** | ||
| * Resolve a node's display summary: its frontmatter `summary` if non-empty, | ||
| * otherwise the first non-empty line of its body, otherwise the empty string. | ||
| * Whitespace is trimmed so a leading blank line in the body never wins. | ||
| */ | ||
| function nodeSummary(node: TreeNode): string { | ||
| const summary = node.frontmatter.summary?.trim(); | ||
| if (summary) return summary; | ||
| for (const line of node.body.split("\n")) { | ||
| const trimmed = line.trim(); | ||
| if (trimmed) return trimmed; | ||
| } | ||
| return ""; | ||
| } | ||
|
|
||
| /** | ||
| * Render one child ref into its index line, or `null` when the ref's target is | ||
| * absent from the supplied indices (validation owns reporting those). | ||
| * | ||
| * A resolvable `node:` child always yields a line — its header (`[node:<id>]`) | ||
| * with a trailing summary when one exists. A `page:` child yields | ||
| * `[page:<slug>] <summary>`; the v2 page index already truncates `summary`. | ||
| */ | ||
| function renderChild( | ||
| kind: "page" | "node", | ||
| ref: string, | ||
| tree: TreeIndex, | ||
| pages: PageIndex, | ||
| ): string | null { | ||
| if (kind === "node") { | ||
| const child = tree.nodes.get(ref); | ||
| if (!child) return null; | ||
| const summary = nodeSummary(child); | ||
| return summary ? `[node:${ref}] ${summary}` : `[node:${ref}]`; | ||
| } | ||
| const entry = pages.bySlug.get(ref); | ||
| if (!entry) return null; | ||
| return `[page:${ref}] ${entry.summary}`; | ||
| } | ||
|
|
||
| /** | ||
| * Compose the prompt-ready index block for `nodeId` from its children's | ||
| * descriptions plus the node's own routing hints. | ||
| * | ||
| * Pure and deterministic: children are emitted in authored order (the order | ||
| * `tree.childrenByNode` preserves from the node's `children` frontmatter), refs | ||
| * whose targets are absent are silently skipped, and the node's | ||
| * `routing_hints` (if present) are appended under a {@link ROUTING_HINTS_LABEL} | ||
| * trailer. A node with no entry in `childrenByNode`, no resolvable children, | ||
| * and no routing hints composes to the empty string. | ||
| * | ||
| * The result is a plain string with no trailing newline, suitable to drop | ||
| * directly into an LLM descent prompt. | ||
| */ | ||
| export function composeNodeIndex( | ||
| nodeId: string, | ||
| tree: TreeIndex, | ||
| pages: PageIndex, | ||
| ): string { | ||
| const blocks: string[] = []; | ||
|
|
||
| const childRefs = tree.childrenByNode.get(nodeId) ?? []; | ||
| for (const { kind, ref } of childRefs) { | ||
| const line = renderChild(kind, ref, tree, pages); | ||
| if (line !== null) blocks.push(line); | ||
| } | ||
|
|
||
| const routingHints = tree.nodes | ||
| .get(nodeId) | ||
| ?.frontmatter.routing_hints?.trim(); | ||
| if (routingHints) { | ||
| blocks.push(`${ROUTING_HINTS_LABEL} ${routingHints}`); | ||
| } | ||
|
|
||
| return blocks.join("\n"); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nodeSummary()returnsfrontmatter.summaryafter a plaintrim(), so a valid multi-line YAML summary is emitted verbatim byrenderChild()and injects embedded newlines into what should be one child entry per line. In that case, the composed index block no longer preserves the intended line-oriented structure and downstream routing prompts can misinterpret child boundaries; v2 already guards against this class of issue by normalizing summary whitespace. Normalize/collapse internal whitespace (or take only the first non-empty line) before returning the summary.Useful? React with 👍 / 👎.