Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@tableau/mcp-server",
"description": "Helping agents see and understand data.",
"version": "2.67.1",
"version": "2.67.2",
"repository": {
"type": "git",
"url": "git+https://github.com/tableau/tableau-mcp.git"
Expand Down
74 changes: 72 additions & 2 deletions src/tools/desktop/authoring/binder/bindTemplate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,30 @@ const MULTI_DATASOURCE_CALC_READBACK_XML = MULTI_DATASOURCE_CALC_BASE_XML.replac
'</datasource></datasources>',
`${INVENTORY_CALC_COLUMN_XML}</datasource></datasources>`,
);
const CAPTIONED_MULTI_DATASOURCE_CALC_BASE_XML = [
"<?xml version='1.0' encoding='utf-8'?>",
"<workbook version='18.1'><datasources>",
"<datasource caption='Orders' inline='true' name='federated.orders'>",
"<connection class='federated'><named-connections>",
"<named-connection caption='Orders' name='textscan.orders' />",
'</named-connections></connection>',
"<column caption='Sales' datatype='real' name='[sales]' role='measure' type='quantitative' />",
'</datasource>',
"<datasource caption='Inventory' inline='true' name='federated.inventory'>",
"<connection class='federated'><named-connections>",
"<named-connection caption='Inventory' name='textscan.inventory' />",
'</named-connections></connection>',
"<column caption='Quantity' datatype='integer' name='[quantity]' role='measure' type='quantitative' />",
'</datasource></datasources>',
"<worksheets><worksheet name='Sheet 1' /></worksheets></workbook>",
].join('');
const CAPTIONED_CALC_COLUMN_XML =
"<column caption='Double Quantity' datatype='real' name='[Calculation_1700000000000]' role='measure' type='quantitative'><calculation class='tableau' formula='[quantity] * 2' /></column>";
const CAPTIONED_MULTI_DATASOURCE_CALC_READBACK_XML =
CAPTIONED_MULTI_DATASOURCE_CALC_BASE_XML.replace(
'</datasource></datasources>',
`${CAPTIONED_CALC_COLUMN_XML}</datasource></datasources>`,
);

