diff --git a/cli/openbb_cli/cli.py b/cli/openbb_cli/cli.py index 0e1ad182466..59c664bbc31 100644 --- a/cli/openbb_cli/cli.py +++ b/cli/openbb_cli/cli.py @@ -454,7 +454,9 @@ def _generate_spec( source_url = server_url.rstrip("/") + openapi_path else: source_url = server_url.rstrip("/") + "/openapi.json" - spec_doc = build_spec_document(openapi, base_url=server_url, source_url=source_url) + spec_doc = build_spec_document( + openapi, base_url=server_url, source_url=source_url, headers=headers + ) if not spec_doc["commands"]: sys.stderr.write( f"generated 0 commands from {source_url} — the document exposes no " diff --git a/cli/openbb_cli/codegen/fetcher_gen.py b/cli/openbb_cli/codegen/fetcher_gen.py index 255d3e74055..1dadae5be72 100644 --- a/cli/openbb_cli/codegen/fetcher_gen.py +++ b/cli/openbb_cli/codegen/fetcher_gen.py @@ -15,8 +15,8 @@ from openbb_cli.codegen.pydantic_gen import ( class_name_from, consolidate_imports, generate_class, + query_field_name, render_function_docstring, - safe_field_name, ) _PATH_TEMPLATE_RE = re.compile(r"\{([^}]+)\}") @@ -45,6 +45,7 @@ class FetcherCommandSpec: base_url: str api_prefix: str provider_name: str + headers: dict[str, str] = field(default_factory=dict) @dataclass @@ -123,6 +124,37 @@ def _module_name_from_command(name: str) -> str: return safe.lower() +_COORDINATE_NAME_RE = re.compile(r"(^|[^a-z])(lat|lon|lng|latitude|longitude)", re.I) + + +def _is_coordinate_name(name: str) -> bool: + """Whether a path parameter is a decimal coordinate (never an integer id).""" + return bool(_COORDINATE_NAME_RE.search(name)) + + +def _mark_null_empty(node: Any) -> Any: + """Flag every object schema so its generated class treats ``''`` as null. + + The marker is a vendor extension read by ``generate_class``; nested object + schemas are visited too, so nested classes get the same behavior. + """ + if isinstance(node, list): + return [_mark_null_empty(n) for n in node] + if not isinstance(node, dict): + return node + out = { + k: ( + v + if k in ("enum", "const", "default", "example", "examples") + else _mark_null_empty(v) + ) + for k, v in node.items() + } + if out.get("type") == "object" or "properties" in out: + out["x-null-empty"] = True + return out + + def _path_template_keys(url_path: str) -> list[str]: """Extract ``{placeholder}`` names from a URL template, in declaration order.""" return [m.group(1) for m in _PATH_TEMPLATE_RE.finditer(url_path)] @@ -163,16 +195,30 @@ def _query_params_schema( for key in ("type", "default", "choices"): if key in raw and raw[key] is not None: prop_schema[key] = raw[key] - if raw.get("in") == "path" and prop_schema.get("type") == "number": + if ( + raw.get("in") == "path" + and prop_schema.get("type") == "number" + and not _is_coordinate_name(name) + ): prop_schema["type"] = "integer" if description: prop_schema["description"] = description - if raw.get("is_list"): + if raw.get("is_list") and raw.get("in", "query") == "query": + # Platform convention: a ``str`` field flagged ``multiple_items_allowed`` + # (comma-separated over REST, list-or-str in Python). + prop_schema = {"type": "string", "x-multiple-items": True} + if description: + prop_schema["description"] = description + if raw.get("choices"): + prop_schema["x-choices"] = list(raw["choices"]) + if raw.get("default") is not None: + prop_schema["default"] = raw["default"] + elif raw.get("is_list"): inner = {"type": prop_schema.get("type", "string")} prop_schema = {"type": "array", "items": inner} if description: prop_schema["description"] = description - if raw.get("choices"): + if raw.get("choices") and "x-multiple-items" not in prop_schema: prop_schema["enum"] = list(raw["choices"]) properties[name] = prop_schema if raw.get("required"): @@ -242,13 +288,41 @@ def _data_schema(cmd_spec: dict[str, Any]) -> dict[str, Any]: if inner.get("type") == "array" and isinstance(inner.get("items"), dict): inner = inner["items"] if isinstance(inner.get("oneOf"), list): + merged = _merge_object_variants(inner) concrete = [c for c in inner["oneOf"] if isinstance(c, dict)] - if concrete: + if merged is not None: + inner = merged + elif concrete: inner = concrete[0] return inner return _unwrap_schema_envelopes(response) +def _with_body_field(schema: dict[str, Any]) -> dict[str, Any]: + """Give a schema-less row a ``value`` field for the raw body. + + A text or PDF response has no row schema; the fetcher puts the body in + ``value``. Declaring it keeps the row from being a field-less model, which + the platform's ``to_df`` cannot tabulate. + """ + structured = ( + schema.get("properties") + or schema.get("additionalProperties") + or any(schema.get(k) for k in ("anyOf", "oneOf", "allOf", "items")) + or schema.get("type") not in (None, "object") + ) + if structured: + return schema + return { + "type": "object", + "properties": { + "value": { + "description": "Response body; the API defines no response schema." + } + }, + } + + def _unwrap_schema_envelopes(schema: Any) -> dict[str, Any]: """Strip schema envelopes that ``unpack_response`` strips at runtime.""" if not isinstance(schema, dict): @@ -256,20 +330,134 @@ def _unwrap_schema_envelopes(schema: Any) -> dict[str, Any]: typed: dict[str, Any] = schema if typed.get("type") == "array": return _unwrap_schema_envelopes(typed.get("items")) + choice_rows = _wrapper_choice_rows(typed) + if choice_rows is not None: + return _final_row_schema(choice_rows) + merged = _merge_object_variants(typed) + if merged is not None: + return _unwrap_schema_envelopes(merged) combinator_target = _first_non_null_combinator_variant(typed) if combinator_target is not None: return _unwrap_schema_envelopes(combinator_target) if _is_scalar_schema(typed): return _wrap_scalar_as_value(typed) + return _unwrap_object_schema(typed) + + +def _unwrap_object_schema(typed: dict[str, Any]) -> dict[str, Any]: + """Resolve the row schema for an object schema that has ``properties``.""" props = typed.get("properties") if not isinstance(props, dict) or not props: return typed + if _response_shape(typed)[1] in ("list", "map"): + # The chosen array's items (or a map's entries) *are* the rows: never + # look for another envelope inside a row (an alert with one array of + # records among its fields is still an alert, not that array). + row = _envelope_inner_schema(typed, props) + return typed if row is None else _final_row_schema(row) inner = _envelope_inner_schema(typed, props) if inner is not None: return _unwrap_schema_envelopes(inner) return typed +def _merge_prop_schemas(a: dict[str, Any], b: dict[str, Any]) -> dict[str, Any]: + """Combine one property's schema from two variants of a union.""" + if a == b: + return a + if a.get("type") == "array" and b.get("type") == "array": + ia, ib = a.get("items"), b.get("items") + if isinstance(ia, dict) and isinstance(ib, dict): + return {**a, "items": _merge_prop_schemas(ia, ib)} + if ( + isinstance(a.get("enum"), list) + and isinstance(b.get("enum"), list) + and a.get("type") == b.get("type") + ): + merged = { + **a, + "enum": [*a["enum"], *[e for e in b["enum"] if e not in a["enum"]]], + } + return merged + + # An enum in one variant and the plain type in another: the plain type admits both. + def strip(s): + return { + k: v for k, v in s.items() if k not in ("enum", "example", "description") + } + + if strip(a) == strip(b): + return {k: v for k, v in a.items() if k != "enum"} + variants: list[dict[str, Any]] = [] + for s in (a, b): + for v in s.get("anyOf") or [s]: + if v not in variants: + variants.append(v) + return {"anyOf": variants} + + +def _wrapper_choice_rows(schema: dict[str, Any]) -> dict[str, Any] | None: + """Row schema of a ``oneOf`` whose variants each wrap one container. + + ``{si: {...}}`` | ``{wi: {...}}`` | ``{fs: {...}}`` means "the response is + whichever of these keys is present", so each variant's single property is + unwrapped like any envelope and the rows are the union of what they yield. + Only containers (arrays or objects) count; a one-field *row* is not a wrapper. + """ + for comb in ("oneOf", "anyOf"): + variants = schema.get(comb) + if not isinstance(variants, list) or len(variants) < 2: + continue + rows: list[dict[str, Any]] = [] + for v in variants: + props = v.get("properties") if isinstance(v, dict) else None + if not isinstance(props, dict) or len(props) != 1: + break + only = next(iter(props.values())) + if not ( + isinstance(only, dict) + and (_is_array_schema(only) or only.get("properties")) + ): + break + rows.append(_unwrap_schema_envelopes(only)) + else: + return {"anyOf": rows} + return None + + +def _merge_object_variants(schema: dict[str, Any]) -> dict[str, Any] | None: + """Merge a ``oneOf`` / ``anyOf`` of object shapes into one object schema. + + The union means "any of these shapes", so the row type carries every + variant's fields; a field is required only if all variants require it, and + a property declared differently across variants takes the union of them. + Returns ``None`` unless the union is made only of two or more objects. + """ + for comb in ("oneOf", "anyOf"): + variants = schema.get(comb) + if not isinstance(variants, list): + continue + objs = [v for v in variants if isinstance(v, dict) and v.get("type") != "null"] + if len(objs) < 2 or not all(v.get("properties") for v in objs): + continue + props: dict[str, Any] = {} + for v in objs: + for name, prop in v["properties"].items(): + props[name] = ( + _merge_prop_schemas(props[name], prop) + if name in props + and isinstance(prop, dict) + and isinstance(props[name], dict) + else props.get(name, prop) + ) + required = set.intersection(*(set(v.get("required") or []) for v in objs)) + out: dict[str, Any] = {"type": "object", "properties": props} + if required: + out["required"] = sorted(required) + return out + return None + + def _first_non_null_combinator_variant(schema: dict[str, Any]) -> dict[str, Any] | None: """Return the first non-null variant of a top-level ``oneOf`` / ``anyOf``.""" for combinator in ("oneOf", "anyOf"): @@ -282,36 +470,160 @@ def _first_non_null_combinator_variant(schema: dict[str, Any]) -> dict[str, Any] return None +_LD_KEYS = frozenset({"@context", "@id", "@type", "@vocab", "@version"}) + + +def _is_array_schema(schema: Any) -> bool: + """Whether a property schema describes an array (directly or via a union).""" + if not isinstance(schema, dict): + return False + if schema.get("type") == "array" or "items" in schema: + return True + return any( + _is_array_schema(v) + for comb in ("anyOf", "oneOf") + for v in schema.get(comb) or [] + ) + + +def _array_items(schema: dict[str, Any]) -> dict[str, Any] | None: + """Return the ``items`` schema of an array property (unwrapping unions).""" + if isinstance(schema.get("items"), dict): + return cast(dict[str, Any], schema["items"]) + for comb in ("anyOf", "oneOf"): + for variant in schema.get(comb) or []: + if _is_array_schema(variant): + return _array_items(variant) + return None + + +def _is_map_schema(schema: Any) -> bool: + """Whether a property is a free-form ``{key: value}`` map.""" + return ( + isinstance(schema, dict) + and schema.get("type") == "object" + and not schema.get("properties") + and bool(schema.get("additionalProperties")) + ) + + +def _has_record_items(schema: dict[str, Any]) -> bool: + """Whether an array property's items are objects (records), per the schema.""" + items = _array_items(schema) + return isinstance(items, dict) and not _is_scalar_schema(items) + + +def _response_shape(response: Any) -> tuple[str | None, str | None]: + """Decide how a response object maps to rows, from the response schema alone. + + Returns ``(key, kind)``: ``"list"`` when one property is the data array (the + only array, or the only one whose items are objects; every other property + is metadata), ``"map"`` when the only property is a free-form + ``{key: value}`` map (one row per entry), ``"single"`` when the object is + itself the row, else ``(None, None)``. The data model and the generated + ``unpack_response`` call both use this, so they cannot disagree. + """ + if not isinstance(response, dict) or response.get("type") == "array": + return None, None + props = response.get("properties") + if not isinstance(props, dict) or not props or "results" in props: + return None, None + if _first_non_null_combinator_variant(response) is not None: + return None, None + eff = {k: v for k, v in props.items() if k not in _LD_KEYS} + arrays = [k for k, v in eff.items() if _is_array_schema(v)] + if len(eff) > 1: + # Beside other fields an array of scalars is a field of the row + # (``securityTypes``), not the rows; it only is the rows when it stands alone. + arrays = [k for k in arrays if _has_record_items(eff[k])] + if len(arrays) > 1: + # e.g. ``@graph`` (station objects) beside ``observationStations`` (the + # same stations as URL strings): the schema's item types say which. + arrays = [k for k in arrays if _has_record_items(eff[k])] + if len(arrays) == 1: + return arrays[0], "list" + if len(eff) == 1 and _is_map_schema(next(iter(eff.values()))): + return next(iter(eff)), "map" + if _single_prop_inner_schema(props) is not None: + return None, None # a one-property wrapper object: unwrapped by the model too + return None, "single" + + def _envelope_inner_schema( - typed: dict[str, Any], # noqa: ARG001 - props: dict[str, Any], + typed: dict[str, Any], + props: dict[str, Any], # noqa: ARG001 ) -> dict[str, Any] | None: """Resolve the inner row schema when ``typed`` is a recognized envelope.""" + key, kind = _response_shape(typed) + if key is None or kind is None: + return _single_prop_inner_schema(props) + value = props[key] + if kind == "map": + extra = value.get("additionalProperties") + extra = extra if isinstance(extra, dict) else {} + variants = [ + v + for v in extra.get("anyOf") or [extra] + if isinstance(v, dict) and v.get("type") != "null" + ] + inner = variants[0] if variants else {} + row_props: dict[str, Any] = {"key": {"type": "string"}} + if inner.get("properties"): + row_props.update(inner["properties"]) + else: + row_props["value"] = inner + return {"type": "object", "properties": row_props} + items = _array_items(value) + if isinstance(items, dict) and not _is_scalar_schema(items): + return items + if isinstance(items, dict): + return _wrap_scalar_as_value(items) + return None + + +def _single_prop_inner_schema(props: dict[str, Any]) -> dict[str, Any] | None: + """Inner schema of a one-property envelope that is not an array or map.""" if len(props) == 1: only_value = next(iter(props.values())) if isinstance(only_value, dict): only_dict = cast(dict[str, Any], only_value) - if only_dict.get("type") == "array": - items = only_dict.get("items") - return cast(dict[str, Any], items) if isinstance(items, dict) else None + if _is_array_schema(only_dict): + items = _array_items(only_dict) + return items if isinstance(items, dict) else None if ( only_dict.get("properties") or only_dict.get("oneOf") or only_dict.get("anyOf") ): return only_dict - array_keys = [ - k for k, v in props.items() if isinstance(v, dict) and v.get("type") == "array" - ] - # Only resolve as an envelope if there is exactly one list of all dicts, - # or if the list is the only element present. - if len(array_keys) == 1: - items = props[array_keys[0]].get("items") - if isinstance(items, dict) and not _is_scalar_schema(items): - return cast(dict[str, Any], items) return None +def _unpack_hint(cmd_spec: dict[str, Any]) -> str: + """Extra ``unpack_response`` arguments derived from the response schema.""" + key, kind = _response_shape(cmd_spec.get("response_schema")) + if kind is None: + return "" + if kind == "single": + return ', kind="single"' + return f", rows_key={key!r}, kind={kind!r}" + + +def _final_row_schema(row: dict[str, Any] | None) -> dict[str, Any]: + """Resolve the row schema without unwrapping it any further.""" + if not isinstance(row, dict): + return {} + if _is_scalar_schema(row): + return _wrap_scalar_as_value(row) + merged = _merge_object_variants(row) + if merged is not None: + return merged + variant = _first_non_null_combinator_variant(row) + if variant is not None: + return _final_row_schema(variant) + return row + + def _is_scalar_schema(schema: dict[str, Any]) -> bool: """Return True when the schema represents a primitive (string/number/etc.).""" return schema.get("type") in { @@ -379,13 +691,13 @@ def _query_dict_construction( The Python source for the ``query_dict`` assignment. """ body_props = (cmd_spec.get("request_body_schema") or {}).get("properties") or {} - excluded = set(path_params) | set(body_props) + excluded = {query_field_name(n) for n in (*path_params, *body_props)} range_params = [ p for p in (cmd_spec.get("parameters") or []) if isinstance(p, dict) and p.get("_socrata_op") in {"date_min", "date_max"} ] - excluded.update(p["name"] for p in range_params) + excluded.update(query_field_name(p["name"]) for p in range_params) if excluded: excluded_literal = ( "{" + ", ".join(repr(name) for name in sorted(excluded)) + "}" @@ -422,6 +734,15 @@ def _query_dict_construction( lines.append( f" _query_dict.setdefault('$order', {date_column!r} + ' DESC')" ) + for param in filter_user_params(cmd_spec.get("parameters") or []): + if ( + isinstance(param, dict) + and param.get("is_list") + and param.get("in", "query") == "query" + ): + lines.extend( + _list_param_lines(param["name"], param.get("list_style") or "multi") + ) rename_pairs: list[tuple[str, str]] = [] for param in cmd_spec.get("parameters") or []: if not isinstance(param, dict): @@ -439,6 +760,28 @@ def _query_dict_construction( return "\n".join(lines) +_LIST_DELIMITERS = {"csv": ",", "space": " ", "pipe": "|"} + + +def _list_param_lines(wire_name: str, style: str) -> list[str]: + """Emit lines turning the model's comma-joined string into the wire format. + + ``multi`` becomes a list (``get_querystring`` repeats the key); the other + styles are re-joined with the delimiter the upstream API expects. + """ + key = repr(wire_name) + lines = [ + f" if isinstance(_query_dict.get({key}), str):", + f" _items = [i.strip() for i in _query_dict[{key}].split(',') if i.strip()]", + ] + if style == "multi": + lines.append(f" _query_dict[{key}] = _items") + else: + delimiter = _LIST_DELIMITERS.get(style, ",") + lines.append(f" _query_dict[{key}] = {delimiter!r}.join(_items)") + return lines + + def _socrata_where_clause_lines(range_params: list[dict[str, Any]]) -> list[str]: """Compose runtime lines that fold Socrata range params into ``$where``.""" field_lookups: list[str] = [] @@ -446,7 +789,7 @@ def _socrata_where_clause_lines(range_params: list[dict[str, Any]]) -> list[str] wire_name = param["name"] column = param["_socrata_column"] op = ">=" if param["_socrata_op"] == "date_min" else "<=" - py_field, _ = safe_field_name(wire_name) + py_field = query_field_name(wire_name) var = f"_range_{py_field}" field_lookups.append( f" {var} = getattr(query, {py_field!r}, None)\n" @@ -467,7 +810,9 @@ def _socrata_where_clause_lines(range_params: list[dict[str, Any]]) -> list[str] return out -def _header_dict_construction(creds: dict[str, dict[str, str]]) -> str: +def _header_dict_construction( + creds: dict[str, dict[str, str]], fixed: dict[str, str] | None = None +) -> str: """Compose the ``_headers = {...}`` block for credential headers. Parameters @@ -480,7 +825,7 @@ def _header_dict_construction(creds: dict[str, dict[str, str]]) -> str: str Python source for the headers dict. """ - lines = [" _headers: dict[str, str] = {}"] + lines = [f" _headers: dict[str, str] = {dict(fixed or {})!r}"] for canonical, info in creds.items(): if info["in"] != "header": continue @@ -547,7 +892,7 @@ def generate_fetcher_module(spec: FetcherCommandSpec) -> GeneratedFetcher: fetcher_class = f"{model_name}Fetcher" qp_schema = _query_params_schema(spec.cmd_spec, spec.provider_name) - data_schema = _data_schema(spec.cmd_spec) or {"type": "object"} + data_schema = _with_body_field(_data_schema(spec.cmd_spec) or {"type": "object"}) qp_generated = generate_class( qp_schema, @@ -556,7 +901,7 @@ def generate_fetcher_module(spec: FetcherCommandSpec) -> GeneratedFetcher: docstring=f"Query parameters for {spec.name}.", ) data_generated = generate_class( - data_schema, + _mark_null_empty(data_schema), class_name=data_class, base_class="Data", docstring=f"Response row for {spec.name}.", @@ -566,6 +911,7 @@ def generate_fetcher_module(spec: FetcherCommandSpec) -> GeneratedFetcher: path_params = _path_template_keys(spec.cmd_spec.get("url_path") or "") imports = { + "import base64", "from typing import Any", "from openbb_core.app.model.abstract.error import OpenBBError", "from openbb_core.provider.abstract.annotated_result import AnnotatedResult", @@ -591,7 +937,7 @@ def generate_fetcher_module(spec: FetcherCommandSpec) -> GeneratedFetcher: query_block = _query_dict_construction( spec.cmd_spec, creds, path_params, spec.provider_name ) - header_block = _header_dict_construction(creds) + header_block = _header_dict_construction(creds, spec.headers) cookie_block = _cookie_dict_construction(creds) if _command_is_streaming(spec.cmd_spec): @@ -621,6 +967,7 @@ def generate_fetcher_module(spec: FetcherCommandSpec) -> GeneratedFetcher: is_streaming=True, ) + hint = _unpack_hint(spec.cmd_spec) source = _render_fetcher_source( spec=spec, qp_class=qp_class, @@ -642,6 +989,10 @@ def generate_fetcher_module(spec: FetcherCommandSpec) -> GeneratedFetcher: imports=consolidate_imports(imports), column_metadata=_column_metadata_from_data_schema(data_schema), ) + if hint: + source = source.replace( + "unpack_response(_payload)", f"unpack_response(_payload{hint})" + ) return GeneratedFetcher( module_name=module_name, @@ -1065,7 +1416,9 @@ def _aextract_body( """ lines: list[str] = list(cred_lines) if has_path_params: - sub_args = ", ".join(f"{p}=getattr(query, {p!r})" for p in path_params) + sub_args = ", ".join( + f"{p}=getattr(query, {query_field_name(p)!r})" for p in path_params + ) lines.append(f" _path = {url_path_template!r}.format({sub_args})") else: lines.append(f" _path = {url_path_template!r}") @@ -1084,7 +1437,7 @@ def _aextract_body( body_literal = ( "{" + ", ".join( - f"{name!r}: getattr(query, {safe_field_name(name)[0]!r}, None)" + f"{name!r}: getattr(query, {query_field_name(name)!r}, None)" for name in body_field_names ) + "}" @@ -1103,7 +1456,20 @@ def _aextract_body( lines.append( " _ct = (_resp.headers.get('Content-Type') or '').lower()" ) - lines.append(" _text = await _resp.text()") + lines.append(" _raw = await _resp.read()") + lines.append( + " _textual = not _ct or any(" + "_t in _ct for _t in ('json', 'text', 'xml', 'csv', 'javascript', 'yaml'))" + ) + lines.append(" if _resp.status < 400 and not _textual:") + lines.append( + " return {'rows': [{'value': " + "base64.b64encode(_raw).decode('ascii')}], " + "'metadata': {'content_type': _ct, 'encoding': 'base64'}}" + ) + lines.append( + " _text = _raw.decode(_resp.get_encoding() or 'utf-8', errors='replace')" + ) lines.append(" if _resp.status >= 400:") lines.append( " raise OpenBBError(" @@ -1121,8 +1487,8 @@ def _aextract_body( ) lines.append(" else:") lines.append( - " return {'rows': [{'content': _text, " - "'content_type': _ct}], 'metadata': {}}" + " return {'rows': [{'value': _text}], " + "'metadata': {'content_type': _ct}}" ) lines.append(" _rows, _metadata = unpack_response(_payload)") lines.append(" return {'rows': _rows, 'metadata': _metadata}") diff --git a/cli/openbb_cli/codegen/package_gen.py b/cli/openbb_cli/codegen/package_gen.py index e6b8fc5cb49..07d8a9a6d4b 100644 --- a/cli/openbb_cli/codegen/package_gen.py +++ b/cli/openbb_cli/codegen/package_gen.py @@ -349,13 +349,66 @@ def _build_root_router( if commands: parts.append("") parts.extend(commands) + # The root router's file must not share a name with a namespace router: a + # spec with a top-level ``congress`` namespace under a provider named + # ``congress`` would otherwise overwrite it and the root would import itself. + taken = {r.module_name.lower() for r in sub_routers} + module_name = root_namespace + while module_name.lower() in taken: + module_name = f"{module_name}_root" return GeneratedRouter( - module_name=root_namespace, + module_name=module_name, entry_point_name=root_namespace, source="\n".join(parts).rstrip() + "\n", ) +def _assert_unique_router_modules(routers: list[GeneratedRouter]) -> None: + """Refuse to emit routers that would overwrite each other's files. + + Every router is one module; two sharing a name (case-insensitively, for + case-folding filesystems) means one silently replaces the other and the + survivor can import itself. Router modules only ever import their own + children, so unique names are what keeps the import graph acyclic. + """ + seen: dict[str, str] = {} + for r in routers: + key = r.module_name.lower() + if key in seen: + raise ValueError( + f"Router module name collision: {r.module_name!r} and " + f"{seen[key]!r} would both be written to routers/{key}.py. " + "Rename one namespace in the spec, or use --router-name." + ) + seen[key] = r.module_name + _assert_acyclic(routers) + + +def _assert_acyclic(routers: list[GeneratedRouter]) -> None: + """Raise if the routers' ``from ...routers. import`` graph has a cycle.""" + import re + + edges = { + r.module_name: set(re.findall(r"\.routers\.(\w+) import", r.source)) + for r in routers + } + state: dict[str, int] = {} + + def visit(node: str, path: list[str]) -> None: + if state.get(node) == 2: + return + if state.get(node) == 1: + cycle = " -> ".join([*path[path.index(node) :], node]) + raise ValueError(f"Circular router import in generated package: {cycle}") + state[node] = 1 + for nxt in sorted(edges.get(node, ())): + visit(nxt, [*path, node]) + state[node] = 2 + + for name in edges: + visit(name, []) + + _SKIP_TOP_NAMESPACES: frozenset[str] = frozenset({"coverage"}) @@ -412,6 +465,7 @@ def generate_packages( """ base_url = (spec_doc.get("base_url") or "").rstrip("/") api_prefix = spec_doc.get("api_prefix") or "" + fixed_headers = {str(k): str(v) for k, v in (spec_doc.get("headers") or {}).items()} filtered_commands = { name: cmd for name, cmd in (spec_doc.get("commands") or {}).items() @@ -458,6 +512,7 @@ def generate_packages( base_url=base_url, api_prefix=api_prefix, provider_name="tools", + headers=fixed_headers, ) ) post_commands.append(post) @@ -480,6 +535,7 @@ def generate_packages( base_url=base_url, api_prefix=api_prefix, provider_name=prov, + headers=fixed_headers, ) ) fetchers_by_provider.setdefault(prov, []).append(fetcher) @@ -522,6 +578,7 @@ def generate_packages( root_has_stream=routers.root_has_stream, ) routers.routers.append(root_router) + _assert_unique_router_modules(routers.routers) providers: list[GeneratedProvider] = [] for prov in target_providers: diff --git a/cli/openbb_cli/codegen/post_gen.py b/cli/openbb_cli/codegen/post_gen.py index 3aac2e97f71..3620b375876 100644 --- a/cli/openbb_cli/codegen/post_gen.py +++ b/cli/openbb_cli/codegen/post_gen.py @@ -42,6 +42,7 @@ class PostCommandSpec: base_url: str api_prefix: str provider_name: str + headers: dict[str, str] = field(default_factory=dict) @dataclass @@ -539,6 +540,7 @@ def generate_post_command_module(spec: PostCommandSpec) -> GeneratedPostCommand: cred_lines=cred_lines, data_class=data_class, params=params, + fixed_headers=spec.headers, ) imports = { @@ -628,6 +630,7 @@ def _render_body_block( cred_lines: list[str], data_class: str, params: list[tuple[str, str, str | None, bool, Any]], + fixed_headers: dict[str, str] | None = None, ) -> str: """Render the function body for a POST command. @@ -651,6 +654,8 @@ def _render_body_block( Name of the response Data class. params : list of tuple Function parameters. + fixed_headers : dict, optional + Headers sent with every request (from ``--header`` at spec generation). Returns ------- @@ -703,7 +708,7 @@ def _render_body_block( f' _url = f"{base_url}{{_path}}" + ("?" + _query_string if _query_string else "")' ) - lines.append(" _headers: dict[str, str] = {}") + lines.append(f" _headers: dict[str, str] = {dict(fixed_headers or {})!r}") for canonical, info in creds.items(): if info["in"] != "header": continue diff --git a/cli/openbb_cli/codegen/pydantic_gen.py b/cli/openbb_cli/codegen/pydantic_gen.py index 821dcf6b8f4..82673d11848 100644 --- a/cli/openbb_cli/codegen/pydantic_gen.py +++ b/cli/openbb_cli/codegen/pydantic_gen.py @@ -69,6 +69,20 @@ def safe_field_name(name: str) -> tuple[str, bool]: return (cleaned, cleaned != name) +def query_field_name(name: str) -> str: + """Snake-case attribute name for a query parameter. + + The platform exposes every query parameter in snake_case (``zoneId`` is + passed as ``zone_id``); the original name is kept as the field alias so + the upstream wire name is unchanged. + """ + safe, _ = safe_field_name(name) + snake = re.sub(r"(?<=[a-z0-9])([A-Z])", r"_\1", safe) + snake = re.sub(r"(?<=[A-Z])([A-Z][a-z])", r"_\1", snake) + snake = re.sub(r"_+", "_", snake).lower().strip("_") or "field" + return f"{snake}_" if keyword.iskeyword(snake) else snake + + def class_name_from(*parts: str) -> str: """Join + camel-case a set of name parts into a Python class identifier.""" words: list[str] = [] @@ -185,12 +199,20 @@ def _resolve_union( return "Any" rendered: list[str] = [] - for variant in non_null: + for index, variant in enumerate(non_null, start=1): + # Untitled object variants would all get the same class name and + # collapse into one; number them so every shape stays distinct. + distinct = ( + isinstance(variant, dict) + and len(non_null) > 1 + and (variant.get("properties") or variant.get("type") == "object") + and not variant.get("title") + ) rendered.append( schema_to_type( variant, parent_class_name=parent_class_name, - field_name=field_name, + field_name=f"{field_name}_v{index}" if distinct else field_name, nested_classes=nested_classes, imports=imports, ) @@ -314,6 +336,7 @@ def _build_field_call( def _resolve_field_description( prop: dict[str, Any], field_name: str | None = None, # noqa: ARG001 + include_choices: bool = True, ) -> str: """Pick the description text for a Pydantic Field.""" text = prop.get("description") or prop.get("title") @@ -327,7 +350,7 @@ def _resolve_field_description( if isinstance(prop.get("const"), (str, int, float, bool)) else None ) - if enum: + if enum and include_choices: rendered = ", ".join(str(v) for v in enum) parts.append(f"Choices: {rendered}.") return " ".join(parts).rstrip() @@ -557,9 +580,13 @@ def generate_class( body_lines: list[str] = [] doc_fields: list[tuple[str, str, str | None, bool, Any, bool]] = [] optional_object_fields: list[str] = [] + multiple_items: dict[str, dict[str, Any]] = {} for raw_name, prop_schema in properties.items(): safe_name, needs_alias = safe_field_name(raw_name) + if base_class == "QueryParams": + safe_name = query_field_name(raw_name) + needs_alias = safe_name != raw_name annotation = schema_to_type( prop_schema if isinstance(prop_schema, dict) else {}, parent_class_name=class_name, @@ -569,12 +596,22 @@ def generate_class( ) is_required = raw_name in required_set prop = prop_schema if isinstance(prop_schema, dict) else {} + if prop.get("x-multiple-items"): + extra: dict[str, Any] = {"multiple_items_allowed": True} + if prop.get("x-choices"): + extra["choices"] = list(prop["x-choices"]) + multiple_items[safe_name] = extra if not is_required and (prop.get("type") == "object" or "properties" in prop): optional_object_fields.append(safe_name) has_default = "default" in prop - if not is_required and "None" not in annotation: + nullable = bool(schema.get("x-null-empty")) # response rows: null == empty + if (not is_required or nullable) and "None" not in annotation: annotation = f"{annotation} | None" - description = _resolve_field_description(prop, field_name=raw_name) + # The platform lists a query parameter's choices itself ("Choices for + # : ..."); repeating them in the description duplicates them. + description = _resolve_field_description( + prop, field_name=raw_name, include_choices=base_class != "QueryParams" + ) breakout = _nested_object_breakout(prop) if breakout: description = ( @@ -622,6 +659,34 @@ def generate_class( " return None if v == [] else v" ) + if multiple_items: + imports.add("from pydantic import field_validator") + imports.add("from typing import Any") + names = ", ".join(repr(n) for n in multiple_items) + body_lines.append(f"\n __json_schema_extra__ = {multiple_items!r}") + body_lines.append( + f'\n @field_validator({names}, mode="before")\n' + " @classmethod\n" + " def _join_multiple_items(cls, v: Any) -> Any:\n" + ' """Accept a list and join it into a comma-separated string."""\n' + " if isinstance(v, (list, tuple, set)):\n" + ' return ",".join(str(i) for i in v)\n' + " return v" + ) + + if schema.get("x-null-empty"): + imports.add("from pydantic import model_validator") + imports.add("from typing import Any") + body_lines.append( + '\n @model_validator(mode="before")\n' + " @classmethod\n" + " def _empty_string_is_null(cls, values: Any) -> Any:\n" + ' """Treat empty strings as null."""\n' + " if isinstance(values, dict):\n" + ' return {k: (None if v == "" else v) for k, v in values.items()}\n' + " return values" + ) + summary = docstring or class_name doc_block = _render_class_docstring(summary, doc_fields) diff --git a/cli/openbb_cli/codegen/router_gen.py b/cli/openbb_cli/codegen/router_gen.py index fbf092abb34..474c16b3fe9 100644 --- a/cli/openbb_cli/codegen/router_gen.py +++ b/cli/openbb_cli/codegen/router_gen.py @@ -127,7 +127,7 @@ def generate_routers( collected=out, is_top_level=True, ) - if top_node.cmd_spec is None: + if top_node.cmd_spec is None or top_node.is_namespace: continue function_name = _safe_segment(top_name) description = (top_node.cmd_spec.get("description") or "").strip() @@ -156,6 +156,23 @@ def generate_routers( return out +def _own_commands(node: NamespaceNode) -> list[tuple[str, NamespaceNode]]: + """Return ``(function_name, node)`` for every command a router module hosts. + + Leaf children mount on their parent's router. A node that is both a + command and a namespace (e.g. ``/alerts`` and ``/alerts/{id}`` alongside + ``/alerts/active``) mounts its command on its *own* router, named after + itself; mounting it on the parent would shadow the namespace. + """ + out: list[tuple[str, NamespaceNode]] = [] + if node.is_command and node.is_namespace: + out.append((_safe_segment(node.name), node)) + for child_name, child in sorted(node.children.items()): + if child.is_command and not child.is_namespace: + out.append((_safe_segment(child_name), child)) + return out + + def _emit_router( node: NamespaceNode, *, @@ -206,8 +223,7 @@ def _emit_router( has_stream = any( (fetcher := fetchers_by_command.get(child.full_path)) is not None and fetcher.is_streaming - for child in node.children.values() - if child.is_command and child.cmd_spec is not None + for _, child in _own_commands(node) ) if has_stream: parts.append("from openbb_core.app.model.stream import OBBStream") @@ -228,15 +244,13 @@ def _emit_router( sub_imports.append((sub_module, _safe_segment(sub_name), sub_node.full_path)) post_imports: list[tuple[str, str]] = [] - for child in node.children.values(): - if child.is_command and child.cmd_spec is not None: - post = post_commands_by_command.get(child.full_path) - if post is not None: - post_module = ( - f"{package_name}.providers.{provider_name}.models." - f"{post.module_name}" - ) - post_imports.append((post_module, post.function_name)) + for _, child in _own_commands(node): + post = post_commands_by_command.get(child.full_path) + if post is not None: + post_module = ( + f"{package_name}.providers.{provider_name}.models.{post.module_name}" + ) + post_imports.append((post_module, post.function_name)) if post_imports: parts.append("") @@ -265,10 +279,9 @@ def _emit_router( if sub_imports: parts.append("") - for child_name, child in sorted(node.children.items()): - if not child.is_command or child.cmd_spec is None: + for function_name, child in _own_commands(node): + if child.cmd_spec is None: continue - function_name = _safe_segment(child_name) description = (child.cmd_spec.get("description") or "").strip() fetcher = fetchers_by_command.get(child.full_path) if fetcher is not None: diff --git a/cli/openbb_cli/dispatchers/_unpack.py b/cli/openbb_cli/dispatchers/_unpack.py index 400bf6e6b2b..7cf762cf14a 100644 --- a/cli/openbb_cli/dispatchers/_unpack.py +++ b/cli/openbb_cli/dispatchers/_unpack.py @@ -18,10 +18,51 @@ def safe_json_loads(text: str) -> Any: return _json.loads(sanitized) +def _drop_context(payload: dict[str, Any]) -> dict[str, Any]: + """Remove JSON-LD ``@context`` (syntax, never data) from a mapping.""" + if "@context" in payload and len(payload) > 1: + return {k: v for k, v in payload.items() if k != "@context"} + return payload + + +def _unpack_by_hint( + payload: Any, rows_key: str | None, kind: str +) -> tuple[list[dict[str, Any]], dict[str, Any]] | None: + """Unpack as the response schema says; ``None`` if the payload doesn't fit.""" + if not isinstance(payload, dict): + return None + if kind == "single": + return [payload], {} + value = payload.get(rows_key) if rows_key else None + metadata = {k: v for k, v in payload.items() if k != rows_key} + if kind == "list" and isinstance(value, list): + if all(isinstance(v, dict) for v in value): + return value, metadata + return [{"value": v} for v in value], metadata + if kind == "map" and isinstance(value, dict): + rows = [ + {"key": k, **v} if isinstance(v, dict) else {"key": k, "value": v} + for k, v in value.items() + ] + return rows, metadata + return None + + def unpack_response( payload: Any, + rows_key: str | None = None, + kind: str | None = None, ) -> tuple[list[dict[str, Any]], dict[str, Any]]: - """Split a raw API payload into (rows, metadata).""" + """Split a raw API payload into (rows, metadata). + + ``kind`` (``"list"``, ``"map"`` or ``"single"``) and ``rows_key`` come from + the response schema when the caller knows it; otherwise the payload shape + is guessed. + """ + if kind: + hinted = _unpack_by_hint(payload, rows_key, kind) + if hinted is not None: + return hinted if isinstance(payload, list): if len(payload) == 1: return unpack_response(payload[0]) @@ -31,6 +72,7 @@ def unpack_response( return [{"value": r} for r in payload], {} if not isinstance(payload, dict): return [{"value": payload}], {} + payload = _drop_context(payload) list_keys = [k for k, v in payload.items() if isinstance(v, list)] if len(list_keys) == 1: key = list_keys[0] diff --git a/cli/openbb_cli/dispatchers/http.py b/cli/openbb_cli/dispatchers/http.py index 85f13493f8b..6ff9a933a33 100644 --- a/cli/openbb_cli/dispatchers/http.py +++ b/cli/openbb_cli/dispatchers/http.py @@ -49,6 +49,10 @@ def _help_for_provider(text: str | None, provider: str) -> str | None: return "\n".join(kept) if kept else None +# Wire delimiter per spec ``list_style``; ``multi`` (repeated key) needs none. +_LIST_DELIMITERS = {"csv": ",", "space": " ", "pipe": "|"} + + def _slim_param(p: dict[str, Any]) -> dict[str, Any]: """Strip empty / falsy fields from a normalized parameter entry.""" out: dict[str, Any] = { @@ -58,6 +62,8 @@ def _slim_param(p: dict[str, Any]) -> dict[str, Any]: } if p.get("is_list"): out["is_list"] = True + if p.get("list_style"): + out["list_style"] = p["list_style"] if p.get("json_arg"): out["json_arg"] = True if p.get("required"): @@ -743,16 +749,20 @@ class HttpDispatcher: """Split dispatch params into ``(query, body, headers)`` by spec ``in`` tag.""" cmd_spec = (self._spec_doc.get("commands") or {}).get(command) or {} locations: dict[str, tuple[str, str]] = {} + delimiters: dict[str, str] = {} for p in command_parameters(cmd_spec): if isinstance(p, dict) and p.get("name"): spec_name = p["name"] loc = p.get("in", "query") + delimiter = _LIST_DELIMITERS.get(p.get("list_style", "multi")) for key in { spec_name, spec_name.replace("-", "_"), spec_name.replace("_", "-"), }: locations[key] = (loc, spec_name) + if p.get("is_list") and delimiter: + delimiters[key] = delimiter query: dict[str, Any] = {} body: dict[str, Any] = {} headers: dict[str, Any] = {} @@ -764,7 +774,10 @@ class HttpDispatcher: elif loc == "body": body[name] = value else: - query[name] = value + out_value = value + if name in delimiters and isinstance(value, (list, tuple)): + out_value = delimiters[name].join(str(v) for v in value) + query[name] = out_value return query, body, headers async def _invoke_auth_hook( @@ -1095,7 +1108,7 @@ def http_dispatcher_from_spec( api_prefix=spec_doc.get("api_prefix", "/api/v1"), command_methods=methods, command_url_paths=url_paths, - headers=headers, + headers={**(spec_doc.get("headers") or {}), **(headers or {})} or None, query_params=query_params, spec_doc=spec_doc, auth_hook=auth_hook, diff --git a/cli/openbb_cli/dispatchers/openapi_schema.py b/cli/openbb_cli/dispatchers/openapi_schema.py index 04fe7ce2565..aa025fff598 100644 --- a/cli/openbb_cli/dispatchers/openapi_schema.py +++ b/cli/openbb_cli/dispatchers/openapi_schema.py @@ -119,8 +119,8 @@ def deref_parameter(spec: dict[str, Any], param: dict[str, Any]) -> dict[str, An seen.add(ref) param = resolve_ref(spec, ref) or {} schema = param.get("schema") - if isinstance(schema, dict) and "$ref" in schema: - param = {**param, "schema": resolve_ref(spec, schema["$ref"]) or schema} + if isinstance(schema, dict): + param = {**param, "schema": deref_schema(spec, schema)} return param @@ -346,27 +346,54 @@ def _deref_response(spec: dict[str, Any], response: dict[str, Any]) -> dict[str, return response +def _accepted_content_types(accept: str | None) -> list[str]: + """Media types from an ``Accept`` header value, in the order given.""" + return [ + part.split(";")[0].strip().lower() + for part in (accept or "").split(",") + if part.split(";")[0].strip() + ] + + def extract_response_schema( - spec: dict[str, Any], operation: dict[str, Any] + spec: dict[str, Any], operation: dict[str, Any], accept: str | None = None ) -> dict[str, Any] | None: - """Pull the primary success-response JSON schema for an operation.""" + """Pull the primary success-response JSON schema for an operation. + + When ``accept`` (an ``Accept`` header sent with every request) names a + media type the response declares, that schema is used, since it is the + shape the server will actually return. + """ responses = operation.get("responses") or {} response: dict[str, Any] | None = None - for key in _SUCCESS_PRIORITY: + # Success responses only. ``default`` is the catch-all for *errors* whenever + # the operation declares any explicit status code, so it only counts when it + # is the operation's sole response; a 4xx/5xx schema is never a row model. + success = [ + k + for k in responses + if str(k).upper() in _SUCCESS_PRIORITY[:-1] or str(k).startswith("2") + ] + candidates = sorted( + success, + key=lambda k: ( + _SUCCESS_PRIORITY.index(str(k).upper()) + if str(k).upper() in _SUCCESS_PRIORITY + else len(_SUCCESS_PRIORITY) + ), + ) + if not candidates and set(responses) == {"default"}: + candidates = ["default"] + for key in candidates: candidate = responses.get(key) if isinstance(candidate, dict): response = _deref_response(spec, candidate) break - if response is None: - for value in responses.values(): - if isinstance(value, dict): - response = _deref_response(spec, value) - break if response is None: return None content = response.get("content") or {} media: dict[str, Any] | None = None - for ct in _JSON_CONTENT_TYPES: + for ct in [*_accepted_content_types(accept), *_JSON_CONTENT_TYPES]: candidate = content.get(ct) if isinstance(candidate, dict): media = candidate @@ -438,8 +465,9 @@ def extract_response_schemas( def _resolve_schema(schema: dict[str, Any]) -> tuple[type, list[Any], bool]: """Resolve an OpenAPI parameter schema to ``(py_type, choices, is_list)``.""" - if "anyOf" in schema: - non_null = [s for s in schema["anyOf"] if s.get("type") != "null"] + combinator = "anyOf" if "anyOf" in schema else "oneOf" if "oneOf" in schema else "" + if combinator: + non_null = [s for s in schema[combinator] if s.get("type") != "null"] if not non_null: return (str, [], False) if len(non_null) == 1: @@ -660,6 +688,9 @@ def _strip_placeholders(segment: str) -> str: if j < 0: break out = out[:i] + out[j + 1 :] + # A segment made only of placeholders and punctuation (``{x},{y}``) is not a name. + if not any(ch.isalnum() for ch in out): + return "" return out.strip(".") diff --git a/cli/openbb_cli/dispatchers/spec.py b/cli/openbb_cli/dispatchers/spec.py index a400d80632f..406a7f4c0a2 100644 --- a/cli/openbb_cli/dispatchers/spec.py +++ b/cli/openbb_cli/dispatchers/spec.py @@ -87,6 +87,50 @@ def _operation_providers(op: dict[str, Any]) -> list[str]: return [] +_COLLECTION_FORMATS = {"csv": "csv", "multi": "multi", "ssv": "space", "pipes": "pipe"} + + +_DELIMITED_RE = re.compile( + r"(? str | None: + """List style a parameter's *description* states, e.g. "(comma-delimited)". + + Many specs type such a parameter as a plain string and say in prose that it + takes several values; the wire format is still a delimited list. + """ + match = _DELIMITED_RE.search(description or "") + return _DELIMITED_STYLE[match.group(1).lower()] if match else None + + +def _list_style(param: dict[str, Any], location: str) -> str: + """Return how an array parameter is serialized on the wire. + + One of ``"multi"`` (``a=1&a=2``), ``"csv"`` (``a=1,2``), ``"space"`` + (``a=1 2``) or ``"pipe"`` (``a=1|2``). Follows the OpenAPI 3 ``style`` / + ``explode`` rules (query default: ``form`` + ``explode=true``) and the + Swagger 2 ``collectionFormat`` field. + """ + fmt = param.get("collectionFormat") + if fmt in _COLLECTION_FORMATS: + return _COLLECTION_FORMATS[fmt] + style = param.get("style") + if style == "spaceDelimited": + return "space" + if style == "pipeDelimited": + return "pipe" + explode = param.get("explode") + if explode is None: + explode = ( + style or ("form" if location in ("query", "cookie") else "simple") + ) == "form" + return "multi" if explode and style in (None, "form") else "csv" + + def _normalize_parameter( param: dict[str, Any], providers_set: set[str] | None = None ) -> dict[str, Any] | None: @@ -99,6 +143,12 @@ def _normalize_parameter( schema = param.get("schema", {}) or {} json_arg = bool(param.get("_json_arg")) py_type, enum_choices, is_list = _resolve_schema(schema) + described = None + if not is_list and location in ("query", "header", "cookie"): + described = _described_delimiter( + param.get("description") or schema.get("description") + ) + is_list = described is not None if json_arg: py_type, enum_choices, is_list = str, [], False choices: list[Any] = list(enum_choices) @@ -133,14 +183,22 @@ def _normalize_parameter( example = schema.get("example") type_name = _TYPE_TO_NAME.get(py_type, "string") - if location == "path" and type_name == "number": + if ( + location == "path" + and type_name == "number" + and not re.search(r"(^|[^a-z])(lat|lon|lng|latitude|longitude)", name, re.I) + ): type_name = "integer" + list_style = ( + {"list_style": described or _list_style(param, location)} if is_list else {} + ) return { "name": name, "in": location, "type": type_name, "is_list": is_list, + **list_style, "required": required, "default": default, "choices": choices, @@ -196,6 +254,7 @@ def _build_operation_entry( method: str, op: dict[str, Any], path_item: dict[str, Any] | None = None, + accept: str | None = None, ) -> dict[str, Any]: """Build the per-URL command entry — params, providers, schemas.""" providers = _operation_providers(op) @@ -229,7 +288,7 @@ def _build_operation_entry( "parameters": params, "providers": providers, "request_body_schema": extract_request_body_schema(spec, op), - "response_schema": extract_response_schema(spec, op), + "response_schema": extract_response_schema(spec, op, accept), "response_schemas": extract_response_schemas(spec, op), } @@ -289,7 +348,7 @@ def _count_path_placeholders(url: str) -> int: def build_command_spec( - spec: dict[str, Any], *, api_prefix: str = "/api/v1" + spec: dict[str, Any], *, api_prefix: str = "/api/v1", accept: str | None = None ) -> dict[str, dict[str, Any]]: """Return ``{dotted_command: {url_path, method, description, parameters[]}}``. @@ -310,7 +369,7 @@ def build_command_spec( if not base_cmd: continue groups.setdefault(base_cmd, []).append( - _build_operation_entry(spec, url, method, op, methods) + _build_operation_entry(spec, url, method, op, methods, accept) ) out: dict[str, dict[str, Any]] = {} for cmd, entries in groups.items(): @@ -353,12 +412,27 @@ def build_spec_document( base_url: str, api_prefix: str | None = None, source_url: str | None = None, + headers: dict[str, str] | None = None, ) -> dict[str, Any]: - """Build the on-disk spec dict from a fetched OpenAPI document.""" + """Build the on-disk spec dict from a fetched OpenAPI document. + + Non-credential ``headers`` (from ``--header``) are recorded in the spec and + applied to every request; header parameters of the same name are dropped + from each command so they are not surfaced as arguments. Credential headers + (``API-Key``, ``Authorization``, ...) are left alone: the credential + detection maps them to the generated ``Credentials`` objects. + """ effective_prefix = ( api_prefix if api_prefix is not None else detect_api_prefix(openapi) ) - return { + accept = next( + (v for k, v in (headers or {}).items() if k.lower() == "accept"), None + ) + commands = build_command_spec(openapi, api_prefix=effective_prefix, accept=accept) + headers = _fixed_headers(commands, headers) + if headers: + _strip_fixed_headers(commands, headers) + doc = { "version": SPEC_VERSION, "generated_at": datetime.now(timezone.utc).isoformat(), "generator": _generator_identifier(), @@ -366,10 +440,50 @@ def build_spec_document( "api_version": str(openapi.get("openapi") or openapi.get("swagger") or ""), "base_url": _resolve_base_url(openapi, base_url), "api_prefix": (("/" + effective_prefix.strip("/")) if effective_prefix else ""), - "commands": build_command_spec(openapi, api_prefix=effective_prefix), + "commands": commands, "routers": build_router_map(openapi, api_prefix=effective_prefix), "reference": build_reference(openapi, api_prefix=effective_prefix), } + if headers: + doc["headers"] = dict(headers) + return doc + + +def _fixed_headers( + commands: dict[str, dict[str, Any]], headers: dict[str, str] | None +) -> dict[str, str]: + """Return the ``--header`` entries that are not credentials.""" + from openbb_cli.codegen.credentials import ( + classify_parameter, + is_credential_name, + normalize_credential_key, + ) + + credential_headers = { + normalize_credential_key(prm["name"]) + for cmd in commands.values() + for prm in cmd.get("parameters") or [] + if prm.get("in") == "header" and classify_parameter(prm) is not None + } + return { + k: v + for k, v in (headers or {}).items() + if not is_credential_name(k) + and normalize_credential_key(k) not in credential_headers + } + + +def _strip_fixed_headers( + commands: dict[str, dict[str, Any]], headers: dict[str, str] +) -> None: + """Remove header parameters supplied by ``headers`` (case-insensitive).""" + fixed = {k.lower() for k in headers} + for cmd in commands.values(): + cmd["parameters"] = [ + prm + for prm in cmd.get("parameters") or [] + if not (prm.get("in") == "header" and str(prm.get("name")).lower() in fixed) + ] def _content_hash(spec_doc: dict[str, Any]) -> str: @@ -426,6 +540,7 @@ class SpecDocument(BaseModel): version: int base_url: str api_prefix: str = "" + headers: dict[str, str] = Field(default_factory=dict) commands: dict[str, _CommandSpec] routers: dict[str, Any] = Field(default_factory=dict) reference: dict[str, Any] = Field(default_factory=dict) diff --git a/cli/tests/test_codegen_fetcher_gen.py b/cli/tests/test_codegen_fetcher_gen.py index 8872d96df4e..3e2139dda44 100644 --- a/cli/tests/test_codegen_fetcher_gen.py +++ b/cli/tests/test_codegen_fetcher_gen.py @@ -147,9 +147,9 @@ def test_query_params_schema_handles_list_param_with_choices(): } out = fg._query_params_schema(cmd, provider_name="fmp") tickers = out["properties"]["tickers"] - assert tickers["type"] == "array" - assert tickers["items"] == {"type": "string"} - assert tickers["enum"] == ["AAPL", "MSFT"] + assert tickers["type"] == "string" + assert tickers["x-multiple-items"] is True + assert tickers["x-choices"] == ["AAPL", "MSFT"] assert tickers["description"] == "Tickers." @@ -245,7 +245,7 @@ def test_data_schema_descends_into_obbject_anyof_array_items(): } -def test_data_schema_descends_into_oneof_inside_results(): +def test_data_schema_merges_oneof_variants_inside_results(): response = { "properties": { "results": { @@ -260,7 +260,7 @@ def test_data_schema_descends_into_oneof_inside_results(): } } out = fg._data_schema({"response_schema": response}) - assert out["properties"] == {"a": {"type": "string"}} + assert out["properties"] == {"a": {"type": "string"}, "b": {"type": "integer"}} def test_data_schema_returns_top_level_for_multi_field_non_obbject_response(): @@ -389,11 +389,12 @@ def test_data_schema_top_level_array_of_scalars_unwraps_to_value_field(): } -def test_data_schema_descends_into_single_array_among_scalar_siblings(): - """Multi-property row with exactly one array property → descend into - the array's items, mirroring runtime ``unpack_response`` behavior - (single array among scalar siblings = the data, siblings = metadata). - The auction-shaped wrapper dissolves to the inner ``details`` shape. +def test_data_schema_does_not_unwrap_inside_a_row(): + """Envelopes are unwrapped down to the rows, and no further. + + ``ambs -> auctions`` are single-property wrappers; the auctions are the + rows (that is what ``unpack_response`` returns), so a row that happens to + hold one array of records (``details``) keeps all of its own fields. """ response = { "type": "object", @@ -422,14 +423,43 @@ def test_data_schema_descends_into_single_array_among_scalar_siblings(): }, } out = fg._data_schema({"response_schema": response}) - # Descended ambs → auctions (single-property envelopes) → auction items - # (multi-property with one array sibling ``details``) → details items. - assert "flag" in out.get("properties", {}) - assert "auctionStatus" not in out.get("properties", {}) + assert set(out["properties"]) == { + "auctionStatus", + "operationId", + "operationDate", + "details", + } -def test_data_schema_descends_oneOf_to_first_concrete_variant(): - """Top-level ``oneOf`` (after envelope strip) selects the first dict variant.""" +def test_row_with_several_arrays_is_not_unwrapped_into_one_of_them(): + """An alert has ``affectedZones`` (strings) and ``references`` (records): it is the row.""" + alert = { + "type": "object", + "properties": { + "id": {"type": "string"}, + "affectedZones": {"type": "array", "items": {"type": "string"}}, + "references": { + "type": "array", + "items": { + "type": "object", + "properties": {"identifier": {"type": "string"}}, + }, + }, + }, + } + response = { + "type": "object", + "properties": { + "@graph": {"type": "array", "items": alert}, + "title": {"type": "string"}, + }, + } + out = fg._data_schema({"response_schema": response}) + assert set(out["properties"]) == {"id", "affectedZones", "references"} + + +def test_data_schema_merges_oneOf_variants_after_envelope_strip(): + """A ``oneOf`` of object shapes models every variant, not just the first.""" response = { "type": "object", "properties": { @@ -445,7 +475,7 @@ def test_data_schema_descends_oneOf_to_first_concrete_variant(): }, } out = fg._data_schema({"response_schema": response}) - assert out.get("properties") == {"a": {"type": "string"}} + assert out.get("properties") == {"a": {"type": "string"}, "b": {"type": "integer"}} def test_unwrap_schema_envelopes_returns_empty_for_non_dict_input(): @@ -1096,3 +1126,222 @@ def test_generate_fetcher_module_required_cookie_reaches_the_request(): assert "_cookies['xero-session'] = _cred_xero_session" in src assert "cookies=_cookies" in src ast.parse(src) + + +def test_query_field_name_snake_cases_with_wire_alias(): + from openbb_cli.codegen.pydantic_gen import generate_class, query_field_name + + assert query_field_name("zoneId") == "zone_id" + assert query_field_name("User-Agent") == "user_agent" + assert query_field_name("HTTPStatus") == "http_status" + src = generate_class( + { + "type": "object", + "properties": {"zoneId": {"type": "string"}}, + "required": ["zoneId"], + }, + class_name="Q", + base_class="QueryParams", + ).source + assert "zone_id: str = Field(" in src and "zoneId" in src + + +def test_response_shape_is_decided_by_the_schema_only(): + arr = { + "type": "array", + "items": {"type": "object", "properties": {"a": {"type": "string"}}}, + } + # exactly one array property -> its items are the rows, whatever the siblings + env = { + "properties": { + "@context": {"anyOf": [{"type": "array"}, {"type": "object"}]}, + "title": {"type": "string"}, + "features": arr, + } + } + assert fg._response_shape(env) == ("features", "list") + # items declared without ``type: array`` still count as an array + assert fg._response_shape({"properties": {"x": {"items": {"type": "object"}}}}) == ( + "x", + "list", + ) + # two arrays: ambiguous, so the object itself is the row + two = {"properties": {"a": arr, "b": arr, "c": {"type": "string"}}} + assert fg._response_shape(two) == (None, "single") + assert 'kind="single"' in fg._unpack_hint({"response_schema": two}) + assert ( + fg._unpack_hint({"response_schema": env}) + == ", rows_key='features', kind='list'" + ) + + +def test_unpack_response_follows_schema_hint_and_keeps_everything(): + from openbb_cli.dispatchers._unpack import unpack_response + + payload = { + "@context": ["u"], + "title": "t", + "features": [{"id": 1}], + "other": [{"z": 1}], + } + rows, meta = unpack_response(payload, rows_key="features", kind="list") + assert rows == [{"id": 1}] + assert meta == {"@context": ["u"], "title": "t", "other": [{"z": 1}]} + rows, meta = unpack_response(payload, kind="single") + assert rows == [payload] and meta == {} + # GeoJSON features are returned as the schema defines them: untouched + feat = { + "type": "FeatureCollection", + "features": [{"type": "Feature", "properties": {"a": 1}}], + } + assert ( + unpack_response(feat, rows_key="features", kind="list")[0] == feat["features"] + ) + + +def test_response_shape_uses_item_types_to_pick_among_arrays_and_unpacks_maps(): + obj = {"type": "object", "properties": {"id": {"type": "string"}}} + graph = {"type": "array", "items": obj} + urls = {"type": "array", "items": {"type": "string"}} + # station list: objects in ``@graph``, the same stations as URLs beside it + assert fg._response_shape( + {"properties": {"@graph": graph, "observationStations": urls}} + ) == ("@graph", "list") + # two arrays of objects stay ambiguous: the object is the row (an observation) + assert fg._response_shape( + {"properties": {"a": graph, "b": graph, "t": {"type": "string"}}} + ) == (None, "single") + # a lone free-form map unpacks to key/value rows + mapped = { + "properties": { + "@context": {"type": "array"}, + "locations": {"type": "object", "additionalProperties": {"type": "string"}}, + } + } + assert fg._response_shape(mapped) == ("locations", "map") + row = fg._unwrap_schema_envelopes(mapped) + assert set(row["properties"]) == {"key", "value"} + + from openbb_cli.dispatchers._unpack import unpack_response + + rows, meta = unpack_response( + {"@context": [], "locations": {"A": "x", "B": None}}, + rows_key="locations", + kind="map", + ) + assert rows == [ + {"key": "A", "value": "x"}, + {"key": "B", "value": None}, + ] and meta == {"@context": []} + + +def test_merge_object_variants_unions_fields_enums_and_required(): + status = {"type": "string", "enum": ["Released", "Cancelled"]} + schema = { + "anyOf": [ + { + "properties": { + "id": {"type": "string"}, + "status": status, + "x": {"type": "integer"}, + }, + "required": ["id", "x"], + }, + # results shape: ``status`` is the plain string "Results" (no enum), has its own fields + { + "properties": { + "id": {"type": "string"}, + "status": {"type": "string"}, + "totals": {"type": "number"}, + }, + "required": ["id"], + }, + ] + } + out = fg._merge_object_variants(schema) + assert set(out["properties"]) == {"id", "status", "x", "totals"} + # an enum in one variant and the plain type in another admits both + assert "enum" not in out["properties"]["status"] + # a field is required only if every variant requires it + assert out["required"] == ["id"] + # same-typed enums are unioned + a = {"type": "string", "enum": ["A", "B"]} + b = {"type": "string", "enum": ["B", "C"]} + assert fg._merge_prop_schemas(a, b)["enum"] == ["A", "B", "C"] + # not a union of 2+ objects -> no merge + assert ( + fg._merge_object_variants({"anyOf": [{"type": "string"}, {"type": "null"}]}) + is None + ) + + +def test_scalar_array_beside_other_fields_is_a_field_not_the_rows(): + soma = { + "type": "object", + "properties": { + "soma": { + "type": "object", + "properties": { + "asOfDate": {"type": "string"}, + "wam": {"type": "number"}, + "securityTypes": {"type": "array", "items": {"type": "string"}}, + }, + } + }, + } + out = fg._data_schema({"response_schema": soma}) + assert set(out["properties"]) == {"asOfDate", "wam", "securityTypes"} + # ...but a scalar array standing alone (beside JSON-LD keys) is the rows + types = { + "type": "object", + "properties": { + "@context": {"type": "array"}, + "eventTypes": {"type": "array", "items": {"type": "string"}}, + }, + } + assert set(fg._data_schema({"response_schema": types})["properties"]) == {"value"} + + +def test_oneof_of_wrapper_variants_models_the_rows_inside_each(): + def variant(key, field): + return { + "properties": { + key: { + "type": "object", + "properties": { + "title": {"type": "string"}, + "details": { + "type": "array", + "items": { + "type": "object", + "properties": {field: {"type": "string"}}, + }, + }, + }, + } + } + } + + response = { + "type": "object", + "properties": { + "guidesheet": { + "type": "object", + "oneOf": [ + variant("si", "cusip"), + variant("wi", "cusip"), + variant("fs", "formLine"), + ], + } + }, + } + out = fg._data_schema({"response_schema": response}) + assert set(out["properties"]) == {"cusip", "formLine"} + # one-field *rows* in a union are not wrappers + flat = { + "oneOf": [ + {"properties": {"a": {"type": "string"}}}, + {"properties": {"b": {"type": "string"}}}, + ] + } + assert fg._wrapper_choice_rows(flat) is None diff --git a/cli/tests/test_codegen_package_gen.py b/cli/tests/test_codegen_package_gen.py index a5b26dd7b38..36433ab2176 100644 --- a/cli/tests/test_codegen_package_gen.py +++ b/cli/tests/test_codegen_package_gen.py @@ -495,3 +495,229 @@ def test_build_root_router_imports_stream_and_post_processing_functions(): assert router.source.index("_shape_rows") < router.source.index( 'router = Router(prefix="")' ) + + +def test_spec_headers_are_baked_into_requests_and_not_surfaced_as_params(): + from openbb_cli.dispatchers.spec import build_spec_document + + openapi = { + "openapi": "3.0.0", + "paths": { + "/things": { + "get": { + "parameters": [ + { + "name": "User-Agent", + "in": "header", + "schema": {"type": "string"}, + }, + {"name": "q", "in": "query", "schema": {"type": "string"}}, + ], + "responses": {"200": {"description": "ok"}}, + } + } + }, + } + doc = build_spec_document( + openapi, + base_url="https://x.test", + headers={"user-agent": "me@example.com"}, + ) + assert doc["headers"] == {"user-agent": "me@example.com"} + names = [p["name"] for p in doc["commands"]["things"]["parameters"]] + assert names == ["q"] + + from openbb_cli.codegen.fetcher_gen import ( + FetcherCommandSpec, + generate_fetcher_module, + ) + + fetcher = generate_fetcher_module( + FetcherCommandSpec( + name="things", + cmd_spec=doc["commands"]["things"], + base_url=doc["base_url"], + api_prefix=doc["api_prefix"], + provider_name="x", + headers=doc["headers"], + ) + ) + assert ( + "_headers: dict[str, str] = {'user-agent': 'me@example.com'}" in fetcher.source + ) + + +def test_credential_headers_stay_params_for_credentials_mapping(): + from openbb_cli.dispatchers.spec import build_spec_document + + openapi = { + "openapi": "3.0.0", + "paths": { + "/things": { + "get": { + "parameters": [ + { + "name": "User-Agent", + "in": "header", + "schema": {"type": "string"}, + }, + { + "name": "API-Key", + "in": "header", + "schema": {"type": "string"}, + }, + ], + "responses": {"200": {"description": "ok"}}, + } + } + }, + } + doc = build_spec_document( + openapi, + base_url="https://x.test", + headers={"User-Agent": "ua", "API-Key": "secret"}, + ) + assert doc["headers"] == {"User-Agent": "ua"} + names = [p["name"] for p in doc["commands"]["things"]["parameters"]] + assert names == ["API-Key"] + + +def _list_doc(**param_extra): + from openbb_cli.dispatchers.spec import build_spec_document + + openapi = { + "openapi": "3.0.0", + "paths": { + "/things": { + "get": { + "parameters": [ + { + "name": "area", + "in": "query", + "schema": {"type": "array", "items": {"type": "string"}}, + **param_extra, + } + ], + "responses": {"200": {"description": "ok"}}, + } + } + }, + } + return build_spec_document(openapi, base_url="https://x.test") + + +def test_list_style_recorded_from_openapi_style_and_explode(): + assert _list_doc()["commands"]["things"]["parameters"][0]["list_style"] == "multi" + csv = _list_doc(style="form", explode=False) + assert csv["commands"]["things"]["parameters"][0]["list_style"] == "csv" + pipe = _list_doc(style="pipeDelimited") + assert pipe["commands"]["things"]["parameters"][0]["list_style"] == "pipe" + + +def test_list_query_param_codegen_uses_multiple_items_and_wire_style(): + from openbb_cli.codegen.fetcher_gen import ( + FetcherCommandSpec, + generate_fetcher_module, + ) + + for extra, expect in ( + ({"style": "form", "explode": False}, "_query_dict['area'] = ','.join(_items)"), + ({}, "_query_dict['area'] = _items"), + ): + doc = _list_doc(**extra) + src = generate_fetcher_module( + FetcherCommandSpec( + name="things", + cmd_spec=doc["commands"]["things"], + base_url=doc["base_url"], + api_prefix=doc["api_prefix"], + provider_name="x", + ) + ).source + assert "area: str | None" in src + assert ( + "__json_schema_extra__ = {'area': {'multiple_items_allowed': True}}" in src + ) + assert "def _join_multiple_items" in src + assert expect in src + compile(src, "gen", "exec") + + +def test_http_dispatcher_joins_csv_lists_and_repeats_multi(): + from openbb_cli.dispatchers.http import http_dispatcher_from_spec + + for extra, expected in ( + ({"style": "form", "explode": False}, "CA,NY"), + ({}, ["CA", "NY"]), + ): + d = http_dispatcher_from_spec(_list_doc(**extra)) + query, _, _ = d._partition_params("things", {"area": ["CA", "NY"]}) + assert query["area"] == expected + + +def test_root_router_module_never_collides_with_a_namespace_router(): + import ast + + spec = { + "version": 5, + "base_url": "https://x.test", + "api_prefix": "", + "commands": { + "congress": { + "url_path": "/congress", + "method": "get", + "parameters": [], + "response_schema": {"type": "object"}, + }, + "congress.list": { + "url_path": "/congress/list", + "method": "get", + "parameters": [], + "response_schema": {"type": "object"}, + }, + }, + } + import tempfile + from pathlib import Path + + with tempfile.TemporaryDirectory() as tmp: + pkgs = pkg.generate_packages( + spec, output_root=Path(tmp), provider_name="congress" + ) + routers = pkgs.packages[0].routers.routers + names = [r.module_name for r in routers] + assert len(names) == len(set(names)) + root = next(r for r in routers if r.entry_point_name == "congress") + ns = next(r for r in routers if r.entry_point_name is None) + assert root.module_name != ns.module_name + # the root imports the namespace router, never itself + assert f"routers.{root.module_name} import" not in root.source + for r in routers: + ast.parse(r.source) + + +def test_duplicate_router_modules_are_rejected_at_generation_time(): + import pytest + + from openbb_cli.codegen.package_gen import _assert_unique_router_modules + from openbb_cli.codegen.router_gen import GeneratedRouter + + with pytest.raises(ValueError, match="collision"): + _assert_unique_router_modules( + [GeneratedRouter("a_b", None, ""), GeneratedRouter("A_B", None, "")] + ) + + +def test_router_import_cycles_are_rejected(): + import pytest + + from openbb_cli.codegen.package_gen import _assert_unique_router_modules + from openbb_cli.codegen.router_gen import GeneratedRouter + + a = GeneratedRouter("a", None, "from p.routers.b import router as _b_router\n") + b = GeneratedRouter("b", None, "from p.routers.a import router as _a_router\n") + with pytest.raises(ValueError, match="Circular router import"): + _assert_unique_router_modules([a, b]) + selfref = GeneratedRouter("c", None, "from p.routers.c import router\n") + with pytest.raises(ValueError, match="Circular router import"): + _assert_unique_router_modules([selfref]) diff --git a/cli/tests/test_codegen_pydantic_gen.py b/cli/tests/test_codegen_pydantic_gen.py index 947311334f4..5aa08252033 100644 --- a/cli/tests/test_codegen_pydantic_gen.py +++ b/cli/tests/test_codegen_pydantic_gen.py @@ -2,6 +2,8 @@ from __future__ import annotations +import ast + import pytest from openbb_cli.codegen import pydantic_gen as pg @@ -1018,3 +1020,21 @@ def test_quick_schema_type_renders_datetime_format(): def test_quick_schema_type_unknown_type_falls_back_to_any(): """A schema with no ``type`` and no other handled keys collapses to ``Any``.""" assert pg._quick_schema_type({}) == "Any" + + +def test_union_of_untitled_object_variants_keeps_distinct_classes(): + schema = { + "type": "object", + "properties": { + "loc": { + "oneOf": [ + {"type": "object", "properties": {"type": {"type": "string"}}}, + {"type": "object", "properties": {"city": {"type": "string"}}}, + ] + } + }, + } + src = pg.render_module(pg.generate_class(schema, class_name="Pt")) + assert "city:" in src and "type:" in src + ast.parse(src) + assert len({line for line in src.splitlines() if line.startswith("class ")}) >= 3 diff --git a/cli/tests/test_codegen_router_gen.py b/cli/tests/test_codegen_router_gen.py index 5edff8fad35..330b6a0fa3b 100644 --- a/cli/tests/test_codegen_router_gen.py +++ b/cli/tests/test_codegen_router_gen.py @@ -365,3 +365,45 @@ def test_generate_routers_mixes_namespaces_and_root_commands(): assert "async def ipos(" in out.routers[0].source assert len(out.root_commands) == 1 assert "async def eod(" in out.root_commands[0] + + +def test_generate_routers_command_that_is_also_a_namespace_lives_in_its_own_router(): + """``/alerts`` (command) with ``/alerts/active`` (child) must not shadow the namespace.""" + tree = nt.build_namespace_tree( + { + "alerts": {"providers": ["wx"], "description": "A specific alert."}, + "alerts.active": {"providers": ["wx"], "description": "Active."}, + "alerts.active.count": {"providers": ["wx"], "description": "Count."}, + } + ) + fetchers = { + path: _Fetcher( + module_name=path.replace(".", "_"), + fetcher_class="F", + model_name=rg._model_name_for(path), + ) + for path in ("alerts", "alerts.active", "alerts.active.count") + } + out = rg.generate_routers( + tree, + package_name="pkg", + provider_name="wx", + fetchers_by_command=fetchers, + post_commands_by_command={}, + ) + + by_module = {r.module_name: r for r in out.routers} + # Not mounted on the root router, where it would shadow ``alerts``. + assert out.root_commands == [] + # Mounted on its own namespace router alongside the sub-router include. + assert 'model="Alerts")' in by_module["alerts"].source + assert "async def alerts(" in by_module["alerts"].source + assert ( + 'include_router(_active_router, prefix="/active")' in by_module["alerts"].source + ) + # Nested namespace+command follows the same rule and no longer collides with its include. + assert "async def active(" not in by_module["alerts"].source + assert "async def active(" in by_module["alerts_active"].source + assert "async def count(" in by_module["alerts_active"].source + for r in out.routers: + ast.parse(r.source) diff --git a/cli/tests/test_dispatchers_openapi_schema.py b/cli/tests/test_dispatchers_openapi_schema.py index d1af8cb6afc..62ec9973877 100644 --- a/cli/tests/test_dispatchers_openapi_schema.py +++ b/cli/tests/test_dispatchers_openapi_schema.py @@ -1125,17 +1125,25 @@ def test_deref_response_resolves_ref(): # --- extract_response_schema branches --- -def test_extract_response_schema_falls_back_to_first_status(): - """If no preferred status (200/2XX/201/default) matches, use the first response.""" +def test_extract_response_schema_never_uses_an_error_response(): + """4xx/5xx schemas are not row models, and ``default`` is an error catch-all.""" from openbb_cli.dispatchers.openapi_schema import extract_response_schema - op = { - "responses": { - "418": {"content": {"application/json": {"schema": {"type": "object"}}}} - } + err = {"content": {"application/json": {"schema": {"type": "object"}}}} + assert extract_response_schema({}, {"responses": {"418": err}}) is None + # a redirect-only operation: ``default`` is the error, not the data + assert ( + extract_response_schema({}, {"responses": {"301": {}, "default": err}}) is None + ) + # ``default`` is the success response only when it is the sole response + assert extract_response_schema({}, {"responses": {"default": err}}) == { + "type": "object" + } + # any other 2xx beats ``default`` + ok = {"content": {"application/json": {"schema": {"type": "array"}}}} + assert extract_response_schema({}, {"responses": {"default": err, "206": ok}}) == { + "type": "array" } - schema = extract_response_schema({}, op) - assert schema == {"type": "object"} def test_extract_response_schema_falls_back_to_first_content_type(): @@ -2487,3 +2495,13 @@ def test_fetch_openapi_raises_generic_error_when_response_has_no_parse_error( monkeypatch.setattr(openapi_schema.httpx, "get", lambda *a, **k: _Resp()) with pytest.raises(ValueError, match="Could not load an OpenAPI document"): openapi_schema.fetch_openapi("http://h", path="/custom.json") + + +def test_url_to_command_drops_placeholder_only_segments(): + from openbb_cli.dispatchers.openapi_schema import url_to_command + + assert ( + url_to_command("/gridpoints/{wfo}/{x},{y}/forecast", api_prefix="") + == "gridpoints.forecast" + ) + assert url_to_command("/points/{latitude},{longitude}", api_prefix="") == "points" diff --git a/cli/tests/test_dispatchers_spec.py b/cli/tests/test_dispatchers_spec.py index 5d33384b6af..9cab8b2c067 100644 --- a/cli/tests/test_dispatchers_spec.py +++ b/cli/tests/test_dispatchers_spec.py @@ -1554,3 +1554,37 @@ def test_build_command_spec_ignores_malformed_path_item_parameters(): } out = build_command_spec(openapi) assert [p["name"] for p in out["x"]["parameters"]] == ["kept"] + + +def test_description_stating_a_delimited_list_makes_the_param_an_array(): + from openbb_cli.dispatchers.spec import _normalize_parameter + + def norm(desc, **extra): + return _normalize_parameter( + { + "name": "op", + "in": "query", + "description": desc, + "schema": {"type": "string", "enum": ["Repo", "Reverse Repo"]}, + **extra, + } + ) + + got = norm("The operation types (comma-delimited) by which to filter.") + assert got["is_list"] is True and got["list_style"] == "csv" + assert got["choices"] == ["Repo", "Reverse Repo"] + assert norm("Pipe separated list of ids.")["list_style"] == "pipe" + assert not norm("Values are not comma-delimited.")["is_list"] + assert not norm("A single value.")["is_list"] + # an explicit array schema keeps its own style + arr = _normalize_parameter( + { + "name": "x", + "in": "query", + "description": "comma-delimited", + "style": "form", + "explode": True, + "schema": {"type": "array", "items": {"type": "string"}}, + } + ) + assert arr["list_style"] == "multi" diff --git a/openbb_platform/core/openbb_core/app/static/package_builder/docstring_generator.py b/openbb_platform/core/openbb_core/app/static/package_builder/docstring_generator.py index 84aae2f70b1..9d07e949604 100644 --- a/openbb_platform/core/openbb_core/app/static/package_builder/docstring_generator.py +++ b/openbb_platform/core/openbb_core/app/static/package_builder/docstring_generator.py @@ -123,6 +123,132 @@ def _literal_choices(type_: Any) -> list: return [] +_MODULE_PREFIX_RE = re.compile(r"\b(?:[A-Za-z_]\w*\.)+(?=[A-Za-z_])") + + +def _strip_module_paths(type_name: str) -> str: + """Drop the dotted module path from every class named in a type string. + + ``list[openbb_x.models.A | openbb_x.models.B]`` -> ``list[A | B]``. Literal + values are left alone, since a quoted ``'a.b'`` is data, not a module path. + """ + if "Literal[" in type_name: + return type_name + return _MODULE_PREFIX_RE.sub("", type_name) + + +def _split_top_level(text: str, separator: str) -> list[str]: + """Split ``text`` on ``separator``, ignoring separators inside ``[...]``.""" + parts: list[str] = [] + depth = 0 + current = "" + index = 0 + while index < len(text): + char = text[index] + if char == "[": + depth += 1 + elif char == "]": + depth -= 1 + if depth == 0 and text.startswith(separator, index): + parts.append(current) + current = "" + index += len(separator) + continue + current += char + index += 1 + parts.append(current) + return parts + + +def _matching_bracket(text: str, open_index: int) -> int: + """Index of the ``]`` closing the ``[`` at ``open_index`` (``-1`` if none).""" + depth = 0 + for index in range(open_index, len(text)): + if text[index] == "[": + depth += 1 + elif text[index] == "]": + depth -= 1 + if depth == 0: + return index + return -1 + + +def _rewrite_optional_and_union(type_str: str) -> str: + """Rewrite ``Optional[X]`` / ``Union[X, Y]`` as ``X | None`` / ``X | Y``. + + Brackets are matched, not guessed, so wrapped generics such as + ``Optional[list[Union[A, B]]]`` become ``list[A | B] | None``. The rightmost + wrapper is rewritten first, which is always an innermost one. + """ + token = re.compile(r"\b(Optional|Union)\[") + while True: + matches = list(token.finditer(type_str)) + if not matches: + return type_str + found = matches[-1] + open_index = found.end() - 1 + close_index = _matching_bracket(type_str, open_index) + if close_index < 0: + return type_str + inner = type_str[open_index + 1 : close_index] + if found.group(1) == "Optional": + parts = [inner.strip()] + has_none = True + else: + parts = [p.strip() for p in _split_top_level(inner, ",") if p.strip()] + has_none = any(p in ("None", "NoneType") for p in parts) + parts = [p for p in parts if p not in ("None", "NoneType")] + rewritten = " | ".join(parts) + if has_none: + rewritten += " | None" + type_str = type_str[: found.start()] + rewritten + type_str[close_index + 1 :] + + +def _format_type_string(type_str: str, char_limit: int | None = None) -> str: + """Render a stringified annotation the way docstrings show it.""" + type_str = ( + type_str.replace("", "") + .replace("typing.", "") + .replace("pydantic.types.", "") + .replace("datetime.date", "date") + .replace("datetime.datetime", "datetime") + .replace("NoneType", "None") + ) + + # Optional[X] -> X | None, Union[X, Y, ...] -> X | Y | ... + type_str = _rewrite_optional_and_union(type_str) + + # Simplify Literal[...] to str (choices are shown in the description) + if "Literal[" in type_str: + has_none = type_str.endswith(" | None") + type_str = re.sub(r"Literal\[[^\]]+\]", "str", type_str) + if has_none and not type_str.endswith(" | None"): + type_str += " | None" + + # Clean up ", None" that might be left over + type_str = type_str.replace(", None", "") + + # Deduplicate top-level alternatives (str | str -> str), keeping order + parts = [p.strip() for p in _split_top_level(type_str, " | ")] + if len(parts) > 1: + has_none = "None" in parts + unique = list(OrderedDict.fromkeys(p for p in parts if p != "None")) + type_str = " | ".join(unique) + if has_none: + type_str += " | None" + + # Apply char_limit if specified (simple truncation with bracket balancing) + if char_limit and len(type_str) > char_limit: + truncated = type_str[:char_limit] + open_brackets = truncated.count("[") - truncated.count("]") + if open_brackets > 0: + truncated += "]" * open_brackets + type_str = truncated + + return type_str + + class DocstringGenerator: """Dynamically generate docstrings for the commands.""" @@ -205,18 +331,10 @@ class DocstringGenerator: type_name = re.sub(r"ForwardRef\('([^']+)'\)", r"\1", type_name) type_name = _QUOTED_TYPE_NAME_RE.sub(r"\1", type_name) if "openbb_" in type_name: - # Strip the dotted module path but preserve any - # generic-container prefix like ``list[`` so - # ``list[openbb_x.y.Foo]`` becomes ``list[Foo]``, - # not just ``Foo]``. - bracket_idx = type_name.find("[") - if bracket_idx != -1: - prefix = type_name[: bracket_idx + 1] - tail = type_name[bracket_idx + 1 :] - tail = tail.rsplit(".", 1)[-1] - type_name = prefix + tail - else: - type_name = type_name.rsplit(".", 1)[-1] + # Strip the dotted module path of every class in the + # type, so ``list[openbb_x.y.A | openbb_x.y.B]`` becomes + # ``list[A | B]`` (not just the last name). + type_name = _strip_module_paths(type_name) if type_name != "NoneType": type_names.append(type_name) @@ -240,10 +358,7 @@ class DocstringGenerator: _type = _QUOTED_TYPE_NAME_RE.sub(r"\1", _type) if "openbb_" in str(_type): - _type = ( - str(_type).split(".", maxsplit=1)[0].split("openbb_")[0] - + str(_type).rsplit(".", maxsplit=1)[-1] - ) + _type = _strip_module_paths(str(_type)) # Render optionality as ``X | None`` (PEP 604), not ``Optional[X]``. _type = ( @@ -365,94 +480,7 @@ class DocstringGenerator: def format_type(type_: str, char_limit: int | None = None) -> str: """Format type in docstrings.""" - type_str = str(_strip_annotated(type_)) - - # Apply the standard formatting first - type_str = ( - type_str.replace("", "") - .replace("typing.", "") - .replace("pydantic.types.", "") - .replace("datetime.date", "date") - .replace("datetime.datetime", "datetime") - .replace("NoneType", "None") - ) - - # Convert Optional[X] to X | None - optional_pattern = r"Optional\[(.+?)\]" - optional_match = re.search(optional_pattern, type_str) - if optional_match: - inner = optional_match.group(1) - type_str = type_str.replace(f"Optional[{inner}]", f"{inner} | None") - - # Convert Union[X, Y, ...] to X | Y | ... format - union_pattern = r"Union\[(.+)\]" - union_match = re.search(union_pattern, type_str) - if union_match: - inner = union_match.group(1) - # Split by comma, but be careful with nested types like list[str] - parts = [] - depth = 0 - current = "" - for char in inner: - if char == "[": - depth += 1 - elif char == "]": - depth -= 1 - elif char == "," and depth == 0: - parts.append(current.strip()) - current = "" - continue - current += char - if current.strip(): - parts.append(current.strip()) - # Remove None and NoneType from parts, we'll add | None at the end if needed - has_none = any(p in ("None", "NoneType") for p in parts) - parts = [p for p in parts if p not in ("None", "NoneType")] - type_str = " | ".join(parts) - if has_none: - type_str += " | None" - - # Simplify Literal[...] to str (choices shown in description) - # Handle Literal[...] | None -> str | None - if "Literal[" in type_str: - # Check if there's | None at the end - has_none = type_str.endswith(" | None") - # Replace any Literal[...] with str - type_str = re.sub(r"Literal\[[^\]]+\]", "str", type_str) - # Ensure | None is preserved - if has_none and not type_str.endswith(" | None"): - type_str += " | None" - - # Clean up ", None" that might be left over - type_str = type_str.replace(", None", "") - - # Deduplicate types while preserving order (e.g. str | str | str -> str) - if " | " in type_str: - parts = [p.strip() for p in type_str.split(" | ")] - has_none = "None" in parts - # Remove None for now, deduplicate, then add back - parts = [p for p in parts if p != "None"] - # Deduplicate while preserving order - seen: set[str] = set() - unique_parts = [] - for p in parts: - if p not in seen: - seen.add(p) - unique_parts.append(p) - type_str = " | ".join(unique_parts) - if has_none: - type_str += " | None" - - # Apply char_limit if specified (simple truncation with bracket balancing) - if char_limit and len(type_str) > char_limit: - truncated = type_str[:char_limit] - open_brackets = truncated.count("[") - truncated.count("]") - if open_brackets > 0: - truncated += "]" * open_brackets - type_str = truncated - - return type_str + return _format_type_string(str(_strip_annotated(type_)), char_limit) def format_schema_description(description: str) -> str: """Format description in docstrings.""" diff --git a/openbb_platform/core/tests/app/static/package_builder/test_docstring_generator_helpers.py b/openbb_platform/core/tests/app/static/package_builder/test_docstring_generator_helpers.py index a1ff4feff55..980fabc78ea 100644 --- a/openbb_platform/core/tests/app/static/package_builder/test_docstring_generator_helpers.py +++ b/openbb_platform/core/tests/app/static/package_builder/test_docstring_generator_helpers.py @@ -1941,3 +1941,78 @@ def test_literal_choices_reads_bare_optional_and_non_literal_types(): assert _literal_choices(Literal["a", "b"]) == ["a", "b"] assert _literal_choices(Literal["a", "b"] | None) == ["a", "b"] assert _literal_choices(int | None) == [] + + +# --- _format_type_string ----------------------------------------------------- + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + # the regressions: a wrapped generic holding a union + ("typing.Optional[list[typing.Union[A, B, C]]]", "list[A | B | C] | None"), + ("Optional[list[Union[A, B]]]", "list[A | B] | None"), + ("Optional[dict[str, list[Union[A, B]]]]", "dict[str, list[A | B]] | None"), + # things that already worked + ("typing.Optional[str]", "str | None"), + ("Optional[list[str]]", "list[str] | None"), + ("typing.Union[str, int, NoneType]", "str | int | None"), + ("typing.Union[list[str], str]", "list[str] | str"), + ("list[typing.Optional[str]]", "list[str | None]"), + ("", "str"), + ("datetime.date", "date"), + # Literal still collapses to str; duplicates still collapse + ("typing.Optional[typing.Literal['a', 'b']]", "str | None"), + ("typing.Union[typing.Literal['a'], typing.Literal['b']]", "str"), + ("str | str | None", "str | None"), + # a union inside brackets is not de-duplicated across the brackets + ("list[A | A] | None", "list[A | A] | None"), + ("list[A | B] | list[A | B] | None", "list[A | B] | None"), + ], +) +def test_format_type_string(raw, expected): + """Type strings are rewritten with brackets matched, never guessed.""" + from openbb_core.app.static.package_builder.docstring_generator import ( + _format_type_string, + ) + + assert _format_type_string(raw) == expected + + +def test_format_type_string_never_produces_unbalanced_brackets(): + """Every rewrite leaves the brackets balanced, including truncation.""" + from openbb_core.app.static.package_builder.docstring_generator import ( + _format_type_string, + ) + + raw = "typing.Optional[list[typing.Union[" + ", ".join(f"Item{i}" for i in range(12)) + "]]]" + full = _format_type_string(raw) + assert full.count("[") == full.count("]") and full.endswith("] | None") + cut = _format_type_string(raw, char_limit=40) + assert cut.count("[") == cut.count("]") + + +def test_get_field_type_strips_every_module_path_in_a_container(): + """``list[openbb_x.A | openbb_x.B | openbb_x.C]`` keeps all three names.""" + from pydantic import BaseModel + + from openbb_core.app.static.package_builder.docstring_generator import ( + DocstringGenerator, + _format_type_string, + _strip_module_paths, + ) + + def render(annotation, required): + # the docs run ``get_field_type`` and then ``_format_type_string`` + return _format_type_string(DocstringGenerator.get_field_type(annotation, required)) + + def model(name): + return type(name, (BaseModel,), {"__module__": "openbb_fake.providers.models"}) + + a, b, c = model("ItemA"), model("ItemB"), model("ItemC") + assert render(list[a | b | c] | None, False) == "list[ItemA | ItemB | ItemC] | None" + assert render(list[a], True) == "list[ItemA]" + assert render(a | None, False) == "ItemA | None" + assert _strip_module_paths("dict[str, openbb_x.m.A]") == "dict[str, A]" + # quoted data in a Literal is not a module path + assert _strip_module_paths("Literal['a.b']") == "Literal['a.b']"