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([]); + }); +});