const boundResult: BinderResult = {
status: 'bound',
Expand Down Expand Up @@ -782,8 +806,10 @@ describe('bindTemplateTool', () => {
});
expect(paramsSchema['session']!.description).toBe('Desktop PID; omit if pinned or sole.');
expect(paramsSchema['target_worksheet']!.description).toBe('Sheet id/name; omit to add.');
expect(paramsSchema['auto_apply']!.description).toBe('Apply now.');
expect(paramsSchema['datasource']!.description).toBe('Calc source id/name.');
expect(paramsSchema['auto_apply']!.description).toBe('Apply now');
expect(paramsSchema['datasource']!.description).toBe(
'Internal datasource name or unique caption',
);
expect(paramsSchema['calcs']!.description).toBe('Author fields.');
expect(
paramsSchema['calcs']!.safeParse([
Expand Down Expand Up @@ -3339,6 +3365,20 @@ async function getToolResult({
);
}

function datasourceBlock(xml: string, datasourceName: string): string {
let cursor = xml.indexOf('<datasource');
while (cursor !== -1) {
const openEnd = xml.indexOf('>', cursor) + 1;
const openTag = xml.slice(cursor, openEnd);
if (openTag.includes(`name='${datasourceName}'`)) {
const closeEnd = xml.indexOf('</datasource>', openEnd) + '</datasource>'.length;
return xml.slice(cursor, closeEnd);
}
cursor = xml.indexOf('<datasource', openEnd);
}
throw new Error(`missing datasource ${datasourceName}`);
}

/**
* Wire the auto-apply seams for one bind-template call. Returns the executor's
* command/document spies and the `getExecutor` factory to hand
Expand Down Expand Up @@ -4887,6 +4927,36 @@ describe('bindTemplateTool auto_apply gate', () => {
);
});

it('resolves a unique datasource caption before authoring an inline calc', async () => {
vi.spyOn(Date, 'now').mockReturnValue(1_700_000_000_000);
const { applyWorkbookDocument, getExecutor } = setupAutoApplyMocks({
workbookReads: [
CAPTIONED_MULTI_DATASOURCE_CALC_BASE_XML,
CAPTIONED_MULTI_DATASOURCE_CALC_READBACK_XML,
],
});

const result = await getToolResult({
session: '1',
ask: 'bar chart of Double Quantity by Region',
datasource: 'Inventory',
calcs: [{ caption: 'Double Quantity', formula: '[Quantity] * 2' }],
auto_apply: true,
getExecutor,
});

expect(result.isError).toBe(false);
invariant(result.content[0].type === 'text');
expect(JSON.parse(result.content[0].text).authored_calcs).toEqual(['Double Quantity']);
const calcApply = applyWorkbookDocument.mock.calls[0]?.[0] as string;
expect(datasourceBlock(calcApply, 'federated.inventory')).toContain(CAPTIONED_CALC_COLUMN_XML);
expect(datasourceBlock(calcApply, 'federated.orders')).not.toContain(CAPTIONED_CALC_COLUMN_XML);
expect(calcApply).toContain("name='textscan.inventory'");
expect(binderModule.bindTemplate).toHaveBeenCalledWith(
expect.objectContaining({ workbookXml: CAPTIONED_MULTI_DATASOURCE_CALC_READBACK_XML }),
);
});

it('returns an actionable error when the requested calc datasource is absent', async () => {
const { applyWorkbookDocument, getExecutor } = setupAutoApplyMocks({
workbookReads: [MULTI_DATASOURCE_CALC_BASE_XML],
Expand Down
6 changes: 3 additions & 3 deletions src/tools/desktop/authoring/binder/bindTemplate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,16 @@ import { proposalSignature } from './proposalSignature.js';

const paramsSchema = {
session: z.string().optional().describe('Desktop PID; omit if pinned or sole.'),
ask: z.string().describe('Ask.'),
ask: z.string(),
proposal: proposalSchema.optional(),
minConfidence: z.number().min(0).max(1).optional(),
auto_apply: z.boolean().optional().describe('Apply now.'),
auto_apply: z.boolean().optional().describe('Apply now'),
skip_validation: z.boolean().optional(),
// Undescribed, this parameter cost 299 repeat binds and 2,562 seconds: with no way to
// learn that it means "edit THIS sheet", the agent left it out on an edit-in-place ask,
// bind-template created a second sheet, and the follow-up edits chased the new sheet.
target_worksheet: z.string().optional().describe('Sheet id/name; omit to add.'),
datasource: z.string().optional().describe('Calc source id/name.'),
datasource: z.string().optional().describe('Internal datasource name or unique caption'),
calcs: z
.array(
z.object({
Expand Down
71 changes: 66 additions & 5 deletions src/tools/desktop/authoring/datasource/authorAction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ describe('authorActionTool', () => {
vi.clearAllMocks();
});

it('describes datasource selection as name or unique caption', async () => {
const tool = getAuthorActionTool(new DesktopMcpServer());
const paramsSchema = (await Provider.from(tool.paramsSchema)) as Record<
string,
{ description?: string }
>;

expect(paramsSchema['datasource']?.description).toBe('Top-level name/unique caption.');
});

it('creates the workbook-level <actions> block and splices an edit-parameter-action, verifying readback', async () => {
const readbackXml = withActions(
BASE_XML,
Expand Down Expand Up @@ -179,6 +189,58 @@ describe('authorActionTool', () => {
expect(membershipAt).toBeLessThan(paramsAt);
});

it('resolves a unique datasource caption to the internal set target', async () => {
const expectedAction =
"<edit-group-action caption='Expand Category' name='[Action1]'>" +
"<activation type='on-select' />" +
"<source type='sheet' worksheet='Profit' />" +
"<add-or-remove-marks value='assign' />" +
"<params><param name='selection-clear-set-option' value='do-nothing' />" +
"<param name='target-group' value='[federated.1syzfv90anwuu119p4zra1ga299n].[Category Set]' /></params>" +
'</edit-group-action>';
const { result, applyWorkbookDocument } = await getToolResult({
args: {
mode: 'set',
caption: 'Expand Category',
sourceWorksheet: 'Profit',
targetSet: 'Category Set',
datasource: 'Sample - Superstore',
},
readbackXml: withActions(BASE_XML, expectedAction),
});

expect(result.isError).toBe(false);
invariant(result.content[0].type === 'text');
expect(JSON.parse(result.content[0].text).targetSet).toBe(
'[federated.1syzfv90anwuu119p4zra1ga299n].[Category Set]',
);
expect(appliedDocumentXml(applyWorkbookDocument)).toContain(expectedAction);
});

it('rejects a duplicate datasource caption before applying a set action', async () => {
const duplicateCaptionXml = BASE_XML.replace(
'</datasources>',
"<datasource caption='Sample - Superstore' name='federated.duplicate'></datasource></datasources>",
);
const { result, applyWorkbookDocument } = await getToolResult({
args: {
mode: 'set',
caption: 'Expand Category',
sourceWorksheet: 'Profit',
targetSet: 'Category Set',
datasource: 'Sample - Superstore',
},
initialXml: duplicateCaptionXml,
});

expect(result.isError).toBe(true);
invariant(result.content[0].type === 'text');
expect(result.content[0].text).toContain('ambiguous');
expect(result.content[0].text).toContain('federated.1syzfv90anwuu119p4zra1ga299n');
expect(result.content[0].text).toContain('federated.duplicate');
expect(applyWorkbookDocument).not.toHaveBeenCalled();
});

it('accepts set-action readback when Desktop backfills single-select', async () => {
const normalizedAction =
"<edit-group-action caption='Expand Category' name='[Action1]'>" +
Expand Down Expand Up @@ -291,7 +353,8 @@ describe('authorActionTool', () => {

expect(result.isError).toBe(true);
invariant(result.content[0].type === 'text');
expect(result.content[0].text).toContain("datasource 'phantom' matched no datasource");
expect(result.content[0].text).toContain('Datasource "phantom" was not found');
expect(result.content[0].text).not.toContain('Candidates: phantom');
expect(applyWorkbookDocument).not.toHaveBeenCalled();
});

Expand All @@ -308,10 +371,8 @@ describe('authorActionTool', () => {

expect(result.isError).toBe(true);
invariant(result.content[0].type === 'text');
expect(result.content[0].text).toContain(
"datasource 'Missing Datasource' matched no datasource; sets found in:",
);
expect(result.content[0].text).toContain('Category Set');
expect(result.content[0].text).toContain('Datasource "Missing Datasource" was not found');
expect(result.content[0].text).toContain('federated.1syzfv90anwuu119p4zra1ga299n');
expect(applyWorkbookDocument).not.toHaveBeenCalled();
});

Expand Down
55 changes: 8 additions & 47 deletions src/tools/desktop/authoring/datasource/authorAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { DesktopMcpServer } from '../../../../server.desktop.js';
import { sessionParam } from '../../params.js';
import { DesktopTool } from '../../tool.js';
import { applyAndVerify } from './applyAndVerify.js';
import { findDatasourceElements, selectTargetDatasource } from './authorCalcCore.js';

const activationSchema = z.enum(['on-select', 'on-hover', 'on-menu']);
const modeSchema = z.enum(['parameter', 'set']);
Expand All @@ -33,7 +34,7 @@ const paramsSchema = {
sourceField: z.string().optional().describe(''),
targetParameter: z.string().optional().describe(''),
targetSet: z.string().optional().describe(''),
datasource: z.string().optional().describe(''),
datasource: z.string().optional().describe('Top-level name/unique caption.'),
Comment thread
mattcfilbert marked this conversation as resolved.
Outdated
setMembership: setMembershipSchema.default('assign').describe(''),
clearSelection: clearSelectionSchema.default('do-nothing').describe(''),
singleSelect: z.boolean().optional().describe(''),
Expand All @@ -59,12 +60,6 @@ type AuthorActionResult = AuthorActionResultBase &
}
);

type DatasourceElement = {
name: string;
caption?: string;
xml: string;
};

type SetCandidate = {
datasourceName: string;
datasourceCaption?: string;
Expand All @@ -78,7 +73,7 @@ export const getAuthorActionTool = (server: DesktopMcpServer): DesktopTool<typeo
server,
name: 'author-action',
title,
description: 'Author action.',
description: 'Add action.',
paramsSchema,
annotations: {
readOnlyHint: false,
Expand Down Expand Up @@ -358,16 +353,11 @@ function resolveTargetSet(
];
}),
);
const matchedDatasourceElements =
datasource === undefined
? datasourceElements
: datasourceElements.filter(
(element) => element.name === datasource || element.caption === datasource,
);
if (datasource !== undefined && matchedDatasourceElements.length === 0) {
return new ArgsValidationError(
`datasource '${datasource}' matched no datasource; sets found in: ${formatSetCandidates(allCandidates)}`,
).toErr();
let matchedDatasourceElements = datasourceElements;
if (datasource !== undefined) {
const selectedDatasource = selectTargetDatasource(liveXml, datasource);
if (selectedDatasource.isErr()) return selectedDatasource;
matchedDatasourceElements = [selectedDatasource.value];
}
const matchedDatasourceNames = new Set(matchedDatasourceElements.map((element) => element.name));
const candidates = allCandidates.filter((candidate) =>
Expand Down Expand Up @@ -402,35 +392,6 @@ function resolveTargetSet(
return new Ok(`${bracketToken(match.datasourceName)}.${bracketToken(match.name)}`);
}

function findDatasourceElements(xml: string): DatasourceElement[] {
const elements: DatasourceElement[] = [];
const blockStart = xml.indexOf('<datasources>');
const blockEnd = xml.indexOf('</datasources>', blockStart);
const scanFrom = blockStart === -1 ? 0 : blockStart;
const scanTo = blockEnd === -1 ? xml.length : blockEnd;
for (const match of xml.matchAll(/<datasource(?=\s)[^>]*\bname=(?:'[^']*'|"[^"]*")[^>]*>/g)) {
if (match.index < scanFrom || match.index >= scanTo || /\/\s*>$/.test(match[0])) {
continue;
}
const name = getAttr(match[0], 'name');
if (name === undefined) {
continue;
}
const openEnd = match.index + match[0].length;
const closeStart = xml.indexOf('</datasource>', openEnd);
if (closeStart === -1 || closeStart > scanTo) {
continue;
}
const caption = getAttr(match[0], 'caption');
elements.push({
name: unescapeXml(name),
caption: caption === undefined ? undefined : unescapeXml(caption),
xml: xml.slice(match.index, closeStart + '</datasource>'.length),
});
}
return elements;
}

function findGroupTags(xml: string): string[] {
return [...xml.matchAll(/<group\b[^>]*>/g)]
.map((match) => match[0])
Expand Down
Loading
Loading