diff --git a/packages/tangle-cli/src/tangle_cli/__init__.py b/packages/tangle-cli/src/tangle_cli/__init__.py index f087eea..9241a25 100644 --- a/packages/tangle-cli/src/tangle_cli/__init__.py +++ b/packages/tangle-cli/src/tangle_cli/__init__.py @@ -14,6 +14,6 @@ try: __version__ = metadata_version("tangle-cli") except PackageNotFoundError: - __version__ = "0.1.15" + __version__ = "0.1.16" __all__ = ["TangleDynamicDiscoveryClient", "__version__"] diff --git a/packages/tangle-cli/src/tangle_cli/component_from_func.py b/packages/tangle-cli/src/tangle_cli/component_from_func.py index 7ea20f3..a286cde 100644 --- a/packages/tangle-cli/src/tangle_cli/component_from_func.py +++ b/packages/tangle-cli/src/tangle_cli/component_from_func.py @@ -2020,6 +2020,15 @@ def build_component_dict( # ============================================================================ +def _within(path: Path, root: Path) -> bool: + """Whether *path* lies inside *root*.""" + try: + path.resolve().relative_to(root.resolve()) + except ValueError: + return False + return True + + def generate_component_yaml( file_path: Path, output_path: Path, @@ -2035,12 +2044,21 @@ def generate_component_yaml( emit_generation_annotations: bool = True, path_annotation_mode: Literal["oss", "td_legacy"] = "oss", unwrapped_inputs: dict[str, Any] | None = None, + logical_output_path: Path | None = None, ) -> bool: """Generate a component YAML file from a Python function. Args: file_path: Path to the Python source file output_path: Where to write the generated YAML + logical_output_path: Where the component is to be UNDERSTOOD to live, + for provenance only. Defaults to ``output_path``. Pass this when + the file is written somewhere incidental -- a private staging + directory, a scratch area -- so the recorded provenance describes + the component rather than the accident of where bytes landed. + Provenance is otherwise derived from the physical path, which + would embed that location and make the emitted YAML, and therefore + its digest, differ between runs of identical source. container_image: Docker image reference function_name: Function to extract (auto-detected if None) dependencies_from: Path to pyproject.toml with pip dependencies @@ -2116,7 +2134,6 @@ def generate_component_yaml( deps = read_dependencies(dependencies_from) # 4. Build annotations - directory = file_path.parent.resolve() module_code = file_path.read_text() annotations: dict[str, str] = { @@ -2148,9 +2165,18 @@ def generate_component_yaml( # basename-only paths outside a git checkout to preserve historical # snapshots. resolved_source = file_path.resolve() - resolved_output = output_path.resolve() + # Provenance describes where the component LIVES, which is not always + # where this call happens to write it. Kept UNRESOLVED: ``td_legacy`` + # annotates the lexical basename, so resolving here would rewrite the + # recorded name whenever the output is a symlink. + annotation_output = logical_output_path if logical_output_path is not None else output_path + resolved_output = annotation_output.resolve() common_dir = Path(os.path.commonpath([resolved_source, resolved_output])) - git_root = get_git_root(directory) + # Discover from the SOURCE's real directory: a symlinked source file + # whose link lives outside the checkout would otherwise find no repo + # and drop every git annotation. + source_dir = resolved_source.parent + git_root = get_git_root(source_dir) use_common_paths = path_annotation_mode == "oss" or git_root is not None def _path_annotation(path: Path) -> str: @@ -2163,7 +2189,7 @@ def _path_annotation(path: Path) -> str: if not strip_source_path: annotations["python_original_code_path"] = _path_annotation(file_path) - annotations["component_yaml_path"] = _path_annotation(output_path) + annotations["component_yaml_path"] = _path_annotation(annotation_output) if emit_generation_annotations: if dependencies_from: annotations["tangle_cli_generation_dependencies_from"] = _path_annotation(dependencies_from) @@ -2172,7 +2198,12 @@ def _path_annotation(path: Path) -> str: # Git info — use the same common ancestor as git_relative_dir when common paths are active. if git_root: - git_info = get_git_info(common_dir) + # Read the repository from a directory KNOWN to be inside it. The + # common ancestor of source and output need not be: any output + # outside the checkout (a configured output_folder, a staging dir) + # drags it out, and reading git there returns nothing, silently + # publishing a component with no repository of origin. + git_info = get_git_info(common_dir if _within(common_dir, git_root) else source_dir) git_info.pop("_git_root", None) # Override git_relative_dir to be the common ancestor try: @@ -2181,7 +2212,7 @@ def _path_annotation(path: Path) -> str: pass annotations.update(git_info) else: - git_info = get_git_info(directory) + git_info = get_git_info(source_dir) git_info.pop("_git_root", None) annotations.update(git_info) diff --git a/packages/tangle-cli/src/tangle_cli/component_generator.py b/packages/tangle-cli/src/tangle_cli/component_generator.py index 7efc56b..0a19cf2 100644 --- a/packages/tangle-cli/src/tangle_cli/component_generator.py +++ b/packages/tangle-cli/src/tangle_cli/component_generator.py @@ -128,6 +128,7 @@ def generate_component_yaml( resolve_root: Path | None = None, emit_generation_annotations: bool = True, unwrapped_inputs: dict[str, Any] | None = None, + logical_output_path: Path | None = None, ) -> bool: """Generate component YAML from a Python function source file. @@ -147,6 +148,8 @@ def generate_component_yaml( unwrapped_inputs: Optional persisted unwrap schema. Hydrate forwards this ``local_from_python.unwrapped_inputs`` payload so component generation expands dict parameters exactly as compile did. + logical_output_path: Where the component is to be understood to + live, for provenance only. Defaults to ``output_path``. Returns: True when generation succeeds, otherwise False. @@ -154,6 +157,12 @@ def generate_component_yaml( from tangle_cli.component_from_func import generate_component_yaml + # Forwarded only when set. These hops are overridable, and an older + # override written against the previous signature must keep working + # for every caller that does not use the seam. + seam: dict[str, Any] = ( + {} if logical_output_path is None else {"logical_output_path": logical_output_path} + ) return generate_component_yaml( file_path=file_path, output_path=output_path, @@ -168,6 +177,7 @@ def generate_component_yaml( resolve_root=resolve_root, emit_generation_annotations=emit_generation_annotations, unwrapped_inputs=unwrapped_inputs, + **seam, ) def regenerate_yaml( @@ -184,6 +194,7 @@ def regenerate_yaml( resolve_root: Path | None = None, emit_generation_annotations: bool = True, unwrapped_inputs: dict[str, Any] | None = None, + logical_output_path: Path | None = None, ) -> bool: """Regenerate a YAML component from a Python function source file. @@ -202,6 +213,10 @@ def regenerate_yaml( emit_generation_annotations: Whether to emit regeneration metadata. unwrapped_inputs: Optional ``local_from_python.unwrapped_inputs`` schema to preserve compile-time unwrap expansion at hydrate time. + logical_output_path: Where the component is to be understood to + live, for provenance only. Defaults to ``output_path``. Note + that the image is still read back from the PHYSICAL output, so + a caller writing to a fresh location supplies ``image``. Returns: True when regeneration succeeds, otherwise False. @@ -218,6 +233,9 @@ def regenerate_yaml( self._log(f" Found dependencies: {deps_file}") final_output.parent.mkdir(parents=True, exist_ok=True) + seam: dict[str, Any] = ( + {} if logical_output_path is None else {"logical_output_path": logical_output_path} + ) return self.run_generation( python_file=python_file, final_output=final_output, @@ -231,6 +249,7 @@ def regenerate_yaml( resolve_root=resolve_root, emit_generation_annotations=emit_generation_annotations, unwrapped_inputs=unwrapped_inputs, + **seam, ) def run_generation( @@ -248,6 +267,7 @@ def run_generation( resolve_root: Path | None = None, emit_generation_annotations: bool = True, unwrapped_inputs: dict[str, Any] | None = None, + logical_output_path: Path | None = None, ) -> bool: """Execute component generation and clean up partial output on failure. @@ -265,6 +285,8 @@ def run_generation( emit_generation_annotations: Whether to emit regeneration metadata. unwrapped_inputs: Optional persisted unwrap schema forwarded to the low-level generator for compile/hydrate interface parity. + logical_output_path: Where the component is to be understood to + live, for provenance only. Defaults to ``final_output``. Returns: True when generation succeeds, otherwise False. On failure, any @@ -274,6 +296,9 @@ def run_generation( try: function_detail = f" function {func_name!r}" if func_name else "" self._log(f" Generating component from {python_file.name}{function_detail}...") + seam: dict[str, Any] = ( + {} if logical_output_path is None else {"logical_output_path": logical_output_path} + ) success = self.generate_component_yaml( file_path=python_file, output_path=final_output, @@ -287,6 +312,7 @@ def run_generation( resolve_root=resolve_root, emit_generation_annotations=emit_generation_annotations, unwrapped_inputs=unwrapped_inputs, + **seam, ) if not success: self._log(" ❌ Failed to generate component", err=True) @@ -340,6 +366,7 @@ def regenerate_yaml( resolve_root: Path | None = None, logger: Any | None = None, unwrapped_inputs: dict[str, Any] | None = None, + logical_output_path: Path | None = None, ) -> bool: """Regenerate component YAML through the default generator. @@ -358,11 +385,16 @@ def regenerate_yaml( logger: Optional logger object used by the generator. unwrapped_inputs: Optional persisted unwrap schema from ``local_from_python.unwrapped_inputs``. + logical_output_path: Where the component is to be understood to live, + for provenance only. Defaults to ``output_path``. Returns: True when regeneration succeeds, otherwise False. """ + seam: dict[str, Any] = ( + {} if logical_output_path is None else {"logical_output_path": logical_output_path} + ) return ComponentGenerator(logger=logger, verbose=verbose).regenerate_yaml( python_file=python_file, output_path=output_path, @@ -375,6 +407,7 @@ def regenerate_yaml( mode=mode, resolve_root=resolve_root, unwrapped_inputs=unwrapped_inputs, + **seam, ) diff --git a/pyproject.toml b/pyproject.toml index ddfb5bf..8a061ee 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "tangle-cli" -version = "0.1.15" +version = "0.1.16" description = "CLI for Tangle, the open-source ML pipeline orchestration platform" readme = "README.md" authors = [ diff --git a/tests/test_logical_output_provenance.py b/tests/test_logical_output_provenance.py new file mode 100644 index 0000000..1c334c7 --- /dev/null +++ b/tests/test_logical_output_provenance.py @@ -0,0 +1,243 @@ +"""Provenance must describe the component, not where its bytes were written. + +A generated component carries annotations saying where it came from: +``component_yaml_path``, ``python_original_code_path`` and the ``git_*`` set. +Those are derived from the output path, so writing to an incidental location -- +a configured ``output_folder`` outside the checkout, a private staging +directory -- silently changes what the component claims about itself, and +changes its bytes, and therefore its digest, between runs of identical source. + +These drive the real generator end to end. A stub cannot show any of this, +because the behaviour under test IS the derivation. +""" +from __future__ import annotations + +import subprocess +from pathlib import Path +from typing import Any + +import pytest +import yaml + +from tangle_cli.component_generator import regenerate_yaml + +_SOURCE = '''\ +def load_orders(limit: int = 10) -> dict: + """Load Orders + + version: "1.0" + """ + return {"limit": limit} +''' + + +@pytest.fixture() +def repo(tmp_path: Path) -> Path: + """A real checkout with a commit and an origin.""" + root = tmp_path / "repo" + (root / "pipelines").mkdir(parents=True) + (root / "pipelines" / "orders.py").write_text(_SOURCE, encoding="utf-8") + # Pinned, not inherited: the branch name is asserted below, and a host with + # init.defaultBranch=main or commit.gpgsign=true would otherwise change or + # block what this fixture produces. + subprocess.run(["git", "init", "-q", "-b", "master", str(root)], check=True) + for args in ( + ("config", "user.email", "t@example.com"), + ("config", "user.name", "T"), + ("config", "commit.gpgsign", "false"), + ("config", "tag.gpgsign", "false"), + ("remote", "add", "origin", "https://example.invalid/acme/orders.git"), + ("add", "-A"), + ("commit", "-qm", "initial"), + ): + subprocess.run(["git", "-C", str(root), *args], check=True, capture_output=True) + return root + + +def _annotations(path: Path) -> dict[str, str]: + document = yaml.safe_load(path.read_text(encoding="utf-8")) + return dict(document.get("metadata", {}).get("annotations", {}) or {}) + + +def _generate(repo: Path, output: Path, *, logical: Path | None = None) -> Path: + output.parent.mkdir(parents=True, exist_ok=True) + assert regenerate_yaml( + python_file=repo / "pipelines" / "orders.py", + output_path=output, + function_name="load_orders", + logical_output_path=logical, + ) + return output + + +def test_an_output_outside_the_checkout_keeps_the_repository_of_origin( + repo: Path, tmp_path: Path +) -> None: + """The pre-existing bug: provenance was read from the common ancestor. + + Reachable today without any staging directory, by configuring an + ``output_folder`` outside the checkout. + """ + annotations = _annotations(_generate(repo, tmp_path / "outside" / "component.yaml")) + + # Before the fix this dict had NO git_* keys at all: git was read from the + # common ancestor of source and output, which is outside the checkout. + assert "acme/orders" in annotations.get("git_remote_url", ""), annotations + assert annotations.get("git_local_sha"), "component has no repository of origin" + assert annotations.get("git_local_branch") == "master" + assert annotations.get("git_relative_dir") == "pipelines" + + +def test_the_logical_path_and_not_the_written_path_is_recorded( + repo: Path, tmp_path: Path +) -> None: + """A caller writing somewhere incidental can still tell the truth.""" + logical = repo / "pipelines" / "generated" / "orders.yaml" + annotations = _annotations( + _generate(repo, tmp_path / "staging" / "component.yaml", logical=logical) + ) + + assert annotations["component_yaml_path"] == "generated/orders.yaml" + assert "staging" not in annotations["component_yaml_path"] + assert annotations["python_original_code_path"] == "orders.py" + assert "acme/orders" in annotations.get("git_remote_url", "") + + +def test_two_writes_to_different_places_produce_identical_bytes( + repo: Path, tmp_path: Path +) -> None: + """The digest must not depend on where the file happened to be written.""" + logical = repo / "pipelines" / "generated" / "orders.yaml" + first = _generate(repo, tmp_path / "one" / "component.yaml", logical=logical) + second = _generate(repo, tmp_path / "two" / "component.yaml", logical=logical) + + assert first.read_text(encoding="utf-8") == second.read_text(encoding="utf-8") + + +def test_no_host_or_staging_path_reaches_the_component(repo: Path, tmp_path: Path) -> None: + """Nothing about this machine belongs in a published component.""" + logical = repo / "pipelines" / "generated" / "orders.yaml" + annotations = _annotations( + _generate(repo, tmp_path / "staging" / "component.yaml", logical=logical) + ) + + rendered = " ".join( + [ + annotations.get("component_yaml_path", ""), + annotations.get("python_original_code_path", ""), + annotations.get("git_relative_dir", ""), + ] + ) + for leak in (str(tmp_path), str(Path.home()), "staging"): + assert leak not in rendered, f"provenance leaks {leak!r}: {rendered!r}" + + +def test_omitting_the_logical_path_preserves_existing_behaviour( + repo: Path, tmp_path: Path +) -> None: + """The seam is opt-in: every existing caller must be unaffected. + + Generating in place is the ordinary case, and it must still describe + itself exactly as before. + """ + in_place = repo / "pipelines" / "orders.yaml" + annotations = _annotations(_generate(repo, in_place)) + + assert annotations["component_yaml_path"] == "orders.yaml" + assert annotations["python_original_code_path"] == "orders.py" + assert "acme/orders" in annotations.get("git_remote_url", "") + assert annotations.get("git_relative_dir") == "pipelines" + + +def test_a_logical_path_equal_to_the_written_path_changes_nothing( + repo: Path, tmp_path: Path +) -> None: + """Passing the seam explicitly must be indistinguishable from omitting it.""" + implicit = _generate(repo, repo / "pipelines" / "a.yaml").read_text(encoding="utf-8") + target = repo / "pipelines" / "a.yaml" + explicit = _generate(repo, target, logical=target).read_text(encoding="utf-8") + + assert implicit == explicit + + +def test_an_override_written_against_the_old_signature_still_works( + repo: Path, tmp_path: Path +) -> None: + """A subclass predating the seam must keep working when it is unused. + + Both hops are overridable, so forwarding the new keyword unconditionally + would break existing overrides that never asked for it. + """ + from tangle_cli.component_generator import ComponentGenerator + + seen: dict[str, Any] = {} + + class _OldGenerator(ComponentGenerator): + # Exactly the pre-change signature: no logical_output_path. + def run_generation( + self, + *, + python_file: Path, + final_output: Path, + image: str, + func_name: str | None, + deps_file: Path | None, + custom_name: str | None, + strip_code: bool, + strip_source_path: bool, + mode: str = "inline", + resolve_root: Path | None = None, + emit_generation_annotations: bool = True, + unwrapped_inputs: dict[str, Any] | None = None, + ) -> bool: + seen["called"] = True + return True + + assert _OldGenerator().regenerate_yaml( + python_file=repo / "pipelines" / "orders.py", + output_path=repo / "pipelines" / "orders.yaml", + function_name="load_orders", + ) + assert seen["called"] + + +def test_a_symlinked_output_keeps_its_lexical_name_in_legacy_mode(tmp_path: Path) -> None: + """``td_legacy`` records the basename it was given, not the link target.""" + from tangle_cli.component_from_func import generate_component_yaml + + outside = tmp_path / "nogit" + outside.mkdir() + (outside / "orders.py").write_text(_SOURCE, encoding="utf-8") + target = outside / "physical-target.yaml" + alias = outside / "logical-alias.yaml" + target.write_text("", encoding="utf-8") + alias.symlink_to(target) + + assert generate_component_yaml( + file_path=outside / "orders.py", + output_path=alias, + container_image="python:3.12", + function_name="load_orders", + path_annotation_mode="td_legacy", + ) + assert _annotations(target)["component_yaml_path"] == "logical-alias.yaml" + + +def test_a_symlinked_source_outside_the_checkout_keeps_git_provenance( + repo: Path, tmp_path: Path +) -> None: + """Git is discovered from the source's REAL directory, not the link's.""" + link = tmp_path / "orders-link.py" + link.symlink_to(repo / "pipelines" / "orders.py") + + output = tmp_path / "staging" / "component.yaml" + output.parent.mkdir(parents=True, exist_ok=True) + assert regenerate_yaml( + python_file=link, + output_path=output, + function_name="load_orders", + logical_output_path=repo / "pipelines" / "generated" / "orders.yaml", + ) + annotations = _annotations(output) + assert "acme/orders" in annotations.get("git_remote_url", ""), annotations + assert annotations.get("git_local_sha") diff --git a/tests/test_packaging.py b/tests/test_packaging.py index 68d5980..6993b5a 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -183,7 +183,7 @@ def test_tangle_cli_wheel_supports_expert_no_deps_import_path_without_tangle_api requires_dist = [line for line in metadata.splitlines() if line.startswith("Requires-Dist: ")] assert not any(name.startswith("tangle_api/") for name in names) assert "tangle_cli/openapi/openapi.json" not in names - assert "Version: 0.1.15" in metadata + assert "Version: 0.1.16" in metadata assert "Requires-Dist: tangle-api==0.1.1" in requires_dist assert not any("extra == 'native'" in line for line in requires_dist) assert "Provides-Extra: native" in metadata diff --git a/uv.lock b/uv.lock index ac7fbb8..65cc609 100644 --- a/uv.lock +++ b/uv.lock @@ -2083,7 +2083,7 @@ requires-dist = [{ name = "pydantic", specifier = ">=2.0" }] [[package]] name = "tangle-cli" -version = "0.1.15" +version = "0.1.16" source = { editable = "." } dependencies = [ { name = "cloud-pipelines" },