mirror of
https://github.com/aaronsb/obsidian-mcp-plugin.git
synced 2026-07-22 06:45:14 +00:00
Merge pull request #265 from aaronsb/fix/retrieval-affordances
fix(retrieval): make paths usable and steer scan-then-follow
This commit is contained in:
commit
f688a8a61f
13 changed files with 502 additions and 64 deletions
|
|
@ -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<GraphStatsResponse['vaultStatistics']>): 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
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -112,7 +112,6 @@ export {
|
|||
tip,
|
||||
summaryFooter,
|
||||
joinLines,
|
||||
formatPath,
|
||||
formatTree
|
||||
} from './utils';
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -32,20 +32,24 @@ export class UniversalFragmentRetriever {
|
|||
query: string,
|
||||
options: RetrievalOptions = {}
|
||||
): SemanticResponse<Fragment[]> {
|
||||
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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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<void> => {
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -360,12 +360,12 @@ const createSemanticTool = (operation: string, visibility?: ToolVisibility): Sem
|
|||
|
||||
export function getOperationDescription(operation: string): string {
|
||||
const descriptions: Record<string, string> = {
|
||||
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<string, unknown> {
|
|||
},
|
||||
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',
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
66
tests/formatters/graph-vault-stats.test.ts
Normal file
66
tests/formatters/graph-vault-stats.test.ts
Normal file
|
|
@ -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');
|
||||
});
|
||||
});
|
||||
81
tests/formatters/path-fidelity.test.ts
Normal file
81
tests/formatters/path-fidelity.test.ts
Normal file
|
|
@ -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');
|
||||
});
|
||||
});
|
||||
73
tests/formatters/read-frontmatter-dedup.test.ts
Normal file
73
tests/formatters/read-frontmatter-dedup.test.ts
Normal file
|
|
@ -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');
|
||||
});
|
||||
});
|
||||
108
tests/fragments-path-scope.test.ts
Normal file
108
tests/fragments-path-scope.test.ts
Normal file
|
|
@ -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([]);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue