From aeab837db2d9ba8cde340797936799685f0d0e89 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Fri, 18 Sep 2026 14:06:38 +0530 Subject: [PATCH] fix: do not offer a parameter the spec marks internal A spec can carry `x-internal: true` on a parameter the server accepts but does not document; the generated clients keep it, so it cannot be dropped upstream. The walker skips it the way it skips a deprecated one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ --- src/unstract_cli/core/params.py | 11 ++++-- tests/test_params.py | 60 +++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/unstract_cli/core/params.py b/src/unstract_cli/core/params.py index fdf9f36..670f960 100644 --- a/src/unstract_cli/core/params.py +++ b/src/unstract_cli/core/params.py @@ -121,13 +121,18 @@ def _from_schema( ) +def _hidden(node: dict[str, Any]) -> bool: + return bool(node.get("deprecated") or node.get("x-internal")) + + def operation_params(product: str, operation_id: str) -> list[Param]: """Every parameter one operation accepts: query, then request body. Path parameters are excluded: they are the route, supplied by the command from configuration, not by the caller as a flag. So are deprecated ones: a superseded spelling the client still accepts would otherwise become a second - flag for the same value. + flag for the same value. And so are internal ones (`x-internal`): the server + accepts them and the clients keep them, but they are not offered. """ operation = find_operation(product, operation_id) params = [ @@ -138,7 +143,7 @@ def operation_params(product: str, operation_id: str) -> list[Param]: required=bool(p.get("required")), ) for p in operation.get("parameters", []) - if p.get("in") == "query" and not p.get("deprecated") + if p.get("in") == "query" and not _hidden(p) ] body = operation.get("requestBody", {}).get("content", {}) @@ -151,7 +156,7 @@ def operation_params(product: str, operation_id: str) -> list[Param]: schema = _resolve_ref(product, ref) mandatory = set(schema.get("required") or ()) for name, prop in (schema.get("properties") or {}).items(): - if prop.get("deprecated"): + if _hidden(prop): continue params.append( _from_schema( diff --git a/tests/test_params.py b/tests/test_params.py index 20e9e36..cc0c787 100644 --- a/tests/test_params.py +++ b/tests/test_params.py @@ -8,6 +8,7 @@ from __future__ import annotations import click +import click.testing import pytest from unstract.api_deployments.client import APIDeploymentsClient from unstract.llmwhisperer.client_v2 import LLMWhispererClientV2 @@ -70,6 +71,65 @@ def test_the_uploaded_document_is_not_a_flag(): assert find_operation("llmwhisperer", "extract")["method"] == "post" +#: A spec carrying the marker on one query parameter and one body property. +MARKED_SPEC = { + "paths": { + "/run": { + "post": { + "operationId": "run", + "parameters": [ + {"name": "mode", "in": "query", "schema": {"type": "string"}}, + { + "name": "trace", + "in": "query", + "schema": {"type": "boolean"}, + "x-internal": True, + }, + ], + "requestBody": { + "content": { + "multipart/form-data": { + "schema": { + "properties": { + "timeout": {"type": "integer"}, + "use_file_history": { + "type": "boolean", + "default": False, + "x-internal": True, + }, + } + } + } + } + }, + } + } + } +} + + +def test_an_internal_parameter_is_not_offered(monkeypatch): + """The server accepts it and the generated clients keep it, so it stays in + the spec; the marker is the only thing saying it is not for callers.""" + monkeypatch.setattr(params_module, "load_spec", lambda product: MARKED_SPEC) + + assert {p.name for p in operation_params("docstudio", "run")} == {"mode", "timeout"} + + +def test_an_internal_parameter_is_unknown_to_the_parser(monkeypatch): + monkeypatch.setattr(params_module, "load_spec", lambda product: MARKED_SPEC) + command = click.Command( + "run", + params=[click_option(p, {}) for p in operation_params("docstudio", "run")], + callback=lambda **_: None, + ) + + for flag in ("--trace", "--use-file-history"): + result = click.testing.CliRunner().invoke(command, [flag]) + assert result.exit_code == 2, flag + assert "No such option" in result.output, flag + + def test_an_unknown_operation_names_itself(): with pytest.raises(KeyError, match="whisper_sideways"): find_operation("llmwhisperer", "whisper_sideways")