From 16e7e7ab45b9c8dc2e441430b84c1e7e6151eec1 Mon Sep 17 00:00:00 2001 From: Gadi Evron Date: Thu, 28 May 2026 13:05:12 -0700 Subject: [PATCH] fix(reporter): correct build_pipeline_output return annotation + docstring to tuple[str, int] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_pipeline_output(...) was declared ``-> str`` with a docstring claiming it returns the single output_path, but its sole return is ``return output_path, len(findings_data)`` — a tuple[str, int]. The annotation and docstring went stale when the function gained a second return value; the binding caller (cli.py cmd_report) was migrated to unpack the tuple but the signature + docstring were not. Fix is annotation/docstring-only (runtime return unchanged, since the caller depends on the tuple): - ``-> str`` -> ``-> tuple[str, int]`` - Returns: docstring now describes the (output_path, findings_count) tuple. No cli.py edit: the sole binding caller already unpacks the tuple correctly and now type-aligns with the corrected annotation. New regression test pins the real contract (return is a 2-tuple) and asserts the annotation + docstring agree with it. RED -> GREEN: 2 failed -> 3 passed. ruff + ast clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- libs/openant-core/core/reporter.py | 5 +- ...t_build_pipeline_output_return_contract.py | 89 +++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 libs/openant-core/tests/report/test_build_pipeline_output_return_contract.py diff --git a/libs/openant-core/core/reporter.py b/libs/openant-core/core/reporter.py index 9536c4de..bc8ed587 100644 --- a/libs/openant-core/core/reporter.py +++ b/libs/openant-core/core/reporter.py @@ -187,7 +187,7 @@ def build_pipeline_output( application_type: str = "web_app", processing_level: str | None = None, step_reports: list[dict] | None = None, -) -> str: +) -> tuple[str, int]: """Build ``pipeline_output.json`` from analysis results. Reads ``results.json`` or ``results_verified.json`` and transforms @@ -206,7 +206,8 @@ def build_pipeline_output( step_reports: Optional list of step report dicts for duration/cost info. Returns: - The *output_path* written to. + A ``(output_path, findings_count)`` tuple: the *output_path* written + to and the number of findings emitted into ``pipeline_output.json``. """ print(f"[Report] Building pipeline_output.json...", file=sys.stderr) diff --git a/libs/openant-core/tests/report/test_build_pipeline_output_return_contract.py b/libs/openant-core/tests/report/test_build_pipeline_output_return_contract.py new file mode 100644 index 00000000..8ac1a8d4 --- /dev/null +++ b/libs/openant-core/tests/report/test_build_pipeline_output_return_contract.py @@ -0,0 +1,89 @@ +"""Regression test for the ``build_pipeline_output`` return-contract bug. + +``build_pipeline_output`` was annotated ``-> str`` +with a docstring claiming it returns "The output_path written to" (a single +str), but its sole ``return`` statement is ``return output_path, +len(findings_data)`` — a ``tuple[str, int]``. The annotation/docstring grew +stale when the function gained a second return value (the findings count); +the one binding caller (``openant/cli.py`` ``cmd_report``) was migrated to +unpack the tuple, but the signature + docstring were left behind. + +This test pins the *real* runtime contract: the function returns a 2-tuple of +``(output_path: str, findings_count: int)``, and the declared return +annotation must agree with that real return type. It fails at base because +the annotation is ``str`` while the value is a tuple. +""" + +import json +import sys +import typing +from pathlib import Path + +import pytest + +# Allow `import core.reporter` when tests run from repo root or elsewhere. +_CORE_ROOT = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(_CORE_ROOT)) + +from core import reporter # noqa: E402 + + +@pytest.fixture +def results_file(tmp_path: Path) -> Path: + """Minimal results.json (no confirmed findings) accepted by the reporter.""" + results = { + "results": [], + "code_by_route": {}, + "metrics": {"total": 0, "vulnerable": 0, "safe": 0}, + } + path = tmp_path / "results.json" + path.write_text(json.dumps(results)) + return path + + +def test_returns_tuple_of_path_and_findings_count(tmp_path: Path, results_file: Path): + """The actual return value is a 2-tuple ``(str, int)``, not a bare str.""" + out_path = tmp_path / "pipeline_output.json" + ret = reporter.build_pipeline_output( + results_path=str(results_file), + output_path=str(out_path), + repo_name="example/return-contract", + language="python", + ) + + assert isinstance(ret, tuple), f"expected a tuple return, got {type(ret).__name__}" + assert len(ret) == 2, f"expected a 2-tuple, got len {len(ret)}" + path, findings_count = ret + assert isinstance(path, str) + assert path == str(out_path) + assert isinstance(findings_count, int) + + +def test_return_annotation_matches_real_tuple_contract(): + """The declared ``-> ...`` annotation must agree with the real tuple return. + + At base the annotation is ``str`` (single value) which contradicts the + real ``tuple[str, int]`` return — this assertion is the RED. + """ + hints = typing.get_type_hints(reporter.build_pipeline_output) + ret_ann = hints.get("return") + + origin = typing.get_origin(ret_ann) + assert origin is tuple, ( + "build_pipeline_output return annotation must be a tuple to match its " + f"real `return output_path, len(findings_data)`; got {ret_ann!r}" + ) + + args = typing.get_args(ret_ann) + assert args == (str, int), ( + f"return annotation should be tuple[str, int]; got tuple{list(args)!r}" + ) + + +def test_docstring_no_longer_claims_single_path_return(): + """The Returns: docstring must not claim a single ``output_path`` return.""" + doc = reporter.build_pipeline_output.__doc__ or "" + assert "The *output_path* written to." not in doc, ( + "docstring still claims a single output_path return; it should describe " + "the (output_path, findings_count) tuple" + )