Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
72 changes: 72 additions & 0 deletions src/desktop/binder/binder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,55 @@ describe('binder/bindTemplate — Call 2 (agent proposal)', () => {
if (res.status === 'escalate') expect(res.reason).toBe('low-confidence');
});

it('threads sort and top_n from a valid proposal into bound args', async () => {
const proposal: BindingProposal = {
template: 'ranking-ordered-bar',
title: 'Top Sales by Region',
bindings: [
{ slot_id: 'region', field: 'Region' },
{ slot_id: 'sales', field: 'Sales' },
],
sort: { by: 'Sales', direction: 'desc' },
top_n: 10,
confidence: 0.9,
};
const res = await bindTemplate({
ask: 'top 10 regions by sales',
workbookXml: WORKBOOK_XML,
manifests,
proposal,
});
expect(res.status).toBe('bound');
if (res.status === 'bound') {
expect(res.args.sort).toEqual({ by: 'Sales', direction: 'desc' });
expect(res.args.top_n).toBe(10);
}
});

it('bad sort.by escalates before apply can use a broken field', async () => {
const proposal: BindingProposal = {
template: 'ranking-ordered-bar',
title: 'Top Sales by Region',
bindings: [
{ slot_id: 'region', field: 'Region' },
{ slot_id: 'sales', field: 'Sales' },
],
sort: { by: 'Definitely Not A Field', direction: 'desc' },
confidence: 0.9,
};
const res = await bindTemplate({
ask: 'regions by sales',
workbookXml: WORKBOOK_XML,
manifests,
proposal,
});
expect(res.status).toBe('escalate');
if (res.status === 'escalate') {
expect(res.reason).toBe('field-not-found');
expect(res.blockers[0].detail).toContain('Definitely Not A Field');
}
});

it('unresolvable field → escalate field-not-found (carries candidates)', async () => {
const proposal: BindingProposal = {
template: 'ranking-ordered-bar',
Expand Down Expand Up @@ -459,6 +508,29 @@ describe('binder/PROPOSAL_OUTPUT_SCHEMA — optional derivation field', () => {
// derivation is optional: not in the required list.
expect(schema.properties.bindings.items.required).not.toContain('derivation');
});

it('advertises optional sort and top_n proposal fields', () => {
const schema = PROPOSAL_OUTPUT_SCHEMA as {
properties: Record<string, unknown>;
required: string[];
};
expect(schema.properties.sort).toEqual({
type: 'object',
additionalProperties: false,
required: ['by', 'direction'],
properties: {
by: { type: 'string', description: 'Sort field.' },
direction: { type: 'string', enum: ['asc', 'desc'], description: 'Sort dir.' },
},
});
expect(schema.properties.top_n).toEqual({
type: 'integer',
minimum: 1,
description: 'Top N.',
});
expect(schema.required).not.toContain('sort');
expect(schema.required).not.toContain('top_n');
});
});

describe('binder/bindTemplate — avoid_when consumption (H3.2)', () => {
Expand Down
64 changes: 63 additions & 1 deletion src/desktop/binder/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,20 @@ import {
type BindingProposal,
type Blocker,
type EscalateReason,
resolveInSummary,
validateBinding,
} from './validate.js';

// Re-exported as the binder's public surface. Bare (source-less) re-exports of the
// locally-imported bindings — a single `export ... from './x.js'` alongside the
// import above would trip the target's `no-duplicate-imports` (includeExports).
export { classifyNoLlm, MAX_CLASSIFIABLE_FIELDS, summarizeSchema, validateBinding };
export {
classifyNoLlm,
MAX_CLASSIFIABLE_FIELDS,
resolveInSummary,
summarizeSchema,
validateBinding,
};
export type { BindingProposal, Blocker, EscalateReason, SchemaField, SchemaSummary };

type ProposeField = CoreLlmProposeInput['fields'][number] & { semanticRole?: string };
Expand Down Expand Up @@ -98,6 +105,8 @@ export interface InjectTemplateArgs {
sheet_type: 'worksheet';
template_parameters: { DATASOURCE: string } & Record<string, string>;
field_mapping: Record<string, string>;
sort?: { by: string; direction: 'asc' | 'desc' };
top_n?: number;
}

export type LlmProposeFn = (input: LlmProposeInput) => Promise<BindingProposal>;
Expand Down Expand Up @@ -199,6 +208,16 @@ export const PROPOSAL_OUTPUT_SCHEMA: Record<string, unknown> = {
},
},
confidence: { type: 'number', minimum: 0, maximum: 1 },
sort: {
type: 'object',
additionalProperties: false,
required: ['by', 'direction'],
properties: {
by: { type: 'string', description: 'Sort field.' },
direction: { type: 'string', enum: ['asc', 'desc'], description: 'Sort dir.' },
},
},
top_n: { type: 'integer', minimum: 1, description: 'Top N.' },
},
};

