diff --git a/ai_evals/adapters/frontend/backendPreview.ts b/ai_evals/adapters/frontend/backendPreview.ts index 57e1cfdf2a..8c1837c808 100644 --- a/ai_evals/adapters/frontend/backendPreview.ts +++ b/ai_evals/adapters/frontend/backendPreview.ts @@ -1,5 +1,5 @@ -import { randomUUID } from 'node:crypto' import type { BackendValidationSettings } from '../../core/backendValidation' +import { buildWorkspaceId } from './workspaceId' interface CompletedJobResultMaybe { completed: boolean @@ -24,7 +24,6 @@ export interface CompletedPreviewJob { const tokenCache = new Map>() const sharedWorkspaceQueue = new Map>() const managedSharedWorkspacePrefixes = ['f/evals/'] -const DEFAULT_WORKSPACE_PREFIX = 'ai-evals' export class BackendPreviewClient { constructor(private readonly settings: BackendValidationSettings) {} @@ -441,16 +440,6 @@ async function withSharedWorkspaceLock(workspaceId: string, body: () => Promi } } -function buildWorkspaceId(caseId: string, attempt: number): string { - const caseSlug = caseId - .toLowerCase() - .replace(/[^a-z0-9-]+/g, '-') - .replace(/^-+|-+$/g, '') - .slice(0, 30) - const suffix = randomUUID().slice(0, 8) - return `${DEFAULT_WORKSPACE_PREFIX}-${caseSlug || 'case'}-a${attempt}-${suffix}` -} - function extractFolderName(path: string): string | null { if (!path.startsWith('f/')) { return null diff --git a/ai_evals/adapters/frontend/mockBackend.ts b/ai_evals/adapters/frontend/mockBackend.ts index 8c1ca431b9..fd66c1cbcc 100644 --- a/ai_evals/adapters/frontend/mockBackend.ts +++ b/ai_evals/adapters/frontend/mockBackend.ts @@ -11,6 +11,7 @@ import type { Script } from '../../../frontend/src/lib/gen' import type { + DataMetric, DataTableTables, DataTableTableSchema, EndpointTool, @@ -112,6 +113,9 @@ export interface BenchmarkWorkspaceRunnables { aiProviders?: BenchmarkWorkspaceAiProvider[] resources?: BenchmarkWorkspaceResource[] datatables?: BenchmarkDatatableSeed[] + /** DuckLake catalog names, as `list_ducklakes` reports them. */ + ducklakes?: string[] + dataMetrics?: DataMetric[] jobs?: BenchmarkWorkspaceJob[] } @@ -673,6 +677,27 @@ export function listBenchmarkDatatables(workspace: string): DataTableTables[] | })) } +// ============= DuckLake catalogs and declared metrics ============= + +/** Seeded DuckLake names, or `null` for a non-benchmark workspace. */ +export function listBenchmarkDucklakes(workspace: string): string[] | null { + const runnables = benchmarkWorkspaceRunnables.get(workspace) + return runnables ? (runnables.ducklakes ?? []) : null +} + +/** + * Seeded metric declarations, or `null` for a non-benchmark workspace. + * + * The `table` / `path_prefix` filters are ignored: which rows a filter selects is + * `canonical_table_path`'s business and is pinned by `ducklakeTools.test.ts`. + * Re-deriving it here would give the eval its own copy of that spec to drift from, + * and the case this serves measures whether the model reaches for the tool at all. + */ +export function listBenchmarkDataMetrics(workspace: string): DataMetric[] | null { + const runnables = benchmarkWorkspaceRunnables.get(workspace) + return runnables ? (runnables.dataMetrics ?? []) : null +} + export function getBenchmarkDatatableSchema(input: { workspace: string datatableName: string diff --git a/ai_evals/adapters/frontend/vitestAdapter.test.ts b/ai_evals/adapters/frontend/vitestAdapter.test.ts index f240d0ee36..bd6419da7c 100644 --- a/ai_evals/adapters/frontend/vitestAdapter.test.ts +++ b/ai_evals/adapters/frontend/vitestAdapter.test.ts @@ -76,7 +76,9 @@ vi.mock('$lib/gen', async () => { listBenchmarkPlainResources, listBenchmarkApps, listBenchmarkDatatables, + listBenchmarkDataMetrics, listBenchmarkDrafts, + listBenchmarkDucklakes, listBenchmarkFlows, listBenchmarkJobs, listBenchmarkScripts, @@ -341,6 +343,10 @@ vi.mock('$lib/gen', async () => { hasBenchmarkWorkspace(data.workspace) ? (listBenchmarkDatatables(data.workspace) ?? []) : actual.WorkspaceService.listDataTableTables(data), + listDucklakes: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) + ? (listBenchmarkDucklakes(data.workspace) ?? []) + : actual.WorkspaceService.listDucklakes(data), getDataTableTableSchema: async (data: { workspace: string datatableName: string @@ -356,6 +362,12 @@ vi.mock('$lib/gen', async () => { }) : actual.WorkspaceService.getDataTableTableSchema(data) }), + DataMetricService: wrapService(actual.DataMetricService, { + listDataMetrics: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) + ? { metrics: listBenchmarkDataMetrics(data.workspace) ?? [] } + : actual.DataMetricService.listDataMetrics(data) + }), ScheduleService: wrapService(actual.ScheduleService, { existsSchedule: async (data: { workspace: string; path: string }) => hasBenchmarkWorkspace(data.workspace) ? false : actual.ScheduleService.existsSchedule(data), diff --git a/ai_evals/adapters/frontend/windmillBackend.ts b/ai_evals/adapters/frontend/windmillBackend.ts index 2247d8e5d5..bf483aa1f4 100644 --- a/ai_evals/adapters/frontend/windmillBackend.ts +++ b/ai_evals/adapters/frontend/windmillBackend.ts @@ -1,9 +1,8 @@ -import { randomUUID } from "node:crypto"; import type { WindmillBackendSettings } from "../../core/windmillBackendSettings"; +import { buildWorkspaceId } from "./workspaceId"; const tokenCache = new Map>(); const sharedWorkspaceQueue = new Map>(); -const DEFAULT_WORKSPACE_PREFIX = "ai-evals"; export class WindmillBackendClient { constructor(private readonly settings: WindmillBackendSettings) {} @@ -179,16 +178,6 @@ async function withSharedWorkspaceLock( } } -function buildWorkspaceId(caseId: string, attempt: number): string { - const caseSlug = caseId - .toLowerCase() - .replace(/[^a-z0-9-]+/g, "-") - .replace(/^-+|-+$/g, "") - .slice(0, 30); - const suffix = randomUUID().slice(0, 8); - return `${DEFAULT_WORKSPACE_PREFIX}-${caseSlug || "case"}-a${attempt}-${suffix}`; -} - async function expectOk(response: Response, context: string): Promise { if (response.ok) { return; diff --git a/ai_evals/adapters/frontend/workspaceId.test.ts b/ai_evals/adapters/frontend/workspaceId.test.ts new file mode 100644 index 0000000000..3573a9e51a --- /dev/null +++ b/ai_evals/adapters/frontend/workspaceId.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from "bun:test"; +import { buildWorkspaceId } from "./workspaceId"; + +describe("buildWorkspaceId", () => { + // `workspace.proper_id` rejects `--`, which a case id can carry itself and + // which truncating a slug on a hyphen produces once the suffix adds its own. + // One id per shape: cut landing on a hyphen, cut landing mid-word, no cut, and + // a doubled hyphen no cut ever reaches. + it("stays within the id length cap and the proper_id format", () => { + for (const caseId of [ + "global-test6-secret-variable-draft", + "global-test23-datatable-query-select", + "short", + "global--test-foo", + ]) { + const id = buildWorkspaceId(caseId, 1); + expect(id.length).toBeLessThanOrEqual(50); + expect(id).toMatch(/^\w+(-\w+)*$/); + } + }); +}); diff --git a/ai_evals/adapters/frontend/workspaceId.ts b/ai_evals/adapters/frontend/workspaceId.ts new file mode 100644 index 0000000000..9545c06a7d --- /dev/null +++ b/ai_evals/adapters/frontend/workspaceId.ts @@ -0,0 +1,22 @@ +import { randomUUID } from "node:crypto"; + +const DEFAULT_WORKSPACE_PREFIX = "ai-evals"; + +// A workspace id must be at most 50 characters AND match `^\w+(-\w+)*$` +// (`workspace.proper_id`), so the case slug yields to the random suffix that +// makes the id unique, and no hyphen may end up doubled — neither one already in +// the case id nor one a truncation leaves for the suffix to follow. +const MAX_WORKSPACE_ID_LENGTH = 50; + +export function buildWorkspaceId(caseId: string, attempt: number): string { + const caseSlug = caseId + .toLowerCase() + .replace(/[^a-z0-9-]+/g, "-") + .replace(/-{2,}/g, "-") + .replace(/^-+|-+$/g, ""); + const suffix = `-a${attempt}-${randomUUID().slice(0, 8)}`; + const head = `${DEFAULT_WORKSPACE_PREFIX}-${caseSlug || "case"}`; + return `${head + .slice(0, MAX_WORKSPACE_ID_LENGTH - suffix.length) + .replace(/-+$/, "")}${suffix}`; +} diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index 951d316524..e8ec660f3b 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -1919,10 +1919,11 @@ - when the lookup fails, tells the user instead of inventing table names - does not write scripts or resources to answer a read-only question -# --- API catalog (search_api_endpoints / call_api_get / call_api_endpoint) --- -# The harness serves the catalog and the executed calls itself (mock -# listMcpTools + benchmark fetch handlers in adapters/frontend), so these cases -# do not require an mcp-enabled eval backend. +# --- Dedicated tools preferred over the API catalog --- +# The harness serves worker/queue reads itself (benchmark fetch handlers in +# adapters/frontend), so these cases do not require an mcp-enabled eval backend. +# The stale `api-catalog` in the id below is kept so results stay comparable +# across benchmark runs. - id: global-test30-api-catalog-workers prompt: |- @@ -1934,23 +1935,42 @@ draftCountExactly: 0 toolExpect: requiredToolsUsed: - - search_api_endpoints - - call_api_get + - list_workers forbiddenToolsUsed: + - call_api_get - call_api_endpoint - write_script - deploy_workspace_item - toolCallArgs: - - tool: call_api_get - field: name - stringIncludesAnyOf: - - listWorkers # Read-only workspace inspection produces no draft; validate via tool use. skipJudge: true judgeChecklist: - - discovers the workers endpoint through the API catalog instead of guessing or fabricating + - reads worker state through list_workers instead of guessing or fabricating - reports worker status from the returned data +- id: global-test37-ducklake-declared-measure + prompt: |- + We track orders in the main ducklake. Write me a duckdb script that reports total + revenue by month. Keep it as a draft, don't deploy it. + initial: ai_evals/fixtures/frontend/global/initial/ducklake_orders_metrics.json + runtime: + maxTurns: 8 + validate: + draftCountExactly: 1 + toolExpect: + requiredToolsUsed: + - list_data_metrics + forbiddenToolsUsed: + - deploy_workspace_item + - delete_workspace_item + # The judge runs: the point is not that the tool was called but that the number it + # describes is the declared one. `revenue` excludes test rows, so an aggregate that + # reproduces it without the filter is plausible, runnable and wrong. + judgeChecklist: + - totals revenue with the declared sum over the amount column rather than an invented aggregate over a guessed column + - excludes test orders from the total, as the declared revenue measure does + - groups by month using the declared order_month expression over order_date + - does not introduce column names absent from the declarations + - id: global-test31-draft-test-run-not-deployed prompt: |- Update `f/evals/global/format_greeting` so the provided name is uppercased in the greeting, then run it with name "ada" to check it works. diff --git a/ai_evals/fixtures/frontend/global/initial/ducklake_orders_metrics.json b/ai_evals/fixtures/frontend/global/initial/ducklake_orders_metrics.json new file mode 100644 index 0000000000..fba88c53d3 --- /dev/null +++ b/ai_evals/fixtures/frontend/global/initial/ducklake_orders_metrics.json @@ -0,0 +1,36 @@ +{ + "workspace": { + "ducklakes": ["main"], + "dataMetrics": [ + { + "script_path": "f/analytics/orders_pipeline", + "table_path": "main/main.orders", + "kind": "measure", + "name": "revenue", + "expr": "sum(amount)", + "filter": "not is_test" + }, + { + "script_path": "f/analytics/orders_pipeline", + "table_path": "main/main.orders", + "kind": "measure", + "name": "order_count", + "expr": "count(*)" + }, + { + "script_path": "f/analytics/orders_pipeline", + "table_path": "main/main.orders", + "kind": "dimension", + "name": "order_month", + "expr": "date_trunc('month', order_date)" + }, + { + "script_path": "f/analytics/orders_pipeline", + "table_path": "main/main.orders", + "kind": "dimension", + "name": "region", + "expr": "region" + } + ] + } +} diff --git a/frontend/src/lib/components/copilot/chat/ducklakeTools.test.ts b/frontend/src/lib/components/copilot/chat/ducklakeTools.test.ts index fdd8c74558..d02d1b9497 100644 --- a/frontend/src/lib/components/copilot/chat/ducklakeTools.test.ts +++ b/frontend/src/lib/components/copilot/chat/ducklakeTools.test.ts @@ -1,6 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { listMock } = vi.hoisted(() => ({ listMock: vi.fn() })) +const { listMock, listMetricsMock } = vi.hoisted(() => ({ + listMock: vi.fn(), + listMetricsMock: vi.fn() +})) vi.mock('./shared', () => ({ createToolDef: (_schema: unknown, name: string, description: string) => ({ @@ -10,7 +13,8 @@ vi.mock('./shared', () => ({ })) vi.mock('$lib/gen', () => ({ - WorkspaceService: { listDucklakes: listMock } + WorkspaceService: { listDucklakes: listMock }, + DataMetricService: { listDataMetrics: listMetricsMock } })) import { getDucklakeTools } from './ducklakeTools' @@ -31,7 +35,10 @@ function run(name: string, args: Record = {}) { }) } -beforeEach(() => listMock.mockReset()) +beforeEach(() => { + listMock.mockReset() + listMetricsMock.mockReset() +}) describe('list_ducklakes', () => { it('returns the configured catalog names', async () => { @@ -51,3 +58,65 @@ describe('list_ducklakes', () => { expect(result).toContain('still draft the pipeline scripts') }) }) + +describe('list_data_metrics', () => { + it('forwards the filters and returns the declarations', async () => { + listMetricsMock.mockResolvedValue({ + metrics: [ + { + script_path: 'f/analytics/rev', + table_path: 'main/main.orders', + kind: 'measure', + name: 'revenue', + expr: 'sum(amount)', + filter: 'not is_test' + } + ] + }) + const result = await run('list_data_metrics', { + table: 'ducklake://main/main.orders', + path_prefix: 'f/analytics', + limit: 50 + }) + expect(listMetricsMock).toHaveBeenCalledWith({ + workspace: 'test-workspace', + table: 'ducklake://main/main.orders', + pathPrefix: 'f/analytics', + perPage: 50 + }) + expect(JSON.parse(result).metrics[0]).toMatchObject({ name: 'revenue', expr: 'sum(amount)' }) + }) + + it('never reports an empty result as proof that nothing is declared', async () => { + listMetricsMock.mockResolvedValue({ metrics: [] }) + const result = await run('list_data_metrics', {}) + // Unreadable declarations are omitted, not flagged, so absence is unprovable. + expect(result).toContain('does not establish') + expect(result).toContain('cannot read') + }) + + // The server matches a lake-less name against nothing, so the readability hedge + // would confirm "nothing is declared" for a filter worth retrying. The scheme is + // optional on the way in, so the retry it names must not carry it back. + it.each(['orders', 'ducklake://orders'])( + 'blames the missing lake, not readability, for table %s', + async (table) => { + listMetricsMock.mockResolvedValue({ metrics: [] }) + const result = await run('list_data_metrics', { table }) + expect(result).toContain('`/orders`') + expect(result).not.toContain('cannot read') + } + ) + + it('warns that more declarations exist when the page is cut short', async () => { + listMetricsMock.mockResolvedValue({ + // A cursor only comes back on a full page, never with an empty one. + metrics: [{ table_path: 't', kind: 'measure', name: 'n', script_path: 's' }], + next_cursor: { table_path: 't', kind: 'measure', name: 'n', script_path: 's' } + }) + const result = await run('list_data_metrics', {}) + // Without this the model reads a partial page as "no such measure" and + // re-derives a number that disagrees with the declared one. + expect(result).toContain('rather than concluding a measure is undeclared') + }) +}) diff --git a/frontend/src/lib/components/copilot/chat/ducklakeTools.ts b/frontend/src/lib/components/copilot/chat/ducklakeTools.ts index 5594e4f3d0..dfadc58db9 100644 --- a/frontend/src/lib/components/copilot/chat/ducklakeTools.ts +++ b/frontend/src/lib/components/copilot/chat/ducklakeTools.ts @@ -1,17 +1,19 @@ import { z } from 'zod' -import { WorkspaceService } from '$lib/gen' +import { DataMetricService, WorkspaceService } from '$lib/gen' import { createToolDef, type Tool } from './shared' /** - * Workspace-scoped DuckLake readiness tool, the pipeline counterpart to - * `list_datatables` in `datatableTools.ts`. + * Workspace-scoped DuckLake tools, the pipeline counterpart to `list_datatables` + * in `datatableTools.ts`. * * A data pipeline materializes DuckLake tables and reads/writes S3 assets, which * only work once the workspace has object storage + a DuckLake catalog - * configured. This tool lets the chat detect that prerequisite (and warn with - * role-appropriate next steps) instead of silently producing a pipeline that - * cannot run. It is a plain read gated only by workspace membership, so it needs - * no app context and belongs in the global tool set. + * configured. `list_ducklakes` lets the chat detect that prerequisite (and warn + * with role-appropriate next steps) instead of silently producing a pipeline that + * cannot run. `list_data_metrics` reads the declarations recorded against those + * lake tables, not the tables themselves. Both are plain reads gated only by + * workspace membership, so they need no app context and belong in the global tool + * set. */ /** List the names of the DuckLake catalogs configured in the workspace. */ @@ -31,9 +33,78 @@ const listDucklakesToolDef = createToolDef( 'List the DuckLake catalogs configured in this workspace, by name. Call this before building or deploying a data pipeline that materializes DuckLake tables or reads/writes S3 assets: if it returns none, the workspace has no object storage + DuckLake configured and the pipeline cannot run until a workspace admin sets it up. Returns names only.' ) +const listDataMetricsSchema = z.object({ + table: z + .string() + .optional() + .describe( + 'Only declarations on this DuckLake table, as `/` or `/.
`, with or without the `ducklake://` scheme. A name with no lake matches nothing and comes back empty.' + ), + path_prefix: z + .string() + .optional() + .describe('Only declarations made by scripts under this path, e.g. `f/analytics`.'), + limit: z + .number() + .int() + .min(1) + .max(1000) + .optional() + .describe('Max number of declarations to return. Defaults to 200.') +}) +const listDataMetricsToolDef = createToolDef( + listDataMetricsSchema, + 'list_data_metrics', + 'List the measures and dimensions declared on DuckLake tables (from `// measure` / `// dimension` annotations in deployed scripts). Call this before writing any aggregate query over a DuckLake table: a declared measure is the canonical definition of that number, and reproducing it yourself silently disagrees with it (a `revenue` measure typically excludes refunds or test rows). Use each returned `expr` verbatim, and when a measure has a `filter` write it as `expr FILTER (WHERE filter)` so measures with different predicates share one GROUP BY. Only declarations whose producing script you can read are returned, so what comes back is never proof of what exists: if the number you need is not here you may still write your own aggregate, but say that you found no declared measure for it rather than implying none exists.' +) + +// The endpoint drops declarations whose producing script the caller cannot read +// (token scope + RLS on `script`), so an empty result means "none declared" or +// "none readable by you" and the tool cannot tell which. +const NO_DATA_METRICS_NOTE = + 'Nothing matched. That does not establish that nothing is declared: declarations whose producing script you cannot read are omitted from this list, not flagged. You may write your own aggregate, but tell the user you found no declared measure you can read rather than stating none is declared.' + +// Well under the endpoint's 1000 cap: a full page is pretty-printed into the +// chat context, and 1000 declarations would cost tens of thousands of tokens. +const DEFAULT_DATA_METRICS_LIMIT = 200 + /** The workspace DuckLake tools, for registration in global mode. */ export function getDucklakeTools(): Tool<{}>[] { return [ + { + def: listDataMetricsToolDef, + planModeSafe: true, + showDetails: true, + fn: async ({ args, workspace, toolId, toolCallbacks }) => { + const parsed = listDataMetricsSchema.parse(args) + toolCallbacks.setToolStatus(toolId, { content: 'Listing declared measures...' }) + const limit = parsed.limit ?? DEFAULT_DATA_METRICS_LIMIT + const { metrics, next_cursor } = await DataMetricService.listDataMetrics({ + workspace, + table: parsed.table, + pathPrefix: parsed.path_prefix, + perPage: limit + }) + // `canonical_table_path` expands a value only once it contains a `/`, so a bare + // table name matches nothing — a retryable filter, not a permissions outcome. + const bareTable = parsed.table?.replace('ducklake://', '') + const emptyNote = + bareTable && !bareTable.includes('/') + ? `Nothing matched \`${parsed.table}\`: a table filter must name its lake, as \`/${bareTable}\`. Re-call with the lake before concluding anything about what is declared.` + : NO_DATA_METRICS_NOTE + const note = next_cursor + ? `More declarations exist beyond the first ${limit}. Re-call with table/path_prefix to target what you are looking for, or with a higher limit (max 1000) for the rest of the list, rather than concluding a measure is undeclared.` + : metrics.length === 0 + ? emptyNote + : undefined + const result = JSON.stringify({ metrics, ...(note ? { note } : {}) }, null, 2) + toolCallbacks.setToolStatus(toolId, { + content: `Listed ${metrics.length} declared measure(s)/dimension(s)`, + result + }) + return result + } + }, { def: listDucklakesToolDef, planModeSafe: true, diff --git a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts index a78f09071a..06109708de 100644 --- a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.test.ts @@ -31,6 +31,24 @@ const CATALOG = [ properties: { page: { type: 'integer' }, per_page: { type: 'integer' } } } }, + { + name: 'listDataMetrics', + description: 'List declared measures and dimensions', + instructions: '', + path: '/w/{workspace}/data_metrics/list', + method: 'GET' + }, + { + name: 'listQueue', + description: 'List queued jobs', + instructions: 'List the jobs waiting in the queue', + path: '/w/{workspace}/jobs/queue/list', + method: 'GET', + query_params_schema: { + type: 'object', + properties: { running: { type: 'boolean' }, per_page: { type: 'integer' } } + } + }, { name: 'getJobUpdates', description: 'Get job updates', @@ -177,11 +195,12 @@ beforeEach(() => { describe('search_api_endpoints', () => { it('matches on name/path tokens, plural-insensitively, and excludes covered endpoints', async () => { - const result = await run('search_api_endpoints', { query: 'worker' }) - expect(result.matches.map((m: any) => m.name)).toEqual(['listWorkers']) - expect(result.matches[0].endpoint).toBe('GET /workers/list') - expect(result.matches[0].params).toEqual(['page', 'per_page']) - expect(result.matches[0].instructions).toContain('ping status') + // Singular "job" matches the plural "jobs" path segment. + const result = await run('search_api_endpoints', { query: 'list queued job' }) + expect(result.matches[0].name).toBe('listQueue') + expect(result.matches[0].endpoint).toBe('GET /w/{workspace}/jobs/queue/list') + expect(result.matches[0].params).toEqual(['running', 'per_page']) + expect(result.matches[0].instructions).toContain('waiting in the queue') const flows = await run('search_api_endpoints', { query: 'create flow' }) expect(flows.matches.map((m: any) => m.name)).not.toContain('createFlow') @@ -191,8 +210,11 @@ describe('search_api_endpoints', () => { it('returns endpoint categories when nothing matches', async () => { const result = await run('search_api_endpoints', { query: 'kubernetes' }) expect(result.matches).toEqual([]) - expect(result.hint).toContain('workers') - expect(result.hint).toContain('jobs') + expect(result.hint).toContain('jobs_u') + // Categories are built from the uncovered endpoints only, so a covered one + // must not be advertised as somewhere to retry. + expect(result.hint).not.toContain('workers') + expect(result.hint).not.toContain('data_metrics') }) }) @@ -253,6 +275,21 @@ describe('call_api_get', () => { expect(search.matches.map((m: any) => m.name)).not.toContain('getScriptByPath') }) + it('refuses the worker and data-metric reads, pointing at their dedicated tools', async () => { + for (const [name, tool, query] of [ + ['listWorkers', 'list_workers', 'workers'], + ['listDataMetrics', 'list_data_metrics', 'data metrics'] + ]) { + const result = await run('call_api_get', { name }) + expect(result.success).toBe(false) + expect(result.error).toContain(tool) + + const search = await run('search_api_endpoints', { query }) + expect(search.matches.map((m: any) => m.name)).not.toContain(name) + expect(search.covered_by_dedicated_tools?.join(' ')).toContain(tool) + } + }) + it('refuses variable reads so variable values never reach the model', async () => { const result = await run('call_api_get', { name: 'getVariable' }) expect(result.success).toBe(false) @@ -273,12 +310,14 @@ describe('call_api_get', () => { const fetchMock = vi.fn().mockResolvedValue({ ok: true, headers: new Headers({ 'content-type': 'application/json' }), - json: async () => [{ worker: 'w1' }] + json: async () => [{ id: 'job-1' }] }) vi.stubGlobal('fetch', fetchMock) - const result = await run('call_api_get', { name: 'listWorkers', params: { page: 2 } }) - expect(fetchMock).toHaveBeenCalledWith('/api/workers/list?page=2', { method: 'GET' }) - expect(result).toEqual({ success: true, data: [{ worker: 'w1' }] }) + const result = await run('call_api_get', { name: 'listQueue', params: { running: true } }) + expect(fetchMock).toHaveBeenCalledWith('/api/w/test-ws/jobs/queue/list?running=true', { + method: 'GET' + }) + expect(result).toEqual({ success: true, data: [{ id: 'job-1' }] }) }) }) @@ -305,7 +344,7 @@ describe('call_api_endpoint', () => { }) it('redirects GET endpoints to call_api_get', async () => { - const result = await run('call_api_endpoint', { name: 'listWorkers' }) + const result = await run('call_api_endpoint', { name: 'listQueue' }) expect(result.error).toContain('call_api_get') }) }) diff --git a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts index 308d222544..42c68c6ebf 100644 --- a/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts +++ b/frontend/src/lib/components/copilot/chat/global/apiCatalogTools.ts @@ -58,6 +58,8 @@ const COVERED_ENDPOINTS: Record = { searchDocs: 'search_docs', readDocsPage: 'read_docs_page', listJobs: 'list_runs', + listWorkers: 'list_workers', + listDataMetrics: 'list_data_metrics', getJob: 'get_run', getJobLogs: 'get_run', runScriptPreviewAndWaitResult: 'test_run_script' @@ -231,7 +233,7 @@ const searchApiEndpointsSchema = z.object({ query: z .string() .describe( - 'Keywords matched against endpoint names, paths, and descriptions (e.g. "workers", "queue", "run flow"). Jobs are called "runs" in the UI.' + 'Keywords matched against endpoint names, paths, and descriptions (e.g. "queue", "run flow", "audit log"). Jobs are called "runs" in the UI.' ) }) @@ -264,7 +266,7 @@ export const apiCatalogTools: Tool<{}>[] = [ def: createToolDef( searchApiEndpointsSchema, 'search_api_endpoints', - 'Search the Windmill REST API endpoint catalog for operations no dedicated tool covers (workers, queue state, job details, running deployed items, deletions, ...). Returns endpoint names to pass to call_api_get or call_api_endpoint.' + 'Search the Windmill REST API endpoint catalog for operations no dedicated tool covers (queue state, job details, running deployed items, deletions, ...). Returns endpoint names to pass to call_api_get or call_api_endpoint.' ), planModeSafe: true, fn: async ({ args, workspace, toolId, toolCallbacks }) => { diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index fe0477a544..822b36135c 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -256,6 +256,9 @@ vi.mock('$lib/gen', async () => { createVariable: vi.fn(async () => 'created'), updateVariable: vi.fn(async () => 'updated') }), + WorkerService: wrapService(actual.WorkerService, { + listWorkers: vi.fn(async () => []) + }), FolderService: wrapService(actual.FolderService, { createFolder: vi.fn(async () => 'created') }), @@ -264,6 +267,17 @@ vi.mock('$lib/gen', async () => { const user = whoamiByWorkspace.get(workspace) if (!user) throw new Error(`not a member of ${workspace}`) return user + }), + // `refreshSuperadmin` cancels the previous in-flight call, so this stands in for + // the CancelablePromise the real client returns. + globalWhoami: vi.fn(() => { + const pending: any = Promise.resolve({ + email: 'devops@windmill.dev', + super_admin: false, + devops: true + }) + pending.cancel = () => {} + return pending }) }), DraftService: wrapService(actual.DraftService, { @@ -389,9 +403,10 @@ import { ScheduleService, ScriptService, UserService, - VariableService + VariableService, + WorkerService } from '$lib/gen' -import { superadmin, userStore, usersWorkspaceStore } from '$lib/stores' +import { devopsRole, superadmin, userStore, usersWorkspaceStore } from '$lib/stores' import { processSecretArgs } from '$lib/components/secretArgUtils' import { clearWorkspaceRoleCache } from '$lib/user' import { get } from 'svelte/store' @@ -738,6 +753,102 @@ describe('global AI tools', () => { ) }) + it('lists workers with the diagnostic fields only', async () => { + vi.mocked(WorkerService.listWorkers).mockResolvedValueOnce([ + { + worker: 'wk-1', + worker_instance: 'host-1', + worker_group: 'gpu', + custom_tags: ['gpu'], + last_ping: 3, + jobs_executed: 12, + started_at: '2024-01-01T00:00:00Z', + ip: '10.0.0.1', + wm_version: 'v1', + memory: 123, + occupancy_rate: 0.5 + } + ]) + + const result = await callGlobalTool('list_workers', {}) + + expect(JSON.parse(result).workers).toEqual([ + { + worker: 'wk-1', + worker_group: 'gpu', + custom_tags: ['gpu'], + last_ping: 3, + jobs_executed: 12 + } + ]) + // Page telemetry must stay out of the model's context. + expect(result).not.toContain('occupancy_rate') + expect(result).not.toContain('10.0.0.1') + }) + + it('says so when the worker page is cut short', async () => { + vi.mocked(WorkerService.listWorkers).mockResolvedValueOnce( + Array(100).fill({ + worker: 'wk', + worker_group: 'default', + custom_tags: [], + last_ping: 1, + jobs_executed: 0 + }) as any + ) + + const result = await callGlobalTool('list_workers', {}) + + // A full page is indistinguishable from the whole fleet, and the model reasons + // about tag coverage from this list. + expect(JSON.parse(result).note).toContain('Only the first 100 workers') + }) + + describe('list_workers with nothing to show', () => { + afterEach(() => { + superadmin.set(undefined) + devopsRole.set(undefined) + }) + + it('never reports an empty list as an absence to a caller workers can be hidden from', async () => { + superadmin.set(false) + devopsRole.set(false) + vi.mocked(WorkerService.listWorkers).mockResolvedValueOnce([]) + + const result = await callGlobalTool('list_workers', {}) + + // An instance hiding workers from a non-devops caller answers with an empty + // list, so absence is unprovable here. + expect(result).toContain('does NOT establish that no workers are running') + expect(result).toContain('devops role') + expect(result).not.toContain('"workers"') + }) + + it('reports an empty list as an absence to a devops caller', async () => { + devopsRole.set('devops@windmill.dev') + vi.mocked(WorkerService.listWorkers).mockResolvedValueOnce([]) + + const result = await callGlobalTool('list_workers', {}) + + // Nothing is hidden from this caller, so hedging would withhold the answer a + // stuck queue is waiting on. + expect(result).toContain('No workers are connected') + expect(result).not.toContain('does NOT establish') + }) + + it('resolves the role before deciding, rather than reading unloaded stores as no role', async () => { + // Both stores start undefined; without the refresh a devops caller whose whoami + // has not landed yet is hedged at instead of answered. + expect(get(superadmin)).toBeUndefined() + expect(get(devopsRole)).toBeUndefined() + vi.mocked(WorkerService.listWorkers).mockResolvedValueOnce([]) + + const result = await callGlobalTool('list_workers', {}) + + expect(result).toContain('No workers are connected') + }) + }) + it('returns args, result and logs of a run in one call', async () => { const result = await callGlobalTool('get_run', { id: 'job-123' }) diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 1511bf24a6..f1dcc0ef0a 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -17,7 +17,8 @@ import { ScriptService, SqsTriggerService, VariableService, - WebsocketTriggerService + WebsocketTriggerService, + WorkerService } from '$lib/gen' import { createTwoFilesPatch } from 'diff' import { deepEqual } from 'fast-equals' @@ -179,6 +180,7 @@ import { workspaceStore } from '$lib/stores' import { getWorkspaceRole, type RoleLookup } from '$lib/user' +import { refreshSuperadmin } from '$lib/refreshUser' import { get } from 'svelte/store' import { canonicalDraftSideValue, @@ -724,6 +726,17 @@ const listRunsSchema = z.object({ .describe('Max number of runs to return, most recent first. Defaults to 30.') }) +// `GET /workers/list` hides workers from a caller without the devops role by +// returning an empty list, not an error, when HIDE_WORKERS_FOR_NON_ADMINS is set. +// The flag is not visible here, so absence is only provable for a devops or +// superadmin caller; every other caller gets the hedge even where nothing is hidden. +const NO_WORKERS_VISIBLE_MESSAGE = + 'No workers came back. This does NOT establish that no workers are running: an instance can hide workers from callers without the devops role, and it does so by returning an empty list rather than an error. ' + + 'Tell the user you cannot see any workers and that worker visibility may be restricted for your account, and suggest they check the Workers page themselves. Never state that no workers are online or that the instance has none.' +const NO_WORKERS_CONNECTED_MESSAGE = + 'No workers are connected to this instance (none pinged in the last 5 minutes). Queued runs will stay queued until a worker starts.' +const WORKER_PAGE_SIZE = 100 + const deleteWorkspaceItemSchema = z.object({ type: itemTypeSchema, path: z.string().describe('Workspace path of the item to delete.'), @@ -1360,7 +1373,7 @@ ${pipelineBullet} ? ' By default it preselects the items this chat modified; pass items (":" entries) to control the selection' : ' Pass items (":" entries naming the items you changed) so the review is scoped to them — omitting items preselects every pending change in the workspace' }, or mode ("draft" or "fork") to force which comparison is shown. Prefer offering this review page over calling deploy_workspace_item directly when several items changed. -- For a Windmill operation no other tool covers (workers, queue state, a run's args, ...), use search_api_endpoints to find a REST endpoint, then call_api_get for reads or call_api_endpoint for mutations (the user is asked to confirm those). Always prefer a dedicated tool when one exists; endpoints for authoring or deleting scripts, flows, apps, schedules, resources, or variables are not available through the API catalog tools — use the draft tools and delete_workspace_item instead. +- For a Windmill operation no other tool covers (queue state, a run's args, ...), use search_api_endpoints to find a REST endpoint, then call_api_get for reads or call_api_endpoint for mutations (the user is asked to confirm those). Always prefer a dedicated tool when one exists; endpoints for authoring or deleting scripts, flows, apps, schedules, resources, or variables are not available through the API catalog tools — use the draft tools and delete_workspace_item instead. - Default to test_run_script, test_run_flow, or test_run_step for any run request, an existing script included; they prefer drafts and need no deployment. Use run_script or run_flow only when the user names the deployed version ("the deployed X", "in production", "for real") — a bare "run X" is not that. For those two, read the item with read_workspace_item version: "deployed" first so the arguments match the deployed schema. test_run_script, test_run_flow, test_run_step, run_script and run_flow all show the user an argument form prefilled with what you sent, so fill in every argument you can infer rather than asking for it in chat. test_run_step's form is the step's own inputs, not the flow's. - When a required decision is ambiguous, use askUserQuestion with two to ten clear proposed answer strings instead of guessing. The user can also type a custom answer when none of the proposed answers fit. Set multiSelect: true only when the answers can genuinely co-apply and the user may pick several (not mutually exclusive). - When the user asks you to remember a lasting preference, always/never do something, or change/stop a behavior going forward, call update_user_instructions to persist it. It edits only the USER INSTRUCTIONS block (not WORKSPACE INSTRUCTIONS). Keep each instruction concise; do not use it for one-off requests scoped to the current task. @@ -3742,6 +3755,49 @@ export const globalTools: Tool<{}>[] = [ return result } }, + { + def: createToolDef( + z.object({}), + 'list_workers', + 'List the workers connected to this Windmill instance (those that pinged in the last 5 minutes), with their worker group, custom tags, seconds since their last ping, and jobs executed. Pair with list_runs to diagnose a stuck queue: runs queued on a tag no listed worker picks up will never start. Three blind spots to report rather than reason past: an empty result states whether no worker is connected or whether workers may be hidden from you, so relay the one it gives instead of picking; a missing custom_tags can mean tags are hidden from you, not unset; and only the 100 most recently pinging workers are listed, so on a bigger instance a tag none of them carries may still be served.' + ), + planModeSafe: true, + showDetails: true, + fn: async ({ toolId, toolCallbacks }) => { + toolCallbacks.setToolStatus(toolId, { content: 'Listing workers...' }) + const pings = await WorkerService.listWorkers({ perPage: WORKER_PAGE_SIZE }) + if (pings.length === 0) { + // Both role stores resolve asynchronously, and an unloaded one must not read as + // an absent role — that hedges the answer this branch exists to give plainly. + // No-ops once they hold a value. + await refreshSuperadmin() + const hiddenFromCaller = !get(superadmin) && !get(devopsRole) + const message = hiddenFromCaller ? NO_WORKERS_VISIBLE_MESSAGE : NO_WORKERS_CONNECTED_MESSAGE + toolCallbacks.setToolStatus(toolId, { + content: hiddenFromCaller ? 'No workers visible' : 'No workers connected', + result: message + }) + return message + } + const workers = pings.map((w) => ({ + worker: w.worker, + worker_group: w.worker_group, + custom_tags: w.custom_tags, + last_ping: w.last_ping, + jobs_executed: w.jobs_executed + })) + const note = + workers.length === WORKER_PAGE_SIZE + ? `Only the first ${WORKER_PAGE_SIZE} workers are listed; more may be connected.` + : undefined + const result = JSON.stringify({ workers, ...(note ? { note } : {}) }, null, 2) + toolCallbacks.setToolStatus(toolId, { + content: `Listed ${workers.length} worker(s)`, + result + }) + return result + } + }, { def: createToolDef( getRunSchema, @@ -4263,7 +4319,7 @@ export const globalTools: Tool<{}>[] = [ }, // Workspace-scoped datatable tools (unrestricted: no whitelist, no creation policy) ...getDatatableTools(), - // Workspace DuckLake readiness (storage prerequisite check for pipelines) + // Workspace DuckLake: pipeline storage prerequisite, and declared measures ...getDucklakeTools(), // Read-only tools over files the user attached to the conversation ...fileTools,