From 69401fd177812b21d6d7b9043b83aef59e0e16a3 Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 22 Jul 2026 07:22:50 +0200 Subject: [PATCH] fix(docs): pin command-reference metavar format as a stable contract (#513) The release-asset reference (help.keboola.com, connection-docs freshness gate) is a published contract, but its option metavar column was rendered via Click's make_metavar(), whose default drifted between releases (bare `TEXT`/`INTEGER` at Click 8.x vs ``/`` later). The downstream connection-docs gate (keboola/connection-docs#1037) detects value-taking options by matching /^<.*>$/ on the metavar span, so a future dependency bump reverting to bare `TEXT` would silently break the docs build. Derive option metavars from Click's version-stable `ParamType.name` instead of `make_metavar()`: value-taking options always render as a `<...>` span (``, ``, ``, `` for choices with literal case preserved), flags carry none. Document the column contract in the module header and add tests that fail CI on a Click/Typer bump that would change the shape, instead of letting the drift surface downstream. --- scripts/gen_command_reference.py | 52 +++++++++++++++++++++- tests/test_gen_command_reference.py | 69 +++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/scripts/gen_command_reference.py b/scripts/gen_command_reference.py index 6ff16ee6..3402b6cf 100644 --- a/scripts/gen_command_reference.py +++ b/scripts/gen_command_reference.py @@ -13,6 +13,18 @@ their subtree, matching scripts/check_command_sync.py -- they are not part of the public documented surface. +Metavar column contract (STABLE -- do not let it drift under a dependency bump): + Value-taking options render as ``| `--flag` `` | required | help |``. + The metavar span is ALWAYS wrapped in angle brackets (````, ````, + ````, ```` for choices); flags carry no metavar span at all. + This is derived from Click's version-stable ``ParamType.name`` (see + ``_stable_option_metavar``), NOT from ``make_metavar()``, whose default + drifted between releases (bare ``TEXT`` at Click 8.x vs ```` later). + A downstream consumer -- the connection-docs freshness gate + (keboola/connection-docs#1037) -- detects value-taking options by matching + ``/^<.*>$/`` on this span, so the shape is a published contract. See issue + #513; ``tests/test_gen_command_reference.py`` fails CI if it drifts. + Usage (run from repo root): python scripts/gen_command_reference.py # print to stdout python scripts/gen_command_reference.py --output PATH # write to file @@ -54,6 +66,44 @@ def _metavar(param: click.Parameter, ctx: click.Context) -> str: return param.make_metavar() # ty: ignore[missing-argument] +# Click's ``ParamType.name`` is a version-stable token; ``make_metavar()`` output +# is not (see module docstring). Map the stable name to the documented metavar so +# the reference-asset contract can't drift under a Typer/Click bump. +_METAVAR_BY_TYPE_NAME: dict[str, str] = { + "text": "str", + "integer": "int", + "integer range": "int", + "float": "float", + "float range": "float", + "path": "path", + "filename": "path", + "file": "file", + "boolean": "bool", + "uuid": "uuid", + "datetime": "datetime", +} + + +def _stable_option_metavar(param: click.Parameter) -> str: + """Angle-bracket-wrapped, Click-version-independent metavar for a value option. + + Choices keep their literal case (````) -- they are + real CLI tokens. Scalars map through ``ParamType.name``; an author-set metavar + wins and is lowercased. Always returns a ``<...>`` span (never empty, never a + bare uppercase ``TEXT``), which is the shape the downstream docs gate matches. + """ + explicit = getattr(param, "metavar", None) + if explicit: + return f"<{explicit.strip('<>[] ').lower()}>" + ptype = param.type + choices = getattr(ptype, "choices", None) + if choices: + return f"<{'|'.join(str(choice) for choice in choices)}>" + type_name = getattr(ptype, "name", None) or "text" + inner = _METAVAR_BY_TYPE_NAME.get(type_name, type_name.replace(" ", "-")) + return f"<{inner}>" + + def _format_param(param: click.Parameter, ctx: click.Context) -> str | None: """Render one parameter as a markdown table row, or None if hidden/help.""" kind = getattr(param, "param_type_name", "") @@ -61,7 +111,7 @@ def _format_param(param: click.Parameter, ctx: click.Context) -> str | None: if getattr(param, "hidden", False) or "--help" in param.opts: return None names = " / ".join(f"`{opt}`" for opt in [*param.opts, *param.secondary_opts]) - metavar = "" if getattr(param, "is_flag", False) else f" `{_metavar(param, ctx)}`" + metavar = "" if getattr(param, "is_flag", False) else f" `{_stable_option_metavar(param)}`" required = "yes" if param.required else "" help_text = (getattr(param, "help", "") or "").replace("\n", " ").strip() return f"| {names}{metavar} | {required} | {help_text} |" diff --git a/tests/test_gen_command_reference.py b/tests/test_gen_command_reference.py index a0e33e11..49441366 100644 --- a/tests/test_gen_command_reference.py +++ b/tests/test_gen_command_reference.py @@ -3,9 +3,11 @@ from __future__ import annotations import importlib.util +import re import sys from pathlib import Path +import click import pytest @@ -67,3 +69,70 @@ def test_header_carries_version(self, reference: str) -> None: from keboola_agent_cli import __version__ assert f"Generated from kbagent v{__version__}" in reference + + +# A value-taking option's metavar span: angle-bracketed, no whitespace inside. +# Choice values (e.g. `readOnly`) keep their literal case, so uppercase is allowed; +# the durable contract the connection-docs gate matches is only ``/^<.*>$/``. +_METAVAR_SPAN = re.compile(r"^<[A-Za-z0-9|_-]+>$") + + +class TestMetavarContract: + """Issue #513: the metavar column is a published contract; pin its shape. + + These tests fail CI on a Typer/Click bump that changes ``make_metavar()`` + (e.g. reverting `` back to bare `TEXT`), so the drift is caught here + instead of silently breaking the downstream connection-docs freshness gate. + """ + + def test_scalar_types_map_to_documented_metavars(self) -> None: + """Each scalar Click type renders as its stable lowercase `<...>` span.""" + stable = _load_script()._stable_option_metavar + cases = { + str: "", + int: "", + float: "", + click.Path(): "", + click.IntRange(0, 5): "", + } + for click_type, expected in cases.items(): + option = click.Option(["--x"], type=click_type) + assert stable(option) == expected + + def test_choice_preserves_literal_case(self) -> None: + """Choice values are real CLI tokens -- their case must not be normalized.""" + stable = _load_script()._stable_option_metavar + option = click.Option(["--role"], type=click.Choice(["admin", "readOnly", "share"])) + assert stable(option) == "" + + def test_explicit_metavar_is_wrapped_and_lowercased(self) -> None: + stable = _load_script()._stable_option_metavar + option = click.Option(["--alias"], metavar="ALIAS") + assert stable(option) == "" + + def test_every_value_option_matches_the_span_contract(self) -> None: + """Every scalar/choice rendering satisfies the downstream `<...>` matcher.""" + stable = _load_script()._stable_option_metavar + for click_type in (str, int, float, click.Path(), click.Choice(["a", "b_c", "D"])): + option = click.Option(["--x"], type=click_type) + assert _METAVAR_SPAN.match(stable(option)) + + def test_flags_carry_no_metavar_span(self, reference: str) -> None: + """A boolean flag row must not gain a `<...>` value span.""" + section = reference.split("## Global options", 1)[1].split("\n## ", 1)[0] + json_row = next(line for line in section.splitlines() if "`--json`" in line) + assert "`<" not in json_row, f"flag row unexpectedly has a metavar: {json_row}" + + def test_no_bare_uppercase_metavar_on_option_rows(self, reference: str) -> None: + """No option row may carry a bare `TEXT`/`INTEGER`/`PATH`/`FLOAT` metavar.""" + offenders = [ + line + for line in reference.splitlines() + if line.startswith("| `--") and re.search(r"`(TEXT|INTEGER|PATH|FLOAT|BOOLEAN)`", line) + ] + assert offenders == [], f"bare uppercase metavars leaked (Click drift?): {offenders[:3]}" + + def test_scalar_and_choice_metavars_documented(self, reference: str) -> None: + """The documented scalar/choice forms are present in the generated asset.""" + for expected in ("``", "``", "``", "``"): + assert expected in reference, f"expected metavar {expected} missing from reference"