Expand Down Expand Up @@ -278,6 +297,47 @@ function validateAndBuild(
return { status: 'escalate', reason, blockers: v.blockers, proposal };
}

if (proposal.sort) {
const sortField = resolveInSummary(summary, proposal.sort.by);
if (sortField.kind === 'ambiguous') {
return {
status: 'escalate',
reason: 'ambiguous-field',
blockers: [
{
code: 'ambiguous-field',
detail: `"${proposal.sort.by}" matches ${sortField.candidates?.length ?? 0} sort fields; disambiguate before binding`,
candidates: (sortField.candidates ?? []).map((c) => c.column_ref),
},
],
proposal,
};
}
if (sortField.kind === 'not_found' || !sortField.field) {
return {
status: 'escalate',
reason: 'field-not-found',
blockers: [
{
code: 'field-not-found',
detail: `no sort.by field named "${proposal.sort.by}" in datasource(s)`,
candidates: (sortField.candidates ?? []).map((c) => c.column_ref),
},
],
proposal,
};
}
}

if (proposal.top_n !== undefined && (!Number.isInteger(proposal.top_n) || proposal.top_n < 1)) {
return {
status: 'escalate',
reason: 'kind-mismatch',
blockers: [{ code: 'kind-mismatch', detail: 'top_n must be a positive integer' }],
proposal,
};
}

