From fba27c77e1620d3b3dafe6f04e0ec03c95e1a5fc Mon Sep 17 00:00:00 2001 From: Aaron Bockelie Date: Mon, 13 Jul 2026 18:29:29 -0500 Subject: [PATCH] fix(retrieval): make paths usable and steer scan-then-follow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured against two vaults with a paired A/B harness (plugin tools vs. a shell with ripgrep, same question, isolated agents). On a graph-shaped question over a densely linked vault — the plugin's home field — it LOST: 90.3k tokens vs 70.1k, 20 calls vs 16, 3 dead ends vs 2. Not because the graph failed. Because the plugin's own output made the graph unusable, and the agent spent the difference working around it. Paths were not usable as identifiers vault.search elided long paths to `first/.../last`, and vault.list printed only the basename. Both destroy the one field an agent must have verbatim: the value it feeds to the very next call. An agent that joined the listed directory to the name it was given got a path that does not exist and a hard "File not found". This is the mechanism behind the repeated-search behaviour we keep seeing. Scan-then-follow requires handing a search hit to graph.neighbors; if the hit's path is elided, that handoff is impossible, so the only move left that still works is another search. The tool was training the behaviour. Paths are now emitted verbatim everywhere, and formatPath — a helper whose sole purpose was to damage them — is deleted. vault.fragments ignored its own `path` parameter `path` was read only as a fallback *query* string, never as a scope, so asking for fragments from one note returned passages from several, top-ranked hit first. An agent reasoning "these are the key passages in the note I named" would attribute content to the wrong file. Now scoped, over-fetching candidates first so scoping does not silently return nothing when other notes outrank the target. graph.statistics threw on the vault-wide call No sourcePath is returned for the vault-wide shape; the formatter read it unconditionally, threw, and fell back to raw JSON behind a "Formatter error" banner. Now formatted — and it reports link density, because that is the number that tells the caller whether following links will beat searching. Steering, since the affordance was invisible Both experiment arms independently concluded the graph was the strongest signal available and both nearly missed it: "graph.neighbors was the MVP and I nearly didn't reach for it"; and from the ripgrep arm, "I had a graph and was forced to walk it as a pile of bytes." Search results now name graph.neighbors with the top hit's path filled in, and the vault/graph tool descriptions teach the pattern: a couple of broad scans, then follow links — rather than many narrow searches that re-run the same ranking. vault.read printed the frontmatter twice A truncated "## Frontmatter" summary above a body that already opens with the raw block. Suppressed when the body carries it; retained for fragment reads, which do not. Fragment strategy `semantic` renamed to `structure` It cuts on headings and paragraphs. It has never been embedding or vector search, but "semantic" in a retrieval slot reads as exactly that to any model trained on the last decade of the literature — so callers selected it expecting matching this index does not do. `semantic` still works as a deprecated alias. All four strategies are now described; previously the enum shipped with no gloss at all and the agent chose blind. Tests: 415 passing. New suites cover path fidelity (including the fabricated path an agent would construct), fragment scoping, the vault-wide stats shape, and the frontmatter duplication. --- src/formatters/graph.ts | 60 +++++++++- src/formatters/index.ts | 1 - src/formatters/search.ts | 16 ++- src/formatters/utils.ts | 10 -- src/formatters/vault.ts | 25 ++-- src/indexing/fragment-retriever.ts | 24 ++-- src/semantic/operations/vault.ts | 88 +++++++++----- src/tools/semantic-tools.ts | 8 +- src/types/fragment.ts | 6 + tests/formatters/graph-vault-stats.test.ts | 66 +++++++++++ tests/formatters/path-fidelity.test.ts | 81 +++++++++++++ .../formatters/read-frontmatter-dedup.test.ts | 73 ++++++++++++ tests/fragments-path-scope.test.ts | 108 ++++++++++++++++++ 13 files changed, 502 insertions(+), 64 deletions(-) create mode 100644 tests/formatters/graph-vault-stats.test.ts create mode 100644 tests/formatters/path-fidelity.test.ts create mode 100644 tests/formatters/read-frontmatter-dedup.test.ts create mode 100644 tests/fragments-path-scope.test.ts diff --git a/src/formatters/graph.ts b/src/formatters/graph.ts index 560980c..eb1e0f4 100644 --- a/src/formatters/graph.ts +++ b/src/formatters/graph.ts @@ -225,7 +225,8 @@ export function formatGraphPath(response: GraphPathResponse): string { * Actual response: { operation, sourcePath, statistics: {...}, message, workflow } */ export interface GraphStatsResponse { - sourcePath: string; + // Absent for the vault-wide call (graph.statistics with no sourcePath). + sourcePath?: string; // Flat format (legacy) inDegree?: number; outDegree?: number; @@ -239,13 +240,29 @@ export interface GraphStatsResponse { unresolvedCount?: number; tagCount?: number; }; + // Vault-wide format — a different shape entirely, returned when no sourcePath is given. + vaultStatistics?: { + totalNotes: number; + totalLinks: number; + orphanCount: number; + averageDegree: number; + largestComponentSize?: number; + isolatedClusters?: number; + }; message?: string; } export function formatGraphStats(response: GraphStatsResponse): string { const lines: string[] = []; - const fileName = response.sourcePath.split('/').pop() || response.sourcePath; + // Vault-wide statistics carry no sourcePath. Reading one unconditionally threw and + // dropped the caller into the raw-JSON fallback. + if (response.vaultStatistics) { + return formatVaultGraphStats(response.vaultStatistics); + } + + const sourcePath = response.sourcePath ?? ''; + const fileName = sourcePath.split('/').pop() || sourcePath || 'vault'; lines.push(header(1, `Stats: ${fileName}`)); lines.push(''); @@ -284,6 +301,45 @@ export function formatGraphStats(response: GraphStatsResponse): string { return joinLines(lines); } +/** + * Format the vault-wide graph statistics (graph.statistics with no sourcePath). + * + * Average degree is the headline number because it tells the caller which retrieval + * strategy will actually pay here: in a densely linked vault, following links from a + * couple of anchor notes beats issuing more searches. + */ +function formatVaultGraphStats(stats: NonNullable): string { + const lines: string[] = []; + + lines.push(header(1, 'Vault graph')); + lines.push(''); + lines.push(property('Notes', stats.totalNotes.toString(), 0)); + lines.push(property('Links', stats.totalLinks.toString(), 0)); + lines.push(property('Average connections per note', stats.averageDegree.toFixed(1), 0)); + lines.push(property('Orphans', stats.orphanCount.toString(), 0)); + + if (stats.largestComponentSize !== undefined) { + lines.push(property('Largest connected component', `${stats.largestComponentSize} notes`, 0)); + } + if (stats.isolatedClusters !== undefined) { + lines.push(property('Separate clusters', stats.isolatedClusters.toString(), 0)); + } + + lines.push(''); + lines.push(divider()); + + if (stats.averageDegree >= 3) { + lines.push(`This vault is densely linked (${stats.averageDegree.toFixed(1)} links per note on average). Its link structure is a stronger signal than keyword frequency:`); + lines.push(tip('Find one or two anchor notes with `vault.search`, then expand with `graph.neighbors(path)` / `graph.traverse(path)` rather than issuing more searches')); + } else { + lines.push(tip('Sparsely linked vault — `vault.search` will usually outperform graph traversal here')); + } + + lines.push(summaryFooter()); + + return joinLines(lines); +} + /** * Format graph.tag-analysis response */ diff --git a/src/formatters/index.ts b/src/formatters/index.ts index 3894d54..53ae7ac 100644 --- a/src/formatters/index.ts +++ b/src/formatters/index.ts @@ -112,7 +112,6 @@ export { tip, summaryFooter, joinLines, - formatPath, formatTree } from './utils'; diff --git a/src/formatters/search.ts b/src/formatters/search.ts index 6e5217c..badfb94 100644 --- a/src/formatters/search.ts +++ b/src/formatters/search.ts @@ -10,8 +10,7 @@ import { divider, tip, summaryFooter, - joinLines, - formatPath + joinLines } from './utils'; export interface SearchResult { @@ -80,7 +79,9 @@ export function formatSearchResults(response: SearchResponse): string { const scoreText = result.score > 0 ? ` (${interpretScore(result.score)})` : ''; lines.push(`${num}. **${result.title}**${scoreText}`); - lines.push(property('Path', formatPath(result.path))); + // Verbatim: the agent's next call feeds this straight to vault.read / + // graph.neighbors. An elided path is not a shorter path, it is a wrong one. + lines.push(property('Path', result.path)); if (result.snippet?.content) { const snippetText = truncate(result.snippet.content, 100); @@ -98,6 +99,15 @@ export function formatSearchResults(response: SearchResponse): string { } lines.push(tip('Use `vault.read(path)` or `view.file(path)` to see full content')); + + // Point at the graph, with the top hit's path already filled in. Search ranks by term + // frequency, so it finds an anchor but not the notes that discuss the topic in other + // words. Expanding from an anchor along real links is what reaches those; without this + // hint the only next move on offer is another search, which re-runs the same ranking. + if (results.length > 0) { + lines.push(tip(`Related notes are often reached better by following links than by searching again — try \`graph.neighbors(path: "${results[0].path}")\` to expand from the top hit`)); + } + lines.push(summaryFooter()); return joinLines(lines); diff --git a/src/formatters/utils.ts b/src/formatters/utils.ts index de5c5fb..b34c0b5 100644 --- a/src/formatters/utils.ts +++ b/src/formatters/utils.ts @@ -116,16 +116,6 @@ export function escapeMarkdown(text: string): string { return text.replace(/([*_`[\]])/g, '\\$1'); } -/** - * Format a path for display (shorten if too long) - */ -export function formatPath(path: string, maxLen: number = 60): string { - if (path.length <= maxLen) return path; - const parts = path.split('/'); - if (parts.length <= 2) return truncate(path, maxLen); - return `${parts[0]}/.../${parts[parts.length - 1]}`; -} - /** * Create a simple ASCII tree structure */ diff --git a/src/formatters/vault.ts b/src/formatters/vault.ts index adce1f8..7d72696 100644 --- a/src/formatters/vault.ts +++ b/src/formatters/vault.ts @@ -48,10 +48,12 @@ export function formatFileList(response: FileListResponse | string[]): string { lines.push(`Found ${paths.length} item${paths.length !== 1 ? 's' : ''}`); lines.push(''); + // Full vault-relative path, not the basename. A listing that shows only names + // invites the agent to join them onto the directory it asked for, which + // fabricates a path when the entries actually live in nested subdirectories. paths.slice(0, truncateAt).forEach(path => { - const name = path.split('/').pop() || path; - const isFolder = !path.includes('.'); - lines.push(`- ${isFolder ? name + '/' : name}`); + const isFolder = !path.split('/').pop()?.includes('.'); + lines.push(`- ${isFolder ? path + '/' : path}`); }); if (paths.length > truncateAt) { @@ -96,11 +98,15 @@ export function formatFileList(response: FileListResponse | string[]): string { lines.push(''); } + // Entries are listed by full vault-relative path, never by basename alone: the path + // is what the agent passes to the next call, and a nested entry shown as a bare name + // reads as though it sits directly under `directory`, which it may not. + // Folders first if (folders.length > 0) { lines.push(header(2, 'Folders')); folders.slice(0, 20).forEach(f => { - lines.push(`- ${f.name}/`); + lines.push(`- ${f.path}/`); }); if (folders.length > 20) { lines.push(`- ... and ${folders.length - 20} more folders`); @@ -113,7 +119,7 @@ export function formatFileList(response: FileListResponse | string[]): string { lines.push(header(2, 'Files')); regularFiles.slice(0, 30).forEach(f => { const sizeText = f.size !== undefined ? ` (${formatFileSize(f.size)})` : ''; - lines.push(`- ${f.name}${sizeText}`); + lines.push(`- ${f.path}${sizeText}`); }); if (regularFiles.length > 30) { lines.push(`- ... and ${regularFiles.length - 30} more files`); @@ -212,8 +218,13 @@ export function formatFileRead(response: FileReadResponse): string { } } - // Frontmatter summary - if (frontmatter && Object.keys(frontmatter).length > 0) { + // Frontmatter summary — skipped when the body already carries the raw frontmatter + // block, which is the normal case for a whole-note read. Printing a truncated summary + // of text that is reproduced verbatim a few lines below costs tokens on the most + // frequently called action and tells the reader nothing new. + const bodyRepeatsFrontmatter = typeof content === 'string' && content.trimStart().startsWith('---'); + + if (frontmatter && Object.keys(frontmatter).length > 0 && !bodyRepeatsFrontmatter) { lines.push(''); lines.push(header(2, 'Frontmatter')); const keys = Object.keys(frontmatter).slice(0, 10); diff --git a/src/indexing/fragment-retriever.ts b/src/indexing/fragment-retriever.ts index fe5724a..57167be 100644 --- a/src/indexing/fragment-retriever.ts +++ b/src/indexing/fragment-retriever.ts @@ -32,20 +32,24 @@ export class UniversalFragmentRetriever { query: string, options: RetrievalOptions = {} ): SemanticResponse { - const { strategy = 'auto', maxFragments = 5 } = options; - + const { strategy = 'auto', maxFragments = 5, scopePath } = options; + let fragments: Fragment[] = []; let selectedStrategy: string = strategy; - + if (strategy === 'auto') { // Choose strategy based on query characteristics selectedStrategy = this.selectOptimalStrategy(query); } - + + // When scoping to one document, over-fetch: the strategies rank across every indexed + // document, so the top `maxFragments` may contain nothing from the requested file. + const candidateCount = scopePath ? Math.max(maxFragments * 10, 50) : maxFragments; + // Execute the selected strategy (all search methods are synchronous) switch (selectedStrategy) { case 'adaptive': - fragments = this.adaptiveIndex.search(query, maxFragments); + fragments = this.adaptiveIndex.search(query, candidateCount); break; case 'proximity': @@ -53,15 +57,19 @@ export class UniversalFragmentRetriever { break; case 'semantic': - fragments = this.semanticIndex.searchWithContext(query, { maxFragments }); + fragments = this.semanticIndex.searchWithContext(query, { maxFragments: candidateCount }); break; default: // Hybrid approach - combine results from multiple strategies - fragments = this.hybridSearch(query, maxFragments); + fragments = this.hybridSearch(query, candidateCount); selectedStrategy = 'hybrid'; } - + + if (scopePath) { + fragments = fragments.filter(fragment => fragment.docPath === scopePath); + } + // Limit to requested number of fragments fragments = fragments.slice(0, maxFragments); diff --git a/src/semantic/operations/vault.ts b/src/semantic/operations/vault.ts index 0434306..88a53ce 100644 --- a/src/semantic/operations/vault.ts +++ b/src/semantic/operations/vault.ts @@ -12,6 +12,23 @@ import { ValidationException } from '../../validation/input-validator'; import { RouterContext } from './router-context'; import { Params, paramStr, paramNum, paramBool, requireParamStr } from './shared'; +type FragmentStrategy = 'auto' | 'adaptive' | 'proximity' | 'semantic'; + +/** + * Resolve the caller-facing fragment strategy onto the internal one. + * + * 'structure' is the honest name for what the index does — it cuts on the document's own + * headings and paragraphs. It is exposed instead of 'semantic', which reads as + * embedding/vector similarity to anything trained on the last decade of retrieval + * literature and led callers to expect matching this index does not do. 'semantic' stays + * accepted so existing callers keep working. + */ +function resolveFragmentStrategy(strategy: string | undefined): FragmentStrategy { + if (strategy === 'structure' || strategy === 'semantic') return 'semantic'; + if (strategy === 'adaptive' || strategy === 'proximity') return strategy; + return 'auto'; +} + /** * Extension of a vault path, including the leading dot ('' when there is none). * A leading dot is not an extension: '.gitignore' has none. @@ -52,7 +69,9 @@ export async function executeVaultOperation(ctx: RouterContext, action: string, } case 'read': { const path = paramStr(params, 'path') ?? ''; - const strategy = paramStr(params, 'strategy') as 'auto' | 'adaptive' | 'proximity' | 'semantic' | undefined; + const strategy = paramStr(params, 'strategy') !== undefined + ? resolveFragmentStrategy(paramStr(params, 'strategy')) + : undefined; return await readFileWithFragments(ctx.api, ctx.fragmentRetriever, { path, returnFullFile: paramBool(params, 'returnFullFile'), @@ -63,8 +82,12 @@ export async function executeVaultOperation(ctx: RouterContext, action: string, }); } case 'fragments': { - // Dedicated fragment search across multiple files - const fragmentQuery = paramStr(params, 'query') ?? paramStr(params, 'path') ?? ''; + // Dedicated fragment search. When `path` is supplied it scopes the search to that + // one file; previously it was only ever read as a fallback *query* string, so + // naming a file returned passages from other files that the caller could easily + // attribute to the file it asked about. + const fragmentPath = paramStr(params, 'path'); + const fragmentQuery = paramStr(params, 'query') ?? fragmentPath ?? ''; // Skip indexing if no query provided if (!fragmentQuery || fragmentQuery.trim().length === 0) { @@ -79,41 +102,48 @@ export async function executeVaultOperation(ctx: RouterContext, action: string, } try { - // Only index files that match the query to avoid indexing entire vault - // This is a lazy indexing approach - index on demand - const searchResults = await ctx.api.searchPaginated(fragmentQuery, 1, 20, 'combined', false); + const indexFile = async (filePath: string): Promise => { + if (!filePath || !filePath.endsWith('.md')) return; + try { + const fileResponse = await ctx.api.getFile(filePath); + let content: string; - // Index only the files that match the search - if (searchResults && searchResults.results && searchResults.results.length > 0) { - for (const result of searchResults.results.slice(0, 20)) { // Limit to first 20 files - try { - const filePath = result.path; - if (filePath && filePath.endsWith('.md')) { - const fileResponse = await ctx.api.getFile(filePath); - let content: string; + if (typeof fileResponse === 'string') { + content = fileResponse; + } else if (fileResponse && typeof fileResponse === 'object' && 'content' in fileResponse) { + content = fileResponse.content; + } else { + return; + } - if (typeof fileResponse === 'string') { - content = fileResponse; - } else if (fileResponse && typeof fileResponse === 'object' && 'content' in fileResponse) { - content = fileResponse.content; - } else { - continue; - } + ctx.fragmentRetriever.indexDocument(`file:${filePath}`, filePath, content); + } catch (e) { + // Skip files that can't be indexed + Debug.log(`Skipping file during fragment indexing:`, e); + } + }; - const docId = `file:${filePath}`; - ctx.fragmentRetriever.indexDocument(docId, filePath, content); - } - } catch (e) { - // Skip files that can't be indexed - Debug.log(`Skipping file during fragment indexing:`, e); + if (fragmentPath) { + // Scoped to one file: index just that file. Searching the vault to decide what + // to index would be wasted work, and could fail to index the very file named. + await indexFile(fragmentPath); + } else { + // Only index files that match the query to avoid indexing entire vault + // This is a lazy indexing approach - index on demand + const searchResults = await ctx.api.searchPaginated(fragmentQuery, 1, 20, 'combined', false); + + if (searchResults && searchResults.results && searchResults.results.length > 0) { + for (const result of searchResults.results.slice(0, 20)) { // Limit to first 20 files + await indexFile(result.path); } } } // Search for fragments in indexed documents const fragmentResponse = ctx.fragmentRetriever.retrieveFragments(fragmentQuery, { - strategy: (paramStr(params, 'strategy') as 'auto' | 'adaptive' | 'proximity' | 'semantic') || 'auto', - maxFragments: paramNum(params, 'maxFragments') || 5 + strategy: resolveFragmentStrategy(paramStr(params, 'strategy')), + maxFragments: paramNum(params, 'maxFragments') || 5, + scopePath: fragmentPath }); return fragmentResponse; diff --git a/src/tools/semantic-tools.ts b/src/tools/semantic-tools.ts index 2b53c53..ce5394b 100644 --- a/src/tools/semantic-tools.ts +++ b/src/tools/semantic-tools.ts @@ -360,12 +360,12 @@ const createSemanticTool = (operation: string, visibility?: ToolVisibility): Sem export function getOperationDescription(operation: string): string { const descriptions: Record = { - vault: '📁 File operations - list, read, create, update, delete, search, fragments, move, rename, copy, split, combine, concatenate. Search supports: operators (file:, path:, content:, tag:), OR/AND, "quoted phrases", /regex/. Options: ranked=true for TF-IDF relevance scoring, searchStrategy (filename|content|combined|auto), includeSnippets for contextual extracts.', + vault: '📁 File operations - list, read, create, update, delete, search, fragments, move, rename, copy, split, combine, concatenate. Search supports: operators (file:, path:, content:, tag:), OR/AND, "quoted phrases", /regex/. Options: ranked=true for TF-IDF relevance scoring, searchStrategy (filename|content|combined|auto), includeSnippets for contextual extracts. Search matches words, not meaning — it will miss notes that cover a topic in different vocabulary. Prefer a couple of BROAD scans over many narrow ones, then follow links from the hits with `graph.neighbors` to reach what search cannot rank.', edit: '✏️ Edit files - window: find/replace with fuzzy matching, append: add to end, patch: modify headings/blocks/frontmatter, at_line: insert at line number, from_buffer: reuse previous window content', view: '👁️ View content - file: entire document, window: ~20 lines around point, active: current editor file, open_in_obsidian: launch in app', workflow: '💡 Get contextual suggestions for next actions based on current state', system: 'ℹ️ System operations - info: server details, commands: available actions, fetch_web: retrieve and process web content', - graph: '🕸️ Graph navigation - traverse: explore connections, neighbors: immediate links, path: find routes between notes, statistics: link counts, backlinks/forwardlinks: directional analysis, search-traverse: connected snippets', + graph: '🕸️ Graph navigation — follow the vault\'s own links. Use this to EXPAND from a note you already found, instead of running another search: search ranks by term frequency, so it cannot reach notes that discuss a topic in different words, but a link to them usually exists. Best pattern: one or two broad `vault.search` scans to find anchor notes, then `neighbors`/`traverse` from those anchors to gather the rest. Actions — neighbors: immediate links of a note (start here); traverse: multi-hop exploration; search-traverse: traverse while matching a query, i.e. scan and follow in one call; path: how two notes connect; backlinks/forwardlinks: directional links; statistics: link counts (call with no sourcePath for vault-wide density); tag-analysis/shared-tags: tag structure.', dataview: '📊 Dataview operations - query: execute DQL queries (LIST FROM "folder", TABLE field FROM #tag WHERE condition), list: get pages with metadata and frontmatter, metadata: extract complete page metadata, validate: check DQL syntax, status: plugin availability. Supports LIST, TABLE, TASK, CALENDAR queries with WHERE filters, sorting, grouping.', bases: '🗃️ Bases operations - list: show all .base files, read: get YAML config, create: new base with views/filters/formulas, query: execute filters on vault notes, view: get table/card view data, evaluate: test formulas, export: CSV/JSON/Markdown. Bases use YAML format with expression-based filters like status == "active" and file.hasTag("project")' }; @@ -441,8 +441,8 @@ function getParametersForOperation(operation: string): Record { }, strategy: { type: 'string', - enum: ['auto', 'adaptive', 'proximity', 'semantic'], - description: 'Fragment retrieval strategy (default: auto)' + enum: ['auto', 'adaptive', 'proximity', 'structure', 'semantic'], + description: 'Fragment retrieval strategy (default: auto). None of these are embedding/vector search — all match words, not meaning. auto: pick per query. adaptive: term-frequency ranked passages. proximity: passages where the query terms appear close together. structure: passages cut on the document\'s own structure (headings, paragraphs) with surrounding context kept. "semantic" is a deprecated alias of "structure" — it never meant vector similarity, and is retained only for compatibility.' }, maxFragments: { type: 'number', diff --git a/src/types/fragment.ts b/src/types/fragment.ts index 78ed00c..c2b3c6e 100644 --- a/src/types/fragment.ts +++ b/src/types/fragment.ts @@ -125,4 +125,10 @@ export interface FragmentRef { export interface RetrievalOptions { strategy?: 'auto' | 'adaptive' | 'proximity' | 'semantic'; maxFragments?: number; + /** + * Restrict results to fragments from this document. Without it, retrieval spans every + * indexed document, so a caller naming a file would receive passages from other files + * and could attribute them to the file it asked about. + */ + scopePath?: string; } \ No newline at end of file diff --git a/tests/formatters/graph-vault-stats.test.ts b/tests/formatters/graph-vault-stats.test.ts new file mode 100644 index 0000000..f39a003 --- /dev/null +++ b/tests/formatters/graph-vault-stats.test.ts @@ -0,0 +1,66 @@ +/** + * graph.statistics — vault-wide shape. + * + * Called without a sourcePath, the operation returns { vaultStatistics: {...} } and no + * sourcePath at all. The formatter read sourcePath unconditionally, threw, and dropped + * the caller into a raw-JSON fallback prefixed "Formatter error". The agent still got + * data, so nothing failed loudly — it just got the ugly path every time. + */ +import { formatGraphStats } from '../../src/formatters/graph'; + +const DENSE = { + totalNotes: 58, + totalLinks: 451, + orphanCount: 0, + averageDegree: 15.55, + largestComponentSize: 58, + isolatedClusters: 1 +}; + +describe('formatGraphStats — vault-wide', () => { + it('should format vault-wide statistics without throwing', () => { + expect(() => formatGraphStats({ vaultStatistics: DENSE })).not.toThrow(); + }); + + it('should report the headline counts', () => { + const output = formatGraphStats({ vaultStatistics: DENSE }); + + expect(output).toContain('58'); + expect(output).toContain('451'); + }); + + it('should steer a densely linked vault toward following links', () => { + const output = formatGraphStats({ vaultStatistics: DENSE }); + + expect(output).toContain('graph.neighbors'); + }); + + it('should steer a sparsely linked vault toward search instead', () => { + const output = formatGraphStats({ + vaultStatistics: { totalNotes: 200, totalLinks: 40, orphanCount: 150, averageDegree: 0.4 } + }); + + expect(output).toContain('vault.search'); + }); +}); + +describe('formatGraphStats — per-note', () => { + it('should still format a single-note response', () => { + const output = formatGraphStats({ + sourcePath: 'notes/hub.md', + statistics: { inDegree: 4, outDegree: 7, totalDegree: 11 } + }); + + expect(output).toContain('hub.md'); + expect(output).toContain('11'); + }); + + it('should flag an orphan note', () => { + const output = formatGraphStats({ + sourcePath: 'notes/lonely.md', + statistics: { inDegree: 0, outDegree: 0, totalDegree: 0 } + }); + + expect(output).toContain('orphan'); + }); +}); diff --git a/tests/formatters/path-fidelity.test.ts b/tests/formatters/path-fidelity.test.ts new file mode 100644 index 0000000..fa01e43 --- /dev/null +++ b/tests/formatters/path-fidelity.test.ts @@ -0,0 +1,81 @@ +/** + * Path fidelity in agent-facing output. + * + * A path is an identifier, not prose. It is the value the agent must hand back to + * vault.read / graph.neighbors on the very next call, so any beautification of it — + * eliding the middle, or printing only the basename — makes the result unusable and + * forces the agent to guess, re-search, or fabricate. + * + * Two measured failures motivate these tests: + * - vault.search elided long paths to `first/.../last`, so a search hit could not be + * fed into vault.read. Agents fell back to another search, which is exactly the + * repeated-search-instead-of-graph-follow behaviour we want to stop rewarding. + * - vault.list printed only the basename, implying files live directly under the + * directory that was listed. Joining the two produced a path that does not exist, + * and the read failed. + */ +import { formatFileList } from '../../src/formatters/vault'; +import { formatSearchResults } from '../../src/formatters/search'; + +// A real path from the test corpus: nested, long, and non-ASCII (em-dash). +const DEEP_PATH = 'Part IV — How We Move It/7. Integration disposition/7.3 The MRP-API hidden hub and its dependency cluster.md'; + +describe('formatSearchResults', () => { + it('should emit the full path of a hit so it can be passed straight to vault.read', () => { + const output = formatSearchResults({ + query: 'MRP-API', + results: [{ path: DEEP_PATH, title: '7.3 The MRP-API hidden hub', score: 1.42 }], + totalResults: 1, + page: 1, + pageSize: 10, + totalPages: 1 + }); + + expect(output).toContain(DEEP_PATH); + expect(output).not.toContain('/.../'); + }); +}); + +describe('formatFileList', () => { + it('should emit full vault-relative paths for a string-array response', () => { + const output = formatFileList([DEEP_PATH]); + + expect(output).toContain(DEEP_PATH); + }); + + it('should emit full vault-relative paths for a structured response', () => { + const output = formatFileList({ + directory: 'Part IV — How We Move It', + files: [ + { path: DEEP_PATH, name: '7.3 The MRP-API hidden hub and its dependency cluster.md' } + ] + }); + + expect(output).toContain(DEEP_PATH); + }); + + it('should not imply a nested file sits directly under the listed directory', () => { + // The bug: only the basename was printed, so an agent joined the directory it asked + // for with the name it was given and produced a path that does not exist on disk. + const output = formatFileList({ + directory: 'Part IV — How We Move It', + files: [ + { path: DEEP_PATH, name: '7.3 The MRP-API hidden hub and its dependency cluster.md' } + ] + }); + + const fabricated = 'Part IV — How We Move It/7.3 The MRP-API hidden hub and its dependency cluster.md'; + expect(output).not.toContain(fabricated); + }); + + it('should still show folders with their full path', () => { + const output = formatFileList({ + directory: '', + files: [ + { path: 'Part IV — How We Move It/7. Integration disposition', name: '7. Integration disposition', isFolder: true } + ] + }); + + expect(output).toContain('Part IV — How We Move It/7. Integration disposition'); + }); +}); diff --git a/tests/formatters/read-frontmatter-dedup.test.ts b/tests/formatters/read-frontmatter-dedup.test.ts new file mode 100644 index 0000000..b820293 --- /dev/null +++ b/tests/formatters/read-frontmatter-dedup.test.ts @@ -0,0 +1,73 @@ +/** + * vault.read must not print the frontmatter twice. + * + * A whole-note read returns the raw file, which already opens with the `---` frontmatter + * block. The formatter additionally rendered a "## Frontmatter" summary above it — one + * that truncates keys at 10 and values at 50 chars. So the most frequently called action + * paid tokens to show a degraded copy of text it was about to show in full. + */ +import { formatFileRead } from '../../src/formatters/vault'; + +const FRONTMATTER = { + title: 'The MRP-API hidden hub and its dependency cluster', + section: '§7.3', + part: 'Part IV — How We Move It' +}; + +const BODY_WITH_FRONTMATTER = [ + '---', + 'title: "The MRP-API hidden hub and its dependency cluster"', + 'section: "§7.3"', + 'part: "Part IV — How We Move It"', + '---', + '', + '# 7.3 The MRP-API hidden hub', + '', + 'It is the single point whose break cascades.' +].join('\n'); + +describe('formatFileRead frontmatter handling', () => { + it('should not render a Frontmatter summary when the body already contains the block', () => { + const output = formatFileRead({ + path: 'notes/7.3.md', + content: BODY_WITH_FRONTMATTER, + frontmatter: FRONTMATTER + }); + + expect(output).not.toContain('## Frontmatter'); + }); + + it('should still return the body verbatim, frontmatter block included', () => { + const output = formatFileRead({ + path: 'notes/7.3.md', + content: BODY_WITH_FRONTMATTER, + frontmatter: FRONTMATTER + }); + + expect(output).toContain('title: "The MRP-API hidden hub and its dependency cluster"'); + expect(output).toContain('It is the single point whose break cascades.'); + }); + + it('should render a Frontmatter summary when the body does not carry the block', () => { + // Fragment/window reads return a body without the raw frontmatter, so the summary is + // the only place the caller can see it — keep it there. + const output = formatFileRead({ + path: 'notes/7.3.md', + content: '# 7.3 The MRP-API hidden hub\n\nIt is the single point whose break cascades.', + frontmatter: FRONTMATTER + }); + + expect(output).toContain('## Frontmatter'); + expect(output).toContain('§7.3'); + }); + + it('should still show the path for a note whose body carries frontmatter', () => { + const output = formatFileRead({ + path: 'notes/7.3.md', + content: BODY_WITH_FRONTMATTER, + frontmatter: FRONTMATTER + }); + + expect(output).toContain('notes/7.3.md'); + }); +}); diff --git a/tests/fragments-path-scope.test.ts b/tests/fragments-path-scope.test.ts new file mode 100644 index 0000000..1ca2f5c --- /dev/null +++ b/tests/fragments-path-scope.test.ts @@ -0,0 +1,108 @@ +/** + * vault.fragments — the `path` parameter must scope the search. + * + * Measured failure: asking for fragments from one note returned "fragments across 3 + * files", with the top-scoring passage coming from a different note entirely. The `path` + * param was only read as a fallback *query* string, never as a scope, so an agent that + * reasoned "these are the key passages in the note I named" would attribute content to + * the wrong file. That is a correctness trap, not a presentation nit. + */ +import { UniversalFragmentRetriever } from '../src/indexing/fragment-retriever'; + +const TARGET = 'scaled-sandwich/organizational-patterns.md'; +const OTHER = 'scaled-sandwich/README.md'; + +function retrieverWithTwoDocs(): UniversalFragmentRetriever { + const retriever = new UniversalFragmentRetriever(); + + retriever.indexDocument( + `file:${TARGET}`, + TARGET, + [ + '# Organizational patterns', + 'Kanban WIP limits prevent overwhelming the human reviewer, maintaining quality.', + 'Pull systems prevent overload. Span of control caps the reviewer at seven items.', + 'Context switching is expensive, so batch similar decisions together.' + ].join('\n\n') + ); + + retriever.indexDocument( + `file:${OTHER}`, + OTHER, + [ + '# Readme', + 'Pull, do not push: use WIP limits to prevent overwhelming reviewers.', + 'Trust is earned through progressive autonomy and Kanban flow management.' + ].join('\n\n') + ); + + return retriever; +} + +describe('fragment retrieval scoping', () => { + // 'adaptive' is pinned in the spanning pair below rather than relying on 'auto': + // auto picks a strategy per query, and on a corpus this small it may happen to return + // hits from one document, which would make the control vacuously pass. + it('should return fragments from every indexed document when unscoped', () => { + const retriever = retrieverWithTwoDocs(); + + const response = retriever.retrieveFragments('kanban WIP limits reviewer', { + maxFragments: 10, + strategy: 'adaptive' + }); + const paths = new Set((response.result ?? []).map(f => f.docPath)); + + expect(paths.has(TARGET)).toBe(true); + expect(paths.has(OTHER)).toBe(true); + }); + + it('should return fragments only from the scoped document', () => { + const retriever = retrieverWithTwoDocs(); + + const response = retriever.retrieveFragments('kanban WIP limits reviewer', { + maxFragments: 10, + strategy: 'adaptive', + scopePath: TARGET + }); + const paths = new Set((response.result ?? []).map(f => f.docPath)); + + expect(paths.has(OTHER)).toBe(false); + expect([...paths]).toEqual([TARGET]); + }); + + it('should scope under the default auto strategy too', () => { + const retriever = retrieverWithTwoDocs(); + + const response = retriever.retrieveFragments('kanban WIP limits reviewer', { + maxFragments: 10, + scopePath: TARGET + }); + + expect((response.result ?? []).every(f => f.docPath === TARGET)).toBe(true); + }); + + it('should still find fragments in the scoped document when another document ranks higher', () => { + const retriever = retrieverWithTwoDocs(); + + // Scoping must not simply filter the unscoped top-N — if the other document's + // passages outrank the target's, filtering afterwards would return nothing. + const response = retriever.retrieveFragments('pull systems prevent overload', { + maxFragments: 1, + scopePath: TARGET + }); + + expect(response.result?.length).toBeGreaterThan(0); + expect(response.result?.[0].docPath).toBe(TARGET); + }); + + it('should return no fragments when scoped to a document that was never indexed', () => { + const retriever = retrieverWithTwoDocs(); + + const response = retriever.retrieveFragments('kanban', { + maxFragments: 5, + scopePath: 'nowhere/missing.md' + }); + + expect(response.result ?? []).toEqual([]); + }); +});