From 3db999786a9d79ef1d58cb6d556edbbbc6990d7d Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Fri, 2 Oct 2026 17:46:55 -0400 Subject: [PATCH 01/24] Add data actions to the client, CLI and skills mb data-action list|get|create|update|archive|delete|execute over /api/action, query-type data actions only, a data-action skill, and e2e coverage on servers that accept data actions without a model. Co-Authored-By: Claude Opus 5.5 --- README.md | 66 +++++ packages/cli/skill-data/data-action/SKILL.md | 125 +++++++++ packages/cli/skill-data/native-sql/SKILL.md | 2 +- .../cli/src/commands/data-action/archive.ts | 30 +++ .../cli/src/commands/data-action/create.ts | 51 ++++ .../cli/src/commands/data-action/delete.ts | 31 +++ .../cli/src/commands/data-action/execute.ts | 44 ++++ packages/cli/src/commands/data-action/get.ts | 25 ++ .../cli/src/commands/data-action/index.ts | 19 ++ packages/cli/src/commands/data-action/list.ts | 22 ++ .../cli/src/commands/data-action/update.ts | 53 ++++ packages/cli/src/commands/validate-query.ts | 5 + packages/cli/src/core/skills.test.ts | 7 +- packages/cli/src/main.ts | 1 + packages/cli/src/output/views/data-action.ts | 24 ++ packages/cli/src/runtime/command-help.test.ts | 10 + packages/client/src/client.ts | 2 + packages/client/src/domain/data-action.ts | 111 ++++++++ packages/client/src/index.test.ts | 6 + packages/client/src/index.ts | 10 + .../client/src/resources/data-action.test.ts | 213 +++++++++++++++ packages/client/src/resources/data-action.ts | 96 +++++++ packages/client/src/version/features.ts | 2 + .../client/src/version/requirements.test.ts | 6 + packages/client/src/version/requirements.ts | 7 + tests/e2e/data-action.e2e.test.ts | 247 ++++++++++++++++++ tests/e2e/skills.e2e.test.ts | 33 ++- 27 files changed, 1239 insertions(+), 9 deletions(-) create mode 100644 packages/cli/skill-data/data-action/SKILL.md create mode 100644 packages/cli/src/commands/data-action/archive.ts create mode 100644 packages/cli/src/commands/data-action/create.ts create mode 100644 packages/cli/src/commands/data-action/delete.ts create mode 100644 packages/cli/src/commands/data-action/execute.ts create mode 100644 packages/cli/src/commands/data-action/get.ts create mode 100644 packages/cli/src/commands/data-action/index.ts create mode 100644 packages/cli/src/commands/data-action/list.ts create mode 100644 packages/cli/src/commands/data-action/update.ts create mode 100644 packages/cli/src/output/views/data-action.ts create mode 100644 packages/client/src/domain/data-action.ts create mode 100644 packages/client/src/resources/data-action.test.ts create mode 100644 packages/client/src/resources/data-action.ts create mode 100644 tests/e2e/data-action.e2e.test.ts diff --git a/README.md b/README.md index b51ccf4d..30bd0654 100644 --- a/README.md +++ b/README.md @@ -1056,6 +1056,72 @@ mb snippet archive 1 mb snippet archive 1 --json ``` +## Data actions + +CRUD on `/api/action` plus `execute`. A data action is a parameterized native SQL write (`INSERT`, `UPDATE`, `DELETE`) filed in a collection. Data actions are off by default: an admin must enable them on the target database first. `mb data-action create` authors data actions only — no `model_id`, no implicit data actions — and needs a server whose data actions do not require a model. + +### `mb data-action list` + +```sh +mb data-action list +mb data-action list --json +``` + +### `mb data-action get ` + +```sh +mb data-action get 1 +mb data-action get 1 --json --full +``` + +### `mb data-action create` + +```sh +cat data-action.json | mb data-action create +mb data-action create --file data-action.json +mb data-action create --file data-action.json --skip-validate +``` + +| Flag | Description | +| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `--body ` | Inline JSON body. | +| `--file ` | Path to JSON body file. | +| `--skip-validate` | Skip the local MBQL 5 pre-flight validation; let the server be the authority. Use only when the bundled schema disagrees with what the server accepts. | + +Body fields: `name` (required), `type` (required, `"query"`), `database_id` (required), `dataset_query` (required native query whose `{{tag}}` placeholders are its inputs), `parameters` (one per template tag), `collection_id` (optional; the root when omitted), `description`, `visualization_settings`. + +### `mb data-action update ` + +Patch a data action; the body carries only the fields to change (`name`, `database_id`, `dataset_query`, `parameters`, `collection_id`, `description`, `visualization_settings`, `archived`). + +```sh +cat patch.json | mb data-action update 1 +mb data-action update 1 --body '{"name":"Rename an order"}' +``` + +### `mb data-action archive ` + +```sh +mb data-action archive 1 +``` + +### `mb data-action delete ` + +Delete a data action and the dashboard buttons that run it. + +```sh +mb data-action delete 1 --yes +``` + +### `mb data-action execute ` + +Run a data action. The body is `{"parameters": {...}}` keyed by parameter id; the result reports `rows-affected`. + +```sh +mb data-action execute 1 --body '{"parameters":{"id":1,"note":"rush"}}' +mb data-action execute 1 --file values.json +``` + ## Segments CRUD on `/api/segment`. A segment is a saved MBQL filter macro tied to a table — used in card filters to share a reusable predicate. Mutating endpoints require a `revision_message` for the audit log. diff --git a/packages/cli/skill-data/data-action/SKILL.md b/packages/cli/skill-data/data-action/SKILL.md new file mode 100644 index 00000000..54f18fb2 --- /dev/null +++ b/packages/cli/skill-data/data-action/SKILL.md @@ -0,0 +1,125 @@ +--- +name: data-action +description: Author and run Metabase data actions with the `mb` CLI — saved, parameterized native SQL writes (INSERT, UPDATE, DELETE) filed in a collection and run with values. Covers enabling them on a database, the body, mapping template tags to parameters, running, and the lifecycle. Triggers — "add a data action", "let users update a row", "a button that inserts a record", "run a write query", "a form that saves to the database", "delete rows from Metabase". +allowed-tools: Read, Write, Edit, Bash, AskUserQuestion +requires: [dataActionsWithoutModel] +--- + +# Data actions + +A **data action** is one native SQL statement that writes — `INSERT`, `UPDATE` or `DELETE` — with named inputs. It lives in a collection, and dashboards, apps and `mb data-action execute` run it with values. + +Flag conventions and `./.scratch` are in `core`; the native query and its template tags follow `native-sql`. This skill covers what a data action adds. + +## Check the database first + +Data actions are off on every database until an admin turns on **Model actions** for it (Admin → Databases → the database). `mb` cannot change that setting. + +```bash +mb db get --full --json +``` + +- `features` must contain `"actions"` — otherwise the driver cannot run writes; pick another database. +- `settings["database-enable-actions"]` must be `true`. If it is not, ask the user to have an admin enable it. Create and execute fail with `Actions are not enabled.` until then. + +## The body + +```json +{ + "name": "Set order note", + "type": "query", + "database_id": 1, + "collection_id": 12, + "dataset_query": { + "lib/type": "mbql/query", + "database": 1, + "stages": [ + { + "lib/type": "mbql.stage/native", + "native": "UPDATE orders SET note = {{note}} WHERE id = {{order_id}}", + "template-tags": { + "order_id": { + "id": "order_id", + "name": "order_id", + "display-name": "Order ID", + "type": "number", + "required": true + }, + "note": { + "id": "note", + "name": "note", + "display-name": "Note", + "type": "text", + "required": true + } + } + } + ] + }, + "parameters": [ + { + "id": "order_id", + "slug": "order_id", + "name": "Order ID", + "type": "number/=", + "target": ["variable", ["template-tag", "order_id"]], + "required": true + }, + { + "id": "note", + "slug": "note", + "name": "Note", + "type": "string/=", + "target": ["variable", ["template-tag", "note"]], + "required": true + } + ] +} +``` + +- `type` is always `"query"`. Never set `model_id` or `type: "implicit"`; `mb data-action create` refuses both. +- `database_id` and `dataset_query.database` are the same id (`mb db list`). Omit `collection_id` for the root collection (`mb collection list`). +- One statement, no `;`. `create` and `update` validate `dataset_query` as `native-sql` does. + +## Parameters: one per template tag + +Each `{{name}}` needs a `template-tags` entry and a `parameters` entry, all using the same name: + +| Tag `type` | Parameter `type` | +| ---------- | ---------------- | +| `number` | `number/=` | +| `text` | `string/=` | +| `date` | `date/single` | +| `boolean` | `boolean/=` | + +- Parameter `id` and `slug` are the tag name; `target` is `["variable", ["template-tag", ""]]`. +- Use raw variables only. A field filter (`type: "dimension"`), snippet or card reference does not work in a write — write the comparison yourself (`WHERE id = {{order_id}}`). +- Set `required: true` on every input the statement cannot run without (`WHERE` keys, `NOT NULL` columns). +- Wrap an optional input in `[[ … ]]` with `required: false`; the clause drops out when no value is given. This makes one action do partial updates: `UPDATE orders SET updated_at = now() [[, note = {{note}}]] [[, status = {{status}}]] WHERE id = {{order_id}}`. + +## Run it + +```bash +mb data-action execute --body '{"parameters":{"order_id":1042,"note":"rush delivery"}}' +``` + +`parameters` is keyed by parameter `id`. The result is `{"rows-affected": }`. A missing required value is refused. A run cannot be undone, so confirm with the user before running against data that matters, and test a new action on a row created for the purpose. + +## Lifecycle + +| Task | Command | +| ------------------------ | ---------------------------------------------------------------------------------------- | +| Find | `mb data-action list` (unarchived only), `mb search --models action ` | +| Inspect | `mb data-action get --full --json` | +| Change SQL, inputs, name | `mb data-action update --file patch.json` (changed fields only) | +| Move | `mb data-action update --body '{"collection_id":}'` | +| Archive / restore | `mb data-action archive ` / `mb data-action update --body '{"archived":false}'` | +| Delete | `mb data-action delete --yes` (also removes the dashboard buttons that run it) | + +Archiving or deleting a collection archives or deletes its data actions. Existing model-bound or implicit data actions still list, run and archive; leave them as they are. + +## Don't + +- Don't create a model for a data action. +- Don't concatenate values into the SQL; every value goes through a template tag. +- Don't run a data action on production data without the user's go-ahead. diff --git a/packages/cli/skill-data/native-sql/SKILL.md b/packages/cli/skill-data/native-sql/SKILL.md index fd9735a9..b2179b51 100644 --- a/packages/cli/skill-data/native-sql/SKILL.md +++ b/packages/cli/skill-data/native-sql/SKILL.md @@ -112,6 +112,6 @@ mb card query 12 --parameters '[{"type":"string/=","target":["dimension",["templ - Don't wrap a field filter in an operator (`WHERE col = {{ff}}`) — write it bare (`WHERE {{ff}}`). - Don't author the flat `{type:"native", …}` form — send the native stage above. -- Don't use native SQL for DDL or multiple statements — the editor is read-only, single-statement; `CREATE`/`UPDATE`/`;`-chained SQL is unsupported. To materialize a table, use a `transform`. +- Don't use native SQL for DDL or multiple statements — the editor is read-only, single-statement; `CREATE`/`UPDATE`/`;`-chained SQL is unsupported. To materialize a table, use a `transform`; to write rows (`INSERT`/`UPDATE`/`DELETE`), use a data action (`data-action`). - Don't expect `[[ ]]` to save you from a case/type mismatch — `WHERE plan = {{p}}` returns zero rows on a case-sensitive engine if the value's case is off; that's a value problem, not syntax. - Don't reach for native when a structured query fits — you lose the engine-independence and readability of an `mbql.stage/mbql` stage. diff --git a/packages/cli/src/commands/data-action/archive.ts b/packages/cli/src/commands/data-action/archive.ts new file mode 100644 index 00000000..867c8402 --- /dev/null +++ b/packages/cli/src/commands/data-action/archive.ts @@ -0,0 +1,30 @@ +import { DataAction } from "@metabase/client/domain/data-action"; +import { dataActionView } from "../../output/views/data-action"; +import { renderSummary } from "../../output/render"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "archive", description: "Archive (soft-delete) a data action by id" }, + requires: ["dataAction.archive"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + id: { type: "positional", description: "Data action id", required: true }, + }, + outputSchema: DataAction, + examples: ["mb data-action archive 1"], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const client = await getClient(); + const updated = await client.dataAction.archive(id); + renderSummary( + updated, + dataActionView, + `Archived data action ${updated.id} "${updated.name}".`, + ctx, + ); + }, +}); diff --git a/packages/cli/src/commands/data-action/create.ts b/packages/cli/src/commands/data-action/create.ts new file mode 100644 index 00000000..f4993266 --- /dev/null +++ b/packages/cli/src/commands/data-action/create.ts @@ -0,0 +1,51 @@ +import { DataAction, DataActionCreateInput } from "@metabase/client/domain/data-action"; +import { dataActionView } from "../../output/views/data-action"; +import { renderSummary } from "../../output/render"; +import { readBody } from "../../runtime/body"; +import { bodyInputFlags } from "../body-flags"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { defineMetabaseCommand } from "../runtime"; +import { + DATA_ACTION_DATASET_QUERY_LABELS, + preflightMbql5Query, + skipValidateFlag, +} from "../validate-query"; + +export default defineMetabaseCommand({ + meta: { name: "create", description: "Create a data action from JSON" }, + details: + 'A data action is a parameterized native query that writes to a database, filed in a collection. The JSON body needs `name`, `type: "query"`, `database_id`, a native `dataset_query` whose `{{tag}}` placeholders are its inputs, and `parameters` targeting those tags; `collection_id` picks the collection (omit for the root). Data actions must be enabled on the database. An MBQL 5 `dataset_query` is checked against a bundled JSON Schema before sending; pass --skip-validate to bypass.', + skills: [ + { skill: "data-action", purpose: "author a data action and its parameters" }, + { skill: "native-sql", purpose: "the native query and its template tags" }, + ], + requires: ["dataAction.create"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...bodyInputFlags, + ...skipValidateFlag, + }, + inputSchema: DataActionCreateInput, + outputSchema: DataAction, + examples: [ + "cat data-action.json | mb data-action create", + "mb data-action create --file data-action.json", + "mb data-action create --file data-action.json --skip-validate", + ], + async run({ args, ctx, getClient }) { + const body = await readBody({ flag: args.body, file: args.file }, DataActionCreateInput); + preflightMbql5Query(body.dataset_query, DATA_ACTION_DATASET_QUERY_LABELS, { + skip: args["skip-validate"] === true, + }); + const client = await getClient(); + const created = await client.dataAction.create(body); + renderSummary( + created, + dataActionView, + `Created data action ${created.id} "${created.name}".`, + ctx, + ); + }, +}); diff --git a/packages/cli/src/commands/data-action/delete.ts b/packages/cli/src/commands/data-action/delete.ts new file mode 100644 index 00000000..dcb7f37a --- /dev/null +++ b/packages/cli/src/commands/data-action/delete.ts @@ -0,0 +1,31 @@ +import { confirmAndDelete, DeleteResult } from "../delete-runtime"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "delete", description: "Delete a data action by id" }, + requires: ["dataAction.delete"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + yes: { type: "boolean", description: "Skip confirmation", default: false }, + id: { type: "positional", description: "Data action id", required: true }, + }, + outputSchema: DeleteResult, + examples: ["mb data-action delete 1 --yes", "mb data-action delete 1"], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const client = await getClient(); + await confirmAndDelete({ + id, + yes: args.yes, + promptMessage: `Delete data action ${id}?`, + successMessage: `Deleted data action ${id}.`, + abortMessage: `Aborted; data action ${id} was not deleted.`, + deleteResource: () => client.dataAction.delete(id), + ctx, + }); + }, +}); diff --git a/packages/cli/src/commands/data-action/execute.ts b/packages/cli/src/commands/data-action/execute.ts new file mode 100644 index 00000000..f88e50b1 --- /dev/null +++ b/packages/cli/src/commands/data-action/execute.ts @@ -0,0 +1,44 @@ +import { + DataActionExecuteInput, + DataActionExecuteResult, +} from "@metabase/client/domain/data-action"; +import { dataActionExecuteResultView } from "../../output/views/data-action"; +import { renderSummary } from "../../output/render"; +import { readBody } from "../../runtime/body"; +import { bodyInputFlags } from "../body-flags"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "execute", description: "Run a data action by id" }, + details: + 'The JSON body is `{"parameters": {...}}`, keyed by the data action\'s parameter ids. The data action writes to its database, so a run cannot be undone by the CLI.', + skills: [{ skill: "data-action", purpose: "the parameter values a data action takes" }], + requires: ["dataAction.execute"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...bodyInputFlags, + id: { type: "positional", description: "Data action id", required: true }, + }, + inputSchema: DataActionExecuteInput, + outputSchema: DataActionExecuteResult, + examples: [ + 'mb data-action execute 1 --body \'{"parameters":{"id":1,"note":"rush"}}\'', + "mb data-action execute 1 --file values.json", + ], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const body = await readBody({ flag: args.body, file: args.file }, DataActionExecuteInput); + const client = await getClient(); + const result = await client.dataAction.execute(id, body); + renderSummary( + result, + dataActionExecuteResultView, + `Ran data action ${id}; ${result["rows-affected"]} rows affected.`, + ctx, + ); + }, +}); diff --git a/packages/cli/src/commands/data-action/get.ts b/packages/cli/src/commands/data-action/get.ts new file mode 100644 index 00000000..d7a640b7 --- /dev/null +++ b/packages/cli/src/commands/data-action/get.ts @@ -0,0 +1,25 @@ +import { DataAction } from "@metabase/client/domain/data-action"; +import { dataActionView } from "../../output/views/data-action"; +import { renderItem } from "../../output/render"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "get", description: "Get a data action by id" }, + requires: ["dataAction.get"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + id: { type: "positional", description: "Data action id", required: true }, + }, + outputSchema: DataAction, + examples: ["mb data-action get 1", "mb data-action get 1 --json --full"], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const client = await getClient(); + const dataAction = await client.dataAction.get(id); + renderItem(dataAction, dataActionView, ctx); + }, +}); diff --git a/packages/cli/src/commands/data-action/index.ts b/packages/cli/src/commands/data-action/index.ts new file mode 100644 index 00000000..10f7402e --- /dev/null +++ b/packages/cli/src/commands/data-action/index.ts @@ -0,0 +1,19 @@ +import { defineCommandGroup } from "../group"; + +export default defineCommandGroup({ + name: "data-action", + description: "Manage Metabase data actions", + skills: [ + { skill: "data-action", purpose: "author and run data actions" }, + { skill: "native-sql", purpose: "the data action's native query and its template tags" }, + ], + subCommands: { + list: () => import("./list").then((mod) => mod.default), + get: () => import("./get").then((mod) => mod.default), + create: () => import("./create").then((mod) => mod.default), + update: () => import("./update").then((mod) => mod.default), + archive: () => import("./archive").then((mod) => mod.default), + delete: () => import("./delete").then((mod) => mod.default), + execute: () => import("./execute").then((mod) => mod.default), + }, +}); diff --git a/packages/cli/src/commands/data-action/list.ts b/packages/cli/src/commands/data-action/list.ts new file mode 100644 index 00000000..cde994f8 --- /dev/null +++ b/packages/cli/src/commands/data-action/list.ts @@ -0,0 +1,22 @@ +import { DataActionCompact } from "@metabase/client/domain/data-action"; +import { dataActionView } from "../../output/views/data-action"; +import { renderList } from "../../output/render"; +import { listEnvelopeSchema } from "../../output/types"; +import { windowList } from "../../output/window"; +import { connectionFlags, listFlags, outputFlags, profileFlag } from "../flags"; +import { defineMetabaseCommand } from "../runtime"; + +export const DataActionListEnvelope = listEnvelopeSchema(DataActionCompact); + +export default defineMetabaseCommand({ + meta: { name: "list", description: "List data actions" }, + requires: ["dataAction.list"], + args: { ...outputFlags, ...listFlags, ...profileFlag, ...connectionFlags }, + outputSchema: DataActionListEnvelope, + examples: ["mb data-action list", "mb data-action list --json"], + async run({ ctx, getClient }) { + const client = await getClient(); + const { data, total } = await client.dataAction.list(); + renderList(windowList(data, ctx.range, total), dataActionView, ctx); + }, +}); diff --git a/packages/cli/src/commands/data-action/update.ts b/packages/cli/src/commands/data-action/update.ts new file mode 100644 index 00000000..bdb30233 --- /dev/null +++ b/packages/cli/src/commands/data-action/update.ts @@ -0,0 +1,53 @@ +import { DataAction, DataActionUpdateInput } from "@metabase/client/domain/data-action"; +import { dataActionView } from "../../output/views/data-action"; +import { renderSummary } from "../../output/render"; +import { readBody } from "../../runtime/body"; +import { bodyInputFlags } from "../body-flags"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; +import { + DATA_ACTION_DATASET_QUERY_LABELS, + preflightMbql5Query, + skipValidateFlag, +} from "../validate-query"; + +export default defineMetabaseCommand({ + meta: { name: "update", description: "Update a data action by id" }, + details: + "The JSON body carries only the fields to change. A new MBQL 5 `dataset_query` is checked against a bundled JSON Schema before sending; pass --skip-validate to bypass.", + skills: [{ skill: "data-action", purpose: "the fields of a data action" }], + requires: ["dataAction.update"], + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...bodyInputFlags, + ...skipValidateFlag, + id: { type: "positional", description: "Data action id", required: true }, + }, + inputSchema: DataActionUpdateInput, + outputSchema: DataAction, + examples: [ + "cat patch.json | mb data-action update 1", + "mb data-action update 1 --file patch.json", + 'mb data-action update 1 --body \'{"name":"Rename an order"}\'', + ], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const body = await readBody({ flag: args.body, file: args.file }, DataActionUpdateInput); + if (body.dataset_query !== undefined) { + preflightMbql5Query(body.dataset_query, DATA_ACTION_DATASET_QUERY_LABELS, { + skip: args["skip-validate"] === true, + }); + } + const client = await getClient(); + const updated = await client.dataAction.update(id, body); + renderSummary( + updated, + dataActionView, + `Updated data action ${updated.id} "${updated.name}".`, + ctx, + ); + }, +}); diff --git a/packages/cli/src/commands/validate-query.ts b/packages/cli/src/commands/validate-query.ts index 6168d615..d5a1166f 100644 --- a/packages/cli/src/commands/validate-query.ts +++ b/packages/cli/src/commands/validate-query.ts @@ -24,6 +24,11 @@ export const CARD_DATASET_QUERY_LABELS: PreflightLabels = { bodyNoun: "dataset_query", }; +export const DATA_ACTION_DATASET_QUERY_LABELS: PreflightLabels = { + contextLabel: "dataAction.dataset_query validation failed", + bodyNoun: "dataset_query", +}; + export const TRANSFORM_SOURCE_QUERY_LABELS: PreflightLabels = { contextLabel: "transform.source.query validation failed", bodyNoun: "source.query", diff --git a/packages/cli/src/core/skills.test.ts b/packages/cli/src/core/skills.test.ts index 83ccde55..9911b161 100644 --- a/packages/cli/src/core/skills.test.ts +++ b/packages/cli/src/core/skills.test.ts @@ -678,7 +678,11 @@ describe("the shipped skills", () => { all.filter((skill) => skill.requires.length > 0).map((skill) => [skill.name, skill.requires]), ); - expect(bound).toEqual({ "git-sync": ["remoteSync"], transform: ["transforms"] }); + expect(bound).toEqual({ + "data-action": ["dataActionsWithoutModel"], + "git-sync": ["remoteSync"], + transform: ["transforms"], + }); const withMarkersLeft = all .filter((skill) => { const content = readSkillContent(skill, { includeExtras: true, profile: profileAt(58) }); @@ -688,6 +692,7 @@ describe("the shipped skills", () => { expect(all.map((skill) => skill.name)).toEqual([ "core", "dashboard", + "data-action", "data-workflow", "document", "git-sync", diff --git a/packages/cli/src/main.ts b/packages/cli/src/main.ts index b58d51c0..2340c5e3 100644 --- a/packages/cli/src/main.ts +++ b/packages/cli/src/main.ts @@ -22,6 +22,7 @@ const main: CommandDef = defineCommand({ card: () => import("./commands/card").then((mod) => mod.default), dashboard: () => import("./commands/dashboard").then((mod) => mod.default), subscription: () => import("./commands/subscription").then((mod) => mod.default), + "data-action": () => import("./commands/data-action").then((mod) => mod.default), alert: () => import("./commands/alert").then((mod) => mod.default), collection: () => import("./commands/collection").then((mod) => mod.default), library: () => import("./commands/library").then((mod) => mod.default), diff --git a/packages/cli/src/output/views/data-action.ts b/packages/cli/src/output/views/data-action.ts new file mode 100644 index 00000000..633375e1 --- /dev/null +++ b/packages/cli/src/output/views/data-action.ts @@ -0,0 +1,24 @@ +import { + type DataAction, + DataActionCompact, + DataActionExecuteResult, +} from "@metabase/client/domain/data-action"; + +import type { ResourceView } from "../view"; + +export const dataActionView: ResourceView = { + compactPick: DataActionCompact, + tableColumns: [ + { key: "id", label: "ID" }, + { key: "name", label: "Name" }, + { key: "type", label: "Type" }, + { key: "collection_id", label: "Collection" }, + { key: "database_id", label: "Database" }, + { key: "archived", label: "Archived" }, + ], +}; + +export const dataActionExecuteResultView: ResourceView = { + compactPick: DataActionExecuteResult, + tableColumns: [{ key: "rows-affected", label: "Rows affected" }], +}; diff --git a/packages/cli/src/runtime/command-help.test.ts b/packages/cli/src/runtime/command-help.test.ts index bcbbdb96..7319e381 100644 --- a/packages/cli/src/runtime/command-help.test.ts +++ b/packages/cli/src/runtime/command-help.test.ts @@ -360,6 +360,13 @@ const ALL_COMMANDS = [ "subscription create", "subscription update", "subscription archive", + "data-action list", + "data-action get", + "data-action create", + "data-action update", + "data-action archive", + "data-action delete", + "data-action execute", "alert list", "alert get", "alert create", @@ -482,6 +489,9 @@ const BODY_COMMANDS = [ "dashboard update-dashcard", "subscription create", "subscription update", + "data-action create", + "data-action update", + "data-action execute", "alert create", "alert update", "collection create", diff --git a/packages/client/src/client.ts b/packages/client/src/client.ts index 2b01acb3..ded24947 100644 --- a/packages/client/src/client.ts +++ b/packages/client/src/client.ts @@ -3,6 +3,7 @@ import { cardResource } from "./resources/card"; import { collectionResource } from "./resources/collection"; import { contentTranslationResource } from "./resources/content-translation"; import { dashboardResource } from "./resources/dashboard"; +import { dataActionResource } from "./resources/data-action"; import { databaseResource } from "./resources/database"; import { datasetResource } from "./resources/dataset"; import { dependencyResource } from "./resources/dependency"; @@ -49,6 +50,7 @@ export function createClient(config: ClientCredentials, options: ClientOptions) collection: collectionResource(transport), contentTranslation: contentTranslationResource(transport), dashboard: dashboardResource(transport), + dataAction: dataActionResource(transport), database: databaseResource(transport), dataset: datasetResource(transport), dependency: dependencyResource(transport), diff --git a/packages/client/src/domain/data-action.ts b/packages/client/src/domain/data-action.ts new file mode 100644 index 00000000..933af00a --- /dev/null +++ b/packages/client/src/domain/data-action.ts @@ -0,0 +1,111 @@ +import { z } from "zod"; + +import type { Features } from "../version/features"; + +import { DatasetQuery } from "./query"; + +export const DataActionParameter = z + .object({ + id: z.string(), + type: z.string(), + name: z.string().optional(), + slug: z.string().optional(), + target: z.unknown().optional(), + required: z.boolean().optional(), + }) + .loose(); +export type DataActionParameter = z.infer; + +const DataActionBase = z + .object({ + id: z.number().int(), + name: z.string(), + description: z.string().nullable(), + type: z.string(), + model_id: z.number().int().nullable(), + // A query-type data action carries its query and database; an implicit one has neither key. + database_id: z.number().int().nullable().optional(), + dataset_query: z.unknown().optional(), + parameters: z.array(DataActionParameter).nullable(), + visualization_settings: z.unknown(), + archived: z.boolean(), + public_uuid: z.string().nullable(), + entity_id: z.string().nullable(), + creator_id: z.number().int(), + created_at: z.string(), + updated_at: z.string(), + }) + .loose(); + +export const DataAction = DataActionBase.extend({ + collection_id: z.number().int().nullable(), +}); +export type DataAction = z.infer; + +function withoutCollection(wire: z.infer): DataAction { + return { ...wire, collection_id: null }; +} + +/** The shape every data action endpoint answers on a server with `features`, read as `DataAction`. */ +export function dataActionSchema(features: Features): z.ZodType { + return features.dataActionCollections ? DataAction : DataActionBase.transform(withoutCollection); +} + +export const DataActionCompact = DataAction.pick({ + id: true, + name: true, + type: true, + collection_id: true, + database_id: true, + archived: true, +}).strip(); +export type DataActionCompact = z.infer; + +const DataActionParameterInput = z + .object({ + id: z.string().min(1), + type: z.string().min(1), + name: z.string().optional(), + slug: z.string().optional(), + target: z.unknown(), + required: z.boolean().optional(), + }) + .loose(); + +export const DataActionCreateInput = z + .object({ + name: z.string().min(1), + type: z.literal("query"), + database_id: z.number().int().positive(), + dataset_query: DatasetQuery, + parameters: z.array(DataActionParameterInput).optional(), + collection_id: z.number().int().positive().nullable().optional(), + description: z.string().nullable().optional(), + visualization_settings: z.unknown().optional(), + }) + .strict(); +export type DataActionCreateInput = z.infer; + +export const DataActionUpdateInput = z + .object({ + name: z.string().min(1).optional(), + database_id: z.number().int().positive().optional(), + dataset_query: DatasetQuery.optional(), + parameters: z.array(DataActionParameterInput).optional(), + collection_id: z.number().int().positive().nullable().optional(), + description: z.string().nullable().optional(), + visualization_settings: z.unknown().optional(), + archived: z.boolean().optional(), + }) + .strict(); +export type DataActionUpdateInput = z.infer; + +const DataActionParameterValue = z.union([z.string(), z.number(), z.boolean(), z.null()]); + +export const DataActionExecuteInput = z + .object({ parameters: z.record(z.string(), DataActionParameterValue) }) + .strict(); +export type DataActionExecuteInput = z.infer; + +export const DataActionExecuteResult = z.object({ "rows-affected": z.number().int() }).loose(); +export type DataActionExecuteResult = z.infer; diff --git a/packages/client/src/index.test.ts b/packages/client/src/index.test.ts index 608146fe..a92e36bf 100644 --- a/packages/client/src/index.test.ts +++ b/packages/client/src/index.test.ts @@ -14,6 +14,9 @@ import { DashboardCreateInput, DashboardUpdateInput, DashcardPatchInput, + DataActionCreateInput, + DataActionExecuteInput, + DataActionUpdateInput, DocumentCopyInput, DocumentCreateInput, DocumentUpdateInput, @@ -96,6 +99,9 @@ const writePath = { DashboardCreateInput: pin(DashboardCreateInput), DashboardUpdateInput: pin(DashboardUpdateInput), DashcardPatchInput: pin(DashcardPatchInput), + DataActionCreateInput: pin(DataActionCreateInput), + DataActionExecuteInput: pin(DataActionExecuteInput), + DataActionUpdateInput: pin(DataActionUpdateInput), DocumentCopyInput: pin(DocumentCopyInput), DocumentCreateInput: pin(DocumentCreateInput), DocumentUpdateInput: pin(DocumentUpdateInput), diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 36be6859..5019fc8f 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -149,6 +149,16 @@ export { DashcardCompact, DashcardPatchInput, } from "./domain/dashboard"; +export { + DataAction, + DataActionCompact, + DataActionCreateInput, + DataActionExecuteInput, + DataActionExecuteResult, + DataActionParameter, + dataActionSchema, + DataActionUpdateInput, +} from "./domain/data-action"; export { Database, DatabaseCompact, diff --git a/packages/client/src/resources/data-action.test.ts b/packages/client/src/resources/data-action.test.ts new file mode 100644 index 00000000..e4492f83 --- /dev/null +++ b/packages/client/src/resources/data-action.test.ts @@ -0,0 +1,213 @@ +import { describe, expect, it } from "vitest"; + +import { createClient } from "../client"; +import type { DataActionCreateInput } from "../domain/data-action"; +import type { ClientCredentials } from "../http/transport"; +import { captureFetch, jsonResponse, TEST_USER_AGENT } from "../testing/fetch-capture"; +import { CapabilityError } from "../version/preflight-error"; +import { createServerProfile, type ServerProfile } from "../version/profile"; + +const CREDENTIALS: ClientCredentials = { + url: "https://mb.example.com/metabase", + credential: { kind: "apiKey", apiKey: "mb_wire_test_key" }, +}; + +const ACTION_V64 = { + id: 7, + name: "Rename order", + description: null, + type: "query", + model_id: null, + database_id: 2, + dataset_query: { + database: 2, + type: "native", + native: { query: "UPDATE orders SET note = {{note}} WHERE id = {{id}}" }, + }, + parameters: [{ id: "id", type: "number/=" }], + visualization_settings: null, + archived: false, + public_uuid: null, + entity_id: "ccccccccccccccccccccc", + creator_id: 1, + created_at: "2026-01-02T03:04:05.678Z", + updated_at: "2026-01-02T03:04:05.678Z", +}; + +const ACTION = { ...ACTION_V64, collection_id: 3 }; + +const CREATE_BODY: DataActionCreateInput = { + name: "Rename order", + type: "query", + database_id: 2, + collection_id: 3, + dataset_query: { + database: 2, + type: "native", + native: { query: "UPDATE orders SET note = {{note}} WHERE id = {{id}}" }, + }, + parameters: [{ id: "id", type: "number/=", target: ["variable", ["template-tag", "id"]] }], +}; + +const HEAD_SERVER = createServerProfile({ + edition: "oss", + version: { kind: "release", tag: "v0.65.0", major: 65, patch: 0 }, + date: null, + hash: null, + tokenFeatures: null, +}); + +const V64_SERVER = createServerProfile({ + edition: "oss", + version: { kind: "release", tag: "v0.64.0", major: 64, patch: 0 }, + date: null, + hash: null, + tokenFeatures: null, +}); + +const JSON_REQUEST_HEADERS = { + accept: "application/json", + "content-type": "application/json", + "user-agent": TEST_USER_AGENT, + "x-api-key": "mb_wire_test_key", +}; + +const JSON_READ_HEADERS = { + accept: "application/json", + "user-agent": TEST_USER_AGENT, + "x-api-key": "mb_wire_test_key", +}; + +function clientOver(responses: Array, server: ServerProfile = HEAD_SERVER) { + const capture = captureFetch(responses); + const mb = createClient(CREDENTIALS, { + userAgent: TEST_USER_AGENT, + fetchImpl: capture.fetch, + server, + }); + return { mb, capture }; +} + +describe("data action resource wire requests", () => { + it("sends the list request without a query string", async () => { + const { mb, capture } = clientOver([jsonResponse([ACTION])]); + + const result = await mb.dataAction.list(); + + expect(result).toEqual({ data: [ACTION], total: null }); + expect(capture.calls).toEqual([ + { + url: "https://mb.example.com/metabase/api/action", + method: "GET", + headers: JSON_READ_HEADERS, + body: null, + }, + ]); + }); + + it("reads a data action without a collection on a server whose data actions have none", async () => { + const { mb } = clientOver([jsonResponse([ACTION_V64])], V64_SERVER); + + const result = await mb.dataAction.list(); + + expect(result).toEqual({ data: [{ ...ACTION_V64, collection_id: null }], total: null }); + }); + + it("sends the get request", async () => { + const { mb, capture } = clientOver([jsonResponse(ACTION)]); + + await mb.dataAction.get(7); + + expect(capture.calls).toEqual([ + { + url: "https://mb.example.com/metabase/api/action/7", + method: "GET", + headers: JSON_READ_HEADERS, + body: null, + }, + ]); + }); + + it("sends the create request with the full body", async () => { + const { mb, capture } = clientOver([jsonResponse(ACTION)]); + + await mb.dataAction.create(CREATE_BODY); + + expect(capture.calls).toEqual([ + { + url: "https://mb.example.com/metabase/api/action", + method: "POST", + headers: JSON_REQUEST_HEADERS, + body: JSON.stringify(CREATE_BODY), + }, + ]); + }); + + it("refuses create on a server whose data actions need a model, before any request", async () => { + const { mb, capture } = clientOver([], V64_SERVER); + + await expect(mb.dataAction.create(CREATE_BODY)).rejects.toBeInstanceOf(CapabilityError); + expect(capture.calls).toEqual([]); + }); + + it("sends the update request with only the patched fields", async () => { + const { mb, capture } = clientOver([jsonResponse(ACTION)]); + + await mb.dataAction.update(7, { name: "Rename an order" }); + + expect(capture.calls).toEqual([ + { + url: "https://mb.example.com/metabase/api/action/7", + method: "PUT", + headers: JSON_REQUEST_HEADERS, + body: JSON.stringify({ name: "Rename an order" }), + }, + ]); + }); + + it("sends archive as an update that sets archived", async () => { + const { mb, capture } = clientOver([jsonResponse({ ...ACTION, archived: true })]); + + await mb.dataAction.archive(7); + + expect(capture.calls).toEqual([ + { + url: "https://mb.example.com/metabase/api/action/7", + method: "PUT", + headers: JSON_REQUEST_HEADERS, + body: JSON.stringify({ archived: true }), + }, + ]); + }); + + it("sends the delete request", async () => { + const { mb, capture } = clientOver([new Response(null, { status: 204 })]); + + await mb.dataAction.delete(7); + + expect(capture.calls).toEqual([ + { + url: "https://mb.example.com/metabase/api/action/7", + method: "DELETE", + headers: { accept: "*/*", "user-agent": TEST_USER_AGENT, "x-api-key": "mb_wire_test_key" }, + body: null, + }, + ]); + }); + + it("sends the execute request with the parameter values", async () => { + const { mb, capture } = clientOver([jsonResponse({ "rows-affected": 1 })]); + + const result = await mb.dataAction.execute(7, { parameters: { id: 1, note: "rush" } }); + + expect(result).toEqual({ "rows-affected": 1 }); + expect(capture.calls).toEqual([ + { + url: "https://mb.example.com/metabase/api/action/7/execute", + method: "POST", + headers: JSON_REQUEST_HEADERS, + body: JSON.stringify({ parameters: { id: 1, note: "rush" } }), + }, + ]); + }); +}); diff --git a/packages/client/src/resources/data-action.ts b/packages/client/src/resources/data-action.ts new file mode 100644 index 00000000..0c877332 --- /dev/null +++ b/packages/client/src/resources/data-action.ts @@ -0,0 +1,96 @@ +import { z } from "zod"; + +import { + type DataAction, + type DataActionCreateInput, + type DataActionExecuteInput, + DataActionExecuteResult, + dataActionSchema, + type DataActionUpdateInput, +} from "../domain/data-action"; +import type { RequestOptions, Transport } from "../http/transport"; +import type { ListResult } from "../list"; + +// Every path parameter here is a numeric id, so no fragment needs `encodeURIComponent`. +export function dataActionResource(transport: Transport) { + async function readSchema(options: RequestOptions): Promise> { + const { features } = await transport.server(options); + return dataActionSchema(features); + } + + /** List the unarchived data actions the caller can see. `GET /api/action` answers a bare array. */ + async function list(options: RequestOptions = {}): Promise> { + await transport.require("dataAction.list", options); + const schema = await readSchema(options); + const data = await transport.requestParsed(z.array(schema), "/api/action", { ...options }); + return { data, total: null }; + } + + /** Get one unarchived data action by id. */ + async function get(id: number, options: RequestOptions = {}): Promise { + await transport.require("dataAction.get", options); + const schema = await readSchema(options); + return transport.requestParsed(schema, `/api/action/${id}`, { ...options }); + } + + /** Create a data action — a parameterized native query that writes to a database — in a collection. */ + async function create( + params: DataActionCreateInput, + options: RequestOptions = {}, + ): Promise { + await transport.require("dataAction.create", options); + const schema = await readSchema(options); + return transport.requestParsed(schema, "/api/action", { + ...options, + method: "POST", + body: params, + }); + } + + /** Update a data action by id, patching only the fields the body carries. */ + async function update( + id: number, + params: DataActionUpdateInput, + options: RequestOptions = {}, + ): Promise { + await transport.require("dataAction.update", options); + const schema = await readSchema(options); + return transport.requestParsed(schema, `/api/action/${id}`, { + ...options, + method: "PUT", + body: params, + }); + } + + /** Archive (soft-delete) a data action by id. Metabase models this as an update, not its own endpoint. */ + async function archive(id: number, options: RequestOptions = {}): Promise { + await transport.require("dataAction.archive", options); + return update(id, { archived: true }, options); + } + + /** Delete a data action by id, removing it and the dashboard buttons that run it. */ + async function remove(id: number, options: RequestOptions = {}): Promise { + await transport.require("dataAction.delete", options); + await transport.requestRaw(`/api/action/${id}`, { + ...options, + method: "DELETE", + expectContentType: "binary", + }); + } + + /** Execute a data action by id with parameter values keyed by parameter id. */ + async function execute( + id: number, + params: DataActionExecuteInput, + options: RequestOptions = {}, + ): Promise { + await transport.require("dataAction.execute", options); + return transport.requestParsed(DataActionExecuteResult, `/api/action/${id}/execute`, { + ...options, + method: "POST", + body: params, + }); + } + + return { list, get, create, update, archive, delete: remove, execute }; +} diff --git a/packages/client/src/version/features.ts b/packages/client/src/version/features.ts index 6d7a92b1..54968a50 100644 --- a/packages/client/src/version/features.ts +++ b/packages/client/src/version/features.ts @@ -15,6 +15,8 @@ export interface FeatureRule { } export const FEATURE_RULES = { + dataActionCollections: { since: 65 }, + dataActionsWithoutModel: { since: 65 }, transforms: { since: 59 }, transformJobActivation: { since: 61 }, measures: { since: 59 }, diff --git a/packages/client/src/version/requirements.test.ts b/packages/client/src/version/requirements.test.ts index d8097bec..f395c4d7 100644 --- a/packages/client/src/version/requirements.test.ts +++ b/packages/client/src/version/requirements.test.ts @@ -12,6 +12,7 @@ import { cardResource } from "../resources/card"; import { collectionResource } from "../resources/collection"; import { contentTranslationResource } from "../resources/content-translation"; import { dashboardResource } from "../resources/dashboard"; +import { dataActionResource } from "../resources/data-action"; import { databaseResource } from "../resources/database"; import { datasetResource } from "../resources/dataset"; import { dependencyResource } from "../resources/dependency"; @@ -273,6 +274,11 @@ const DRIVES: ReadonlyArray = [ invoke: (t) => dashboardResource(t).get(1), wireError: "unexpected request: GET /api/dashboard/1", }, + { + key: "dataAction.get", + invoke: (t) => dataActionResource(t).get(1), + wireError: "unexpected request: GET /api/action/1", + }, { key: "database.schemas", invoke: (t) => databaseResource(t).schemas(1), diff --git a/packages/client/src/version/requirements.ts b/packages/client/src/version/requirements.ts index 637b003e..512d618c 100644 --- a/packages/client/src/version/requirements.ts +++ b/packages/client/src/version/requirements.ts @@ -32,6 +32,13 @@ export const METHOD_REQUIREMENTS = { "dashboard.checkCardReferences": [], "dashboard.parameterValues": [], "dashboard.searchParameterValues": [], + "dataAction.list": [], + "dataAction.get": [], + "dataAction.create": ["dataActionsWithoutModel"], + "dataAction.update": [], + "dataAction.archive": [], + "dataAction.delete": [], + "dataAction.execute": [], "database.list": [], "database.get": [], "database.schemas": [], diff --git a/tests/e2e/data-action.e2e.test.ts b/tests/e2e/data-action.e2e.test.ts new file mode 100644 index 00000000..0d8bba1e --- /dev/null +++ b/tests/e2e/data-action.e2e.test.ts @@ -0,0 +1,247 @@ +import { afterEach, beforeAll, describe, expect, it } from "vitest"; + +import { + DataAction, + type DataActionCreateInput, + DataActionExecuteResult, +} from "@metabase/client/domain/data-action"; +import { createTransport, type Transport } from "@metabase/client/http/transport"; +import { parseJson } from "@metabase/client/json"; +import { z } from "zod"; + +import { DataActionListEnvelope } from "../../packages/cli/src/commands/data-action/list"; +import { DeleteResult } from "../../packages/cli/src/commands/delete-runtime"; +import { USER_AGENT } from "../../packages/cli/src/core/user-agent"; +import { readBootstrap, type E2EBootstrap } from "./bootstrap-data"; +import { cliErrorCategory, cliErrorMessage } from "./cli-error"; +import { cleanupConfigHome, mkTempConfigHome, runCli } from "./run-cli"; +import { SEEDED } from "./seed/seeded"; +import { requireServer, serverHas } from "./server-gate"; + +const DOWNGRADE_REMEDY = "Or install an `@metabase/cli` release that targets this server."; + +const ACTION_NAME = "Touch customer"; + +// Rewrites a column to its own value, so a run affects one row without changing the warehouse. +const TOUCH_CUSTOMER_SQL = "UPDATE customers SET full_name = full_name WHERE id = {{customer_id}}"; + +function createBody(): DataActionCreateInput { + return { + name: ACTION_NAME, + type: "query", + database_id: SEEDED.warehouseDbId, + collection_id: SEEDED.defaultCollectionId, + dataset_query: { + "lib/type": "mbql/query", + database: SEEDED.warehouseDbId, + stages: [ + { + "lib/type": "mbql.stage/native", + native: TOUCH_CUSTOMER_SQL, + "template-tags": { + customer_id: { + id: "customer_id", + name: "customer_id", + "display-name": "Customer ID", + type: "number", + required: true, + }, + }, + }, + ], + }, + parameters: [ + { + id: "customer_id", + slug: "customer_id", + name: "Customer ID", + type: "number/=", + target: ["variable", ["template-tag", "customer_id"]], + required: true, + }, + ], + }; +} + +const skipReason = requireServer("data-action › data-action e2e", ["dataActionsWithoutModel"]); + +describe.skipIf(skipReason !== null)("data-action e2e", () => { + let bootstrap: E2EBootstrap; + let adminClient: Transport; + const tempDirs: string[] = []; + + beforeAll(async () => { + bootstrap = await readBootstrap(); + adminClient = createTransport( + { url: bootstrap.baseUrl, credential: { kind: "apiKey", apiKey: bootstrap.adminApiKey } }, + { userAgent: USER_AGENT }, + ); + }); + + afterEach(async () => { + await Promise.all(tempDirs.splice(0).map(cleanupConfigHome)); + }); + + async function cli(args: string[], stdin?: string) { + const configHome = await mkTempConfigHome(); + tempDirs.push(configHome); + return runCli({ + args, + ...(stdin === undefined ? {} : { stdin }), + configHome, + env: { MB_URL: bootstrap.baseUrl, MB_API_KEY: bootstrap.adminApiKey }, + }); + } + + async function enableActionsOnWarehouse(): Promise { + await adminClient.requestParsed(z.unknown(), `/api/database/${SEEDED.warehouseDbId}`, { + method: "PUT", + body: { settings: { "database-enable-actions": true } }, + }); + } + + async function createAction(): Promise { + const result = await cli( + ["data-action", "create", "--json", "--full"], + JSON.stringify(createBody()), + ); + expect(result.exitCode, result.stderr).toBe(0); + return parseJson(result.stdout, DataAction); + } + + it("create is refused while actions are off on the database", async () => { + const result = await cli(["data-action", "create", "--json"], JSON.stringify(createBody())); + + expect(result.exitCode).not.toBe(0); + expect(cliErrorMessage(result.stderr)).toContain("Actions are not enabled."); + expect(result.stdout).toBe(""); + }); + + it("create files a model-less query action in its collection, and list and get read it back", async () => { + await enableActionsOnWarehouse(); + + const created = await createAction(); + expect(created).toMatchObject({ + name: ACTION_NAME, + type: "query", + model_id: null, + database_id: SEEDED.warehouseDbId, + collection_id: SEEDED.defaultCollectionId, + archived: false, + }); + + const listed = await cli(["data-action", "list", "--json"]); + expect(listed.exitCode, listed.stderr).toBe(0); + expect(parseJson(listed.stdout, DataActionListEnvelope).data).toEqual([ + { + id: created.id, + name: ACTION_NAME, + type: "query", + collection_id: SEEDED.defaultCollectionId, + database_id: SEEDED.warehouseDbId, + archived: false, + }, + ]); + + const fetched = await cli(["data-action", "get", String(created.id), "--json", "--full"]); + expect(fetched.exitCode, fetched.stderr).toBe(0); + expect(parseJson(fetched.stdout, DataAction)).toMatchObject({ + id: created.id, + parameters: [expect.objectContaining({ id: "customer_id", type: "number/=" })], + }); + }); + + it("execute runs the write with the parameter values and reports the rows it affected", async () => { + await enableActionsOnWarehouse(); + const created = await createAction(); + + const result = await cli( + ["data-action", "execute", String(created.id), "--json"], + JSON.stringify({ parameters: { customer_id: 1 } }), + ); + + expect(result.exitCode, result.stderr).toBe(0); + expect(parseJson(result.stdout, DataActionExecuteResult)).toEqual({ "rows-affected": 1 }); + }); + + it("execute without a required parameter is refused by the server", async () => { + await enableActionsOnWarehouse(); + const created = await createAction(); + + const result = await cli( + ["data-action", "execute", String(created.id), "--json"], + JSON.stringify({ parameters: {} }), + ); + + expect(result.exitCode).not.toBe(0); + expect(result.stdout).toBe(""); + }); + + it("update renames, archive hides it from list, and delete removes it", async () => { + await enableActionsOnWarehouse(); + const created = await createAction(); + + const renamed = await cli( + ["data-action", "update", String(created.id), "--json", "--full"], + JSON.stringify({ name: "Touch a customer" }), + ); + expect(renamed.exitCode, renamed.stderr).toBe(0); + expect(parseJson(renamed.stdout, DataAction).name).toBe("Touch a customer"); + + const archived = await cli(["data-action", "archive", String(created.id), "--json", "--full"]); + expect(archived.exitCode, archived.stderr).toBe(0); + expect(parseJson(archived.stdout, DataAction).archived).toBe(true); + + const listed = await cli(["data-action", "list", "--json"]); + expect(listed.exitCode, listed.stderr).toBe(0); + expect(parseJson(listed.stdout, DataActionListEnvelope).data).toEqual([]); + + const deleted = await cli(["data-action", "delete", String(created.id), "--yes", "--json"]); + expect(deleted.exitCode, deleted.stderr).toBe(0); + expect(parseJson(deleted.stdout, DeleteResult)).toEqual({ + id: created.id, + deleted: true, + aborted: false, + }); + }); +}); + +describe.skipIf(serverHas("dataActionsWithoutModel"))( + "data-action on servers that need a model", + () => { + let bootstrap: E2EBootstrap; + const tempDirs: string[] = []; + + beforeAll(async () => { + bootstrap = await readBootstrap(); + }); + + afterEach(async () => { + await Promise.all(tempDirs.splice(0).map(cleanupConfigHome)); + }); + + it("create refuses with CapabilityError (exit 2) after a live probe, before sending the body", async () => { + const configHome = await mkTempConfigHome(); + tempDirs.push(configHome); + + const result = await runCli({ + args: ["data-action", "create", "--json"], + stdin: JSON.stringify(createBody()), + configHome, + env: { MB_URL: bootstrap.baseUrl, MB_API_KEY: bootstrap.adminApiKey }, + }); + + if (bootstrap.server.version === null) { + throw new Error( + "a server with no parsed version is placed past the window, never below it", + ); + } + expect(result.exitCode).toBe(2); + expect(cliErrorCategory(result.stderr)).toBe("capability"); + expect(cliErrorMessage(result.stderr)).toBe( + `This operation requires Metabase v65+ (this server is ${bootstrap.server.version.tag}). Upgrade Metabase to use it.\n${DOWNGRADE_REMEDY}`, + ); + expect(result.stdout).toBe(""); + }); + }, +); diff --git a/tests/e2e/skills.e2e.test.ts b/tests/e2e/skills.e2e.test.ts index c316796f..704a6368 100644 --- a/tests/e2e/skills.e2e.test.ts +++ b/tests/e2e/skills.e2e.test.ts @@ -25,6 +25,7 @@ const BUNDLED_VISIBLE = discoverSkills([SKILL_DATA_DIR]).filter((skill) => !skil const BUNDLED_VISIBLE_NAMES = [ "core", "dashboard", + "data-action", "data-workflow", "document", "git-sync", @@ -94,6 +95,19 @@ const TRANSFORM_UNAVAILABLE_ON_58 = { }, }; +const DATA_ACTION_UNAVAILABLE_ON_58 = { + name: "data-action", + failure: { + reason: "version-too-old", + detail: + "This operation requires Metabase v65+ (this server is v0.58.0). Upgrade Metabase to use it.", + feature: "dataActionsWithoutModel", + since: 65, + tokenFeature: null, + serverVersion: "v0.58.0", + }, +}; + const GIT_SYNC_UNAVAILABLE_ON_58 = { name: "git-sync", failure: { @@ -107,6 +121,8 @@ const GIT_SYNC_UNAVAILABLE_ON_58 = { }, }; +const FEATURE_BOUND_ON_58 = new Set(["data-action", "git-sync", "transform"]); + describe("skills e2e", () => { const tempDirs: string[] = []; @@ -120,11 +136,11 @@ describe("skills e2e", () => { return dir; } - it("ships exactly the eleven visible skills this suite names", () => { + it("ships exactly the twelve visible skills this suite names", () => { expect(BUNDLED_VISIBLE.map((skill) => skill.name)).toEqual([...BUNDLED_VISIBLE_NAMES]); }); - it("list returns the eleven bundled non-hidden skills, sorted by name, with `unavailable: null` and a clean stderr when there is no cached probe", async () => { + it("list returns the twelve bundled non-hidden skills, sorted by name, with `unavailable: null` and a clean stderr when there is no cached probe", async () => { const result = await runCli({ args: ["skills", "list", "--json"], configHome: await makeIsolatedConfigHome(), @@ -153,12 +169,14 @@ describe("skills e2e", () => { const result = await runCli({ args: ["skills", "list", "--json"], configHome }); expect(result.exitCode, result.stderr).toBe(0); - const usable = BUNDLED_VISIBLE.filter( - (skill) => skill.name !== "git-sync" && skill.name !== "transform", - ); + const usable = BUNDLED_VISIBLE.filter((skill) => !FEATURE_BOUND_ON_58.has(skill.name)); expect(parseJson(result.stdout, SkillListEnvelope)).toEqual({ ...fullList(usable), - unavailable: [GIT_SYNC_UNAVAILABLE_ON_58, TRANSFORM_UNAVAILABLE_ON_58], + unavailable: [ + DATA_ACTION_UNAVAILABLE_ON_58, + GIT_SYNC_UNAVAILABLE_ON_58, + TRANSFORM_UNAVAILABLE_ON_58, + ], }); expect(result.stderr).toBe(""); }); @@ -185,10 +203,11 @@ describe("skills e2e", () => { expect(result.exitCode).toBe(0); expect(namesInTextListing(result.stdout)).toEqual( - BUNDLED_VISIBLE_NAMES.filter((name) => name !== "git-sync" && name !== "transform"), + BUNDLED_VISIBLE_NAMES.filter((name) => !FEATURE_BOUND_ON_58.has(name)), ); expect(result.stderr).toBe( [ + `Skipped skill "data-action": ${DATA_ACTION_UNAVAILABLE_ON_58.failure.detail} Pass --unfiltered to print it anyway.`, `Skipped skill "git-sync": ${GIT_SYNC_UNAVAILABLE_ON_58.failure.detail} Pass --unfiltered to print it anyway.`, `Skipped skill "transform": ${TRANSFORM_UNAVAILABLE_ON_58.failure.detail} Pass --unfiltered to print it anyway.`, ].join("\n"), From 99fb3e573fda79d5921451a564f58ae1da725096 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Tue, 15 Sep 2026 23:06:00 -0400 Subject: [PATCH 02/24] Add transform-test support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A transform test pins a transform's behaviour without touching real data: every table it reads is replaced by an input fixture, the transform runs into a temp table, and each expectation checks that output. `transform-test list/get/create/update/delete/run` over `/api/transform-test`, with the schemas the endpoint speaks — inputs as SQL or inline rows, expectations as `empty` or `equals`, and a run report naming what each expectation found. `run` exits non-zero when the test does not pass, so it drops into CI as it stands. Gated at v64, the first version serving the endpoint. Co-Authored-By: Claude Opus 5 --- README.md | 61 +++++++++++++++++++ packages/cli/skill-data/core/SKILL.md | 2 +- .../cli/src/commands/transform-test/create.ts | 38 ++++++++++++ .../cli/src/commands/transform-test/delete.ts | 31 ++++++++++ .../cli/src/commands/transform-test/get.ts | 26 ++++++++ .../cli/src/commands/transform-test/index.ts | 14 +++++ .../cli/src/commands/transform-test/list.ts | 31 ++++++++++ .../cli/src/commands/transform-test/run.ts | 61 +++++++++++++++++++ .../cli/src/commands/transform-test/update.ts | 36 +++++++++++ packages/cli/src/main.ts | 1 + .../cli/src/output/views/transform-test.ts | 13 ++++ packages/cli/src/runtime/command-help.test.ts | 23 +++++++ 12 files changed, 336 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/commands/transform-test/create.ts create mode 100644 packages/cli/src/commands/transform-test/delete.ts create mode 100644 packages/cli/src/commands/transform-test/get.ts create mode 100644 packages/cli/src/commands/transform-test/index.ts create mode 100644 packages/cli/src/commands/transform-test/list.ts create mode 100644 packages/cli/src/commands/transform-test/run.ts create mode 100644 packages/cli/src/commands/transform-test/update.ts create mode 100644 packages/cli/src/output/views/transform-test.ts diff --git a/README.md b/README.md index 30bd0654..985d71e3 100644 --- a/README.md +++ b/README.md @@ -381,6 +381,67 @@ mb transform-tag delete 5 --yes | ------- | --------------------------------------------------------------------------------------------------------------------------------- | | `--yes` | Skip the interactive confirmation prompt. In non-TTY contexts the prompt is skipped automatically (kubectl/gh/docker convention). | +## Transform tests + +CRUD and run on `/api/transform-test`. Requires Metabase v64 or newer. A transform test pins a transform's behaviour without touching real data: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. + +An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` (each with a `database_type` the warehouse accepts as a `CAST` target) and `rows`. An expectation is either `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with the rows the output must hold exactly. + +### `mb transform-test list` + +```sh +mb transform-test list --json +mb transform-test list --transform 1 +``` + +| Flag | Description | +| ------------------ | ------------------------------------ | +| `--transform ` | Only the tests of this transform id. | + +### `mb transform-test get ` + +```sh +mb transform-test get 1 --json +``` + +### `mb transform-test create` + +```sh +mb transform-test create --file transform-test.json +``` + +| Flag | Description | +| --------------- | ----------------------- | +| `--body ` | Inline JSON body. | +| `--file ` | Path to JSON body file. | + +### `mb transform-test update ` + +Only the fields the body carries are patched; `inputs` and `expectations` replace what is stored rather than merging into it. + +```sh +mb transform-test update 1 --body '{"name":"renamed"}' +``` + +### `mb transform-test delete ` + +```sh +mb transform-test delete 1 --yes +``` + +| Flag | Description | +| ------- | --------------------------------------------------------------------------------------------------------------------------------- | +| `--yes` | Skip the interactive confirmation prompt. In non-TTY contexts the prompt is skipped automatically (kubectl/gh/docker convention). | + +### `mb transform-test run ` + +Runs the transform against the fixtures and reports what each expectation found. Exits non-zero when the test does not pass, so it drops straight into CI. + +```sh +mb transform-test run 1 +mb transform-test run 1 --json +``` + ## Databases Read warehouse metadata from `/api/database`. The `db` group exposes the full database list, the per-database record with optional table/field hydration, schema and table inspection, and the two manual-sync triggers. diff --git a/packages/cli/skill-data/core/SKILL.md b/packages/cli/skill-data/core/SKILL.md index 2e20af60..b7fef892 100644 --- a/packages/cli/skill-data/core/SKILL.md +++ b/packages/cli/skill-data/core/SKILL.md @@ -12,7 +12,7 @@ Top-level command groups (run `mb --help` to discover verbs): ``` auth | db | table | field | upload | content-translation | query | card | dashboard | snippet | segment | measure | collection | library -document | glossary | timeline | timeline-event | transform | transform-job | transform-tag | alert | subscription | setting +document | glossary | timeline | timeline-event | transform | transform-job | transform-tag | transform-test | alert | subscription | setting search | dependency | git-sync | setup | eid | uuid | upgrade | skills ``` diff --git a/packages/cli/src/commands/transform-test/create.ts b/packages/cli/src/commands/transform-test/create.ts new file mode 100644 index 00000000..50487782 --- /dev/null +++ b/packages/cli/src/commands/transform-test/create.ts @@ -0,0 +1,38 @@ +import { TransformTest, TransformTestCreateInput } from "@metabase/client/domain/transform-test"; + +import { renderSummary } from "../../output/render"; +import { transformTestView } from "../../output/views/transform-test"; +import { readBody } from "../../runtime/body"; +import { bodyInputFlags } from "../body-flags"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "create", description: "Create a transform test from JSON" }, + details: + 'The JSON body needs a `transform_id`, a `name`, the `inputs` standing in for every table the transform reads, and the `expectations` checked against its output. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` and `rows`. An expectation is either `type: "empty"` with the `sql` that must return nothing, or `type: "equals"` with the rows the output must hold.', + capabilities: { minVersion: 64 }, + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...bodyInputFlags, + }, + inputSchema: TransformTestCreateInput, + outputSchema: TransformTest, + examples: [ + "cat transform-test.json | mb transform-test create", + "mb transform-test create --file transform-test.json", + ], + async run({ args, ctx, getClient }) { + const body = await readBody({ flag: args.body, file: args.file }, TransformTestCreateInput); + const client = await getClient(); + const created = await client.transformTest.create(body); + renderSummary( + created, + transformTestView, + `Created transform test ${created.id} "${created.name}".`, + ctx, + ); + }, +}); diff --git a/packages/cli/src/commands/transform-test/delete.ts b/packages/cli/src/commands/transform-test/delete.ts new file mode 100644 index 00000000..e057deeb --- /dev/null +++ b/packages/cli/src/commands/transform-test/delete.ts @@ -0,0 +1,31 @@ +import { confirmAndDelete, DeleteResult } from "../delete-runtime"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "delete", description: "Delete a transform test by id" }, + capabilities: { minVersion: 64 }, + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + yes: { type: "boolean", description: "Skip confirmation", default: false }, + id: { type: "positional", description: "Transform test id", required: true }, + }, + outputSchema: DeleteResult, + examples: ["mb transform-test delete 1 --yes", "mb transform-test delete 1"], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const client = await getClient(); + await confirmAndDelete({ + id, + yes: args.yes, + promptMessage: `Delete transform test ${id}?`, + successMessage: `Deleted transform test ${id}.`, + abortMessage: `Aborted; transform test ${id} was not deleted.`, + deleteResource: () => client.transformTest.delete(id), + ctx, + }); + }, +}); diff --git a/packages/cli/src/commands/transform-test/get.ts b/packages/cli/src/commands/transform-test/get.ts new file mode 100644 index 00000000..66e46c96 --- /dev/null +++ b/packages/cli/src/commands/transform-test/get.ts @@ -0,0 +1,26 @@ +import { TransformTest } from "@metabase/client/domain/transform-test"; + +import { renderItem } from "../../output/render"; +import { transformTestView } from "../../output/views/transform-test"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "get", description: "Get a transform test by id" }, + capabilities: { minVersion: 64 }, + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + id: { type: "positional", description: "Transform test id", required: true }, + }, + outputSchema: TransformTest, + examples: ["mb transform-test get 1", "mb transform-test get 1 --json"], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const client = await getClient(); + const transformTest = await client.transformTest.get(id); + renderItem(transformTest, transformTestView, ctx); + }, +}); diff --git a/packages/cli/src/commands/transform-test/index.ts b/packages/cli/src/commands/transform-test/index.ts new file mode 100644 index 00000000..757ae02b --- /dev/null +++ b/packages/cli/src/commands/transform-test/index.ts @@ -0,0 +1,14 @@ +import { defineCommandGroup } from "../group"; + +export default defineCommandGroup({ + name: "transform-test", + description: "Manage transform tests — fixtures and expectations run against temp tables", + subCommands: { + list: () => import("./list").then((mod) => mod.default), + get: () => import("./get").then((mod) => mod.default), + create: () => import("./create").then((mod) => mod.default), + update: () => import("./update").then((mod) => mod.default), + delete: () => import("./delete").then((mod) => mod.default), + run: () => import("./run").then((mod) => mod.default), + }, +}); diff --git a/packages/cli/src/commands/transform-test/list.ts b/packages/cli/src/commands/transform-test/list.ts new file mode 100644 index 00000000..059e3c59 --- /dev/null +++ b/packages/cli/src/commands/transform-test/list.ts @@ -0,0 +1,31 @@ +import { TransformTestCompact } from "@metabase/client/domain/transform-test"; + +import { renderList } from "../../output/render"; +import { listEnvelopeSchema } from "../../output/types"; +import { windowList } from "../../output/window"; +import { transformTestView } from "../../output/views/transform-test"; +import { connectionFlags, listFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export const TransformTestListEnvelope = listEnvelopeSchema(TransformTestCompact); + +export default defineMetabaseCommand({ + meta: { name: "list", description: "List transform tests" }, + capabilities: { minVersion: 64 }, + args: { + ...outputFlags, + ...listFlags, + ...profileFlag, + ...connectionFlags, + transform: { type: "string", description: "Only the tests of this transform id" }, + }, + outputSchema: TransformTestListEnvelope, + examples: ["mb transform-test list", "mb transform-test list --transform 1 --json"], + async run({ args, ctx, getClient }) { + const client = await getClient(); + const transformId = args.transform === undefined ? undefined : parseId(args.transform); + const { data } = await client.transformTest.list({ "transform-id": transformId }); + renderList(windowList(data, ctx.range), transformTestView, ctx); + }, +}); diff --git a/packages/cli/src/commands/transform-test/run.ts b/packages/cli/src/commands/transform-test/run.ts new file mode 100644 index 00000000..f82a3ba6 --- /dev/null +++ b/packages/cli/src/commands/transform-test/run.ts @@ -0,0 +1,61 @@ +import type { + TransformTestExpectationResult, + TransformTestRunResult, + TransformTestStatus, +} from "@metabase/client/domain/transform-test"; +import { TransformTestRunResult as TransformTestRunResultSchema } from "@metabase/client/domain/transform-test"; + +import { renderSummary } from "../../output/render"; +import type { ResourceView } from "../../output/view"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +const transformTestRunResultView: ResourceView = { + compactPick: TransformTestRunResultSchema, + tableColumns: [{ key: "status", label: "Status" }], +}; + +function namesOf( + expectations: TransformTestExpectationResult[], + status: TransformTestStatus, +): string[] { + return expectations.filter((expectation) => expectation.status === status).map((e) => e.name); +} + +function summaryLine(id: number, result: TransformTestRunResult): string { + if (result.status === "passed") { + return `Transform test ${id} passed ${result.expectations.length} expectation(s).`; + } + const failed = namesOf(result.expectations, "failed"); + const errored = namesOf(result.expectations, "error"); + const parts = [ + failed.length > 0 ? `failed: ${failed.join(", ")}` : null, + errored.length > 0 ? `could not run: ${errored.join(", ")}` : null, + ].filter((part) => part !== null); + return `Transform test ${id} ${result.status} — ${parts.join("; ")}.`; +} + +export default defineMetabaseCommand({ + meta: { name: "run", description: "Run a transform test by id" }, + details: + "Runs the transform against temp tables built from the test's inputs, checks every expectation against its output, and drops the temp tables. Nothing reads or writes a real table. A failing expectation exits non-zero; --json reports what each expectation found, including the rows a comparison disagreed on.", + capabilities: { minVersion: 64 }, + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + id: { type: "positional", description: "Transform test id", required: true }, + }, + outputSchema: TransformTestRunResultSchema, + examples: ["mb transform-test run 1", "mb transform-test run 1 --json"], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const client = await getClient(); + const result = await client.transformTest.run(id); + renderSummary(result, transformTestRunResultView, summaryLine(id, result), ctx); + if (result.status !== "passed") { + throw new Error(`transform test ${id} ${result.status}`); + } + }, +}); diff --git a/packages/cli/src/commands/transform-test/update.ts b/packages/cli/src/commands/transform-test/update.ts new file mode 100644 index 00000000..9116b969 --- /dev/null +++ b/packages/cli/src/commands/transform-test/update.ts @@ -0,0 +1,36 @@ +import { TransformTest, TransformTestUpdateInput } from "@metabase/client/domain/transform-test"; + +import { renderSummary } from "../../output/render"; +import { transformTestView } from "../../output/views/transform-test"; +import { readBody } from "../../runtime/body"; +import { bodyInputFlags } from "../body-flags"; +import { connectionFlags, outputFlags, profileFlag } from "../flags"; +import { parseId } from "../parse-id"; +import { defineMetabaseCommand } from "../runtime"; + +export default defineMetabaseCommand({ + meta: { name: "update", description: "Update a transform test from JSON" }, + details: + "Only the fields the body carries are patched. `inputs` and `expectations` replace what is stored rather than merging into it.", + capabilities: { minVersion: 64 }, + args: { + ...outputFlags, + ...profileFlag, + ...connectionFlags, + ...bodyInputFlags, + id: { type: "positional", description: "Transform test id", required: true }, + }, + inputSchema: TransformTestUpdateInput, + outputSchema: TransformTest, + examples: [ + "cat patch.json | mb transform-test update 1", + "mb transform-test update 1 --file patch.json", + ], + async run({ args, ctx, getClient }) { + const id = parseId(args.id); + const body = await readBody({ flag: args.body, file: args.file }, TransformTestUpdateInput); + const client = await getClient(); + const updated = await client.transformTest.update(id, body); + renderSummary(updated, transformTestView, `Updated transform test ${updated.id}.`, ctx); + }, +}); diff --git a/packages/cli/src/main.ts b/packages/cli/src/main.ts index 2340c5e3..8636fb12 100644 --- a/packages/cli/src/main.ts +++ b/packages/cli/src/main.ts @@ -31,6 +31,7 @@ const main: CommandDef = defineCommand({ transform: () => import("./commands/transform").then((mod) => mod.default), "transform-job": () => import("./commands/transform-job").then((mod) => mod.default), "transform-tag": () => import("./commands/transform-tag").then((mod) => mod.default), + "transform-test": () => import("./commands/transform-test").then((mod) => mod.default), setting: () => import("./commands/setting").then((mod) => mod.default), search: () => import("./commands/search").then((mod) => mod.default), "git-sync": () => import("./commands/git-sync").then((mod) => mod.default), diff --git a/packages/cli/src/output/views/transform-test.ts b/packages/cli/src/output/views/transform-test.ts new file mode 100644 index 00000000..a194f851 --- /dev/null +++ b/packages/cli/src/output/views/transform-test.ts @@ -0,0 +1,13 @@ +import { type TransformTest, TransformTestCompact } from "@metabase/client/domain/transform-test"; + +import type { ResourceView } from "../view"; + +export const transformTestView: ResourceView = { + compactPick: TransformTestCompact, + tableColumns: [ + { key: "id", label: "ID" }, + { key: "transform_id", label: "Transform" }, + { key: "name", label: "Name" }, + { key: "description", label: "Description" }, + ], +}; diff --git a/packages/cli/src/runtime/command-help.test.ts b/packages/cli/src/runtime/command-help.test.ts index 7319e381..1fae8820 100644 --- a/packages/cli/src/runtime/command-help.test.ts +++ b/packages/cli/src/runtime/command-help.test.ts @@ -417,6 +417,12 @@ const ALL_COMMANDS = [ "transform-tag create", "transform-tag update", "transform-tag delete", + "transform-test list", + "transform-test get", + "transform-test create", + "transform-test update", + "transform-test delete", + "transform-test run", "setting list", "setting get", "setting set", @@ -652,6 +658,23 @@ describe("command tree contract", () => { }); }); + it("gates every transform-test command at the version that first serves the endpoint", async () => { + const entries = await allEntries(); + const capabilities = Object.fromEntries( + entries + .filter((entry) => entry.command.startsWith("transform-test ")) + .map((entry) => [entry.command, entry.capabilities]), + ); + expect(capabilities).toEqual({ + "transform-test list": TRANSFORM_TEST_CAPABILITIES, + "transform-test get": TRANSFORM_TEST_CAPABILITIES, + "transform-test create": TRANSFORM_TEST_CAPABILITIES, + "transform-test update": TRANSFORM_TEST_CAPABILITIES, + "transform-test delete": TRANSFORM_TEST_CAPABILITIES, + "transform-test run": TRANSFORM_TEST_CAPABILITIES, + }); + }); + it("advertises a --limit default on search alone, so the shared flag description stays true", async () => { const entries = await allEntries(); const withDefault = Object.fromEntries( From 544cd92bc3de4cb8f516ff4682316e675cdf18be Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Tue, 15 Sep 2026 23:39:31 -0400 Subject: [PATCH 03/24] Close the transform-test bodies and document the shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server closes `::transform-test.create` and `::transform-test.update`, so a body cloned from `get` earned an opaque 400. Closing them here names the offending key instead; a stored test needs `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` stripped before it can be sent back, which the command help, the README and the transform skill now say. `get` projects through the compact view, so `--full` is what shows `inputs` and `expectations` — the examples and docs advertise it. An `equals` expectation needs `format: "sql"` or `format: "rows"`, which the docs omitted, leaving a body written from them to fail on a three-branch union error. Co-Authored-By: Claude Opus 5 --- README.md | 17 +++- packages/cli/skill-data/core/SKILL.md | 3 + packages/cli/skill-data/transform/SKILL.md | 51 +++++++++++- .../cli/src/commands/transform-test/create.ts | 2 +- .../cli/src/commands/transform-test/get.ts | 4 +- .../cli/src/commands/transform-test/update.ts | 2 +- .../client/src/domain/transform-test.test.ts | 80 +++++++++++++++++++ packages/client/src/domain/transform-test.ts | 10 ++- 8 files changed, 161 insertions(+), 8 deletions(-) create mode 100644 packages/client/src/domain/transform-test.test.ts diff --git a/README.md b/README.md index 985d71e3..da2f4651 100644 --- a/README.md +++ b/README.md @@ -385,7 +385,17 @@ mb transform-tag delete 5 --yes CRUD and run on `/api/transform-test`. Requires Metabase v64 or newer. A transform test pins a transform's behaviour without touching real data: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. -An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` (each with a `database_type` the warehouse accepts as a `CAST` target) and `rows`. An expectation is either `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with the rows the output must hold exactly. +An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` (each with a `database_type` the warehouse accepts as a `CAST` target) and `rows`. + +An expectation is either `type: "empty"` with the `sql` that must return no rows, or `type: "equals"`, which takes the same `format` split as an input — `format: "rows"` with the `columns` and `rows` the output must hold exactly, or `format: "sql"` with a query returning them. An `equals` without a `format` is refused. + +Create and update bodies are closed, so a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent back: + +```sh +mb transform-test get 1 --full --json \ + | jq 'del(.id, .entity_id, .creator_id, .created_at, .updated_at)' \ + | mb transform-test update 1 +``` ### `mb transform-test list` @@ -400,8 +410,11 @@ mb transform-test list --transform 1 ### `mb transform-test get ` +The compact form carries the id, transform, name and description; `--full` adds the `inputs` and `expectations` themselves. + ```sh -mb transform-test get 1 --json +mb transform-test get 1 +mb transform-test get 1 --full --json ``` ### `mb transform-test create` diff --git a/packages/cli/skill-data/core/SKILL.md b/packages/cli/skill-data/core/SKILL.md index b7fef892..3c03f89a 100644 --- a/packages/cli/skill-data/core/SKILL.md +++ b/packages/cli/skill-data/core/SKILL.md @@ -149,6 +149,9 @@ Routine verb shapes (list / get / create / update), every flag, and output schem - **transform.** Iterate with `transform update `, never `delete` + `create` (keeps the row, `entity_id`, materialized table, and YAML filename — avoids `_2` suffixes and noisy git history). `transform run` needs `--wait` (or `--sync`, which also waits for the output table to register and returns `target_table_id`) or you get only `{run_id, final:null}`. (→ `transform`.) + +- **transform-test.** `transform-test run ` checks a transform against fixtures in temp tables — no real table is read — and exits non-zero unless it passes. (→ `transform`.) + - **setup is one-shot.** `mb setup` walks `/api/setup` for a **fresh** instance only — errors against an already-configured one. Mostly for bootstrapping local / e2e instances. - **eid** translates a string entity id → numeric id: `mb eid --model --json`. Entity ids are NanoIDs that can start with `-`, which the positional form misreads as a flag (shell quotes don't help) — for those, use `--body '{"entity_ids":{"card":["-…"]}}'` (the id is a JSON string value, immune to flag parsing). diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index cb9e5bba..01270112 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -1,6 +1,6 @@ --- name: transform -description: Author and run Metabase transforms via `mb` — body shape (native SQL or structured MBQL), create + run-with-wait, run inspection, dependencies, cancel, the `update`-vs-recreate iteration rule, the writable-keys-only PATCH contract, plus transform tags and tag-driven transform-job schedules. Load when the user touches transforms — "create a transform", "run a transform", "fix a failing transform", "list transform runs", "cancel a running transform", "manage transform tags", "run a transform job", or anything `mb transform …` / `mb transform-job …` / `mb transform-tag …`. +description: Author and run Metabase transforms via `mb` — body shape (native SQL or structured MBQL), create + run-with-wait, run inspection, dependencies, cancel, the `update`-vs-recreate iteration rule, the writable-keys-only PATCH contract, transform tests (fixtures and expectations run against temp tables), plus transform tags and tag-driven transform-job schedules. Load when the user touches transforms — "create a transform", "run a transform", "fix a failing transform", "test a transform", "list transform runs", "cancel a running transform", "manage transform tags", "run a transform job", or anything `mb transform …` / `mb transform-job …` / `mb transform-tag …` / `mb transform-test …`. allowed-tools: Read, Write, Edit, Bash, AskUserQuestion requires: [transforms] --- @@ -173,6 +173,55 @@ mb transform run "$ID" --wait --profile --json # → succeeded If you really must `create + delete` instead, do the `delete` **before** the first `git-sync export` so the failed entity never lands in git history — an export of a soft-failed state is noise that needs a follow-up cleanup commit. See `git-sync`, "Read state before mutating", for the ordering rule. +## Transform tests (v64+) + +A transform test replaces every table the transform reads with a fixture, runs it into a temp table, and checks that output. No real table is read or written. + +```bash +mb transform-test list --transform --profile --json # --transform is optional +mb transform-test get --full --profile --json # --full for inputs/expectations +mb transform-test create --file ./.scratch/test.json --profile --json +mb transform-test update --file ./.scratch/patch.json --profile --json +mb transform-test delete --yes --profile +mb transform-test run --profile --json # exits non-zero unless it passes +``` + +**`inputs`** — one per table the transform reads, each naming a `table` plus either `format: "sql"` with `sql`, or `format: "rows"` with `columns` (each a `name` and a `database_type` the warehouse takes as a `CAST` target) and `rows`. + +**`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"`, which needs the same `format` split as an input (`"rows"` with `columns`/`rows`, or `"sql"` with a query). An `equals` without a `format` is refused. + +```json +{ + "transform_id": 1, + "name": "adults only", + "inputs": [ + { + "table": { "schema": "public", "name": "people" }, + "format": "rows", + "columns": [ + { "name": "id", "database_type": "INTEGER" }, + { "name": "age", "database_type": "INTEGER" } + ], + "rows": [ + { "id": 1, "age": 30 }, + { "id": 2, "age": 12 } + ] + } + ], + "expectations": [ + { + "type": "equals", + "name": "keeps only the adult", + "format": "rows", + "columns": [{ "name": "id", "database_type": "INTEGER" }], + "rows": [{ "id": 1 }] + } + ] +} +``` + +Create and update bodies are closed — strip `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` from a `get --full` body before sending it back. + ## Drop the materialized table (keep the transform) ```bash diff --git a/packages/cli/src/commands/transform-test/create.ts b/packages/cli/src/commands/transform-test/create.ts index 50487782..d1a4bfaf 100644 --- a/packages/cli/src/commands/transform-test/create.ts +++ b/packages/cli/src/commands/transform-test/create.ts @@ -10,7 +10,7 @@ import { defineMetabaseCommand } from "../runtime"; export default defineMetabaseCommand({ meta: { name: "create", description: "Create a transform test from JSON" }, details: - 'The JSON body needs a `transform_id`, a `name`, the `inputs` standing in for every table the transform reads, and the `expectations` checked against its output. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` and `rows`. An expectation is either `type: "empty"` with the `sql` that must return nothing, or `type: "equals"` with the rows the output must hold.', + 'The JSON body needs a `transform_id`, a `name`, the `inputs` standing in for every table the transform reads, and the `expectations` checked against its output. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` and `rows`. An expectation is either `type: "empty"` with the `sql` that must return nothing, or `type: "equals"`, which takes the same `format` split as an input: `format: "rows"` with the `columns` and `rows` the output must hold, or `format: "sql"` with a query returning them. The body is closed: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.', capabilities: { minVersion: 64 }, args: { ...outputFlags, diff --git a/packages/cli/src/commands/transform-test/get.ts b/packages/cli/src/commands/transform-test/get.ts index 66e46c96..b3872fca 100644 --- a/packages/cli/src/commands/transform-test/get.ts +++ b/packages/cli/src/commands/transform-test/get.ts @@ -8,6 +8,8 @@ import { defineMetabaseCommand } from "../runtime"; export default defineMetabaseCommand({ meta: { name: "get", description: "Get a transform test by id" }, + details: + "The compact form carries the id, transform, name and description. Pass --full for the `inputs` and `expectations` themselves, which is also the body to edit and send back to `update`.", capabilities: { minVersion: 64 }, args: { ...outputFlags, @@ -16,7 +18,7 @@ export default defineMetabaseCommand({ id: { type: "positional", description: "Transform test id", required: true }, }, outputSchema: TransformTest, - examples: ["mb transform-test get 1", "mb transform-test get 1 --json"], + examples: ["mb transform-test get 1", "mb transform-test get 1 --full --json"], async run({ args, ctx, getClient }) { const id = parseId(args.id); const client = await getClient(); diff --git a/packages/cli/src/commands/transform-test/update.ts b/packages/cli/src/commands/transform-test/update.ts index 9116b969..7b5699dd 100644 --- a/packages/cli/src/commands/transform-test/update.ts +++ b/packages/cli/src/commands/transform-test/update.ts @@ -11,7 +11,7 @@ import { defineMetabaseCommand } from "../runtime"; export default defineMetabaseCommand({ meta: { name: "update", description: "Update a transform test from JSON" }, details: - "Only the fields the body carries are patched. `inputs` and `expectations` replace what is stored rather than merging into it.", + "Only the fields the body carries are patched. `inputs` and `expectations` replace what is stored rather than merging into it. The body is closed: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.", capabilities: { minVersion: 64 }, args: { ...outputFlags, diff --git a/packages/client/src/domain/transform-test.test.ts b/packages/client/src/domain/transform-test.test.ts new file mode 100644 index 00000000..025a811e --- /dev/null +++ b/packages/client/src/domain/transform-test.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "vitest"; + +import { + TransformTest, + TransformTestCreateInput, + TransformTestUpdateInput, +} from "./transform-test"; + +const STORED = { + id: 3, + entity_id: "AbCdEfGhIjKlMnOpQrStU", + transform_id: 7, + creator_id: 1, + name: "people summary holds one row", + description: null, + inputs: [{ table: { schema: "public", name: "people" }, format: "sql", sql: "SELECT 1 AS id" }], + expectations: [{ type: "empty", name: "no null ids", sql: "SELECT * FROM out" }], + created_at: "2026-09-15T12:00:00Z", + updated_at: "2026-09-15T12:00:00Z", +}; + +const READ_ONLY_KEYS = ["id", "entity_id", "creator_id", "created_at", "updated_at"] as const; + +function withoutReadOnlyKeys(stored: TransformTest): Record { + const body: Record = { ...stored }; + for (const key of READ_ONLY_KEYS) { + delete body[key]; + } + return body; +} + +describe("TransformTestCreateInput", () => { + it("names the read-only keys a body cloned from a stored test still carries", () => { + const result = TransformTestCreateInput.safeParse(TransformTest.parse(STORED)); + + expect(result.success).toBe(false); + expect(result.error?.issues).toEqual([ + { + code: "unrecognized_keys", + keys: [...READ_ONLY_KEYS], + path: [], + message: 'Unrecognized keys: "id", "entity_id", "creator_id", "created_at", "updated_at"', + }, + ]); + }); + + it("accepts that same body once those keys are stripped", () => { + const body = withoutReadOnlyKeys(TransformTest.parse(STORED)); + + expect(TransformTestCreateInput.parse(body)).toEqual({ + transform_id: 7, + name: "people summary holds one row", + description: null, + inputs: [ + { table: { schema: "public", name: "people" }, format: "sql", sql: "SELECT 1 AS id" }, + ], + expectations: [{ type: "empty", name: "no null ids", sql: "SELECT * FROM out" }], + }); + }); +}); + +describe("TransformTestUpdateInput", () => { + it("names a read-only key rather than sending it for the server to refuse", () => { + const result = TransformTestUpdateInput.safeParse({ id: 3, name: "renamed" }); + + expect(result.success).toBe(false); + expect(result.error?.issues).toEqual([ + { + code: "unrecognized_keys", + keys: ["id"], + path: [], + message: 'Unrecognized key: "id"', + }, + ]); + }); + + it("patches a single field", () => { + expect(TransformTestUpdateInput.parse({ name: "renamed" })).toEqual({ name: "renamed" }); + }); +}); diff --git a/packages/client/src/domain/transform-test.ts b/packages/client/src/domain/transform-test.ts index 7f095ead..4ae2d5d0 100644 --- a/packages/client/src/domain/transform-test.ts +++ b/packages/client/src/domain/transform-test.ts @@ -106,6 +106,11 @@ export const TransformTestCompact = TransformTest.pick({ }).strip(); export type TransformTestCompact = z.infer; +/** + * The server closes this map, so an unrecognized key is a 400 there. Refusing it here names the key + * instead: a body cloned from `get` has to shed `id`, `entity_id`, `creator_id`, `created_at` and + * `updated_at` before it can be sent back. + */ export const TransformTestCreateInput = z .object({ transform_id: z.number().int().positive(), @@ -114,9 +119,10 @@ export const TransformTestCreateInput = z inputs: z.array(TransformTestInput), expectations: z.array(TransformTestExpectation), }) - .loose(); + .strict(); export type TransformTestCreateInput = z.infer; +/** Closed for the same reason as [[TransformTestCreateInput]]; every field is optional. */ export const TransformTestUpdateInput = z .object({ transform_id: z.number().int().positive().optional(), @@ -125,7 +131,7 @@ export const TransformTestUpdateInput = z inputs: z.array(TransformTestInput).optional(), expectations: z.array(TransformTestExpectation).optional(), }) - .loose(); + .strict(); export type TransformTestUpdateInput = z.infer; export const TransformTestResultColumn = z From 8dfbce17aa6ea87d0b00757db35728e29fcaed63 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Tue, 15 Sep 2026 23:43:24 -0400 Subject: [PATCH 04/24] Show both expectation types in the transform-test example Co-Authored-By: Claude Opus 5 --- packages/cli/skill-data/transform/SKILL.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index 01270112..27bced2a 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -215,6 +215,11 @@ mb transform-test run --profile --json # exits non-zero u "format": "rows", "columns": [{ "name": "id", "database_type": "INTEGER" }], "rows": [{ "id": 1 }] + }, + { + "type": "empty", + "name": "no minor survived the filter", + "sql": "SELECT * FROM public.adults WHERE age < 18" } ] } From 1f2fc2910e532a4e5a483593d163813ef014e141 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Tue, 15 Sep 2026 23:44:58 -0400 Subject: [PATCH 05/24] Give the two example expectations different jobs `equals` pins the exact rows; `empty` asserts an invariant the row data alone would not show. Co-Authored-By: Claude Opus 5 --- packages/cli/skill-data/transform/SKILL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index 27bced2a..8fc55383 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -211,15 +211,15 @@ mb transform-test run --profile --json # exits non-zero u "expectations": [ { "type": "equals", - "name": "keeps only the adult", + "name": "exactly one row, id 1", "format": "rows", "columns": [{ "name": "id", "database_type": "INTEGER" }], "rows": [{ "id": 1 }] }, { "type": "empty", - "name": "no minor survived the filter", - "sql": "SELECT * FROM public.adults WHERE age < 18" + "name": "no null ids", + "sql": "SELECT * FROM public.adults WHERE id IS NULL" } ] } From 532981d6aa7026c9273453c1bfffbf112cb23cd8 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Thu, 17 Sep 2026 14:48:44 -0600 Subject: [PATCH 06/24] Rename database_type to cast_type --- packages/cli/skill-data/transform/SKILL.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index 8fc55383..30489b70 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -186,7 +186,9 @@ mb transform-test delete --yes --profile mb transform-test run --profile --json # exits non-zero unless it passes ``` -**`inputs`** — one per table the transform reads, each naming a `table` plus either `format: "sql"` with `sql`, or `format: "rows"` with `columns` (each a `name` and a `database_type` the warehouse takes as a `CAST` target) and `rows`. +**`inputs`** — one per table the transform reads, each naming a `table` plus either `format: "sql"` with `sql`, or `format: "rows"` with `columns` (each a `name` and a `cast_type`) and `rows`. + +**`cast_type` is a `CAST` target, not a column type**, and the two vocabularies differ per warehouse: MySQL casts to `SIGNED` and reports `INTEGER`; ClickHouse takes `Nullable(Int32)` for a column that is `Int64`. So a body is warehouse-specific — don't copy a `database_type` out of a run result into a `cast_type`, and don't expect one body to run everywhere. **`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"`, which needs the same `format` split as an input (`"rows"` with `columns`/`rows`, or `"sql"` with a query). An `equals` without a `format` is refused. @@ -199,8 +201,8 @@ mb transform-test run --profile --json # exits non-zero u "table": { "schema": "public", "name": "people" }, "format": "rows", "columns": [ - { "name": "id", "database_type": "INTEGER" }, - { "name": "age", "database_type": "INTEGER" } + { "name": "id", "cast_type": "INTEGER" }, + { "name": "age", "cast_type": "INTEGER" } ], "rows": [ { "id": 1, "age": 30 }, @@ -213,7 +215,7 @@ mb transform-test run --profile --json # exits non-zero u "type": "equals", "name": "exactly one row, id 1", "format": "rows", - "columns": [{ "name": "id", "database_type": "INTEGER" }], + "columns": [{ "name": "id", "cast_type": "INTEGER" }], "rows": [{ "id": 1 }] }, { From ded9ef1e3223dfed841ccb341e95f0962a9ed315 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Thu, 17 Sep 2026 14:57:51 -0600 Subject: [PATCH 07/24] Check in an adapted form of DWT skill --- packages/cli/skill-data/core/SKILL.md | 17 +- .../skill-data/transform-test-plan/SKILL.md | 189 +++++++++++++ .../references/checklist.md | 81 ++++++ .../transform-test-plan/references/checks.md | 259 ++++++++++++++++++ 4 files changed, 539 insertions(+), 7 deletions(-) create mode 100644 packages/cli/skill-data/transform-test-plan/SKILL.md create mode 100644 packages/cli/skill-data/transform-test-plan/references/checklist.md create mode 100644 packages/cli/skill-data/transform-test-plan/references/checks.md diff --git a/packages/cli/skill-data/core/SKILL.md b/packages/cli/skill-data/core/SKILL.md index 3c03f89a..3f105c97 100644 --- a/packages/cli/skill-data/core/SKILL.md +++ b/packages/cli/skill-data/core/SKILL.md @@ -6,7 +6,7 @@ allowed-tools: Read, Write, Edit, Bash, AskUserQuestion # metabase-cli (core) -The official Metabase CLI (`mb`) drives a Metabase instance over its REST API: auth, list/get/create/update/delete on every resource, query and transform execution, content search, git-sync (representations ↔ instance), and entity-id translation. +`mb` drives a Metabase instance over its REST API: CRUD on every resource, query and transform execution, search, git-sync (representations ↔ instance), and entity-id translation. Top-level command groups (run `mb --help` to discover verbs): @@ -16,7 +16,7 @@ document | glossary | timeline | timeline-event | transform | transform-job | tr search | dependency | git-sync | setup | eid | uuid | upgrade | skills ``` -The conventions below — auth, flags, output, body input — hold across **every** group. Per-command flags and examples live in each command's `--help`; add `--json` for the machine-readable form with the output JSON Schema. A few flows have their own skills (see "Specialized skills"). When a card needs a query, prefer MBQL over native SQL (portable, pre-flight-validated — load `mbql`); fall back to native SQL when MBQL can't express it. +The conventions below — auth, flags, output, body input — hold across **every** group. When a card needs a query, prefer MBQL over native SQL (portable, pre-flight-validated — load `mbql`); fall back to native SQL when MBQL can't express it. ## Auth & profiles @@ -28,7 +28,7 @@ mb auth status --json # → {profile, present, url} for the d mb auth status --profile --json # health probe for one profile ``` -`auth list` is the primary enumeration path — one call returns every profile with sanitized URL, an `authenticated` flag, and a probe `status` (`ok` / `auth-failed` / `network-error` / `server-error` / `not-probed`). Use it before asking which profile to pick. +`auth list` is the primary enumeration path — one call returns every profile with sanitized URL, an `authenticated` flag, and a probe `status` (`ok` / `auth-failed` / `network-error` / `server-error` / `not-probed`). - One profile and intent doesn't disambiguate → use it. - Several → ask via `AskUserQuestion`, presenting the names from `auth list`. @@ -70,10 +70,10 @@ Every list verb takes `--limit ` (items this call returns) and `--offset ` - **`has_more` decides whether to keep going — never compare counts.** `total` is the server's count on endpoints that report one and `null` on those that don't, so arithmetic over it is not a termination condition. - **To continue, pass `next_offset` back as `--offset`.** When `has_more` is true `next_offset` is past the offset you sent, so the loop advances; when false the walk is over and `next_offset` is `null`. -- **`truncated` means the byte cap cut the output, not that the data ran out.** `has_more`/`next_offset` are recomputed to the cut point, so a capped list resumes like any window. Its `bytes` is what the untruncated answer would have measured. Narrow rows with `--fields` rather than raising `--max-bytes` — a bigger cap spends context on fields you didn't ask for, and the cap counts only what you asked for, so `--fields` buys rows directly. A capped list always returns at least one row; when not even one fits it exits 2 with "the smallest response this list can produce is N bytes, over the M-byte --max-bytes cap; …". +- **`truncated` means the byte cap cut the output, not that the data ran out.** `has_more`/`next_offset` are recomputed to the cut point, so a capped list resumes like any window. Its `bytes` is what the untruncated answer would have measured. Narrow rows with `--fields` rather than raising `--max-bytes` — the cap counts only what you asked for, so `--fields` buys rows directly. A capped list always returns at least one row; when not even one fits it exits 2 with "the smallest response this list can produce is N bytes, over the M-byte --max-bytes cap; …". - `limit` is echoed only when you passed `--limit` — except `mb search`, which defaults to `--limit 20` (an unbounded search is expensive server-side) and so always reports one. On nouns the server doesn't page, one large `--limit` with narrow `--fields` is a single request; many small `--offset` hops are one request each. -The whole walk, literally: +The whole walk: ```bash offset=0 @@ -105,7 +105,7 @@ mb create --file ./.scratch/body.json --profile --json Single-quoted `'EOF'` stops the shell interpolating `$vars` inside the JSON. -Write working files to **`./.scratch`** in the current directory (`mkdir -p ./.scratch` first), never `/tmp` — better permissions, they persist across the session, and the user can review them. +Write working files to **`./.scratch`** in the current directory (`mkdir -p ./.scratch` first), never `/tmp` — they persist across the session and the user can review them. ## Discovering commands and schemas @@ -124,7 +124,7 @@ mb transform --help --json | jq -r '.commands[].command' # verbs under "transfo ## Resource quirks worth memorizing -Routine verb shapes (list / get / create / update), every flag, and output schemas live in each command's `--help` (add `--json` for output schemas). Below is only what help does _not_ tell you: footguns and non-obvious behaviors. +Only what `--help` does _not_ tell you: footguns and non-obvious behaviors. - **db traversal: the hydration ladder.** Start with `database get --include tables` — the compact table map (id, name, schema, description per table), one call that fits most databases. Pick the relevant tables, then `table fields ` per table (bounded: fields are per-table). `--include tables.fields` is the full rollup — small databases only. Hundreds of tables? Traverse by schema (`database schemas ` → `database schema-tables `) or look tables up by name (`search --models table --db-id --limit 10`). `db sync-schema` / `rescan-values` queue async work and return `{status:"ok"}` at once; `sync-schema --wait` blocks until `initial_sync_status: complete`. - **table fields.** `table get` never returns fields on its own — pass `--include fields` (compact; the underlying query_metadata response also carries FK targets and dimensions, visible under `--full`) or use `table fields ` (list envelope). `table update` patches table-level metadata only; physical columns aren't editable. @@ -175,6 +175,9 @@ This file is enough for any single-command task. For anything deeper, load the r - **`transform`** — transform body JSON, create + run-with-wait, run inspection, tags, jobs. + +- **`transform-test-plan`** — deciding _what_ to test in a transform: the fixture cast, the expectations, the coverage matrix. (`transform` has the `mb transform-test` shapes.) + - **`document`** — Metabase documents (TipTap body, embedding cards). - **`git-sync`** — round-tripping content to/from a git remote. diff --git a/packages/cli/skill-data/transform-test-plan/SKILL.md b/packages/cli/skill-data/transform-test-plan/SKILL.md new file mode 100644 index 00000000..d404e7cd --- /dev/null +++ b/packages/cli/skill-data/transform-test-plan/SKILL.md @@ -0,0 +1,189 @@ +--- +name: transform-test-plan +description: Derive a comprehensive test plan for a transform — the fixture cast, the expectations, hand-derived expected rows, and a coverage matrix — from the model's declared design. Covers input partitioning (zero-case, multiplicity, dirty rows), grain / conservation / recomputation / conformance checks, and known-quirk conventions. Load when the user wants tests planned or written for transforms — "write tests for my transforms", "is my model right", "test plan for this pipeline", "add data quality checks" — whether the model is mid-build or already deployed. The `mb transform-test` command and body shapes live in the `transform` skill; this one decides what to test. +allowed-tools: Read, Write, Edit, Bash, AskUserQuestion +--- + +# Planning transform tests + +Turn a transform into a fixture cast that proves the logic on small known rows, expectations that +state the model's invariants, and a coverage matrix showing what's checked and what's deliberately +not. Mechanics — the `inputs`/`expectations` body shape and every `mb transform-test` verb — live in +the `transform` skill (`mb skills get transform`); load it before authoring, and never restate it +here. + +Every check derives from what the model **declares** — detected from its SQL, confirmed with its +owner — never from conformance to a modeling doctrine. One plan serves two moments: while the model +is **built**, checks pin each design decision; once **deployed**, the same SQL screens production +tables for anomalies. + +## Operating rules + +- **Detect, then derive.** Classify what the transform is and which conventions it uses (the + checklist); derive checks only from that. Star, one-big-table, partial denormalization — all + fine; never flag a style. +- **Judgment calls go through the checklist.** The session's autonomy setting governs which answers + you supply yourself and which you bring to the user — it never makes the checklist a formality. + Every answer you supply yourself is recorded in the plan as a stated assumption, paired with the + named expectation that enforces it — reversing the decision then breaks a test, not a paragraph. + And regardless of setting, when genuinely unsure, ask — a wrong-but-confident grain poisons every + downstream check. +- **Expected rows are derived by hand** from the fixture story and business meaning — never captured + from the transform's output, which asserts only that the transform equals itself. +- **Batteries stay off until declared structure switches them on.** No snapshot-density checks + without a snapshot, no version-history checks without effective/end/current columns. An empty + section beats a speculative one. + +## The procedure + +1. **Profile the real inputs**: per table, row count; per column, min/max/distinct-count/null + incidence; orphan counts across declared links (`mb field summary`, `mb query`). Profiling feeds + domains, bounds, null partitions, and key candidates — and every fixture edge cites the real-data + condition that warrants it, with its count ("the warehouse has 67 ship-before-order rows"). +2. **Detect.** Read the transform's SQL (`mb transform get --full --json`) for: grain + candidates (`GROUP BY` keys, the joins' driving table), join types (orphan handling), correlated + aggregates (stored aggregates), `_` naming (copies), effective/end/current columns + (version history), `now()`/`current_date` (volatile columns). A transform whose grain you cannot + state in one phrase is itself a finding — raise it before writing any test. +3. **Confirm.** Walk [references/checklist.md](references/checklist.md). Three stages: classify the + model, per-table declarations, per-column declarations. Each question carries its detection hint; + answer autonomously where the hint resolves, ask where it doesn't. In build-along mode these are + design questions — treat an undecided answer as a decision to make together, not a blocker. +4. **Derive.** Route every output column through [references/checks.md](references/checks.md) — + declared property → expectation shape, fixture implication, expected-row convention. Read it in + full once per plan; it is the plan's content. +5. **Design the fixture cast**: one small cast per transform (≈5–10 rows per table), human-named + rows ("Alice Premium"), every row a named edge — zero-case entities for every outer join and + aggregation, ≥2-member groups for every grouping and join, one dirty row per screenable defect, + boundary dates. Document it as a table (row → attributes → purpose) in the plan. + + Write each input as `format: "rows"` — the columns and their values _are_ the cast, and the story + stays readable in the body. Reach for `format: "sql"` only when the cast is genuinely easier as a + query — a generated series, say. + + Every column carries a `cast_type`: the target its cells are cast to, which answers to the + warehouse's `CAST` grammar rather than naming one of its column types. MySQL casts to `SIGNED` + and reports the column as `INTEGER`; ClickHouse takes `Nullable(Int32)` where the column is + `Int64`. So a cast type is a per-warehouse choice — a cast written for one engine does not carry + to another, and the `database_type` a run reports is the output column's real type, never a + `cast_type` to paste back. + +6. **Hand-derive the expected rows**, arithmetic recorded in the plan (premium: 3 orders / 350.50 / + 3.0). Every fixture row's fate appears in some expected cell. Pin NULL-vs-0-vs-empty for every + zero-case row — that cell is the null policy's only enforcement. +7. **Author the test.** One test per transform, per coherent story: an `equals` pinning the output, + and `empty` expectations stating the invariants that survive a change to the cast. + + ```bash + mb transform-test create --file ./.scratch/orders-test.json --profile --json + mb transform-test run --profile --json + ``` + + Iterate with `mb transform-test update --file`, never delete + create — the same rule the + `transform` skill states for transforms, and for the same reasons: the row, its `entity_id` and + its YAML filename all survive. + + Tests live with the transform: they serialize and git-sync with it. Give each expectation a name that states the invariant, because the name is what a + failure leads with ("revenue never negative", not "check 3"), and open each `empty` expectation's + SQL with a `--` contract comment: the invariant, and the failure modes it catches ("catches both + dropped orders and join fan-out"). + + Where one invariant applies to several transforms, duplicate the SQL. Nothing is shared between + tests, so give each copy its own name and comment rather than one that only makes sense next to + its twin. + +8. **Emit the coverage matrix** in the plan: rows = output columns with the table's grain; columns = + check classes (grain, conservation, recomputation, conformance, domains, referential integrity, + temporal, screens); cells name the covering expectation or expected-row cell, or state + `gap: `. Scan shared-attribute columns across transforms for cross-table agreement + obligations. Empty cells are honest; silent gaps are not. +9. **Prove the tests have teeth.** Once per test: corrupt one expected cell, `run`, confirm + `cell-mismatches` names exactly that column; revert. Then perturb one input cell and confirm + exactly the declared output cells move. Every `empty` expectation passes against an empty output, + so a green test can still be vacuous. + +## Severity: error, or the tolerated oddity + +Every expectation is pass or fail, and one failure fails the run — `run` exits non-zero, so a test +is a CI gate. **error** = forbidden, and it becomes an `empty` expectation. There is no warn +severity, so **never author an expectation you expect to fire**: a permanently red test trains +everyone to ignore the result and holds the gate down for everyone else. + +A tolerated-but-surfaced oddity — one the owner lives with, like orphan rows or ship-before-order +dates — gets encoded the two ways that hold: **pin it in the `equals` rows** (an orphan passing +through with NULLs is a cell in the expected output, so reversing the tolerance breaks the test), +and **record it in the plan's known-quirks list** with its real-data count, so the tolerance stays a +conscious choice. + +Under budget pressure cut business-rule checks first, then cross-table structure checks; never +single-column screens (domains, ranges, nulls) — cheapest, and the last line. + +## When a check exposes a live bug + +Non-negotiable: **never soften the test to green** — expected values state correct behavior; matching +them to buggy output documents the bug as intended — and **surface the finding with its blast radius +at both scales**, fixture ("1530.24 of 1600.74 fixture dollars survive") and warehouse ("908 of +2,050 orders dropped"). What happens next follows the session's terms, not a fixed protocol: propose +and apply the fix now (when the user wants it or the autonomy setting covers it), or — when the fix +must wait — hold the correct expectation and record the red in the plan with the minimal fix body, +ready for `mb transform update --file`. A stored test has no red-by-design state, so a deferred +fix must be visible in the plan or the test reads as broken. Either way, once green the test stays +as the regression guard. + +## When the doctrine doesn't apply + +The vocabulary follows Kimball's dimensional modeling (grain, additivity, conformed attributes, +slowly changing dimensions) — precise, widely understood terms. Real models are Kimball-inspired at +most; no check may score adherence: + +- Full calendar date dimensions are rare. Never demand one; test date _semantics_ — ranges, + orderings, volatile derivations. +- Surrogate keys are doctrine, natural-key joins are practice. Test whichever key the model + declares; never flag natural-key joins. +- "No NULL FKs / no NULL attributes" is doctrine routinely dropped. Null policy is three independent + declarations — measures, attributes, FKs — each detected and confirmed, never presumed. +- One-big-table is legitimate: it still has a grain, its copies still need agreement checks, its + functional dependencies still hold. +- A transform-level `ORDER BY` has no testable effect — output tables carry no row order and the + `equals` comparison is a multiset. Flag it as probable dead weight (clustering hints aside); never + write an ordering expectation. + +## One transform at a time + +Each test covers one transform. When the transform under test reads another transform's output, +that target table is an input like any other: declare it and fake it. Derive those rows from the +base transform's own expected output, so the two tests tell one story, and note the coupling in both +plans — changing the base's expected rows means changing this test's input. + +## Worked example, condensed + +`orders` + `customers` → _enriched_orders_ (order grain; LEFT JOIN attaches `customer_name`, +`tier`). + +Cast, 7 orders: two tiers; Alice and Carol with 2 orders each (multiplicity); two never shipped +(zero-case for the shipping join); order 106 shipped before ordered (real oddity, count cited); +order 107's customer_id matches no customer (orphan). Two `rows` inputs, one per source table. + +Derived, per catalog: grain uniqueness on `order_id`; row and amount conservation from input to +output, as scalar subqueries over the target and the seeded input; `tier` domain ⊆ {standard, +premium}; orphan = keep-with-NULLs → an expected row pinning the NULL pass-through; ship-before-order +tolerated → the plan's known-quirks list with the warehouse count. The whole output pinned in one +`equals`, hand-computed, arithmetic in the plan. The inner-vs-LEFT-join bug this cast catches — +unshipped orders silently dropped from revenue — is what the zero-case rows exist for. + +## Don't + +- Don't author expectations before loading the `transform` skill — the body shape, the closed + create/update contract, and the verb flags live there. +- Don't capture expected rows from the transform's own output — hand-derive them or they assert + nothing. +- Don't emit checks for structure the model doesn't declare (snapshot density, version history, + bridge weights) — an inapplicable battery buries real findings. +- Don't let a fixture cast go all-clean — no zero-case, no orphan, no dirty row proves the happy + path and nothing else; the bugs live in the edges. +- Don't write an expectation you expect to fail. +- Don't name a table in an `empty` expectation that is neither the transform's target nor a declared + input. Only those two are rewritten to temp tables; anything else is left exactly as written and + reads the real table. +- Don't surface bare check-ids ("per C3…") to the user — name the check in plain words; the ids are + for your cross-referencing, not their reading. diff --git a/packages/cli/skill-data/transform-test-plan/references/checklist.md b/packages/cli/skill-data/transform-test-plan/references/checklist.md new file mode 100644 index 00000000..58b1b374 --- /dev/null +++ b/packages/cli/skill-data/transform-test-plan/references/checklist.md @@ -0,0 +1,81 @@ +# Confirmation checklist + +The judgment calls a plan depends on, in derivation order. Each question carries a **detect** hint — +how to answer it from the SQL, the metadata, or profiling. Answer autonomously where the hint +resolves cleanly; ask where it doesn't. Every answer you supply yourself goes into the emitted plan +as a stated assumption, phrased so the owner can falsify it at a glance ("Assuming `discount` can +never be negative — correct?"). + +In build-along mode these are design questions: the owner is deciding, not recalling. An undecided +answer is a decision to make together — surface the options and the check each implies (the +`checks.md` entry named in parentheses). + +## Stage 1 — classify the model (per transform) + +1. **Grain: one row per what?** (G1, G2) + Detect: `GROUP BY` keys; otherwise the driving table of the joins. Confirm whenever detection and + any written description disagree — that disagreement is itself a finding. +2. **Fact-table type — and if periodic snapshot, dense or sparse?** (F2) + Detect: event-grain with a single event date → transaction; entity×period grain → periodic + snapshot (density is the one yes/no that flips its whole invariant set — always confirm); one row + per pipeline occurrence with milestone-date columns → accumulating snapshot. + +## Stage 2 — per-table declarations + +3. **Conservation ties: output rows/sums tie to which input, with which declared exclusions?** (A2) + Detect: `WHERE` clauses and join types name the exclusions (filtered rows, dropped duplicates). + Exclusions must be declared, or conservation appears to fail. +4. **Zero-group policy: does a group with no contributing rows appear (with zeros) or stay absent?** + (A5) + Detect: driving table of the rollup — grouping the detail can't produce empty groups; joining a + dimension first can. +5. **Orphan handling per join: drop / keep-with-NULLs / default row — and is an orphan tolerated or + forbidden?** (C3, I5) + Detect: join type. INNER = drop, LEFT = keep-with-NULLs, COALESCE to a sentinel = default row. + Tolerated-vs-forbidden is the owner's call — profiling says whether orphans exist today, not + whether they're acceptable. Forbidden becomes an `empty` expectation; tolerated is pinned in the + expected rows and recorded in the known-quirks list. +6. **Version history: effective/end/current columns anywhere? If not, confirm overwrite-everywhere.** + (T1) + Detect: column names. Absence means history is silently rewritten in rollups — state that + consequence when confirming, not just the mechanism. +7. **Does this transform read another transform's output?** (A5) + Detect: another transform's target table in this one's SQL. It becomes a declared input like any + other, faked from that transform's own expected output — confirm which rows, and record the + coupling in both plans. + +## Stage 3 — per-column declarations + +8. **Each measure: additive, semi-additive, or non-additive?** (A1) + Detect: sums and counts are additive; balances and levels are semi-additive; ratios, rates, and + unit prices are non-additive. Confirm the ambiguous ones (a "score"? a "quantity on hand"?). +9. **Legal bounds per measure?** (Q1) + Detect: profiling min/max suggests, business meaning decides — can `discount` be negative? can + `quantity` be zero? Derived bounds are free (a sum of non-negatives is non-negative). +10. **Each derived measure: the exact recomputation rule?** (A3) + Detect: the SELECT expression is the rule — but rounding, business-day adjustments, and NULL + handling are conventions to confirm, not read. +11. **Which columns are stored aggregates, over which detail and filter?** (A4) + Detect: correlated subqueries / joined-aggregate CTEs in the SQL. +12. **Which categorical columns have closed domains — enumerate; is NULL a member?** (C2) + Detect: profiling distinct values gives today's set; the owner confirms it's closed rather than + merely small so far. +13. **Which columns are denormalized copies, of which owning attribute?** (C1) + Detect: `_` naming; any column functionally dependent on a non-grain key. Every hit + must be classified: copy (C1), stored aggregate (A4), or smuggled coarser-grain fact — a + finding. +14. **Which many-to-one edges hold within one output table?** (C4) + Detect: hierarchy-shaped column pairs (product/category, zip/state). Applies where no owning + table is in scope; otherwise C1 covers it. +15. **Which date orderings are business-guaranteed vs. known-violated and tolerated (with real-data + counts)?** (T2) + Detect: profiling counts the violations that exist; the owner decides tolerated vs. bug. +16. **Null policy per column — measures, attributes, FKs separately.** (Q2) + Detect: the SQL shows what's produced (COALESCE, CASE); the owner confirms intent — "count of + nothing": 0 or NULL? empty date: NULL or sentinel? +17. **Which columns are volatile across runs?** (T3) + Detect: `now()` / `current_date` / run-metadata expressions in the SQL. A volatile column is left + out of the `equals` columns; its form can still be asserted with an `empty`. +18. **Per tolerated oddity: does it stay tolerated, and what is its current count?** (Q3) + Detect: can't — this is the conscious-choice question, and the plan's known-quirks list is where + it survives. diff --git a/packages/cli/skill-data/transform-test-plan/references/checks.md b/packages/cli/skill-data/transform-test-plan/references/checks.md new file mode 100644 index 00000000..f046cf5a --- /dev/null +++ b/packages/cli/skill-data/transform-test-plan/references/checks.md @@ -0,0 +1,259 @@ +# Check catalog + +Every check the plan can derive, grouped by theme. Per entry: **when it applies** (the declared +property that switches it on — blank means always), **assert** (the expectation shape), and +**fixtures** (what the input cast must contain for the check to have teeth). + +Expectation SQL names the transform's **target table** and its **declared input tables** under their +real names; both are rewritten to the run's temp tables, so per-row recomputation joins against +inputs are writable. Anything else you name is left exactly as written and reads the real table. + +Ids (G1, A2, …) are for cross-referencing within the plan documents only — never surface them to the +user bare. + +Throughout, `` stands for the transform's target table as written in the transform's own +definition, and `` for a declared input table. + +## Grain & keys + +**G1 — Declared grain.** Every output table states "one row per X"; the declaration anchors every +other check. + +- Applies: always. Elicit or detect (GROUP BY keys; the join's driving table). +- Assert: nothing directly — G1 is the plan's opening move. A table with no statable grain, or a + grain stated inconsistently between docs and SQL, is a finding before any SQL runs. +- Fixtures: the grain declaration decides the cast's row structure. + +**G2 — Grain-key uniqueness.** The grain key stays a key; doubles as the fan-out guard for every +enriching join. + +- Applies: always — one-big-table, rollups, and stars all have a grain. +- Assert (`empty`): `SELECT , COUNT(*) FROM GROUP BY HAVING +COUNT(*) > 1`. NULL grain values form their own bucket. +- Fixtures: a parent with ≥2 children on every join (I4) is what makes this check able to fail. + +## Additivity & reconciliation + +**A1 — Measure additivity classification.** Additive / semi-additive (balances) / non-additive +(ratios, rates, unit prices) decides which aggregations are valid tests. + +- Applies: every numeric measure; per-column declaration. +- Assert: routes the measure — additive → A2; semi-additive → sum across non-time slices only (a + test summing a balance over time is a bug in the plan); non-additive → A3 recomputation, never + reconciled by summing. +- Fixtures: none directly; one coverage-matrix axis (measure × valid aggregation set). + +**A2 — Conservation reconciliation.** Row counts and additive-measure sums tie from input to output; +catches dropped rows and join double-counting at once. + +- Applies: per declared tie (which input, which declared exclusions). +- Assert (`empty`): independent scalar subqueries compared with `IS DISTINCT FROM`, returning the + mismatched pair: + `SELECT (SELECT SUM(t.m) FROM t) AS output_sum, (SELECT SUM(s.m) FROM s) AS +input_sum WHERE (…) IS DISTINCT FROM (…)`. + Never reconcile via a fact-to-fact join — cardinality is uncontrollable and wrong results are + silent. +- Fixtures: amounts chosen so partial survival is visible (distinct values, odd cents). + +**A3 — Derived-measure recomputation.** Averages, lags, ratios, rounded presentations recompute from +their components per row. + +- Applies: every derived column, with its declared rule (rounding, business-day adjustment, NULL + handling). +- Assert (`empty`): `SELECT FROM t WHERE t. IS DISTINCT FROM `. +- Fixtures: component values whose derivation is non-trivial (a NULL in the AVG, a negative lag). + +**A4 — Stored aggregates on entity tables.** Lifetime/rollup stats carried on an entity-grain table +(`lifetime_orders`, `review_count`, `first_order_date`) equal recomputation from detail — per row +_and_ in total. + +- Applies: columns detected as correlated aggregates over a detail table. +- Assert (`empty`), two per column: per-row — `WHERE t. IS DISTINCT FROM (SELECT +COUNT(*)/SUM(…)/MIN(…) FROM d WHERE d. = t.)`; total — the A2 scalar-pair shape. + Totals alone cancel offsetting per-row errors; the per-row form is the one that catches them. +- Fixtures: an entity with several detail rows and an entity with none (I3/I4). + +**A5 — Rollup consistency.** A layered rollup always agrees with its base. + +- Applies: any transform reading another transform's output. The base is a declared input like any + other, faked with rows taken from the base transform's own expected output. +- Assert (`empty`): every rollup measure equals the corresponding aggregation over the base (A2 + shape); group-set equality both directions — `SELECT FROM EXCEPT SELECT DISTINCT + FROM ` and the reverse, filtered by the declared zero-group policy. +- Fixtures: a group with no contributing rows pins the zero-group policy. + +## Fact-table type + +**F2 — Fact-table type bundle.** Transaction / periodic snapshot / accumulating snapshot each carry +a distinct invariant set. + +- Applies: per fact-shaped output, always classified; **transaction** needs nothing beyond G2 + A2 + (sparsity is legitimate). +- Periodic snapshot, if declared _dense_: `COUNT(*) = |entities| × |periods|` (or the declared + subset); per-entity gap detection in the period series; inactive-period representation (zero vs + NULL) pinned in the expected rows. If sparse, density checks off — the one yes/no flips the whole + set. +- Accumulating snapshot: milestone dates monotone in pipeline order where set (`WHERE < +` per adjacent pair); unset-milestone default (NULL vs sentinel) pinned in the expected + rows; completion flags ∈ {0,1} and consistent with their date's set-ness; lags via A3. +- Fixtures (accumulating): occurrences at every completion stage — none, some, all milestones. + +## Conformance & domains + +**C1 — Denormalized-copy agreement.** Every copied attribute agrees with the owning table's value +for that key. + +- Applies: columns declared as copies (detect: `_` naming; any column functionally + dependent on a non-grain key). Each such column must be a declared copy (this check), a stored + aggregate (A4), or it's a smuggled coarser-grain fact — a finding: it double-counts under + summation. +- Assert (`empty`): `SELECT t., t., d. FROM t JOIN d ON +t. = d. WHERE t. IS DISTINCT FROM d.`. +- Scope: in a test, the copies are produced by the very join under test, so this join-form is near- + tautological — the C4 functional-dependency form plus expected-row cell pinning carries the test. + The join-form against an independently materialized owning table is the _drift_ check, which + belongs to whatever screens the deployed tables. +- Fixtures: copies with distinct values per entity so a crossed join is visible. + +**C2 — Closed-domain screen.** A categorical column's values stay inside the declared enumeration. + +- Applies: per column declared closed (detect from profiling; confirm the set and whether NULL is a + member). +- Assert (`empty`): `SELECT , FROM WHERE IS NOT NULL AND NOT IN +()`. +- Fixtures: every domain value represented where practical; one out-of-domain input row if the + source can produce one (I5). + +**C3 — Referential integrity / orphan policy.** Every FK resolves, or the declared orphan handling is +pinned. + +- Applies: per join, conditioned on the declared response — drop / keep-with-NULLs / default row; + tolerated or forbidden. +- Assert, forbidden (`empty`): `SELECT t. FROM t LEFT JOIN d ON t. = +d. WHERE t. IS NOT NULL AND d. IS NULL`. + Tolerated: not an expectation — pin the orphan's pass-through as a row in the `equals` (NULL in + the copied attributes), and record the tolerance with its warehouse count in the plan. + Default-row convention adds: distinct unknown keys must not collapse into one output row. +- Fixtures: one orphan row (I5). Without it the join direction is untested — an inner join silently + dropping unmatched rows is the classic bug this catches. + +**C4 — Many-to-one consistency.** Each declared many-to-one edge holds within the output (product → +one category; zip → one state). + +- Applies: per declared edge; where an owning table is in scope, C1 subsumes it — this is the + one-big-table variant with nothing to join against. +- Assert (`empty`): `SELECT t., COUNT(DISTINCT t.) FROM t GROUP BY t. +HAVING COUNT(DISTINCT t.) > 1`. +- Fixtures: a violating input row if the source can produce one (I5), pinning the transform's + behavior on dirty input. + +## Temporal & version history + +**T1 — Change handling per attribute.** How the model treats a changed source attribute decides the +temporal fixtures. + +- Applies: detect effective/end/current housekeeping columns. +- Present → version-history battery (`empty` each): per durable key exactly one current row; + `effective < end` per row; intervals contiguous and non-overlapping; current row's end = the + declared far-future default; fact rows join the version whose interval contains the fact date. +- Absent → confirm overwrite-everywhere as a stated assumption (history is silently rewritten in + rollups), and derive the propagation probe: change an attribute in an input, run, assert the + output regrouped. +- Fixtures: a before/after change pair; for version history, an entity with ≥2 versions and a fact + row dated inside each interval. + +**T2 — Date-pair ordering.** Business-guaranteed orderings asserted; known violations recorded. + +- Applies: per declared date pair (ordered ≤ shipped ≤ delivered; signup ≤ first order), each + classified guaranteed vs. tolerated-violated. +- Assert: guaranteed (`empty`) — `SELECT , , FROM WHERE < +`. Tolerated — pin the violating row's downstream arithmetic (a negative lag inside an + average) in the expected rows, and record the real-data count in the plan. +- Fixtures: one violating row for every tolerated ordering. + +**T3 — Volatile columns.** Values that change across runs can't be pinned. + +- Applies: columns derived from `now()`/`current_date` (age), run metadata (load timestamps, batch + ids). +- Assert: leave the column out of the `equals` `columns` list — an undeclared column never enters the + comparison. Separately assert its form where warranted (`empty`): `WHERE age NOT BETWEEN 0 AND +120`. The stable source column (`birth_date`) stays exact in the expected rows. +- Fixtures: none special; the split is the point — omit the volatile, pin the stable. + +## Screens & severity + +**Q1 — Range / sign screens.** Each measure's declared bounds hold. + +- Applies: per bounded measure; bounds from business meaning plus profiling (derived bounds are + free: a sum of non-negatives is non-negative). +- Assert (`empty`): `SELECT , FROM WHERE < OR > ` (one-sided + where only one bound exists). +- Fixtures: boundary values where the bound is business-set. + +**Q2 — Null policy, three ways.** Measures, descriptive attributes, and FKs carry independent null +policies; never presume one from another. + +- Applies: per column, asked separately — "count of nothing": 0 or NULL? empty date: NULL or + sentinel? FK: see C3. +- Assert: declared non-null columns get `WHERE IS NULL` (`empty`). Otherwise the policy is + enforced by the expected-row cell of a zero-case fixture row — NULL vs 0 vs empty is invisible + until a fixture forces the choice into a cell. +- Fixtures: the zero-case row (I3) is the enforcement mechanism. + +**Q3 — Naming & the tolerated oddity.** Every expectation declares its meaning. + +- Applies: always, every expectation. +- Convention: the expectation's `name` states the invariant in plain words, because a failure leads + with it. Each `empty` SQL opens with 1–3 comment lines naming the invariant and the failure modes + it catches. Budget cuts drop business-rule checks first, then cross-table structure checks, never + single-column screens. +- A forbidden condition is an `empty` expectation. A tolerated one is pinned in the expected rows + and recorded in the plan's known-quirks list with its warehouse count. Never author an expectation + designed to fire. + +## Input modeling (fixture-design rules) + +**I1 — Profiling-derived partitions.** Fixture edges and tolerated non-ties cite profiled reality +with counts; the plan's known-quirks section carries each tolerated oddity with its reason and +count. + +**I2 — Shared fixture cast.** One small human-named cast per transform, every row a named edge, +documented as a story table (row → attributes → purpose) in the plan. Where one transform's output +is another's input, derive the faked rows from the first's expected output so the two tests tell one +story. + +**I3 — Zero-case partitions.** For every outer join and aggregation relation: one entity with zero +matches. Its expected row pins the absence representation (Q2) and the join direction (C3) — the +all-clean cast is the single most common cause of vacuous tests. When the schema distinguishes +states the profiled data never exhibits (a literal zero in a source that only has NULLs and +positives), fixture the missing state — nothing else forces it onto an expected cell. + +**I4 — Multiplicity partitions.** Every aggregation gets a >1-member group and an exactly-1 group +(the 0 case is I3); every join gets a parent with ≥2 children. Expected values then differ from any +single row's, so copy-through bugs can't pass. + +**I5 — Dirty-input pinning.** Per screenable defect the source can carry (orphan FK, out-of-domain +value, duplicate natural key, hierarchy violation, ordering violation): one fixture row exhibiting +it, an expected row pinning the transform's response (drop / pass-through / default), and — where +the defect is forbidden — the matching `empty` expectation. Which duplicate survives a dedupe is +invisible until a conflicting-duplicate fixture pins it. + +**I6 — Mutation probes.** Run at plan-validation time, not on every run: corrupt one expected cell → +`cell-mismatches` must name that exact column; perturb one input cell → exactly the declared output +cells move. Both guard against vacuous tests: every `empty` expectation passes against an empty +output. + +## Plan-level artifacts + +**P1 — Coverage matrix.** Output columns (with the table's grain) × check classes; cells name the +covering expectation or expected-row cell, or state `gap: `; shared-attribute columns scanned +across transforms for agreement obligations (C1). + +**P2 — Hand-derived expected rows.** From the fixture story and business meaning, arithmetic recorded +in the plan; never captured from output; every fixture row's fate appears in some expected cell. + +**P3 — Live-bug handling.** A check failing against a deployed transform is a real finding: never +soften the test to green; quantify the damage at fixture and warehouse scale. Then fix now, or record +the red in the plan with the minimal fix body, per the session's terms. A stored test has no +red-by-design state, so a deferred fix must be visible in the plan or the test reads as broken. From 6151659393d3b923149875bd2a7ff2a34394ea68 Mon Sep 17 00:00:00 2001 From: Timothy Dean Date: Thu, 17 Sep 2026 15:45:39 -0600 Subject: [PATCH 08/24] Let the transform skill handle the impl details, keep the test skill to test philosophy and design --- .../skill-data/transform-test-plan/SKILL.md | 39 +++++-------------- packages/cli/skill-data/transform/SKILL.md | 2 + 2 files changed, 12 insertions(+), 29 deletions(-) diff --git a/packages/cli/skill-data/transform-test-plan/SKILL.md b/packages/cli/skill-data/transform-test-plan/SKILL.md index d404e7cd..16f02347 100644 --- a/packages/cli/skill-data/transform-test-plan/SKILL.md +++ b/packages/cli/skill-data/transform-test-plan/SKILL.md @@ -57,16 +57,9 @@ tables for anomalies. aggregation, ≥2-member groups for every grouping and join, one dirty row per screenable defect, boundary dates. Document it as a table (row → attributes → purpose) in the plan. - Write each input as `format: "rows"` — the columns and their values _are_ the cast, and the story - stays readable in the body. Reach for `format: "sql"` only when the cast is genuinely easier as a - query — a generated series, say. - - Every column carries a `cast_type`: the target its cells are cast to, which answers to the - warehouse's `CAST` grammar rather than naming one of its column types. MySQL casts to `SIGNED` - and reports the column as `INTEGER`; ClickHouse takes `Nullable(Int32)` where the column is - `Int64`. So a cast type is a per-warehouse choice — a cast written for one engine does not carry - to another, and the `database_type` a run reports is the output column's real type, never a - `cast_type` to paste back. + Express the cast as literal rows rather than as a query, so the story stays legible in the test + itself; a query is worth it only when the rows are mechanical to generate. A cast is written + against one warehouse and does not carry to another. 6. **Hand-derive the expected rows**, arithmetic recorded in the plan (premium: 3 orders / 350.50 / 3.0). Every fixture row's fate appears in some expected cell. Pin NULL-vs-0-vs-empty for every @@ -74,16 +67,7 @@ tables for anomalies. 7. **Author the test.** One test per transform, per coherent story: an `equals` pinning the output, and `empty` expectations stating the invariants that survive a change to the cast. - ```bash - mb transform-test create --file ./.scratch/orders-test.json --profile --json - mb transform-test run --profile --json - ``` - - Iterate with `mb transform-test update --file`, never delete + create — the same rule the - `transform` skill states for transforms, and for the same reasons: the row, its `entity_id` and - its YAML filename all survive. - - Tests live with the transform: they serialize and git-sync with it. Give each expectation a name that states the invariant, because the name is what a + Give each expectation a name that states the invariant, because the name is what a failure leads with ("revenue never negative", not "check 3"), and open each `empty` expectation's SQL with a `--` contract comment: the invariant, and the failure modes it catches ("catches both dropped orders and join fan-out"). @@ -104,10 +88,10 @@ tables for anomalies. ## Severity: error, or the tolerated oddity -Every expectation is pass or fail, and one failure fails the run — `run` exits non-zero, so a test -is a CI gate. **error** = forbidden, and it becomes an `empty` expectation. There is no warn -severity, so **never author an expectation you expect to fire**: a permanently red test trains -everyone to ignore the result and holds the gate down for everyone else. +Every expectation is pass or fail, and one failure fails the run. **error** = forbidden, and it +becomes an `empty` expectation. There is no warn severity, so **never author an expectation you +expect to fire**: a permanently red test trains everyone to ignore the result, and a red run gates +everyone else's work. A tolerated-but-surfaced oddity — one the owner lives with, like orphan rows or ship-before-order dates — gets encoded the two ways that hold: **pin it in the `equals` rows** (an orphan passing @@ -125,8 +109,8 @@ them to buggy output documents the bug as intended — and **surface the finding at both scales**, fixture ("1530.24 of 1600.74 fixture dollars survive") and warehouse ("908 of 2,050 orders dropped"). What happens next follows the session's terms, not a fixed protocol: propose and apply the fix now (when the user wants it or the autonomy setting covers it), or — when the fix -must wait — hold the correct expectation and record the red in the plan with the minimal fix body, -ready for `mb transform update --file`. A stored test has no red-by-design state, so a deferred +must wait — hold the correct expectation and record the red in the plan with the minimal fix body. +A stored test has no red-by-design state, so a deferred fix must be visible in the plan or the test reads as broken. Either way, once green the test stays as the regression guard. @@ -182,8 +166,5 @@ unshipped orders silently dropped from revenue — is what the zero-case rows ex - Don't let a fixture cast go all-clean — no zero-case, no orphan, no dirty row proves the happy path and nothing else; the bugs live in the edges. - Don't write an expectation you expect to fail. -- Don't name a table in an `empty` expectation that is neither the transform's target nor a declared - input. Only those two are rewritten to temp tables; anything else is left exactly as written and - reads the real table. - Don't surface bare check-ids ("per C3…") to the user — name the check in plain words; the ids are for your cross-referencing, not their reading. diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index 30489b70..1a075eae 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -192,6 +192,8 @@ mb transform-test run --profile --json # exits non-zero u **`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"`, which needs the same `format` split as an input (`"rows"` with `columns`/`rows`, or `"sql"` with a query). An `equals` without a `format` is refused. +**An `empty` query may only name the transform's target table and its declared input tables.** Those are rewritten to the run's temp tables; any other table you name is left exactly as written and reads the real one — the single way a test run can touch production data. + ```json { "transform_id": 1, From 3194dd20b543cbef5ecb4778ed430469fc907c97 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Thu, 17 Sep 2026 20:35:45 -0400 Subject: [PATCH 09/24] Transform tests ship in v65 Co-Authored-By: Claude Opus 5 --- README.md | 2 +- packages/cli/skill-data/transform/SKILL.md | 2 +- packages/cli/src/commands/transform-test/create.ts | 2 +- packages/cli/src/commands/transform-test/delete.ts | 2 +- packages/cli/src/commands/transform-test/get.ts | 2 +- packages/cli/src/commands/transform-test/list.ts | 2 +- packages/cli/src/commands/transform-test/run.ts | 2 +- packages/cli/src/commands/transform-test/update.ts | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index da2f4651..94d3ab0d 100644 --- a/README.md +++ b/README.md @@ -383,7 +383,7 @@ mb transform-tag delete 5 --yes ## Transform tests -CRUD and run on `/api/transform-test`. Requires Metabase v64 or newer. A transform test pins a transform's behaviour without touching real data: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. +CRUD and run on `/api/transform-test`. Requires Metabase v65 or newer. A transform test pins a transform's behaviour without touching real data: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` (each with a `database_type` the warehouse accepts as a `CAST` target) and `rows`. diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index 1a075eae..dda46e4c 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -173,7 +173,7 @@ mb transform run "$ID" --wait --profile --json # → succeeded If you really must `create + delete` instead, do the `delete` **before** the first `git-sync export` so the failed entity never lands in git history — an export of a soft-failed state is noise that needs a follow-up cleanup commit. See `git-sync`, "Read state before mutating", for the ordering rule. -## Transform tests (v64+) +## Transform tests (v65+) A transform test replaces every table the transform reads with a fixture, runs it into a temp table, and checks that output. No real table is read or written. diff --git a/packages/cli/src/commands/transform-test/create.ts b/packages/cli/src/commands/transform-test/create.ts index d1a4bfaf..c1007332 100644 --- a/packages/cli/src/commands/transform-test/create.ts +++ b/packages/cli/src/commands/transform-test/create.ts @@ -11,7 +11,7 @@ export default defineMetabaseCommand({ meta: { name: "create", description: "Create a transform test from JSON" }, details: 'The JSON body needs a `transform_id`, a `name`, the `inputs` standing in for every table the transform reads, and the `expectations` checked against its output. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` and `rows`. An expectation is either `type: "empty"` with the `sql` that must return nothing, or `type: "equals"`, which takes the same `format` split as an input: `format: "rows"` with the `columns` and `rows` the output must hold, or `format: "sql"` with a query returning them. The body is closed: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.', - capabilities: { minVersion: 64 }, + capabilities: { minVersion: 65 }, args: { ...outputFlags, ...profileFlag, diff --git a/packages/cli/src/commands/transform-test/delete.ts b/packages/cli/src/commands/transform-test/delete.ts index e057deeb..8edc6e50 100644 --- a/packages/cli/src/commands/transform-test/delete.ts +++ b/packages/cli/src/commands/transform-test/delete.ts @@ -5,7 +5,7 @@ import { defineMetabaseCommand } from "../runtime"; export default defineMetabaseCommand({ meta: { name: "delete", description: "Delete a transform test by id" }, - capabilities: { minVersion: 64 }, + capabilities: { minVersion: 65 }, args: { ...outputFlags, ...profileFlag, diff --git a/packages/cli/src/commands/transform-test/get.ts b/packages/cli/src/commands/transform-test/get.ts index b3872fca..d1e50b85 100644 --- a/packages/cli/src/commands/transform-test/get.ts +++ b/packages/cli/src/commands/transform-test/get.ts @@ -10,7 +10,7 @@ export default defineMetabaseCommand({ meta: { name: "get", description: "Get a transform test by id" }, details: "The compact form carries the id, transform, name and description. Pass --full for the `inputs` and `expectations` themselves, which is also the body to edit and send back to `update`.", - capabilities: { minVersion: 64 }, + capabilities: { minVersion: 65 }, args: { ...outputFlags, ...profileFlag, diff --git a/packages/cli/src/commands/transform-test/list.ts b/packages/cli/src/commands/transform-test/list.ts index 059e3c59..6ca266f6 100644 --- a/packages/cli/src/commands/transform-test/list.ts +++ b/packages/cli/src/commands/transform-test/list.ts @@ -12,7 +12,7 @@ export const TransformTestListEnvelope = listEnvelopeSchema(TransformTestCompact export default defineMetabaseCommand({ meta: { name: "list", description: "List transform tests" }, - capabilities: { minVersion: 64 }, + capabilities: { minVersion: 65 }, args: { ...outputFlags, ...listFlags, diff --git a/packages/cli/src/commands/transform-test/run.ts b/packages/cli/src/commands/transform-test/run.ts index f82a3ba6..104f1ad7 100644 --- a/packages/cli/src/commands/transform-test/run.ts +++ b/packages/cli/src/commands/transform-test/run.ts @@ -40,7 +40,7 @@ export default defineMetabaseCommand({ meta: { name: "run", description: "Run a transform test by id" }, details: "Runs the transform against temp tables built from the test's inputs, checks every expectation against its output, and drops the temp tables. Nothing reads or writes a real table. A failing expectation exits non-zero; --json reports what each expectation found, including the rows a comparison disagreed on.", - capabilities: { minVersion: 64 }, + capabilities: { minVersion: 65 }, args: { ...outputFlags, ...profileFlag, diff --git a/packages/cli/src/commands/transform-test/update.ts b/packages/cli/src/commands/transform-test/update.ts index 7b5699dd..1086face 100644 --- a/packages/cli/src/commands/transform-test/update.ts +++ b/packages/cli/src/commands/transform-test/update.ts @@ -12,7 +12,7 @@ export default defineMetabaseCommand({ meta: { name: "update", description: "Update a transform test from JSON" }, details: "Only the fields the body carries are patched. `inputs` and `expectations` replace what is stored rather than merging into it. The body is closed: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.", - capabilities: { minVersion: 64 }, + capabilities: { minVersion: 65 }, args: { ...outputFlags, ...profileFlag, From ac9f5e6de2da2468f975695436d8c35ed54afdce Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Tue, 22 Sep 2026 17:30:03 -0400 Subject: [PATCH 10/24] fix(cli): run transform-test commands on main's client Declare each command's client method through requires in place of a minimum version, gate the transform-test skill passages and the transform-test-plan skill on transformTests, and name the enterprise endpoint in the README. --- README.md | 2 +- packages/cli/skill-data/core/SKILL.md | 8 +++---- .../skill-data/transform-test-plan/SKILL.md | 1 + packages/cli/skill-data/transform/SKILL.md | 6 ++++- .../cli/src/commands/transform-test/create.ts | 2 +- .../cli/src/commands/transform-test/delete.ts | 2 +- .../cli/src/commands/transform-test/get.ts | 2 +- .../cli/src/commands/transform-test/list.ts | 2 +- .../cli/src/commands/transform-test/run.ts | 7 +++--- .../cli/src/commands/transform-test/update.ts | 2 +- packages/cli/src/core/skills.test.ts | 2 ++ packages/cli/src/runtime/command-help.test.ts | 22 ++++++++++--------- 12 files changed, 34 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 94d3ab0d..3074e241 100644 --- a/README.md +++ b/README.md @@ -383,7 +383,7 @@ mb transform-tag delete 5 --yes ## Transform tests -CRUD and run on `/api/transform-test`. Requires Metabase v65 or newer. A transform test pins a transform's behaviour without touching real data: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. +CRUD and run on `/api/ee/transform-test`. Requires Metabase v65 or newer. A transform test pins a transform's behaviour without touching real data: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` (each with a `database_type` the warehouse accepts as a `CAST` target) and `rows`. diff --git a/packages/cli/skill-data/core/SKILL.md b/packages/cli/skill-data/core/SKILL.md index 3f105c97..ed163e66 100644 --- a/packages/cli/skill-data/core/SKILL.md +++ b/packages/cli/skill-data/core/SKILL.md @@ -148,8 +148,8 @@ Only what `--help` does _not_ tell you: footguns and non-obvious behaviors. - **search vs. list.** Enumerate with `… list` verbs; `search` ranks a query or spans resources. `--search-native-query` matches native query text and keeps only query-bearing models; dashboard questions need `--include-dashboard-questions`. - **transform.** Iterate with `transform update `, never `delete` + `create` (keeps the row, `entity_id`, materialized table, and YAML filename — avoids `_2` suffixes and noisy git history). `transform run` needs `--wait` (or `--sync`, which also waits for the output table to register and returns `target_table_id`) or you get only `{run_id, final:null}`. (→ `transform`.) - - + + - **transform-test.** `transform-test run ` checks a transform against fixtures in temp tables — no real table is read — and exits non-zero unless it passes. (→ `transform`.) - **setup is one-shot.** `mb setup` walks `/api/setup` for a **fresh** instance only — errors against an already-configured one. Mostly for bootstrapping local / e2e instances. @@ -174,8 +174,8 @@ This file is enough for any single-command task. For anything deeper, load the r - **`notification`** — scheduled delivery: question alerts (`mb alert`) and dashboard subscriptions (`mb subscription`). Choosing between them, the two schedule/recipient contracts, channel prerequisites, testing a send. - **`transform`** — transform body JSON, create + run-with-wait, run inspection, tags, jobs. - - + + - **`transform-test-plan`** — deciding _what_ to test in a transform: the fixture cast, the expectations, the coverage matrix. (`transform` has the `mb transform-test` shapes.) - **`document`** — Metabase documents (TipTap body, embedding cards). diff --git a/packages/cli/skill-data/transform-test-plan/SKILL.md b/packages/cli/skill-data/transform-test-plan/SKILL.md index 16f02347..9c436685 100644 --- a/packages/cli/skill-data/transform-test-plan/SKILL.md +++ b/packages/cli/skill-data/transform-test-plan/SKILL.md @@ -2,6 +2,7 @@ name: transform-test-plan description: Derive a comprehensive test plan for a transform — the fixture cast, the expectations, hand-derived expected rows, and a coverage matrix — from the model's declared design. Covers input partitioning (zero-case, multiplicity, dirty rows), grain / conservation / recomputation / conformance checks, and known-quirk conventions. Load when the user wants tests planned or written for transforms — "write tests for my transforms", "is my model right", "test plan for this pipeline", "add data quality checks" — whether the model is mid-build or already deployed. The `mb transform-test` command and body shapes live in the `transform` skill; this one decides what to test. allowed-tools: Read, Write, Edit, Bash, AskUserQuestion +requires: [transformTests] --- # Planning transform tests diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index dda46e4c..5b36599b 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -173,7 +173,9 @@ mb transform run "$ID" --wait --profile --json # → succeeded If you really must `create + delete` instead, do the `delete` **before** the first `git-sync export` so the failed entity never lands in git history — an export of a soft-failed state is noise that needs a follow-up cleanup commit. See `git-sync`, "Read state before mutating", for the ordering rule. -## Transform tests (v65+) + + +## Transform tests A transform test replaces every table the transform reads with a fixture, runs it into a temp table, and checks that output. No real table is read or written. @@ -231,6 +233,8 @@ mb transform-test run --profile --json # exits non-zero u Create and update bodies are closed — strip `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` from a `get --full` body before sending it back. + + ## Drop the materialized table (keep the transform) ```bash diff --git a/packages/cli/src/commands/transform-test/create.ts b/packages/cli/src/commands/transform-test/create.ts index c1007332..fd27faae 100644 --- a/packages/cli/src/commands/transform-test/create.ts +++ b/packages/cli/src/commands/transform-test/create.ts @@ -11,7 +11,7 @@ export default defineMetabaseCommand({ meta: { name: "create", description: "Create a transform test from JSON" }, details: 'The JSON body needs a `transform_id`, a `name`, the `inputs` standing in for every table the transform reads, and the `expectations` checked against its output. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` and `rows`. An expectation is either `type: "empty"` with the `sql` that must return nothing, or `type: "equals"`, which takes the same `format` split as an input: `format: "rows"` with the `columns` and `rows` the output must hold, or `format: "sql"` with a query returning them. The body is closed: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.', - capabilities: { minVersion: 65 }, + requires: ["transformTest.create"], args: { ...outputFlags, ...profileFlag, diff --git a/packages/cli/src/commands/transform-test/delete.ts b/packages/cli/src/commands/transform-test/delete.ts index 8edc6e50..93e25085 100644 --- a/packages/cli/src/commands/transform-test/delete.ts +++ b/packages/cli/src/commands/transform-test/delete.ts @@ -5,7 +5,7 @@ import { defineMetabaseCommand } from "../runtime"; export default defineMetabaseCommand({ meta: { name: "delete", description: "Delete a transform test by id" }, - capabilities: { minVersion: 65 }, + requires: ["transformTest.delete"], args: { ...outputFlags, ...profileFlag, diff --git a/packages/cli/src/commands/transform-test/get.ts b/packages/cli/src/commands/transform-test/get.ts index d1e50b85..8f87009b 100644 --- a/packages/cli/src/commands/transform-test/get.ts +++ b/packages/cli/src/commands/transform-test/get.ts @@ -10,7 +10,7 @@ export default defineMetabaseCommand({ meta: { name: "get", description: "Get a transform test by id" }, details: "The compact form carries the id, transform, name and description. Pass --full for the `inputs` and `expectations` themselves, which is also the body to edit and send back to `update`.", - capabilities: { minVersion: 65 }, + requires: ["transformTest.get"], args: { ...outputFlags, ...profileFlag, diff --git a/packages/cli/src/commands/transform-test/list.ts b/packages/cli/src/commands/transform-test/list.ts index 6ca266f6..ab0ed8bd 100644 --- a/packages/cli/src/commands/transform-test/list.ts +++ b/packages/cli/src/commands/transform-test/list.ts @@ -12,7 +12,7 @@ export const TransformTestListEnvelope = listEnvelopeSchema(TransformTestCompact export default defineMetabaseCommand({ meta: { name: "list", description: "List transform tests" }, - capabilities: { minVersion: 65 }, + requires: ["transformTest.list"], args: { ...outputFlags, ...listFlags, diff --git a/packages/cli/src/commands/transform-test/run.ts b/packages/cli/src/commands/transform-test/run.ts index 104f1ad7..bafa4700 100644 --- a/packages/cli/src/commands/transform-test/run.ts +++ b/packages/cli/src/commands/transform-test/run.ts @@ -1,7 +1,6 @@ import type { TransformTestExpectationResult, TransformTestRunResult, - TransformTestStatus, } from "@metabase/client/domain/transform-test"; import { TransformTestRunResult as TransformTestRunResultSchema } from "@metabase/client/domain/transform-test"; @@ -16,9 +15,11 @@ const transformTestRunResultView: ResourceView = { tableColumns: [{ key: "status", label: "Status" }], }; +type ExpectationStatus = TransformTestExpectationResult["status"]; + function namesOf( expectations: TransformTestExpectationResult[], - status: TransformTestStatus, + status: ExpectationStatus, ): string[] { return expectations.filter((expectation) => expectation.status === status).map((e) => e.name); } @@ -40,7 +41,7 @@ export default defineMetabaseCommand({ meta: { name: "run", description: "Run a transform test by id" }, details: "Runs the transform against temp tables built from the test's inputs, checks every expectation against its output, and drops the temp tables. Nothing reads or writes a real table. A failing expectation exits non-zero; --json reports what each expectation found, including the rows a comparison disagreed on.", - capabilities: { minVersion: 65 }, + requires: ["transformTest.run"], args: { ...outputFlags, ...profileFlag, diff --git a/packages/cli/src/commands/transform-test/update.ts b/packages/cli/src/commands/transform-test/update.ts index 1086face..91cacd73 100644 --- a/packages/cli/src/commands/transform-test/update.ts +++ b/packages/cli/src/commands/transform-test/update.ts @@ -12,7 +12,7 @@ export default defineMetabaseCommand({ meta: { name: "update", description: "Update a transform test from JSON" }, details: "Only the fields the body carries are patched. `inputs` and `expectations` replace what is stored rather than merging into it. The body is closed: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.", - capabilities: { minVersion: 65 }, + requires: ["transformTest.update"], args: { ...outputFlags, ...profileFlag, diff --git a/packages/cli/src/core/skills.test.ts b/packages/cli/src/core/skills.test.ts index 9911b161..4cfa1f29 100644 --- a/packages/cli/src/core/skills.test.ts +++ b/packages/cli/src/core/skills.test.ts @@ -682,6 +682,7 @@ describe("the shipped skills", () => { "data-action": ["dataActionsWithoutModel"], "git-sync": ["remoteSync"], transform: ["transforms"], + "transform-test-plan": ["transformTests"], }); const withMarkersLeft = all .filter((skill) => { @@ -702,6 +703,7 @@ describe("the shipped skills", () => { "native-sql", "notification", "transform", + "transform-test-plan", "visualization", ]); expect(withMarkersLeft).toEqual([]); diff --git a/packages/cli/src/runtime/command-help.test.ts b/packages/cli/src/runtime/command-help.test.ts index 1fae8820..d645c834 100644 --- a/packages/cli/src/runtime/command-help.test.ts +++ b/packages/cli/src/runtime/command-help.test.ts @@ -512,6 +512,8 @@ const BODY_COMMANDS = [ "transform-job update", "transform-tag create", "transform-tag update", + "transform-test create", + "transform-test update", "setup", "snippet create", "snippet update", @@ -658,20 +660,20 @@ describe("command tree contract", () => { }); }); - it("gates every transform-test command at the version that first serves the endpoint", async () => { + it("declares each transform-test command's single client method", async () => { const entries = await allEntries(); - const capabilities = Object.fromEntries( + const requires = Object.fromEntries( entries .filter((entry) => entry.command.startsWith("transform-test ")) - .map((entry) => [entry.command, entry.capabilities]), + .map((entry) => [entry.command, entry.requires]), ); - expect(capabilities).toEqual({ - "transform-test list": TRANSFORM_TEST_CAPABILITIES, - "transform-test get": TRANSFORM_TEST_CAPABILITIES, - "transform-test create": TRANSFORM_TEST_CAPABILITIES, - "transform-test update": TRANSFORM_TEST_CAPABILITIES, - "transform-test delete": TRANSFORM_TEST_CAPABILITIES, - "transform-test run": TRANSFORM_TEST_CAPABILITIES, + expect(requires).toEqual({ + "transform-test list": requiresOf("transformTest.list"), + "transform-test get": requiresOf("transformTest.get"), + "transform-test create": requiresOf("transformTest.create"), + "transform-test update": requiresOf("transformTest.update"), + "transform-test delete": requiresOf("transformTest.delete"), + "transform-test run": requiresOf("transformTest.run"), }); }); From ee280b2b5857a50a1ce6bd509169a796e6500f59 Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Mon, 28 Sep 2026 14:24:11 -0400 Subject: [PATCH 11/24] fix(cli): match transform-test prose to Metabase master Input columns cast through `cast_type`; expectation SQL naming a table outside the output and declared inputs is refused with `transform-test.unremapped-reference` rather than reading the real table; MySQL reports `SIGNED` casts as `BIGINT`; the endpoint needs the `transforms-testing` premium feature. Source: enterprise/backend/src/metabase_enterprise/transform_testing/{schema,validator,api}.clj and api_routes/routes.clj on origin/master (e28f66a5ac2). --- README.md | 6 +++--- .../cli/skill-data/transform-test-plan/references/checks.md | 3 ++- packages/cli/skill-data/transform/SKILL.md | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 3074e241..e971c872 100644 --- a/README.md +++ b/README.md @@ -383,11 +383,11 @@ mb transform-tag delete 5 --yes ## Transform tests -CRUD and run on `/api/ee/transform-test`. Requires Metabase v65 or newer. A transform test pins a transform's behaviour without touching real data: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. +CRUD and run on `/api/ee/transform-test`. Requires Metabase v65 or newer with the `transforms-testing` premium feature. A transform test pins a transform's behaviour without touching real data: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. -An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` (each with a `database_type` the warehouse accepts as a `CAST` target) and `rows`. +An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` (each a `name` and a `cast_type` the warehouse accepts as a `CAST` target) and `rows`; every row carries exactly the declared columns. -An expectation is either `type: "empty"` with the `sql` that must return no rows, or `type: "equals"`, which takes the same `format` split as an input — `format: "rows"` with the `columns` and `rows` the output must hold exactly, or `format: "sql"` with a query returning them. An `equals` without a `format` is refused. +An expectation is either `type: "empty"` with the `sql` that must return no rows, or `type: "equals"`, which takes the same `format` split as an input — `format: "rows"` with the `columns` and `rows` the output must hold exactly, or `format: "sql"` with a query returning them. An `equals` without a `format` is refused. Expectation names are unique within a test. Expectation SQL may name only the transform's target table and its declared input tables, which are rewritten to the run's temp tables; any other table is refused with `transform-test.unremapped-reference`. Create and update bodies are closed, so a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent back: diff --git a/packages/cli/skill-data/transform-test-plan/references/checks.md b/packages/cli/skill-data/transform-test-plan/references/checks.md index f046cf5a..e2839004 100644 --- a/packages/cli/skill-data/transform-test-plan/references/checks.md +++ b/packages/cli/skill-data/transform-test-plan/references/checks.md @@ -6,7 +6,8 @@ property that switches it on — blank means always), **assert** (the expectatio Expectation SQL names the transform's **target table** and its **declared input tables** under their real names; both are rewritten to the run's temp tables, so per-row recomputation joins against -inputs are writable. Anything else you name is left exactly as written and reads the real table. +inputs are writable. Anything else you name is refused (`transform-test.unremapped-reference`), so no +check can read a real table. Ids (G1, A2, …) are for cross-referencing within the plan documents only — never surface them to the user bare. diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index 5b36599b..a2e7b01e 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -190,11 +190,11 @@ mb transform-test run --profile --json # exits non-zero u **`inputs`** — one per table the transform reads, each naming a `table` plus either `format: "sql"` with `sql`, or `format: "rows"` with `columns` (each a `name` and a `cast_type`) and `rows`. -**`cast_type` is a `CAST` target, not a column type**, and the two vocabularies differ per warehouse: MySQL casts to `SIGNED` and reports `INTEGER`; ClickHouse takes `Nullable(Int32)` for a column that is `Int64`. So a body is warehouse-specific — don't copy a `database_type` out of a run result into a `cast_type`, and don't expect one body to run everywhere. +**`cast_type` is a `CAST` target, not a column type**, and the two vocabularies differ per warehouse: MySQL casts to `SIGNED` and reports `BIGINT`; ClickHouse takes `Nullable(Int32)` for a column that is `Int64`. So a body is warehouse-specific — don't copy a `database_type` out of a run result into a `cast_type`, and don't expect one body to run everywhere. **`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"`, which needs the same `format` split as an input (`"rows"` with `columns`/`rows`, or `"sql"` with a query). An `equals` without a `format` is refused. -**An `empty` query may only name the transform's target table and its declared input tables.** Those are rewritten to the run's temp tables; any other table you name is left exactly as written and reads the real one — the single way a test run can touch production data. +**Expectation SQL may only name the transform's target table and its declared input tables.** Those are rewritten to the run's temp tables; any other table is refused with `transform-test.unremapped-reference` (400), at `create`/`update` as well as `run`. Expectation names are unique within a test, and every `rows` row carries exactly its declared columns — both refused with the offending name. ```json { From 886357ee1bd2c2dcdc2e95221aa91ae92e1d9b5f Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Mon, 28 Sep 2026 15:25:50 -0400 Subject: [PATCH 12/24] fix(cli): report a failed transform test as an outcome, bound its run, and match the prose to the server A failed transform-test run throws FailedOutcomeError (category outcome, exit 1) instead of a bare Error the CLI reported as an unknown crash. transform-test run takes --timeout, passed through the resource method's request options, since the run endpoint is synchronous. The transform skill, core skill, README and command details describe equals as rows-only, say that only sql inputs touch the source database, name the testable transform kinds and drivers, warn about table-qualified columns, quote the server's duplicate-name refusal, and state that bodies are closed at every level; the check catalog notes the MySQL spelling of IS DISTINCT FROM. --- README.md | 13 ++++++++---- packages/cli/skill-data/core/SKILL.md | 2 +- .../transform-test-plan/references/checks.md | 3 +++ packages/cli/skill-data/transform/SKILL.md | 8 +++---- .../cli/src/commands/transform-test/create.ts | 2 +- .../cli/src/commands/transform-test/get.ts | 2 +- .../cli/src/commands/transform-test/run.ts | 21 +++++++++++++++---- .../cli/src/commands/transform-test/update.ts | 2 +- packages/cli/src/output/error.test.ts | 2 ++ packages/cli/src/output/error.ts | 1 + packages/client/src/errors.ts | 15 +++++++++++++ packages/client/src/http/transport.ts | 2 +- 12 files changed, 56 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index e971c872..d554b1c1 100644 --- a/README.md +++ b/README.md @@ -383,13 +383,13 @@ mb transform-tag delete 5 --yes ## Transform tests -CRUD and run on `/api/ee/transform-test`. Requires Metabase v65 or newer with the `transforms-testing` premium feature. A transform test pins a transform's behaviour without touching real data: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. +CRUD and run on `/api/ee/transform-test`. Requires Metabase v65 or newer with the `transforms-testing` premium feature. A transform test pins a transform's behaviour: every table the transform reads is replaced by an `input` fixture, the transform runs into a temp table, and each `expectation` checks that output. The temp tables are dropped when the run ends. The transform and the expectations read only those temp tables; a `format: "sql"` input runs verbatim against the transform's source database, so it may read real tables. Only query transforms (native SQL or MBQL) can be tested, on Postgres, MySQL, H2, ClickHouse, Redshift or SQL Server. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` (each a `name` and a `cast_type` the warehouse accepts as a `CAST` target) and `rows`; every row carries exactly the declared columns. -An expectation is either `type: "empty"` with the `sql` that must return no rows, or `type: "equals"`, which takes the same `format` split as an input — `format: "rows"` with the `columns` and `rows` the output must hold exactly, or `format: "sql"` with a query returning them. An `equals` without a `format` is refused. Expectation names are unique within a test. Expectation SQL may name only the transform's target table and its declared input tables, which are rewritten to the run's temp tables; any other table is refused with `transform-test.unremapped-reference`. +An expectation is either `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly. Expectation names are unique within a test. Expectation SQL may name only the transform's target table and its declared input tables, which are rewritten to the run's temp tables; any other table, or a column qualified by a table name rather than an alias, is refused with `transform-test.unremapped-reference`. -Create and update bodies are closed, so a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent back: +Create and update bodies are closed at every level, so a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent back: ```sh mb transform-test get 1 --full --json \ @@ -448,13 +448,18 @@ mb transform-test delete 1 --yes ### `mb transform-test run ` -Runs the transform against the fixtures and reports what each expectation found. Exits non-zero when the test does not pass, so it drops straight into CI. +Runs the transform against the fixtures and reports what each expectation found. Exits 1 when the test does not pass, so it drops straight into CI. ```sh mb transform-test run 1 mb transform-test run 1 --json +mb transform-test run 1 --timeout 300000 ``` +| Flag | Description | +| ---------------- | -------------------------------------------------------------------------- | +| `--timeout ` | Request timeout in ms (default 30000); the run is one synchronous request. | + ## Databases Read warehouse metadata from `/api/database`. The `db` group exposes the full database list, the per-database record with optional table/field hydration, schema and table inspection, and the two manual-sync triggers. diff --git a/packages/cli/skill-data/core/SKILL.md b/packages/cli/skill-data/core/SKILL.md index ed163e66..7356f545 100644 --- a/packages/cli/skill-data/core/SKILL.md +++ b/packages/cli/skill-data/core/SKILL.md @@ -150,7 +150,7 @@ Only what `--help` does _not_ tell you: footguns and non-obvious behaviors. - **transform.** Iterate with `transform update `, never `delete` + `create` (keeps the row, `entity_id`, materialized table, and YAML filename — avoids `_2` suffixes and noisy git history). `transform run` needs `--wait` (or `--sync`, which also waits for the output table to register and returns `target_table_id`) or you get only `{run_id, final:null}`. (→ `transform`.) -- **transform-test.** `transform-test run ` checks a transform against fixtures in temp tables — no real table is read — and exits non-zero unless it passes. (→ `transform`.) +- **transform-test.** `transform-test run ` checks a transform against fixtures in temp tables — only a `format: "sql"` input reads the source database — and exits 1 unless it passes. (→ `transform`.) - **setup is one-shot.** `mb setup` walks `/api/setup` for a **fresh** instance only — errors against an already-configured one. Mostly for bootstrapping local / e2e instances. - **eid** translates a string entity id → numeric id: `mb eid --model --json`. Entity ids are NanoIDs that can start with `-`, which the positional form misreads as a flag (shell quotes don't help) — for those, use `--body '{"entity_ids":{"card":["-…"]}}'` (the id is a JSON string value, immune to flag parsing). diff --git a/packages/cli/skill-data/transform-test-plan/references/checks.md b/packages/cli/skill-data/transform-test-plan/references/checks.md index e2839004..154ca5b5 100644 --- a/packages/cli/skill-data/transform-test-plan/references/checks.md +++ b/packages/cli/skill-data/transform-test-plan/references/checks.md @@ -15,6 +15,9 @@ user bare. Throughout, `` stands for the transform's target table as written in the transform's own definition, and `` for a declared input table. +`IS DISTINCT FROM` is the null-safe inequality; MySQL spells it `NOT (a <=> b)`, and where neither +parses, `a <> b OR (a IS NULL) <> (b IS NULL)`. + ## Grain & keys **G1 — Declared grain.** Every output table states "one row per X"; the declaration anchors every diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index a2e7b01e..a84a1eca 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -177,7 +177,7 @@ If you really must `create + delete` instead, do the `delete` **before** the fir ## Transform tests -A transform test replaces every table the transform reads with a fixture, runs it into a temp table, and checks that output. No real table is read or written. +A transform test replaces every table the transform reads with a fixture, runs it into a temp table, and checks that output. The transform and the expectations read only the run's temp tables; a `format: "sql"` input runs verbatim against the transform's source database, so it may read real tables. Only query transforms (native SQL or MBQL) can be tested, on a source database whose driver supports it — Postgres, MySQL, H2, ClickHouse, Redshift, SQL Server; not BigQuery or Snowflake — otherwise `create`/`update`/`run` refuse with `transform-test.unsupported-transform` / `unsupported-driver` (422). ```bash mb transform-test list --transform --profile --json # --transform is optional @@ -192,9 +192,9 @@ mb transform-test run --profile --json # exits non-zero u **`cast_type` is a `CAST` target, not a column type**, and the two vocabularies differ per warehouse: MySQL casts to `SIGNED` and reports `BIGINT`; ClickHouse takes `Nullable(Int32)` for a column that is `Int64`. So a body is warehouse-specific — don't copy a `database_type` out of a run result into a `cast_type`, and don't expect one body to run everywhere. -**`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"`, which needs the same `format` split as an input (`"rows"` with `columns`/`rows`, or `"sql"` with a query). An `equals` without a `format` is refused. +**`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly. -**Expectation SQL may only name the transform's target table and its declared input tables.** Those are rewritten to the run's temp tables; any other table is refused with `transform-test.unremapped-reference` (400), at `create`/`update` as well as `run`. Expectation names are unique within a test, and every `rows` row carries exactly its declared columns — both refused with the offending name. +**Expectation SQL may only name the transform's target table and its declared input tables.** Those are rewritten to the run's temp tables; any other table is refused with `transform-test.unremapped-reference` (400), at `create`/`update` as well as `run`. A column qualified by its table name (`people.id`) is refused the same way, in expectation SQL and in the transform's own SQL alike: qualify by an alias (`FROM people p … p.id`) or leave the column bare — for the transform's SQL that means editing the transform. Expectation names are unique within a test (refused with "expectation names must be unique within a test"), and every `rows` row carries exactly its declared columns (refused naming the missing or undeclared columns). ```json { @@ -231,7 +231,7 @@ mb transform-test run --profile --json # exits non-zero u } ``` -Create and update bodies are closed — strip `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` from a `get --full` body before sending it back. +Create and update bodies are closed at every level (the top level is checked before sending; a stray nested key is a 400) — strip `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` from a `get --full` body before sending it back. diff --git a/packages/cli/src/commands/transform-test/create.ts b/packages/cli/src/commands/transform-test/create.ts index fd27faae..eec06f7c 100644 --- a/packages/cli/src/commands/transform-test/create.ts +++ b/packages/cli/src/commands/transform-test/create.ts @@ -10,7 +10,7 @@ import { defineMetabaseCommand } from "../runtime"; export default defineMetabaseCommand({ meta: { name: "create", description: "Create a transform test from JSON" }, details: - 'The JSON body needs a `transform_id`, a `name`, the `inputs` standing in for every table the transform reads, and the `expectations` checked against its output. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` and `rows`. An expectation is either `type: "empty"` with the `sql` that must return nothing, or `type: "equals"`, which takes the same `format` split as an input: `format: "rows"` with the `columns` and `rows` the output must hold, or `format: "sql"` with a query returning them. The body is closed: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.', + 'The JSON body needs a `transform_id`, a `name`, the `inputs` standing in for every table the transform reads, and the `expectations` checked against its output. An input names its `table` and carries either `format: "sql"` with a `sql` query or `format: "rows"` with `columns` and `rows`. An expectation is either `type: "empty"` with the `sql` that must return nothing, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly. The body is closed at every level: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.', requires: ["transformTest.create"], args: { ...outputFlags, diff --git a/packages/cli/src/commands/transform-test/get.ts b/packages/cli/src/commands/transform-test/get.ts index 8f87009b..f4a4ce63 100644 --- a/packages/cli/src/commands/transform-test/get.ts +++ b/packages/cli/src/commands/transform-test/get.ts @@ -9,7 +9,7 @@ import { defineMetabaseCommand } from "../runtime"; export default defineMetabaseCommand({ meta: { name: "get", description: "Get a transform test by id" }, details: - "The compact form carries the id, transform, name and description. Pass --full for the `inputs` and `expectations` themselves, which is also the body to edit and send back to `update`.", + "The compact form carries the id, transform, name and description. Pass --full for the `inputs` and `expectations` themselves, which is also the body to edit and send back to `update` once `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` are shed.", requires: ["transformTest.get"], args: { ...outputFlags, diff --git a/packages/cli/src/commands/transform-test/run.ts b/packages/cli/src/commands/transform-test/run.ts index bafa4700..706ad36f 100644 --- a/packages/cli/src/commands/transform-test/run.ts +++ b/packages/cli/src/commands/transform-test/run.ts @@ -3,9 +3,12 @@ import type { TransformTestRunResult, } from "@metabase/client/domain/transform-test"; import { TransformTestRunResult as TransformTestRunResultSchema } from "@metabase/client/domain/transform-test"; +import { FailedOutcomeError } from "@metabase/client/errors"; +import { DEFAULT_TIMEOUT_MS } from "@metabase/client/http/transport"; import { renderSummary } from "../../output/render"; import type { ResourceView } from "../../output/view"; +import { interruptSignal } from "../../runtime/interrupt"; import { connectionFlags, outputFlags, profileFlag } from "../flags"; import { parseId } from "../parse-id"; import { defineMetabaseCommand } from "../runtime"; @@ -40,23 +43,33 @@ function summaryLine(id: number, result: TransformTestRunResult): string { export default defineMetabaseCommand({ meta: { name: "run", description: "Run a transform test by id" }, details: - "Runs the transform against temp tables built from the test's inputs, checks every expectation against its output, and drops the temp tables. Nothing reads or writes a real table. A failing expectation exits non-zero; --json reports what each expectation found, including the rows a comparison disagreed on.", + "Runs the transform against temp tables built from the test's inputs, checks every expectation against its output, and drops the temp tables. The transform and the expectations read only those temp tables; a `format: \"sql\"` input runs verbatim against the transform's source database. The run is one synchronous request, so --timeout bounds the whole of it. --json reports what each expectation found, including the rows a comparison disagreed on. A test that does not pass exits 1.", requires: ["transformTest.run"], args: { ...outputFlags, ...profileFlag, ...connectionFlags, + timeout: { + type: "string", + description: "Request timeout in ms", + default: String(DEFAULT_TIMEOUT_MS), + }, id: { type: "positional", description: "Transform test id", required: true }, }, outputSchema: TransformTestRunResultSchema, - examples: ["mb transform-test run 1", "mb transform-test run 1 --json"], + examples: [ + "mb transform-test run 1", + "mb transform-test run 1 --json", + "mb transform-test run 1 --timeout 300000", + ], async run({ args, ctx, getClient }) { const id = parseId(args.id); + const timeoutMs = parseId(args.timeout, "timeout"); const client = await getClient(); - const result = await client.transformTest.run(id); + const result = await client.transformTest.run(id, { timeoutMs, signal: interruptSignal }); renderSummary(result, transformTestRunResultView, summaryLine(id, result), ctx); if (result.status !== "passed") { - throw new Error(`transform test ${id} ${result.status}`); + throw new FailedOutcomeError(`transform test ${id} ${result.status}`); } }, }); diff --git a/packages/cli/src/commands/transform-test/update.ts b/packages/cli/src/commands/transform-test/update.ts index 91cacd73..dadf9d70 100644 --- a/packages/cli/src/commands/transform-test/update.ts +++ b/packages/cli/src/commands/transform-test/update.ts @@ -11,7 +11,7 @@ import { defineMetabaseCommand } from "../runtime"; export default defineMetabaseCommand({ meta: { name: "update", description: "Update a transform test from JSON" }, details: - "Only the fields the body carries are patched. `inputs` and `expectations` replace what is stored rather than merging into it. The body is closed: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.", + "Only the fields the body carries are patched. `inputs` and `expectations` replace what is stored rather than merging into it. The body is closed at every level: a test read back with `get --full` has to shed `id`, `entity_id`, `creator_id`, `created_at` and `updated_at` before it can be sent.", requires: ["transformTest.update"], args: { ...outputFlags, diff --git a/packages/cli/src/output/error.test.ts b/packages/cli/src/output/error.test.ts index 5c02f3af..c4b22977 100644 --- a/packages/cli/src/output/error.test.ts +++ b/packages/cli/src/output/error.test.ts @@ -73,6 +73,7 @@ const CATEGORIES: ReadonlyArray = [ "capability", "abort", "internal", + "outcome", "unknown", ]; @@ -88,6 +89,7 @@ const EXPECTED_EXIT_CODES: Record = { capability: 2, abort: 130, internal: 1, + outcome: 1, unknown: 1, }; diff --git a/packages/cli/src/output/error.ts b/packages/cli/src/output/error.ts index ac405588..6d30994f 100644 --- a/packages/cli/src/output/error.ts +++ b/packages/cli/src/output/error.ts @@ -42,6 +42,7 @@ export function exitCodeFor(category: ErrorCategory): number { case "response-shape": case "timeout": case "internal": + case "outcome": case "unknown": { return FAILURE_EXIT_CODE; } diff --git a/packages/client/src/errors.ts b/packages/client/src/errors.ts index a9ab2f9c..411081f3 100644 --- a/packages/client/src/errors.ts +++ b/packages/client/src/errors.ts @@ -14,6 +14,7 @@ export type ErrorCategory = | "capability" | "abort" | "internal" + | "outcome" | "unknown"; export interface NetworkErrorDetail { @@ -219,6 +220,20 @@ export class InternalError extends MetabaseError { } } +// The operation ran to completion and what it reports is a negative result — a test that failed, a +// run that ended in error. Nothing went wrong in the client or the CLI, so there is no detail to +// debug: the report is in the output the caller already has. +export class FailedOutcomeError extends MetabaseError { + readonly category = "outcome"; + readonly isRetryable = false; + readonly developerDetail = null; + + constructor(message: string) { + super(message); + this.name = "FailedOutcomeError"; + } +} + export class AbortError extends MetabaseError { readonly category = "abort"; readonly isRetryable = false; diff --git a/packages/client/src/http/transport.ts b/packages/client/src/http/transport.ts index a2aa54d7..80d6ddfd 100644 --- a/packages/client/src/http/transport.ts +++ b/packages/client/src/http/transport.ts @@ -32,7 +32,7 @@ export type ExpectedContentType = "json" | "text" | "binary"; export const DEFAULT_METHOD: HttpMethod = "GET"; -const DEFAULT_TIMEOUT_MS = 30_000; +export const DEFAULT_TIMEOUT_MS = 30_000; const OCTET_STREAM_CONTENT_TYPE = "application/octet-stream"; const TEXT_CONTENT_TYPE_PREFIX = "text/"; const ERROR_BODY_BYTE_CAP = 64 * 1024; From 8fae38a50062c6ec2b560ecf3f35ac72564f6075 Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Mon, 28 Sep 2026 15:37:20 -0400 Subject: [PATCH 13/24] fix(client): name the failed-result error category failed and use it for every negative result FailedResultError (category failed, exit 1) replaces FailedOutcomeError, is exported from the client root and listed in its error taxonomy. transform run under --wait/--sync and every git-sync wait throw it for a run or task that ended anything but successful, so the CLI reports the result rather than an unknown crash. The transform skill says run exits 1, that an equals without a format is refused, and names the testable drivers as the whole list. --- packages/cli/skill-data/transform/SKILL.md | 6 +++--- packages/cli/src/commands/git-sync/sync-task.ts | 5 +++-- packages/cli/src/commands/transform-test/run.ts | 4 ++-- packages/cli/src/commands/transform/run.ts | 7 ++++--- packages/cli/src/output/error.test.ts | 4 ++-- packages/cli/src/output/error.ts | 2 +- packages/client/README.md | 8 +++++--- packages/client/src/errors.ts | 12 ++++++------ packages/client/src/index.ts | 1 + 9 files changed, 27 insertions(+), 22 deletions(-) diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index a84a1eca..40c38646 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -177,7 +177,7 @@ If you really must `create + delete` instead, do the `delete` **before** the fir ## Transform tests -A transform test replaces every table the transform reads with a fixture, runs it into a temp table, and checks that output. The transform and the expectations read only the run's temp tables; a `format: "sql"` input runs verbatim against the transform's source database, so it may read real tables. Only query transforms (native SQL or MBQL) can be tested, on a source database whose driver supports it — Postgres, MySQL, H2, ClickHouse, Redshift, SQL Server; not BigQuery or Snowflake — otherwise `create`/`update`/`run` refuse with `transform-test.unsupported-transform` / `unsupported-driver` (422). +A transform test replaces every table the transform reads with a fixture, runs it into a temp table, and checks that output. The transform and the expectations read only the run's temp tables; a `format: "sql"` input runs verbatim against the transform's source database, so it may read real tables. Only query transforms (native SQL or MBQL) can be tested, and only on Postgres, MySQL, H2, ClickHouse, Redshift or SQL Server; otherwise `create`/`update`/`run` refuse with `transform-test.unsupported-transform` / `unsupported-driver` (422). ```bash mb transform-test list --transform --profile --json # --transform is optional @@ -185,14 +185,14 @@ mb transform-test get --full --profile --json # --full for input mb transform-test create --file ./.scratch/test.json --profile --json mb transform-test update --file ./.scratch/patch.json --profile --json mb transform-test delete --yes --profile -mb transform-test run --profile --json # exits non-zero unless it passes +mb transform-test run --profile --json # exits 1 unless it passes ``` **`inputs`** — one per table the transform reads, each naming a `table` plus either `format: "sql"` with `sql`, or `format: "rows"` with `columns` (each a `name` and a `cast_type`) and `rows`. **`cast_type` is a `CAST` target, not a column type**, and the two vocabularies differ per warehouse: MySQL casts to `SIGNED` and reports `BIGINT`; ClickHouse takes `Nullable(Int32)` for a column that is `Int64`. So a body is warehouse-specific — don't copy a `database_type` out of a run result into a `cast_type`, and don't expect one body to run everywhere. -**`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly. +**`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly; an `equals` without a `format` is refused. **Expectation SQL may only name the transform's target table and its declared input tables.** Those are rewritten to the run's temp tables; any other table is refused with `transform-test.unremapped-reference` (400), at `create`/`update` as well as `run`. A column qualified by its table name (`people.id`) is refused the same way, in expectation SQL and in the transform's own SQL alike: qualify by an alias (`FROM people p … p.id`) or leave the column bare — for the transform's SQL that means editing the transform. Expectation names are unique within a test (refused with "expectation names must be unique within a test"), and every `rows` row carries exactly its declared columns (refused naming the missing or undeclared columns). diff --git a/packages/cli/src/commands/git-sync/sync-task.ts b/packages/cli/src/commands/git-sync/sync-task.ts index e4cb8503..9085db3e 100644 --- a/packages/cli/src/commands/git-sync/sync-task.ts +++ b/packages/cli/src/commands/git-sync/sync-task.ts @@ -1,6 +1,7 @@ import { z } from "zod"; import { isSyncTaskFailed, SyncTask } from "@metabase/client/domain/git-sync"; +import { FailedResultError } from "@metabase/client/errors"; import type { PollOptions } from "@metabase/client/poll"; import type { ResourceView } from "../../output/view"; @@ -38,9 +39,9 @@ export function throwIfFailedTask(final: SyncTask | null, verb: string): void { } const failure = withDetail(`git-sync ${verb} ${final.status}`, taskDetail(final)); if (final.status === "conflict") { - throw new Error(`${sentence(failure)} ${CONFLICT_REMEDY}`); + throw new FailedResultError(`${sentence(failure)} ${CONFLICT_REMEDY}`); } - throw new Error(failure); + throw new FailedResultError(failure); } // A task that ends in conflict carries no error message: the labels of the entities it conflicted diff --git a/packages/cli/src/commands/transform-test/run.ts b/packages/cli/src/commands/transform-test/run.ts index 706ad36f..fdaa4fa4 100644 --- a/packages/cli/src/commands/transform-test/run.ts +++ b/packages/cli/src/commands/transform-test/run.ts @@ -3,7 +3,7 @@ import type { TransformTestRunResult, } from "@metabase/client/domain/transform-test"; import { TransformTestRunResult as TransformTestRunResultSchema } from "@metabase/client/domain/transform-test"; -import { FailedOutcomeError } from "@metabase/client/errors"; +import { FailedResultError } from "@metabase/client/errors"; import { DEFAULT_TIMEOUT_MS } from "@metabase/client/http/transport"; import { renderSummary } from "../../output/render"; @@ -69,7 +69,7 @@ export default defineMetabaseCommand({ const result = await client.transformTest.run(id, { timeoutMs, signal: interruptSignal }); renderSummary(result, transformTestRunResultView, summaryLine(id, result), ctx); if (result.status !== "passed") { - throw new FailedOutcomeError(`transform test ${id} ${result.status}`); + throw new FailedResultError(`transform test ${id} ${result.status}`); } }, }); diff --git a/packages/cli/src/commands/transform/run.ts b/packages/cli/src/commands/transform/run.ts index 32f321a5..083a677a 100644 --- a/packages/cli/src/commands/transform/run.ts +++ b/packages/cli/src/commands/transform/run.ts @@ -1,4 +1,5 @@ import { isTransformRunFailed, TransformRunResult } from "@metabase/client/domain/transform"; +import { FailedResultError } from "@metabase/client/errors"; import { renderSummary } from "../../output/render"; import type { ResourceView } from "../../output/view"; @@ -19,7 +20,7 @@ const transformRunResultView: ResourceView = { export default defineMetabaseCommand({ meta: { name: "run", description: "Trigger a transform run by id" }, details: - "Starts a run and returns immediately. --wait polls the run to a terminal status. --sync additionally waits until the run's output table is registered and returns its `target_table_id`, so you can build MBQL cards against it — the run registers the table itself, so no separate `db sync-schema` is needed; --sync implies waiting for the run.", + "Starts a run and returns immediately. --wait polls the run to a terminal status. --sync additionally waits until the run's output table is registered and returns its `target_table_id`, so you can build MBQL cards against it — the run registers the table itself, so no separate `db sync-schema` is needed; --sync implies waiting for the run. While waiting, a run that does not succeed exits 1.", requires: ["transform.run"], args: { ...outputFlags, @@ -63,7 +64,7 @@ export default defineMetabaseCommand({ // Waiting was asked for, so the absence of a run to report on means none was started. if (result.run_id === null || result.final === null) { renderSummary(result, transformRunResultView, result.message, ctx); - throw new Error(`transform run did not start: ${result.message}`); + throw new FailedResultError(`transform run did not start: ${result.message}`); } const status = result.final.status; @@ -75,7 +76,7 @@ export default defineMetabaseCommand({ ); if (isTransformRunFailed(status)) { - throw new Error(`transform run ${result.run_id} ${status}`); + throw new FailedResultError(`transform run ${result.run_id} ${status}`); } }, }); diff --git a/packages/cli/src/output/error.test.ts b/packages/cli/src/output/error.test.ts index c4b22977..db5c2ab0 100644 --- a/packages/cli/src/output/error.test.ts +++ b/packages/cli/src/output/error.test.ts @@ -73,7 +73,7 @@ const CATEGORIES: ReadonlyArray = [ "capability", "abort", "internal", - "outcome", + "failed", "unknown", ]; @@ -89,7 +89,7 @@ const EXPECTED_EXIT_CODES: Record = { capability: 2, abort: 130, internal: 1, - outcome: 1, + failed: 1, unknown: 1, }; diff --git a/packages/cli/src/output/error.ts b/packages/cli/src/output/error.ts index 6d30994f..99e2be7d 100644 --- a/packages/cli/src/output/error.ts +++ b/packages/cli/src/output/error.ts @@ -42,7 +42,7 @@ export function exitCodeFor(category: ErrorCategory): number { case "response-shape": case "timeout": case "internal": - case "outcome": + case "failed": case "unknown": { return FAILURE_EXIT_CODE; } diff --git a/packages/client/README.md b/packages/client/README.md index 61869800..33e268c3 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -374,8 +374,8 @@ Types: `MetabaseClient`, `Transport`, `ClientCredentials`, `ClientOptions`, `Req ### Errors `MetabaseError` is the abstract root of the taxonomy: `NetworkError`, `TimeoutError`, -`ValidationError`, `ResponseShapeError`, `ConfigError`, `InternalError`, `AbortError`, -`ChainedRequestError`, `UnknownError`, and `HttpError`. Every one carries a `category` +`ValidationError`, `ResponseShapeError`, `ConfigError`, `InternalError`, `FailedResultError`, +`AbortError`, `ChainedRequestError`, `UnknownError`, and `HttpError`. Every one carries a `category` (`ErrorCategory`), an `isRetryable`, a `userMessage`, and a `developerDetail` typed per class — `NetworkErrorDetail`, `TimeoutErrorDetail` (the `HttpTimeoutDetail | PollingTimeoutDetail` union), `ValidationErrorDetail`, `ResponseShapeErrorDetail` (the @@ -387,7 +387,9 @@ carries the request context and the Zod issues, and a hand-decoded body or heade `metabase-…` response header — carries the source it was read from and the raw value. `ConfigError` and `InternalError` split blame: the first is input a caller could correct, the second -is a caller that violated a function's contract, which is a bug in the calling code. `HttpError` +is a caller that violated a function's contract, which is a bug in the calling code. +`FailedResultError` is neither: the operation ran and its result is a failure — a test that did not +pass, a run that ended errored — with nothing to debug beyond the result itself. `HttpError` carries the status and response body, redacted of known secrets at construction, plus a `kind` (`HttpErrorKind`) separating a route Metabase does not serve from a row that is gone; `isHttpNotFound(value)` answers the coarser question of whether a thrown value is an `HttpError` with diff --git a/packages/client/src/errors.ts b/packages/client/src/errors.ts index 411081f3..9c652fef 100644 --- a/packages/client/src/errors.ts +++ b/packages/client/src/errors.ts @@ -14,7 +14,7 @@ export type ErrorCategory = | "capability" | "abort" | "internal" - | "outcome" + | "failed" | "unknown"; export interface NetworkErrorDetail { @@ -221,16 +221,16 @@ export class InternalError extends MetabaseError { } // The operation ran to completion and what it reports is a negative result — a test that failed, a -// run that ended in error. Nothing went wrong in the client or the CLI, so there is no detail to -// debug: the report is in the output the caller already has. -export class FailedOutcomeError extends MetabaseError { - readonly category = "outcome"; +// run that ended in error. Nothing went wrong in the client or its caller, so there is no detail to +// debug: the report is in the result the caller already has. +export class FailedResultError extends MetabaseError { + readonly category = "failed"; readonly isRetryable = false; readonly developerDetail = null; constructor(message: string) { super(message); - this.name = "FailedOutcomeError"; + this.name = "FailedResultError"; } } diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 5019fc8f..5a1c0245 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -21,6 +21,7 @@ export { ChainedRequestError, ConfigError, errorMessage, + FailedResultError, InternalError, isFileNotFoundError, MetabaseError, From 1880d933cbad921f6edfc8cbfdbb02bb1a5eae4f Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Fri, 2 Oct 2026 14:20:20 -0400 Subject: [PATCH 14/24] bump --- packages/cli/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index adac9969..ec69bd84 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@metabase/cli", - "version": "0.4.0", + "version": "0.4.1", "description": "Metabase CLI", "license": "AGPL-3.0", "repository": { From 5663bfaf51207d4358ada5f50cc5dbbaf634aacc Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Fri, 2 Oct 2026 15:10:36 -0400 Subject: [PATCH 15/24] fix(cli): report a failed transform test's outcome when its report is over --max-bytes A failing run's --json report could pass the default cap, so the run that already happened ended as a config error with exit 2 and nothing printed. renderSummaryWithinMaxBytes hands the over-cap refusal back instead of throwing it; transform-test run then exits 1 with the expectations that did not pass and the cap message. The run-result view moves to output/views. --- .../cli/src/commands/transform-test/run.ts | 34 +++++------ packages/cli/src/output/render.ts | 56 +++++++++++++++---- .../cli/src/output/views/transform-test.ts | 11 +++- 3 files changed, 72 insertions(+), 29 deletions(-) diff --git a/packages/cli/src/commands/transform-test/run.ts b/packages/cli/src/commands/transform-test/run.ts index fdaa4fa4..540db90d 100644 --- a/packages/cli/src/commands/transform-test/run.ts +++ b/packages/cli/src/commands/transform-test/run.ts @@ -1,23 +1,17 @@ -import type { - TransformTestExpectationResult, +import { + type TransformTestExpectationResult, TransformTestRunResult, } from "@metabase/client/domain/transform-test"; -import { TransformTestRunResult as TransformTestRunResultSchema } from "@metabase/client/domain/transform-test"; -import { FailedResultError } from "@metabase/client/errors"; +import { ConfigError, FailedResultError } from "@metabase/client/errors"; import { DEFAULT_TIMEOUT_MS } from "@metabase/client/http/transport"; -import { renderSummary } from "../../output/render"; -import type { ResourceView } from "../../output/view"; +import { renderSummaryWithinMaxBytes } from "../../output/render"; +import { transformTestRunResultView } from "../../output/views/transform-test"; import { interruptSignal } from "../../runtime/interrupt"; import { connectionFlags, outputFlags, profileFlag } from "../flags"; import { parseId } from "../parse-id"; import { defineMetabaseCommand } from "../runtime"; -const transformTestRunResultView: ResourceView = { - compactPick: TransformTestRunResultSchema, - tableColumns: [{ key: "status", label: "Status" }], -}; - type ExpectationStatus = TransformTestExpectationResult["status"]; function namesOf( @@ -43,7 +37,7 @@ function summaryLine(id: number, result: TransformTestRunResult): string { export default defineMetabaseCommand({ meta: { name: "run", description: "Run a transform test by id" }, details: - "Runs the transform against temp tables built from the test's inputs, checks every expectation against its output, and drops the temp tables. The transform and the expectations read only those temp tables; a `format: \"sql\"` input runs verbatim against the transform's source database. The run is one synchronous request, so --timeout bounds the whole of it. --json reports what each expectation found, including the rows a comparison disagreed on. A test that does not pass exits 1.", + "Runs the transform against temp tables built from the test's inputs, checks every expectation against its output, and drops the temp tables. The transform and the expectations read only those temp tables; a `format: \"sql\"` input runs verbatim against the transform's source database. The run is one synchronous request, so --timeout bounds the whole of it. --json reports what each expectation found, including the rows a comparison disagreed on. A test that does not pass exits 1, also when its report is over --max-bytes and is not printed; the error then names the expectations that did not pass.", requires: ["transformTest.run"], args: { ...outputFlags, @@ -56,7 +50,7 @@ export default defineMetabaseCommand({ }, id: { type: "positional", description: "Transform test id", required: true }, }, - outputSchema: TransformTestRunResultSchema, + outputSchema: TransformTestRunResult, examples: [ "mb transform-test run 1", "mb transform-test run 1 --json", @@ -67,9 +61,17 @@ export default defineMetabaseCommand({ const timeoutMs = parseId(args.timeout, "timeout"); const client = await getClient(); const result = await client.transformTest.run(id, { timeoutMs, signal: interruptSignal }); - renderSummary(result, transformTestRunResultView, summaryLine(id, result), ctx); - if (result.status !== "passed") { - throw new FailedResultError(`transform test ${id} ${result.status}`); + const summary = summaryLine(id, result); + const oversize = renderSummaryWithinMaxBytes(result, transformTestRunResultView, summary, ctx); + if (result.status === "passed") { + if (oversize !== null) { + throw new ConfigError(oversize); + } + return; } + // The run has already happened, so a report too big to print still owes the caller its outcome. + const failure = + oversize === null ? `transform test ${id} ${result.status}` : `${summary} Its ${oversize}`; + throw new FailedResultError(failure); }, }); diff --git a/packages/cli/src/output/render.ts b/packages/cli/src/output/render.ts index 2801c295..9dbee55e 100644 --- a/packages/cli/src/output/render.ts +++ b/packages/cli/src/output/render.ts @@ -34,10 +34,12 @@ export function writeText(text: string): void { type KeyValuePair = readonly [label: string, value: string]; export function renderItem(item: T, view: ResourceView, opts: RenderOptions): void { + writeWithinMaxBytes(itemBody(item, view, opts), opts); +} + +function itemBody(item: T, view: ResourceView, opts: RenderOptions): string { const projected = applyProjection(item, view, opts.full, opts.fields); - const body = renderItemBody(item, view, projected, opts) + "\n"; - assertItemWithinMaxBytes(body, opts); - process.stdout.write(body); + return renderItemBody(item, view, projected, opts) + "\n"; } // Default text/human view prints `summaryText` — a bare scalar for single-value lookups @@ -53,13 +55,35 @@ export function renderSummary( summaryText: string | (() => string), opts: RenderOptions, ): void { + writeWithinMaxBytes(summaryBody(item, view, summaryText, opts), opts); +} + +// renderSummary for a command whose result outranks the cap: over it, nothing is written and the +// refusal comes back as a message, so the caller can still report what the result was. +export function renderSummaryWithinMaxBytes( + item: T, + view: ResourceView, + summaryText: string | (() => string), + opts: RenderOptions, +): string | null { + const body = summaryBody(item, view, summaryText, opts); + const oversize = oversizeMessage(body, opts); + if (oversize === null) { + process.stdout.write(body); + } + return oversize; +} + +function summaryBody( + item: T, + view: ResourceView, + summaryText: string | (() => string), + opts: RenderOptions, +): string { if (opts.format === "json" || opts.fields !== undefined || opts.full) { - renderItem(item, view, opts); - return; + return itemBody(item, view, opts); } - const body = (typeof summaryText === "function" ? summaryText() : summaryText) + "\n"; - assertItemWithinMaxBytes(body, opts); - process.stdout.write(body); + return (typeof summaryText === "function" ? summaryText() : summaryText) + "\n"; } export function renderList( @@ -199,13 +223,21 @@ function renderKeyValueLines(pairs: ReadonlyArray): string { return pairs.map(([label, value]) => `${label.padEnd(padding)} ${value}`).join("\n"); } -function assertItemWithinMaxBytes(body: string, opts: RenderOptions): void { +function writeWithinMaxBytes(body: string, opts: RenderOptions): void { + const oversize = oversizeMessage(body, opts); + if (oversize !== null) { + throw new ConfigError(oversize); + } + process.stdout.write(body); +} + +function oversizeMessage(body: string, opts: RenderOptions): string | null { if (opts.maxBytes <= 0) { - return; + return null; } const bytes = Buffer.byteLength(body, "utf8"); if (bytes <= opts.maxBytes) { - return; + return null; } - throw new ConfigError(itemOversizeMessage(bytes, opts.maxBytes, opts.oversizeHint)); + return itemOversizeMessage(bytes, opts.maxBytes, opts.oversizeHint); } diff --git a/packages/cli/src/output/views/transform-test.ts b/packages/cli/src/output/views/transform-test.ts index a194f851..3d221478 100644 --- a/packages/cli/src/output/views/transform-test.ts +++ b/packages/cli/src/output/views/transform-test.ts @@ -1,4 +1,8 @@ -import { type TransformTest, TransformTestCompact } from "@metabase/client/domain/transform-test"; +import { + type TransformTest, + TransformTestCompact, + TransformTestRunResult, +} from "@metabase/client/domain/transform-test"; import type { ResourceView } from "../view"; @@ -11,3 +15,8 @@ export const transformTestView: ResourceView = { { key: "description", label: "Description" }, ], }; + +export const transformTestRunResultView: ResourceView = { + compactPick: TransformTestRunResult, + tableColumns: [{ key: "status", label: "Status" }], +}; From 38edabaed30ce3c54c040253ca6a1136b2a1e2dc Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Fri, 2 Oct 2026 15:10:36 -0400 Subject: [PATCH 16/24] fix(cli): spell transform-test list's filter --transform-id Matches transform runs --transform-id, and a malformed value names the flag. --- README.md | 8 ++++---- packages/cli/src/commands/transform-test/list.ts | 9 ++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index d554b1c1..ad334412 100644 --- a/README.md +++ b/README.md @@ -401,12 +401,12 @@ mb transform-test get 1 --full --json \ ```sh mb transform-test list --json -mb transform-test list --transform 1 +mb transform-test list --transform-id 1 ``` -| Flag | Description | -| ------------------ | ------------------------------------ | -| `--transform ` | Only the tests of this transform id. | +| Flag | Description | +| --------------------- | ------------------------------------ | +| `--transform-id ` | Only the tests of this transform id. | ### `mb transform-test get ` diff --git a/packages/cli/src/commands/transform-test/list.ts b/packages/cli/src/commands/transform-test/list.ts index ab0ed8bd..09dc6603 100644 --- a/packages/cli/src/commands/transform-test/list.ts +++ b/packages/cli/src/commands/transform-test/list.ts @@ -18,13 +18,16 @@ export default defineMetabaseCommand({ ...listFlags, ...profileFlag, ...connectionFlags, - transform: { type: "string", description: "Only the tests of this transform id" }, + "transform-id": { type: "string", description: "Only the tests of this transform id" }, }, outputSchema: TransformTestListEnvelope, - examples: ["mb transform-test list", "mb transform-test list --transform 1 --json"], + examples: ["mb transform-test list", "mb transform-test list --transform-id 1 --json"], async run({ args, ctx, getClient }) { const client = await getClient(); - const transformId = args.transform === undefined ? undefined : parseId(args.transform); + const transformId = + args["transform-id"] === undefined + ? undefined + : parseId(args["transform-id"], "--transform-id"); const { data } = await client.transformTest.list({ "transform-id": transformId }); renderList(windowList(data, ctx.range), transformTestView, ctx); }, From e428baf98575dbf3d878e6c5c398a76e12430c00 Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Fri, 2 Oct 2026 15:10:36 -0400 Subject: [PATCH 17/24] docs(skills): ground the transform-test prose in the server and restore core's dropped lines The plan skill says how to load its references and no longer claims every empty expectation passes on an empty output; its prose is unwrapped. The transform skill says equals compares only declared columns and that format sql can never pass. Core is main's text plus the transform-test entries. --- packages/cli/skill-data/core/SKILL.md | 14 +- .../skill-data/transform-test-plan/SKILL.md | 175 +++---------- .../references/checklist.md | 89 ++----- .../transform-test-plan/references/checks.md | 245 +++++------------- packages/cli/skill-data/transform/SKILL.md | 4 +- 5 files changed, 141 insertions(+), 386 deletions(-) diff --git a/packages/cli/skill-data/core/SKILL.md b/packages/cli/skill-data/core/SKILL.md index 7356f545..0390c9eb 100644 --- a/packages/cli/skill-data/core/SKILL.md +++ b/packages/cli/skill-data/core/SKILL.md @@ -6,7 +6,7 @@ allowed-tools: Read, Write, Edit, Bash, AskUserQuestion # metabase-cli (core) -`mb` drives a Metabase instance over its REST API: CRUD on every resource, query and transform execution, search, git-sync (representations ↔ instance), and entity-id translation. +The official Metabase CLI (`mb`) drives a Metabase instance over its REST API: auth, list/get/create/update/delete on every resource, query and transform execution, content search, git-sync (representations ↔ instance), and entity-id translation. Top-level command groups (run `mb --help` to discover verbs): @@ -16,7 +16,7 @@ document | glossary | timeline | timeline-event | transform | transform-job | tr search | dependency | git-sync | setup | eid | uuid | upgrade | skills ``` -The conventions below — auth, flags, output, body input — hold across **every** group. When a card needs a query, prefer MBQL over native SQL (portable, pre-flight-validated — load `mbql`); fall back to native SQL when MBQL can't express it. +The conventions below — auth, flags, output, body input — hold across **every** group. Per-command flags and examples live in each command's `--help`; add `--json` for the machine-readable form with the output JSON Schema. A few flows have their own skills (see "Specialized skills"). When a card needs a query, prefer MBQL over native SQL (portable, pre-flight-validated — load `mbql`); fall back to native SQL when MBQL can't express it. ## Auth & profiles @@ -28,7 +28,7 @@ mb auth status --json # → {profile, present, url} for the d mb auth status --profile --json # health probe for one profile ``` -`auth list` is the primary enumeration path — one call returns every profile with sanitized URL, an `authenticated` flag, and a probe `status` (`ok` / `auth-failed` / `network-error` / `server-error` / `not-probed`). +`auth list` is the primary enumeration path — one call returns every profile with sanitized URL, an `authenticated` flag, and a probe `status` (`ok` / `auth-failed` / `network-error` / `server-error` / `not-probed`). Use it before asking which profile to pick. - One profile and intent doesn't disambiguate → use it. - Several → ask via `AskUserQuestion`, presenting the names from `auth list`. @@ -70,10 +70,10 @@ Every list verb takes `--limit ` (items this call returns) and `--offset ` - **`has_more` decides whether to keep going — never compare counts.** `total` is the server's count on endpoints that report one and `null` on those that don't, so arithmetic over it is not a termination condition. - **To continue, pass `next_offset` back as `--offset`.** When `has_more` is true `next_offset` is past the offset you sent, so the loop advances; when false the walk is over and `next_offset` is `null`. -- **`truncated` means the byte cap cut the output, not that the data ran out.** `has_more`/`next_offset` are recomputed to the cut point, so a capped list resumes like any window. Its `bytes` is what the untruncated answer would have measured. Narrow rows with `--fields` rather than raising `--max-bytes` — the cap counts only what you asked for, so `--fields` buys rows directly. A capped list always returns at least one row; when not even one fits it exits 2 with "the smallest response this list can produce is N bytes, over the M-byte --max-bytes cap; …". +- **`truncated` means the byte cap cut the output, not that the data ran out.** `has_more`/`next_offset` are recomputed to the cut point, so a capped list resumes like any window. Its `bytes` is what the untruncated answer would have measured. Narrow rows with `--fields` rather than raising `--max-bytes` — a bigger cap spends context on fields you didn't ask for, and the cap counts only what you asked for, so `--fields` buys rows directly. A capped list always returns at least one row; when not even one fits it exits 2 with "the smallest response this list can produce is N bytes, over the M-byte --max-bytes cap; …". - `limit` is echoed only when you passed `--limit` — except `mb search`, which defaults to `--limit 20` (an unbounded search is expensive server-side) and so always reports one. On nouns the server doesn't page, one large `--limit` with narrow `--fields` is a single request; many small `--offset` hops are one request each. -The whole walk: +The whole walk, literally: ```bash offset=0 @@ -105,7 +105,7 @@ mb create --file ./.scratch/body.json --profile --json Single-quoted `'EOF'` stops the shell interpolating `$vars` inside the JSON. -Write working files to **`./.scratch`** in the current directory (`mkdir -p ./.scratch` first), never `/tmp` — they persist across the session and the user can review them. +Write working files to **`./.scratch`** in the current directory (`mkdir -p ./.scratch` first), never `/tmp` — better permissions, they persist across the session, and the user can review them. ## Discovering commands and schemas @@ -124,7 +124,7 @@ mb transform --help --json | jq -r '.commands[].command' # verbs under "transfo ## Resource quirks worth memorizing -Only what `--help` does _not_ tell you: footguns and non-obvious behaviors. +Routine verb shapes (list / get / create / update), every flag, and output schemas live in each command's `--help` (add `--json` for output schemas). Below is only what help does _not_ tell you: footguns and non-obvious behaviors. - **db traversal: the hydration ladder.** Start with `database get --include tables` — the compact table map (id, name, schema, description per table), one call that fits most databases. Pick the relevant tables, then `table fields ` per table (bounded: fields are per-table). `--include tables.fields` is the full rollup — small databases only. Hundreds of tables? Traverse by schema (`database schemas ` → `database schema-tables `) or look tables up by name (`search --models table --db-id --limit 10`). `db sync-schema` / `rescan-values` queue async work and return `{status:"ok"}` at once; `sync-schema --wait` blocks until `initial_sync_status: complete`. - **table fields.** `table get` never returns fields on its own — pass `--include fields` (compact; the underlying query_metadata response also carries FK targets and dimensions, visible under `--full`) or use `table fields ` (list envelope). `table update` patches table-level metadata only; physical columns aren't editable. diff --git a/packages/cli/skill-data/transform-test-plan/SKILL.md b/packages/cli/skill-data/transform-test-plan/SKILL.md index 9c436685..082bdc40 100644 --- a/packages/cli/skill-data/transform-test-plan/SKILL.md +++ b/packages/cli/skill-data/transform-test-plan/SKILL.md @@ -7,165 +7,76 @@ requires: [transformTests] # Planning transform tests -Turn a transform into a fixture cast that proves the logic on small known rows, expectations that -state the model's invariants, and a coverage matrix showing what's checked and what's deliberately -not. Mechanics — the `inputs`/`expectations` body shape and every `mb transform-test` verb — live in -the `transform` skill (`mb skills get transform`); load it before authoring, and never restate it -here. +Turn a transform into a fixture cast that proves the logic on small known rows, expectations that state the model's invariants, and a coverage matrix showing what's checked and what's deliberately not. Mechanics — the `inputs`/`expectations` body shape and every `mb transform-test` verb — live in the `transform` skill (`mb skills get transform`); load it before authoring, and never restate it here. The two references this skill routes through, `references/checklist.md` and `references/checks.md`, come with `mb skills get transform-test-plan --full`, or read them from `mb skills path transform-test-plan`. -Every check derives from what the model **declares** — detected from its SQL, confirmed with its -owner — never from conformance to a modeling doctrine. One plan serves two moments: while the model -is **built**, checks pin each design decision; once **deployed**, the same SQL screens production -tables for anomalies. +Every check derives from what the model **declares** — detected from its SQL, confirmed with its owner — never from conformance to a modeling doctrine. One plan serves two moments: while the model is **built**, checks pin each design decision; once **deployed**, the same SQL screens production tables for anomalies. ## Operating rules -- **Detect, then derive.** Classify what the transform is and which conventions it uses (the - checklist); derive checks only from that. Star, one-big-table, partial denormalization — all - fine; never flag a style. -- **Judgment calls go through the checklist.** The session's autonomy setting governs which answers - you supply yourself and which you bring to the user — it never makes the checklist a formality. - Every answer you supply yourself is recorded in the plan as a stated assumption, paired with the - named expectation that enforces it — reversing the decision then breaks a test, not a paragraph. - And regardless of setting, when genuinely unsure, ask — a wrong-but-confident grain poisons every - downstream check. -- **Expected rows are derived by hand** from the fixture story and business meaning — never captured - from the transform's output, which asserts only that the transform equals itself. -- **Batteries stay off until declared structure switches them on.** No snapshot-density checks - without a snapshot, no version-history checks without effective/end/current columns. An empty - section beats a speculative one. +- **Detect, then derive.** Classify what the transform is and which conventions it uses (the checklist); derive checks only from that. Star, one-big-table, partial denormalization — all fine; never flag a style. +- **Judgment calls go through the checklist.** The session's autonomy setting governs which answers you supply yourself and which you bring to the user — it never makes the checklist a formality. Every answer you supply yourself is recorded in the plan as a stated assumption, paired with the named expectation that enforces it — reversing the decision then breaks a test, not a paragraph. And regardless of setting, when genuinely unsure, ask — a wrong-but-confident grain poisons every downstream check. +- **Expected rows are derived by hand** from the fixture story and business meaning — never captured from the transform's output, which asserts only that the transform equals itself. +- **Batteries stay off until declared structure switches them on.** No snapshot-density checks without a snapshot, no version-history checks without effective/end/current columns. An empty section beats a speculative one. ## The procedure -1. **Profile the real inputs**: per table, row count; per column, min/max/distinct-count/null - incidence; orphan counts across declared links (`mb field summary`, `mb query`). Profiling feeds - domains, bounds, null partitions, and key candidates — and every fixture edge cites the real-data - condition that warrants it, with its count ("the warehouse has 67 ship-before-order rows"). -2. **Detect.** Read the transform's SQL (`mb transform get --full --json`) for: grain - candidates (`GROUP BY` keys, the joins' driving table), join types (orphan handling), correlated - aggregates (stored aggregates), `_` naming (copies), effective/end/current columns - (version history), `now()`/`current_date` (volatile columns). A transform whose grain you cannot - state in one phrase is itself a finding — raise it before writing any test. -3. **Confirm.** Walk [references/checklist.md](references/checklist.md). Three stages: classify the - model, per-table declarations, per-column declarations. Each question carries its detection hint; - answer autonomously where the hint resolves, ask where it doesn't. In build-along mode these are - design questions — treat an undecided answer as a decision to make together, not a blocker. -4. **Derive.** Route every output column through [references/checks.md](references/checks.md) — - declared property → expectation shape, fixture implication, expected-row convention. Read it in - full once per plan; it is the plan's content. -5. **Design the fixture cast**: one small cast per transform (≈5–10 rows per table), human-named - rows ("Alice Premium"), every row a named edge — zero-case entities for every outer join and - aggregation, ≥2-member groups for every grouping and join, one dirty row per screenable defect, - boundary dates. Document it as a table (row → attributes → purpose) in the plan. - - Express the cast as literal rows rather than as a query, so the story stays legible in the test - itself; a query is worth it only when the rows are mechanical to generate. A cast is written - against one warehouse and does not carry to another. - -6. **Hand-derive the expected rows**, arithmetic recorded in the plan (premium: 3 orders / 350.50 / - 3.0). Every fixture row's fate appears in some expected cell. Pin NULL-vs-0-vs-empty for every - zero-case row — that cell is the null policy's only enforcement. -7. **Author the test.** One test per transform, per coherent story: an `equals` pinning the output, - and `empty` expectations stating the invariants that survive a change to the cast. - - Give each expectation a name that states the invariant, because the name is what a - failure leads with ("revenue never negative", not "check 3"), and open each `empty` expectation's - SQL with a `--` contract comment: the invariant, and the failure modes it catches ("catches both - dropped orders and join fan-out"). - - Where one invariant applies to several transforms, duplicate the SQL. Nothing is shared between - tests, so give each copy its own name and comment rather than one that only makes sense next to - its twin. - -8. **Emit the coverage matrix** in the plan: rows = output columns with the table's grain; columns = - check classes (grain, conservation, recomputation, conformance, domains, referential integrity, - temporal, screens); cells name the covering expectation or expected-row cell, or state - `gap: `. Scan shared-attribute columns across transforms for cross-table agreement - obligations. Empty cells are honest; silent gaps are not. -9. **Prove the tests have teeth.** Once per test: corrupt one expected cell, `run`, confirm - `cell-mismatches` names exactly that column; revert. Then perturb one input cell and confirm - exactly the declared output cells move. Every `empty` expectation passes against an empty output, - so a green test can still be vacuous. +1. **Profile the real inputs**: per table, row count; per column, min/max/distinct-count/null incidence; orphan counts across declared links (`mb field summary`, `mb query`). Profiling feeds domains, bounds, null partitions, and key candidates — and every fixture edge cites the real-data condition that warrants it, with its count ("the warehouse has 67 ship-before-order rows"). +2. **Detect.** Read the transform's SQL (`mb transform get --full --json`) for: grain candidates (`GROUP BY` keys, the joins' driving table), join types (orphan handling), correlated aggregates (stored aggregates), `_` naming (copies), effective/end/current columns (version history), `now()`/`current_date` (volatile columns). A transform whose grain you cannot state in one phrase is itself a finding — raise it before writing any test. +3. **Confirm.** Walk [references/checklist.md](references/checklist.md). Three stages: classify the model, per-table declarations, per-column declarations. Each question carries its detection hint; answer autonomously where the hint resolves, ask where it doesn't. In build-along mode these are design questions — treat an undecided answer as a decision to make together, not a blocker. +4. **Derive.** Route every output column through [references/checks.md](references/checks.md) — declared property → expectation shape, fixture implication, expected-row convention. Read it in full once per plan; it is the plan's content. +5. **Design the fixture cast**: one small cast per transform (≈5–10 rows per table), human-named rows ("Alice Premium"), every row a named edge — zero-case entities for every outer join and aggregation, ≥2-member groups for every grouping and join, one dirty row per screenable defect, boundary dates. Document it as a table (row → attributes → purpose) in the plan. + + Express the cast as literal rows rather than as a query, so the story stays legible in the test itself; a query is worth it only when the rows are mechanical to generate. A cast is written against one warehouse and does not carry to another. + +6. **Hand-derive the expected rows**, arithmetic recorded in the plan (premium: 3 orders / 350.50 / 3.0). Every fixture row's fate appears in some expected cell. Pin NULL-vs-0-vs-empty for every zero-case row — that cell is the null policy's only enforcement. +7. **Author the test.** One test per transform, per coherent story: an `equals` pinning the output, declaring every output column, since `equals` compares only the columns it declares, and `empty` expectations stating the invariants that survive a change to the cast. + + Give each expectation a name that states the invariant, because the name is what a failure leads with ("revenue never negative", not "check 3"), and open each `empty` expectation's SQL with a `--` contract comment: the invariant, and the failure modes it catches ("catches both dropped orders and join fan-out"). + + Where one invariant applies to several transforms, duplicate the SQL. Nothing is shared between tests, so give each copy its own name and comment rather than one that only makes sense next to its twin. + +8. **Emit the coverage matrix** in the plan: rows = output columns with the table's grain; columns = check classes (grain, conservation, recomputation, conformance, domains, referential integrity, temporal, screens); cells name the covering expectation or expected-row cell, or state `gap: `. Scan shared-attribute columns across transforms for cross-table agreement obligations. Empty cells are honest; silent gaps are not. +9. **Prove the tests have teeth.** Once per test: corrupt one expected cell, `run`, confirm `cell-mismatches` names exactly that column; revert. Then perturb one input cell and confirm exactly the declared output cells move. An `empty` expectation that only selects output rows passes against an empty output, so a green test can still be vacuous. ## Severity: error, or the tolerated oddity -Every expectation is pass or fail, and one failure fails the run. **error** = forbidden, and it -becomes an `empty` expectation. There is no warn severity, so **never author an expectation you -expect to fire**: a permanently red test trains everyone to ignore the result, and a red run gates -everyone else's work. +Every expectation is pass or fail, and one failure fails the run. **error** = forbidden, and it becomes an `empty` expectation. There is no warn severity, so **never author an expectation you expect to fire**: a permanently red test trains everyone to ignore the result, and a red run gates everyone else's work. -A tolerated-but-surfaced oddity — one the owner lives with, like orphan rows or ship-before-order -dates — gets encoded the two ways that hold: **pin it in the `equals` rows** (an orphan passing -through with NULLs is a cell in the expected output, so reversing the tolerance breaks the test), -and **record it in the plan's known-quirks list** with its real-data count, so the tolerance stays a -conscious choice. +A tolerated-but-surfaced oddity — one the owner lives with, like orphan rows or ship-before-order dates — gets encoded the two ways that hold: **pin it in the `equals` rows** (an orphan passing through with NULLs is a cell in the expected output, so reversing the tolerance breaks the test), and **record it in the plan's known-quirks list** with its real-data count, so the tolerance stays a conscious choice. -Under budget pressure cut business-rule checks first, then cross-table structure checks; never -single-column screens (domains, ranges, nulls) — cheapest, and the last line. +Under budget pressure cut business-rule checks first, then cross-table structure checks; never single-column screens (domains, ranges, nulls) — cheapest, and the last line. ## When a check exposes a live bug -Non-negotiable: **never soften the test to green** — expected values state correct behavior; matching -them to buggy output documents the bug as intended — and **surface the finding with its blast radius -at both scales**, fixture ("1530.24 of 1600.74 fixture dollars survive") and warehouse ("908 of -2,050 orders dropped"). What happens next follows the session's terms, not a fixed protocol: propose -and apply the fix now (when the user wants it or the autonomy setting covers it), or — when the fix -must wait — hold the correct expectation and record the red in the plan with the minimal fix body. -A stored test has no red-by-design state, so a deferred -fix must be visible in the plan or the test reads as broken. Either way, once green the test stays -as the regression guard. +Non-negotiable: **never soften the test to green** — expected values state correct behavior; matching them to buggy output documents the bug as intended — and **surface the finding with its blast radius at both scales**, fixture ("1530.24 of 1600.74 fixture dollars survive") and warehouse ("908 of 2,050 orders dropped"). What happens next follows the session's terms, not a fixed protocol: propose and apply the fix now (when the user wants it or the autonomy setting covers it), or — when the fix must wait — hold the correct expectation and record the red in the plan with the minimal fix body. A stored test has no red-by-design state, so a deferred fix must be visible in the plan or the test reads as broken. Either way, once green the test stays as the regression guard. ## When the doctrine doesn't apply -The vocabulary follows Kimball's dimensional modeling (grain, additivity, conformed attributes, -slowly changing dimensions) — precise, widely understood terms. Real models are Kimball-inspired at -most; no check may score adherence: - -- Full calendar date dimensions are rare. Never demand one; test date _semantics_ — ranges, - orderings, volatile derivations. -- Surrogate keys are doctrine, natural-key joins are practice. Test whichever key the model - declares; never flag natural-key joins. -- "No NULL FKs / no NULL attributes" is doctrine routinely dropped. Null policy is three independent - declarations — measures, attributes, FKs — each detected and confirmed, never presumed. -- One-big-table is legitimate: it still has a grain, its copies still need agreement checks, its - functional dependencies still hold. -- A transform-level `ORDER BY` has no testable effect — output tables carry no row order and the - `equals` comparison is a multiset. Flag it as probable dead weight (clustering hints aside); never - write an ordering expectation. +The vocabulary follows Kimball's dimensional modeling (grain, additivity, conformed attributes, slowly changing dimensions) — precise, widely understood terms. Real models are Kimball-inspired at most; no check may score adherence: + +- Full calendar date dimensions are rare. Never demand one; test date _semantics_ — ranges, orderings, volatile derivations. +- Surrogate keys are doctrine, natural-key joins are practice. Test whichever key the model declares; never flag natural-key joins. +- "No NULL FKs / no NULL attributes" is doctrine routinely dropped. Null policy is three independent declarations — measures, attributes, FKs — each detected and confirmed, never presumed. +- One-big-table is legitimate: it still has a grain, its copies still need agreement checks, its functional dependencies still hold. +- A transform-level `ORDER BY` has no testable effect — output tables carry no row order and the `equals` comparison is a multiset. Flag it as probable dead weight (clustering hints aside); never write an ordering expectation. ## One transform at a time -Each test covers one transform. When the transform under test reads another transform's output, -that target table is an input like any other: declare it and fake it. Derive those rows from the -base transform's own expected output, so the two tests tell one story, and note the coupling in both -plans — changing the base's expected rows means changing this test's input. +Each test covers one transform. When the transform under test reads another transform's output, that target table is an input like any other: declare it and fake it. Derive those rows from the base transform's own expected output, so the two tests tell one story, and note the coupling in both plans — changing the base's expected rows means changing this test's input. ## Worked example, condensed -`orders` + `customers` → _enriched_orders_ (order grain; LEFT JOIN attaches `customer_name`, -`tier`). +`orders` + `customers` → _enriched_orders_ (order grain; LEFT JOIN attaches `customer_name`, `tier`). -Cast, 7 orders: two tiers; Alice and Carol with 2 orders each (multiplicity); two never shipped -(zero-case for the shipping join); order 106 shipped before ordered (real oddity, count cited); -order 107's customer_id matches no customer (orphan). Two `rows` inputs, one per source table. +Cast, 7 orders: two tiers; Alice and Carol with 2 orders each (multiplicity); two never shipped (zero-case for the shipping join); order 106 shipped before ordered (real oddity, count cited); order 107's customer_id matches no customer (orphan). Two `rows` inputs, one per source table. -Derived, per catalog: grain uniqueness on `order_id`; row and amount conservation from input to -output, as scalar subqueries over the target and the seeded input; `tier` domain ⊆ {standard, -premium}; orphan = keep-with-NULLs → an expected row pinning the NULL pass-through; ship-before-order -tolerated → the plan's known-quirks list with the warehouse count. The whole output pinned in one -`equals`, hand-computed, arithmetic in the plan. The inner-vs-LEFT-join bug this cast catches — -unshipped orders silently dropped from revenue — is what the zero-case rows exist for. +Derived, per catalog: grain uniqueness on `order_id`; row and amount conservation from input to output, as scalar subqueries over the target and the seeded input; `tier` domain ⊆ {standard, premium}; orphan = keep-with-NULLs → an expected row pinning the NULL pass-through; ship-before-order tolerated → the plan's known-quirks list with the warehouse count. The whole output, every column, pinned in one `equals`, hand-computed, arithmetic in the plan. The inner-vs-LEFT-join bug this cast catches — unshipped orders silently dropped from revenue — is what the zero-case rows exist for. ## Don't -- Don't author expectations before loading the `transform` skill — the body shape, the closed - create/update contract, and the verb flags live there. -- Don't capture expected rows from the transform's own output — hand-derive them or they assert - nothing. -- Don't emit checks for structure the model doesn't declare (snapshot density, version history, - bridge weights) — an inapplicable battery buries real findings. -- Don't let a fixture cast go all-clean — no zero-case, no orphan, no dirty row proves the happy - path and nothing else; the bugs live in the edges. +- Don't author expectations before loading the `transform` skill — the body shape, the closed create/update contract, and the verb flags live there. +- Don't capture expected rows from the transform's own output — hand-derive them or they assert nothing. +- Don't emit checks for structure the model doesn't declare (snapshot density, version history, bridge weights) — an inapplicable battery buries real findings. +- Don't let a fixture cast go all-clean — no zero-case, no orphan, no dirty row proves the happy path and nothing else; the bugs live in the edges. - Don't write an expectation you expect to fail. -- Don't surface bare check-ids ("per C3…") to the user — name the check in plain words; the ids are - for your cross-referencing, not their reading. +- Don't surface bare check-ids ("per C3…") to the user — name the check in plain words; the ids are for your cross-referencing, not their reading. diff --git a/packages/cli/skill-data/transform-test-plan/references/checklist.md b/packages/cli/skill-data/transform-test-plan/references/checklist.md index 58b1b374..3f5609e9 100644 --- a/packages/cli/skill-data/transform-test-plan/references/checklist.md +++ b/packages/cli/skill-data/transform-test-plan/references/checklist.md @@ -1,81 +1,32 @@ # Confirmation checklist -The judgment calls a plan depends on, in derivation order. Each question carries a **detect** hint — -how to answer it from the SQL, the metadata, or profiling. Answer autonomously where the hint -resolves cleanly; ask where it doesn't. Every answer you supply yourself goes into the emitted plan -as a stated assumption, phrased so the owner can falsify it at a glance ("Assuming `discount` can -never be negative — correct?"). +The judgment calls a plan depends on, in derivation order. Each question carries a **detect** hint — how to answer it from the SQL, the metadata, or profiling. Answer autonomously where the hint resolves cleanly; ask where it doesn't. Every answer you supply yourself goes into the emitted plan as a stated assumption, phrased so the owner can falsify it at a glance ("Assuming `discount` can never be negative — correct?"). -In build-along mode these are design questions: the owner is deciding, not recalling. An undecided -answer is a decision to make together — surface the options and the check each implies (the -`checks.md` entry named in parentheses). +In build-along mode these are design questions: the owner is deciding, not recalling. An undecided answer is a decision to make together — surface the options and the check each implies (the `checks.md` entry named in parentheses). ## Stage 1 — classify the model (per transform) -1. **Grain: one row per what?** (G1, G2) - Detect: `GROUP BY` keys; otherwise the driving table of the joins. Confirm whenever detection and - any written description disagree — that disagreement is itself a finding. -2. **Fact-table type — and if periodic snapshot, dense or sparse?** (F2) - Detect: event-grain with a single event date → transaction; entity×period grain → periodic - snapshot (density is the one yes/no that flips its whole invariant set — always confirm); one row - per pipeline occurrence with milestone-date columns → accumulating snapshot. +1. **Grain: one row per what?** (G1, G2) Detect: `GROUP BY` keys; otherwise the driving table of the joins. Confirm whenever detection and any written description disagree — that disagreement is itself a finding. +2. **Fact-table type — and if periodic snapshot, dense or sparse?** (F2) Detect: event-grain with a single event date → transaction; entity×period grain → periodic snapshot (density is the one yes/no that flips its whole invariant set — always confirm); one row per pipeline occurrence with milestone-date columns → accumulating snapshot. ## Stage 2 — per-table declarations -3. **Conservation ties: output rows/sums tie to which input, with which declared exclusions?** (A2) - Detect: `WHERE` clauses and join types name the exclusions (filtered rows, dropped duplicates). - Exclusions must be declared, or conservation appears to fail. -4. **Zero-group policy: does a group with no contributing rows appear (with zeros) or stay absent?** - (A5) - Detect: driving table of the rollup — grouping the detail can't produce empty groups; joining a - dimension first can. -5. **Orphan handling per join: drop / keep-with-NULLs / default row — and is an orphan tolerated or - forbidden?** (C3, I5) - Detect: join type. INNER = drop, LEFT = keep-with-NULLs, COALESCE to a sentinel = default row. - Tolerated-vs-forbidden is the owner's call — profiling says whether orphans exist today, not - whether they're acceptable. Forbidden becomes an `empty` expectation; tolerated is pinned in the - expected rows and recorded in the known-quirks list. -6. **Version history: effective/end/current columns anywhere? If not, confirm overwrite-everywhere.** - (T1) - Detect: column names. Absence means history is silently rewritten in rollups — state that - consequence when confirming, not just the mechanism. -7. **Does this transform read another transform's output?** (A5) - Detect: another transform's target table in this one's SQL. It becomes a declared input like any - other, faked from that transform's own expected output — confirm which rows, and record the - coupling in both plans. +3. **Conservation ties: output rows/sums tie to which input, with which declared exclusions?** (A2) Detect: `WHERE` clauses and join types name the exclusions (filtered rows, dropped duplicates). Exclusions must be declared, or conservation appears to fail. +4. **Zero-group policy: does a group with no contributing rows appear (with zeros) or stay absent?** (A5) Detect: driving table of the rollup — grouping the detail can't produce empty groups; joining a dimension first can. +5. **Orphan handling per join: drop / keep-with-NULLs / default row — and is an orphan tolerated or forbidden?** (C3, I5) Detect: join type. INNER = drop, LEFT = keep-with-NULLs, COALESCE to a sentinel = default row. Tolerated-vs-forbidden is the owner's call — profiling says whether orphans exist today, not whether they're acceptable. Forbidden becomes an `empty` expectation; tolerated is pinned in the expected rows and recorded in the known-quirks list. +6. **Version history: effective/end/current columns anywhere? If not, confirm overwrite-everywhere.** (T1) Detect: column names. Absence means history is silently rewritten in rollups — state that consequence when confirming, not just the mechanism. +7. **Does this transform read another transform's output?** (A5) Detect: another transform's target table in this one's SQL. It becomes a declared input like any other, faked from that transform's own expected output — confirm which rows, and record the coupling in both plans. ## Stage 3 — per-column declarations -8. **Each measure: additive, semi-additive, or non-additive?** (A1) - Detect: sums and counts are additive; balances and levels are semi-additive; ratios, rates, and - unit prices are non-additive. Confirm the ambiguous ones (a "score"? a "quantity on hand"?). -9. **Legal bounds per measure?** (Q1) - Detect: profiling min/max suggests, business meaning decides — can `discount` be negative? can - `quantity` be zero? Derived bounds are free (a sum of non-negatives is non-negative). -10. **Each derived measure: the exact recomputation rule?** (A3) - Detect: the SELECT expression is the rule — but rounding, business-day adjustments, and NULL - handling are conventions to confirm, not read. -11. **Which columns are stored aggregates, over which detail and filter?** (A4) - Detect: correlated subqueries / joined-aggregate CTEs in the SQL. -12. **Which categorical columns have closed domains — enumerate; is NULL a member?** (C2) - Detect: profiling distinct values gives today's set; the owner confirms it's closed rather than - merely small so far. -13. **Which columns are denormalized copies, of which owning attribute?** (C1) - Detect: `_` naming; any column functionally dependent on a non-grain key. Every hit - must be classified: copy (C1), stored aggregate (A4), or smuggled coarser-grain fact — a - finding. -14. **Which many-to-one edges hold within one output table?** (C4) - Detect: hierarchy-shaped column pairs (product/category, zip/state). Applies where no owning - table is in scope; otherwise C1 covers it. -15. **Which date orderings are business-guaranteed vs. known-violated and tolerated (with real-data - counts)?** (T2) - Detect: profiling counts the violations that exist; the owner decides tolerated vs. bug. -16. **Null policy per column — measures, attributes, FKs separately.** (Q2) - Detect: the SQL shows what's produced (COALESCE, CASE); the owner confirms intent — "count of - nothing": 0 or NULL? empty date: NULL or sentinel? -17. **Which columns are volatile across runs?** (T3) - Detect: `now()` / `current_date` / run-metadata expressions in the SQL. A volatile column is left - out of the `equals` columns; its form can still be asserted with an `empty`. -18. **Per tolerated oddity: does it stay tolerated, and what is its current count?** (Q3) - Detect: can't — this is the conscious-choice question, and the plan's known-quirks list is where - it survives. +8. **Each measure: additive, semi-additive, or non-additive?** (A1) Detect: sums and counts are additive; balances and levels are semi-additive; ratios, rates, and unit prices are non-additive. Confirm the ambiguous ones (a "score"? a "quantity on hand"?). +9. **Legal bounds per measure?** (Q1) Detect: profiling min/max suggests, business meaning decides — can `discount` be negative? can `quantity` be zero? Derived bounds are free (a sum of non-negatives is non-negative). +10. **Each derived measure: the exact recomputation rule?** (A3) Detect: the SELECT expression is the rule — but rounding, business-day adjustments, and NULL handling are conventions to confirm, not read. +11. **Which columns are stored aggregates, over which detail and filter?** (A4) Detect: correlated subqueries / joined-aggregate CTEs in the SQL. +12. **Which categorical columns have closed domains — enumerate; is NULL a member?** (C2) Detect: profiling distinct values gives today's set; the owner confirms it's closed rather than merely small so far. +13. **Which columns are denormalized copies, of which owning attribute?** (C1) Detect: `_` naming; any column functionally dependent on a non-grain key. Every hit must be classified: copy (C1), stored aggregate (A4), or smuggled coarser-grain fact — a finding. +14. **Which many-to-one edges hold within one output table?** (C4) Detect: hierarchy-shaped column pairs (product/category, zip/state). Applies where no owning table is in scope; otherwise C1 covers it. +15. **Which date orderings are business-guaranteed vs. known-violated and tolerated (with real-data counts)?** (T2) Detect: profiling counts the violations that exist; the owner decides tolerated vs. bug. +16. **Null policy per column — measures, attributes, FKs separately.** (Q2) Detect: the SQL shows what's produced (COALESCE, CASE); the owner confirms intent — "count of nothing": 0 or NULL? empty date: NULL or sentinel? +17. **Which columns are volatile across runs?** (T3) Detect: `now()` / `current_date` / run-metadata expressions in the SQL. A volatile column is left out of the `equals` columns; its form can still be asserted with an `empty`. +18. **Per tolerated oddity: does it stay tolerated, and what is its current count?** (Q3) Detect: can't — this is the conscious-choice question, and the plan's known-quirks list is where it survives. diff --git a/packages/cli/skill-data/transform-test-plan/references/checks.md b/packages/cli/skill-data/transform-test-plan/references/checks.md index 154ca5b5..8714787a 100644 --- a/packages/cli/skill-data/transform-test-plan/references/checks.md +++ b/packages/cli/skill-data/transform-test-plan/references/checks.md @@ -1,263 +1,156 @@ # Check catalog -Every check the plan can derive, grouped by theme. Per entry: **when it applies** (the declared -property that switches it on — blank means always), **assert** (the expectation shape), and -**fixtures** (what the input cast must contain for the check to have teeth). +Every check the plan can derive, grouped by theme. Per entry: **when it applies** (the declared property that switches it on — blank means always), **assert** (the expectation shape), and **fixtures** (what the input cast must contain for the check to have teeth). -Expectation SQL names the transform's **target table** and its **declared input tables** under their -real names; both are rewritten to the run's temp tables, so per-row recomputation joins against -inputs are writable. Anything else you name is refused (`transform-test.unremapped-reference`), so no -check can read a real table. +Expectation SQL names the transform's **target table** and its **declared input tables** under their real names; both are rewritten to the run's temp tables, so per-row recomputation joins against inputs are writable. Anything else you name is refused (`transform-test.unremapped-reference`), so no check can read a real table. -Ids (G1, A2, …) are for cross-referencing within the plan documents only — never surface them to the -user bare. +Ids (G1, A2, …) are for cross-referencing within the plan documents only — never surface them to the user bare. -Throughout, `` stands for the transform's target table as written in the transform's own -definition, and `` for a declared input table. +Throughout, `` stands for the transform's target table as written in the transform's own definition, and `` for a declared input table. -`IS DISTINCT FROM` is the null-safe inequality; MySQL spells it `NOT (a <=> b)`, and where neither -parses, `a <> b OR (a IS NULL) <> (b IS NULL)`. +`IS DISTINCT FROM` is the null-safe inequality; MySQL spells it `NOT (a <=> b)`, and where neither parses, `a <> b OR (a IS NULL) <> (b IS NULL)`. ## Grain & keys -**G1 — Declared grain.** Every output table states "one row per X"; the declaration anchors every -other check. +**G1 — Declared grain.** Every output table states "one row per X"; the declaration anchors every other check. - Applies: always. Elicit or detect (GROUP BY keys; the join's driving table). -- Assert: nothing directly — G1 is the plan's opening move. A table with no statable grain, or a - grain stated inconsistently between docs and SQL, is a finding before any SQL runs. +- Assert: nothing directly — G1 is the plan's opening move. A table with no statable grain, or a grain stated inconsistently between docs and SQL, is a finding before any SQL runs. - Fixtures: the grain declaration decides the cast's row structure. -**G2 — Grain-key uniqueness.** The grain key stays a key; doubles as the fan-out guard for every -enriching join. +**G2 — Grain-key uniqueness.** The grain key stays a key; doubles as the fan-out guard for every enriching join. - Applies: always — one-big-table, rollups, and stars all have a grain. -- Assert (`empty`): `SELECT , COUNT(*) FROM GROUP BY HAVING -COUNT(*) > 1`. NULL grain values form their own bucket. +- Assert (`empty`): `SELECT , COUNT(*) FROM GROUP BY HAVING COUNT(*) > 1`. NULL grain values form their own bucket. - Fixtures: a parent with ≥2 children on every join (I4) is what makes this check able to fail. ## Additivity & reconciliation -**A1 — Measure additivity classification.** Additive / semi-additive (balances) / non-additive -(ratios, rates, unit prices) decides which aggregations are valid tests. +**A1 — Measure additivity classification.** Additive / semi-additive (balances) / non-additive (ratios, rates, unit prices) decides which aggregations are valid tests. - Applies: every numeric measure; per-column declaration. -- Assert: routes the measure — additive → A2; semi-additive → sum across non-time slices only (a - test summing a balance over time is a bug in the plan); non-additive → A3 recomputation, never - reconciled by summing. +- Assert: routes the measure — additive → A2; semi-additive → sum across non-time slices only (a test summing a balance over time is a bug in the plan); non-additive → A3 recomputation, never reconciled by summing. - Fixtures: none directly; one coverage-matrix axis (measure × valid aggregation set). -**A2 — Conservation reconciliation.** Row counts and additive-measure sums tie from input to output; -catches dropped rows and join double-counting at once. +**A2 — Conservation reconciliation.** Row counts and additive-measure sums tie from input to output; catches dropped rows and join double-counting at once. - Applies: per declared tie (which input, which declared exclusions). -- Assert (`empty`): independent scalar subqueries compared with `IS DISTINCT FROM`, returning the - mismatched pair: - `SELECT (SELECT SUM(t.m) FROM t) AS output_sum, (SELECT SUM(s.m) FROM s) AS -input_sum WHERE (…) IS DISTINCT FROM (…)`. - Never reconcile via a fact-to-fact join — cardinality is uncontrollable and wrong results are - silent. +- Assert (`empty`): independent scalar subqueries compared with `IS DISTINCT FROM`, returning the mismatched pair: `SELECT (SELECT SUM(t.m) FROM t) AS output_sum, (SELECT SUM(s.m) FROM s) AS input_sum WHERE (…) IS DISTINCT FROM (…)`. Never reconcile via a fact-to-fact join — cardinality is uncontrollable and wrong results are silent. - Fixtures: amounts chosen so partial survival is visible (distinct values, odd cents). -**A3 — Derived-measure recomputation.** Averages, lags, ratios, rounded presentations recompute from -their components per row. +**A3 — Derived-measure recomputation.** Averages, lags, ratios, rounded presentations recompute from their components per row. -- Applies: every derived column, with its declared rule (rounding, business-day adjustment, NULL - handling). -- Assert (`empty`): `SELECT FROM t WHERE t. IS DISTINCT FROM `. +- Applies: every derived column, with its declared rule (rounding, business-day adjustment, NULL handling). +- Assert (`empty`): `SELECT FROM t WHERE t. IS DISTINCT FROM `. - Fixtures: component values whose derivation is non-trivial (a NULL in the AVG, a negative lag). -**A4 — Stored aggregates on entity tables.** Lifetime/rollup stats carried on an entity-grain table -(`lifetime_orders`, `review_count`, `first_order_date`) equal recomputation from detail — per row -_and_ in total. +**A4 — Stored aggregates on entity tables.** Lifetime/rollup stats carried on an entity-grain table (`lifetime_orders`, `review_count`, `first_order_date`) equal recomputation from detail — per row _and_ in total. - Applies: columns detected as correlated aggregates over a detail table. -- Assert (`empty`), two per column: per-row — `WHERE t. IS DISTINCT FROM (SELECT -COUNT(*)/SUM(…)/MIN(…) FROM d WHERE d. = t.)`; total — the A2 scalar-pair shape. - Totals alone cancel offsetting per-row errors; the per-row form is the one that catches them. +- Assert (`empty`), two per column: per-row — `WHERE t. IS DISTINCT FROM (SELECT COUNT(*)/SUM(…)/MIN(…) FROM d WHERE d. = t.)`; total — the A2 scalar-pair shape. Totals alone cancel offsetting per-row errors; the per-row form is the one that catches them. - Fixtures: an entity with several detail rows and an entity with none (I3/I4). **A5 — Rollup consistency.** A layered rollup always agrees with its base. -- Applies: any transform reading another transform's output. The base is a declared input like any - other, faked with rows taken from the base transform's own expected output. -- Assert (`empty`): every rollup measure equals the corresponding aggregation over the base (A2 - shape); group-set equality both directions — `SELECT FROM EXCEPT SELECT DISTINCT - FROM ` and the reverse, filtered by the declared zero-group policy. +- Applies: any transform reading another transform's output. The base is a declared input like any other, faked with rows taken from the base transform's own expected output. +- Assert (`empty`): every rollup measure equals the corresponding aggregation over the base (A2 shape); group-set equality both directions — `SELECT FROM EXCEPT SELECT DISTINCT FROM ` and the reverse, filtered by the declared zero-group policy. - Fixtures: a group with no contributing rows pins the zero-group policy. ## Fact-table type -**F2 — Fact-table type bundle.** Transaction / periodic snapshot / accumulating snapshot each carry -a distinct invariant set. - -- Applies: per fact-shaped output, always classified; **transaction** needs nothing beyond G2 + A2 - (sparsity is legitimate). -- Periodic snapshot, if declared _dense_: `COUNT(*) = |entities| × |periods|` (or the declared - subset); per-entity gap detection in the period series; inactive-period representation (zero vs - NULL) pinned in the expected rows. If sparse, density checks off — the one yes/no flips the whole - set. -- Accumulating snapshot: milestone dates monotone in pipeline order where set (`WHERE < -` per adjacent pair); unset-milestone default (NULL vs sentinel) pinned in the expected - rows; completion flags ∈ {0,1} and consistent with their date's set-ness; lags via A3. +**F2 — Fact-table type bundle.** Transaction / periodic snapshot / accumulating snapshot each carry a distinct invariant set. + +- Applies: per fact-shaped output, always classified; **transaction** needs nothing beyond G2 + A2 (sparsity is legitimate). +- Periodic snapshot, if declared _dense_: `COUNT(*) = |entities| × |periods|` (or the declared subset); per-entity gap detection in the period series; inactive-period representation (zero vs NULL) pinned in the expected rows. If sparse, density checks off — the one yes/no flips the whole set. +- Accumulating snapshot: milestone dates monotone in pipeline order where set (`WHERE < ` per adjacent pair); unset-milestone default (NULL vs sentinel) pinned in the expected rows; completion flags ∈ {0,1} and consistent with their date's set-ness; lags via A3. - Fixtures (accumulating): occurrences at every completion stage — none, some, all milestones. ## Conformance & domains -**C1 — Denormalized-copy agreement.** Every copied attribute agrees with the owning table's value -for that key. - -- Applies: columns declared as copies (detect: `_` naming; any column functionally - dependent on a non-grain key). Each such column must be a declared copy (this check), a stored - aggregate (A4), or it's a smuggled coarser-grain fact — a finding: it double-counts under - summation. -- Assert (`empty`): `SELECT t., t., d. FROM t JOIN d ON -t. = d. WHERE t. IS DISTINCT FROM d.`. -- Scope: in a test, the copies are produced by the very join under test, so this join-form is near- - tautological — the C4 functional-dependency form plus expected-row cell pinning carries the test. - The join-form against an independently materialized owning table is the _drift_ check, which - belongs to whatever screens the deployed tables. +**C1 — Denormalized-copy agreement.** Every copied attribute agrees with the owning table's value for that key. + +- Applies: columns declared as copies (detect: `_` naming; any column functionally dependent on a non-grain key). Each such column must be a declared copy (this check), a stored aggregate (A4), or it's a smuggled coarser-grain fact — a finding: it double-counts under summation. +- Assert (`empty`): `SELECT t., t., d. FROM t JOIN d ON t. = d. WHERE t. IS DISTINCT FROM d.`. +- Scope: in a test, the copies are produced by the very join under test, so this join-form is near- tautological — the C4 functional-dependency form plus expected-row cell pinning carries the test. The join-form against an independently materialized owning table is the _drift_ check, which belongs to whatever screens the deployed tables. - Fixtures: copies with distinct values per entity so a crossed join is visible. **C2 — Closed-domain screen.** A categorical column's values stay inside the declared enumeration. -- Applies: per column declared closed (detect from profiling; confirm the set and whether NULL is a - member). -- Assert (`empty`): `SELECT , FROM WHERE IS NOT NULL AND NOT IN -()`. -- Fixtures: every domain value represented where practical; one out-of-domain input row if the - source can produce one (I5). - -**C3 — Referential integrity / orphan policy.** Every FK resolves, or the declared orphan handling is -pinned. - -- Applies: per join, conditioned on the declared response — drop / keep-with-NULLs / default row; - tolerated or forbidden. -- Assert, forbidden (`empty`): `SELECT t. FROM t LEFT JOIN d ON t. = -d. WHERE t. IS NOT NULL AND d. IS NULL`. - Tolerated: not an expectation — pin the orphan's pass-through as a row in the `equals` (NULL in - the copied attributes), and record the tolerance with its warehouse count in the plan. - Default-row convention adds: distinct unknown keys must not collapse into one output row. -- Fixtures: one orphan row (I5). Without it the join direction is untested — an inner join silently - dropping unmatched rows is the classic bug this catches. - -**C4 — Many-to-one consistency.** Each declared many-to-one edge holds within the output (product → -one category; zip → one state). - -- Applies: per declared edge; where an owning table is in scope, C1 subsumes it — this is the - one-big-table variant with nothing to join against. -- Assert (`empty`): `SELECT t., COUNT(DISTINCT t.) FROM t GROUP BY t. -HAVING COUNT(DISTINCT t.) > 1`. -- Fixtures: a violating input row if the source can produce one (I5), pinning the transform's - behavior on dirty input. +- Applies: per column declared closed (detect from profiling; confirm the set and whether NULL is a member). +- Assert (`empty`): `SELECT , FROM WHERE IS NOT NULL AND NOT IN ()`. +- Fixtures: every domain value represented where practical; one out-of-domain input row if the source can produce one (I5). + +**C3 — Referential integrity / orphan policy.** Every FK resolves, or the declared orphan handling is pinned. + +- Applies: per join, conditioned on the declared response — drop / keep-with-NULLs / default row; tolerated or forbidden. +- Assert, forbidden (`empty`): `SELECT t. FROM t LEFT JOIN d ON t. = d. WHERE t. IS NOT NULL AND d. IS NULL`. Tolerated: not an expectation — pin the orphan's pass-through as a row in the `equals` (NULL in the copied attributes), and record the tolerance with its warehouse count in the plan. Default-row convention adds: distinct unknown keys must not collapse into one output row. +- Fixtures: one orphan row (I5). Without it the join direction is untested — an inner join silently dropping unmatched rows is the classic bug this catches. + +**C4 — Many-to-one consistency.** Each declared many-to-one edge holds within the output (product → one category; zip → one state). + +- Applies: per declared edge; where an owning table is in scope, C1 subsumes it — this is the one-big-table variant with nothing to join against. +- Assert (`empty`): `SELECT t., COUNT(DISTINCT t.) FROM t GROUP BY t. HAVING COUNT(DISTINCT t.) > 1`. +- Fixtures: a violating input row if the source can produce one (I5), pinning the transform's behavior on dirty input. ## Temporal & version history -**T1 — Change handling per attribute.** How the model treats a changed source attribute decides the -temporal fixtures. +**T1 — Change handling per attribute.** How the model treats a changed source attribute decides the temporal fixtures. - Applies: detect effective/end/current housekeeping columns. -- Present → version-history battery (`empty` each): per durable key exactly one current row; - `effective < end` per row; intervals contiguous and non-overlapping; current row's end = the - declared far-future default; fact rows join the version whose interval contains the fact date. -- Absent → confirm overwrite-everywhere as a stated assumption (history is silently rewritten in - rollups), and derive the propagation probe: change an attribute in an input, run, assert the - output regrouped. -- Fixtures: a before/after change pair; for version history, an entity with ≥2 versions and a fact - row dated inside each interval. +- Present → version-history battery (`empty` each): per durable key exactly one current row; `effective < end` per row; intervals contiguous and non-overlapping; current row's end = the declared far-future default; fact rows join the version whose interval contains the fact date. +- Absent → confirm overwrite-everywhere as a stated assumption (history is silently rewritten in rollups), and derive the propagation probe: change an attribute in an input, run, assert the output regrouped. +- Fixtures: a before/after change pair; for version history, an entity with ≥2 versions and a fact row dated inside each interval. **T2 — Date-pair ordering.** Business-guaranteed orderings asserted; known violations recorded. -- Applies: per declared date pair (ordered ≤ shipped ≤ delivered; signup ≤ first order), each - classified guaranteed vs. tolerated-violated. -- Assert: guaranteed (`empty`) — `SELECT , , FROM WHERE < -`. Tolerated — pin the violating row's downstream arithmetic (a negative lag inside an - average) in the expected rows, and record the real-data count in the plan. +- Applies: per declared date pair (ordered ≤ shipped ≤ delivered; signup ≤ first order), each classified guaranteed vs. tolerated-violated. +- Assert: guaranteed (`empty`) — `SELECT , , FROM WHERE < `. Tolerated — pin the violating row's downstream arithmetic (a negative lag inside an average) in the expected rows, and record the real-data count in the plan. - Fixtures: one violating row for every tolerated ordering. **T3 — Volatile columns.** Values that change across runs can't be pinned. -- Applies: columns derived from `now()`/`current_date` (age), run metadata (load timestamps, batch - ids). -- Assert: leave the column out of the `equals` `columns` list — an undeclared column never enters the - comparison. Separately assert its form where warranted (`empty`): `WHERE age NOT BETWEEN 0 AND -120`. The stable source column (`birth_date`) stays exact in the expected rows. +- Applies: columns derived from `now()`/`current_date` (age), run metadata (load timestamps, batch ids). +- Assert: leave the column out of the `equals` `columns` list — an undeclared column never enters the comparison. Separately assert its form where warranted (`empty`): `WHERE age NOT BETWEEN 0 AND 120`. The stable source column (`birth_date`) stays exact in the expected rows. - Fixtures: none special; the split is the point — omit the volatile, pin the stable. ## Screens & severity **Q1 — Range / sign screens.** Each measure's declared bounds hold. -- Applies: per bounded measure; bounds from business meaning plus profiling (derived bounds are - free: a sum of non-negatives is non-negative). -- Assert (`empty`): `SELECT , FROM WHERE < OR > ` (one-sided - where only one bound exists). +- Applies: per bounded measure; bounds from business meaning plus profiling (derived bounds are free: a sum of non-negatives is non-negative). +- Assert (`empty`): `SELECT , FROM WHERE < OR > ` (one-sided where only one bound exists). - Fixtures: boundary values where the bound is business-set. -**Q2 — Null policy, three ways.** Measures, descriptive attributes, and FKs carry independent null -policies; never presume one from another. +**Q2 — Null policy, three ways.** Measures, descriptive attributes, and FKs carry independent null policies; never presume one from another. -- Applies: per column, asked separately — "count of nothing": 0 or NULL? empty date: NULL or - sentinel? FK: see C3. -- Assert: declared non-null columns get `WHERE IS NULL` (`empty`). Otherwise the policy is - enforced by the expected-row cell of a zero-case fixture row — NULL vs 0 vs empty is invisible - until a fixture forces the choice into a cell. +- Applies: per column, asked separately — "count of nothing": 0 or NULL? empty date: NULL or sentinel? FK: see C3. +- Assert: declared non-null columns get `WHERE IS NULL` (`empty`). Otherwise the policy is enforced by the expected-row cell of a zero-case fixture row — NULL vs 0 vs empty is invisible until a fixture forces the choice into a cell. - Fixtures: the zero-case row (I3) is the enforcement mechanism. **Q3 — Naming & the tolerated oddity.** Every expectation declares its meaning. - Applies: always, every expectation. -- Convention: the expectation's `name` states the invariant in plain words, because a failure leads - with it. Each `empty` SQL opens with 1–3 comment lines naming the invariant and the failure modes - it catches. Budget cuts drop business-rule checks first, then cross-table structure checks, never - single-column screens. -- A forbidden condition is an `empty` expectation. A tolerated one is pinned in the expected rows - and recorded in the plan's known-quirks list with its warehouse count. Never author an expectation - designed to fire. +- Convention: the expectation's `name` states the invariant in plain words, because a failure leads with it. Each `empty` SQL opens with 1–3 comment lines naming the invariant and the failure modes it catches. Budget cuts drop business-rule checks first, then cross-table structure checks, never single-column screens. +- A forbidden condition is an `empty` expectation. A tolerated one is pinned in the expected rows and recorded in the plan's known-quirks list with its warehouse count. Never author an expectation designed to fire. ## Input modeling (fixture-design rules) -**I1 — Profiling-derived partitions.** Fixture edges and tolerated non-ties cite profiled reality -with counts; the plan's known-quirks section carries each tolerated oddity with its reason and -count. +**I1 — Profiling-derived partitions.** Fixture edges and tolerated non-ties cite profiled reality with counts; the plan's known-quirks section carries each tolerated oddity with its reason and count. -**I2 — Shared fixture cast.** One small human-named cast per transform, every row a named edge, -documented as a story table (row → attributes → purpose) in the plan. Where one transform's output -is another's input, derive the faked rows from the first's expected output so the two tests tell one -story. +**I2 — Shared fixture cast.** One small human-named cast per transform, every row a named edge, documented as a story table (row → attributes → purpose) in the plan. Where one transform's output is another's input, derive the faked rows from the first's expected output so the two tests tell one story. -**I3 — Zero-case partitions.** For every outer join and aggregation relation: one entity with zero -matches. Its expected row pins the absence representation (Q2) and the join direction (C3) — the -all-clean cast is the single most common cause of vacuous tests. When the schema distinguishes -states the profiled data never exhibits (a literal zero in a source that only has NULLs and -positives), fixture the missing state — nothing else forces it onto an expected cell. +**I3 — Zero-case partitions.** For every outer join and aggregation relation: one entity with zero matches. Its expected row pins the absence representation (Q2) and the join direction (C3) — the all-clean cast is the single most common cause of vacuous tests. When the schema distinguishes states the profiled data never exhibits (a literal zero in a source that only has NULLs and positives), fixture the missing state — nothing else forces it onto an expected cell. -**I4 — Multiplicity partitions.** Every aggregation gets a >1-member group and an exactly-1 group -(the 0 case is I3); every join gets a parent with ≥2 children. Expected values then differ from any -single row's, so copy-through bugs can't pass. +**I4 — Multiplicity partitions.** Every aggregation gets a >1-member group and an exactly-1 group (the 0 case is I3); every join gets a parent with ≥2 children. Expected values then differ from any single row's, so copy-through bugs can't pass. -**I5 — Dirty-input pinning.** Per screenable defect the source can carry (orphan FK, out-of-domain -value, duplicate natural key, hierarchy violation, ordering violation): one fixture row exhibiting -it, an expected row pinning the transform's response (drop / pass-through / default), and — where -the defect is forbidden — the matching `empty` expectation. Which duplicate survives a dedupe is -invisible until a conflicting-duplicate fixture pins it. +**I5 — Dirty-input pinning.** Per screenable defect the source can carry (orphan FK, out-of-domain value, duplicate natural key, hierarchy violation, ordering violation): one fixture row exhibiting it, an expected row pinning the transform's response (drop / pass-through / default), and — where the defect is forbidden — the matching `empty` expectation. Which duplicate survives a dedupe is invisible until a conflicting-duplicate fixture pins it. -**I6 — Mutation probes.** Run at plan-validation time, not on every run: corrupt one expected cell → -`cell-mismatches` must name that exact column; perturb one input cell → exactly the declared output -cells move. Both guard against vacuous tests: every `empty` expectation passes against an empty -output. +**I6 — Mutation probes.** Run at plan-validation time, not on every run: corrupt one expected cell → `cell-mismatches` must name that exact column; perturb one input cell → exactly the declared output cells move. Both guard against vacuous tests: an `empty` expectation that only selects output rows passes against an empty output. ## Plan-level artifacts -**P1 — Coverage matrix.** Output columns (with the table's grain) × check classes; cells name the -covering expectation or expected-row cell, or state `gap: `; shared-attribute columns scanned -across transforms for agreement obligations (C1). +**P1 — Coverage matrix.** Output columns (with the table's grain) × check classes; cells name the covering expectation or expected-row cell, or state `gap: `; shared-attribute columns scanned across transforms for agreement obligations (C1). -**P2 — Hand-derived expected rows.** From the fixture story and business meaning, arithmetic recorded -in the plan; never captured from output; every fixture row's fate appears in some expected cell. +**P2 — Hand-derived expected rows.** From the fixture story and business meaning, arithmetic recorded in the plan; never captured from output; every fixture row's fate appears in some expected cell. -**P3 — Live-bug handling.** A check failing against a deployed transform is a real finding: never -soften the test to green; quantify the damage at fixture and warehouse scale. Then fix now, or record -the red in the plan with the minimal fix body, per the session's terms. A stored test has no -red-by-design state, so a deferred fix must be visible in the plan or the test reads as broken. +**P3 — Live-bug handling.** A check failing against a deployed transform is a real finding: never soften the test to green; quantify the damage at fixture and warehouse scale. Then fix now, or record the red in the plan with the minimal fix body, per the session's terms. A stored test has no red-by-design state, so a deferred fix must be visible in the plan or the test reads as broken. diff --git a/packages/cli/skill-data/transform/SKILL.md b/packages/cli/skill-data/transform/SKILL.md index 40c38646..bf8d9662 100644 --- a/packages/cli/skill-data/transform/SKILL.md +++ b/packages/cli/skill-data/transform/SKILL.md @@ -180,7 +180,7 @@ If you really must `create + delete` instead, do the `delete` **before** the fir A transform test replaces every table the transform reads with a fixture, runs it into a temp table, and checks that output. The transform and the expectations read only the run's temp tables; a `format: "sql"` input runs verbatim against the transform's source database, so it may read real tables. Only query transforms (native SQL or MBQL) can be tested, and only on Postgres, MySQL, H2, ClickHouse, Redshift or SQL Server; otherwise `create`/`update`/`run` refuse with `transform-test.unsupported-transform` / `unsupported-driver` (422). ```bash -mb transform-test list --transform --profile --json # --transform is optional +mb transform-test list --transform-id --profile --json # --transform-id is optional mb transform-test get --full --profile --json # --full for inputs/expectations mb transform-test create --file ./.scratch/test.json --profile --json mb transform-test update --file ./.scratch/patch.json --profile --json @@ -192,7 +192,7 @@ mb transform-test run --profile --json # exits 1 unless i **`cast_type` is a `CAST` target, not a column type**, and the two vocabularies differ per warehouse: MySQL casts to `SIGNED` and reports `BIGINT`; ClickHouse takes `Nullable(Int32)` for a column that is `Int64`. So a body is warehouse-specific — don't copy a `database_type` out of a run result into a `cast_type`, and don't expect one body to run everywhere. -**`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly; an `equals` without a `format` is refused. +**`expectations`** — `type: "empty"` with the `sql` that must return no rows, or `type: "equals"` with `format: "rows"` and the `columns` and `rows` the output must hold exactly. `equals` compares the rows as a multiset over the declared columns only, so an output column it does not declare is never checked. An `equals` without a `format` is refused, and one with `format: "sql"` is saved but reports `error` on every run, so its test can never pass. **Expectation SQL may only name the transform's target table and its declared input tables.** Those are rewritten to the run's temp tables; any other table is refused with `transform-test.unremapped-reference` (400), at `create`/`update` as well as `run`. A column qualified by its table name (`people.id`) is refused the same way, in expectation SQL and in the transform's own SQL alike: qualify by an alias (`FROM people p … p.id`) or leave the column bare — for the transform's SQL that means editing the transform. Expectation names are unique within a test (refused with "expectation names must be unique within a test"), and every `rows` row carries exactly its declared columns (refused naming the missing or undeclared columns). From 5a254f7d4737be01cf3bb17aba4a15cd09ae3a29 Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Fri, 2 Oct 2026 15:42:07 -0400 Subject: [PATCH 18/24] docs(skills): keep core under the default output cap Restoring main's core text left no room for the transform-test quirk, so core opened `skills get --all --json` on an empty window. The quirk goes (the transform skill carries it) and the transform-test-plan entry shrinks to one phrase. --- packages/cli/skill-data/core/SKILL.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/cli/skill-data/core/SKILL.md b/packages/cli/skill-data/core/SKILL.md index 0390c9eb..940974d7 100644 --- a/packages/cli/skill-data/core/SKILL.md +++ b/packages/cli/skill-data/core/SKILL.md @@ -148,9 +148,6 @@ Routine verb shapes (list / get / create / update), every flag, and output schem - **search vs. list.** Enumerate with `… list` verbs; `search` ranks a query or spans resources. `--search-native-query` matches native query text and keeps only query-bearing models; dashboard questions need `--include-dashboard-questions`. - **transform.** Iterate with `transform update `, never `delete` + `create` (keeps the row, `entity_id`, materialized table, and YAML filename — avoids `_2` suffixes and noisy git history). `transform run` needs `--wait` (or `--sync`, which also waits for the output table to register and returns `target_table_id`) or you get only `{run_id, final:null}`. (→ `transform`.) - - -- **transform-test.** `transform-test run ` checks a transform against fixtures in temp tables — only a `format: "sql"` input reads the source database — and exits 1 unless it passes. (→ `transform`.) - **setup is one-shot.** `mb setup` walks `/api/setup` for a **fresh** instance only — errors against an already-configured one. Mostly for bootstrapping local / e2e instances. - **eid** translates a string entity id → numeric id: `mb eid --model --json`. Entity ids are NanoIDs that can start with `-`, which the positional form misreads as a flag (shell quotes don't help) — for those, use `--body '{"entity_ids":{"card":["-…"]}}'` (the id is a JSON string value, immune to flag parsing). @@ -176,7 +173,7 @@ This file is enough for any single-command task. For anything deeper, load the r - **`transform`** — transform body JSON, create + run-with-wait, run inspection, tags, jobs. -- **`transform-test-plan`** — deciding _what_ to test in a transform: the fixture cast, the expectations, the coverage matrix. (`transform` has the `mb transform-test` shapes.) +- **`transform-test-plan`** — planning transform tests. - **`document`** — Metabase documents (TipTap body, embedding cards). From efb54aa2b5f5dc188d331d1ce3bc9386a1275690 Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Fri, 2 Oct 2026 15:48:29 -0400 Subject: [PATCH 19/24] test: drop the skills e2e suite It never reached a server: the v58 cases ran the binary against a seeded probe, re-checking the filtering the unit tier already covers, and it hard-coded the shipped skill list, so every new skill broke it. core/skills.test.ts loads and pins the shipped skills, and get.test.ts holds core under the default cap. SkillPathListEnvelope had no other importer and is no longer exported. --- packages/cli/src/commands/skills/path.ts | 2 +- tests/e2e/skills.e2e.test.ts | 389 ----------------------- 2 files changed, 1 insertion(+), 390 deletions(-) delete mode 100644 tests/e2e/skills.e2e.test.ts diff --git a/packages/cli/src/commands/skills/path.ts b/packages/cli/src/commands/skills/path.ts index 014321a5..db070cda 100644 --- a/packages/cli/src/commands/skills/path.ts +++ b/packages/cli/src/commands/skills/path.ts @@ -20,7 +20,7 @@ const SkillPath = z.object({ }); type SkillPathJson = z.infer; -export const SkillPathListEnvelope = listEnvelopeSchema(SkillPath); +const SkillPathListEnvelope = listEnvelopeSchema(SkillPath); const skillPathView: ResourceView = { compactPick: SkillPath, diff --git a/tests/e2e/skills.e2e.test.ts b/tests/e2e/skills.e2e.test.ts deleted file mode 100644 index 704a6368..00000000 --- a/tests/e2e/skills.e2e.test.ts +++ /dev/null @@ -1,389 +0,0 @@ -import { readFileSync } from "node:fs"; -import { join, resolve } from "node:path"; - -import { afterEach, describe, expect, it } from "vitest"; -import type { z } from "zod"; - -import { parseJson } from "@metabase/client/json"; - -import { SkillGetEnvelope } from "../../packages/cli/src/commands/skills/get"; -import { SkillListEnvelope } from "../../packages/cli/src/commands/skills/list"; -import { SkillPathListEnvelope } from "../../packages/cli/src/commands/skills/path"; -import { - discoverSkills, - type SkillContent, - type SkillInfo, -} from "../../packages/cli/src/core/skills"; -import { cliErrorMessage } from "./cli-error"; -import { cleanupConfigHome, mkTempConfigHome, runCli } from "./run-cli"; -import { seedProbedProfile } from "./seed-profile"; - -// The shipped skills are the expected values: the binary under test bundles this very directory. -const SKILL_DATA_DIR = resolve(import.meta.dirname, "..", "..", "packages", "cli", "skill-data"); -const BUNDLED_VISIBLE = discoverSkills([SKILL_DATA_DIR]).filter((skill) => !skill.hidden); - -const BUNDLED_VISIBLE_NAMES = [ - "core", - "dashboard", - "data-action", - "data-workflow", - "document", - "git-sync", - "mbql", - "metadata", - "native-sql", - "notification", - "transform", - "visualization", -] as const; - -const UNFILTERED_NOTE = - 'Skills are unfiltered: there is no profile "default" (run `mb auth login` to create one and record its server).'; -const SKILL_OVERSIZE_HINT = - "a skill body is indivisible — pass --max-bytes 0 to print it whole, or `mb skills path ` to read it from disk"; - -function bundledSkill(name: string): SkillInfo { - const skill = BUNDLED_VISIBLE.find((candidate) => candidate.name === name); - if (skill === undefined) { - throw new Error(`no bundled skill named "${name}" under ${SKILL_DATA_DIR}`); - } - return skill; -} - -function listRow(skill: SkillInfo): z.infer["data"][number] { - return { name: skill.name, description: skill.description }; -} - -// A skill as `skills get` prints it with nothing to resolve against: SKILL.md verbatim, no extras. -function asWritten(skill: SkillInfo): SkillContent { - return { - name: skill.name, - description: skill.description, - body: readFileSync(join(skill.dir, "SKILL.md"), "utf8"), - references: [], - templates: [], - }; -} - -function fullList(rows: readonly SkillInfo[]): z.infer { - return { - data: rows.map(listRow), - returned: rows.length, - offset: 0, - total: rows.length, - has_more: false, - next_offset: null, - unavailable: null, - }; -} - -// The names a text listing prints at column zero; descriptions sit indented beneath them. -function namesInTextListing(stdout: string): string[] { - return stdout.split("\n").filter((line) => line !== "" && !line.startsWith(" ")); -} - -const TRANSFORM_UNAVAILABLE_ON_58 = { - name: "transform", - failure: { - reason: "version-too-old", - detail: - "This operation requires Metabase v59+ (this server is v0.58.0). Upgrade Metabase to use it.", - feature: "transforms", - since: 59, - tokenFeature: null, - serverVersion: "v0.58.0", - }, -}; - -const DATA_ACTION_UNAVAILABLE_ON_58 = { - name: "data-action", - failure: { - reason: "version-too-old", - detail: - "This operation requires Metabase v65+ (this server is v0.58.0). Upgrade Metabase to use it.", - feature: "dataActionsWithoutModel", - since: 65, - tokenFeature: null, - serverVersion: "v0.58.0", - }, -}; - -const GIT_SYNC_UNAVAILABLE_ON_58 = { - name: "git-sync", - failure: { - reason: "version-too-old", - detail: - "This operation requires Metabase v60+ (this server is v0.58.0). Upgrade Metabase to use it.", - feature: "remoteSync", - since: 60, - tokenFeature: "remote_sync", - serverVersion: "v0.58.0", - }, -}; - -const FEATURE_BOUND_ON_58 = new Set(["data-action", "git-sync", "transform"]); - -describe("skills e2e", () => { - const tempDirs: string[] = []; - - afterEach(async () => { - await Promise.all(tempDirs.splice(0).map(cleanupConfigHome)); - }); - - async function makeIsolatedConfigHome(): Promise { - const dir = await mkTempConfigHome(); - tempDirs.push(dir); - return dir; - } - - it("ships exactly the twelve visible skills this suite names", () => { - expect(BUNDLED_VISIBLE.map((skill) => skill.name)).toEqual([...BUNDLED_VISIBLE_NAMES]); - }); - - it("list returns the twelve bundled non-hidden skills, sorted by name, with `unavailable: null` and a clean stderr when there is no cached probe", async () => { - const result = await runCli({ - args: ["skills", "list", "--json"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillListEnvelope)).toEqual(fullList(BUNDLED_VISIBLE)); - expect(result.stderr).toBe(""); - }); - - it("list in text mode notes that nothing was filtered when there is no cached probe", async () => { - const result = await runCli({ - args: ["skills", "list", "--format", "text"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode).toBe(0); - expect(namesInTextListing(result.stdout)).toEqual([...BUNDLED_VISIBLE_NAMES]); - expect(result.stderr).toBe(UNFILTERED_NOTE); - }); - - it("list against a v58 profile omits the feature-bound skills and reports them under `unavailable`", async () => { - const configHome = await makeIsolatedConfigHome(); - await seedProbedProfile(configHome, 58); - - const result = await runCli({ args: ["skills", "list", "--json"], configHome }); - - expect(result.exitCode, result.stderr).toBe(0); - const usable = BUNDLED_VISIBLE.filter((skill) => !FEATURE_BOUND_ON_58.has(skill.name)); - expect(parseJson(result.stdout, SkillListEnvelope)).toEqual({ - ...fullList(usable), - unavailable: [ - DATA_ACTION_UNAVAILABLE_ON_58, - GIT_SYNC_UNAVAILABLE_ON_58, - TRANSFORM_UNAVAILABLE_ON_58, - ], - }); - expect(result.stderr).toBe(""); - }); - - it("list --unfiltered against a v58 profile lists every skill and reports `unavailable: null`", async () => { - const configHome = await makeIsolatedConfigHome(); - await seedProbedProfile(configHome, 58); - - const result = await runCli({ - args: ["skills", "list", "--unfiltered", "--json"], - configHome, - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillListEnvelope)).toEqual(fullList(BUNDLED_VISIBLE)); - expect(result.stderr).toBe(""); - }); - - it("list in text mode against a v58 profile names each skipped skill on stderr", async () => { - const configHome = await makeIsolatedConfigHome(); - await seedProbedProfile(configHome, 58); - - const result = await runCli({ args: ["skills", "list", "--format", "text"], configHome }); - - expect(result.exitCode).toBe(0); - expect(namesInTextListing(result.stdout)).toEqual( - BUNDLED_VISIBLE_NAMES.filter((name) => !FEATURE_BOUND_ON_58.has(name)), - ); - expect(result.stderr).toBe( - [ - `Skipped skill "data-action": ${DATA_ACTION_UNAVAILABLE_ON_58.failure.detail} Pass --unfiltered to print it anyway.`, - `Skipped skill "git-sync": ${GIT_SYNC_UNAVAILABLE_ON_58.failure.detail} Pass --unfiltered to print it anyway.`, - `Skipped skill "transform": ${TRANSFORM_UNAVAILABLE_ON_58.failure.detail} Pass --unfiltered to print it anyway.`, - ].join("\n"), - ); - }); - - it("get transform against a v58 profile withholds the body, and --unfiltered prints it", async () => { - const configHome = await makeIsolatedConfigHome(); - await seedProbedProfile(configHome, 58); - - const withheld = await runCli({ args: ["skills", "get", "transform", "--json"], configHome }); - expect(withheld.exitCode, withheld.stderr).toBe(0); - expect(parseJson(withheld.stdout, SkillGetEnvelope)).toEqual({ - data: [], - returned: 0, - offset: 0, - total: 0, - has_more: false, - next_offset: null, - unavailable: [TRANSFORM_UNAVAILABLE_ON_58], - }); - - const printed = await runCli({ - args: ["skills", "get", "transform", "--unfiltered", "--json", "--max-bytes", "0"], - configHome, - }); - expect(printed.exitCode, printed.stderr).toBe(0); - expect(parseJson(printed.stdout, SkillGetEnvelope)).toEqual({ - data: [asWritten(bundledSkill("transform"))], - returned: 1, - offset: 0, - total: 1, - has_more: false, - next_offset: null, - unavailable: null, - }); - }); - - it("get core returns the SKILL.md body with frontmatter intact and no references unless --full", async () => { - const result = await runCli({ - args: ["skills", "get", "core", "--json"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillGetEnvelope)).toEqual({ - data: [asWritten(bundledSkill("core"))], - returned: 1, - offset: 0, - total: 1, - has_more: false, - next_offset: null, - unavailable: null, - }); - }); - - it("get --all returns every non-hidden skill (with --max-bytes 0 to opt out of the list cap)", async () => { - const result = await runCli({ - args: ["skills", "get", "--all", "--json", "--max-bytes", "0"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillGetEnvelope)).toEqual({ - data: BUNDLED_VISIBLE.map(asWritten), - returned: BUNDLED_VISIBLE.length, - offset: 0, - total: BUNDLED_VISIBLE.length, - has_more: false, - next_offset: null, - unavailable: null, - }); - expect(result.stderr).toBe(""); - }); - - it("get answers a cap too small for even one skill with an empty window and no resumption point", async () => { - const result = await runCli({ - args: ["skills", "get", "core", "--json", "--max-bytes", "200"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - // What the answer would have measured is the answer itself, uncapped: the notice quotes it so - // a caller can size the cap that would carry it. - const uncapped = await runCli({ - args: ["skills", "get", "core", "--json", "--max-bytes", "0"], - configHome: await makeIsolatedConfigHome(), - }); - const fullBytes = Buffer.byteLength(uncapped.stdout.trimEnd(), "utf8"); - - expect(parseJson(result.stdout, SkillGetEnvelope)).toEqual({ - data: [], - returned: 0, - offset: 0, - total: 1, - has_more: true, - next_offset: null, - truncated: { reason: "max_bytes", bytes: fullBytes }, - unavailable: null, - }); - expect(result.stderr).toBe(`… cut at ${fullBytes} bytes; ${SKILL_OVERSIZE_HINT}`); - }); - - it("get accepts comma-separated names", async () => { - const result = await runCli({ - args: ["skills", "get", "git-sync,transform", "--json", "--max-bytes", "0"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillGetEnvelope)).toEqual({ - data: [asWritten(bundledSkill("git-sync")), asWritten(bundledSkill("transform"))], - returned: 2, - offset: 0, - total: 2, - has_more: false, - next_offset: null, - unavailable: null, - }); - }); - - it("get rejects an unknown skill name with exit 2 and a ConfigError message listing available names", async () => { - const result = await runCli({ - args: ["skills", "get", "does-not-exist"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode).toBe(2); - expect(cliErrorMessage(result.stderr)).toBe( - `unknown skill name(s): does-not-exist (available: ${BUNDLED_VISIBLE_NAMES.join(", ")})`, - ); - }); - - it("get without a name or --all errors with exit 2", async () => { - const result = await runCli({ - args: ["skills", "get"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode).toBe(2); - expect(cliErrorMessage(result.stderr)).toBe( - "provide a skill name (comma-separated for multiple) or --all", - ); - }); - - it("path with no name lists every non-hidden skill's directory", async () => { - const result = await runCli({ - args: ["skills", "path", "--json"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillPathListEnvelope)).toEqual({ - data: BUNDLED_VISIBLE.map((skill) => ({ name: skill.name, dir: skill.dir })), - returned: BUNDLED_VISIBLE.length, - offset: 0, - total: BUNDLED_VISIBLE.length, - has_more: false, - next_offset: null, - }); - }); - - it("path returns a single-item envelope", async () => { - const result = await runCli({ - args: ["skills", "path", "core", "--json"], - configHome: await makeIsolatedConfigHome(), - }); - - expect(result.exitCode, result.stderr).toBe(0); - expect(parseJson(result.stdout, SkillPathListEnvelope)).toEqual({ - data: [{ name: "core", dir: bundledSkill("core").dir }], - returned: 1, - offset: 0, - total: 1, - has_more: false, - next_offset: null, - }); - }); -}); From 98ca47458be874d3d206cf85c53b0cfd3549a664 Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Wed, 7 Oct 2026 15:03:16 -0400 Subject: [PATCH 20/24] fix(client): explain a data action update the server rejects for its collection A server whose data actions have no collection answers `collection_id` on an update with a 400 naming it (its update schema is closed), so update declares the field as a dataActionCollections parameter and the rejection reads as the missing feature. The e2e lane for servers that need a model gates through requireServerWithout, so its skip is reported, and its title says the server rejects create rather than the client refusing it before sending. --- packages/client/src/resources/data-action.ts | 11 +++++++++-- tests/e2e/data-action.e2e.test.ts | 11 ++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/client/src/resources/data-action.ts b/packages/client/src/resources/data-action.ts index 30ed1f53..3445c298 100644 --- a/packages/client/src/resources/data-action.ts +++ b/packages/client/src/resources/data-action.ts @@ -10,7 +10,14 @@ import { } from "../domain/data-action"; import type { RequestOptions, Transport } from "../http/transport"; import type { ListResult } from "../list"; -import { explainer } from "../version/refusal"; +import { explainer, type ParameterRequirement } from "../version/refusal"; + +// A server whose data actions have no collection rejects `collection_id` with a 400 naming it. +function collectionFeatures(params: DataActionUpdateInput): ParameterRequirement[] { + return params.collection_id === undefined + ? [] + : [{ feature: "dataActionCollections", fields: ["collection_id"] }]; +} // Every path parameter here is a numeric id, so no fragment needs `encodeURIComponent`. export function dataActionResource(transport: Transport) { @@ -92,7 +99,7 @@ export function dataActionResource(transport: Transport) { list, get, create: explain("create", create), - update, + update: explain("update", update, (_id, params) => collectionFeatures(params)), archive, delete: remove, execute, diff --git a/tests/e2e/data-action.e2e.test.ts b/tests/e2e/data-action.e2e.test.ts index 0d8bba1e..0dc4b65f 100644 --- a/tests/e2e/data-action.e2e.test.ts +++ b/tests/e2e/data-action.e2e.test.ts @@ -16,7 +16,7 @@ import { readBootstrap, type E2EBootstrap } from "./bootstrap-data"; import { cliErrorCategory, cliErrorMessage } from "./cli-error"; import { cleanupConfigHome, mkTempConfigHome, runCli } from "./run-cli"; import { SEEDED } from "./seed/seeded"; -import { requireServer, serverHas } from "./server-gate"; +import { requireServer, requireServerWithout } from "./server-gate"; const DOWNGRADE_REMEDY = "Or install an `@metabase/cli` release that targets this server."; @@ -206,7 +206,12 @@ describe.skipIf(skipReason !== null)("data-action e2e", () => { }); }); -describe.skipIf(serverHas("dataActionsWithoutModel"))( +const withoutModellessSkipReason = requireServerWithout( + "data-action › data-action on servers that need a model", + ["dataActionsWithoutModel"], +); + +describe.skipIf(withoutModellessSkipReason !== null)( "data-action on servers that need a model", () => { let bootstrap: E2EBootstrap; @@ -220,7 +225,7 @@ describe.skipIf(serverHas("dataActionsWithoutModel"))( await Promise.all(tempDirs.splice(0).map(cleanupConfigHome)); }); - it("create refuses with CapabilityError (exit 2) after a live probe, before sending the body", async () => { + it("create is explained as the missing feature (exit 2) when the server rejects it", async () => { const configHome = await mkTempConfigHome(); tempDirs.push(configHome); From 2a77c1943fb614370cc5241890f7460d3ac813cd Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Wed, 7 Oct 2026 21:42:50 -0400 Subject: [PATCH 21/24] fix --- packages/cli/package.json | 2 +- packages/cli/skill-data/core/SKILL.md | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index cb31024f..53629262 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@metabase/cli", - "version": "0.5.0", + "version": "0.5.1", "description": "Metabase CLI", "license": "AGPL-3.0", "repository": { diff --git a/packages/cli/skill-data/core/SKILL.md b/packages/cli/skill-data/core/SKILL.md index fb978ca7..cf84549e 100644 --- a/packages/cli/skill-data/core/SKILL.md +++ b/packages/cli/skill-data/core/SKILL.md @@ -13,7 +13,7 @@ Top-level command groups (run `mb --help` to discover verbs): ``` auth | db | table | field | upload | content-translation | query | card | dashboard | snippet | segment | measure | collection | library document | glossary | timeline | timeline-event | transform | transform-job | transform-tag | transform-test | alert | subscription | setting -search | dependency | git-sync | setup | eid | uuid | upgrade | skills +search | dependency | git-sync | data-action | setup | eid | uuid | upgrade | skills ``` The conventions below — auth, flags, output, body input — hold across **every** group. Per-command flags and examples live in each command's `--help`; add `--json` for the machine-readable form with the output JSON Schema. A few flows have their own skills (see "Specialized skills"). When a card needs a query, prefer MBQL over native SQL (portable, pre-flight-validated — load `mbql`); fall back to native SQL when MBQL can't express it. @@ -124,7 +124,7 @@ mb transform --help --json | jq -r '.commands[].command' # verbs under "transfo ## Resource quirks worth memorizing -Routine verb shapes (list / get / create / update), every flag, and output schemas live in each command's `--help` (add `--json` for output schemas). Below is only what help does _not_ tell you: footguns and non-obvious behaviors. +What `--help` doesn't say: footguns and non-obvious behaviors. - **db traversal: the hydration ladder.** Start with `database get --include tables` — the compact table map (id, name, schema, description per table), one call that fits most databases. Pick the relevant tables, then `table fields ` per table (bounded: fields are per-table). `--include tables.fields` is the full rollup — small databases only. Hundreds of tables? Traverse by schema (`database schemas ` → `database schema-tables `) or look tables up by name (`search --models table --db-id --limit 10`). `db sync-schema` / `rescan-values` queue async work and return `{status:"ok"}` at once; `sync-schema --wait` blocks until `initial_sync_status: complete`. - **table fields.** `table get` never returns fields on its own — pass `--include fields` (compact; the underlying query_metadata response also carries FK targets and dimensions, visible under `--full`) or use `table fields ` (list envelope). `table update` patches table-level metadata only; physical columns aren't editable. @@ -179,7 +179,10 @@ This file is enough for any single-command task. For anything deeper, load the r - **`transform-test-plan`** — planning transform tests. - **`document`** — Metabase documents (TipTap body, embedding cards). - + +- **`data-action`** — saved SQL writes: enabling, body, parameters, running. + + - **`git-sync`** — round-tripping content to/from a git remote. - **`data-workflow`** — the guided, end-to-end data workflow: investigate raw data, build clean analysis-ready tables, define reusable segments/measures/metrics, answer questions, build dashboards. **Start here when the user states a goal rather than a single verb** — "make sense of my data", "build a data model", "be my data analyst". It detects where the data is and routes to the right stage. From 4b082fac9607fc395531f97a21e3e59eca070af5 Mon Sep 17 00:00:00 2001 From: Aleksandr Lesnenko Date: Wed, 7 Oct 2026 21:46:01 -0400 Subject: [PATCH 22/24] ci --- tests/e2e/data-action.e2e.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/e2e/data-action.e2e.test.ts b/tests/e2e/data-action.e2e.test.ts index 0dc4b65f..d79f1415 100644 --- a/tests/e2e/data-action.e2e.test.ts +++ b/tests/e2e/data-action.e2e.test.ts @@ -244,7 +244,9 @@ describe.skipIf(withoutModellessSkipReason !== null)( expect(result.exitCode).toBe(2); expect(cliErrorCategory(result.stderr)).toBe("capability"); expect(cliErrorMessage(result.stderr)).toBe( - `This operation requires Metabase v65+ (this server is ${bootstrap.server.version.tag}). Upgrade Metabase to use it.\n${DOWNGRADE_REMEDY}`, + `This operation requires Metabase v65+ (this server is ${bootstrap.server.version.tag}). Upgrade Metabase to use it.\n` + + "Metabase answered 400: model_id: missing required key, received: nil\n" + + DOWNGRADE_REMEDY, ); expect(result.stdout).toBe(""); }); From 3e4c2b33c278322b22e67cd5ced5f245b6ab3972 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Thu, 8 Oct 2026 12:36:37 -0400 Subject: [PATCH 23/24] Sync response and input enums with Metabase master Adds the library-dashboards collection type that broke e2e (ee-head), plus the other values master can send or accept: data-apps/data-actions namespaces, the exploration model (collection items, search, eid), action collection items, new timeline icons, canceled job runs, errored transform test runs and transform-test revisions. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- packages/client/src/domain/collection.ts | 12 ++++++++++-- packages/client/src/domain/eid-translation.ts | 1 + packages/client/src/domain/revision.ts | 1 + packages/client/src/domain/search.ts | 1 + packages/client/src/domain/timeline.ts | 13 ++++++++++++- packages/client/src/domain/transform-job.ts | 2 +- packages/client/src/domain/transform-test.ts | 2 +- packages/client/src/resources/collection.test.ts | 2 +- packages/client/src/resources/library.test.ts | 2 +- packages/client/src/resources/revision.ts | 1 + 11 files changed, 31 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index d2e08769..6bcd26ff 100644 --- a/README.md +++ b/README.md @@ -2070,7 +2070,7 @@ mb search revenue --search-native-query --include-metadata --full --json | Flag | Description | | ------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `--models`, `-m` | Comma-separated model filter: `card,dataset,metric,dashboard,collection,database,table,segment,measure,document,action,transform,indexed-entity`. | +| `--models`, `-m` | Comma-separated model filter: `card,dataset,metric,dashboard,collection,database,table,segment,measure,document,exploration,action,transform,indexed-entity`. | | `--archived` | Include archived items only. | | `--limit` | Max results to return (default `20` — `search` is the one list verb with its own default). | | `--offset` | Where the window starts, applied by the server (default `0`). | diff --git a/packages/client/src/domain/collection.ts b/packages/client/src/domain/collection.ts index 0416b003..8cae3c19 100644 --- a/packages/client/src/domain/collection.ts +++ b/packages/client/src/domain/collection.ts @@ -14,6 +14,7 @@ export const CollectionType = z.enum([ "library", "library-data", "library-metrics", + "library-dashboards", "tenant-specific-root-collection", ]); export type CollectionType = z.infer; @@ -24,6 +25,8 @@ export const CollectionNamespace = z.enum([ "analytics", "shared-tenant-collection", "tenant-specific", + "data-apps", + "data-actions", ]); export type CollectionNamespace = z.infer; @@ -38,6 +41,7 @@ export const COLLECTION_ITEM_FILTER_MODELS = [ "snippet", "collection", "document", + "exploration", "table", "transform", "measure", @@ -48,8 +52,12 @@ export const COLLECTION_ITEM_FILTER_MODELS = [ export const CollectionItemFilterModel = z.enum(COLLECTION_ITEM_FILTER_MODELS); export type CollectionItemFilterModel = z.infer; -// `indexed-entity` is a valid response model but not accepted as a filter value. -export const COLLECTION_ITEM_MODELS = [...COLLECTION_ITEM_FILTER_MODELS, "indexed-entity"] as const; +// `indexed-entity` and `action` are valid response models but not accepted as filter values. +export const COLLECTION_ITEM_MODELS = [ + ...COLLECTION_ITEM_FILTER_MODELS, + "indexed-entity", + "action", +] as const; export const CollectionItemModel = z.enum(COLLECTION_ITEM_MODELS); export type CollectionItemModel = z.infer; diff --git a/packages/client/src/domain/eid-translation.ts b/packages/client/src/domain/eid-translation.ts index f7d1403b..cc97f068 100644 --- a/packages/client/src/domain/eid-translation.ts +++ b/packages/client/src/domain/eid-translation.ts @@ -10,6 +10,7 @@ export const EID_MODELS = [ "dataset", "dimension", "document", + "exploration", "measure", "metric", "permissions-group", diff --git a/packages/client/src/domain/revision.ts b/packages/client/src/domain/revision.ts index 8d5a5950..eb0331e1 100644 --- a/packages/client/src/domain/revision.ts +++ b/packages/client/src/domain/revision.ts @@ -7,6 +7,7 @@ export const RevisionEntity = z.enum([ "measure", "segment", "transform", + "transform-test", ]); export type RevisionEntity = z.infer; diff --git a/packages/client/src/domain/search.ts b/packages/client/src/domain/search.ts index 3fe614d2..3e3385a4 100644 --- a/packages/client/src/domain/search.ts +++ b/packages/client/src/domain/search.ts @@ -11,6 +11,7 @@ export const SEARCH_MODELS = [ "segment", "measure", "document", + "exploration", "action", "transform", "indexed-entity", diff --git a/packages/client/src/domain/timeline.ts b/packages/client/src/domain/timeline.ts index c1faf1b9..9a818ceb 100644 --- a/packages/client/src/domain/timeline.ts +++ b/packages/client/src/domain/timeline.ts @@ -1,6 +1,17 @@ import { z } from "zod"; -export const TimelineIcon = z.enum(["star", "cake", "mail", "warning", "bell", "cloud"]); +export const TimelineIcon = z.enum([ + "star", + "cake", + "mail", + "mail_at", + "warning", + "bell", + "cloud", + "info", + "note", + "event", +]); export type TimelineIcon = z.infer; export const TimelineEvent = z diff --git a/packages/client/src/domain/transform-job.ts b/packages/client/src/domain/transform-job.ts index 1426cace..63e171c7 100644 --- a/packages/client/src/domain/transform-job.ts +++ b/packages/client/src/domain/transform-job.ts @@ -3,7 +3,7 @@ import { z } from "zod"; import type { Features } from "../version/features"; import { CronUiDisplayType } from "./cron"; -const JobRunStatus = z.enum(["started", "succeeded", "failed", "timeout"]); +const JobRunStatus = z.enum(["started", "succeeded", "failed", "timeout", "canceled"]); const JobRunMethod = z.enum(["manual", "cron"]); diff --git a/packages/client/src/domain/transform-test.ts b/packages/client/src/domain/transform-test.ts index 4ae2d5d0..f982cc77 100644 --- a/packages/client/src/domain/transform-test.ts +++ b/packages/client/src/domain/transform-test.ts @@ -234,7 +234,7 @@ export type TransformTestExpectationResult = z.infer { expect(error.userMessage).toBe( "Metabase returned unexpected response shape:\n" + ' [1].type: Invalid option: expected one of "instance-analytics"|"trash"|"library"|' + - '"library-data"|"library-metrics"|"tenant-specific-root-collection"', + '"library-data"|"library-metrics"|"library-dashboards"|"tenant-specific-root-collection"', ); }); }); diff --git a/packages/client/src/resources/library.test.ts b/packages/client/src/resources/library.test.ts index b8b5b10f..0561da73 100644 --- a/packages/client/src/resources/library.test.ts +++ b/packages/client/src/resources/library.test.ts @@ -61,7 +61,7 @@ const UNPINNED_ENUM_COLLECTION = { const ABSENT_LIBRARY = { data: null }; const MISSING_TYPE_ISSUE = - 'Invalid option: expected one of "instance-analytics"|"trash"|"library"|"library-data"|"library-metrics"|"tenant-specific-root-collection"'; + 'Invalid option: expected one of "instance-analytics"|"trash"|"library"|"library-data"|"library-metrics"|"library-dashboards"|"tenant-specific-root-collection"'; const TARGET_COLLECTION = { id: 11, diff --git a/packages/client/src/resources/revision.ts b/packages/client/src/resources/revision.ts index 732bd5e1..ed17fcf5 100644 --- a/packages/client/src/resources/revision.ts +++ b/packages/client/src/resources/revision.ts @@ -28,6 +28,7 @@ const RevisionApiRevert: z.ZodType = z.union([ const ENTITY_FEATURES: Partial> = { measure: "measures", transform: "transforms", + "transform-test": "transformTests", }; function entityFeatures(entity: RevisionEntity): ParameterRequirement[] { From 9ff67554747a6f19681c19998635e2897c96ad62 Mon Sep 17 00:00:00 2001 From: Alexander Polyankin Date: Thu, 8 Oct 2026 13:16:00 -0400 Subject: [PATCH 24/24] Follow Metabase head in the data action and Library e2e tests Head refuses a model-less data action in a regular collection, so the e2e files it in the data actions root, and the Library gained a Dashboards child. Co-Authored-By: Claude Opus 5.5 --- packages/client/src/version/features.ts | 3 ++- tests/e2e/data-action.e2e.test.ts | 8 ++++---- tests/e2e/library.e2e.test.ts | 11 ++++++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/client/src/version/features.ts b/packages/client/src/version/features.ts index ecd53097..5f9431db 100644 --- a/packages/client/src/version/features.ts +++ b/packages/client/src/version/features.ts @@ -7,7 +7,7 @@ import type { TokenFeatures } from "../domain/session-properties"; // version it arrived in, so a rule reads the same after the majors around it leave the window. // The table is the one place a server behaviour is pinned to a generation, so it also holds rules // only the e2e suite branches on (`invalidMbqlIsBadRequest`, `collectionItemsTotalOnEmptyPage`, -// `transformTargetTableLinkedOnCreate`); a rule with no reader in `resources/` is not an orphan. +// `transformTargetTableLinkedOnCreate`, `libraryDashboards`); a rule with no reader in `resources/` is not an orphan. export interface FeatureRule { readonly since: number; readonly until?: number; @@ -21,6 +21,7 @@ export const FEATURE_RULES = { transformJobActivation: { since: 61 }, measures: { since: 59 }, library: { since: 59, tokenFeature: "library" }, + libraryDashboards: { since: 65 }, remoteSync: { since: 58, tokenFeature: "remote_sync" }, remoteSyncRemoteChanges: { since: 59, tokenFeature: "remote_sync" }, remoteSyncCollectionsOnlyUpdate: { since: 60 }, diff --git a/tests/e2e/data-action.e2e.test.ts b/tests/e2e/data-action.e2e.test.ts index d79f1415..7fce16a6 100644 --- a/tests/e2e/data-action.e2e.test.ts +++ b/tests/e2e/data-action.e2e.test.ts @@ -30,7 +30,7 @@ function createBody(): DataActionCreateInput { name: ACTION_NAME, type: "query", database_id: SEEDED.warehouseDbId, - collection_id: SEEDED.defaultCollectionId, + collection_id: null, dataset_query: { "lib/type": "mbql/query", database: SEEDED.warehouseDbId, @@ -117,7 +117,7 @@ describe.skipIf(skipReason !== null)("data-action e2e", () => { expect(result.stdout).toBe(""); }); - it("create files a model-less query action in its collection, and list and get read it back", async () => { + it("create files a model-less query action in the data actions root, and list and get read it back", async () => { await enableActionsOnWarehouse(); const created = await createAction(); @@ -126,7 +126,7 @@ describe.skipIf(skipReason !== null)("data-action e2e", () => { type: "query", model_id: null, database_id: SEEDED.warehouseDbId, - collection_id: SEEDED.defaultCollectionId, + collection_id: null, archived: false, }); @@ -137,7 +137,7 @@ describe.skipIf(skipReason !== null)("data-action e2e", () => { id: created.id, name: ACTION_NAME, type: "query", - collection_id: SEEDED.defaultCollectionId, + collection_id: null, database_id: SEEDED.warehouseDbId, archived: false, }, diff --git a/tests/e2e/library.e2e.test.ts b/tests/e2e/library.e2e.test.ts index 0b654655..268e55cb 100644 --- a/tests/e2e/library.e2e.test.ts +++ b/tests/e2e/library.e2e.test.ts @@ -10,7 +10,7 @@ import { readBootstrap, type E2EBootstrap } from "./bootstrap-data"; import { cliErrorMessage } from "./cli-error"; import { cleanupConfigHome, mkTempConfigHome, runCli } from "./run-cli"; import { SEEDED } from "./seed/seeded"; -import { requireServer } from "./server-gate"; +import { requireServer, serverHas } from "./server-gate"; const LIBRARY_UNAVAILABLE = requireServer("library › with the library feature", ["library"]); const REMOTE_SYNC_UNAVAILABLE = requireServer( @@ -40,7 +40,16 @@ function childrenByName(library: Library): LibraryChild[] { return library.effective_children.toSorted((a, b) => a.name.localeCompare(b.name)); } +const LIBRARY_DASHBOARDS_CHILD = { + id: expect.any(Number), + name: "Dashboards", + description: null, + type: "library-dashboards", + is_remote_synced: false, +}; + const LIBRARY_CHILDREN = [ + ...(serverHas("libraryDashboards") ? [LIBRARY_DASHBOARDS_CHILD] : []), { id: SEEDED.libraryDataCollectionId, name: "Data",