if (proposal.confidence !== undefined && proposal.confidence < minConfidence) {
return {
status: 'escalate',
Expand All @@ -302,6 +362,8 @@ function validateAndBuild(
sheet_type: 'worksheet',
template_parameters: { DATASOURCE: v.datasource },
field_mapping: v.field_mapping,
...(proposal.sort ? { sort: proposal.sort } : {}),
...(proposal.top_n !== undefined ? { top_n: proposal.top_n } : {}),
};
return {
status: 'bound',
Expand Down
4 changes: 3 additions & 1 deletion src/desktop/binder/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ export interface BindingProposal {
template: string;
title: string;
bindings: Array<{ slot_id: string; field: string; derivation?: Derivation }>; // field = a NAME from SchemaSummary.fields
sort?: { by: string; direction: 'asc' | 'desc' };
top_n?: number;
confidence?: number;
}

Expand Down Expand Up @@ -232,7 +234,7 @@ interface Resolution {
* but returns the matched SchemaField directly, so gates 3/4/7 have the resolved
* field's role/type/datatype/isAggregated (which `resolveField` does not expose).
*/
function resolveInSummary(s: SchemaSummary, query: string): Resolution {
export function resolveInSummary(s: SchemaSummary, query: string): Resolution {
const q = query.trim();
if (!q) return { kind: 'not_found', candidates: [] };
const qBare = bareName(q);
Expand Down
13 changes: 12 additions & 1 deletion src/desktop/commands/workbook/focusAppliedSheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import { log } from '../../../logging/logger.js';
import { WithExecutorAndAbortSignal } from '../../toolExecutor/toolExecutor.js';
import { listDashboards } from './listDashboards.js';
import { listWorksheets } from './listWorksheets.js';
import {
nameMayNeedRawCommandResolution,
resolveDashboardCommandName,
resolveWorksheetCommandName,
} from './nameResolution.js';

type ApplyCommand = 'load-worksheet' | 'load-dashboard';

Expand Down Expand Up @@ -62,10 +67,16 @@ export async function focusAppliedSheetBestEffort({
return;
}

const commandSheetName = nameMayNeedRawCommandResolution(sheetName)
? appliedVia === 'load-dashboard'
? ((await resolveDashboardCommandName(sheetName, { executor, signal })) ?? sheetName)
: ((await resolveWorksheetCommandName(sheetName, { executor, signal })) ?? sheetName)
: sheetName;

const result = await executor.executeCommand({
namespace: 'tabdoc',
command: 'goto-sheet',
args: { sheet: sheetName },
args: { sheet: commandSheetName },
signal,
});

Expand Down
38 changes: 38 additions & 0 deletions src/desktop/commands/workbook/getDashboardXml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,44 @@ describe('getDashboardXml (Agent API transport, default)', () => {
}),
);
});

it('falls back to the raw escaped Desktop command name for a literal ampersand name', async () => {
const mockXml = '<dashboard name="P&amp;L Overview"><zones></zones></dashboard>';
const mockExecutor = {
executeCommand: vi.fn(async (params: any) => {
if (params.command === 'list-dashboards') {
return Ok({
command_id: 'cmd-list',
status: 'completed',
parsedResult: {
dashboards: JSON.stringify({
count: 1,
dashboards: [{ name: 'P&amp;L Overview' }],
}),
},
});
}
return Ok({
command_id: 'cmd-123',
status: 'completed',
parsedResult: {
dashboardXml: params.args.dashboardName === 'P&amp;L Overview' ? mockXml : '<empty/>',
},
});
}),
} as unknown as LocalExecutor;

const result = await getDashboardXml({
dashboardName: 'P&L Overview',
executor: mockExecutor,
signal: mockSignal,
});

expect(result.isOk()).toBe(true);
if (result.isOk()) {
expect(result.value).toBe(mockXml);
}
});
});

describe('getDashboardXml (External Client API transport, TABLEAU_EXTERNAL_API gate)', () => {
Expand Down
40 changes: 39 additions & 1 deletion src/desktop/commands/workbook/getDashboardXml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
WithExecutorAndAbortSignal,
} from '../../toolExecutor/toolExecutor.js';
import { getWorkbookXml } from './getWorkbookXml.js';
import { nameMayNeedRawCommandResolution, resolveDashboardCommandName } from './nameResolution.js';

export type GetDashboardXmlError = (
| { type: 'no-dashboard-found' }
Expand Down Expand Up @@ -35,6 +36,40 @@ async function getDashboardXmlViaAgentApi({
executor,
signal,
}: { dashboardName: string } & WithExecutorAndAbortSignal): Promise<GetDashboardXmlResult> {
const result = await getDashboardXmlViaAgentApiName({ dashboardName, executor, signal });
if (result.isOk() || !nameMayNeedRawCommandResolution(dashboardName)) {
return result;
}

if (
result.error.type !== 'get-dashboard-xml-error' ||
result.error.error.type !== 'no-dashboard-found'
) {
return result;
}

const commandName = await resolveDashboardCommandName(dashboardName, { executor, signal });
if (!commandName || commandName === dashboardName) {
return result;
}

return getDashboardXmlViaAgentApiName({
dashboardName: commandName,
requestedDashboardName: dashboardName,
executor,
signal,
});
}

async function getDashboardXmlViaAgentApiName({
dashboardName,
requestedDashboardName = dashboardName,
executor,
signal,
}: {
dashboardName: string;
requestedDashboardName?: string;
} & WithExecutorAndAbortSignal): Promise<GetDashboardXmlResult> {
const result = await executor.executeCommand({
namespace: 'tabui',
command: 'save-dashboard',
Expand All @@ -57,7 +92,10 @@ async function getDashboardXmlViaAgentApi({
if (dashboardCount === 0) {
return Err({
type: 'get-dashboard-xml-error',
error: { type: 'no-dashboard-found', message: `No dashboard found for "${dashboardName}".` },
error: {
type: 'no-dashboard-found',
message: `No dashboard found for "${requestedDashboardName}".`,
},
});
}

Expand Down
Loading