From a79d2da18db6cc25136bec83ef4aa8dfc789c372 Mon Sep 17 00:00:00 2001 From: Petr Date: Fri, 21 Aug 2026 22:57:28 +0200 Subject: [PATCH] fix(storage): write column descriptions where MCP reads them (#624) `storage describe-column` / `describe-batch` stored each description as a flat `KBC.column.{name}.description` entry on the TABLE's metadata, on the documented assumption that Keboola exposes no user-writable column-metadata endpoint. That assumption is wrong: the same `POST /v2/storage/tables/{id}/metadata` kbagent already calls accepts a `columnsMetadata` payload with `provider: user`, and that native `columnMetadata` store is what the Keboola UI and the Keboola MCP server (`get_table_detail`, `search`) read. Nothing read the flat keys, so column descriptions written by kbagent were invisible to every AI client using the MCP server -- the exact audience they are written for. The failure was silent in both directions: `table-detail` read back kbagent's own convention, so the descriptions were reported as correctly applied. - client: new `set_table_column_metadata()` posting a JSON `columnsMetadata` payload (the nested per-column shape has no form-encoded equivalent). - service: `describe_columns()` writes `KBC.description` per column there; `get_table_detail()` reads it from `columnMetadata` first and keeps the flat keys as a fallback so older descriptions stay visible, with `columnMetadata` winning when both carry a value. - the repetitive per-column metadata elif chain becomes a `_COLUMN_META_FIELDS` mapping, keeping the file inside its grandfathered line budget. There is no bulk migration: re-running `describe-column` on an affected table rewrites it into the right place. --- .claude-plugin/marketplace.json | 2 +- plugins/kbagent/.claude-plugin/plugin.json | 2 +- .../kbagent/references/commands-reference.md | 2 +- .../skills/kbagent/references/gotchas.md | 26 ++-- .../references/storage-describe-workflow.md | 28 ++-- pyproject.toml | 2 +- src/keboola_agent_cli/changelog.py | 20 +++ .../client/storage_tables.py | 57 +++++++- src/keboola_agent_cli/commands/context.py | 6 +- src/keboola_agent_cli/commands/storage.py | 11 +- .../services/storage_service.py | 71 ++++++---- tests/test_client.py | 43 +++++- tests/test_storage_describe_service.py | 124 ++++++++++++++++-- uv.lock | 2 +- 14 files changed, 317 insertions(+), 79 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 01aba376..f34e2f21 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "kbagent", - "version": "0.87.0", + "version": "0.87.1", "source": "./plugins/kbagent", "description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, sync configs as files, manage dev branches, and debug SQL in workspaces", "category": "development" diff --git a/plugins/kbagent/.claude-plugin/plugin.json b/plugins/kbagent/.claude-plugin/plugin.json index b4fd0bf2..88f3f52f 100644 --- a/plugins/kbagent/.claude-plugin/plugin.json +++ b/plugins/kbagent/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "kbagent", - "version": "0.87.0", + "version": "0.87.1", "description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, sync configs as files, manage dev branches, and debug SQL in workspaces", "author": { "name": "Keboola", diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index aa9a5e70..d8edbe44 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -178,7 +178,7 @@ Requires a **super-admin** Manage API token (same kind as `org setup`). Same def - `storage snapshot-delete --project NAME --snapshot-id ID [--snapshot-id ...] [--dry-run] [--yes]` (since v0.75.0) -- delete snapshots (destructive: forecloses restores; source tables untouched). Batch-tolerant, exit 1 when any ID failed - `storage describe-bucket --project NAME --bucket-id ID [--text STR | --file PATH | --stdin] [--branch ID]` -- set a bucket description (stored as `KBC.description` in bucket metadata, upsert). Provide exactly one of `--text`, `--file`, `--stdin`. Read back via `storage bucket-detail` - `storage describe-table --project NAME --table-id ID [--text STR | --file PATH | --stdin] [--branch ID]` -- set a table description (stored as `KBC.description` in table metadata, upsert). Provide exactly one of `--text`, `--file`, `--stdin`. Read back via `storage table-detail` -- `storage describe-column --project NAME --table-id ID --column NAME=DESCRIPTION [--column ...] [--branch ID]` -- set one or more column descriptions. Stored as `KBC.column.{name}.description` keys in the table's metadata (Keboola has no user-writable column-metadata endpoint). Read back in `storage table-detail` under `column_details[].description` +- `storage describe-column --project NAME --table-id ID --column NAME=DESCRIPTION [--column ...] [--branch ID]` -- set one or more column descriptions. Stored as `KBC.description` inside the table's native `columnMetadata` store (since v0.87.1; `POST .../metadata` accepts a `columnsMetadata` payload with `provider: user`) -- the store the Keboola UI and the Keboola MCP server actually read. Read back in `storage table-detail` under `column_details[].description`. kbagent `<= 0.86.0` wrote flat `KBC.column.{name}.description` table metadata that MCP never read (#624); re-run this command on an affected table to migrate it - `storage describe-batch --project NAME --from-file PATH [--branch ID]` -- apply bucket/table/column descriptions from a YAML file (top-level `buckets`, `tables`, `columns` sections, all optional). Partial-failure tolerant: per-item errors are collected and reported, the batch does not abort. Non-zero exit only when at least one item failed ## Storage Files diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 4d09fbb6..4d8b18e6 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -2622,15 +2622,23 @@ so setting a branch's description will **not** update the dashboard. `kbagent storage describe-bucket / describe-table / describe-column / describe-batch` write descriptive metadata onto storage objects. Three behaviors are easy to miss: -- **Column descriptions use a metadata-key convention, not a column endpoint.** - The Keboola Storage API has no user-writable column-level metadata endpoint, - so `describe-column` stores each description as a `KBC.column.{name}.description` - entry on the **table's** metadata (upsert). `storage table-detail` reads them - back via the same key and surfaces them under `column_details[].description`. - Renaming or deleting a column does NOT automatically clean these entries up - (they remain on the table's metadata under the old name). Same convention for - table and bucket descriptions: stored as `KBC.description` (provider=user) on - the object's metadata. +- **Column descriptions written before v0.87.1 are invisible to MCP clients.** + `describe-column` used to store each description as a flat + `KBC.column.{name}.description` entry on the **table's** metadata, on the + mistaken assumption that Keboola had no user-writable column-metadata + endpoint. It does: `POST /v2/storage/tables/{id}/metadata` accepts a + `columnsMetadata` payload with `provider: user`, and that native + `columnMetadata` store is where the Keboola UI and the Keboola MCP server + (`get_table_detail`, `search`) read column descriptions from. **(since + v0.87.1)** `describe-column` / `describe-batch` write there. The failure was + silent in both directions — `storage table-detail` read back kbagent's own + convention, so the descriptions looked correctly applied while no AI client + could see them (#624). kbagent still reads the flat keys as a fallback, and + `columnMetadata` wins when both carry a value, so **re-running + `describe-column` on an affected table is the migration** — there is no bulk + migration command. Renaming or deleting a column does NOT clean the old + entries up under either convention. Table and bucket descriptions are + unaffected: still `KBC.description` (provider=user) on the object's metadata. - **`describe-batch` is partial-failure-tolerant.** Item-level errors are collected into `result.errors[]` but the batch keeps processing the remaining items. The CLI exits non-zero only if `error_count > 0`, so in scripts always diff --git a/plugins/kbagent/skills/kbagent/references/storage-describe-workflow.md b/plugins/kbagent/skills/kbagent/references/storage-describe-workflow.md index de12e37e..2664c5c5 100644 --- a/plugins/kbagent/skills/kbagent/references/storage-describe-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/storage-describe-workflow.md @@ -35,10 +35,16 @@ Descriptions are stored as metadata entries on the object: - **Bucket description** -- `KBC.description` (provider=user) on bucket metadata - **Table description** -- `KBC.description` (provider=user) on table metadata -- **Column description** -- `KBC.column.{column_name}.description` on the - **table's** metadata. Keboola has no user-writable column-metadata endpoint, - so this key convention is the storage layer for column descriptions. Read +- **Column description** -- `KBC.description` (provider=user) inside the + table's **`columnMetadata`** store, written through the `columnsMetadata` + payload of `POST /v2/storage/tables/{id}/metadata`. This is the store the + Keboola UI and the Keboola MCP server read column descriptions from. Read them back via `storage table-detail` (`column_details[].description`). + **(since v0.87.1)** -- kbagent `<= 0.86.0` wrote flat + `KBC.column.{name}.description` keys on the **table's** metadata instead, + which no MCP consumer ever read (#624). kbagent still reads that convention + as a fallback, so old descriptions stay visible here, but they remain + invisible to MCP until you re-run `describe-column` on the table. Descriptions are `upsert`: calling `describe-*` with a new text replaces whatever was there before. There is no append mode. @@ -106,10 +112,12 @@ kbagent --json storage describe-column \ --column "created_at=Server-side creation timestamp (UTC)" ``` -Column descriptions live under `KBC.column.{name}.description` on the -**table's** metadata -- they are NOT attached to the column record itself. -If you rename or delete a column, the old key lingers until you manually -clean it up (there is no `--delete-column-description` command today). +Column descriptions live under `KBC.description` inside the table's +`columnMetadata` store, keyed by column name (since v0.87.1; before that they +were flat `KBC.column.{name}.description` keys on the table's metadata that MCP +never read -- see #624). If you rename or delete a column, the old entry +lingers until you manually clean it up (there is no +`--delete-column-description` command today). Read back via `storage table-detail`: @@ -246,8 +254,10 @@ entries with `provider="user"` are considered the canonical description. ## Key behaviors - `describe-*` is **upsert** -- no append mode; re-running replaces the value. -- Column descriptions piggy-back on table metadata via the - `KBC.column.{name}.description` key convention. +- Column descriptions are written as `KBC.description` into the table's + `columnMetadata` store (since v0.87.1), so MCP clients see them. Values + written by an older kbagent under `KBC.column.{name}.description` are still + read here but are invisible to MCP -- re-run `describe-column` to migrate. - `describe-batch` is **partial-failure-tolerant** -- check `errors[]` even on exit code 0. - All commands support `--branch ID` to target a dev branch. diff --git a/pyproject.toml b/pyproject.toml index 4daf1f29..b79c3f72 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keboola-cli" -version = "0.87.0" +version = "0.87.1" description = "AI-friendly CLI for managing Keboola projects" readme = "README.md" requires-python = ">=3.12" diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index 45744bef..1764d1b8 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -24,6 +24,26 @@ # Ordered newest-first. Each value is a list of brief one-line descriptions. CHANGELOG: dict[str, list[str]] = { + "0.87.1": [ + "Fix (#624): `storage describe-column` / `describe-batch` now write column " + "descriptions where every Keboola consumer actually reads them. They used to be " + "stored as flat `KBC.column.{name}.description` entries on the TABLE's metadata, on " + "the documented but incorrect assumption that Keboola exposes no user-writable " + "column-metadata endpoint. It does: the same `POST /v2/storage/tables/{id}/metadata` " + "kbagent already called accepts a `columnsMetadata` payload with `provider: user`, " + "and that native `columnMetadata` store is what the Keboola UI and the Keboola MCP " + "server (`get_table_detail`, `search`) read. Nothing ever read the flat keys, so " + "column descriptions written by kbagent were invisible to every AI client using the " + "MCP server -- the exact audience they are written for. The failure was silent in " + "BOTH directions: `storage table-detail` read back kbagent's own convention, so the " + "descriptions were reported as correctly applied. `table-detail` now reads " + "`KBC.description` from `columnMetadata` first and keeps the flat keys as a fallback, " + "so descriptions written by an older kbagent stay visible here; when both carry a " + "value, `columnMetadata` wins. There is NO bulk migration -- re-running " + "`describe-column` (or `describe-batch`) on an affected table rewrites it into the " + "right place, and until you do, those tables remain invisible to MCP clients. Table " + "and bucket descriptions are unaffected.", + ], "0.87.0": [ "New (#626): `data-app create` gains `--workspace / --no-workspace` and grants " "Storage access BY DEFAULT. The flag writes `runtime.workspace.enabled: true`, which " diff --git a/src/keboola_agent_cli/client/storage_tables.py b/src/keboola_agent_cli/client/storage_tables.py index 08f6b2a2..c52ad98f 100644 --- a/src/keboola_agent_cli/client/storage_tables.py +++ b/src/keboola_agent_cli/client/storage_tables.py @@ -123,10 +123,10 @@ def set_table_metadata( POST /v2/storage/tables/{id}/metadata Provider is always ``"user"`` for CLI-originated descriptions. - Column-level descriptions use the namespaced key convention - ``KBC.column.{colname}.description`` stored at table-metadata level - (Keboola Storage API does not expose a user-writable column-metadata - endpoint; ``columnMetadata`` is populated exclusively by components). + This method writes **table-level** metadata only. For per-column + descriptions use :meth:`set_table_column_metadata`, which targets the + native ``columnMetadata`` store the platform (and the Keboola MCP + server) actually reads. Args: table_id: Full table ID (e.g. "in.c-bucket.table"). @@ -145,6 +145,55 @@ def set_table_metadata( response = self._request("POST", f"{prefix}/tables/{safe_id}/metadata", data=form) return response.json() + def set_table_column_metadata( + self, + table_id: str, + columns: dict[str, list[tuple[str, str]]], + branch_id: int | None = None, + ) -> dict[str, Any]: + """Upsert per-column metadata on a storage table. + + POST /v2/storage/tables/{id}/metadata with a JSON ``columnsMetadata`` + payload. + + The same endpoint that takes table-level ``metadata`` also accepts a + ``columnsMetadata`` object with ``provider: user``, writing into the + native per-column store that ``GET /v2/storage/tables/{id}`` returns as + ``columnMetadata``. That is the store the platform UI and the Keboola + MCP server read column descriptions from; the older flat + ``KBC.column.{name}.description`` table-metadata convention is invisible + to them (issue #624). + + Unlike the table-level sibling this sends JSON rather than form data -- + the nested per-column shape has no form-encoded equivalent. + + Args: + table_id: Full table ID (e.g. "in.c-bucket.table"). + columns: Mapping of column name -> ordered ``(key, value)`` tuples. + branch_id: If set, target a specific dev branch. + + Returns: + Response dict from the API, carrying ``metadata`` and + ``columnsMetadata`` for the table after the upsert. + """ + prefix = f"/v2/storage/branch/{branch_id}" if branch_id else "/v2/storage" + safe_id = quote(table_id, safe="") + payload: dict[str, Any] = { + "provider": "user", + "columnsMetadata": { + name: [ + # ``columnName`` is redundant with the mapping key but the + # platform's own clients send it; mirror them rather than + # rely on the server inferring it. + {"key": key, "value": value, "columnName": name} + for key, value in entries + ] + for name, entries in columns.items() + }, + } + response = self._request("POST", f"{prefix}/tables/{safe_id}/metadata", json=payload) + return response.json() + def get_bucket_detail( self, bucket_id: str, diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index 3138a3db..6df742b5 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -724,8 +724,10 @@ Set the KBC.description metadata on a table (upsert). Readable via table-detail --json .data.description. kbagent storage describe-column --project NAME --table-id ID --column NAME=DESC [--column ...] [--branch ID] - Set per-column descriptions stored as KBC.column.{{name}}.description in table metadata (upsert). - Readable via table-detail --json .data.column_details[].description. + Set per-column descriptions as KBC.description in the table's columnMetadata (upsert) -- the store + the Keboola UI and MCP server read. Readable via table-detail --json .data.column_details[].description. + kbagent <= 0.86.0 wrote flat KBC.column.{{name}}.description table metadata that MCP never read (#624); + re-run describe-column on an affected table to repair it. kbagent storage describe-batch --project NAME --from-file YAML [--branch ID] Apply bucket/table/column descriptions from a YAML file. Sections: buckets, tables, columns (all optional). diff --git a/src/keboola_agent_cli/commands/storage.py b/src/keboola_agent_cli/commands/storage.py index cf08fcdb..c8d1c3c7 100644 --- a/src/keboola_agent_cli/commands/storage.py +++ b/src/keboola_agent_cli/commands/storage.py @@ -1864,10 +1864,13 @@ def storage_describe_column( ) -> None: """Set descriptions on one or more columns of a storage table. - Descriptions are stored as KBC.column.{name}.description keys in table - metadata (upsert). Keboola Storage does not expose a user-writable - column-level metadata endpoint; this convention lets you annotate columns - and read them back via 'storage table-detail'. + Descriptions are stored as KBC.description in the table's per-column + metadata (upsert) -- the same place the Keboola UI and the Keboola MCP + server read them from. Read them back via 'storage table-detail'. + + kbagent <= 0.86.0 wrote them as flat KBC.column.{name}.description table + metadata instead, which no MCP consumer ever read; re-run this command on + an affected table to repair it. Example: diff --git a/src/keboola_agent_cli/services/storage_service.py b/src/keboola_agent_cli/services/storage_service.py index ebcc2977..cb7ce2cb 100644 --- a/src/keboola_agent_cli/services/storage_service.py +++ b/src/keboola_agent_cli/services/storage_service.py @@ -81,6 +81,22 @@ def _safe_download_target(base: Path, server_name: str) -> Path: r"\s*$" ) +# Column-metadata key -> the field it lands on in ``column_details``. Straight +# pass-through of the raw string value; keys needing a transform (nullable) or +# absent from a table's columnMetadata stay special-cased at the call site. +_COLUMN_META_FIELDS = { + "KBC.datatype.basetype": "type", + # Native backend type (e.g. "VARCHAR", "NUMBER", "TIMESTAMP_TZ") -- + # distinct from the Keboola basetype it maps to. + "KBC.datatype.type": "native_type", + # Length as stored: "40", "18,2", "255", ... + "KBC.datatype.length": "length", + "KBC.datatype.default": "default", + # Written by `storage describe-column` since 0.87.1, and by the Keboola UI + # and MCP server. See the legacy fallback in get_table_detail (#624). + "KBC.description": "description", +} + def _read_csv_header(file_path: str, delimiter: str = ",") -> list[str]: """Return column names from the first row of a CSV file. @@ -654,39 +670,34 @@ def get_table_detail( column_metadata = table.get("columnMetadata", {}) raw_metadata: list[dict[str, Any]] = table.get("metadata", []) - # Extract description and per-column descriptions from metadata list + # Extract the table description and the LEGACY per-column descriptions + # from the table-level metadata list. Legacy: kbagent <= 0.86.0 wrote + # column descriptions as flat ``KBC.column.{name}.description`` keys + # here instead of into ``columnMetadata`` (#624). They are still read so + # that tables described by an older kbagent do not lose their + # descriptions in this output, but ``columnMetadata`` wins. description = "" - col_descriptions: dict[str, str] = {} + legacy_col_descriptions: dict[str, str] = {} for m in raw_metadata: key = m.get("key", "") if key == "KBC.description" and m.get("provider") == "user": description = m.get("value", "") or "" elif key.startswith("KBC.column.") and key.endswith(".description"): col_name = key[len("KBC.column.") : -len(".description")] - col_descriptions[col_name] = m.get("value", "") or "" + legacy_col_descriptions[col_name] = m.get("value", "") or "" column_details = [] for col in columns: col_info: dict[str, Any] = {"name": col} - meta = column_metadata.get(col, []) - for m in meta: + for m in column_metadata.get(col, []): key = m.get("key", "") value = m.get("value", "") - if key == "KBC.datatype.basetype": - col_info["type"] = value - elif key == "KBC.datatype.type": - # Native backend type (e.g. "VARCHAR", "NUMBER", "TIMESTAMP_TZ") - # -- distinct from the Keboola basetype it maps to. - col_info["native_type"] = value - elif key == "KBC.datatype.length": - # Length as stored: "40", "18,2", "255", ... - col_info["length"] = value + if key in _COLUMN_META_FIELDS: + col_info[_COLUMN_META_FIELDS[key]] = value elif key == "KBC.datatype.nullable": col_info["nullable"] = value == "1" - elif key == "KBC.datatype.default": - col_info["default"] = value - if col in col_descriptions: - col_info["description"] = col_descriptions[col] + if "description" not in col_info and col in legacy_col_descriptions: + col_info["description"] = legacy_col_descriptions[col] column_details.append(col_info) return { @@ -2530,12 +2541,18 @@ def describe_columns( ) -> dict[str, Any]: """Set per-column descriptions on a storage table. - Column descriptions are stored as namespaced table metadata using the - key convention ``KBC.column.{colname}.description``. Keboola's - Storage API does not provide a user-writable column-level metadata - endpoint (``columnMetadata`` is populated exclusively by processing - components); this convention is the supported alternative for - annotating columns from the CLI. + Descriptions are written as ``KBC.description`` inside the table's + native per-column metadata store (``columnMetadata``), which is where + the platform UI and the Keboola MCP server read them from. + + Until 0.87.1 they were written as flat ``KBC.column.{colname}.description`` + table-level metadata instead, on the mistaken assumption that no + user-writable column-metadata endpoint existed. It does -- the same + ``POST .../metadata`` endpoint takes a ``columnsMetadata`` payload -- + and the old convention was invisible to every MCP consumer while + ``table-detail`` still reported the descriptions as applied (#624). + Re-running this command on an affected table repairs it; the read path + keeps a fallback so already-written values stay visible in kbagent. Args: alias: Project alias. @@ -2550,12 +2567,12 @@ def describe_columns( raise ValueError("At least one column description must be provided.") projects = self.resolve_projects([alias]) project = projects[alias] - entries = [(f"KBC.column.{name}.description", desc) for name, desc in columns.items()] + entries = {name: [("KBC.description", desc)] for name, desc in columns.items()} client = self._client_factory(project.stack_url, project.token) try: - result = client.set_table_metadata( + result = client.set_table_column_metadata( table_id=table_id, - entries=entries, + columns=entries, branch_id=branch_id, ) finally: diff --git a/tests/test_client.py b/tests/test_client.py index 8e6fc3eb..4a2dd4dd 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -4022,8 +4022,13 @@ def test_set_table_metadata_with_branch(self, httpx_mock) -> None: ) assert result == _STORAGE_META_RESPONSE - def test_set_table_metadata_column_convention(self, httpx_mock) -> None: - """Column descriptions use KBC.column.{name}.description key convention.""" + def test_set_table_column_metadata_posts_columns_metadata_json(self, httpx_mock) -> None: + """Per-column metadata goes out as a JSON columnsMetadata payload (#624). + + The flat form-encoded ``KBC.column.{name}.description`` convention this + replaced wrote into table metadata, which no MCP consumer reads. + """ + import json as _json from urllib.parse import quote as url_quote safe_id = url_quote("in.c-b.tbl", safe="") @@ -4034,12 +4039,38 @@ def test_set_table_metadata_column_convention(self, httpx_mock) -> None: status_code=201, ) with KeboolaClient(stack_url=_BASE, token=_TOKEN) as client: - client.set_table_metadata( + result = client.set_table_column_metadata( table_id="in.c-b.tbl", - entries=[("KBC.column.city.description", "City name")], + columns={"city": [("KBC.description", "City name")]}, + ) + assert result == _STORAGE_META_RESPONSE + request = httpx_mock.get_request() + assert request.headers["content-type"].startswith("application/json") + body = _json.loads(request.content) + assert body == { + "provider": "user", + "columnsMetadata": { + "city": [{"key": "KBC.description", "value": "City name", "columnName": "city"}] + }, + } + + def test_set_table_column_metadata_branch(self, httpx_mock) -> None: + """A dev branch id selects the branch-scoped metadata endpoint.""" + from urllib.parse import quote as url_quote + + safe_id = url_quote("in.c-b.tbl", safe="") + httpx_mock.add_response( + url=f"{_BASE}/v2/storage/branch/7/tables/{safe_id}/metadata", + method="POST", + json=_STORAGE_META_RESPONSE, + status_code=201, + ) + with KeboolaClient(stack_url=_BASE, token=_TOKEN) as client: + client.set_table_column_metadata( + table_id="in.c-b.tbl", + columns={"city": [("KBC.description", "City name")]}, + branch_id=7, ) - body = httpx_mock.get_request().content.decode().replace("%5B", "[").replace("%5D", "]") - assert "KBC.column.city.description" in body class TestAssertSafeDownloadUrl: diff --git a/tests/test_storage_describe_service.py b/tests/test_storage_describe_service.py index ca9fa7a2..54b09a7a 100644 --- a/tests/test_storage_describe_service.py +++ b/tests/test_storage_describe_service.py @@ -197,14 +197,17 @@ def test_success_namespaced_keys(self, tmp_path: Path) -> None: assert result["table_id"] == "in.c-sales.orders" assert result["columns"]["order_id"] == "Unique order identifier" assert result["columns"]["total"] == "Order total in USD" - mock_client.set_table_metadata.assert_called_once_with( + mock_client.set_table_column_metadata.assert_called_once_with( table_id="in.c-sales.orders", - entries=[ - ("KBC.column.order_id.description", "Unique order identifier"), - ("KBC.column.total.description", "Order total in USD"), - ], + columns={ + "order_id": [("KBC.description", "Unique order identifier")], + "total": [("KBC.description", "Order total in USD")], + }, branch_id=None, ) + # The legacy flat-key table-metadata write must not happen any more -- + # it is the write MCP consumers never saw (#624). + mock_client.set_table_metadata.assert_not_called() mock_client.close.assert_called_once() def test_empty_columns_raises_value_error(self, tmp_path: Path) -> None: @@ -215,12 +218,12 @@ def test_empty_columns_raises_value_error(self, tmp_path: Path) -> None: with pytest.raises(ValueError, match="At least one column"): service.describe_columns(alias="prod", table_id="in.c-sales.orders", columns={}) - mock_client.set_table_metadata.assert_not_called() + mock_client.set_table_column_metadata.assert_not_called() def test_with_branch(self, tmp_path: Path) -> None: store = _make_store(tmp_path) mock_client = MagicMock() - mock_client.set_table_metadata.return_value = [] + mock_client.set_table_column_metadata.return_value = {} service = _make_service(store, mock_client) service.describe_columns( @@ -230,9 +233,9 @@ def test_with_branch(self, tmp_path: Path) -> None: branch_id=77, ) - mock_client.set_table_metadata.assert_called_once_with( + mock_client.set_table_column_metadata.assert_called_once_with( table_id="in.c-sales.orders", - entries=[("KBC.column.col1.description", "First column")], + columns={"col1": [("KBC.description", "First column")]}, branch_id=77, ) @@ -268,10 +271,11 @@ def test_success_all_sections(self, tmp_path: Path) -> None: assert "bucket" in applied_types assert "table" in applied_types assert "columns" in applied_types - # Bucket metadata called once (for the bucket), table metadata called twice - # (once for table description, once for column descriptions) + # Bucket metadata once; table metadata once (the table description); + # column descriptions go through the separate columnsMetadata write (#624). assert mock_client.set_bucket_metadata.call_count == 1 - assert mock_client.set_table_metadata.call_count == 2 + assert mock_client.set_table_metadata.call_count == 1 + assert mock_client.set_table_column_metadata.call_count == 1 def test_file_not_found(self, tmp_path: Path) -> None: store = _make_store(tmp_path) @@ -638,7 +642,12 @@ class TestGetTableDetailDescriptionExtraction: """Verify get_table_detail extracts table + per-column descriptions from metadata.""" def test_extracts_table_and_column_descriptions(self, tmp_path: Path) -> None: - """KBC.description + KBC.column.{name}.description are surfaced on the response.""" + """Table KBC.description + the LEGACY flat per-column keys are surfaced. + + The flat ``KBC.column.{name}.description`` keys are what kbagent <= + 0.86.0 wrote (#624); reading them is a back-compat fallback so tables + described by an older kbagent keep showing their descriptions here. + """ store = _make_store(tmp_path) mock_client = MagicMock() mock_client.get_table_detail.return_value = { @@ -689,6 +698,95 @@ def test_extracts_table_and_column_descriptions(self, tmp_path: Path) -> None: assert isinstance(result["metadata"], list) assert len(result["metadata"]) == 3 + def test_reads_description_from_column_metadata(self, tmp_path: Path) -> None: + """KBC.description inside columnMetadata is the primary source (#624). + + This is where describe-column writes since 0.86.1, and where the + Keboola UI and MCP server read from. + """ + store = _make_store(tmp_path) + mock_client = MagicMock() + mock_client.get_table_detail.return_value = { + "id": "in.c-sales.orders", + "name": "orders", + "displayName": "orders", + "bucket": {"id": "in.c-sales"}, + "columns": ["order_id", "total"], + "primaryKey": ["order_id"], + "rowsCount": 42, + "columnMetadata": { + "order_id": [ + { + "id": "10", + "key": "KBC.datatype.basetype", + "value": "INTEGER", + "provider": "storage", + }, + { + "id": "11", + "key": "KBC.description", + "value": "Unique order identifier", + "provider": "user", + }, + ], + "total": [ + { + "id": "12", + "key": "KBC.description", + "value": "Order total in USD", + "provider": "user", + } + ], + }, + "metadata": [], + } + service = _make_service(store, mock_client) + + result = service.get_table_detail(alias="prod", table_id="in.c-sales.orders") + + col_map = {c["name"]: c for c in result["column_details"]} + assert col_map["order_id"]["description"] == "Unique order identifier" + assert col_map["order_id"]["type"] == "INTEGER" + assert col_map["total"]["description"] == "Order total in USD" + + def test_column_metadata_description_wins_over_legacy_flat_key(self, tmp_path: Path) -> None: + """When both conventions carry a value, columnMetadata wins (#624). + + A table described by an old kbagent and then re-described by a current + one holds both; the one MCP consumers see must be the one kbagent + reports. + """ + store = _make_store(tmp_path) + mock_client = MagicMock() + mock_client.get_table_detail.return_value = { + "id": "in.c-sales.orders", + "name": "orders", + "displayName": "orders", + "bucket": {"id": "in.c-sales"}, + "columns": ["order_id"], + "primaryKey": [], + "rowsCount": 0, + "columnMetadata": { + "order_id": [ + {"id": "1", "key": "KBC.description", "value": "current", "provider": "user"} + ] + }, + "metadata": [ + { + "id": "2", + "key": "KBC.column.order_id.description", + "value": "stale", + "provider": "user", + } + ], + } + service = _make_service(store, mock_client) + + result = service.get_table_detail(alias="prod", table_id="in.c-sales.orders") + + col_map = {c["name"]: c for c in result["column_details"]} + assert col_map["order_id"]["description"] == "current" + def test_surfaces_bucket_backend(self, tmp_path: Path) -> None: """The owning bucket's storage backend is exposed on the response. diff --git a/uv.lock b/uv.lock index 2d96ab21..ee16bd2d 100644 --- a/uv.lock +++ b/uv.lock @@ -581,7 +581,7 @@ wheels = [ [[package]] name = "keboola-cli" -version = "0.87.0" +version = "0.87.1" source = { editable = "." } dependencies = [ { name = "croniter" },