From 8a5fdf5dc4618904aab3ed5245ee9b058c0abbe6 Mon Sep 17 00:00:00 2001 From: Volv G Date: Thu, 17 Sep 2026 17:16:17 -0700 Subject: [PATCH] Describe generated components by logical path, not write location (v0.1.16) Component provenance annotations were derived from wherever the YAML happened to be written. Two defects followed. Git provenance could vanish entirely. Repository details were read from the common ancestor of the source file and the output path; when a caller wrote outside the checkout that ancestor is not in the repository, so every git_* annotation was silently dropped. This is reachable today by configuring an output folder outside the checkout. Bytes were unstable. Because the physical path reached the annotations, the same source generated to two locations produced different content, and so a different digest. Repository details are now read from the source file's own directory, which is inside the checkout whenever the source is. A generated component also keeps its identity when written somewhere incidental: the optional logical_output_path records where the component is to be understood to live, while the bytes are written where the caller asked. The parameter is opt-in and forwarded only when set, so existing callers -- including overrides written against the previous signatures -- are unaffected, and the annotation path is kept unresolved so symlinked outputs still record the name they were given. Assisted-By: devx/6257e672-aaa7-443c-a4e1-a0150a485d9d --- .../tangle-cli/src/tangle_cli/__init__.py | 2 +- .../src/tangle_cli/component_from_func.py | 43 +++- .../src/tangle_cli/component_generator.py | 33 +++ pyproject.toml | 2 +- tests/test_logical_output_provenance.py | 243 ++++++++++++++++++ tests/test_packaging.py | 2 +- uv.lock | 2 +- 7 files changed, 317 insertions(+), 10 deletions(-) create mode 100644 tests/test_logical_output_provenance.py 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" },