From aba09afb1c53580bb832dcfa0c97c73667d222fa Mon Sep 17 00:00:00 2001 From: Danglewood <85772166+deeleeramone@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:21:39 -0700 Subject: [PATCH] fixes for docstring generation --- .gitignore | 1 + openbb_platform/core/openbb/.build.lock | 1 - .../app/static/package_builder/builder.py | 51 +++- .../package_builder/docstring_generator.py | 138 ++++++++--- .../package_builder/method_definition.py | 55 +++-- .../openbb_core/app/static/utils/linters.py | 2 +- .../test_docstring_generator_helpers.py | 220 +++++++++++++++++- .../test_method_definition_helpers.py | 43 ++++ .../tests/app/static/test_package_builder.py | 25 ++ .../static/test_package_builder_generated.py | 2 +- .../tests/app/static/utils/test_linters.py | 16 ++ openbb_platform/dev_install.py | 129 ++++++++-- .../extensions/mcp_server/tests/conftest.py | 56 +++++ 13 files changed, 659 insertions(+), 80 deletions(-) delete mode 100644 openbb_platform/core/openbb/.build.lock diff --git a/.gitignore b/.gitignore index 04093524cf8..9f8de90db33 100644 --- a/.gitignore +++ b/.gitignore @@ -52,6 +52,7 @@ cli/openbb_cli/assets/styles/user/* # Platform openbb_platform/core/openbb/package/* +openbb_platform/core/openbb/.build.lock # Dev Container env obb/* diff --git a/openbb_platform/core/openbb/.build.lock b/openbb_platform/core/openbb/.build.lock deleted file mode 100644 index 0c3fdca90b6..00000000000 --- a/openbb_platform/core/openbb/.build.lock +++ /dev/null @@ -1 +0,0 @@ -7295 \ No newline at end of file diff --git a/openbb_platform/core/openbb_core/app/static/package_builder/builder.py b/openbb_platform/core/openbb_core/app/static/package_builder/builder.py index 4af694bf41b..752c0246486 100644 --- a/openbb_platform/core/openbb_core/app/static/package_builder/builder.py +++ b/openbb_platform/core/openbb_core/app/static/package_builder/builder.py @@ -1,11 +1,13 @@ """Top-level PackageBuilder coordinator.""" import contextlib +import io import os import re import shutil import signal import sys +import tokenize import traceback from json import dumps, load from pathlib import Path @@ -15,6 +17,53 @@ from typing import ( ) _LIST_ANNOTATION_RE = re.compile(r"\bList\b") +_PROTECTED_TOKENS = { + tokenize.STRING, + tokenize.COMMENT, + *( + getattr(tokenize, name) + for name in ("FSTRING_MIDDLE", "TSTRING_MIDDLE") + if hasattr(tokenize, name) + ), +} + + +def _normalize_code_annotations(code: str) -> str: + """Rewrite ``typing.X`` and ``List`` to builtins outside string literals and comments. + + Parameters + ---------- + code : str + Generated Python source. + + Returns + ------- + str + The source with annotations normalized and docstrings, descriptions, + and comments left untouched. + """ + line_starts = [0] + for line in code.splitlines(keepends=True): + line_starts.append(line_starts[-1] + len(line)) + try: + tokens = list(tokenize.generate_tokens(io.StringIO(code).readline)) + except (tokenize.TokenError, SyntaxError): + return _LIST_ANNOTATION_RE.sub("list", code.replace("typing.", "")) + parts: list[str] = [] + cursor = 0 + for token in tokens: + if token.type not in _PROTECTED_TOKENS: + continue + start = line_starts[token.start[0] - 1] + token.start[1] + end = line_starts[token.end[0] - 1] + token.end[1] + segment = code[cursor:start] + parts.append(_LIST_ANNOTATION_RE.sub("list", segment.replace("typing.", ""))) + parts.append(code[start:end]) + cursor = end + tail = code[cursor:] + parts.append(_LIST_ANNOTATION_RE.sub("list", tail.replace("typing.", ""))) + return "".join(parts) + from importlib_metadata import entry_points @@ -256,7 +305,7 @@ class PackageBuilder: self.console.log(str(package_path)) with package_path.open("w", encoding="utf-8", newline="\n") as file: - file.write(_LIST_ANNOTATION_RE.sub("list", code.replace("typing.", ""))) + file.write(_normalize_code_annotations(code) if extension == "py" else code) @staticmethod def _read(path: Path) -> dict: 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 fe2e24dac38..84aae2f70b1 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 @@ -54,6 +54,74 @@ from openbb_core.app.static.package_builder._indent import ( # noqa: F401 create_indent, ) +_MERGED_PROVIDER_DESCRIPTION_RE = re.compile(r"(\(provider:\s*[^)]+\))\s*;\s*") +_QUOTED_TYPE_NAME_RE = re.compile(r"'([A-Za-z_][\w.]*)'") + + +def _strip_annotated(type_: Any) -> Any: + """Remove ``Annotated`` metadata from a type, recursing into unions and containers. + + Parameters + ---------- + type_ : Any + The type to unwrap. + + Returns + ------- + Any + The type with every ``Annotated[X, ...]`` replaced by ``X``. + """ + origin = get_origin(type_) + if origin is Annotated: + return _strip_annotated(get_args(type_)[0]) + if origin is Union or origin is UnionType: + return Union[tuple(_strip_annotated(arg) for arg in get_args(type_))] # noqa: UP007 + if origin in (list, dict, set, tuple) and get_args(type_): + return origin[tuple(_strip_annotated(arg) for arg in get_args(type_))] + return type_ + + +def _literal_type_name(type_: Any) -> str: + """Return the value type of a ``Literal``, e.g. ``str`` for ``Literal['a', 'b']``. + + Parameters + ---------- + type_ : Any + A ``Literal`` type. + + Returns + ------- + str + The sorted, ``|``-joined type names of the literal's values. + """ + return " | ".join(sorted({type(arg).__name__ for arg in get_args(type_)})) + + +def _literal_choices(type_: Any) -> list: + """Return the values of every ``Literal`` in a type, including inside unions. + + Parameters + ---------- + type_ : Any + The type to inspect. + + Returns + ------- + list + The literal values, in declaration order. + """ + type_ = _strip_annotated(type_) + if get_origin(type_) is Literal: + return list(get_args(type_)) + if get_origin(type_) is Union or get_origin(type_) is UnionType: + return [ + value + for arg in get_args(type_) + if get_origin(arg) is Literal + for value in get_args(arg) + ] + return [] + class DocstringGenerator: """Dynamically generate docstrings for the commands.""" @@ -92,6 +160,13 @@ class DocstringGenerator: if "BeforeValidator" in str(_type): _type = "Optional[int]" if is_optional else "int" + _type = _strip_annotated(_type) + + if isinstance(_type, TypeVar): + _type = "Any" + elif get_origin(_type) is Literal: + _type = _literal_type_name(_type) + origin = get_origin(_type) # On Python 3.10-3.13, ``X | Y`` produces ``types.UnionType`` # whose ``get_origin()`` is ``types.UnionType`` — distinct from @@ -108,6 +183,10 @@ class DocstringGenerator: has_none = True continue if get_origin(arg) is Literal: + type_names.append(_literal_type_name(arg)) + continue + if isinstance(arg, TypeVar): + type_names.append("Any") continue type_name = str(arg) if hasattr(arg, "__forward_arg__"): @@ -124,6 +203,7 @@ class DocstringGenerator: # Unwrap any ForwardRef('X') left inside a container repr, # e.g. ``list[ForwardRef('DataFrame')]`` -> ``list[DataFrame]``. 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 @@ -156,6 +236,8 @@ class DocstringGenerator: .replace("NoneType", "None") .replace(", None", "") ) + if "Literal[" not in _type: + _type = _QUOTED_TYPE_NAME_RE.sub(r"\1", _type) if "openbb_" in str(_type): _type = ( @@ -187,7 +269,7 @@ class DocstringGenerator: providers: str | None, ) -> str: """Get the command output description.""" - available_providers = providers or "Optional[str]" + available_providers = providers or "str | None" indent = 2 obbject_description = ( @@ -196,9 +278,9 @@ class DocstringGenerator: f"{create_indent(indent + 2)}Serializable results.\n" f"{create_indent(indent + 1)}provider : {available_providers}\n" f"{create_indent(indent + 2)}Provider name.\n" - f"{create_indent(indent + 1)}warnings : Optional[list[Warning_]]\n" + f"{create_indent(indent + 1)}warnings : list[Warning_] | None\n" f"{create_indent(indent + 2)}List of warnings.\n" - f"{create_indent(indent + 1)}chart : Optional[Chart]\n" + f"{create_indent(indent + 1)}chart : Chart | None\n" f"{create_indent(indent + 2)}Chart object.\n" f"{create_indent(indent + 1)}extra : dict[str, Any]\n" f"{create_indent(indent + 2)}Extra info.\n" @@ -219,9 +301,9 @@ class DocstringGenerator: " use ``async for row in stream``.\n" f"{create_indent(indent + 1)}id : str\n" f"{create_indent(indent + 2)}Unique identifier of the stream.\n" - f"{create_indent(indent + 1)}provider : Optional[str]\n" + f"{create_indent(indent + 1)}provider : str | None\n" f"{create_indent(indent + 2)}Provider name.\n" - f"{create_indent(indent + 1)}warnings : Optional[list[Warning_]]\n" + f"{create_indent(indent + 1)}warnings : list[Warning_] | None\n" f"{create_indent(indent + 2)}List of warnings.\n" f"{create_indent(indent + 1)}extra : dict[str, Any]\n" f"{create_indent(indent + 2)}Extra info.\n" @@ -283,7 +365,7 @@ class DocstringGenerator: def format_type(type_: str, char_limit: int | None = None) -> str: """Format type in docstrings.""" - type_str = str(type_) + type_str = str(_strip_annotated(type_)) # Apply the standard formatting first type_str = ( @@ -409,7 +491,7 @@ class DocstringGenerator: if line.startswith("Multiple comma separated items allowed"): # Save current provider's choices first if current_provider and current_choices: - provider_choices[current_provider] = " ".join( + provider_choices[current_provider] = "\n".join( current_choices ) current_provider = None @@ -420,7 +502,7 @@ class DocstringGenerator: if line.startswith("Choices for "): # Save previous provider's choices if any if current_provider and current_choices: - provider_choices[current_provider] = " ".join( + provider_choices[current_provider] = "\n".join( current_choices ) @@ -436,7 +518,7 @@ class DocstringGenerator: # Save the last provider's choices if current_provider and current_choices: - provider_choices[current_provider] = " ".join(current_choices) + provider_choices[current_provider] = "\n".join(current_choices) # Extract multiple items text from main_description if not already found if not multi_items_text: @@ -448,9 +530,11 @@ class DocstringGenerator: multi_items_text = multi_match.group().strip() main_description = re.sub(multi_pattern, "", main_description) - # Handle semicolon-separated provider descriptions - if ";" in main_description and "(provider:" in main_description: - parts = main_description.split(";") + if _MERGED_PROVIDER_DESCRIPTION_RE.search(main_description): + pieces = _MERGED_PROVIDER_DESCRIPTION_RE.split(main_description) + parts = [ + pieces[i] + pieces[i + 1] for i in range(0, len(pieces) - 1, 2) + ] + [pieces[-1]] provider_sections = [] # Extract provider tag pattern @@ -613,13 +697,13 @@ class DocstringGenerator: if provider_param: _, description = get_param_info(provider_param) # type: ignore provider_param._annotation = str # type: ignore - docstring += f"{create_indent(2)}provider : str\n" + docstring += f"{create_indent(2)}provider : str | None\n" docstring += f"{create_indent(3)}{format_description(description)}\n" # Explicit parameters for param_name, param in explicit_params.items(): type_, description = get_param_info(param) - type_str = format_type(str(type_), char_limit=86) + type_str = format_type(type_, char_limit=86) docstring += f"{create_indent(2)}{param_name} : {type_str}\n" docstring += f"{create_indent(3)}{format_description(description)}\n" @@ -781,8 +865,7 @@ class DocstringGenerator: line_length > 0 and line_length + len(choice_str) + 2 > line_limit ): - # End the current line - formatted_choices.append("\n") + formatted_choices.append(",\n") line_length = 0 # Add comma and space if not the first choice in the line @@ -998,15 +1081,15 @@ class DocstringGenerator: type_str = cls.get_field_type( p_type, param.default is Parameter.empty ) - # Emit at column 0; a single blanket indent at the end of - # this branch shifts the whole docstring to the method's - # docstring column. Param name aligns with the header, - # description sits one level in. param_section += f"{param_name} : {type_str}\n" if description and description.strip() != '""': param_section += f"{create_indent(1)}{description}\n" + if choices := _literal_choices(p_type): + formatted = ", ".join(repr(choice) for choice in choices) + param_section += f"{create_indent(1)}Choices: {formatted}\n" + result_doc += param_section + "\n" if "returns" in sections and not doc_has_returns: @@ -1046,6 +1129,10 @@ class DocstringGenerator: fields = getattr(return_annotation, "model_fields", {}) for field_name, field in fields.items(): + if type_name.startswith("OBBject") and ( + field_name == "id" + ): + continue field_type = cls.get_field_type( field.annotation, field.is_required ) @@ -1054,17 +1141,10 @@ class DocstringGenerator: if field.description else "" ) - - if type_name.startswith("OBBject"): - if field_name != "id": - returns_section += "\n" - - returns_section += f"{create_indent(1)}{field_name.strip()} : {field_type}" - else: - returns_section += f"{create_indent(1)}{field_name} : {field_type}\n" + returns_section += f"{create_indent(1)}{field_name.strip()} : {field_type}\n" if description: returns_section += ( - f"\n{create_indent(2)}{description}" + f"{create_indent(2)}{description}\n" ) except (AttributeError, TypeError): diff --git a/openbb_platform/core/openbb_core/app/static/package_builder/method_definition.py b/openbb_platform/core/openbb_core/app/static/package_builder/method_definition.py index 18d9bb0b96d..e2c6e7790ed 100644 --- a/openbb_platform/core/openbb_core/app/static/package_builder/method_definition.py +++ b/openbb_platform/core/openbb_core/app/static/package_builder/method_definition.py @@ -3,7 +3,6 @@ import builtins import inspect import re -import textwrap from collections import OrderedDict from collections.abc import Callable from inspect import Parameter, _empty, isclass, signature @@ -1111,6 +1110,32 @@ class MethodDefinition: s = s[7:] return s + def normalize_type_names(text: str) -> str: + """Rewrite ``typing`` and fully-qualified names in a rendered type.""" + text = text.replace("NoneType", "None") + text = text.replace("pandas.core.frame.DataFrame", "pandas.DataFrame") + text = text.replace("openbb_core.provider.abstract.data.Data", "Data") + text = text.replace("ForwardRef('Data')", "Data") + text = text.replace("ForwardRef('DataFrame')", "DataFrame") + text = text.replace("ForwardRef('Series')", "Series") + text = text.replace("ForwardRef('ndarray')", "ndarray") + text = re.sub(r"\bDict\b", "dict", text) + text = re.sub(r"\bList\b", "list", text) + return text.replace("typing.", "") + + def split_preserving_whitespace(text: str, width: int) -> list[str]: + """Split text into chunks of about ``width`` characters that rejoin exactly.""" + chunks: list[str] = [] + current = "" + for word in re.findall(r"\S+\s*|\s+", text): + if current and len(current) + len(word.rstrip()) > width: + chunks.append(current) + current = "" + current += word + if current: + chunks.append(current) + return chunks + def stringify_param(param: Parameter) -> str: """Format a parameter as a string.""" if not ( @@ -1119,10 +1144,10 @@ class MethodDefinition: isinstance(m, OpenBBField) for m in param.annotation.__metadata__ ) ): - return str(param) + return normalize_type_names(str(param)) type_hint = param.annotation.__args__[0] - type_repr = get_type_repr(type_hint) + type_repr = normalize_type_names(get_type_repr(type_hint)) meta = next( m for m in param.annotation.__metadata__ if isinstance(m, OpenBBField) ) @@ -1137,9 +1162,7 @@ class MethodDefinition: if len(desc) <= max_width: desc_repr = repr(desc) else: - parts = textwrap.wrap(desc, width=max_width) - # For function signature context, don't add extra indentation - # The parameter will be properly indented by the calling context + parts = split_preserving_whitespace(desc, max_width) joined = "\n ".join(f"{repr(p)}" for p in parts) desc_repr = f"(\n {joined}" + "\n )" @@ -1166,23 +1189,7 @@ class MethodDefinition: return final_param params_list = [stringify_param(p) for p in formatted_params.values()] - func_params = ",\n ".join(params_list) - - func_params = func_params.replace("NoneType", "None") - func_params = func_params.replace( - "pandas.core.frame.DataFrame", "pandas.DataFrame" - ) - func_params = func_params.replace( - "openbb_core.provider.abstract.data.Data", "Data" - ) - func_params = func_params.replace("ForwardRef('Data')", "Data") - func_params = func_params.replace("ForwardRef('DataFrame')", "DataFrame") - func_params = func_params.replace("ForwardRef('Series')", "Series") - func_params = func_params.replace("ForwardRef('ndarray')", "ndarray") - func_params = func_params.replace("Dict", "dict").replace("List", "list") - func_params = func_params.replace("typing.", "") - - return func_params + return ",\n ".join(params_list) @staticmethod def build_func_returns(return_type: type) -> str: @@ -1263,7 +1270,7 @@ class MethodDefinition: ) code = ( - f'{create_indent(2)}"""{doc}{create_indent(2)}""" # noqa: E501\n\n' + f'{create_indent(2)}"""{doc.rstrip()}\n{create_indent(2)}""" # noqa: E501\n\n' if doc else "" ) diff --git a/openbb_platform/core/openbb_core/app/static/utils/linters.py b/openbb_platform/core/openbb_core/app/static/utils/linters.py index c7ea2735bb9..ffcf2780082 100644 --- a/openbb_platform/core/openbb_core/app/static/utils/linters.py +++ b/openbb_platform/core/openbb_core/app/static/utils/linters.py @@ -55,7 +55,7 @@ class Linters: def ruff(self): """Run ruff.""" - flags = ["check", "--fix", "--unsafe-fixes"] + flags = ["check", "--fix-only", "--unsafe-fixes"] if not self.verbose and not Env().DEBUG_MODE: flags.append("--silent") self.run(linter="ruff", flags=flags) 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 1d677e6852e..5372d2040a8 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 @@ -158,7 +158,7 @@ def test_get_obbject_description_default_provider_placeholder(): out = DocstringGenerator.get_OBBject_description("MyResults", None) assert "OBBject" in out assert "results : MyResults" in out - assert "provider : Optional[str]" in out + assert "provider : str | None" in out assert "warnings" in out assert "chart" in out assert "extra" in out @@ -257,7 +257,7 @@ def test_get_OBBject_description_with_providers(): def test_get_OBBject_description_default_providers(): out = DocstringGenerator.get_OBBject_description("X", None) - assert "Optional[str]" in out + assert "str | None" in out def test_get_OBBStream_description_mirrors_obbject_field_format(): @@ -265,8 +265,8 @@ def test_get_OBBStream_description_mirrors_obbject_field_format(): assert "OBBStream" in out assert "results : AsyncIterator[MyFeed]" in out assert "id : str" in out - assert "provider : Optional[str]" in out - assert "warnings : Optional[list[Warning_]]" in out + assert "provider : str | None" in out + assert "warnings : list[Warning_] | None" in out assert "extra : dict[str, Any]" in out assert "start : (output=None, handler=None) -> OBBStream" in out assert "stop : (timeout=5.0) -> None" in out @@ -1703,3 +1703,215 @@ def test_generate_non_model_strips_parameters_section_for_dependency_params( ) assert "Do a thing." in out assert "The resolved principal." not in out + + +def _param_with_description(name, description): + from inspect import Parameter + + annotation = Annotated[str, OpenBBField(description=description)] + param = Parameter( + name=name, + kind=Parameter.POSITIONAL_OR_KEYWORD, + annotation=annotation, + default=None, + ) + object.__setattr__(param, "_annotation", annotation) + return param + + +def _render_params(**params): + return DocstringGenerator.generate_model_docstring( + model_name="NoSuchModelXYZ", + summary="S", + explicit_params=dict(params), + kwarg_params={}, + returns={}, + results_type="", + sections=["parameters"], + ) + + +def test_generate_model_docstring_keeps_single_provider_semicolons_intact(): + desc = ( + "Data column(s) to return, comma-separated. Choices: capacity (megawatts);" + " outage (megawatts); percent_outage (percent). (provider: eia)" + ) + out = _render_params(data_type=_param_with_description("data_type", desc)) + assert "Data column(s) to return, comma-separated." in out + assert "capacity (megawatts); outage (megawatts); percent_outage (percent)." in out + + +def test_generate_model_docstring_keeps_semicolons_inside_examples(): + desc = ( + "A semicolon delimited list of tag names. Example: 'japan;imports'" + " (provider: fred)" + ) + out = _render_params(tag_names=_param_with_description("tag_names", desc)) + assert "Example: 'japan;imports'" in out + + +def test_generate_model_docstring_splits_merged_provider_descriptions(): + desc = "Alpha detail. (provider: a);\n Beta detail. (provider: b)" + out = _render_params(kind=_param_with_description("kind", desc)) + assert "(provider: a)" in out + assert "Alpha detail." in out + assert "(provider: b)" in out + assert "Beta detail." in out + + +def test_generate_model_docstring_wrapped_choices_keep_separating_commas(): + import re + from types import SimpleNamespace + from typing import Literal + + choices = [f"choice_number_{i:02d}" for i in range(12)] + kwarg_param = SimpleNamespace( + _annotation=Literal["seed"], + type=Literal["seed"], + default=SimpleNamespace( + description="Pick one. (provider: wrap)", + json_schema_extra={"wrap": {"choices": choices}}, + ), + annotation=Literal["seed"], + ) + out = DocstringGenerator.generate_model_docstring( + model_name="NoSuchModelXYZ", + summary="S", + explicit_params={}, + kwarg_params={"wrapped": kwarg_param}, + returns={}, + results_type="", + sections=["parameters"], + ) + assert not re.search(r"'\s+'", out) + assert ", ".join(f"'{c}'" for c in choices) == re.sub( + r",\s+", ", ", out[out.index("'choice_number_00'") : out.rindex("'") + 1] + ) + assert max(len(line) for line in out.splitlines()) < 120 + + +def test_generate_model_docstring_strips_annotated_metadata_from_kwarg_types(): + from types import SimpleNamespace + + from annotated_types import Ge + + kwarg_param = SimpleNamespace( + _annotation=Annotated[int, Ge(0)] | None, + type=Annotated[int, Ge(0)] | None, + default=SimpleNamespace(description="Row limit.", json_schema_extra=None), + annotation=Annotated[int, Ge(0)] | None, + ) + out = DocstringGenerator.generate_model_docstring( + model_name="NoSuchModelXYZ", + summary="S", + explicit_params={}, + kwarg_params={"limit": kwarg_param}, + returns={}, + results_type="", + sections=["parameters"], + ) + assert "limit : int | None" in out + assert "Annotated" not in out + assert "Ge(" not in out + + +def test_generate_model_docstring_provider_type_matches_none_default(): + out = _render_params(provider=_param_with_description("provider", "The provider.")) + assert "provider : str | None" in out + + +def test_get_field_type_optional_literal_renders_value_type(): + from typing import Literal + + assert ( + DocstringGenerator.get_field_type(Literal["a", "b"] | None, False) + == "str | None" + ) + assert DocstringGenerator.get_field_type(Literal["a", "b"], True) == "str" + + +def test_get_field_type_strips_annotated_metadata(): + from annotated_types import Gt + + out = DocstringGenerator.get_field_type(Annotated[float, Gt(0)] | None, False) + assert out == "float | None" + + +def test_get_field_type_unquotes_string_forward_refs(): + forward_ref = "DataFrame" + out = DocstringGenerator.get_field_type(list[forward_ref] | dict, True) + assert out == "dict | list[DataFrame]" + + +def test_get_field_type_typevar_renders_any(): + from typing import TypeVar + + T = TypeVar("T") + assert DocstringGenerator.get_field_type(T | None, False) == "Any | None" + + +def test_generate_non_model_lists_literal_choices_and_hides_obbject_id(monkeypatch): + from inspect import Parameter + from typing import Literal + + from openbb_core.app.static.package_builder import docstring_generator as dg + + class _SS: + class _PS: + docstring_sections = ["description", "parameters", "returns"] + docstring_max_length = None + + python_settings = _PS() + + class _Svc: + system_settings = _SS() + + monkeypatch.setattr(dg, "SystemService", _Svc) + + annotation = Annotated[ + Literal["coal", "petroleum"] | None, OpenBBField(description="The group.") + ] + + async def _func(group: annotation = None) -> OBBject: + """List the datasets.""" + + param = Parameter( + name="group", + kind=Parameter.POSITIONAL_OR_KEYWORD, + annotation=annotation, + default=None, + ) + object.__setattr__(param, "_annotation", annotation) + + out = DocstringGenerator.generate( + path="/x/datasets", + func=_func, + formatted_params=OrderedDict({"group": param}), + model_name=None, + ) + assert "group : str | None" in out + assert "Choices: 'coal', 'petroleum'" in out + assert "id : str" not in out + assert "results : Any | None" in out + assert "warnings : list[Warning_] | None" in out + assert "\n\n\n" not in out + + +def test_generate_model_docstring_strips_annotated_metadata_from_explicit_params(): + from inspect import Parameter + + from annotated_types import Ge + + annotation = Annotated[ + Annotated[int, Ge(0)] | None, OpenBBField(description="Row limit.") + ] + param = Parameter( + name="limit", + kind=Parameter.POSITIONAL_OR_KEYWORD, + annotation=annotation, + default=None, + ) + object.__setattr__(param, "_annotation", annotation) + out = _render_params(limit=param) + assert "limit : int | None" in out + assert "Annotated" not in out diff --git a/openbb_platform/core/tests/app/static/package_builder/test_method_definition_helpers.py b/openbb_platform/core/tests/app/static/package_builder/test_method_definition_helpers.py index 1dc9d7210e7..a4c22b748f5 100644 --- a/openbb_platform/core/tests/app/static/package_builder/test_method_definition_helpers.py +++ b/openbb_platform/core/tests/app/static/package_builder/test_method_definition_helpers.py @@ -1746,3 +1746,46 @@ def test_format_params_expands_query_params_model_field_types(): # ``symbol`` is not in TYPE_EXPANSION -> the declared type is kept as-is. assert get_args(out["symbol"].annotation)[0] is str + + +def test_build_func_params_wrapped_description_keeps_spaces_and_case(): + description = ( + "The provider to use, by default None. If None, the priority list " + "configured in the settings is used. List of Dict values is accepted." + ) + annotation = Annotated[Literal["fred"] | None, OpenBBField(description=description)] + param_map = OrderedDict( + provider=Parameter( + "provider", + Parameter.POSITIONAL_OR_KEYWORD, + annotation=annotation, + default=None, + ) + ) + rendered = MethodDefinition.build_func_params(param_map) + namespace: dict = {} + exec( # noqa: S102 + "from typing import Annotated, Literal\n" + "from openbb_core.app.model.field import OpenBBField\n" + f"def f({rendered}): pass\n", + namespace, + ) + rendered_annotation = namespace["f"].__annotations__["provider"] + assert rendered_annotation.__metadata__[0].description == description + + +def test_build_func_params_normalizes_typing_names_in_types_only(): + from typing import Dict, List # noqa: UP035 + + annotation = Annotated[ + List[Dict[str, int]] | None, # noqa: UP006 + OpenBBField(description="List of Dict rows."), + ] + param_map = OrderedDict( + rows=Parameter( + "rows", Parameter.POSITIONAL_OR_KEYWORD, annotation=annotation, default=None + ) + ) + rendered = MethodDefinition.build_func_params(param_map) + assert "list[dict[str, int]]" in rendered + assert "'List of Dict rows.'" in rendered diff --git a/openbb_platform/core/tests/app/static/test_package_builder.py b/openbb_platform/core/tests/app/static/test_package_builder.py index b4648ebbe9c..83eef5c4b0f 100644 --- a/openbb_platform/core/tests/app/static/test_package_builder.py +++ b/openbb_platform/core/tests/app/static/test_package_builder.py @@ -1326,3 +1326,28 @@ def test_build_purges_on_keyboard_interrupt(tmp_openbb_dir): assert mock_clean.call_count == 2 # console.error should NOT be called for KeyboardInterrupt mock_error.assert_not_called() + + +def test_normalize_code_annotations_leaves_strings_and_comments_untouched(): + from openbb_core.app.static.package_builder.builder import ( + _normalize_code_annotations, + ) + + code = ( + "def f(x: typing.List[int]) -> List:\n" + ' """List of values from typing.Any."""\n' + " y = 'List every row' # List of rows\n" + " return x\n" + ) + out = _normalize_code_annotations(code) + assert "def f(x: list[int]) -> list:" in out + assert '"""List of values from typing.Any."""' in out + assert "'List every row' # List of rows" in out + + +def test_normalize_code_annotations_falls_back_on_untokenizable_code(): + from openbb_core.app.static.package_builder.builder import ( + _normalize_code_annotations, + ) + + assert _normalize_code_annotations("x: typing.List[int] = (") == "x: list[int] = (" diff --git a/openbb_platform/core/tests/app/static/test_package_builder_generated.py b/openbb_platform/core/tests/app/static/test_package_builder_generated.py index 2733b3b2293..62d942a58d8 100644 --- a/openbb_platform/core/tests/app/static/test_package_builder_generated.py +++ b/openbb_platform/core/tests/app/static/test_package_builder_generated.py @@ -217,7 +217,7 @@ def test_generated_command_method_imports_and_has_expected_signature( The generator deliberately emits a superset of imports (``import pandas``, ``import numpy``, ``from openbb_core.app.provider_interface - import OBBject_``, etc.) and relies on ``ruff --fix --unsafe-fixes`` + import OBBject_``, etc.) and relies on ``ruff --fix-only --unsafe-fixes`` in ``_run_linters`` to strip those that are unused by the rendered method body — see the comment at ``ImportDefinition.build`` (package_builder.py ~line 560). With ``lint=True`` the generated diff --git a/openbb_platform/core/tests/app/static/utils/test_linters.py b/openbb_platform/core/tests/app/static/utils/test_linters.py index d7af535be3d..4fd55aa2f89 100644 --- a/openbb_platform/core/tests/app/static/utils/test_linters.py +++ b/openbb_platform/core/tests/app/static/utils/test_linters.py @@ -71,6 +71,22 @@ def test_ruff_strips_unused_imports_via_module_invocation(tmp_path, monkeypatch) assert "import os" not in sample.read_text() +@pytest.mark.skipif( + importlib.util.find_spec("ruff") is None, + reason="ruff not installed in this environment", +) +def test_ruff_does_not_report_unfixable_violations(tmp_path, capfd): + """Leftover violations in generated code are not reported in verbose mode.""" + sample = tmp_path / "sample.py" + sample.write_text("import os\n\nundefined_name\n") + + Linters(tmp_path, verbose=True).ruff() + + assert "import os" not in sample.read_text() + out, err = capfd.readouterr() + assert "F821" not in out + err + + def test_run_logs_not_found_when_module_missing(tmp_path, capsys): """Missing linter should be reported, not silently skipped. diff --git a/openbb_platform/dev_install.py b/openbb_platform/dev_install.py index 91b2bc4b90c..4cfb69feb1a 100644 --- a/openbb_platform/dev_install.py +++ b/openbb_platform/dev_install.py @@ -4,6 +4,7 @@ import argparse import os +import re import shutil import subprocess import sys @@ -23,6 +24,7 @@ PACKAGE_GLOBS = ( ROUTERS_PATH = Path("openbb_platform/extensions") ALWAYS_INSTALLED_ROUTERS = frozenset({"openbb-news"}) UV_BUILD_LOG = "uv_build_frontend=debug" +REQUIREMENT_NAME_RE = re.compile(r"^\s*([A-Za-z0-9][A-Za-z0-9._-]*)") def log(message: str) -> None: @@ -146,8 +148,24 @@ def is_optional_router(package: Path, pyproject: dict) -> bool: ) -def get_install_args(package: Path, pyproject: dict) -> list[str]: - """Return the uv arguments that install a package with its extras and dev group. +def normalize_name(name: str) -> str: + """Return the PEP 503 normalized form of a distribution name. + + Parameters + ---------- + name : str + The distribution name. + + Returns + ------- + str + The lowercase name with runs of ``-``, ``_``, and ``.`` collapsed to ``-``. + """ + return re.sub(r"[-_.]+", "-", name).lower() + + +def get_editable_target(package: Path, pyproject: dict) -> str: + """Return the editable install target for a package, with all of its extras. Parameters ---------- @@ -158,15 +176,62 @@ def get_install_args(package: Path, pyproject: dict) -> list[str]: Returns ------- - list[str] - The editable requirement, followed by the dev group when the package has one. + str + The package path, followed by ``[extra,...]`` when it declares extras. """ extras = sorted(pyproject.get("project", {}).get("optional-dependencies", {})) - target = package.as_posix() + (f"[{','.join(extras)}]" if extras else "") - args = ["--editable", target] - if "dev" in pyproject.get("dependency-groups", {}): - args += ["--group", f"{package.as_posix()}/pyproject.toml:dev"] - return args + return package.as_posix() + (f"[{','.join(extras)}]" if extras else "") + + +def resolve_dev_requirements( + pyprojects: dict[Path, dict], selected: list[Path] +) -> tuple[list[Path], list[str], list[str]]: + """Resolve the ``dev`` dependency groups of the selected packages. + + Parameters + ---------- + pyprojects : dict[Path, dict] + The parsed pyproject.toml of every tracked package, by directory. + selected : list[Path] + The packages being installed. + + Returns + ------- + tuple[list[Path], list[str], list[str]] + Tracked packages that are only dev dependencies and install editable, + requirements to install from the index, and local path-sourced + requirements outside the tracked packages, such as test fixtures, + that are left out. + """ + tracked = { + normalize_name(pyproject["project"]["name"]): package + for package, pyproject in pyprojects.items() + } + editables: list[Path] = [] + requirements: list[str] = [] + skipped: list[str] = [] + for package in selected: + pyproject = pyprojects[package] + sources = { + normalize_name(name): source + for name, source in pyproject.get("tool", {}) + .get("uv", {}) + .get("sources", {}) + .items() + } + for requirement in pyproject.get("dependency-groups", {}).get("dev", []): + match = REQUIREMENT_NAME_RE.match(requirement) + name = normalize_name(match.group(1)) if match else "" + if name in tracked: + target = tracked[name] + if target not in selected and target not in editables: + editables.append(target) + elif isinstance(sources.get(name), dict) and "path" in sources[name]: + if requirement not in skipped: + skipped.append(requirement) + elif requirement not in requirements: + requirements.append(requirement) + return editables, requirements, skipped def main(argv: list[str] | None = None) -> None: @@ -185,17 +250,43 @@ def main(argv: list[str] | None = None) -> None: ) args = parser.parse_args(argv) log(f"Installing into {sys.executable}") + pyprojects = { + package: load_toml(REPO_PATH / package / "pyproject.toml") + for package in get_tracked_packages() + } + selected = [ + package + for package, pyproject in pyprojects.items() + if args.routers or not is_optional_router(package, pyproject) + ] install_args: list[str] = [] - skipped: list[str] = [] - for package in get_tracked_packages(): - pyproject = load_toml(REPO_PATH / package / "pyproject.toml") - if args.routers or not is_optional_router(package, pyproject): - install_args += get_install_args(package, pyproject) - log(f" + {package.as_posix()}") - else: - skipped.append(package.name) - if skipped: - log(f"Skipping routers (pass --routers to include): {', '.join(skipped)}") + for package in selected: + install_args += [ + "--editable", + get_editable_target(package, pyprojects[package]), + ] + log(f" + {package.as_posix()}") + skipped_routers = [ + package.name for package in pyprojects if package not in selected + ] + editables, requirements, skipped_local = resolve_dev_requirements( + pyprojects, selected + ) + for package in editables: + install_args += [ + "--editable", + get_editable_target(package, pyprojects[package]), + ] + log(f" + {package.as_posix()} (dev dependency)") + install_args += requirements + log(f"Dev requirements: {', '.join(requirements)}") + if skipped_routers: + log( + "Skipping routers (pass --routers to include): " + f"{', '.join(skipped_routers)}" + ) + if skipped_local: + log(f"Skipping local test packages: {', '.join(skipped_local)}") run_step( "Resolving, building, and installing with uv " "(package builds and their hook output stream below)", diff --git a/openbb_platform/extensions/mcp_server/tests/conftest.py b/openbb_platform/extensions/mcp_server/tests/conftest.py index d0b0df4c990..e4efec13f4f 100644 --- a/openbb_platform/extensions/mcp_server/tests/conftest.py +++ b/openbb_platform/extensions/mcp_server/tests/conftest.py @@ -1,12 +1,68 @@ """Shared fixtures and ``openbb_core`` submodule bindings for the test suite.""" +import importlib import logging +import shutil import socket +import subprocess import sys import threading import time from collections.abc import Iterator from contextlib import contextmanager +from importlib.metadata import PackageNotFoundError, distribution +from importlib.util import find_spec +from pathlib import Path + +FIXTURE_EXTENSION = Path(__file__).parent / "fixtures" / "openbb_mcp_fixture" + + +def _pip_install_command() -> list[str]: + """Return the command that installs packages into the running interpreter. + + Returns + ------- + list[str] + ``uv pip install`` when uv is available, otherwise ``pip install``. + """ + if find_spec("uv"): + return [ + sys.executable, + "-m", + "uv", + "pip", + "install", + "--python", + sys.executable, + ] + if uv := shutil.which("uv"): + return [uv, "pip", "install", "--python", sys.executable] + return [sys.executable, "-m", "pip", "install"] + + +def _install_fixture_extension() -> None: + """Install the ``openbb-mcp-fixture`` extension the tests dispatch against.""" + try: + distribution("openbb-mcp-fixture") + except PackageNotFoundError: + subprocess.run( # noqa: S603 + [ + *_pip_install_command(), + "--no-deps", + "--editable", + str(FIXTURE_EXTENSION), + ], + check=True, + ) + if str(FIXTURE_EXTENSION) not in sys.path: + sys.path.append(str(FIXTURE_EXTENSION)) + importlib.invalidate_caches() + from openbb_core.app.extension_loader import ExtensionLoader + + type(ExtensionLoader)._instances.pop(ExtensionLoader, None) + + +_install_fixture_extension() import openbb_core.api import openbb_core.api.app_loader