mirror of
https://github.com/aaronsb/obsidian-mcp-plugin.git
synced 2026-07-22 06:45:14 +00:00
fix(dataview): dedicated list/metadata formatters so non-raw output renders (#220)
dataview.list and dataview.metadata non-raw output always rendered "No results found": the dispatcher force-cast their {success,count,pages} and {success,path,metadata} producer shapes into the {values} query formatter (formatDataviewQuery reads response.values), which always hit the empty branch. raw:true was unaffected.
A page list and a single page's metadata aren't query results, so add dedicated formatDataviewPages and formatDataviewMetadata and route the two dispatcher cases to them instead of formatDataviewQuery. Both surface producer-side failures (bad source, page-not-found) instead of a misleading empty result.
Adds mock-faithful regression tests through formatResponse() covering both happy paths and failure paths (the #216 mock-faithfulness lesson). Verified live against Dataview 0.5.68.
This commit is contained in:
parent
065a62f638
commit
5436548972
3 changed files with 217 additions and 1 deletions
|
|
@ -192,6 +192,150 @@ export function formatDataviewStatus(response: DataviewStatusResponse): string {
|
|||
return joinLines(lines);
|
||||
}
|
||||
|
||||
/**
|
||||
* Format dataview.list response
|
||||
*
|
||||
* `listPages()` returns `{ success, source, count, pages: [...] }`, not the
|
||||
* `{ values }` shape `formatDataviewQuery` reads — routing it through the query
|
||||
* formatter always hit the `!values` branch and rendered "No results found"
|
||||
* regardless of data (#220). A page list isn't really a query result, so it
|
||||
* gets its own formatter.
|
||||
*/
|
||||
export interface DataviewPagesResponse {
|
||||
success: boolean;
|
||||
source?: string;
|
||||
count?: number;
|
||||
pages?: Array<Record<string, unknown>>;
|
||||
error?: string;
|
||||
}
|
||||
|
||||
export function formatDataviewPages(response: DataviewPagesResponse): string {
|
||||
const lines: string[] = [];
|
||||
|
||||
lines.push(header(1, 'Dataview: Pages'));
|
||||
lines.push('');
|
||||
lines.push(property('Source', response.source ?? 'all', 0));
|
||||
|
||||
if (response.success === false) {
|
||||
lines.push('');
|
||||
lines.push(`❌ Query failed: ${response.error || 'Unknown error'}`);
|
||||
lines.push(summaryFooter());
|
||||
return joinLines(lines);
|
||||
}
|
||||
|
||||
const pages = response.pages ?? [];
|
||||
const total = response.count ?? pages.length;
|
||||
lines.push(property('Count', String(total), 0));
|
||||
lines.push('');
|
||||
|
||||
if (pages.length === 0) {
|
||||
lines.push('No pages found.');
|
||||
lines.push(summaryFooter());
|
||||
return joinLines(lines);
|
||||
}
|
||||
|
||||
pages.slice(0, 30).forEach((page, i) => {
|
||||
const path = typeof page.path === 'string' ? page.path : JSON.stringify(page);
|
||||
lines.push(`${i + 1}. ${truncate(path, 60)}`);
|
||||
});
|
||||
|
||||
if (total > 30) {
|
||||
lines.push(`\n... and ${total - 30} more pages`);
|
||||
}
|
||||
|
||||
lines.push(divider());
|
||||
lines.push(tip('Use `dataview.metadata(path)` for one page, or `vault.read(path)` to open it'));
|
||||
lines.push(summaryFooter());
|
||||
|
||||
return joinLines(lines);
|
||||
}
|
||||
|
||||
/**
|
||||
* Format dataview.metadata response
|
||||
*
|
||||
* `getPageMetadata()` returns `{ success, path, metadata: {...} }` — same #220
|
||||
* shape mismatch as the page list. A single page's metadata is not a query
|
||||
* result either, so it gets a dedicated formatter rather than the `{ values }`
|
||||
* query path.
|
||||
*/
|
||||
export interface DataviewMetadataResponse {
|
||||
success: boolean;
|
||||
path: string;
|
||||
metadata?: {
|
||||
file?: Record<string, unknown>;
|
||||
tags?: unknown[];
|
||||
aliases?: unknown[];
|
||||
outlinks?: unknown[];
|
||||
inlinks?: unknown[];
|
||||
tasks?: number;
|
||||
lists?: number;
|
||||
custom?: Record<string, unknown>;
|
||||
};
|
||||
error?: string;
|
||||
}
|
||||
|
||||
export function formatDataviewMetadata(response: DataviewMetadataResponse): string {
|
||||
const lines: string[] = [];
|
||||
|
||||
lines.push(header(1, 'Dataview: Metadata'));
|
||||
lines.push('');
|
||||
lines.push(property('Path', response.path, 0));
|
||||
|
||||
if (response.success === false || !response.metadata) {
|
||||
lines.push('');
|
||||
lines.push(`❌ ${response.error || 'No metadata available'}`);
|
||||
lines.push(summaryFooter());
|
||||
return joinLines(lines);
|
||||
}
|
||||
|
||||
const m = response.metadata;
|
||||
const tags = Array.isArray(m.tags) ? m.tags : [];
|
||||
const aliases = Array.isArray(m.aliases) ? m.aliases : [];
|
||||
const outlinks = Array.isArray(m.outlinks) ? m.outlinks : [];
|
||||
const inlinks = Array.isArray(m.inlinks) ? m.inlinks : [];
|
||||
|
||||
lines.push('');
|
||||
if (tags.length > 0) {
|
||||
lines.push(property('Tags', tags.map(t => String(t)).join(', '), 0));
|
||||
}
|
||||
if (aliases.length > 0) {
|
||||
lines.push(property('Aliases', aliases.map(a => String(a)).join(', '), 0));
|
||||
}
|
||||
lines.push(property('Outlinks', String(outlinks.length), 0));
|
||||
lines.push(property('Inlinks', String(inlinks.length), 0));
|
||||
lines.push(property('Tasks', String(m.tasks ?? 0), 0));
|
||||
lines.push(property('Lists', String(m.lists ?? 0), 0));
|
||||
|
||||
const custom = (m.custom && typeof m.custom === 'object') ? m.custom : {};
|
||||
const customKeys = Object.keys(custom);
|
||||
if (customKeys.length > 0) {
|
||||
lines.push('');
|
||||
lines.push(header(2, 'Frontmatter'));
|
||||
customKeys.slice(0, 15).forEach(key => {
|
||||
const value = custom[key];
|
||||
let display: string;
|
||||
if (value === null || value === undefined) {
|
||||
display = String(value);
|
||||
} else if (typeof value === 'object') {
|
||||
display = JSON.stringify(value);
|
||||
} else {
|
||||
const primitive = value as string | number | boolean | bigint | symbol;
|
||||
display = String(primitive);
|
||||
}
|
||||
lines.push(property(key, truncate(display, 50), 0));
|
||||
});
|
||||
if (customKeys.length > 15) {
|
||||
lines.push(`... and ${customKeys.length - 15} more fields`);
|
||||
}
|
||||
}
|
||||
|
||||
lines.push(divider());
|
||||
lines.push(tip('Use `vault.read(path)` to view the full note'));
|
||||
lines.push(summaryFooter());
|
||||
|
||||
return joinLines(lines);
|
||||
}
|
||||
|
||||
/**
|
||||
* Format bases.query response
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -67,6 +67,8 @@ import {
|
|||
import {
|
||||
formatDataviewQuery,
|
||||
formatDataviewStatus,
|
||||
formatDataviewPages,
|
||||
formatDataviewMetadata,
|
||||
formatBasesQuery,
|
||||
formatBasesList,
|
||||
formatBasesRead,
|
||||
|
|
@ -74,6 +76,8 @@ import {
|
|||
formatBasesExport,
|
||||
DataviewQueryResponse,
|
||||
DataviewStatusResponse,
|
||||
DataviewPagesResponse,
|
||||
DataviewMetadataResponse,
|
||||
BasesQueryResponse,
|
||||
BasesListResponse,
|
||||
BasesReadResponse,
|
||||
|
|
@ -167,6 +171,8 @@ export {
|
|||
// Dataview
|
||||
formatDataviewQuery,
|
||||
formatDataviewStatus,
|
||||
formatDataviewPages,
|
||||
formatDataviewMetadata,
|
||||
formatBasesQuery,
|
||||
formatBasesList,
|
||||
formatBasesRead,
|
||||
|
|
@ -174,6 +180,8 @@ export {
|
|||
formatBasesExport,
|
||||
DataviewQueryResponse,
|
||||
DataviewStatusResponse,
|
||||
DataviewPagesResponse,
|
||||
DataviewMetadataResponse,
|
||||
BasesQueryResponse,
|
||||
BasesListResponse,
|
||||
BasesReadResponse,
|
||||
|
|
@ -623,8 +631,9 @@ export function formatResponse(
|
|||
case 'dataview.status':
|
||||
return formatDataviewStatus(normalized as DataviewStatusResponse);
|
||||
case 'dataview.list':
|
||||
return formatDataviewPages(normalized as DataviewPagesResponse);
|
||||
case 'dataview.metadata':
|
||||
return formatDataviewQuery({ ...(normalized as Record<string, unknown>), type: 'list', successful: true } as DataviewQueryResponse);
|
||||
return formatDataviewMetadata(normalized as DataviewMetadataResponse);
|
||||
|
||||
// Bases operations
|
||||
case 'bases.list':
|
||||
|
|
|
|||
|
|
@ -423,6 +423,69 @@ describe('Dataview Integration', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// #220: dataview.list / dataview.metadata non-raw output always rendered
|
||||
// "No results found" because their {count,pages} / {metadata} producer shapes
|
||||
// were force-cast into the {values} query formatter. These exercise the full
|
||||
// producer → formatResponse() path with the real Dataview producer shapes.
|
||||
describe('Formatted output (#220)', () => {
|
||||
test('dataview.list renders page paths and count, not "No results found"', () => {
|
||||
const app = new MockApp(true, true);
|
||||
const api = new MockObsidianAPI(app);
|
||||
const tool = new DataviewTool(api as any);
|
||||
|
||||
const output = formatResponse('dataview', 'list', tool.listPages('#tag'), false);
|
||||
|
||||
expect(output).toContain('Dataview: Pages');
|
||||
expect(output).not.toContain('No results found');
|
||||
expect(output).toContain('Count');
|
||||
expect(output).toContain('Note1.md');
|
||||
expect(output).toContain('Note2.md');
|
||||
});
|
||||
|
||||
test('dataview.metadata renders page metadata and frontmatter, not "No results found"', () => {
|
||||
const app = new MockApp(true, true);
|
||||
const api = new MockObsidianAPI(app);
|
||||
const tool = new DataviewTool(api as any);
|
||||
|
||||
const output = formatResponse('dataview', 'metadata', tool.getPageMetadata('Note1.md'), false);
|
||||
|
||||
expect(output).toContain('Dataview: Metadata');
|
||||
expect(output).not.toContain('No results found');
|
||||
expect(output).toContain('Note1.md');
|
||||
// custom frontmatter fields surfaced
|
||||
expect(output).toContain('priority');
|
||||
expect(output).toContain('high');
|
||||
});
|
||||
|
||||
test('dataview.list surfaces a producer failure instead of a false "No results found"', () => {
|
||||
const app = new MockApp(true, true);
|
||||
const api = new MockObsidianAPI(app);
|
||||
const tool = new DataviewTool(api as any);
|
||||
|
||||
const dvApi = (app.plugins.plugins['dataview'] as any).api as MockDataviewAPI;
|
||||
dvApi.pages = (() => { throw new Error('bad source'); }) as any;
|
||||
|
||||
const output = formatResponse('dataview', 'list', tool.listPages('nonsense'), false);
|
||||
|
||||
expect(output).toContain('Query failed');
|
||||
expect(output).toContain('bad source');
|
||||
expect(output).not.toContain('No results found');
|
||||
});
|
||||
|
||||
test('dataview.metadata surfaces a page-not-found failure', () => {
|
||||
const app = new MockApp(true, true);
|
||||
const api = new MockObsidianAPI(app);
|
||||
const tool = new DataviewTool(api as any);
|
||||
|
||||
// page() returns null for an unknown path → producer returns {success:false, error}
|
||||
const output = formatResponse('dataview', 'metadata', tool.getPageMetadata('Missing.md'), false);
|
||||
|
||||
expect(output).toContain('Dataview: Metadata');
|
||||
expect(output).toContain('Missing.md');
|
||||
expect(output).toContain('Page not found');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Tool Availability Detection', () => {
|
||||
test('should detect when Dataview tool is not available', () => {
|
||||
const app = new MockApp(false);
|
||||
|
|
|
|||
Loading…
Reference in a new issue