mirror of
https://github.com/aaronsb/obsidian-mcp-plugin.git
synced 2026-07-22 06:45:14 +00:00
fix(dataview): render GROUP BY results (list + task) instead of leaking group wrappers (#220)
GROUP BY nests rows under a group wrapper — {key, rows}, or a list-pair {$widget:'dataview:list-pair', key, value} for LIST. The producer's list branch passed wrappers through (the formatter then rendered them raw) and the task branch mangled each group into an empty task, so grouped rows never showed.
formatQueryResult now unwraps groups (flattening inner DataArrays to plain arrays) in the list and task branches, and the list/task formatters render each group's key and rows. Adds DataArray-wrapped GROUP BY fixtures + tests through executeQuery -> formatResponse (mock-faithful).
LIST GROUP BY verified live on Dataview 0.5.68; TASK GROUP BY covered by synthetic fixtures (no grouped-task data in the test vault). TABLE GROUP BY is outside #220's stated scope.
This commit is contained in:
Earl Plak
parent
5436548972
commit
1c4183116a
3 changed files with 227 additions and 31 deletions
|
|
@ -108,19 +108,80 @@ 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;
|
||||
const rows = obj.rows ?? obj.value;
|
||||
if (Array.isArray(rows) && ('key' in obj || obj.$widget === 'dataview:list-pair')) {
|
||||
return { key: obj.key, rows: rows as DataviewValue[] };
|
||||
}
|
||||
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<string, unknown>;
|
||||
const filePath = obj.path ?? (obj.file as Record<string, unknown> | 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<string, unknown>;
|
||||
const filePath = obj.path ?? (obj.file as Record<string, unknown> | undefined)?.path;
|
||||
return typeof filePath === 'string' ? filePath : JSON.stringify(item);
|
||||
}
|
||||
return String(item);
|
||||
}
|
||||
|
||||
function formatDataviewList(items: DataviewValue[]): string {
|
||||
// GROUP BY: every element is a list-pair group — render each group's key and
|
||||
// its rows, not the wrapper object (#220).
|
||||
const groups = items.map(asGroup);
|
||||
if (items.length > 0 && groups.every(g => g !== null)) {
|
||||
const lines: string[] = [];
|
||||
groups.slice(0, 20).forEach(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`);
|
||||
}
|
||||
lines.push('');
|
||||
});
|
||||
if (groups.length > 20) {
|
||||
lines.push(`... and ${groups.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<string, unknown>;
|
||||
const filePath = obj.path ?? (obj.file as Record<string, unknown> | 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 +198,45 @@ 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: every element is a group — render its key then its tasks (#220).
|
||||
const groups = tasks.map(asGroup);
|
||||
if (tasks.length > 0 && groups.every(g => g !== null)) {
|
||||
const lines: string[] = [];
|
||||
groups.slice(0, 20).forEach(group => {
|
||||
lines.push(`**${truncate(groupKeyLabel(group.key), 60)}** (${group.rows.length})`);
|
||||
group.rows.slice(0, 30).forEach(taskItem => lines.push(renderTask(taskItem)));
|
||||
if (group.rows.length > 30) {
|
||||
lines.push(` ... and ${group.rows.length - 30} more`);
|
||||
}
|
||||
lines.push('');
|
||||
});
|
||||
if (groups.length > 20) {
|
||||
lines.push(`... and ${groups.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) {
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>;
|
||||
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',
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
@ -486,6 +525,48 @@ describe('Dataview Integration', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// #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');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Tool Availability Detection', () => {
|
||||
test('should detect when Dataview tool is not available', () => {
|
||||
const app = new MockApp(false);
|
||||
|
|
|
|||
Loading…
Reference in a new issue