diff --git a/src/formatters/dataview.ts b/src/formatters/dataview.ts index 9c86fac..23eaf9d 100644 --- a/src/formatters/dataview.ts +++ b/src/formatters/dataview.ts @@ -67,6 +67,40 @@ export function formatDataviewQuery(response: DataviewQueryResponse): string { return joinLines(lines); } +/** + * Render a single table cell value. Dataview hands back rich objects — `Link` + * (`{ path, display }`) and Luxon `DateTime` (`toISO()`) — not primitives. + * Without coercion they dumped as raw JSON (`{"path":…}`) and quoted ISO + * strings (#220). Collapse a Link to its display/path, a DateTime to ISO, and + * leave primitives as-is. + */ +function coerceCell(val: unknown): string { + if (val === null || val === undefined) { + return ''; + } + if (typeof val === 'string') { + return val; + } + if (typeof val === 'number' || typeof val === 'boolean' || typeof val === 'bigint') { + return String(val); + } + if (typeof val === 'object') { + const obj = val as Record; + if (typeof obj.path === 'string') { + return typeof obj.display === 'string' && obj.display ? obj.display : obj.path; + } + if (typeof obj.toISO === 'function') { + const iso = (obj.toISO as () => unknown)(); + if (typeof iso === 'string' && iso) return iso; + } + if (val instanceof Date) { + return val.toISOString(); + } + return JSON.stringify(val); + } + return JSON.stringify(val); +} + function formatDataviewTable(headers: string[], rows: DataviewValue[]): string { const lines: string[] = []; @@ -87,16 +121,7 @@ function formatDataviewTable(headers: string[], rows: DataviewValue[]): string { } else if (row !== null && typeof row === 'object') { val = row[headers[i]]; } - let display: string; - if (val === null || val === undefined) { - display = ''; - } else if (typeof val === 'object') { - display = JSON.stringify(val); - } else { - const primitive = val as string | number | boolean | bigint | symbol; - display = String(primitive); - } - return truncate(display, 30); + return truncate(coerceCell(val), 30); }); lines.push('| ' + cells.join(' | ') + (hasMore ? ' | ...' : '') + ' |'); }); @@ -108,19 +133,99 @@ function formatDataviewTable(headers: string[], rows: DataviewValue[]): string { return lines.join('\n'); } +/** A GROUP BY result element. Dataview wraps each group as a list-pair + * (`{ $widget: 'dataview:list-pair', key, value }` for LIST) or `{ key, rows }` + * for table-style groups. Without unwrapping, the row formatters rendered the + * wrapper object itself instead of its members (#220). */ +interface DataviewGroup { + key: unknown; + rows: DataviewValue[]; +} + +function asGroup(item: DataviewValue): DataviewGroup | null { + if (item === null || typeof item !== 'object' || Array.isArray(item)) { + return null; + } + const obj = item; + if (!('key' in obj || obj.$widget === 'dataview:list-pair')) { + return null; + } + const inner = obj.rows ?? obj.value; + // The producer (`unwrapGroup`) normally flattens the inner DataArray to a + // plain array before a group reaches here. Accept a raw DataArray (`.array()`) + // too, so the formatter stays in agreement with the producer's group + // predicate rather than silently leaking a wrapper if an unflattened group + // ever reaches it (PR #249 review, F2). + if (Array.isArray(inner)) { + return { key: obj.key, rows: inner as DataviewValue[] }; + } + if (inner && typeof (inner as { array?: () => unknown[] }).array === 'function') { + return { key: obj.key, rows: (inner as { array: () => DataviewValue[] }).array() }; + } + return null; +} + +function groupKeyLabel(key: unknown): string { + if (key === null || key === undefined || key === '') { + return '(no group)'; + } + if (typeof key === 'string') { + return key; + } + if (typeof key === 'number' || typeof key === 'bigint' || typeof key === 'boolean') { + return String(key); + } + if (typeof key === 'object') { + const obj = key as Record; + const filePath = obj.path ?? (obj.file as Record | undefined)?.path; + return typeof filePath === 'string' ? filePath : JSON.stringify(key); + } + return JSON.stringify(key); +} + +function renderListItem(item: DataviewValue): string { + if (item && typeof item === 'object') { + const obj = item as Record; + const filePath = obj.path ?? (obj.file as Record | undefined)?.path; + return typeof filePath === 'string' ? filePath : JSON.stringify(item); + } + return String(item); +} + function formatDataviewList(items: DataviewValue[]): string { + // GROUP BY: elements arrive as list-pair group wrappers — render each group's + // key and its rows, not the wrapper object (#220). Switch to grouped + // rendering if *any* element is a group (not all), and render a stray + // non-group element as a plain top-level row, so one unrecognized group can't + // collapse the whole result back to raw wrappers (PR #249 review, F1). + const groups = items.map(asGroup); + if (items.length > 0 && groups.some(g => g !== null)) { + const lines: string[] = []; + items.slice(0, 20).forEach((item, i) => { + const group = groups[i]; + if (group) { + lines.push(`**${truncate(groupKeyLabel(group.key), 60)}** (${group.rows.length})`); + group.rows.slice(0, 30).forEach(row => { + lines.push(`- ${truncate(renderListItem(row), 60)}`); + }); + if (group.rows.length > 30) { + lines.push(` ... and ${group.rows.length - 30} more`); + } + } else { + lines.push(`- ${truncate(renderListItem(item), 60)}`); + } + lines.push(''); + }); + if (items.length > 20) { + lines.push(`... and ${items.length - 20} more groups`); + } + return lines.join('\n').trimEnd(); + } + const lines: string[] = []; items.slice(0, 30).forEach((item, i) => { - let text: string; - if (item && typeof item === 'object') { - const obj = item as Record; - const filePath = obj.path ?? (obj.file as Record | undefined)?.path; - text = typeof filePath === 'string' ? filePath : JSON.stringify(item); - } else { - text = String(item); - } - lines.push(`${i + 1}. ${truncate(text, 60)}`); + lines.push(`${i + 1}. ${truncate(renderListItem(item), 60)}`); }); if (items.length > 30) { @@ -137,22 +242,53 @@ interface DataviewTask { path?: string; } +function renderTask(taskItem: DataviewValue): string { + const task = (taskItem !== null && typeof taskItem === 'object' && !Array.isArray(taskItem) + ? taskItem + : {}) as DataviewTask; + const checkbox = task.completed ? '[x]' : '[ ]'; + const taskString = taskItem !== null && typeof taskItem === 'object' + ? JSON.stringify(taskItem) + : String(taskItem); + const text = task.text || task.task || taskString; + let line = `- ${checkbox} ${truncate(text, 60)}`; + if (task.path) { + line += `\n from: ${task.path}`; + } + return line; +} + function formatDataviewTasks(tasks: DataviewValue[]): string { + // GROUP BY: elements arrive as group wrappers — render each group's key then + // its tasks (#220). Switch to grouped rendering if *any* element is a group + // (not all), and render a stray non-group element as a plain task, so one + // unrecognized group can't collapse the whole result (PR #249 review, F1). + const groups = tasks.map(asGroup); + if (tasks.length > 0 && groups.some(g => g !== null)) { + const lines: string[] = []; + tasks.slice(0, 20).forEach((taskItem, i) => { + const group = groups[i]; + if (group) { + lines.push(`**${truncate(groupKeyLabel(group.key), 60)}** (${group.rows.length})`); + group.rows.slice(0, 30).forEach(row => lines.push(renderTask(row))); + if (group.rows.length > 30) { + lines.push(` ... and ${group.rows.length - 30} more`); + } + } else { + lines.push(renderTask(taskItem)); + } + lines.push(''); + }); + if (tasks.length > 20) { + lines.push(`... and ${tasks.length - 20} more groups`); + } + return lines.join('\n').trimEnd(); + } + const lines: string[] = []; tasks.slice(0, 30).forEach(taskItem => { - const task = (taskItem !== null && typeof taskItem === 'object' && !Array.isArray(taskItem) - ? taskItem - : {}) as DataviewTask; - const checkbox = task.completed ? '[x]' : '[ ]'; - const taskString = taskItem !== null && typeof taskItem === 'object' - ? JSON.stringify(taskItem) - : String(taskItem); - const text = task.text || task.task || taskString; - lines.push(`- ${checkbox} ${truncate(text, 60)}`); - if (task.path) { - lines.push(` from: ${task.path}`); - } + lines.push(renderTask(taskItem)); }); if (tasks.length > 30) { @@ -192,6 +328,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>; + 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; + tags?: unknown[]; + aliases?: unknown[]; + outlinks?: unknown[]; + inlinks?: unknown[]; + tasks?: number; + lists?: number; + custom?: Record; + }; + 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 */ diff --git a/src/formatters/index.ts b/src/formatters/index.ts index 5781822..3894d54 100644 --- a/src/formatters/index.ts +++ b/src/formatters/index.ts @@ -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), type: 'list', successful: true } as DataviewQueryResponse); + return formatDataviewMetadata(normalized as DataviewMetadataResponse); // Bases operations case 'bases.list': diff --git a/src/tools/dataview-tool.ts b/src/tools/dataview-tool.ts index 1190152..1284ad5 100644 --- a/src/tools/dataview-tool.ts +++ b/src/tools/dataview-tool.ts @@ -58,6 +58,25 @@ function toPlainArray(value: unknown): unknown[] { return []; } +/** + * GROUP BY results nest rows under a group wrapper — `{ key, rows }`, or a + * list-pair `{ $widget: 'dataview:list-pair', key, value }` for LIST. The inner + * collection may itself be a Dataview DataArray, so it's run through + * toPlainArray. Returns null when the element is a plain row, not a group. + */ +function unwrapGroup(el: unknown): { key: unknown; rows: unknown[] } | null { + if (!el || typeof el !== 'object' || Array.isArray(el)) return null; + const obj = el as Record; + const inner = obj.rows ?? obj.value; + const isGroup = 'key' in obj || obj.$widget === 'dataview:list-pair'; + if (!isGroup) return null; + if (!Array.isArray(inner) && !(inner && typeof (inner as { array?: () => unknown[] }).array === 'function')) { + return null; + } + return { key: obj.key, rows: toPlainArray(inner) }; +} + + /** Dataview link value */ interface DataviewLink { path: string; @@ -373,9 +392,14 @@ export class DataviewTool { // Handle different result types switch (payload.type) { case 'list': + // GROUP BY: keep each group as {key, rows} (rows flattened to a plain + // array) instead of leaking the list-pair wrapper downstream (#220). return { type: 'list', - values: toPlainArray(payload.values) + values: toPlainArray(payload.values).map((el: unknown) => { + const group = unwrapGroup(el); + return group ? { key: group.key, rows: group.rows } : el; + }) }; case 'table': return { @@ -386,19 +410,26 @@ export class DataviewTool { return typeof tableRow?.array === 'function' ? tableRow.array() : row; }) }; - case 'task': + case 'task': { + const mapTask = (task: unknown) => { + const dvTask = task as DataviewTask; + return { + text: dvTask.text, + completed: dvTask.completed, + line: dvTask.line, + path: dvTask.path + }; + }; + // GROUP BY: preserve the group wrapper and map its inner rows as tasks, + // rather than mangling each group into an empty task (#220). return { type: 'task', - values: toPlainArray(payload.values).map((task: unknown) => { - const dvTask = task as DataviewTask; - return { - text: dvTask.text, - completed: dvTask.completed, - line: dvTask.line, - path: dvTask.path - }; + values: toPlainArray(payload.values).map((el: unknown) => { + const group = unwrapGroup(el); + return group ? { key: group.key, rows: group.rows.map(mapTask) } : mapTask(el); }) }; + } case 'calendar': return { type: 'calendar', diff --git a/tests/dataview-integration.test.ts b/tests/dataview-integration.test.ts index 6ec70d5..e2ae36c 100644 --- a/tests/dataview-integration.test.ts +++ b/tests/dataview-integration.test.ts @@ -41,6 +41,45 @@ class MockDataviewAPI { // monad — { successful, value, error } — where value.values is a PLAIN // array (not a wrapped DataArray). The previous mock used the wrong flat + // wrapped shape, which is why CI stayed green while real Dataview broke (#216). + + // GROUP BY: Dataview nests rows under group wrappers, and the collections + // are DataArrays (`.array()`), not plain arrays. Mocked faithfully so the + // producer's group-unwrap + flatten is actually exercised (#220). + if (query.includes('GROUP BY') && query.includes('TASK')) { + return { + successful: true, + value: { + type: 'task', + values: { + array: () => [ + { key: 'open', rows: { array: () => [ + { text: 'task one', completed: false, path: 'Note1.md' }, + { text: 'task two', completed: true, path: 'Note2.md' } + ] } }, + { key: null, rows: { array: () => [ + { text: 'ungrouped task', completed: false, path: 'Note3.md' } + ] } } + ] + } + } + }; + } + + if (query.includes('GROUP BY')) { + return { + successful: true, + value: { + type: 'list', + values: { + array: () => [ + { $widget: 'dataview:list-pair', key: 'groupA', value: { array: () => ['Note 1', 'Note 2'] } }, + { $widget: 'dataview:list-pair', key: null, value: { array: () => ['Note 3'] } } + ] + } + } + }; + } + if (query.includes('LIST')) { return { successful: true, @@ -51,6 +90,28 @@ class MockDataviewAPI { }; } + if (query.includes('TABLE') && query.includes('mtime')) { + // Rich TABLE: rows are DataArrays of cells; cells are a Link object and a + // Luxon-style DateTime (toISO), not primitives (#220 cell rendering). + return { + successful: true, + value: { + type: 'table', + headers: ['File', 'file.mtime'], + values: [ + { array: () => [ + { path: 'Note1.md', display: 'Note1', type: 'file' }, + { toISO: () => '2026-01-02T03:04:05.000Z' } + ] }, + { array: () => [ + { path: 'Note2.md' }, + { toISO: () => '2026-01-03T00:00:00.000Z' } + ] } + ] + } + }; + } + if (query.includes('TABLE')) { return { successful: true, @@ -423,6 +484,133 @@ 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'); + }); + }); + + // #220 (folded in): GROUP BY queries nest rows under group wrappers; the + // list/task branches rendered the wrappers (or, for tasks, mangled them into + // empty tasks) instead of the grouped rows. Driven end-to-end through + // executeQuery → formatResponse with DataArray-wrapped fixtures. + describe('GROUP BY formatted output (#220)', () => { + test('LIST ... GROUP BY renders group keys and rows, not the list-pair wrapper', async () => { + const app = new MockApp(true, true); + const api = new MockObsidianAPI(app); + const tool = new DataviewTool(api as any); + + const result = await tool.executeQuery('LIST rows.file.name FROM #tag GROUP BY group'); + const output = formatResponse('dataview', 'query', result, false); + + expect(output).toContain('Dataview: LIST'); + expect(output).not.toContain('No results found'); + expect(output).not.toContain('list-pair'); // wrapper not leaked + expect(output).not.toContain('$widget'); + expect(output).toContain('groupA'); + expect(output).toContain('Note 1'); + expect(output).toContain('Note 2'); + expect(output).toContain('(no group)'); // null group key + expect(output).toContain('Note 3'); + }); + + test('TASK ... GROUP BY renders group keys and their tasks', async () => { + const app = new MockApp(true, true); + const api = new MockObsidianAPI(app); + const tool = new DataviewTool(api as any); + + const result = await tool.executeQuery('TASK FROM #tag GROUP BY status'); + const output = formatResponse('dataview', 'query', result, false); + + expect(output).toContain('Dataview: TASK'); + expect(output).not.toContain('No results found'); + expect(output).toContain('open'); // group key + expect(output).toContain('task one'); + expect(output).toContain('[x] task two'); // completed checkbox preserved + expect(output).toContain('(no group)'); + expect(output).toContain('ungrouped task'); + }); + }); + + // #220 (folded in, from the maintainer's comment): TABLE cells that are + // Dataview Link / DateTime objects rendered as raw JSON / quoted ISO strings. + describe('TABLE cell rendering (#220)', () => { + test('Link cells collapse to display/path and dates render unquoted', async () => { + const app = new MockApp(true, true); + const api = new MockObsidianAPI(app); + const tool = new DataviewTool(api as any); + + const result = await tool.executeQuery('TABLE file.mtime FROM ""'); + const output = formatResponse('dataview', 'query', result, false); + + expect(output).toContain('Dataview: TABLE'); + // Link → display (first row) / path (second row), not raw JSON + expect(output).not.toContain('{"path"'); + expect(output).toContain('Note1'); // display + expect(output).toContain('Note2.md'); // path fallback + // DateTime → bare ISO, no surrounding quotes + expect(output).toContain('2026-01-02T03:04:05.000Z'); + expect(output).not.toContain('"2026-01-02T03:04:05.000Z"'); + }); + }); + describe('Tool Availability Detection', () => { test('should detect when Dataview tool is not available', () => { const app = new MockApp(false);