From 7ff8f55ff5cfacc803adb0f43dcda0650f1709d9 Mon Sep 17 00:00:00 2001 From: Silin Gupta Date: Tue, 7 Jul 2026 14:43:04 -0400 Subject: [PATCH 1/6] feat(tangle-cli): add pipeline compile command and PipelineCompiler Port the Python-pipeline compiler from the internal tangle-deploy package into the OSS CLI so the generic compile logic lives in OSS: - pipeline_compiler.py: PipelineCompiler(TangleCliHandler) executor plus the generic compile_pipeline() free function. The zone-root seam (ZONE_ROOT_MARKERS) is left empty here for downstream injection. - schema_validation.py + schemas/dehydrated_pipeline_schema.json: Draft 2020-12 structural validation, semantic checks, and the no-template-delimiter output contract. - pipelines.compile_pipeline_file facade and a cyclopts `compile` subcommand under `tangle sdk pipelines`, following the hydrate facade. - jsonschema>=4.0.0 runtime dependency (uv.lock updated). - Tests for the compiler, CLI command, and schema validation, with colocated python_pipeline fixtures. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/tangle_cli/pipeline_compiler.py | 2226 +++++++++++++++++ .../tangle-cli/src/tangle_cli/pipelines.py | 48 +- .../src/tangle_cli/pipelines_cli.py | 73 + .../src/tangle_cli/schema_validation.py | 460 ++++ .../schemas/dehydrated_pipeline_schema.json | 254 ++ tests/fixtures/python_pipeline/config.yaml | 1 + .../python_pipeline/dup_task_pipeline.py | 14 + .../python_pipeline/empty_pipeline.py | 14 + .../python_pipeline/multi_arg_pipeline.py | 37 + .../python_pipeline/multi_pipeline.py | 18 + tests/fixtures/python_pipeline/no_pipeline.py | 8 + tests/fixtures/python_pipeline/noop.yaml | 16 + tests/fixtures/python_pipeline/pipeline.py | 13 + .../fixtures/python_pipeline/task_pipeline.py | 24 + tests/test_pipeline_compiler.py | 692 +++++ tests/test_pipelines_cli.py | 2 +- tests/test_schema_validation.py | 233 ++ 17 files changed, 4131 insertions(+), 2 deletions(-) create mode 100644 packages/tangle-cli/src/tangle_cli/pipeline_compiler.py create mode 100644 packages/tangle-cli/src/tangle_cli/schema_validation.py create mode 100644 packages/tangle-cli/src/tangle_cli/schemas/dehydrated_pipeline_schema.json create mode 100644 tests/fixtures/python_pipeline/config.yaml create mode 100644 tests/fixtures/python_pipeline/dup_task_pipeline.py create mode 100644 tests/fixtures/python_pipeline/empty_pipeline.py create mode 100644 tests/fixtures/python_pipeline/multi_arg_pipeline.py create mode 100644 tests/fixtures/python_pipeline/multi_pipeline.py create mode 100644 tests/fixtures/python_pipeline/no_pipeline.py create mode 100644 tests/fixtures/python_pipeline/noop.yaml create mode 100644 tests/fixtures/python_pipeline/pipeline.py create mode 100644 tests/fixtures/python_pipeline/task_pipeline.py create mode 100644 tests/test_pipeline_compiler.py create mode 100644 tests/test_schema_validation.py diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py new file mode 100644 index 0000000..06e78c7 --- /dev/null +++ b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py @@ -0,0 +1,2226 @@ +"""Compile a Python-authored Tangle pipeline to a single YAML file. + +Surface:: + + tangle sdk pipelines compile -o [--pipeline name] [--override key=value ...] + +The compile driver: + +1. Loads ``pipeline.py`` as a Python module via ``importlib``. +2. Selects the root :class:`PipelineFn` via ``vars(mod).values()`` — the + single one defined in the file, or the one named by ``--pipeline`` when + the file defines several (e.g. a parent + same-file nested children). +3. Loads the cfg from ``/`` and overlays + ``--override key=value`` pairs (CLI wins). ``cfg`` is compile-time + only — its values are baked into emitted constants, never copied + into the output YAML. +4. Traces the pipeline and emits the body dict. +5. Writes the single dehydrated pipeline YAML. No wrapper config and no + ``.yaml.j2`` template sidecar are written. When the pipeline uses + ``@task``-decorated components, an auxiliary ``.components.yaml`` + resolver sidecar is also written and each ``@task`` task's + ``componentRef`` is rewritten to a pure + ``resolve://./.components.yaml#`` URL. + +Exit codes (surfaced by the CLI layer): +- 0 — success. +- 1 — :class:`tangle_cli.python_pipeline.errors.CompileError`. + +The generic compile logic lives here as free functions plus the +:class:`PipelineCompiler` command handler (a :class:`TangleCliHandler` +subclass, mirroring :class:`tangle_cli.pipeline_hydrator.PipelineHydrator`). +Distributions that carry a zone concept extend the :data:`ZONE_ROOT_MARKERS` +seam so the compiler can resolve an explicit relative +``@registered(gen_config="rel/path")`` against a zone root. +""" + +from __future__ import annotations + +import importlib.util +import inspect +import os +import re +import sys +import uuid +from collections.abc import Iterator, Mapping +from contextlib import contextmanager +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, get_type_hints + +import yaml + +from .handler import TangleCliHandler +from .python_pipeline.cfg import Cfg, _coerce_override, load_cfg +from .python_pipeline.compiler_context import ( + BroadcastLayer, + CompileContext, + PipelineCompileKey, + SubgraphArtifact, + canonical_repo_path, + overrides_fingerprint, +) +from .python_pipeline.emit import _TASK_URL_PLACEHOLDER, emit_pipeline +from .python_pipeline.errors import CompileError +from .python_pipeline.pipeline import PipelineFn +from .python_pipeline.ref import CallableRef +from .python_pipeline.registered import _REGISTERED_URL_PLACEHOLDER +from .python_pipeline.subpipeline import _SUBPIPELINE_URL_PLACEHOLDER, SubpipelineRef +from .python_pipeline.trace import trace_pipeline +from .python_pipeline.types import In +from .schema_validation import SchemaValidationError, validate_dehydrated_pipeline +from .utils import dump_yaml + + +@dataclass +class CompileResult: + """Outcome of a :func:`compile_pipeline` call. + + Attributes: + pipeline_path: Resolved path of the single emitted pipeline YAML. + components_path: Path of the ``@task`` ``local_from_python`` + resolver sidecar, or ``None`` when no sidecar was emitted. + task_count: Number of tasks traced into the ROOT graph. + warnings: Non-fatal messages collected during compilation. + subgraph_paths: Paths of the child graph sidecars written under + ``.subgraphs/`` for nested ``subpipeline(...)`` calls. + Empty for single-pipeline / ``@task``-only compiles. Additive + field — existing callers that ignore it are unaffected. + """ + + pipeline_path: Path + components_path: Path | None = None + task_count: int = 0 + warnings: list[str] = field(default_factory=list) + subgraph_paths: list[Path] = field(default_factory=list) + + +def compile_pipeline( + script: Path, + output: Path, + overrides: Mapping[str, str] | None = None, + *, + pipeline_name: str | None = None, + emit_components_sidecar: bool = True, +) -> CompileResult: + """Compile ``script`` to a single pipeline YAML at ``output``. + + Args: + script: Path to the Python authoring file. The file holds the + single root ``@pipeline``-decorated function, or several + (a parent + same-file nested children, or independent + siblings) one of which is selected by ``pipeline_name``. + output: Path for the compiled pipeline YAML. The dehydrated + pipeline body is always written here. When the pipeline uses + ``@task`` components an additional ``.components.yaml`` + resolver sidecar is written alongside it. No wrapper config + and no ``.yaml.j2`` sidecar are ever written. + overrides: Already-parsed ``key=value`` config overrides. CLI + wins over file-defined values. + pipeline_name: When the file defines several pipelines, selects + which root to compile — matched against the decorated + function ``__name__`` first, then the ``@pipeline`` display + name (``--pipeline``). Optional (and ignored unless it must + disambiguate) when the file defines exactly one pipeline. + Same-file nested children are reached via the selected root's + ``subpipeline(...)`` calls, not by this name. + emit_components_sidecar: When ``True`` (the default), ``@task`` + components are preserved by emitting a sibling + ``.components.yaml`` ``local_from_python`` resolver and + rewriting their ``componentRef`` to a pure ``resolve://`` URL. + When ``False`` and the pipeline uses ``@task`` components, + compilation fails with a :class:`CompileError` (a valid + dehydrated pipeline cannot be produced without the sidecar). + Pipelines that use only ``ref(url=...)`` are unaffected. + + Returns: + A :class:`CompileResult`. ``components_path`` is the sidecar path + when one was written, else ``None``. + + Raises: + CompileError: for user-facing problems (missing script, no/ + multiple ``@pipeline`` functions, missing/invalid config, + unreachable ``@task`` source/dependency files). + """ + overrides = dict(overrides or {}) + + # 1. Validate the script path. + script_path = Path(script).resolve() + if not script_path.exists(): + raise CompileError(f"pipeline file not found: {script}") + + # The root module is exec'd under a unique synthetic name (restored by + # ``_load_pipeline_fn``), but its top-level ``from sibling import ...`` + # statements (and trace-time sibling imports inside cycle-style children) + # register sibling modules under their REAL names; left behind they would + # let a SUBSEQUENT in-process compile of a DIFFERENT bundle reuse the + # STALE cached sibling. We purge bundle-local modules AFTER the full + # compile, so the already-captured child ``PipelineFn`` objects + # (referenced by the traced functions) stay valid throughout (P2 fix). + # Collect every source directory the compile imports from (root + each + # child's own source dir, which may differ from the root's) so the purge + # also reaches children authored in sibling directories. + purge_dirs: set[Path] = {script_path.parent.resolve()} + + # Python caches imported modules by NAME (not path), so a PRE-EXISTING + # ``sys.modules`` entry for a bundle-local sibling (left by a prior + # in-process compile, a test helper, or a REPL) would shadow this bundle's + # OWN sibling file. Evict any such shadowing entry up front so the root's + # ``from sibling import ...`` resolves FRESH from this bundle (P2 fix, the + # pre-existing-pollution half; the after-compile purge below handles the + # entries this compile itself adds). + _evict_shadowed_bundle_modules(script_path.parent) + + try: + # 2. Load the user pipeline module + select the ROOT PipelineFn. When + # the file defines several pipelines, ``pipeline_name`` + # (``--pipeline``) chooses which to emit (Decision G). Imported + # child pipelines are reachable to ``subpipeline(child)`` but are + # never selectable compile targets; same-file nested children are + # compiled via the ``subpipeline`` recursion below. + pipeline_fn = _load_pipeline_fn(script_path, pipeline_name) + + # 3. Resolve the root output path up front. Child subgraph sidecars are + # written under ``.subgraphs/`` next to it, and the relative + # ``file://`` / ``resolve://`` URLs that point at the bundle are + # derived from these paths. + output_path = output.resolve() if output.is_absolute() else (Path.cwd() / output).resolve() + + # 4. Build the recursive compile context. ALL artifacts (root + every + # nested child sidecar) are traced, emitted, and rewritten IN MEMORY + # first; nothing is written until the whole bundle validates + # (Decision C). ``planned_files`` lets asset-policy validation accept + # refs to sidecars the compiler is about to write (Decision J). + ctx = CompileContext( + root_output_path=output_path, + subgraph_dir=output_path.with_name(output_path.stem + ".subgraphs"), + root_overrides=overrides, + emit_components_sidecar=emit_components_sidecar, + source_dirs=purge_dirs, + ) + + # 5. Compile the root (and, recursively, all children) into in-memory + # artifacts. The root cfg is resolved relative to the script's own + # directory; each child's cfg is resolved relative to ITS OWN source + # directory inside ``_compile_pipeline_fn`` (Decision F). + root_artifact = _compile_pipeline_fn( + pipeline_fn, + output_path, + ctx, + overrides, + is_root=True, + base_dir=script_path.parent, + ) + + # 6. The full bundle: the root plus every deduped child in the registry. + # Keying children by compile key means a child reached twice (or via + # a diamond) appears exactly once (Decision M). + artifacts: list[SubgraphArtifact] = [root_artifact, *ctx.registry.values()] + + # 7. Validate EVERY artifact before writing ANY file. Per artifact: + # validate_dehydrated_pipeline (top-level guard + schema + + # no-template scan + semantic checks) -> dump -> reload -> validate + # again -> asset policy. On any failure nothing is written + # (Decision C / J). + for artifact in artifacts: + _validate_artifact(artifact, ctx) + + # 8. Write the whole bundle only after every artifact validated. + for artifact in artifacts: + _write_artifact(artifact) + + return CompileResult( + pipeline_path=root_artifact.output_path, + components_path=root_artifact.components_path, + task_count=root_artifact.task_count, + warnings=ctx.warnings, + subgraph_paths=[a.output_path for a in artifacts if not a.is_root], + ) + finally: + # Purge bundle-local modules so a subsequent in-process compile + # re-imports them fresh (P2). Runs on success AND on error. + _purge_bundle_local_modules(purge_dirs) + + +# --------------------------------------------------------------------------- +# Recursive in-memory compile (Decision C) + + +@contextmanager +def _temp_sys_path(directory: Path) -> Iterator[None]: + """Temporarily put ``directory`` on ``sys.path`` (leak-free). + + The root module is exec'd by :func:`_load_pipeline_fn` with its source + dir on ``sys.path``, but that entry is removed before tracing. Some + fixtures defer sibling imports to TRACE time (e.g. the cycle fixtures + ``from cycle_b import ...`` inside the body), so the source dir must be + importable while the body runs. The entry is restored on exit so + repeated/concurrent compiles never leak global import state. + """ + d = str(directory) + added = d not in sys.path + if added: + sys.path.insert(0, d) + try: + yield + finally: + if added: + try: + sys.path.remove(d) + except ValueError: # pragma: no cover — defensive + pass + + +def _compile_pipeline_fn( + pipeline_fn: PipelineFn, + output_path: Path, + ctx: CompileContext, + overrides: Mapping[str, Any], + *, + is_root: bool, + base_dir: Path, + rebroadcast_overrides: Mapping[str, Any] | None = None, +) -> SubgraphArtifact: + """Trace + emit ``pipeline_fn`` into an in-memory :class:`SubgraphArtifact`. + + Does NOT write any file. The given ``pipeline_fn`` is ALREADY imported + (the root via :func:`_load_pipeline_fn`; a child via its parent module's + ``import``), so it is traced directly — the module is never reloaded, + which sidesteps ``sys.modules`` cache issues with sibling children. + + Steps: + + 1. Resolve + load cfg relative to ``base_dir`` (the pipeline's OWN + source dir — Decision F config isolation). + 2. Trace in a fresh ``GraphBuilder`` and emit the dehydrated body. + 3. Build (do NOT write) this artifact's ``@task`` ``local_from_python`` + components sidecar and rewrite its ``@task`` refs to pure + ``resolve://`` URLs. + 4. (Phase 4) compile any ``subpipeline(child)`` children recursively and + rewrite the parent task refs to pure ``file://`` URLs. + + Returns the planned artifact; validation + writing happen later in + :func:`compile_pipeline` once the whole bundle is built. + """ + # Record this pipeline's source dir so the driver can purge any modules + # it imports from there after the compile (P2 sibling-import leak fix). + try: + ctx.source_dirs.add(base_dir.resolve()) + except OSError: # pragma: no cover — defensive + pass + + # Evict any stale modules that would shadow THIS pipeline's bundle-local + # siblings before tracing, so trace-time sibling imports inside the body + # (e.g. cycle-style ``from sibling import ...``) resolve fresh from the + # bundle rather than a cached entry pointing elsewhere (P2 fix). + _evict_shadowed_bundle_modules(base_dir) + + # 1. cfg is resolved + loaded relative to the pipeline's own source dir. + # The ROOT coerces its CLI ``--override`` strings (yaml.safe_load); a + # CHILD receives already-typed native ``effective`` overrides (broadcast + # + explicit ``.override_config``) that must pass through unchanged. + cfg_path = _resolve_cfg_path_in_dir(pipeline_fn, base_dir) + uses_cfg = _pipeline_accepts_cfg(pipeline_fn) + should_load_config = uses_cfg or pipeline_fn.propagate_config + if should_load_config: + _assert_config_output_path_is_separate( + cfg_path, + output_path, + pipeline_fn=pipeline_fn, + is_root=is_root, + ) + loaded_cfg, raw_cfg = _load_cfg_and_raw( + cfg_path, + overrides, + coerce=is_root, + pipeline_fn=pipeline_fn, + usage="cfg" if uses_cfg else "propagate_config", + ) + cfg = loaded_cfg if uses_cfg else Cfg({}) + if not uses_cfg and pipeline_fn.config_path: + ctx.warnings.append( + f"pipeline {pipeline_fn.name!r} declares config={pipeline_fn.config_path!r} " + "but its function has no `cfg` parameter. The config file is still " + "loaded because propagate_config=True broadcasts it to descendant " + "subpipelines. Add a `cfg` parameter if this pipeline also needs to " + "read the config directly." + ) + else: + if is_root and overrides: + keys = sorted(overrides) + raise CompileError( + f"pipeline {pipeline_fn.name!r} received --override values {keys!r}, " + "but its function has no `cfg` parameter and propagate_config is " + "not enabled. Add a `cfg` parameter to read compile-time config, " + "enable propagate_config=True to broadcast overrides to descendants, " + "or remove the unused override(s)." + ) + if pipeline_fn.config_path: + ctx.warnings.append( + f"pipeline {pipeline_fn.name!r} declares config={pipeline_fn.config_path!r} " + "but its function has no `cfg` parameter, so the config file was " + "not loaded. Remove `config=` when no compile-time config is needed, " + "or add a `cfg` parameter to use it." + ) + cfg = Cfg({}) + raw_cfg = {} + + # 2. Trace + emit. The source dir is on sys.path so any trace-time + # sibling imports inside the body resolve. ``emit_pipeline`` also + # returns the set of argument JSON paths wrapped in ``raw(...)`` — + # legitimate RUNTIME template placeholders the no-template-delimiter + # output guard must skip for THIS artifact's body. + with _temp_sys_path(base_dir): + builder = trace_pipeline(pipeline_fn, cfg=cfg, inputs={}) + body_dict, exempt_paths = emit_pipeline(builder) + + # 3. Compute the canonical compile key for dedup / cycle detection. + key = _compile_key_for(pipeline_fn, cfg_path, overrides) + + # 4. @task local_from_python sidecar for THIS artifact (built, not + # written). Each @task ref is rewritten to a pure + # ``resolve://./.components.yaml#`` URL BEFORE schema + # validation, so the transient ``local-from-python://pending`` + # placeholder never reaches output. + components_entries: dict[str, Any] = {} + components_path: Path | None = None + task_refs = builder.task_refs_for_local_from_python + if task_refs and not ctx.emit_components_sidecar: + task_ids = sorted(tid for tid, _ref in task_refs) + raise CompileError( + "pipeline uses @task component(s) that require a " + "local_from_python resolver sidecar, but " + "emit_components_sidecar=False. Set it to True (the default) to " + f"compile @task pipelines. Offending task(s): {task_ids!r}." + ) + if task_refs: + components_path = output_path.with_name(output_path.stem + ".components.yaml") + components_entries = _build_local_from_python_components(task_refs, components_yaml_dir=components_path.parent) + _rewrite_task_componentref_urls( + body_dict=body_dict, + task_refs=task_refs, + components_yaml_name=components_path.name, + ) + + # 4b. A CHILD artifact is written under ``.subgraphs/``, away from + # its own source directory. Its author-written relative local refs + # (plain ``ref(url="file://./leaf.yaml")``) point at files next to + # the child SOURCE, so relocate them to be relative to the child + # SIDECAR directory — the URL still resolves to the SAME original + # file (no copying), just from the sidecar's location. Compiler- + # managed refs (``@task`` resolver + subpipeline) already point at + # bundle files and are skipped. The root is never relocated: its + # output dir is its bundle root (in-place compile contract). + if not is_root: + _relocate_child_local_refs( + body_dict=body_dict, + builder=builder, + source_dir=base_dir, + sidecar_dir=output_path.parent, + ) + + # 4c. @registered refs point at an EXISTING gen_config.yaml (the + # operation is registered/published elsewhere), so there is no + # sidecar to generate. Rewrite each registered task's + # ``registered://pending`` sentinel to a pure + # ``resolve:///gen_config.yaml#`` URL, computed + # against THIS artifact's output dir so nested subpipeline children + # (written under ``.subgraphs/``) get a correct relative path. + # Runs AFTER 4b relocation so it never touches the relocation pass. + registered_refs = builder.task_refs_for_registered + if registered_refs: + _rewrite_registered_componentref_urls( + body_dict=body_dict, + registered_refs=registered_refs, + artifact_output_dir=output_path.parent, + ) + + artifact = SubgraphArtifact( + key=key, + output_path=output_path, + body=body_dict, + is_root=is_root, + task_count=len(builder.tasks), + components_entries=components_entries or None, + components_path=components_path if components_entries else None, + exempt_paths=exempt_paths, + ) + + # 5. Register the files this artifact will write so asset-policy + # validation accepts refs to them before they exist on disk. + ctx.planned_files.add(output_path.resolve()) + if artifact.components_path is not None: + ctx.planned_files.add(artifact.components_path.resolve()) + + # 6. Compile every nested subpipeline child into its own graph sidecar + # and rewrite this artifact's subpipeline tasks to pure ``file://`` + # refs. This artifact's key is on the active stack while children are + # compiled so a child that reaches back here is detected as a cycle + # (Decision L). Subpipeline children appear only when NM1 recorded + # them; root-only / @task-only compiles skip this entirely. + # + # When ``propagate_config`` is set, push a broadcast layer carrying + # THIS pipeline's OWN config (Decision §4: "broadcast my OWN config"). + # The layer is ``raw_cfg`` overlaid with ``rebroadcast``. ``rebroadcast`` + # is this pipeline's OWN re-broadcastable overrides: + # * ROOT (``rebroadcast_overrides is None``) — its CLI ``--override`` + # values, COERCED with the same ``yaml.safe_load`` coercion + # ``load_cfg(coerce=True)`` applied to the root's own cfg, so the + # broadcast carries the typed value the root itself sees (Finding 2), + # not the raw CLI string. + # * flagged CHILD (``rebroadcast_overrides == {}``) — nothing extra: + # a flagged child broadcasts ONLY its own ``raw_cfg``. The explicit + # ``.override_config`` values set on the edge INTO this child have + # their DEPTH governed by the CALLER's flag (Decision §4 line 122), + # so the caller — not this child — pushes a per-edge layer for them + # (see ``_process_subpipeline_children``). This keeps "nearest + # flagged ancestor that DEFINES the key wins": an outer ancestor's + # value still reaches descendants via the ancestor's OWN layer, which + # remains on the stack. + # Nearest-wins falls out of stack order: an inner layer is iterated last + # in ``_effective_overrides_for_child`` and overwrites outer layers. BOTH + # stacks must pop in ``finally`` so a child that raises CompileError + # leaves them symmetric. + if rebroadcast_overrides is None: + # ROOT: coerce CLI override strings to the YAML type the root's own cfg + # already carries, so the broadcast layer is type-consistent with cfg. + rebroadcast: Mapping[str, Any] = {k: _coerce_override(v) for k, v in overrides.items()} + else: + rebroadcast = rebroadcast_overrides + ctx.active_stack.append(key) + pushed_broadcast = False + if pipeline_fn.propagate_config: + ctx.broadcast_stack.append(BroadcastLayer(config={**raw_cfg, **dict(rebroadcast)})) + pushed_broadcast = True + try: + _process_subpipeline_children(artifact, builder, ctx, parent_propagate_config=pipeline_fn.propagate_config) + finally: + ctx.active_stack.pop() + if pushed_broadcast: + ctx.broadcast_stack.pop() + + return artifact + + +def _process_subpipeline_children( + parent_artifact: SubgraphArtifact, + builder: Any, + ctx: CompileContext, + *, + parent_propagate_config: bool, +) -> None: + """Compile each ``subpipeline(child)(...)`` recorded during ``builder``'s + trace and rewrite the parent task's ``componentRef`` to a pure + ``file://`` URL pointing at the child's graph sidecar. + + For every ``(task_id, SubpipelineRef)`` recorded in + ``builder.task_refs_for_subpipelines``: + + * compute the child :class:`PipelineCompileKey`; + * CYCLE check — if the key is already on ``ctx.active_stack`` raise a + :class:`CompileError` naming the full chain (Decision L); this also + covers self-reference (``A -> A``); + * MAX-DEPTH guard — refuse chains deeper than ``ctx.max_depth``; + * DEDUP — if the key is already compiled (``ctx.registry``) reuse that + child's sidecar; otherwise recurse via :func:`_compile_pipeline_fn` + into ``.subgraphs/-.yaml`` (ALL children + and grandchildren live flat under the ROOT's ``.subgraphs/`` dir); + * rewrite the parent task's ``componentRef`` to a pure ``file://`` URL + relative to the REFERENCING artifact's directory (root→child: + ``./compiled.subgraphs/.yaml``; child→grandchild same dir: + ``./.yaml``), replacing the ``subpipeline://pending`` sentinel. + + Writes nothing — it only mutates ``parent_artifact.body`` and populates + ``ctx.registry`` / ``ctx.planned_files``. + """ + sub_refs: list[tuple[str, SubpipelineRef]] = builder.task_refs_for_subpipelines + if not sub_refs: + return + + parent_tasks = parent_artifact.body.get("implementation", {}).get("graph", {}).get("tasks", {}) + # task_id -> declared child output names, used by the cross-file + # ``taskOutput`` safety net once every child has been compiled. + subpipeline_output_names: dict[str, set[str]] = {} + for task_id, sub_ref in sub_refs: + child_fn = sub_ref.child + child_base_dir = _pipeline_base_dir(child_fn, fallback=ctx.subgraph_dir.parent) + child_cfg_path = _resolve_cfg_path_in_dir(child_fn, child_base_dir) + + # Resolve this child's EFFECTIVE overrides: broadcast from flagged + # ancestors (lenient, nearest-wins) + the explicit ``.override_config`` + # on this edge (strict, wins). When NEITHER a broadcast layer is + # active NOR an explicit override was set, skip the raw-cfg read + # entirely and use ``{}``. The fast path does NOT let a config-taking + # child without a config.yaml compile — that child still raises + # "config file not found" in its own ``_compile_pipeline_fn``. What it + # buys is: one fewer raw-cfg read, and exact preservation of Decision-F + # default isolation (empty effective overrides -> empty + # ``overrides_fingerprint`` -> the SAME dedup key the child gets with no + # feature in play) whenever there is nothing to broadcast or override. + # Ambient PASS-THROUGH context (Fix B / §7): the resolved + # broadcast/override keys this child does NOT declare and therefore + # flow PAST it, unchanged, to its descendants. It is folded into the + # child's compile key so the SAME child reached under two different + # ancestor-broadcast contexts gets distinct keys (and distinct + # sidecars) whenever the difference can affect its descendants — even + # when the child's OWN effective overrides are identical. + ambient_passthrough: dict[str, Any] = {} + if ctx.broadcast_stack or sub_ref.config_overrides: + child_raw = _read_raw_cfg(child_cfg_path) + effective = _effective_overrides_for_child( + child_raw=child_raw, + broadcast_stack=ctx.broadcast_stack, + explicit=sub_ref.config_overrides, + child_name=child_fn.name, + parent_task_id=task_id, + ) + # Resolve the ambient context from the stack AS IT STANDS NOW (the + # per-edge override INTO this child is not pushed yet — correct, + # it is already in this child's ``effective``). The pass-through is + # exactly the resolved keys the child does not declare. + if ctx.broadcast_stack: + resolved_ambient = _resolve_ambient_context(ctx.broadcast_stack) + ambient_passthrough = {k: v for k, v in resolved_ambient.items() if k not in child_raw} + else: + effective = {} + + child_key = _compile_key_for( + child_fn, + child_cfg_path, + effective, + fingerprint_context=f"on subpipeline task {task_id!r} -> child pipeline {child_fn.name!r}", + ambient_context=ambient_passthrough or None, + ) + + # Cycle detection (covers self-reference). active_stack holds the + # chain from the root down to (and including) the artifact whose + # children we are compiling. + if child_key in ctx.active_stack: + chain = [*ctx.active_stack, child_key] + chain_str = " -> ".join(k.display() for k in chain) + raise CompileError( + f"nested pipeline cycle detected: {chain_str}. Recursive " + "Python pipelines are not supported; break the cycle or use a " + "published component boundary." + ) + + child_artifact = ctx.registry.get(child_key) + if child_artifact is None: + # Max-depth guard: the chain TO the child would be one deeper + # than the current stack. + if len(ctx.active_stack) + 1 > ctx.max_depth: + chain = [*ctx.active_stack, child_key] + chain_str = " -> ".join(k.display() for k in chain) + raise CompileError( + f"nested pipeline max depth {ctx.max_depth} exceeded: " + f"{chain_str}. Reduce nesting depth or restructure the " + "pipeline graph." + ) + slug = _slugify(child_fn.name) + child_output_path = ctx.subgraph_dir / f"{slug}-{child_key.hash8()}.yaml" + # Per-edge explicit-override depth (Decision §4 line 122-124): an + # explicit ``.override_config`` set on THIS edge flows deep iff the + # CALLER (this parent) is flagged. When it is, push a broadcast + # layer carrying JUST this edge's explicit overrides as the INNERMOST + # (nearest) layer while the child's subtree compiles, then pop it. + # This makes the explicit override (a) win over the parent's own + # broadcast for the child's DESCENDANTS (explicit always wins, and + # nearest-wins puts it last), and (b) reach descendants that declare + # the key (lenient). The child ITSELF already got the explicit + # override strictly via ``effective``; this per-edge layer is pushed + # AROUND the recursion only, so it affects the child's DESCENDANTS, + # not the child's own ``effective`` / ``child_key`` (computed above). + # The flagged-child re-broadcast passes ``{}`` — a flagged child + # broadcasts only its OWN config; this edge's explicit override + # depth belongs to the caller and is handled by THIS layer. + pushed_edge_override = False + if parent_propagate_config and sub_ref.config_overrides: + # ``explicit=True`` puts this layer in the explicit tier, which + # is resolved AFTER the whole broadcast tier in + # ``_effective_overrides_for_child`` — so this edge's override + # outranks even a NEARER flagged descendant's own-config + # broadcast of the same key (PROPAGATE_CONFIG_DESIGN §4). + ctx.broadcast_stack.append(BroadcastLayer(config=dict(sub_ref.config_overrides), explicit=True)) + pushed_edge_override = True + try: + child_artifact = _compile_pipeline_fn( + child_fn, + child_output_path, + ctx, + effective, + is_root=False, + base_dir=child_base_dir, + rebroadcast_overrides={}, + ) + finally: + if pushed_edge_override: + ctx.broadcast_stack.pop() + ctx.registry[child_key] = child_artifact + parent_artifact.children.append(child_artifact) + else: + # Reused (dedup / diamond) — still a child of this parent for + # structural completeness, but compiled only once. + if child_artifact not in parent_artifact.children: + parent_artifact.children.append(child_artifact) + + # Rewrite this task's componentRef to a pure file:// URL relative to + # the REFERENCING artifact's directory. + rel = _relpath_posix(child_artifact.output_path, parent_artifact.output_path.parent) + if task_id not in parent_tasks: + raise CompileError( + f"subpipeline ref recorded for task {task_id!r} but no such " + "task in the emitted body (internal error)." + ) + parent_tasks[task_id]["componentRef"] = {"url": f"file://{rel}"} + + # INPUT interface validation (Decision E). The compiled child body + # is the source of truth for declared inputs; ``wait_for`` / + # ``depends_on`` are NOT special — they must be declared child + # In[...] inputs if passed. + _validate_subpipeline_inputs( + parent_task_id=task_id, + parent_args=parent_tasks[task_id].get("arguments", {}), + child_artifact=child_artifact, + ) + subpipeline_output_names[task_id] = _child_output_names(child_artifact.body) + + # OUTPUT cross-file validation (Decision E / K) — a safety net beyond the + # strict SubpipelineOutputProxy: every ``taskOutput`` in the parent body + # that targets a subpipeline task must name an output the child declares. + _validate_subpipeline_output_refs(parent_artifact.body, subpipeline_output_names) + + +def _child_input_specs(child_body: dict[str, Any]) -> tuple[list[str], set[str]]: + """Return ``(declared_input_names, required_input_names)`` from a child's + compiled body ``inputs`` block. + + An input is REQUIRED unless it is marked ``optional`` (the tracer sets + ``optional: true`` for any ``In[T]`` parameter that has a default). + Declared names preserve declaration order for stable error messages. + """ + inputs = child_body.get("inputs", []) or [] + names: list[str] = [] + required: set[str] = set() + for spec in inputs: + if not isinstance(spec, dict): + continue + name = spec.get("name") + if name is None: + continue + names.append(name) + if not spec.get("optional"): + required.add(name) + return names, required + + +def _child_output_names(child_body: dict[str, Any]) -> set[str]: + """Return the set of output names declared by a child's compiled body.""" + outs = child_body.get("outputs", []) or [] + return {name for o in outs if isinstance(o, dict) and isinstance(name := o.get("name"), str)} + + +def _validate_subpipeline_inputs( + *, + parent_task_id: str, + parent_args: dict[str, Any], + child_artifact: SubgraphArtifact, +) -> None: + """Validate a subpipeline call's arguments against the child interface. + + Rejects an UNKNOWN argument name (not a declared child input) and a + MISSING REQUIRED child input (declared, non-optional, not supplied). + Omitted optional/default child inputs are allowed. Raises a clear + :class:`CompileError` BEFORE any file is written (Decision E). + """ + declared, required = _child_input_specs(child_artifact.body) + child_name = child_artifact.key.pipeline_name + arg_names = set(parent_args.keys()) + + unknown = sorted(arg_names - set(declared)) + if unknown: + raise CompileError( + f"subpipeline task {parent_task_id!r} passes unknown input " + f"{unknown[0]!r} to child pipeline {child_name!r}. Declared child " + f"inputs: {declared}." + ) + + missing = sorted(required - arg_names) + if missing: + raise CompileError( + f"subpipeline task {parent_task_id!r} calls child pipeline " + f"{child_name!r} without required input {missing[0]!r}. Pass " + f"{missing[0]}=... or give the child In[...] parameter a default." + ) + + +def _validate_subpipeline_output_refs( + body: dict[str, Any], + subpipeline_output_names: dict[str, set[str]], +) -> None: + """Assert every ``taskOutput`` targeting a subpipeline task names a + declared child output. + + Scans both task ``arguments`` and graph ``outputValues``. This is the + compiler-owned cross-file safety net that protects the serialized YAML + even if a proxy bug let an undeclared output slip through (Decision K). + """ + if not subpipeline_output_names: + return + graph = body.get("implementation", {}).get("graph", {}) + tasks = graph.get("tasks", {}) if isinstance(graph, dict) else {} + + def _check(value: Any, loc: str) -> None: + if not isinstance(value, dict): + return + task_output = value.get("taskOutput") + if not isinstance(task_output, dict): + return + target = task_output.get("taskId") + if target not in subpipeline_output_names: + return + out_name = task_output.get("outputName") + declared = subpipeline_output_names[target] + if out_name not in declared: + raise CompileError( + f"{loc} references output {out_name!r} of subpipeline task " + f"{target!r}, but that child pipeline declares only " + f"{sorted(declared)}." + ) + + if isinstance(tasks, dict): + for task_id, task in tasks.items(): + if not isinstance(task, dict): + continue + arguments = task.get("arguments") + if isinstance(arguments, dict): + for arg_name, value in arguments.items(): + _check(value, f"task {task_id!r} argument {arg_name!r}") + output_values = graph.get("outputValues") if isinstance(graph, dict) else None + if isinstance(output_values, dict): + for out_key, value in output_values.items(): + _check(value, f"outputValues key {out_key!r}") + + +def _pipeline_base_dir(pipeline_fn: PipelineFn, *, fallback: Path) -> Path: + """Resolve the source directory a child pipeline's cfg + sibling imports + are relative to. + + Prefers the ``@pipeline`` decorator's captured ``caller_dir`` (the + child's own source file directory — Decision F), then the decorated + function's source file, then ``fallback`` (the root bundle directory) + for dynamically built pipelines with no on-disk source. + """ + if pipeline_fn.caller_dir is not None: + return pipeline_fn.caller_dir + src = _pipeline_source_path(pipeline_fn) + if src is not None: + return src.parent + return fallback + + +def _slugify(name: str) -> str: + """Slugify a pipeline display name for a child sidecar filename. + + Lowercases, replaces any run of non-alphanumeric characters with a + single hyphen, and trims leading/trailing hyphens (``"Judge Options"`` + -> ``"judge-options"``). Collisions between two children that slug to + the same name are disambiguated by the ``hash8`` suffix. + """ + slug = re.sub(r"[^a-z0-9]+", "-", name.strip().lower()).strip("-") + return slug or "pipeline" + + +def _compile_key_for( + pipeline_fn: PipelineFn, + cfg_path: Path, + overrides: Mapping[str, Any], + *, + fingerprint_context: str | None = None, + ambient_context: Mapping[str, Any] | None = None, +) -> PipelineCompileKey: + """Build the canonical :class:`PipelineCompileKey` for ``pipeline_fn``. + + Paths are canonicalised per Decision B (repo-relative POSIX inside a git + repo, else resolved absolute POSIX). A dynamically built pipeline whose + source cannot be located falls back to a qualname-derived sentinel so it + still produces a stable, distinct key. + + ``fingerprint_context`` is a human label naming the affected pipeline / + child edge; it is woven into the :func:`overrides_fingerprint` error if an + override value is not a compile-time constant, so a non-serializable + ``.override_config`` value surfaces as an actionable :class:`CompileError` + rather than a bare ``TypeError``. + + ``ambient_context`` is the AMBIENT PASS-THROUGH context (Fix B / §7): the + active broadcast/override keys this node does NOT declare and therefore + flow PAST it to its descendants. Folding it into the fingerprint keeps the + same node distinct when it is reached under two different ancestor-broadcast + contexts that its descendants would resolve differently. + + Fingerprint encoding (collision-proof, byte-stable): + + * **Empty / ``None`` ambient** (always true for the default-isolation + compile) -> ``overrides_fingerprint(overrides)`` UNCHANGED, byte-for-byte + identical to before Fix B. + * **Non-empty ambient** -> a small envelope keyed by reserved sentinels + (``"\\x00effective"`` / ``"\\x00ambient"``) that cannot collide with real + config keys, routed through :func:`overrides_fingerprint` so its + non-serializable-value -> :class:`CompileError` detection (which recurses + into dicts) still fires for BOTH effective and ambient values. + """ + src = _pipeline_source_path(pipeline_fn) + if src is not None: + source_canon = canonical_repo_path(src) + else: + source_canon = f"" + if ambient_context: + # Reserved keys (NUL-prefixed) can never be real config keys, so an + # envelope never collides with an effective-only fingerprint over the + # same keys (e.g. effective={x:1},ambient={} vs effective={x:1},ambient={y:2}). + fingerprint_input: Mapping[str, Any] = { + "\x00effective": dict(sorted(overrides.items())), + "\x00ambient": dict(sorted(ambient_context.items())), + } + else: + # Byte-identical to today's key: the default-isolation guarantee (§7). + fingerprint_input = overrides + return PipelineCompileKey( + source_path=source_canon, + function_qualname=pipeline_fn.fn.__qualname__, + pipeline_name=pipeline_fn.name, + config_path=canonical_repo_path(cfg_path), + overrides_fingerprint=overrides_fingerprint(fingerprint_input, context=fingerprint_context), + ) + + +def _validate_artifact(artifact: SubgraphArtifact, ctx: CompileContext) -> None: + """Validate one planned artifact in memory (no writes). + + Runs a leftover-sentinel scan (a missed @task / subpipeline rewrite), + ``validate_dehydrated_pipeline`` on the body, a dump/reload + re-validation (to catch dumper issues), and the relative-local-ref asset + policy. The dumped text is cached on the artifact so the write pass + emits exactly the bytes that were validated. + """ + body = artifact.body + label = None if artifact.is_root else _artifact_label(artifact) + + # No pending compiler sentinel may survive into a written artifact. A + # surviving sentinel means a missed @task or subpipeline rewrite — fail + # with a targeted internal error BEFORE schema validation so the cause + # is obvious, and write nothing. + _assert_no_pending_sentinels(body, label) + + # ``raw(...)`` argument paths whose template delimiters are legitimate + # RUNTIME placeholders the no-template-delimiter output guard must skip + # for THIS artifact's body. Empty unless the artifact used ``raw(...)``. + exempt_paths = artifact.exempt_paths + + try: + validate_dehydrated_pipeline(body, exempt_paths) + except SchemaValidationError as e: + if label is None: + raise CompileError(str(e)) from e + raise CompileError(f"{label}: {e}") from e + + dumped = dump_yaml(body, sort_keys=False) + reloaded = yaml.safe_load(dumped) + try: + validate_dehydrated_pipeline(reloaded, exempt_paths) + except SchemaValidationError as e: + prefix = "" if label is None else f"{label}: " + raise CompileError(f"{prefix}compiled YAML failed re-validation after dump/reload: {e}") from e + + # Asset policy (Decision J, extended to children in Phase 8). EVERY + # artifact's relative local refs are validated relative to THAT + # artifact's own output directory: + # * the ROOT body relative to the root output dir (``label`` is None, + # preserving the verbatim single-pipeline error message); + # * each CHILD body relative to ITS child-sidecar dir (``label`` adds + # child-sidecar + task context to the error). + # Generated bundle files the compiler is about to write — child graph + # sidecars and child ``@task`` components sidecars — are in + # ``ctx.planned_files`` and count as present, so the compiler-managed + # parent→child / child→child / child @task refs pass. A child's + # author-written relative leaf ref was relocated (NM2) to be relative to + # the child-sidecar dir, so it is validated against the real source-side + # file via ``../``; a missing external leaf fails clearly here, before + # any file is written. + _validate_local_component_refs_for_artifact( + body, + artifact.output_path.parent, + ctx.planned_files, + artifact_label=label, + ) + artifact.dumped_text = dumped + + +# Pending componentRef sentinels stamped during trace/emit. All MUST be +# rewritten before validation; a survivor is an internal compiler bug. +_PENDING_SENTINELS = ( + _SUBPIPELINE_URL_PLACEHOLDER, + _TASK_URL_PLACEHOLDER, + _REGISTERED_URL_PLACEHOLDER, +) + + +def _assert_no_pending_sentinels(body_dict: dict[str, Any], artifact_label: str | None) -> None: + """Raise if any task ``componentRef.url`` still holds a pending compiler + sentinel (``subpipeline://pending`` / ``local-from-python://pending`` / + ``registered://pending``). + + These are stamped during trace/emit and rewritten to pure refs by the + compile driver; a survivor means a rewrite was missed. Fail clearly with + task context so the bug is obvious, and (because this runs before the + write pass) leave nothing on disk. + """ + where = artifact_label or "root pipeline" + tasks = body_dict.get("implementation", {}).get("graph", {}).get("tasks", {}) + for task_id, task in tasks.items(): + if not isinstance(task, dict): + continue + cref = task.get("componentRef") + if not isinstance(cref, dict): + continue + url = cref.get("url") + if isinstance(url, str) and url in _PENDING_SENTINELS: + raise CompileError( + f"internal error: {where} task {task_id!r} still carries the " + f"unresolved compiler sentinel {url!r} in its componentRef " + "(a missed @task / subpipeline rewrite). No output was written." + ) + + +def _write_artifact(artifact: SubgraphArtifact) -> None: + """Write a validated artifact (body YAML + optional @task sidecar). + + Only called after the WHOLE bundle has validated, so no partial bundle + is ever left on disk. + """ + assert artifact.dumped_text is not None # set by _validate_artifact + artifact.output_path.parent.mkdir(parents=True, exist_ok=True) + artifact.output_path.write_text(artifact.dumped_text) + if artifact.components_entries: + assert artifact.components_path is not None # narrow for type-checkers + artifact.components_path.write_text(dump_yaml(artifact.components_entries, sort_keys=False)) + + +def _artifact_label(artifact: SubgraphArtifact) -> str: + """Short ``child pipeline '' ()`` label for + error messages on a non-root artifact.""" + return f"child pipeline {artifact.key.pipeline_name!r} " f"({artifact.output_path.name})" + + +# --------------------------------------------------------------------------- +# @task local_from_python sidecar helpers +# +# These build the +# ``.components.yaml`` resolver config for @task-derived refs and +# rewrite each task's componentRef to a pure ``resolve://`` URL. Paths are +# POSIX and relative to the sidecar directory so the compiled bundle is +# portable (the sidecar and the files it points at can move together). + + +def _fragment_for_task(ref: CallableRef) -> str: + """Stable fragment key for a @task ref in the components sidecar. + + Uses the hyphenated function name, matching tangle-deploy's own + ``_resolve_local_from_python`` output-filename convention + (``my_task`` -> ``my-task``). Multiple call sites for the SAME + function share one fragment — the local_from_python entry is keyed by + source file, not by call site. + """ + assert ref._task_function_name is not None # only @task refs reach here + return ref._task_function_name.replace("_", "-") + + +def _relpath_posix(target: Path, start: Path) -> str: + """``os.path.relpath`` but always POSIX-style for YAML stability. + + Raises: + CompileError: when no relative path can be formed (e.g. ``target`` + and ``start`` are on different drives on Windows). + """ + try: + rel = os.path.relpath(str(target), str(start)) + except ValueError as e: + raise CompileError( + f"cannot form a relative path from {start} to {target}: {e}. " + "Compile into the pipeline source directory, or reference the " + "component with an absolute file://, gs://, or resolve:// URL." + ) from e + # Normalise to POSIX separators so the sidecar diffs are stable. + rel = rel.replace(os.sep, "/") + # Prefix bare relative paths with ``./`` to match the + # local_from_python convention. + if not rel.startswith(".") and not rel.startswith("/"): + rel = "./" + rel + return rel + + +# ---------------------------------------------------------------------------- +# @registered URL rewrite. Unlike @task (which builds a sidecar), @registered +# references an EXISTING gen_config.yaml: the driver only resolves that config +# and rewrites each registered task's ``registered://pending`` sentinel to a +# pure ``resolve:///gen_config.yaml#`` URL. Resolution +# touches the filesystem (marker walk, nearest-gen_config walk, relpath) and so +# happens lazily here at compile time, never at decoration/import time. + + +def _fragment_for_registered(ref: CallableRef) -> str: + """Fragment key for a @registered ref's ``resolve://`` URL. + + Per decision 2, the author-supplied ``fragment`` is used VERBATIM as the + top-level key in ``gen_config.yaml``. When omitted, it defaults to the + function name VERBATIM (no hyphenation, unlike :func:`_fragment_for_task`) + — gen_config fragments are hand-authored keys, not generated filenames, so + we must not mangle them. + """ + fragment = ref._registered_fragment or ref._registered_function_name + assert fragment is not None # only @registered refs reach here + return fragment + + +# Zone-root marker filenames — the extension seam for distributions that +# carry a zone concept. An explicit relative ``@registered(gen_config=...)`` +# path is resolved against the nearest ancestor directory that contains one +# of these marker files (see :func:`_find_zone_root`). +# +# EMPTY by default: the open-source ``tangle`` CLI has no zone concept, so an +# explicit relative gen_config path is unsupported until a downstream +# distribution registers a marker. A distribution that DOES carry a zone +# concept appends its own marker filename at import time to restore zone-root +# resolution. This mirrors the mutable module-level registry seams +# elsewhere in the CLI (e.g. the hydrator's ``COMPONENT_RESOLVERS``): mutate +# the list in place rather than rebinding it, so callers that already imported +# the name observe the addition. +ZONE_ROOT_MARKERS: list[str] = [] + + +def _find_zone_root(source: Path) -> Path | None: + """Return the nearest ancestor dir of ``source`` holding a zone-root + marker (see :data:`ZONE_ROOT_MARKERS`), or ``None`` if none exists. + + An explicit ``@registered(gen_config="rel/path")`` is resolved relative to + this directory (decision 1) so authors write the gen_config path the same + way the distribution's existing ``resolve://`` references do. When no + markers are registered (the default open-source build) this always returns + ``None`` and explicit relative gen_config paths are rejected with an + actionable error. + """ + if not ZONE_ROOT_MARKERS: + return None + for ancestor in source.parents: + if any((ancestor / marker).is_file() for marker in ZONE_ROOT_MARKERS): + return ancestor + return None + + +def _find_nearest_gen_config(source: Path) -> Path | None: + """Return the nearest ancestor ``gen_config.yaml`` of ``source``, or + ``None`` if none exists. + + Used as the default when ``@registered`` is given no explicit + ``gen_config=`` (decision 1): the operation file is assumed to live under + (or beside) the gen_config.yaml it is registered in. + """ + for ancestor in source.parents: + candidate = ancestor / "gen_config.yaml" + if candidate.is_file(): + return candidate + return None + + +def _resolve_registered_gen_config(ref: CallableRef) -> Path | str: + """Resolve a @registered ref's gen_config to a value :func:`_rewrite_registered_componentref_urls` emits. + + Return contract (this is what ``_rewrite`` keys off of): + + - a returned ``str`` is emitted VERBATIM after the ``resolve://`` scheme, + with NO relpath. Two cases produce a ``str``: + * a genuinely remote scheme (``gs://`` / ``http://`` / ``https://``) — + the hydrator fetches it directly and it cannot be existence-checked + at compile time; + * a validated absolute LOCAL path — the resolved, on-disk-checked + ``os.fspath(...)`` of a ``file://`` URL or an absolute filesystem + path. Emitted as ``resolve:///abs/x.yaml#frag``, which the hydrator + resolves via ``Path("/abs/x.yaml")``. + - a returned ``Path`` is relpath'd against the artifact output dir by + ``_rewrite`` (the relative/marker and omitted/nearest cases). + + Resolution rules (decision 1): + + - ``gs://`` / ``http(s)://`` -> returned VERBATIM as a ``str``. + - ``file://...`` -> the ``file://`` prefix is STRIPPED and the remainder + treated as a LOCAL path. WHY strip: the hydrator's ``resolve://`` parser + does NOT understand a nested ``file://`` (it does ``Path(file_path)`` + with no scheme stripping and special-cases ``gs://`` only), so a + ``file://``-wrapped payload like ``resolve://file:///abs/x.yaml`` fails + to hydrate — ``Path("file:///abs/x.yaml")`` is mis-parsed as a relative + segment. Stripping at compile time makes documented ``file://`` usage + actually resolvable. A non-absolute remainder is resolved against the op + source file's directory. The path is then ``.resolve()``-d, + existence-checked, and returned as ``os.fspath(...)`` (a ``str``). + - an absolute filesystem path (no scheme) -> ``.resolve()``-d, + existence-checked, returned as ``os.fspath(...)`` (a ``str``). + - explicit relative ``gen_config=`` -> resolved against the zone root + (the nearest ancestor holding a registered :data:`ZONE_ROOT_MARKERS` + marker), existence-checked, returned as a ``Path``. + - omitted ``gen_config`` -> the nearest ancestor ``gen_config.yaml``, + returned as a ``Path``. + + Raises: + CompileError: when an explicit relative path has no zone-root marker + above the source; when an omitted path finds no ancestor + gen_config.yaml; when an explicit relative path resolves to a + non-existent file; or when a ``file://`` URL / absolute path + resolves to a path that does not exist on disk. + """ + raw = ref._registered_gen_config + source = ref._registered_source_path + assert source is not None # only @registered refs reach here + + # Genuinely remote schemes -> used verbatim; the hydrator fetches them + # directly and they can't be existence-checked at compile time. + if raw is not None and (raw.startswith("gs://") or raw.startswith("http://") or raw.startswith("https://")): + return raw + + # file:// URL or absolute filesystem path -> a validated absolute LOCAL + # path, emitted verbatim (no relpath). The file:// scheme is stripped here + # because the hydrator's resolve:// parser doesn't understand it. + if raw is not None and (raw.startswith("file://") or os.path.isabs(raw)): + if raw.startswith("file://"): + # Mirror the hydrator's own file:// stripping + # (pipeline_hydrator.py: ``url[len("file://"):]``). + local = raw[len("file://") :] + local_path = Path(local) + # A non-absolute remainder (e.g. ``file://rel/x.yaml``) is resolved + # against the op source file's directory. + if not local_path.is_absolute(): + local_path = source.parent / local_path + else: + local_path = Path(raw) + resolved = local_path.resolve() + if not resolved.exists(): + raise CompileError( + f"@registered(gen_config={raw!r}) resolves to {resolved}, which does not " "exist on disk." + ) + return os.fspath(resolved) + + if raw is not None: + # Explicit relative path -> marker-relative (zone root). + root = _find_zone_root(source) + if root is None: + if ZONE_ROOT_MARKERS: + markers = " / ".join(repr(m) for m in ZONE_ROOT_MARKERS) + raise CompileError( + f"@registered(gen_config={raw!r}) is a relative path but no " + f"zone-root marker ({markers}) was found in any ancestor " + f"directory of {source}. Add the marker at the zone root, or " + "pass an absolute / gs:// gen_config path instead." + ) + raise CompileError( + f"@registered(gen_config={raw!r}) is a relative path, but this " + "build has no zone-root markers registered, so a zone root " + f"cannot be located for {source}. Pass an absolute / file:// / " + "gs:// gen_config path instead, or omit gen_config to use the " + "nearest ancestor 'gen_config.yaml'." + ) + resolved = (root / raw).resolve() + else: + # Omitted -> nearest ancestor gen_config.yaml. + nearest = _find_nearest_gen_config(source) + if nearest is None: + raise CompileError( + "@registered could not find a 'gen_config.yaml' in any ancestor " + f"directory of {source}. Pass gen_config=... explicitly (relative " + "to the zone-root marker, or an absolute / gs:// path)." + ) + resolved = nearest + + if not resolved.exists(): + raise CompileError( + f"@registered gen_config not found on disk: {resolved}. Check the " + "path and the zone-root marker location." + ) + return resolved + + +def _rewrite_registered_componentref_urls( + *, + body_dict: dict[str, Any], + registered_refs: list[tuple[str, CallableRef]], + artifact_output_dir: Path, +) -> None: + """Rewrite each @registered task's ``componentRef`` to a pure resolve URL. + + Mutates ``body_dict`` in place. Each entry in ``registered_refs`` is a + ``(task_id, CallableRef)`` tuple; the task_id is the key under + ``implementation.graph.tasks`` whose componentRef is replaced with + ``{"url": "resolve:///gen_config.yaml#"}``. + + The relative path is computed against ``artifact_output_dir`` (this + artifact's own output directory) so nested subpipeline children, written + under ``.subgraphs/``, get a correct relative path. The emission + form depends on what :func:`_resolve_registered_gen_config` returns: + + - ``gs://`` / ``http(s)://`` -> emitted verbatim after ``resolve://`` + (genuinely remote, no relpath). + - a ``file://`` URL or an absolute path -> resolved to a validated + absolute LOCAL path (existence-checked at compile time) and emitted as + an absolute ``resolve://`` URL — the ``file://`` scheme is stripped at + resolve time so the hydrator's ``resolve://`` parser can read it. + - relative / marker / omitted -> relpath'd against ``artifact_output_dir``. + """ + tasks = body_dict.get("implementation", {}).get("graph", {}).get("tasks", {}) + for task_id, ref in registered_refs: + target = _resolve_registered_gen_config(ref) + fragment = _fragment_for_registered(ref) + if isinstance(target, str): + # Remote / absolute override -> used verbatim, no relpath. + url = f"resolve://{target}#{fragment}" + else: + url = f"resolve://{_relpath_posix(target, artifact_output_dir)}#{fragment}" + if task_id not in tasks: + raise CompileError( + f"@registered ref recorded for task {task_id!r} but no such task " + "in the emitted body (internal error)." + ) + tasks[task_id]["componentRef"] = {"url": url} + + +def _relocate_child_local_refs( + *, + body_dict: dict[str, Any], + builder: Any, + source_dir: Path, + sidecar_dir: Path, +) -> None: + """Rewrite a child's author-written relative local componentRefs from + being relative to its SOURCE dir to relative to its SIDECAR dir. + + A child compiles into ``.subgraphs/`` but its ``ref(url=...)`` + URLs were authored relative to the child's own source file. Rewriting + them keeps each ref pointing at the SAME original file (no copying) so + the existing hydrator — which resolves child refs relative to the + loaded sidecar's directory — still finds it. + + Skips compiler-managed tasks (``@task`` resolver sidecars, nested + subpipeline refs, and ``@registered`` refs): those are rewritten by + their own driver passes to URLs already computed against the sidecar / + output directory, not source-relative files. Every relative form (bare + ``file://x.yaml``, ``./`` and ``../``) is relocated; only absolute / + remote URLs (``file:///``, ``gs://``, ``http(s)://``) are left + untouched. + """ + managed: set[str] = {tid for tid, _ref in builder.task_refs_for_local_from_python} + managed |= {tid for tid, _ref in builder.task_refs_for_subpipelines} + managed |= {tid for tid, _ref in builder.task_refs_for_registered} + tasks = body_dict.get("implementation", {}).get("graph", {}).get("tasks", {}) + for task_id, task in tasks.items(): + if task_id in managed or not isinstance(task, dict): + continue + cref = task.get("componentRef") + if not isinstance(cref, dict): + continue + url = cref.get("url") + if not isinstance(url, str): + continue + relocated = _relocate_relative_local_url(url, source_dir, sidecar_dir) + if relocated is not None: + cref["url"] = relocated + + +def _relocate_relative_local_url(url: str, source_dir: Path, sidecar_dir: Path) -> str | None: + """Return ``url`` rewritten relative to ``sidecar_dir`` instead of + ``source_dir``, preserving the scheme and any ``#fragment``. + + EVERY RELATIVE local ``file://``/``resolve://`` URL is relocated -- + bare (``file://child.yaml``), ``./`` (``file://./child.yaml``) and + ``../`` (``file://../child.yaml``) forms alike -- because the hydrator + resolves ANY non-absolute ``file://``/``resolve://`` path relative to + the loaded YAML's directory (see + ``PipelineHydrator._fetch_component_from_file_url``). Returns ``None`` + (leave as-is) for absolute (``file:///abs``) or remote URLs the + compiler must not touch, for the empty path, AND for payloads that are + themselves a remote/absolute URL (``resolve://gs://…``, + ``resolve://https://…``, ``file://gs://…``) — those are not relative + local paths and must reach the hydrator as-authored rather than being + mangled into a bogus local path (the hydrator supports ``gs://`` + resolve configs; unsupported nested-remote refs surface their own + clear error there). + """ + for scheme in ("file://", "resolve://"): + if not url.startswith(scheme): + continue + rest = url[len(scheme) :] + path_part, sep, fragment = rest.partition("#") + # Skip ABSOLUTE local refs (``file:///abs`` -> ``path_part`` starts + # with ``/``), the empty path, AND nested remote/absolute payloads + # (``resolve://gs://…``, ``file://gs://…``, ``resolve://https://…``). + # A nested URL always contains ``://``; a relative local path never + # does, so ``"://" in path_part`` cleanly discriminates the two. + # (A scheme-looking relative path like ``file://foo://bar.yaml`` is + # also skipped — an intentional, acceptable tradeoff: such ambiguous + # scheme-looking local paths are unsupported, and skipping is + # preferred over a brittle hydrator-scheme allowlist.) + if not path_part or path_part.startswith("/") or "://" in path_part: + return None + target = (source_dir / path_part).resolve() + new_rel = _relpath_posix(target, sidecar_dir) + return f"{scheme}{new_rel}#{fragment}" if sep else f"{scheme}{new_rel}" + return None + + +def _build_local_from_python_components( + task_refs: list[tuple[str, CallableRef]], *, components_yaml_dir: Path +) -> dict[str, Any]: + """Build the ``.components.yaml`` content for @task refs. + + Returns an ordered map ``{fragment: {name?, local_from_python: + {image?, function, dependencies_from?, file}}}``, DEDUPED by FUNCTION + (the fragment = hyphenated function name). The SAME @task function + called from multiple task sites collapses to one entry; TWO DISTINCT + @task functions defined in ONE file each get their own entry (they + share the same ``file:`` but carry different ``function:`` keys and + distinct fragments). This matches how ``_rewrite_task_componentref_urls`` + points each task at its OWN function fragment — deduping by source path + instead would drop every function but the first and leave the others' + ``resolve://...#`` refs dangling. + + The ``function`` field is always emitted so hydrate's + ``regenerate_yaml`` extracts the right function: it otherwise defaults + to the file STEM, which is wrong whenever the @task function name + differs from the source filename (the common case). + + Paths in ``local_from_python.{file,dependencies_from}`` are POSIX and + relative to ``components_yaml_dir`` so the sidecar is portable: as + long as the layout under that directory matches at compile- and + hydrate-time, the paths resolve correctly. + + Raises: + CompileError: when two distinct source files map to the same + fragment (function-name collision), when a referenced local + file (the @task source or its ``dependencies_from``) is + unreachable, or when a relative path cannot be formed (see + :func:`_relpath_posix`). + """ + seen_fragments: dict[str, Path] = {} + entries: dict[str, Any] = {} + for _task_id, ref in task_refs: + source = ref._task_source_path + if source is None: # defensive — only @task refs are recorded here + continue + fragment = _fragment_for_task(ref) + + prior_source = seen_fragments.get(fragment) + if prior_source is not None: + # Already emitted this fragment. Fine when it is the SAME source + # (the same @task called from multiple sites). A DIFFERENT + # source sharing the function name would silently collide on the + # resolve:// fragment, so reject it loudly. + if prior_source != source: + raise CompileError( + "two distinct @task source files map to the same sidecar " + f"fragment {fragment!r}: {prior_source} and {source}. " + "Rename one of the @task functions so each has a unique " + "name (the function name becomes the resolve:// fragment)." + ) + continue + + if not source.exists(): + raise CompileError( + f"@task source file is unreachable: {source}. Compile into " + "the pipeline source directory, or reference the component " + "with an absolute file://, gs://, or resolve:// URL." + ) + + local_from_python: dict[str, Any] = {} + if ref._task_image is not None: + local_from_python["image"] = ref._task_image + # Always pin the function name. Without it the hydrator defaults + # to the file stem and extracts the wrong symbol. + assert ref._task_function_name is not None + local_from_python["function"] = ref._task_function_name + if ref._task_dependencies_from is not None: + deps = ref._task_dependencies_from + if not deps.exists(): + raise CompileError( + f"@task dependencies_from file is unreachable: {deps}. " + "Point dependencies_from at an existing file or drop it." + ) + local_from_python["dependencies_from"] = _relpath_posix(deps, components_yaml_dir) + local_from_python["file"] = _relpath_posix(source, components_yaml_dir) + + seen_fragments[fragment] = source + # The component name is NOT emitted here. A top-level ``name`` on a + # resolve entry means "resolve a published component by this name" to + # the hydrator (PipelineHydrator._resolve_primary), which would let a + # same-named library component silently win over this local @task. The + # component's name comes from its source docstring (``Metadata: Name:``) + # at hydrate time, read by regenerate_yaml. + entry: dict[str, Any] = {"local_from_python": local_from_python} + entries[fragment] = entry + return entries + + +def _rewrite_task_componentref_urls( + *, + body_dict: dict[str, Any], + task_refs: list[tuple[str, CallableRef]], + components_yaml_name: str, +) -> None: + """Rewrite each @task task's ``componentRef`` to a pure resolve URL. + + Mutates ``body_dict`` in place. Each entry in ``task_refs`` is a + ``(task_id, CallableRef)`` tuple; the task_id is the key under + ``implementation.graph.tasks`` whose componentRef is replaced with + ``{"url": "resolve://./#"}``. + """ + tasks = body_dict.get("implementation", {}).get("graph", {}).get("tasks", {}) + for task_id, ref in task_refs: + fragment = _fragment_for_task(ref) + url = f"resolve://./{components_yaml_name}#{fragment}" + if task_id not in tasks: + raise CompileError( + f"@task ref recorded for task {task_id!r} but no such task in " "the emitted body (internal error)." + ) + tasks[task_id]["componentRef"] = {"url": url} + + +def _relative_local_ref_target(url: str) -> str | None: + """Return the relative path portion of a RELATIVE local componentRef + URL, stripped of any ``#fragment``. + + Matches EVERY relative ``file://``/``resolve://`` form -- bare + (``file://child.yaml``), ``file://./``, ``file://../`` and the + ``resolve://`` equivalents -- because the hydrator resolves ANY + non-absolute path relative to the compiled YAML's directory. Returns + ``None`` for absolute (``file:///``), remote (``gs://``, + ``http(s)://``), empty, payloads that are themselves a remote/absolute + URL (``resolve://gs://…``, ``resolve://https://…``, ``file://gs://…``), + or otherwise non-relative URLs — those are the user's responsibility + and are not checked at compile time (the hydrator handles supported + nested-remote configs like ``resolve://gs://…`` and surfaces its own + clear error for unsupported ones). + """ + for scheme in ("file://", "resolve://"): + if url.startswith(scheme): + rest = url[len(scheme) :].split("#", 1)[0] + # Skip absolute local refs (``file:///abs``), the empty path, + # AND nested remote/absolute payloads (``resolve://gs://…``, + # ``file://gs://…``, ``resolve://https://…``). A nested URL + # always contains ``://``; a relative local path never does, so + # ``"://" in rest`` cleanly discriminates the two. (A + # scheme-looking relative path like ``file://foo://bar.yaml`` is + # also skipped — an intentional, acceptable tradeoff over a + # brittle hydrator-scheme allowlist.) Treat every remaining + # relative form (bare, ``./``, ``../``) as a relative local ref, + # mirroring the hydrator. + if not rest or rest.startswith("/") or "://" in rest: + return None + return rest + return None + + +def _validate_local_component_refs_for_artifact( + body_dict: dict[str, Any], + output_dir: Path, + planned_files: set[Path], + *, + artifact_label: str | None = None, +) -> None: + """Assert every RELATIVE local componentRef target either exists relative + to ``output_dir`` or is a file the compiler is about to write. + + Hydrate resolves componentRef URLs relative to the artifact YAML's own + location, so a relative ``file://./x`` / ``resolve://./x`` ref is only + resolvable if ``x`` sits next to that artifact. Generated bundle files + (child sidecars, ``@task`` components sidecars) are passed in + ``planned_files`` — they validate as "present" before they are written + (Decision J). External relative refs must already exist on disk. + + Args: + artifact_label: ``None`` for the ROOT (uses the legacy error wording + relative to the OUTPUT directory); a ``child pipeline '' + ()`` label for a child sidecar (uses child-context wording + relative to the child sidecar's directory). + + Raises: + CompileError: with actionable guidance when a referenced local + component file is unreachable. We do NOT silently copy files. + """ + tasks = body_dict.get("implementation", {}).get("graph", {}).get("tasks", {}) + for task_id, task in tasks.items(): + if not isinstance(task, dict): + continue + cref = task.get("componentRef") + if not isinstance(cref, dict): + continue + url = cref.get("url") + if not isinstance(url, str): + continue + rel = _relative_local_ref_target(url) + if rel is None: + continue + target = (output_dir / rel).resolve() + if target in planned_files: + continue # a sidecar this same compile is about to write + if target.exists(): + continue + if artifact_label is None: + raise CompileError( + f"task {task_id!r} references local component {url!r}, but the " + f"target does not exist relative to the output directory: " + f"{target}. Hydrate resolves componentRef URLs relative to the " + "compiled YAML's location, so the referenced file must sit " + "next to the compiled output. Fix options: compile into the " + "pipeline source directory so referenced files are colocated " + "with the output; place the referenced component next to the " + "compiled YAML; or use an absolute file:///… / gs://… URL or a " + "published name: ref." + ) + raise CompileError( + f"{artifact_label} task {task_id!r} references local component " + f"{url!r}, but hydrate will resolve it relative to the child " + f"sidecar directory: {target}. Place the referenced component next " + "to the child sidecar, compile into a bundle directory with that " + "layout, or use an absolute file:///… / gs://… / resolve://… " + "reference." + ) + + +# --------------------------------------------------------------------------- +# Orchestration helpers + + +def _parse_overrides(entries: list[str]) -> dict[str, str]: + """Parse ``--override key=value`` strings into a dict. + + Raises: + CompileError: if an entry has no ``=`` separator. + """ + overrides: dict[str, str] = {} + for entry in entries: + if "=" not in entry: + raise CompileError(f"--override expects key=value, got {entry!r} (no '=' separator)") + key, _, value = entry.partition("=") + overrides[key] = value + return overrides + + +def _evict_shadowed_bundle_modules(bundle_dir: Path) -> None: + """Evict ``sys.modules`` entries that shadow a module file in ``bundle_dir``. + + Python caches imported modules by NAME, not path, so a previously imported + top-level module (from a prior in-process compile, a test helper, or a + REPL) named e.g. ``child_pipeline`` would be reused by a sibling + ``from child_pipeline import ...`` even when the bundle being compiled + ships its OWN, DIFFERENT ``child_pipeline.py``. Evicting any cached entry + whose name matches a ``.py`` stem in ``bundle_dir`` but whose ``__file__`` + differs forces a FRESH import from the bundle's ``sys.path``. Re-importing + is always safe; the after-compile purge then drops the freshly imported + entries too, leaving global import state clean. + """ + try: + entries = list(bundle_dir.iterdir()) + except OSError: # pragma: no cover — defensive (missing/inaccessible dir) + return + for entry in entries: + if entry.suffix != ".py": + continue + mod = sys.modules.get(entry.stem) + if mod is None: + continue + file = getattr(mod, "__file__", None) + if not file: + continue + try: + same = Path(file).resolve() == entry.resolve() + except OSError: # pragma: no cover — defensive + same = False + if not same: + del sys.modules[entry.stem] + + +def _purge_bundle_local_modules(source_dirs: set[Path]) -> None: + """Drop every ``sys.modules`` entry loaded from the bundle's source tree. + + ``_load_pipeline_fn`` exec's the root module under a unique synthetic name + (restored in its own ``finally``), but the root's top-level + ``from sibling import ...`` statements — and any trace-time sibling + imports inside cycle-style children — register sibling/helper modules + under their REAL names in ``sys.modules``. Those entries are not cleaned + up by the synthetic-name / ``sys.path`` restoration, so a SUBSEQUENT + in-process compile of a DIFFERENT bundle that happens to define a module + with the SAME name (e.g. another ``child_pipeline.py`` in a different temp + dir) would reuse the STALE cached module and validate against the wrong + child. + + This is called AFTER the full compile (success or failure). By then the + traced functions already hold their child ``PipelineFn`` objects by + reference, so removing the ``sys.modules`` entries is safe — the only goal + is that the next compile re-imports fresh. + + A module is purged whenever its ``__file__`` resolves under one of + ``source_dirs`` (the root script dir and every compiled child's own source + dir). The ``modules_before`` set is deliberately NOT consulted: the + up-front eviction may have REPLACED a pre-existing cached name with this + bundle's own module, so a name that pre-dated the compile can still hold a + bundle-local module that must be cleared. Modules outside the bundle tree + (stdlib, site-packages, the CLI package itself) are never touched. The + leak-free synthetic-root-name and ``sys.path`` restoration in + ``_load_pipeline_fn`` / ``_temp_sys_path`` is left intact. + """ + resolved_dirs: list[Path] = [] + for d in source_dirs: + try: + resolved_dirs.append(d.resolve()) + except OSError: # pragma: no cover — defensive + continue + if not resolved_dirs: + return + for name in list(sys.modules): + mod = sys.modules.get(name) + file = getattr(mod, "__file__", None) + if not file: + continue + try: + mod_path = Path(file).resolve() + except OSError: # pragma: no cover — defensive + continue + if any(mod_path.is_relative_to(d) for d in resolved_dirs): + del sys.modules[name] + + +def _candidate_names(fn: PipelineFn) -> tuple[str, str]: + """Return ``(function_name, display_name)`` for ``--pipeline`` matching. + + ``function_name`` is the decorated function's ``__name__`` (a clean, + shell-friendly identifier such as ``options_standardization``); + ``display_name`` is the ``@pipeline(name=...)`` value emitted as the + YAML ``name:`` (may contain spaces, e.g. ``"Options Standardization"``). + Selection matches the function name first, then the display name. + """ + func_name = getattr(fn.fn, "__name__", "") or "" + return (func_name, fn.name) + + +def _format_candidates(candidates: list[PipelineFn]) -> str: + """Render candidates as ``function_name ("Display Name")`` for errors.""" + return ", ".join(f'{func} ("{disp}")' for func, disp in (_candidate_names(c) for c in candidates)) + + +def _select_by_name(candidates: list[PipelineFn], pipeline_name: str) -> list[PipelineFn]: + """Filter ``candidates`` matching ``pipeline_name``. + + The function ``__name__`` is matched first (preferred — it is the + shell-friendly identifier and the documented disambiguator); only when + no function name matches does the ``@pipeline`` display name apply. This + ordering means a file with two same-display-name siblings can always be + disambiguated by passing a function name. + """ + by_func = [c for c in candidates if _candidate_names(c)[0] == pipeline_name] + if by_func: + return by_func + return [c for c in candidates if _candidate_names(c)[1] == pipeline_name] + + +def _load_pipeline_fn(module_path: Path, pipeline_name: str | None = None) -> PipelineFn: + """Load ``module_path`` as a module and return a single PipelineFn. + + When the file defines exactly one root pipeline it is returned + directly. When it defines several, ``pipeline_name`` selects which one + to compile (matched against the function ``__name__`` first, then the + ``@pipeline`` display name — Decision G / ``--pipeline``). The selected + pipeline's same-file nested children are still reachable to + ``subpipeline(child)(...)`` and are compiled via the recursion driver, + never via this candidate list. + + Leak-free: the module's parent dir is temporarily added to + ``sys.path`` (so top-level ``import`` of sibling modules resolves + during exec) and the module is registered under a UNIQUE name in + ``sys.modules``. Both are restored in a ``finally`` so repeated or + concurrent in-process compiles never collide and no global import + state leaks. The loaded module object stays alive via the returned + ``PipelineFn`` regardless of the ``sys.modules`` cleanup. + """ + module_dir = str(module_path.parent) + # Unique module name so repeated/concurrent in-process compiles don't + # collide and so we can cleanly remove our own sys.modules entry. + module_name = f"_tangle_user_pipeline_{uuid.uuid4().hex}" + + spec = importlib.util.spec_from_file_location(module_name, str(module_path)) + if spec is None or spec.loader is None: + raise CompileError(f"could not load module spec from {module_path}") + mod = importlib.util.module_from_spec(spec) + + # Snapshot global import state so we can restore it leak-free. + added_to_sys_path = module_dir not in sys.path + prior_mod = sys.modules.get(module_name) + if added_to_sys_path: + sys.path.insert(0, module_dir) + sys.modules[module_name] = mod + try: + spec.loader.exec_module(mod) + except Exception as e: + # Re-surface user-module exceptions wrapped as CompileError so + # the CLI exits 1 cleanly. + raise CompileError(f"error importing pipeline file {module_path}: {e}") from e + finally: + # Restore the sys.modules entry (delete ours, or put back prior). + if prior_mod is not None: + sys.modules[module_name] = prior_mod + else: + sys.modules.pop(module_name, None) + # Remove the dir from sys.path only if we added it. + if added_to_sys_path: + try: + sys.path.remove(module_dir) + except ValueError: # pragma: no cover — defensive + pass + + all_candidates = [v for v in vars(mod).values() if isinstance(v, PipelineFn)] + if not all_candidates: + raise CompileError(f"no @pipeline-decorated function found in {module_path}") + + # Root discovery selects the pipeline(s) DEFINED IN the target file. A + # parent module that imports child pipelines (so it can wrap them with + # ``subpipeline(child)(...)``) must not count those imports as roots + # (Decision G). Compare each candidate's source file against + # ``module_path``; imported children resolve to a DIFFERENT file and are + # ignored as compile targets (they remain reachable to ``subpipeline``). + # Several pipelines DEFINED in this one file are now allowed: when more + # than one is local the caller selects which to emit via ``pipeline_name`` + # (``--pipeline``); same-file nested children are then compiled through + # the ``subpipeline`` recursion, not picked from this candidate list. + target = module_path.resolve() + local_candidates: list[PipelineFn] = [] + undetermined: list[PipelineFn] = [] + for cand in all_candidates: + src = _pipeline_source_path(cand) + if src is None: + undetermined.append(cand) + elif src == target: + local_candidates.append(cand) + # else: defined in another file (imported child) -> not a root. + + if not local_candidates and not undetermined: + # Every candidate is defined in another file: the target imports + # pipelines but defines none of its own. + names = [getattr(c, "name", "?") for c in all_candidates] + raise CompileError( + f"no @pipeline-decorated function defined in {module_path}; found " + f"only imported pipeline(s): {names!r}. Define the compile-target " + "@pipeline in this file (imported child pipelines are wrapped with " + "subpipeline(child)(...), not compiled directly)." + ) + + if pipeline_name is not None: + # Explicit selection (``--pipeline``). Match against the function + # name then the display name, across local AND undetermined-source + # candidates (D4 — an exec'd module still exposes ``fn.__name__``). + # Imported children are never selectable as a root. + searchable = [*local_candidates, *undetermined] + matches = _select_by_name(searchable, pipeline_name) + if len(matches) == 1: + return matches[0] + if not matches: + raise CompileError( + f"pipeline {pipeline_name!r} not found in {module_path}; " + f"available: {_format_candidates(searchable)}." + ) + # Two or more matched (same display name on distinct functions). + func_names = ", ".join(_candidate_names(c)[0] for c in matches) + raise CompileError( + f"pipeline name {pipeline_name!r} is ambiguous in {module_path}; " f"use the function name: {func_names}." + ) + + # No explicit selection. Prefer locals; fall back to undetermined with + # the single-candidate dynamic auto-select (Decision G / D4): a single + # exec-built PipelineFn whose source cannot be resolved is still a valid + # root, but two undetermined candidates cannot be told apart from + # imports, so they fall through to the multiple-pipelines error. + if local_candidates: + candidates = local_candidates + else: + if len(all_candidates) == 1: + return all_candidates[0] + candidates = undetermined + + if len(candidates) == 1: + return candidates[0] + + raise CompileError( + f"multiple @pipeline-decorated functions found in {module_path}: " + f"{_format_candidates(candidates)}. Pass --pipeline to select " + "one (function name or display name)." + ) + + +def _pipeline_source_path(pipeline_fn: PipelineFn) -> Path | None: + """Resolve the source file that DEFINES ``pipeline_fn``'s function. + + Returns the symlink-resolved :class:`Path`, or ``None`` when the + source is UNDETERMINED — i.e. a dynamically built function whose + source cannot be located. Used by :func:`_load_pipeline_fn` to + distinguish locally defined root pipelines from imported child + pipelines, and to honour Decision G's single-candidate dynamic + fallback. + + A source is treated as undetermined (``None``) when: + + * it cannot be read at all (``inspect`` failure, no ``co_filename``); + * it is a pseudo filename produced by ``exec``/``eval``/the REPL, + e.g. ```` / ```` (matched as ``<...>``); or + * the resolved path does not exist on disk (so it cannot be + meaningfully compared against the real target module file). + """ + fn = pipeline_fn.fn + src: str | None + try: + src = inspect.getsourcefile(fn) or inspect.getfile(fn) + except (TypeError, OSError): + src = None + if not src: + code = getattr(fn, "__code__", None) + src = getattr(code, "co_filename", None) + if not src: + return None + # Pseudo filenames from exec/eval/REPL ("", "", ...) + # are not real on-disk sources — treat as undetermined so the + # single-candidate dynamic fallback in _load_pipeline_fn can apply. + if src.startswith("<") and src.endswith(">"): + return None + try: + resolved = Path(src).resolve() + except OSError: # pragma: no cover — defensive + return None + # A non-existent path (e.g. a synthetic co_filename) cannot be + # compared against the target module file — treat as undetermined. + if not resolved.exists(): + return None + return resolved + + +def _pipeline_accepts_cfg(pipeline_fn: PipelineFn) -> bool: + """Return whether the pipeline function has a real ``cfg`` parameter. + + ``@pipeline(config=...)`` only matters when the authored function accepts a + parameter named ``cfg`` (and that parameter is not an ``In[T]`` graph input). + Pipelines without such a parameter cannot observe compile-time config, so + requiring the file would make stale decorator metadata unnecessarily fatal. + """ + try: + sig = inspect.signature(pipeline_fn.fn) + except (TypeError, ValueError): # pragma: no cover — defensive + return False + param = sig.parameters.get("cfg") + if param is None: + return False + + annotation = param.annotation + try: + resolved_hints = get_type_hints(pipeline_fn.fn, include_extras=True) + annotation = resolved_hints.get("cfg", annotation) + except Exception: + pass + return getattr(annotation, "__origin__", None) is not In + + +def _resolve_cfg_path(pipeline_fn: PipelineFn, module_path: Path) -> Path: + """Resolve the cfg path declared by ``@pipeline(config=...)``. + + Relative to the file holding the decorated function. If the + decorator omits ``config=``, returns the module's directory / + ``config.yaml`` as the default. Thin wrapper over + :func:`_resolve_cfg_path_in_dir` for callers that have the module FILE + rather than its directory. + """ + return _resolve_cfg_path_in_dir(pipeline_fn, module_path.parent) + + +def _resolve_cfg_path_in_dir(pipeline_fn: PipelineFn, base_dir: Path) -> Path: + """Resolve ``@pipeline(config=...)`` relative to ``base_dir``. + + ``base_dir`` is the pipeline's OWN source directory — the root script's + parent for the root, or the child ``PipelineFn``'s source directory for + a nested child (Decision F: each pipeline loads its own config relative + to its own file). If the decorator omits ``config=``, defaults to + ``/config.yaml``. + """ + if pipeline_fn.config_path: + cfg_path = Path(pipeline_fn.config_path) + else: + cfg_path = Path("config.yaml") + if not cfg_path.is_absolute(): + cfg_path = base_dir / cfg_path + return cfg_path.resolve() + + +def _assert_config_output_path_is_separate( + cfg_path: Path, + output_path: Path, + *, + pipeline_fn: PipelineFn, + is_root: bool, +) -> None: + """Reject ``@pipeline(config=...)`` paths that collide with output. + + The config file is a compile-time INPUT read before the compiler writes + anything. Treating a missing output path as an empty config would mask + typos, and treating an existing output path as config would make the + compiler read a previous compiled artifact as its input config. Fail + before any writes with guidance that explains the two distinct paths. + """ + if cfg_path.resolve() != output_path.resolve(): + return + + output_label = "--output" if is_root else "generated child sidecar output" + raise CompileError( + f"@pipeline config for {pipeline_fn.name!r} resolves to the same path as " + f"the {output_label}: {cfg_path}. The `config=` argument names a " + "compile-time input config file; it is read before the compiled YAML " + "is written. tangle-deploy creates the output file automatically after " + "validation, so do not point `config=` at the output. Use a separate " + "config file (for example an empty `*.compile_config.yaml`) or omit " + "`config=` to use `config.yaml`, and keep `--output` for the compiled YAML." + ) + + +def _load_cfg_and_raw( + cfg_path: Path, + overrides: Mapping[str, Any], + *, + coerce: bool = True, + pipeline_fn: PipelineFn, + usage: str = "cfg", +): + """Load cfg via ``cfg.load_cfg`` AND read the raw YAML dict. + + The raw dict (config.yaml WITHOUT overrides applied) is returned so the + caller can build a broadcast layer from this pipeline's OWN config. + ``coerce`` is threaded to :func:`load_cfg`: the ROOT passes ``coerce=True`` + so its CLI ``--override`` strings keep YAML coercion; a CHILD passes + ``coerce=False`` so its already-typed native ``effective`` overrides pass + through unchanged. Overrides are applied to the :class:`Cfg` object only — + the compiled YAML carries no config keys. + """ + import yaml + + if not cfg_path.exists(): + source = ( + f"@pipeline(config={pipeline_fn.config_path!r})" + if pipeline_fn.config_path + else "the default config.yaml" + ) + if usage == "propagate_config": + reason = ( + "has propagate_config=True, so " + f"{source} is required as the config payload to broadcast to " + "descendant subpipelines. Create the file, remove `config=`, " + "or disable propagate_config if there is no config to broadcast." + ) + else: + reason = ( + "has a `cfg` parameter, so " + f"{source} is required as a compile-time input. Create the file, " + "or remove the `cfg` parameter (and omit `config=`) if the " + "pipeline does not use compile-time config." + ) + raise CompileError( + f"config file not found: {cfg_path}. Pipeline {pipeline_fn.name!r} {reason}" + ) + raw = yaml.safe_load(cfg_path.read_text()) or {} + if not isinstance(raw, dict): + raise CompileError(f"config at {cfg_path} must be a YAML mapping, got {type(raw).__name__}") + cfg = load_cfg(cfg_path, overrides=dict(overrides), coerce=coerce) + return cfg, raw + + +def _read_raw_cfg(cfg_path: Path) -> dict[str, Any]: + """Read a child's raw config.yaml as a plain mapping (no overrides). + + Used to drive the strict/lenient override resolution in + :func:`_effective_overrides_for_child` — both the broadcast same-name + overlay and the explicit ``.override_config`` typo check key off the keys + the child actually declares. Mirrors the error style of + :func:`_load_cfg_and_raw` (missing file / non-mapping config). + """ + import yaml + + if not cfg_path.exists(): + raise CompileError(f"config file not found: {cfg_path}") + raw = yaml.safe_load(cfg_path.read_text()) or {} + if not isinstance(raw, dict): + raise CompileError(f"config at {cfg_path} must be a YAML mapping, got {type(raw).__name__}") + return raw + + +def _resolve_broadcast_stack( + broadcast_stack: list[BroadcastLayer], + *, + key_filter: set[str] | None, +) -> dict[str, Any]: + """Resolve the active broadcast stack into a single key/value map applying + the two precedence tiers from PROPAGATE_CONFIG_DESIGN §4 (lines 128-159). + + Tiers, lowest -> highest: + + 1. **Broadcast tier** — layers with ``explicit=False`` (a flagged + pipeline's own config). Iterated OUTER -> INNER so the nearest layer + that defines a key wins WITHIN the tier. + 2. **Explicit tier** — layers with ``explicit=True`` (a flagged caller's + per-edge ``.override_config`` that flows deep). Iterated OUTER -> INNER + (nearest-wins) and applied AFTER the entire broadcast tier, so the + explicit tier ALWAYS outranks the broadcast tier regardless of depth. + + ``key_filter`` is the lenient same-name guard: when given (the set of keys + a node actually declares), only those keys are folded in — keys the node + does not declare are skipped. Pass ``None`` to fold in EVERY key present in + the stack (used by the ambient pass-through resolver, which needs the full + resolved context over all keys regardless of what any single node declares). + + This is the single source of the tier logic; both + :func:`_effective_overrides_for_child` and :func:`_resolve_ambient_context` + route through it so their precedence can never diverge. + """ + + def _allowed(key: str) -> bool: + return key_filter is None or key in key_filter + + resolved: dict[str, Any] = {} + # Broadcast tier first (own-config layers), outer -> inner (inner wins). + for layer in broadcast_stack: + if layer.explicit: + continue + for key, value in layer.config.items(): + if _allowed(key): + resolved[key] = value + # Explicit tier on top (per-edge override layers), outer -> inner. Applied + # after the whole broadcast tier so explicit always outranks broadcast. + for layer in broadcast_stack: + if not layer.explicit: + continue + for key, value in layer.config.items(): + if _allowed(key): + resolved[key] = value + return resolved + + +def _effective_overrides_for_child( + *, + child_raw: Mapping[str, Any], + broadcast_stack: list[BroadcastLayer], + explicit: Mapping[str, Any], + child_name: str, + parent_task_id: str, +) -> dict[str, Any]: + """Resolve a child's effective overrides across the two precedence tiers + plus the strict direct edge (PROPAGATE_CONFIG_DESIGN §4, lines 128-159). + + Layered lowest -> highest precedence: + + 1. **Broadcast tier** (lenient, nearest-wins) — flagged ancestors' own + config flowing deep. Applied only for keys the child declares + (``key in child_raw``). + 2. **Explicit tier** (lenient, nearest-wins) — a flagged caller's per-edge + ``.override_config`` that flows deep. Applied AFTER the whole broadcast + tier so it always outranks broadcast, even from a NEARER flagged + descendant. Still lenient (``key in child_raw``). + 3. The **direct edge** ``.override_config`` into THIS child (``explicit`` + param) — STRICT: a key NOT present in the child's own ``config.yaml`` + is a :class:`CompileError` (typo protection); otherwise it overlays, + winning over both tiers. + + Tiers 1-2 are resolved by the shared :func:`_resolve_broadcast_stack` + helper (with the child's declared keys as the lenient ``key_filter``). + """ + declared = set(child_raw) + effective: dict[str, Any] = _resolve_broadcast_stack(broadcast_stack, key_filter=declared) + # 3. Direct-edge explicit overrides — strict, wins over both tiers. + for key, value in explicit.items(): + if key not in child_raw: + raise CompileError( + f"subpipeline task {parent_task_id!r} sets .override_config(" + f"{key}=...) for child pipeline {child_name!r}, but {key!r} is " + f"not a key in that child's config.yaml. Declared child config " + f"keys: {sorted(child_raw)}." + ) + effective[key] = value + return effective + + +def _resolve_ambient_context(broadcast_stack: list[BroadcastLayer]) -> dict[str, Any]: + """Resolve the FULL ambient context map over ALL keys in the active stack. + + Applies the SAME two precedence tiers as + :func:`_effective_overrides_for_child` via the shared + :func:`_resolve_broadcast_stack` helper, but with NO ``key_filter`` — every + key present in the stack is folded in, regardless of what any single node + declares. Used by Fix B to compute the ambient PASS-THROUGH context (§7): + the keys that flow PAST a node, unchanged, to its descendants. + """ + return _resolve_broadcast_stack(broadcast_stack, key_filter=None) + + +# --------------------------------------------------------------------------- +# Command handler + + +class PipelineCompiler(TangleCliHandler): + """Compile a Python-authored pipeline to a dehydrated YAML bundle. + + The object-oriented entry point for the compile command, mirroring + :class:`tangle_cli.pipeline_hydrator.PipelineHydrator`: a + :class:`~tangle_cli.handler.TangleCliHandler` subclass that drives the + module-level :func:`compile_pipeline` free functions and reports the + written artifacts (and any non-fatal warnings) through ``self.log``. + + Compilation is fully offline — it traces the local Python authoring file + and emits YAML — so no Tangle API client is required; the handler base is + still used for its shared logger/``dry_run`` plumbing and to give + downstream distributions a single class to subclass. + + Distributions that carry a zone concept subclass this handler (and extend + the :data:`ZONE_ROOT_MARKERS` seam) so an explicit relative + ``@registered(gen_config=...)`` resolves against a zone root, while the + generic trace/emit/validate/write logic stays here. + """ + + def compile_file( + self, + script: Path, + output: Path, + *, + overrides: Mapping[str, str] | None = None, + pipeline_name: str | None = None, + emit_components_sidecar: bool = True, + ) -> CompileResult: + """Compile ``script`` to a single dehydrated pipeline YAML at ``output``. + + Thin object-oriented wrapper over :func:`compile_pipeline`: it runs the + compile and logs the written artifact paths and any non-fatal warnings + through ``self.log``. See :func:`compile_pipeline` for the full + argument, return, and error contract — in particular it raises + :class:`CompileError` for user-facing problems (missing script, no / + multiple ``@pipeline`` functions, invalid config, unreachable + ``@task`` sources) and :class:`SchemaValidationError` when a compiled + artifact fails dehydrated-schema validation. + """ + result = compile_pipeline( + script, + output, + overrides, + pipeline_name=pipeline_name, + emit_components_sidecar=emit_components_sidecar, + ) + self.log.info(f"wrote {result.pipeline_path}") + if result.components_path is not None: + self.log.info(f"wrote {result.components_path}") + for subgraph_path in result.subgraph_paths: + self.log.info(f"wrote {subgraph_path}") + self.log.info(f"compiled {result.task_count} task(s)") + for warning in result.warnings: + self.log.info(f"warning: {warning}") + return result diff --git a/packages/tangle-cli/src/tangle_cli/pipelines.py b/packages/tangle-cli/src/tangle_cli/pipelines.py index ef04557..15b64b5 100644 --- a/packages/tangle-cli/src/tangle_cli/pipelines.py +++ b/packages/tangle-cli/src/tangle_cli/pipelines.py @@ -13,7 +13,7 @@ from collections import deque from dataclasses import dataclass from pathlib import Path -from typing import Any, Iterable, Mapping +from typing import TYPE_CHECKING, Any, Iterable, Mapping import yaml @@ -28,6 +28,9 @@ ) from .utils import dump_yaml +if TYPE_CHECKING: + from .pipeline_compiler import CompileResult + POSITION_ANNOTATION = "editor.position" __all__ = [ @@ -243,6 +246,49 @@ def hydrate_pipeline_file( ) +# --------------------------------------------------------------------------- +# Compile +# --------------------------------------------------------------------------- + + +def compile_pipeline_file( + script: str | Path, + output: str | Path, + *, + overrides: Mapping[str, str] | None = None, + pipeline_name: str | None = None, + emit_components_sidecar: bool = True, + logger: Any | None = None, +) -> CompileResult: + """Compile a Python-authored pipeline to a dehydrated YAML bundle. + + Instantiates the ported :class:`~tangle_cli.pipeline_compiler.PipelineCompiler` + handler and delegates to its ``compile_file`` method, translating the + compiler's domain errors into :class:`PipelineValidationError` for a uniform + CLI failure contract. Mirrors :func:`hydrate_pipeline_file`. + + The :class:`~tangle_cli.pipeline_compiler.CompileResult` is returned as-is — + unlike hydrate, the compiler already exposes its public result type, so there + is nothing to repackage. + """ + + from .pipeline_compiler import PipelineCompiler + from .python_pipeline.errors import CompileError + from .schema_validation import SchemaValidationError + + compiler = PipelineCompiler(logger=logger) + try: + return compiler.compile_file( + Path(script), + Path(output), + overrides=dict(overrides) if overrides else None, + pipeline_name=pipeline_name, + emit_components_sidecar=emit_components_sidecar, + ) + except (CompileError, SchemaValidationError) as exc: + raise PipelineValidationError(str(exc)) from exc + + # --------------------------------------------------------------------------- # Layout # --------------------------------------------------------------------------- diff --git a/packages/tangle-cli/src/tangle_cli/pipelines_cli.py b/packages/tangle-cli/src/tangle_cli/pipelines_cli.py index 4f545d7..6f5f22b 100644 --- a/packages/tangle-cli/src/tangle_cli/pipelines_cli.py +++ b/packages/tangle-cli/src/tangle_cli/pipelines_cli.py @@ -19,6 +19,7 @@ from .logger import logger_for_log_type from .pipelines import ( PipelineValidationError, + compile_pipeline_file, generate_mermaid, hydrate_pipeline_file, layout_pipeline_file, @@ -125,6 +126,18 @@ def _parse_vars(values: list[str] | dict[str, object] | None) -> dict[str, str]: return parsed +def _parse_overrides(values: list[str] | None) -> dict[str, str]: + parsed: dict[str, str] = {} + for value in values or []: + if "=" not in value: + raise SystemExit("--override entries must use KEY=VALUE syntax") + key, parsed_value = value.split("=", 1) + if not key: + raise SystemExit("--override entries must use KEY=VALUE syntax") + parsed[key] = parsed_value + return parsed + + @app.command(name="hydrate") def pipelines_hydrate( pipeline_path: pathlib.Path, @@ -223,6 +236,66 @@ def pipelines_hydrate( ) +@app.command(name="compile") +def pipelines_compile( + pipeline_path: pathlib.Path, + *, + output: Annotated[ + pathlib.Path, + Parameter( + name="--output", + alias="-o", + help="Output path for the compiled dehydrated pipeline YAML.", + ), + ], + pipeline: Annotated[ + str | None, + Parameter( + name="--pipeline", + help=( + "Select the root @pipeline function by name when the file " + "defines several." + ), + ), + ] = None, + override: Annotated[ + list[str] | None, + Parameter( + name="--override", + help="Compile-time config override as KEY=VALUE. Repeat for multiple.", + negative_iterable=(), + ), + ] = None, + log_type: LogTypeOption = "console", +) -> None: + """Compile a Python-authored pipeline to a dehydrated YAML bundle.""" + + logger, finalize_logs = logger_for_log_type(log_type) + try: + result = compile_pipeline_file( + pipeline_path, + output, + overrides=_parse_overrides(override), + pipeline_name=pipeline, + logger=logger, + ) + except PipelineValidationError as exc: + raise SystemExit(str(exc)) from exc + finally: + finalize_logs() + + print( + f"Compiled {pipeline_path} -> {result.pipeline_path} " + f"({result.task_count} task(s))." + ) + if result.components_path is not None: + print(f"Wrote component sidecar: {result.components_path}") + for subgraph_path in result.subgraph_paths: + print(f"Wrote subgraph: {subgraph_path}") + for warning in result.warnings: + print(f"warning: {warning}") + + @app.command(name="layout") def pipelines_layout( pipeline_path: pathlib.Path, diff --git a/packages/tangle-cli/src/tangle_cli/schema_validation.py b/packages/tangle-cli/src/tangle_cli/schema_validation.py new file mode 100644 index 0000000..5179f2a --- /dev/null +++ b/packages/tangle-cli/src/tangle_cli/schema_validation.py @@ -0,0 +1,460 @@ +"""Standalone validation helpers for the *dehydrated* pipeline schema. + +This module is intentionally decoupled from the hydrator: it only loads +the packaged ``dehydrated_pipeline_schema.json`` and offers pure helpers +the compiler (and tests) can call. It does NOT change any existing +``PipelineHydrator`` behavior. + +Public surface: + +* :func:`load_dehydrated_schema` — load (and cache) the packaged schema. +* :func:`validate_dehydrated_data` — JSON-Schema (Draft 2020-12) + validation, raising :class:`SchemaValidationError` with the single + best/most-specific message. +* :func:`iter_template_delimiters` / :func:`assert_no_template_delimiters` + — generic "no template delimiters" output contract: the compiled + output must contain no ``{{``, ``{%`` or ``{#`` in any string. This is + generic (not SQL/Jinja aware): a compiled dehydrated pipeline carries + final rendered values only. +* :func:`is_dehydrated_pipeline` — shape detector (no raise): top-level + ``name`` + ``implementation.graph.tasks``, no ``template_file``, and + task ``arguments`` values that are raw string constants or ``graphInput`` + / ``taskOutput`` wrappers. +* :func:`validate_dehydrated_pipeline` — JSON-Schema validation PLUS the + deeper semantic checks jsonschema cannot express cleanly (dangling + ``taskOutput.taskId``, undeclared ``graphInput.inputName``, + ``outputValues`` ↔ ``outputs`` correspondence, scalar metadata + annotations, pure componentRefs) and the no-template-delimiter scan. + +Everything here is standalone — it never changes ``PipelineHydrator`` +behavior. ``compile_pipeline`` uses :func:`validate_dehydrated_pipeline` +for its richer pre-write check; an explicit, read-only +``PipelineHydrator.validate_dehydrated_file`` convenience also delegates +here. +""" +from __future__ import annotations + +import json +from collections.abc import Collection, Iterator, Mapping +from functools import lru_cache +from pathlib import Path +from typing import Any + +import jsonschema +from jsonschema.validators import validator_for + +# Template delimiters that must never appear in a compiled dehydrated +# pipeline's strings. The compiled output is the final, rendered form; +# any surviving delimiter means an upstream template was not rendered. +_TEMPLATE_DELIMITERS = ("{{", "{%", "{#") + +_SCHEMA_FILENAME = "dehydrated_pipeline_schema.json" + + +class SchemaValidationError(ValueError): + """Raised when a dehydrated pipeline fails schema/contract validation. + + The compiler wraps this in a ``CompileError`` so the CLI exits 1 with + a friendly message; tests may assert on it directly. + """ + + +def _schema_path() -> Path: + """Locate the packaged dehydrated schema JSON. + + Prefers :mod:`importlib.resources` so it resolves when ``tangle_cli`` + is installed as a wheel (``schemas/*`` is declared package data). Falls + back to a path next to this module for editable / source checkouts. + """ + try: + from importlib.resources import files + + resource = files("tangle_cli") / "schemas" / _SCHEMA_FILENAME + # ``as_file`` would be needed for zip imports, but tangle_cli is + # always installed unzipped; a direct filesystem path is fine here. + candidate = Path(str(resource)) + if candidate.exists(): + return candidate + except (ModuleNotFoundError, FileNotFoundError, TypeError): # pragma: no cover + pass + # Fallback: alongside this module. + return Path(__file__).parent / "schemas" / _SCHEMA_FILENAME + + +@lru_cache(maxsize=1) +def load_dehydrated_schema() -> dict[str, Any]: + """Load and cache the packaged dehydrated pipeline JSON schema.""" + path = _schema_path() + try: + return json.loads(path.read_text()) + except FileNotFoundError as e: # pragma: no cover — packaging error + raise SchemaValidationError( + f"dehydrated pipeline schema not found at {path}" + ) from e + + +def validate_dehydrated_data(data: Mapping[str, Any]) -> None: + """Validate ``data`` against the dehydrated pipeline schema. + + Uses the draft declared in the schema's ``$schema`` (Draft 2020-12) + via :func:`jsonschema.validators.validator_for`. + + Raises: + SchemaValidationError: with the single best/most-specific message + when ``data`` does not conform. + """ + schema = load_dehydrated_schema() + validator_cls = validator_for(schema) + validator_cls.check_schema(schema) + validator = validator_cls(schema) + + errors = sorted(validator.iter_errors(data), key=lambda e: list(e.absolute_path)) + if not errors: + return + + best = jsonschema.exceptions.best_match(errors) + if best is None: # pragma: no cover — errors is non-empty here + best = errors[0] + location = ( + ".".join(str(p) for p in best.absolute_path) + if best.absolute_path + else "root" + ) + raise SchemaValidationError( + f"dehydrated pipeline failed schema validation at {location}: " + f"{best.message}" + ) + + +def iter_template_delimiters(data: Any, _path: str = "") -> Iterator[tuple[str, str]]: + """Yield ``(json_path, delimiter)`` for every string containing a + template delimiter, walking ``data`` recursively. + + Generic scan: keys and values of mappings and items of sequences are + all inspected. The scan is operation-agnostic — it knows nothing + about SQL or Jinja semantics, only the three delimiter tokens. + """ + if isinstance(data, str): + for delim in _TEMPLATE_DELIMITERS: + if delim in data: + yield _path or "root", delim + return + if isinstance(data, Mapping): + for key, value in data.items(): + key_path = f"{_path}.{key}" if _path else str(key) + # Inspect the key itself too — keys are emitted verbatim. + if isinstance(key, str): + for delim in _TEMPLATE_DELIMITERS: + if delim in key: + yield f"{key_path} (key)", delim + yield from iter_template_delimiters(value, key_path) + return + if isinstance(data, (list, tuple)): + for i, item in enumerate(data): + item_path = f"{_path}[{i}]" + yield from iter_template_delimiters(item, item_path) + return + + +def assert_no_template_delimiters( + data: Mapping[str, Any], + exempt_paths: Collection[str] = (), +) -> None: + """Assert the compiled output contains no template delimiters. + + Args: + data: the compiled (dehydrated) pipeline dict to scan. + exempt_paths: JSON paths (in the dot-delimited form + :func:`iter_template_delimiters` yields) whose delimiters are a + legitimate RUNTIME placeholder and must NOT fail the guard — + e.g. a ``run-query`` ``sql_query`` carrying a ``{{input_1}}`` + sentinel the op substitutes at run time, authored via + :func:`tangle_cli.python_pipeline.raw`. An offender at one of + these exact paths is skipped; every OTHER delimiter still fails, + so real compile-time template leaks are still caught. Defaults + to no exemptions, so existing callers are unaffected. + + Raises: + SchemaValidationError: listing the offending locations when any + non-exempt string contains ``{{``, ``{%`` or ``{#``. + """ + allowed = set(exempt_paths) + offenders = [ + (path, delim) + for path, delim in iter_template_delimiters(data) + if path not in allowed + ] + if not offenders: + return + detail = "; ".join(f"{path} contains {delim!r}" for path, delim in offenders) + raise SchemaValidationError( + "compiled pipeline output must contain no template delimiters " + "({{, {% or {#}) — the dehydrated output carries final rendered " + f"values only. Offending location(s): {detail}. Render any " + "templated value in your pipeline code before passing it as a " + "task argument, or wrap a genuine runtime placeholder in " + "tangle_cli.python_pipeline.raw(...) to mark it intentional." + ) + + +# --------------------------------------------------------------------------- +# Phase 5: dehydrated-pipeline shape detection + semantic validation. +# +# These helpers are standalone. They never touch PipelineHydrator behavior. + +# The ONLY top-level keys a dehydrated pipeline may carry. Compile-time +# config is baked into raw string constants and never emitted, so anything +# else at the top level is a leaked config key. +_ALLOWED_TOP_LEVEL_KEYS = frozenset( + {"name", "description", "metadata", "inputs", "outputs", "implementation"} +) + +# The reference-only ArgumentValue wrappers. A constant is NOT a wrapper — +# it is a raw string (matching the runnable Tangle argument contract). +_REFERENCE_ARGUMENT_KEYS = ("graphInput", "taskOutput") + + +def _is_argument_value(value: Any) -> bool: + """True when ``value`` looks like a runnable ArgumentValue — a raw + string constant, or a mapping carrying a ``graphInput`` / ``taskOutput`` + wrapper. + + There is no ambiguity: a raw string constant (even one whose text is + ``"graphInput"`` or JSON like ``'{"graphInput": ...}'``) is a string, + never the object wrapper shapes — so it can never collide with a + ``graphInput`` / ``taskOutput`` mapping. + """ + if isinstance(value, str): + return True + return isinstance(value, Mapping) and any( + key in value for key in _REFERENCE_ARGUMENT_KEYS + ) + + +def is_dehydrated_pipeline(data: Any) -> bool: + """Detect a *dehydrated* pipeline by shape (never raises). + + A dehydrated pipeline has: + + * a top-level ``name`` and ``implementation.graph.tasks`` (non-empty); + * NO ``template_file`` (it is the final rendered form, not a wrapper); + * task ``arguments`` values AND graph ``outputValues`` values (when + present) that are raw string constants or ``graphInput`` / + ``taskOutput`` wrappers — a non-string raw value (a bare + number/list/object) or a legacy ``{constantValue: ...}`` wrapper is + not a runnable argument value, so it means the input is not yet + dehydrated. + + This is intentionally lenient about everything else (it is a detector, + not a validator); use :func:`validate_dehydrated_pipeline` for strict + checking. + """ + if not isinstance(data, Mapping): + return False + if "template_file" in data: + return False + if "name" not in data: + return False + + implementation = data.get("implementation") + if not isinstance(implementation, Mapping): + return False + graph = implementation.get("graph") + if not isinstance(graph, Mapping): + return False + tasks = graph.get("tasks") + if not isinstance(tasks, Mapping) or not tasks: + return False + + for task in tasks.values(): + if not isinstance(task, Mapping): + return False + arguments = task.get("arguments") + if arguments is None: + continue + if not isinstance(arguments, Mapping): + return False + for value in arguments.values(): + if not _is_argument_value(value): + return False + + # Graph outputValues use the SAME runnable argument-value contract, so a + # legacy ``{constantValue: ...}`` (or any non-string raw value) there must + # not pass the detector either. + output_values = graph.get("outputValues") + if output_values is not None: + if not isinstance(output_values, Mapping): + return False + for value in output_values.values(): + if not _is_argument_value(value): + return False + return True + + +def _declared_names(specs: Any) -> set[str]: + """Collect the ``name`` values from an inputs/outputs spec list.""" + names: set[str] = set() + if isinstance(specs, list): + for spec in specs: + if isinstance(spec, Mapping) and isinstance(spec.get("name"), str): + names.add(spec["name"]) + return names + + +def _assert_pure_component_ref(component_ref: Any, loc: str) -> None: + """Assert a componentRef is a PURE ref — no inline ``spec`` / ``text``. + + Redundant with the schema's ``not`` clause, but kept as an explicit, + clearly-messaged semantic check. + """ + if not isinstance(component_ref, Mapping): + return + for forbidden in ("spec", "text"): + if forbidden in component_ref: + raise SchemaValidationError( + f"{loc} must be a pure reference; inline {forbidden!r} is " + "forbidden in a dehydrated pipeline (use url/digest/name)." + ) + + +def _check_argument_refs( + value: Any, + task_ids: set[str], + input_names: set[str], + *, + loc: str, +) -> None: + """Validate the graph references inside one ArgumentValue. + + * ``taskOutput.taskId`` must reference an emitted task id. + * ``graphInput.inputName`` must reference a declared top-level input. + """ + if not isinstance(value, Mapping): + return + task_output = value.get("taskOutput") + if isinstance(task_output, Mapping): + task_id = task_output.get("taskId") + if task_id not in task_ids: + raise SchemaValidationError( + f"{loc}: taskOutput.taskId {task_id!r} does not reference an " + f"emitted task. Known task ids: {sorted(task_ids)}." + ) + graph_input = value.get("graphInput") + if isinstance(graph_input, Mapping): + input_name = graph_input.get("inputName") + if input_name not in input_names: + raise SchemaValidationError( + f"{loc}: graphInput.inputName {input_name!r} does not reference " + f"a declared top-level input. Declared inputs: " + f"{sorted(input_names)}." + ) + + +def _validate_semantics(data: Mapping[str, Any]) -> None: + """Run the dehydrated semantic checks. Assumes ``data`` already passed + :func:`validate_dehydrated_data` (so the structure is well-formed).""" + implementation = data.get("implementation", {}) + graph = implementation.get("graph", {}) if isinstance(implementation, Mapping) else {} + tasks = graph.get("tasks", {}) if isinstance(graph, Mapping) else {} + task_ids = set(tasks.keys()) if isinstance(tasks, Mapping) else set() + + input_names = _declared_names(data.get("inputs")) + output_names = _declared_names(data.get("outputs")) + outputs_present = "outputs" in data + + if isinstance(tasks, Mapping): + for task_id, task in tasks.items(): + if not isinstance(task, Mapping): + continue + _assert_pure_component_ref( + task.get("componentRef"), f"tasks.{task_id}.componentRef" + ) + arguments = task.get("arguments") + if isinstance(arguments, Mapping): + for arg_name, value in arguments.items(): + _check_argument_refs( + value, + task_ids, + input_names, + loc=f"tasks.{task_id}.arguments.{arg_name}", + ) + + output_values = graph.get("outputValues") if isinstance(graph, Mapping) else None + if isinstance(output_values, Mapping): + for out_key, value in output_values.items(): + _check_argument_refs( + value, task_ids, input_names, loc=f"outputValues.{out_key}" + ) + if outputs_present and out_key not in output_names: + raise SchemaValidationError( + f"outputValues key {out_key!r} does not correspond to any " + f"declared top-level output. Declared outputs: " + f"{sorted(output_names)}." + ) + + metadata = data.get("metadata") + if isinstance(metadata, Mapping): + annotations = metadata.get("annotations") + if isinstance(annotations, Mapping): + for key, value in annotations.items(): + # bool is a subclass of int, so it is covered by int. + if not (value is None or isinstance(value, (str, int, float))): + raise SchemaValidationError( + f"metadata.annotations[{key!r}] must be a scalar " + f"(str/number/bool) or null, got " + f"{type(value).__name__!r}." + ) + + +def validate_dehydrated_pipeline( + data: Mapping[str, Any], + exempt_paths: Collection[str] = (), +) -> None: + """Strictly validate a *dehydrated* pipeline dict. + + Runs, in order: + + 1. a top-level guard — reject ``template_file`` and any top-level key + outside the schema-allowed set (leaked compile-time config); + 2. JSON-Schema (Draft 2020-12) structural validation + (:func:`validate_dehydrated_data`); + 3. the no-template-delimiter output contract + (:func:`assert_no_template_delimiters`); + 4. semantic checks: ``taskOutput.taskId`` / ``graphInput.inputName`` + existence, ``outputValues`` ↔ ``outputs`` correspondence, scalar + metadata annotations, and pure componentRefs. + + Args: + data: the dehydrated pipeline dict to validate. + exempt_paths: JSON paths whose template delimiters are legitimate + RUNTIME placeholders (authored via + :func:`tangle_cli.python_pipeline.raw`) and must be skipped + by the no-template-delimiter guard in step 3. Defaults to no + exemptions so existing callers are unaffected. See + :func:`assert_no_template_delimiters`. + + Raises: + SchemaValidationError: with a clear, specific message on the first + violation found. + """ + if not isinstance(data, Mapping): + raise SchemaValidationError( + f"dehydrated pipeline must be a mapping, got {type(data).__name__!r}." + ) + if "template_file" in data: + raise SchemaValidationError( + "dehydrated pipeline must not contain a 'template_file' key — it is " + "the final rendered form, not a Jinja template wrapper." + ) + extra = set(data) - _ALLOWED_TOP_LEVEL_KEYS + if extra: + raise SchemaValidationError( + "dehydrated pipeline has disallowed top-level key(s): " + f"{sorted(extra)}. Allowed top-level keys: " + f"{sorted(_ALLOWED_TOP_LEVEL_KEYS)}. Compile-time config is baked " + "into raw string constants and is never emitted at the top level." + ) + + validate_dehydrated_data(data) + assert_no_template_delimiters(data, exempt_paths) + _validate_semantics(data) diff --git a/packages/tangle-cli/src/tangle_cli/schemas/dehydrated_pipeline_schema.json b/packages/tangle-cli/src/tangle_cli/schemas/dehydrated_pipeline_schema.json new file mode 100644 index 0000000..47fcc8a --- /dev/null +++ b/packages/tangle-cli/src/tangle_cli/schemas/dehydrated_pipeline_schema.json @@ -0,0 +1,254 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://tangleml.com/schemas/tangle/dehydrated_pipeline_schema.json", + "title": "Tangle Dehydrated Pipeline Schema", + "description": "Formal schema for a Tangle pipeline in its *dehydrated* form. A dehydrated pipeline replaces every inline `componentSpec` with a lightweight `componentRef` (by url, digest, or name). Subgraphs are extracted to separate files and referenced via `componentRef.url` (typically `file://...`). Hydrate with `tangle sdk pipelines hydrate` to produce the full inline form.", + "type": "object", + "additionalProperties": false, + "required": ["name", "implementation"], + "properties": { + "name": { + "type": "string", + "minLength": 1, + "description": "Pipeline name." + }, + "description": { + "type": "string", + "description": "Free-form human-readable description." + }, + "metadata": { + "$ref": "#/$defs/Metadata" + }, + "inputs": { + "type": "array", + "items": { "$ref": "#/$defs/InputSpec" }, + "description": "Top-level pipeline inputs." + }, + "outputs": { + "type": "array", + "items": { "$ref": "#/$defs/OutputSpec" }, + "description": "Top-level pipeline outputs." + }, + "implementation": { + "type": "object", + "additionalProperties": false, + "required": ["graph"], + "properties": { + "graph": { "$ref": "#/$defs/GraphSpec" } + } + } + }, + + "$defs": { + "Metadata": { + "type": "object", + "additionalProperties": true, + "properties": { + "annotations": { + "type": "object", + "additionalProperties": { "type": ["string", "number", "boolean", "null"] } + }, + "labels": { + "type": "object", + "additionalProperties": { "type": "string" } + } + } + }, + + "TypeName": { + "description": "Tangle parameter type. Common values include String, Integer, Float, Boolean, JsonObject, JsonArray, GcsPath, BqTable. New types may be added; treat as an open enum.", + "type": "string", + "minLength": 1 + }, + + "InputSpec": { + "type": "object", + "additionalProperties": false, + "required": ["name", "type"], + "properties": { + "name": { "type": "string", "minLength": 1 }, + "type": { "$ref": "#/$defs/TypeName" }, + "description": { "type": "string" }, + "default": {}, + "optional": { "type": "boolean" }, + "annotations": { "type": "object", "additionalProperties": true } + } + }, + + "OutputSpec": { + "type": "object", + "additionalProperties": false, + "required": ["name", "type"], + "properties": { + "name": { "type": "string", "minLength": 1 }, + "type": { "$ref": "#/$defs/TypeName" }, + "description": { "type": "string" }, + "annotations": { "type": "object", "additionalProperties": true } + } + }, + + "GraphSpec": { + "type": "object", + "additionalProperties": false, + "required": ["tasks"], + "properties": { + "tasks": { + "type": "object", + "minProperties": 1, + "propertyNames": { "minLength": 1 }, + "additionalProperties": { "$ref": "#/$defs/TaskSpec" }, + "description": "Map of task-id -> TaskSpec. Task ids are referenced from `taskOutput.taskId`." + }, + "outputValues": { + "type": "object", + "additionalProperties": { "$ref": "#/$defs/ArgumentValue" }, + "description": "Wire graph outputs to task outputs / graph inputs / constants." + } + } + }, + + "TaskSpec": { + "type": "object", + "additionalProperties": false, + "required": ["componentRef"], + "properties": { + "componentRef": { "$ref": "#/$defs/DehydratedComponentRef" }, + "arguments": { + "type": "object", + "additionalProperties": { "$ref": "#/$defs/ArgumentValue" }, + "description": "Map of component input name -> argument source." + }, + "isEnabled": { "type": "boolean" }, + "executionOptions": { "$ref": "#/$defs/ExecutionOptionsSpec" }, + "annotations": { + "type": "object", + "additionalProperties": true, + "description": "Free-form per-task annotations (e.g. `display_name`)." + } + } + }, + + "DehydratedComponentRef": { + "description": "Reference to a component. In a *dehydrated* pipeline this MUST be a pure reference — inline `spec`/`text` are forbidden. At least one of `url`, `digest`, or `name` must be present. `digest` may co-exist with `url`/`name` to pin to a specific version while still preserving the locator.", + "type": "object", + "additionalProperties": false, + "properties": { + "url": { + "type": "string", + "minLength": 1, + "description": "Locator for the component YAML. Supported schemes: `file://`, `http(s)://`, `gs://`, `resolve://`.", + "anyOf": [ + { "pattern": "^file://" }, + { "pattern": "^https?://" }, + { "pattern": "^gs://" }, + { "pattern": "^resolve://" } + ] + }, + "digest": { + "type": "string", + "pattern": "^[A-Fa-f0-9]{64}$", + "description": "SHA-256 hex digest of the published component YAML. Auto-follows `superseded_by` at hydration time." + }, + "name": { + "type": "string", + "minLength": 1, + "description": "Component name as published in the Tangle Component Library. Resolves to the latest matching version at hydration time." + } + }, + "anyOf": [ + { "required": ["url"] }, + { "required": ["digest"] }, + { "required": ["name"] } + ], + "not": { + "anyOf": [ + { "required": ["spec"] }, + { "required": ["text"] } + ] + } + }, + + "ArgumentValue": { + "description": "Source of a task argument or graph output. Each argument has EXACTLY ONE source: a raw string constant, a `graphInput` wrapper, or a `taskOutput` wrapper (mixing sources in one object is rejected). The `graphInput` / `taskOutput` wrappers use the same shapes as runnable Tangle pipelines. The ONLY dehydrated-vs-runnable difference is `componentRef` (a lightweight ref) vs an inline `componentSpec`; argument values are identical to the runnable contract.", + "oneOf": [ + { "type": "string" }, + { "$ref": "#/$defs/GraphInputArgument" }, + { "$ref": "#/$defs/TaskOutputArgument" } + ] + }, + + "GraphInputArgument": { + "type": "object", + "additionalProperties": false, + "required": ["graphInput"], + "properties": { + "graphInput": { "$ref": "#/$defs/GraphInputReference" } + } + }, + + "GraphInputReference": { + "type": "object", + "required": ["inputName"], + "properties": { + "inputName": { "type": "string" }, + "type": { + "anyOf": [ + { "type": "string" }, + { "type": "object", "additionalProperties": true }, + { "type": "array", "items": {} }, + { "type": "null" } + ], + "default": null + } + } + }, + + "TaskOutputArgument": { + "type": "object", + "additionalProperties": false, + "required": ["taskOutput"], + "properties": { + "taskOutput": { "$ref": "#/$defs/TaskOutputReference" } + } + }, + + "TaskOutputReference": { + "type": "object", + "required": ["outputName", "taskId"], + "properties": { + "outputName": { "type": "string" }, + "taskId": { "type": "string" } + } + }, + + "ExecutionOptionsSpec": { + "type": "object", + "additionalProperties": true, + "properties": { + "cachingStrategy": { "$ref": "#/$defs/CachingStrategySpec" }, + "retryStrategy": { "$ref": "#/$defs/RetryStrategySpec" }, + "timeout": { "type": "string", "description": "Go-style duration, e.g. `30m`, `2h`." } + } + }, + + "CachingStrategySpec": { + "type": "object", + "additionalProperties": true, + "properties": { + "maxCacheStaleness": { + "type": ["string", "null"], + "description": "Maximum allowed cache age, e.g. `P7D` (ISO-8601) or `7d`." + } + } + }, + + "RetryStrategySpec": { + "type": "object", + "additionalProperties": true, + "properties": { + "maxRetries": { "type": "integer", "minimum": 0 }, + "backoff": { "type": "string", "description": "Go-style duration, e.g. `30s`." } + } + } + } +} diff --git a/tests/fixtures/python_pipeline/config.yaml b/tests/fixtures/python_pipeline/config.yaml new file mode 100644 index 0000000..20e9ff3 --- /dev/null +++ b/tests/fixtures/python_pipeline/config.yaml @@ -0,0 +1 @@ +foo: bar diff --git a/tests/fixtures/python_pipeline/dup_task_pipeline.py b/tests/fixtures/python_pipeline/dup_task_pipeline.py new file mode 100644 index 0000000..469d462 --- /dev/null +++ b/tests/fixtures/python_pipeline/dup_task_pipeline.py @@ -0,0 +1,14 @@ +"""Two tasks forced to the SAME id via ``.named("Dup")``. + +The emitter walks tasks in trace order and must reject a duplicate task +id explicitly rather than silently dropping the earlier task when the +tasks dict is built. +""" +from tangle_cli.python_pipeline import In, Out, pipeline, ref + + +@pipeline("Dup Task Pipeline") +def dup_task_pipeline(parent_wait_token: In[str], cfg) -> Out[str]: + first = ref(url="file://./noop.yaml").named("Dup")(wait_for=parent_wait_token) + second = ref(url="file://./noop.yaml").named("Dup")(wait_for=parent_wait_token) + return second diff --git a/tests/fixtures/python_pipeline/empty_pipeline.py b/tests/fixtures/python_pipeline/empty_pipeline.py new file mode 100644 index 0000000..97d2d74 --- /dev/null +++ b/tests/fixtures/python_pipeline/empty_pipeline.py @@ -0,0 +1,14 @@ +"""Pipeline whose body calls no ``ref(...)`` — an EMPTY graph. + +The emitter must reject this up front: the dehydrated schema requires at +least one task (``minProperties: 1``). No ``Out[T]`` is declared so the +trace's return check passes and the empty-graph guard in ``emit`` is the +one that fires. +""" +from tangle_cli.python_pipeline import pipeline + + +@pipeline("Empty Pipeline") +def empty_pipeline(cfg): + # No ref(...) calls → zero tasks → CompileError at emit time. + return None diff --git a/tests/fixtures/python_pipeline/multi_arg_pipeline.py b/tests/fixtures/python_pipeline/multi_arg_pipeline.py new file mode 100644 index 0000000..2d295b1 --- /dev/null +++ b/tests/fixtures/python_pipeline/multi_arg_pipeline.py @@ -0,0 +1,37 @@ +"""Multi-argument pipeline fixture exercising every runnable ArgumentValue shape. + +The first task receives several STRING constants (one of them a +``json.dumps`` payload, proving structured data is stringified by the +author, never by tangle-cli), a ``wait_for`` edge bound to an ``In`` +param, and a NON-edge-key argument (``run_mode``) ALSO bound to an ``In`` +param — proving the emitter dispatches purely on the value's type, never +on the argument key. + +The downstream task consumes the upstream task's BARE output via +``depends_on`` and a NAMED output (``produce_data.rows_written``) via a +plain argument key — exercising both ``taskOutput`` shapes and proving +the ``depends_on`` key is preserved verbatim. +""" +import json + +from tangle_cli.python_pipeline import In, Out, pipeline, ref + + +@pipeline("Multi Arg Pipeline") +def multi_arg_pipeline( + parent_wait_token: In[str], + run_mode: In[str], + cfg, +) -> Out[str]: + produce_data = ref(url="file://./noop.yaml")( + a_string="hello world", + a_multiline="line one\nline two\n", + a_json_payload=json.dumps({"mode": "dry_run", "batch_size": 100}), + wait_for=parent_wait_token, + run_mode=run_mode, + ) + consume_data = ref(url="file://./noop.yaml")( + depends_on=produce_data, + rows=produce_data.rows_written, + ) + return consume_data diff --git a/tests/fixtures/python_pipeline/multi_pipeline.py b/tests/fixtures/python_pipeline/multi_pipeline.py new file mode 100644 index 0000000..965c36f --- /dev/null +++ b/tests/fixtures/python_pipeline/multi_pipeline.py @@ -0,0 +1,18 @@ +"""Fixture with TWO ``@pipeline``-decorated functions. + +Used to assert that compile fails cleanly when more than one pipeline is +found in a single file. +""" +from tangle_cli.python_pipeline import In, Out, pipeline, ref + + +@pipeline("First Pipeline") +def first_pipeline(parent_wait_token: In[str], cfg) -> Out[str]: + wait_for_noop = ref(url="file://./noop.yaml")(wait_for=parent_wait_token) + return wait_for_noop + + +@pipeline("Second Pipeline") +def second_pipeline(parent_wait_token: In[str], cfg) -> Out[str]: + wait_for_noop = ref(url="file://./noop.yaml")(wait_for=parent_wait_token) + return wait_for_noop diff --git a/tests/fixtures/python_pipeline/no_pipeline.py b/tests/fixtures/python_pipeline/no_pipeline.py new file mode 100644 index 0000000..96e4905 --- /dev/null +++ b/tests/fixtures/python_pipeline/no_pipeline.py @@ -0,0 +1,8 @@ +"""Fixture with NO ``@pipeline``-decorated function. + +Used to assert that compile fails cleanly when no pipeline is found. +""" +from tangle_cli.python_pipeline import ref + +# A plain ref is not a PipelineFn, so discovery must find zero pipelines. +some_ref = ref(url="file://./noop.yaml") diff --git a/tests/fixtures/python_pipeline/noop.yaml b/tests/fixtures/python_pipeline/noop.yaml new file mode 100644 index 0000000..d0e8567 --- /dev/null +++ b/tests/fixtures/python_pipeline/noop.yaml @@ -0,0 +1,16 @@ +name: Noop +description: Minimal no-op component referenced by compile fixtures via file://./noop.yaml. +inputs: + - name: wait_for + type: String + optional: true +outputs: + - name: wait_for_output + type: String +implementation: + container: + image: python:3.12 + command: + - sh + - -c + - "true" diff --git a/tests/fixtures/python_pipeline/pipeline.py b/tests/fixtures/python_pipeline/pipeline.py new file mode 100644 index 0000000..0aba266 --- /dev/null +++ b/tests/fixtures/python_pipeline/pipeline.py @@ -0,0 +1,13 @@ +"""Minimal Python-authored pipeline fixture for compile tests. + +One task referencing a sibling component YAML by ``file://`` URL, wired +to a single graph input via ``wait_for`` and returned as the pipeline's +``Out[str]`` slot. +""" +from tangle_cli.python_pipeline import In, Out, pipeline, ref + + +@pipeline("Noop Pipeline") +def noop_pipeline(parent_wait_token: In[str], cfg) -> Out[str]: + wait_for_noop = ref(url="file://./noop.yaml")(wait_for=parent_wait_token) + return wait_for_noop diff --git a/tests/fixtures/python_pipeline/task_pipeline.py b/tests/fixtures/python_pipeline/task_pipeline.py new file mode 100644 index 0000000..158c056 --- /dev/null +++ b/tests/fixtures/python_pipeline/task_pipeline.py @@ -0,0 +1,24 @@ +"""Fixture using a ``@task``-decorated component. + +``compile`` emits a sibling ``.components.yaml`` resolver sidecar +with one ``local_from_python`` entry for ``my_task`` and rewrites the +task's componentRef to a pure ``resolve://./.components.yaml#my-task`` +URL. Hydrate regenerates the local component spec from this source file. +""" +from tangle_cli.python_pipeline import In, Out, pipeline, task + + +@task(image="python:3.12") +def my_task(greeting: str = "hello"): + """Write a greeting. + + Metadata: + Name: My Task + """ + print(greeting) + + +@pipeline("Task Pipeline") +def task_pipeline(parent_wait_token: In[str], cfg) -> Out[str]: + run_my_task = my_task(wait_for=parent_wait_token) + return run_my_task diff --git a/tests/test_pipeline_compiler.py b/tests/test_pipeline_compiler.py new file mode 100644 index 0000000..af98b06 --- /dev/null +++ b/tests/test_pipeline_compiler.py @@ -0,0 +1,692 @@ +"""Tests for ``tangle sdk pipelines compile`` (the ported PipelineCompiler). + +Ported from tangle-deploy's ``test_pipeline_compiler.py``. The generic +compile behavior — free functions, dehydrated single-file output, runnable +argument emission, ``@task`` sidecars, and user-facing failure modes — is +exercised directly against ``tangle_cli.pipeline_compiler``. CLI coverage +targets the cyclopts ``compile`` command (``tangle sdk pipelines compile``) +and the ``compile_pipeline_file`` facade rather than tangle-deploy's typer +surface. +""" +import shutil +import sys +from pathlib import Path + +import pytest +import yaml + +from tangle_cli import cli +from tangle_cli.pipeline_compiler import ( + ZONE_ROOT_MARKERS, + CompileResult, + PipelineCompiler, + _parse_overrides, + compile_pipeline, +) +from tangle_cli.pipelines import PipelineValidationError, compile_pipeline_file +from tangle_cli.python_pipeline.errors import CompileError +from tangle_cli.schema_validation import validate_dehydrated_data + +FIXTURES = Path(__file__).parent / "fixtures" / "python_pipeline" + + +def run_app(app, args: list[str]) -> None: + try: + app(args) + except SystemExit as exc: + if exc.code not in (0, None): + raise + + +def _provide_noop(out: Path) -> None: + """Colocate the referenced ``noop.yaml`` component next to the output. + + Fixtures like ``pipeline.py`` / ``multi_arg_pipeline.py`` reference + ``file://./noop.yaml``; the compiler validates that relative local + componentRef targets exist relative to the OUTPUT directory, so the + referenced component must sit next to the compiled YAML. + """ + out.parent.mkdir(parents=True, exist_ok=True) + shutil.copy(FIXTURES / "noop.yaml", out.parent / "noop.yaml") + + +# --------------------------------------------------------------------------- +# Free-function compile: single dehydrated YAML, result contract, sys hygiene. + + +def test_compile_writes_single_dehydrated_yaml(tmp_path): + out = tmp_path / "compiled.yaml" + _provide_noop(out) + compile_pipeline(FIXTURES / "pipeline.py", out) + + # No wrapper/template sidecars are written. The output dir may legitimately + # also contain the referenced component (noop.yaml) we colocated above. + assert out.exists() + assert not out.with_name("compiled.yaml.j2").exists() + assert not list(tmp_path.glob("*.yaml.j2")) + # This (non-@task) pipeline emits NO components sidecar. + assert not (tmp_path / "compiled.components.yaml").exists() + + data = yaml.safe_load(out.read_text()) + # No wrapper config: no ``template_file`` and no copied cfg keys. + assert "template_file" not in data + assert "foo" not in data + # It is a pipeline body. + assert data["name"] == "Noop Pipeline" + assert "implementation" in data + + tasks = data["implementation"]["graph"]["tasks"] + assert "Wait For Noop" in tasks + task_body = tasks["Wait For Noop"] + + # componentRef is a PURE ref — no inline spec / text. + cref = task_body["componentRef"] + assert cref == {"url": "file://./noop.yaml"} + assert "spec" not in cref + assert "text" not in cref + + # wait_for graph input is emitted as a {graphInput: {inputName}} edge. + assert task_body["arguments"]["wait_for"] == { + "graphInput": {"inputName": "parent_wait_token"} + } + + # The Out[str] return wires to outputValues via {taskOutput: {...}}. + assert data["implementation"]["graph"]["outputValues"]["wait_for_output"] == { + "taskOutput": {"taskId": "Wait For Noop", "outputName": "wait_for_output"} + } + + +def test_compile_pipeline_returns_result(tmp_path): + out = tmp_path / "compiled.yaml" + _provide_noop(out) + result = compile_pipeline(FIXTURES / "pipeline.py", out) + assert isinstance(result, CompileResult) + assert result.pipeline_path == out.resolve() + assert result.components_path is None + assert result.task_count == 1 + assert result.warnings == [] + + +def test_compile_creates_missing_output_parent(tmp_path): + """The normal ``--output`` path is created at write time after validation.""" + out = tmp_path / "nested" / "compiled.yaml" + + result = compile_pipeline(FIXTURES / "task_pipeline.py", out) + + assert result.pipeline_path == out.resolve() + assert out.exists() + assert out.with_name("compiled.components.yaml").exists() + + +def test_compile_pipeline_does_not_leak_sys_state(tmp_path): + """compile_pipeline must not leave residue on sys.path / sys.modules.""" + path_before = list(sys.path) + modules_before = set(sys.modules.keys()) + + out = tmp_path / "compiled.yaml" + _provide_noop(out) + compile_pipeline(FIXTURES / "pipeline.py", out) + + # sys.path is byte-for-byte identical → no fixture dir residue. + assert sys.path == path_before + fixtures_dir = str(FIXTURES.resolve()) + if fixtures_dir not in path_before: + assert fixtures_dir not in sys.path + + # No leftover user-module entry in sys.modules. + new_modules = set(sys.modules.keys()) - modules_before + leaked_user_modules = [ + m for m in new_modules if m.startswith("_tangle_user_pipeline_") + ] + assert leaked_user_modules == [] + + +def test_parse_overrides_rejects_missing_equals(): + assert _parse_overrides(["a=1", "b=two"]) == {"a": "1", "b": "two"} + with pytest.raises(CompileError): + _parse_overrides(["no_equals"]) + + +# --------------------------------------------------------------------------- +# Free-function compile: user-facing failure modes. + + +def test_compile_unresolvable_local_ref_fails(tmp_path): + """A relative file:// componentRef whose target is NOT colocated with + the output fails clearly with guidance, and writes no output.""" + out = tmp_path / "compiled.yaml" + # Deliberately do NOT colocate noop.yaml next to the output. + with pytest.raises(CompileError) as exc: + compile_pipeline(FIXTURES / "pipeline.py", out) + msg = str(exc.value) + assert "file://./noop.yaml" in msg + assert "output directory" in msg + # Guidance for the user. + assert "compile into the pipeline source directory" in msg.lower() + # No output written on failure (neither pipeline nor sidecar). + assert not out.exists() + assert not (tmp_path / "compiled.components.yaml").exists() + + +def test_compile_empty_graph_fails(tmp_path): + """A pipeline whose body calls no ref(...) -> CompileError.""" + out = tmp_path / "compiled.yaml" + with pytest.raises(CompileError) as exc: + compile_pipeline(FIXTURES / "empty_pipeline.py", out) + assert "no tasks" in str(exc.value) + assert not out.exists() + + +def test_compile_duplicate_task_id_fails(tmp_path): + """Two tasks forced to the same id via .named('Dup') -> CompileError.""" + out = tmp_path / "compiled.yaml" + with pytest.raises(CompileError) as exc: + compile_pipeline(FIXTURES / "dup_task_pipeline.py", out) + assert "duplicate task id" in str(exc.value) + assert "Dup" in str(exc.value) + assert not out.exists() + + +def test_compile_rejects_config_output_path_collision(tmp_path): + """If ``@pipeline(config=...)`` points at ``--output``, fail clearly. + + ``config=`` is a compile-time input, not the output file to create. The + compiler should not auto-create the output early and then accidentally + read that empty file as config (or read a stale compiled YAML as config). + """ + out = tmp_path / "compiled.yaml" + src = tmp_path / "self_config_pipeline.py" + src.write_text( + "from tangle_cli.python_pipeline import pipeline\n" + "\n" + "@pipeline('Self Config Pipeline', config='compiled.yaml')\n" + "def self_config_pipeline(cfg):\n" + " pass\n" + ) + + with pytest.raises(CompileError) as exc: + compile_pipeline(src, out) + + msg = str(exc.value) + assert "same path as the --output" in msg + assert "compile-time input config file" in msg + assert "creates the output file automatically" in msg + assert "*.compile_config.yaml" in msg + assert not out.exists() + + +def test_compile_without_cfg_param_ignores_missing_explicit_config(tmp_path): + """A stale ``config=`` decorator does not block compile when unused. + + If the function has no ``cfg`` parameter, the config file is not observable + by the pipeline body. Compile succeeds with a warning instead of requiring + a pointless local config file. + """ + out = tmp_path / "compiled.yaml" + _provide_noop(out) + src = tmp_path / "no_cfg_pipeline.py" + src.write_text( + "from tangle_cli.python_pipeline import In, Out, pipeline, ref\n" + "\n" + "@pipeline('No Cfg Pipeline', config='missing_compile_config.yaml')\n" + "def no_cfg_pipeline(parent_wait_token: In[str]) -> Out[str]:\n" + " wait_for_noop = ref(url='file://./noop.yaml')(wait_for=parent_wait_token)\n" + " return wait_for_noop\n" + ) + + result = compile_pipeline(src, out) + + assert out.exists() + assert result.warnings == [ + "pipeline 'No Cfg Pipeline' declares config='missing_compile_config.yaml' " + "but its function has no `cfg` parameter, so the config file was not " + "loaded. Remove `config=` when no compile-time config is needed, or add " + "a `cfg` parameter to use it." + ] + + +def test_compile_missing_config_with_cfg_param_explains_contract(tmp_path): + """A missing config still fails when the pipeline body accepts ``cfg``.""" + out = tmp_path / "compiled.yaml" + src = tmp_path / "needs_cfg_pipeline.py" + src.write_text( + "from tangle_cli.python_pipeline import In, Out, pipeline, ref\n" + "\n" + "@pipeline('Needs Cfg Pipeline', config='missing_compile_config.yaml')\n" + "def needs_cfg_pipeline(parent_wait_token: In[str], cfg) -> Out[str]:\n" + " wait_for_noop = ref(url='file://./noop.yaml')(wait_for=parent_wait_token)\n" + " return wait_for_noop\n" + ) + + with pytest.raises(CompileError) as exc: + compile_pipeline(src, out) + + msg = str(exc.value) + assert "config file not found" in msg + assert "has a `cfg` parameter" in msg + assert "@pipeline(config='missing_compile_config.yaml')" in msg + assert "compile-time input" in msg + assert "remove the `cfg` parameter" in msg + assert not out.exists() + + +def test_compile_rejects_overrides_without_cfg_param(tmp_path): + out = tmp_path / "compiled.yaml" + _provide_noop(out) + src = tmp_path / "no_cfg_pipeline.py" + src.write_text( + "from tangle_cli.python_pipeline import In, Out, pipeline, ref\n" + "\n" + "@pipeline('No Cfg Pipeline')\n" + "def no_cfg_pipeline(parent_wait_token: In[str]) -> Out[str]:\n" + " wait_for_noop = ref(url='file://./noop.yaml')(wait_for=parent_wait_token)\n" + " return wait_for_noop\n" + ) + + with pytest.raises(CompileError) as exc: + compile_pipeline(src, out, overrides={"mode": "dry_run"}) + + msg = str(exc.value) + assert "received --override values ['mode']" in msg + assert "has no `cfg` parameter" in msg + assert not out.exists() + + +@pytest.mark.parametrize( + "literal, type_name", + [ + ("42", "int"), + ("3.14", "float"), + ("True", "bool"), + ("None", "NoneType"), + ("[1, 2, 'three']", "list"), + ("{'mode': 'dry_run'}", "dict"), + ], +) +def test_compile_non_string_constant_rejected(literal, type_name, tmp_path): + """A non-string constant (int/float/bool/None/list/dict) is rejected at + compile with a generic stringify-guidance error, and NO output is + written. The runnable Tangle argument contract is string-only.""" + out = tmp_path / "compiled.yaml" + _provide_noop(out) + # The pipeline is compiled from its own source dir, where the compiler + # loads /config.yaml by default; provide an empty one. + (tmp_path / "config.yaml").write_text("{}\n") + src = tmp_path / "bad_const_pipeline.py" + src.write_text( + "from tangle_cli.python_pipeline import In, Out, pipeline, ref\n" + "\n" + "@pipeline('Bad Const Pipeline')\n" + "def bad_const_pipeline(parent_wait_token: In[str]) -> Out[str]:\n" + f" run_bad = ref(url='file://./noop.yaml')(bad={literal}, " + "wait_for=parent_wait_token)\n" + " return run_bad\n" + ) + with pytest.raises(CompileError) as exc: + compile_pipeline(src, out) + + msg = str(exc.value) + assert "unsupported constant type" in msg + assert type_name in msg + assert "bad" in msg # names the offending argument + # Carries the stringify guidance and the runnable value contract. + assert "json.dumps" in msg + assert "string constants, graphInput, or taskOutput" in msg + # No partial output is written on failure. + assert not out.exists() + + +# --------------------------------------------------------------------------- +# @task sidecar emission. + + +def test_compile_task_decorator_emits_sidecar(tmp_path): + """@task pipelines compile: the main YAML is written and a + ``.components.yaml`` resolver sidecar is emitted alongside it.""" + out = tmp_path / "compiled.yaml" + result = compile_pipeline(FIXTURES / "task_pipeline.py", out) + assert out.exists() + assert result.components_path == out.with_name("compiled.components.yaml") + assert result.components_path.exists() + # Main pipeline validates as dehydrated (resolve:// URL is valid). + validate_dehydrated_data(yaml.safe_load(out.read_text())) + + +# --------------------------------------------------------------------------- +# Runnable argument-value emission (raw string constant / graphInput / +# taskOutput). Dispatch is on the VALUE's type, never the argument KEY. + + +@pytest.fixture(scope="module") +def multi_arg_args(tmp_path_factory): + """Compiled ``arguments`` block of the multi-arg fixture's two tasks.""" + tmp_path = tmp_path_factory.mktemp("multi_arg") + out = tmp_path / "compiled.yaml" + out.parent.mkdir(parents=True, exist_ok=True) + shutil.copy(FIXTURES / "noop.yaml", out.parent / "noop.yaml") + compile_pipeline(FIXTURES / "multi_arg_pipeline.py", out) + data = yaml.safe_load(out.read_text()) + tasks = data["implementation"]["graph"]["tasks"] + return data, tasks["Produce Data"]["arguments"], tasks["Consume Data"]["arguments"] + + +def test_compile_string_constants_emit_raw(multi_arg_args): + """Each string constant emits as a RAW string (the runnable Tangle + argument shape) — no ``constantValue`` wrapper. A ``json.dumps`` + payload is just another string constant (the author stringifies + structured data, never tangle-cli).""" + _data, produce, _consume = multi_arg_args + + assert produce["a_string"] == "hello world" + assert produce["a_multiline"] == "line one\nline two\n" + assert produce["a_json_payload"] == '{"mode": "dry_run", "batch_size": 100}' + + # Every constant is a plain string — no wrapper object anywhere. + for value in ( + produce["a_string"], + produce["a_multiline"], + produce["a_json_payload"], + ): + assert isinstance(value, str) + + +def test_compile_graph_input_is_key_independent(multi_arg_args): + """An ``In`` param emits ``graphInput`` regardless of the argument + key — both the edge key ``wait_for`` and the plain key ``run_mode``.""" + _data, produce, _consume = multi_arg_args + + # Edge key. + assert produce["wait_for"] == {"graphInput": {"inputName": "parent_wait_token"}} + + # NON-edge key bound to an In param ALSO emits graphInput (proves the + # emitter dispatches on the value type, not the argument name). + assert produce["run_mode"] == {"graphInput": {"inputName": "run_mode"}} + + +def test_compile_task_outputs_bare_and_named(multi_arg_args): + """Bare task output -> ``outputName: wait_for_output``; attribute + access -> that named output.""" + _data, _produce, consume = multi_arg_args + + # Bare proxy via ``depends_on`` -> canonical done sentinel. + assert consume["depends_on"] == { + "taskOutput": {"taskId": "Produce Data", "outputName": "wait_for_output"} + } + + # ``produce_data.rows_written`` -> named output. + assert consume["rows"] == { + "taskOutput": {"taskId": "Produce Data", "outputName": "rows_written"} + } + + +def test_compile_depends_on_key_preserved_verbatim(multi_arg_args): + """The literal ``depends_on`` key is preserved as the argument key — + only the value is wrapped.""" + _data, _produce, consume = multi_arg_args + assert "depends_on" in consume + # And it was not rewritten to ``wait_for`` or anything else. + assert "wait_for" not in consume + + +def test_compile_validates_schema(multi_arg_args): + """The compiled output passes packaged-schema validation. (compile + already validates internally; this asserts it explicitly too.)""" + data, _produce, _consume = multi_arg_args + # Should not raise. + validate_dehydrated_data(data) + + +# --------------------------------------------------------------------------- +# PipelineCompiler handler + ZONE_ROOT_MARKERS seam. + + +def test_pipeline_compiler_compile_file_returns_result(tmp_path): + """The object-oriented handler drives the same compile and returns the + module-level CompileResult, mirroring PipelineHydrator.""" + out = tmp_path / "compiled.yaml" + _provide_noop(out) + result = PipelineCompiler().compile_file(FIXTURES / "pipeline.py", out) + assert isinstance(result, CompileResult) + assert result.pipeline_path == out.resolve() + assert result.task_count == 1 + + +def test_zone_root_markers_empty_by_default(): + """OSS ships no zone-root markers; downstream distributions extend the + seam. An empty seam means _find_zone_root always returns None.""" + assert ZONE_ROOT_MARKERS == [] + + +# --------------------------------------------------------------------------- +# compile_pipeline_file facade (pipelines.py). + + +def test_compile_pipeline_file_returns_result(tmp_path): + out = tmp_path / "compiled.yaml" + _provide_noop(out) + result = compile_pipeline_file(FIXTURES / "pipeline.py", out) + assert isinstance(result, CompileResult) + assert result.pipeline_path == out.resolve() + assert result.task_count == 1 + + +def test_compile_pipeline_file_wraps_compile_error(tmp_path): + """The facade translates the compiler's CompileError into the CLI's + uniform PipelineValidationError (mirrors hydrate_pipeline_file).""" + out = tmp_path / "compiled.yaml" + # noop.yaml deliberately not colocated -> unresolvable local ref. + with pytest.raises(PipelineValidationError) as exc: + compile_pipeline_file(FIXTURES / "pipeline.py", out) + assert "file://./noop.yaml" in str(exc.value) + assert not out.exists() + + +# --------------------------------------------------------------------------- +# cyclopts `compile` command (tangle sdk pipelines compile). + + +def test_compile_cli_writes_output(tmp_path, capsys): + out = tmp_path / "compiled.yaml" + _provide_noop(out) + app = cli.build_app() + + run_app( + app, + ["sdk", "pipelines", "compile", str(FIXTURES / "pipeline.py"), "-o", str(out)], + ) + + assert out.exists() + assert yaml.safe_load(out.read_text())["name"] == "Noop Pipeline" + assert "Compiled" in capsys.readouterr().out + + +def test_compile_cli_help_exits_zero(capsys): + app = cli.build_app() + run_app(app, ["sdk", "pipelines", "compile", "--help"]) + assert "compile" in capsys.readouterr().out.lower() + + +def test_compile_cli_missing_script_fails(tmp_path): + out = tmp_path / "compiled.yaml" + app = cli.build_app() + with pytest.raises(SystemExit) as exc_info: + app( + [ + "sdk", + "pipelines", + "compile", + str(tmp_path / "does_not_exist.py"), + "-o", + str(out), + ] + ) + assert exc_info.value.code != 0 + assert "not found" in str(exc_info.value) + assert not out.exists() + + +def test_compile_cli_no_pipeline_fails(tmp_path): + out = tmp_path / "compiled.yaml" + app = cli.build_app() + with pytest.raises(SystemExit) as exc_info: + app( + [ + "sdk", + "pipelines", + "compile", + str(FIXTURES / "no_pipeline.py"), + "-o", + str(out), + ] + ) + assert exc_info.value.code != 0 + assert "no @pipeline" in str(exc_info.value).lower() + assert not out.exists() + + +def test_compile_cli_multiple_pipelines_fails(tmp_path): + """Without --pipeline, a multi-pipeline file errors with a message that + mentions --pipeline and lists each candidate by function and display name.""" + out = tmp_path / "compiled.yaml" + app = cli.build_app() + with pytest.raises(SystemExit) as exc_info: + app( + [ + "sdk", + "pipelines", + "compile", + str(FIXTURES / "multi_pipeline.py"), + "-o", + str(out), + ] + ) + assert exc_info.value.code != 0 + msg = str(exc_info.value) + assert "multiple" in msg.lower() + assert "--pipeline" in msg + assert "first_pipeline" in msg + assert "second_pipeline" in msg + assert "First Pipeline" in msg + assert "Second Pipeline" in msg + assert not out.exists() + + +def test_compile_cli_select_pipeline_by_function_name(tmp_path): + out = tmp_path / "first.yaml" + _provide_noop(out) + app = cli.build_app() + run_app( + app, + [ + "sdk", + "pipelines", + "compile", + str(FIXTURES / "multi_pipeline.py"), + "--pipeline", + "first_pipeline", + "-o", + str(out), + ], + ) + assert yaml.safe_load(out.read_text())["name"] == "First Pipeline" + + +def test_compile_cli_select_pipeline_by_display_name(tmp_path): + out = tmp_path / "second.yaml" + _provide_noop(out) + app = cli.build_app() + run_app( + app, + [ + "sdk", + "pipelines", + "compile", + str(FIXTURES / "multi_pipeline.py"), + "--pipeline", + "Second Pipeline", + "-o", + str(out), + ], + ) + assert yaml.safe_load(out.read_text())["name"] == "Second Pipeline" + + +def test_compile_cli_unknown_pipeline_name_fails(tmp_path): + out = tmp_path / "compiled.yaml" + _provide_noop(out) + app = cli.build_app() + with pytest.raises(SystemExit) as exc_info: + app( + [ + "sdk", + "pipelines", + "compile", + str(FIXTURES / "multi_pipeline.py"), + "--pipeline", + "nope", + "-o", + str(out), + ] + ) + assert exc_info.value.code != 0 + assert "not found" in str(exc_info.value).lower() + assert "first_pipeline" in str(exc_info.value) + assert not out.exists() + + +def test_compile_cli_bad_override_format_fails(tmp_path): + out = tmp_path / "compiled.yaml" + app = cli.build_app() + with pytest.raises(SystemExit) as exc_info: + app( + [ + "sdk", + "pipelines", + "compile", + str(FIXTURES / "pipeline.py"), + "-o", + str(out), + "--override", + "no_equals", + ] + ) + assert exc_info.value.code != 0 + assert "override" in str(exc_info.value).lower() + assert not out.exists() + + +def test_compile_cli_unresolvable_local_ref_exit_nonzero(tmp_path): + out = tmp_path / "compiled.yaml" + app = cli.build_app() + with pytest.raises(SystemExit) as exc_info: + app( + [ + "sdk", + "pipelines", + "compile", + str(FIXTURES / "pipeline.py"), + "-o", + str(out), + ] + ) + assert exc_info.value.code != 0 + assert "noop.yaml" in str(exc_info.value) + assert not out.exists() + + +def test_compile_cli_task_decorator_exit_zero(tmp_path): + out = tmp_path / "compiled.yaml" + app = cli.build_app() + run_app( + app, + [ + "sdk", + "pipelines", + "compile", + str(FIXTURES / "task_pipeline.py"), + "-o", + str(out), + ], + ) + assert out.exists() + assert out.with_name("compiled.components.yaml").exists() diff --git a/tests/test_pipelines_cli.py b/tests/test_pipelines_cli.py index a6b503c..83fe0b8 100644 --- a/tests/test_pipelines_cli.py +++ b/tests/test_pipelines_cli.py @@ -88,8 +88,8 @@ def test_sdk_pipelines_help_lists_local_commands(capsys): assert "hydrate" in output assert "diagram" in output assert "layout" in output + assert "compile" in output assert "pipeline-runs" not in output - assert "compile" not in output def test_pipeline_hydrator_generic_local_resolver_priority(tmp_path: Path): diff --git a/tests/test_schema_validation.py b/tests/test_schema_validation.py new file mode 100644 index 0000000..536a574 --- /dev/null +++ b/tests/test_schema_validation.py @@ -0,0 +1,233 @@ +"""Tests for the ported ``tangle_cli.schema_validation`` module. + +Covers the packaged dehydrated-schema loader, JSON-Schema structural +validation, the no-template-delimiter output contract (including +``exempt_paths``), the dehydrated shape detector, and the semantic checks +layered on top of the schema (dangling ``taskOutput.taskId``, undeclared +``graphInput.inputName``, ``outputValues`` ↔ ``outputs`` correspondence, +and the top-level key / ``template_file`` guards). +""" +import copy + +import pytest + +from tangle_cli.schema_validation import ( + SchemaValidationError, + assert_no_template_delimiters, + is_dehydrated_pipeline, + iter_template_delimiters, + load_dehydrated_schema, + validate_dehydrated_data, + validate_dehydrated_pipeline, +) + + +def _valid_pipeline() -> dict: + """A minimal schema-valid, semantically-consistent dehydrated pipeline.""" + return { + "name": "Demo Pipeline", + "inputs": [{"name": "in1", "type": "String"}], + "outputs": [{"name": "out1", "type": "String"}], + "implementation": { + "graph": { + "tasks": { + "extract": { + "componentRef": {"url": "file://./noop.yaml"}, + "arguments": { + "wait_for": {"graphInput": {"inputName": "in1"}}, + }, + }, + "load": { + "componentRef": {"name": "Load"}, + "arguments": { + "rows": { + "taskOutput": { + "taskId": "extract", + "outputName": "rows", + } + }, + "a_constant": "hello world", + }, + }, + }, + "outputValues": { + "out1": { + "taskOutput": {"taskId": "load", "outputName": "result"} + }, + }, + } + }, + } + + +# --------------------------------------------------------------------------- +# Schema loading + structural validation. + + +def test_load_dehydrated_schema_has_expected_identity(): + schema = load_dehydrated_schema() + assert schema["$schema"] == "https://json-schema.org/draft/2020-12/schema" + assert schema["$id"] == ( + "https://tangleml.com/schemas/tangle/dehydrated_pipeline_schema.json" + ) + # Loader is cached: same object every call. + assert load_dehydrated_schema() is schema + + +def test_validate_dehydrated_data_accepts_valid(): + validate_dehydrated_data(_valid_pipeline()) # should not raise + + +def test_validate_dehydrated_data_rejects_inline_component_spec(): + data = _valid_pipeline() + data["implementation"]["graph"]["tasks"]["extract"]["componentRef"] = { + "spec": {"name": "Extract"} + } + with pytest.raises(SchemaValidationError): + validate_dehydrated_data(data) + + +def test_validate_dehydrated_data_rejects_missing_name(): + data = _valid_pipeline() + del data["name"] + with pytest.raises(SchemaValidationError): + validate_dehydrated_data(data) + + +def test_validate_dehydrated_data_rejects_empty_tasks(): + data = _valid_pipeline() + data["implementation"]["graph"]["tasks"] = {} + with pytest.raises(SchemaValidationError): + validate_dehydrated_data(data) + + +# --------------------------------------------------------------------------- +# Template-delimiter output contract. + + +def test_iter_template_delimiters_finds_offenders(): + data = {"name": "x", "sql": "SELECT {{ value }}"} + found = dict(iter_template_delimiters(data)) + assert "sql" in found + assert found["sql"] == "{{" + + +def test_assert_no_template_delimiters_passes_clean(): + assert_no_template_delimiters(_valid_pipeline()) # should not raise + + +def test_assert_no_template_delimiters_flags_leaked_template(): + data = _valid_pipeline() + data["implementation"]["graph"]["tasks"]["load"]["arguments"][ + "a_constant" + ] = "{{ not_rendered }}" + with pytest.raises(SchemaValidationError) as exc: + assert_no_template_delimiters(data) + assert "{{" in str(exc.value) + + +def test_assert_no_template_delimiters_honors_exempt_paths(): + data = _valid_pipeline() + args = data["implementation"]["graph"]["tasks"]["load"]["arguments"] + args["a_constant"] = "{{input_1}}" + path = "implementation.graph.tasks.load.arguments.a_constant" + # Sanity: the offender is at the path we expect. + assert (path, "{{") in list(iter_template_delimiters(data)) + # Exempting that exact path is accepted; a wrong path is not. + assert_no_template_delimiters(data, exempt_paths=[path]) + with pytest.raises(SchemaValidationError): + assert_no_template_delimiters(data, exempt_paths=["some.other.path"]) + + +# --------------------------------------------------------------------------- +# Shape detection. + + +def test_is_dehydrated_pipeline_true_for_valid(): + assert is_dehydrated_pipeline(_valid_pipeline()) is True + + +def test_is_dehydrated_pipeline_false_for_template_file(): + data = _valid_pipeline() + data["template_file"] = "pipeline.yaml.j2" + assert is_dehydrated_pipeline(data) is False + + +def test_is_dehydrated_pipeline_false_for_non_runnable_argument(): + data = _valid_pipeline() + # A bare int is not a runnable argument value (string / graphInput / + # taskOutput) -> not yet dehydrated. + data["implementation"]["graph"]["tasks"]["load"]["arguments"]["rows"] = 42 + assert is_dehydrated_pipeline(data) is False + + +def test_is_dehydrated_pipeline_false_for_non_mapping(): + assert is_dehydrated_pipeline(["not", "a", "pipeline"]) is False + + +# --------------------------------------------------------------------------- +# Full validate_dehydrated_pipeline: guards + semantics. + + +def test_validate_dehydrated_pipeline_accepts_valid(): + validate_dehydrated_pipeline(_valid_pipeline()) # should not raise + + +def test_validate_dehydrated_pipeline_rejects_template_file(): + data = _valid_pipeline() + data["template_file"] = "pipeline.yaml.j2" + with pytest.raises(SchemaValidationError) as exc: + validate_dehydrated_pipeline(data) + assert "template_file" in str(exc.value) + + +def test_validate_dehydrated_pipeline_rejects_extra_top_level_key(): + data = _valid_pipeline() + data["foo"] = "leaked config" + with pytest.raises(SchemaValidationError) as exc: + validate_dehydrated_pipeline(data) + assert "foo" in str(exc.value) + + +def test_validate_dehydrated_pipeline_rejects_dangling_task_output(): + data = _valid_pipeline() + data["implementation"]["graph"]["tasks"]["load"]["arguments"]["rows"] = { + "taskOutput": {"taskId": "missing", "outputName": "rows"} + } + with pytest.raises(SchemaValidationError) as exc: + validate_dehydrated_pipeline(data) + assert "missing" in str(exc.value) + + +def test_validate_dehydrated_pipeline_rejects_undeclared_graph_input(): + data = _valid_pipeline() + data["implementation"]["graph"]["tasks"]["extract"]["arguments"][ + "wait_for" + ] = {"graphInput": {"inputName": "nope"}} + with pytest.raises(SchemaValidationError) as exc: + validate_dehydrated_pipeline(data) + assert "nope" in str(exc.value) + + +def test_validate_dehydrated_pipeline_rejects_output_value_without_output(): + data = _valid_pipeline() + data["implementation"]["graph"]["outputValues"]["ghost"] = { + "taskOutput": {"taskId": "load", "outputName": "result"} + } + with pytest.raises(SchemaValidationError) as exc: + validate_dehydrated_pipeline(data) + assert "ghost" in str(exc.value) + + +def test_validate_dehydrated_pipeline_rejects_non_scalar_annotation(): + data = _valid_pipeline() + data["metadata"] = {"annotations": {"owner": {"nested": "object"}}} + with pytest.raises(SchemaValidationError): + validate_dehydrated_pipeline(data) + + +def test_validate_dehydrated_pipeline_does_not_mutate_input(): + data = _valid_pipeline() + snapshot = copy.deepcopy(data) + validate_dehydrated_pipeline(data) + assert data == snapshot From 6b71eb63c83623155a81bc475da518580505c31a Mon Sep 17 00:00:00 2001 From: Silin Gupta Date: Tue, 14 Jul 2026 14:12:53 -0400 Subject: [PATCH 2/6] Address review: subpipeline/@registered/config test coverage + drop dead code Follow-up to the PipelineCompiler port, addressing an independent review of the PR: - Add compile tests for the headline features the command is named after: subpipeline child-sidecar emission (parent componentRef rewrite, no subpipeline://pending leak), .override_config precedence on a subpipeline edge, and @registered gen_config resolution (omitted -> nearest ancestor; relative rejected when ZONE_ROOT_MARKERS is empty; relative resolved once a marker is appended to the seam). New hermetic fixtures under tests/fixtures/python_pipeline/. - Remove the dead _parse_overrides from pipeline_compiler.py; the live copy lives in pipelines_cli.py and is covered by the CLI bad-override test. - Strip remaining internal-tool references from user-facing surfaces: the config/output-collision CompileError message, the _fragment_for_task docstring, and the _coerce_override docstring in cfg.py. - Widen the DSL no-internal-references guard to scan the sibling OSS compile modules (component_from_func, pipeline_compiler, schema_validation, pipelines, pipelines_cli), not just the DSL package. Full suite: 745 passed. uv lock --check and git diff --check clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/tangle_cli/pipeline_compiler.py | 21 +-- .../src/tangle_cli/python_pipeline/cfg.py | 4 +- .../registered_zone/gen_config.yaml | 6 + .../registered_zone/registered_op_pipeline.py | 28 ++++ .../registered_op_relative_pipeline.py | 26 +++ .../registered_zone/zone_root.marker | 1 + .../subpipeline_child_config.yaml | 1 + .../subpipeline_config_pipeline.py | 34 ++++ .../python_pipeline/subpipeline_pipeline.py | 31 ++++ tests/test_pipeline_compiler.py | 150 ++++++++++++++++-- tests/test_python_pipeline_dsl.py | 20 ++- 11 files changed, 282 insertions(+), 40 deletions(-) create mode 100644 tests/fixtures/python_pipeline/registered_zone/gen_config.yaml create mode 100644 tests/fixtures/python_pipeline/registered_zone/registered_op_pipeline.py create mode 100644 tests/fixtures/python_pipeline/registered_zone/registered_op_relative_pipeline.py create mode 100644 tests/fixtures/python_pipeline/registered_zone/zone_root.marker create mode 100644 tests/fixtures/python_pipeline/subpipeline_child_config.yaml create mode 100644 tests/fixtures/python_pipeline/subpipeline_config_pipeline.py create mode 100644 tests/fixtures/python_pipeline/subpipeline_pipeline.py diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py index 06e78c7..eb9babb 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py @@ -1024,8 +1024,8 @@ def _artifact_label(artifact: SubgraphArtifact) -> str: def _fragment_for_task(ref: CallableRef) -> str: """Stable fragment key for a @task ref in the components sidecar. - Uses the hyphenated function name, matching tangle-deploy's own - ``_resolve_local_from_python`` output-filename convention + Uses the hyphenated function name, matching the ``local_from_python`` + resolver's output-filename convention (``my_task`` -> ``my-task``). Multiple call sites for the SAME function share one fragment — the local_from_python entry is keyed by source file, not by call site. @@ -1594,21 +1594,6 @@ def _validate_local_component_refs_for_artifact( # Orchestration helpers -def _parse_overrides(entries: list[str]) -> dict[str, str]: - """Parse ``--override key=value`` strings into a dict. - - Raises: - CompileError: if an entry has no ``=`` separator. - """ - overrides: dict[str, str] = {} - for entry in entries: - if "=" not in entry: - raise CompileError(f"--override expects key=value, got {entry!r} (no '=' separator)") - key, _, value = entry.partition("=") - overrides[key] = value - return overrides - - def _evict_shadowed_bundle_modules(bundle_dir: Path) -> None: """Evict ``sys.modules`` entries that shadow a module file in ``bundle_dir``. @@ -1981,7 +1966,7 @@ def _assert_config_output_path_is_separate( f"@pipeline config for {pipeline_fn.name!r} resolves to the same path as " f"the {output_label}: {cfg_path}. The `config=` argument names a " "compile-time input config file; it is read before the compiled YAML " - "is written. tangle-deploy creates the output file automatically after " + "is written. The compiler creates the output file automatically after " "validation, so do not point `config=` at the output. Use a separate " "config file (for example an empty `*.compile_config.yaml`) or omit " "`config=` to use `config.yaml`, and keep `--output` for the compiled YAML." diff --git a/packages/tangle-cli/src/tangle_cli/python_pipeline/cfg.py b/packages/tangle-cli/src/tangle_cli/python_pipeline/cfg.py index 63373fd..b44a334 100644 --- a/packages/tangle-cli/src/tangle_cli/python_pipeline/cfg.py +++ b/packages/tangle-cli/src/tangle_cli/python_pipeline/cfg.py @@ -43,8 +43,8 @@ def _coerce_override(value: str) -> Any: """Type a single ``--override key=value`` *value* string. - Override values arrive from the CLI as raw strings (see - ``pipeline_compiler._parse_overrides``). To give an override the SAME + Override values arrive from the CLI as raw strings (parsed from + ``--override key=value`` by the ``pipelines`` CLI). To give an override the SAME Python type the equivalent ``key: value`` line in ``config.yaml`` would (which is already typed via ``yaml.safe_load``), we run each value through the same loader. diff --git a/tests/fixtures/python_pipeline/registered_zone/gen_config.yaml b/tests/fixtures/python_pipeline/registered_zone/gen_config.yaml new file mode 100644 index 0000000..4ffbe24 --- /dev/null +++ b/tests/fixtures/python_pipeline/registered_zone/gen_config.yaml @@ -0,0 +1,6 @@ +# Minimal gen_config.yaml for @registered compile fixtures. The compiler +# existence-checks this file and emits a resolve:// URL pointing at it; the +# #fragment is not validated at compile time (surfaces at hydrate time). +components: + run-query: + description: Placeholder component fragment referenced by the @registered op. diff --git a/tests/fixtures/python_pipeline/registered_zone/registered_op_pipeline.py b/tests/fixtures/python_pipeline/registered_zone/registered_op_pipeline.py new file mode 100644 index 0000000..887e4a7 --- /dev/null +++ b/tests/fixtures/python_pipeline/registered_zone/registered_op_pipeline.py @@ -0,0 +1,28 @@ +"""Fixture exercising ``@registered`` gen_config resolution. + +``@registered`` references an EXISTING ``gen_config.yaml`` (generating no +sidecar of its own). At compile the task's ``componentRef`` is rewritten +from the ``registered://pending`` sentinel to a pure +``resolve:///gen_config.yaml#`` URL, relative to the output +dir. With ``gen_config`` omitted, resolution falls back to the nearest +ancestor ``gen_config.yaml`` (this directory's) — the default OSS path, +requiring no zone-root marker. +""" +from tangle_cli.python_pipeline import In, Out, pipeline, registered + + +@registered(fragment="run-query") +def run_query(sql_query: str = "SELECT 1") -> str: + """Run a query. + + Metadata: + Name: Run Query + Version: 1.0.0 + """ + ... + + +@pipeline("Registered Pipeline") +def registered_pipeline(parent_wait_token: In[str]) -> Out[str]: + run = run_query.named("Run Query")(wait_for=parent_wait_token) + return run diff --git a/tests/fixtures/python_pipeline/registered_zone/registered_op_relative_pipeline.py b/tests/fixtures/python_pipeline/registered_zone/registered_op_relative_pipeline.py new file mode 100644 index 0000000..a7dc788 --- /dev/null +++ b/tests/fixtures/python_pipeline/registered_zone/registered_op_relative_pipeline.py @@ -0,0 +1,26 @@ +"""Fixture exercising ``@registered`` with an EXPLICIT RELATIVE gen_config. + +A relative ``gen_config`` is resolved against the nearest ancestor holding a +registered ``ZONE_ROOT_MARKERS`` marker (the "zone root"). In the default +open-source build ``ZONE_ROOT_MARKERS`` is empty, so a relative path is +REJECTED with an actionable error; a downstream distribution that carries a +zone concept appends its marker filename to restore resolution. +""" +from tangle_cli.python_pipeline import In, Out, pipeline, registered + + +@registered(fragment="run-query", gen_config="gen_config.yaml") +def run_query_rel(sql_query: str = "SELECT 1") -> str: + """Run a query. + + Metadata: + Name: Run Query + Version: 1.0.0 + """ + ... + + +@pipeline("Registered Relative Pipeline") +def registered_relative_pipeline(parent_wait_token: In[str]) -> Out[str]: + run = run_query_rel.named("Run Query")(wait_for=parent_wait_token) + return run diff --git a/tests/fixtures/python_pipeline/registered_zone/zone_root.marker b/tests/fixtures/python_pipeline/registered_zone/zone_root.marker new file mode 100644 index 0000000..1e37ca2 --- /dev/null +++ b/tests/fixtures/python_pipeline/registered_zone/zone_root.marker @@ -0,0 +1 @@ +zone-root marker for @registered relative-gen_config compile fixtures diff --git a/tests/fixtures/python_pipeline/subpipeline_child_config.yaml b/tests/fixtures/python_pipeline/subpipeline_child_config.yaml new file mode 100644 index 0000000..aa18e78 --- /dev/null +++ b/tests/fixtures/python_pipeline/subpipeline_child_config.yaml @@ -0,0 +1 @@ +message: from-file diff --git a/tests/fixtures/python_pipeline/subpipeline_config_pipeline.py b/tests/fixtures/python_pipeline/subpipeline_config_pipeline.py new file mode 100644 index 0000000..3776b67 --- /dev/null +++ b/tests/fixtures/python_pipeline/subpipeline_config_pipeline.py @@ -0,0 +1,34 @@ +"""Fixture exercising compile-time ``cfg`` override on a subpipeline edge. + +The child consumes ``cfg.message`` and emits it as a task-argument constant. +``.override_config(message=...)`` on the subpipeline edge sets the child's +COMPILE-TIME ``cfg`` value (a distinct namespace from ``.bind`` runtime +inputs), and the overridden value wins over the child's own ``config.yaml``. +The override is validated STRICT against the child's config keys. +""" +from tangle_cli.python_pipeline import In, Out, pipeline, subpipeline, task + + +@task(image="python:3.12") +def emit_task(message: str = "default"): + """Emit a message constant. + + Metadata: + Name: Emit Task + """ + print(message) + + +@pipeline("Config Child", config="subpipeline_child_config.yaml") +def config_child(seed: In[str], cfg) -> Out[str]: + run = emit_task(message=cfg.message, wait_for=seed) + return run + + +@pipeline("Config Parent") +def config_parent(parent_wait_token: In[str], cfg) -> Out[str]: + return ( + subpipeline(config_child) + .named("Run Child") + .override_config(message="from-override")(seed=parent_wait_token) + ) diff --git a/tests/fixtures/python_pipeline/subpipeline_pipeline.py b/tests/fixtures/python_pipeline/subpipeline_pipeline.py new file mode 100644 index 0000000..5535be4 --- /dev/null +++ b/tests/fixtures/python_pipeline/subpipeline_pipeline.py @@ -0,0 +1,31 @@ +"""Fixture exercising subpipeline composition. + +The parent pipeline embeds ``child_pipeline`` as a subpipeline. ``compile`` +emits the child as its own sidecar under ``.subgraphs/`` and rewrites +the parent task's ``componentRef.url`` from the ``subpipeline://pending`` +sentinel to a ``file://`` reference at that child sidecar. The child uses a +``@task`` so its own component sidecar is a hermetically-generated file (no +external relative componentRef to colocate). +""" +from tangle_cli.python_pipeline import In, Out, pipeline, subpipeline, task + + +@task(image="python:3.12") +def child_task(greeting: str = "hi"): + """Write a greeting. + + Metadata: + Name: Child Task + """ + print(greeting) + + +@pipeline("Child Pipeline") +def child_pipeline(seed: In[str], cfg) -> Out[str]: + run_child_task = child_task(wait_for=seed) + return run_child_task + + +@pipeline("Parent Pipeline") +def parent_pipeline(parent_wait_token: In[str], cfg) -> Out[str]: + return subpipeline(child_pipeline).named("Run Child")(seed=parent_wait_token) diff --git a/tests/test_pipeline_compiler.py b/tests/test_pipeline_compiler.py index af98b06..3113bbc 100644 --- a/tests/test_pipeline_compiler.py +++ b/tests/test_pipeline_compiler.py @@ -1,12 +1,11 @@ -"""Tests for ``tangle sdk pipelines compile`` (the ported PipelineCompiler). - -Ported from tangle-deploy's ``test_pipeline_compiler.py``. The generic -compile behavior — free functions, dehydrated single-file output, runnable -argument emission, ``@task`` sidecars, and user-facing failure modes — is -exercised directly against ``tangle_cli.pipeline_compiler``. CLI coverage -targets the cyclopts ``compile`` command (``tangle sdk pipelines compile``) -and the ``compile_pipeline_file`` facade rather than tangle-deploy's typer -surface. +"""Tests for ``tangle sdk pipelines compile`` (the PipelineCompiler). + +The generic compile behavior — free functions, dehydrated single-file output, +runnable argument emission, ``@task`` sidecars, subpipeline child sidecars, +``@registered`` resolution, and user-facing failure modes — is exercised +directly against ``tangle_cli.pipeline_compiler``. CLI coverage targets the +cyclopts ``compile`` command (``tangle sdk pipelines compile``) and the +``compile_pipeline_file`` facade. """ import shutil import sys @@ -20,7 +19,6 @@ ZONE_ROOT_MARKERS, CompileResult, PipelineCompiler, - _parse_overrides, compile_pipeline, ) from tangle_cli.pipelines import PipelineValidationError, compile_pipeline_file @@ -141,12 +139,6 @@ def test_compile_pipeline_does_not_leak_sys_state(tmp_path): assert leaked_user_modules == [] -def test_parse_overrides_rejects_missing_equals(): - assert _parse_overrides(["a=1", "b=two"]) == {"a": "1", "b": "two"} - with pytest.raises(CompileError): - _parse_overrides(["no_equals"]) - - # --------------------------------------------------------------------------- # Free-function compile: user-facing failure modes. @@ -690,3 +682,129 @@ def test_compile_cli_task_decorator_exit_zero(tmp_path): ) assert out.exists() assert out.with_name("compiled.components.yaml").exists() + + +# --------------------------------------------------------------------------- +# Subpipeline composition: a child @pipeline embedded via subpipeline(...) is +# emitted as its own sidecar under .subgraphs/ and the parent task's +# componentRef is rewritten from the subpipeline://pending sentinel to a +# file:// reference at that child. + + +def test_compile_subpipeline_emits_child_sidecar(tmp_path): + """A subpipeline child compiles to a sidecar under ``.subgraphs/`` + and the parent task's componentRef is rewritten to a ``file://`` URL at + that child — the ``subpipeline://pending`` sentinel never survives.""" + out = tmp_path / "compiled.yaml" + result = compile_pipeline( + FIXTURES / "subpipeline_pipeline.py", out, pipeline_name="Parent Pipeline" + ) + + # Exactly one child sidecar, named after the child pipeline (content-hash + # suffix), living under the parent's .subgraphs/ directory. + assert len(result.subgraph_paths) == 1 + child = result.subgraph_paths[0] + assert child.exists() + assert child.parent.name == "compiled.subgraphs" + assert child.name.startswith("child-pipeline-") + assert child.name.endswith(".yaml") + + # Parent has a single task whose componentRef points at the child sidecar. + parent = yaml.safe_load(out.read_text()) + tasks = parent["implementation"]["graph"]["tasks"] + assert list(tasks) == ["Run Child"] + assert ( + tasks["Run Child"]["componentRef"]["url"] + == f"file://./compiled.subgraphs/{child.name}" + ) + assert "subpipeline://pending" not in out.read_text() + + # Both parent and child are valid dehydrated pipelines. + validate_dehydrated_data(parent) + validate_dehydrated_data(yaml.safe_load(child.read_text())) + + +def test_compile_subpipeline_override_config_wins(tmp_path): + """``.override_config`` on a subpipeline edge sets the child's COMPILE-TIME + cfg, and the overridden value wins over the child's own config.yaml. The + child emits ``cfg.message`` as a task-argument constant, so the override is + observable in the child sidecar (the value can only come from the edge + override: it is neither the ``@task`` default nor the config.yaml value).""" + out = tmp_path / "compiled.yaml" + result = compile_pipeline( + FIXTURES / "subpipeline_config_pipeline.py", out, pipeline_name="Config Parent" + ) + child = yaml.safe_load(result.subgraph_paths[0].read_text()) + tasks = child["implementation"]["graph"]["tasks"] + (task,) = tasks.values() + assert task["arguments"]["message"] == "from-override" + + +# --------------------------------------------------------------------------- +# @registered gen_config resolution. @registered references an EXISTING +# gen_config.yaml and generates no sidecar of its own; the task's componentRef +# is rewritten from the registered://pending sentinel to a resolve:// URL. + +_REGISTERED_ZONE = FIXTURES / "registered_zone" + + +def test_compile_registered_omitted_gen_config_uses_nearest(tmp_path): + """With ``gen_config`` omitted, resolution falls back to the nearest + ancestor ``gen_config.yaml`` — the default OSS path, needing no zone-root + marker. The emitted URL is a ``resolve://`` reference at that file with the + verbatim ``#fragment``, and @registered generates no sidecar.""" + out = tmp_path / "compiled.yaml" + result = compile_pipeline( + _REGISTERED_ZONE / "registered_op_pipeline.py", + out, + pipeline_name="Registered Pipeline", + ) + assert result.components_path is None + assert result.subgraph_paths == [] + + data = yaml.safe_load(out.read_text()) + (task,) = data["implementation"]["graph"]["tasks"].values() + url = task["componentRef"]["url"] + # The relpath from the tmp output dir to the fixture is environment + # dependent; assert the scheme and the target/fragment tail only. + assert url.startswith("resolve://") + assert url.endswith("registered_zone/gen_config.yaml#run-query") + assert "registered://pending" not in out.read_text() + + +def test_compile_registered_relative_gen_config_rejected_without_marker(tmp_path): + """A relative ``gen_config`` is rejected in the default OSS build because + ``ZONE_ROOT_MARKERS`` is empty, so no zone root can be located.""" + assert ZONE_ROOT_MARKERS == [] # guard: no marker leaked in from another test + with pytest.raises(CompileError) as exc_info: + compile_pipeline( + _REGISTERED_ZONE / "registered_op_relative_pipeline.py", + tmp_path / "compiled.yaml", + pipeline_name="Registered Relative Pipeline", + ) + assert "zone-root markers" in str(exc_info.value) + + +def test_compile_registered_relative_gen_config_resolved_with_marker(tmp_path): + """A downstream distribution appends its marker filename to the + ``ZONE_ROOT_MARKERS`` seam (mutating the list in place) to restore + zone-root resolution; a relative ``gen_config`` then resolves against the + nearest ancestor holding that marker.""" + ZONE_ROOT_MARKERS.append("zone_root.marker") + try: + out = tmp_path / "compiled.yaml" + compile_pipeline( + _REGISTERED_ZONE / "registered_op_relative_pipeline.py", + out, + pipeline_name="Registered Relative Pipeline", + ) + data = yaml.safe_load(out.read_text()) + (task,) = data["implementation"]["graph"]["tasks"].values() + url = task["componentRef"]["url"] + assert url.startswith("resolve://") + assert url.endswith("registered_zone/gen_config.yaml#run-query") + finally: + # Restore the empty OSS default so ordering never leaks a marker into + # test_compile_registered_relative_gen_config_rejected_without_marker + # or test_zone_root_markers_empty_by_default. + ZONE_ROOT_MARKERS.remove("zone_root.marker") diff --git a/tests/test_python_pipeline_dsl.py b/tests/test_python_pipeline_dsl.py index 5867f80..7988877 100644 --- a/tests/test_python_pipeline_dsl.py +++ b/tests/test_python_pipeline_dsl.py @@ -106,11 +106,23 @@ class TestNoInternalReferences: ``register_authoring_import_module`` — OSS never names it.""" def test_no_tangle_deploy_references_in_source(self): - # Scan the whole vendored DSL package plus the sibling compiler module - # (``component_from_func.py``), which drives the authoring-import strip - # and must stay decoupled from the downstream package name. + # Scan the whole vendored DSL package plus the sibling modules that make + # up the OSS compile surface: the authoring-import strip driver + # (``component_from_func.py``) and the ported compiler + its schema + # validator + the ``pipelines`` facade/CLI. All must stay decoupled from + # the downstream package name (a leaked ref here would surface in a + # user-facing CompileError, as one did before this guard was widened). scanned = list(_DSL_DIR.glob("*.py")) - scanned.append(_DSL_DIR.parent / "component_from_func.py") + scanned += [ + _DSL_DIR.parent / name + for name in ( + "component_from_func.py", + "pipeline_compiler.py", + "schema_validation.py", + "pipelines.py", + "pipelines_cli.py", + ) + ] offenders = [] for path in scanned: text = path.read_text(encoding="utf-8") From 5214e01879005214cc3e24c5ccbb2b368bdd9018 Mon Sep 17 00:00:00 2001 From: Silin Gupta Date: Tue, 14 Jul 2026 14:30:27 -0400 Subject: [PATCH 3/6] Remove dead cfg-path wrapper + scrub internal product names from OSS surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second independent review of #28 surfaced two verified findings; fixing both here per request: 1. Remove the dead `_resolve_cfg_path(pipeline_fn, module_path)` wrapper in pipeline_compiler.py — a thin, zero-caller shim over `_resolve_cfg_path_in_dir` (the actual resolver, retained). 2. Scrub internal product/infra names from OSS docstrings + examples so the open-source surface never names Oasis / UPI / Comet / areas-ml image paths: - task_env.py: example env `UPI`/`areas-ml-upi:main`/`publish_to_comet`/ `[UPI Clustering]`/"Oasis runner" -> generic `TRAINING`/`python:3.12`/ `train_model`/"downstream runner". - registered.py: relative-path resolution doc referenced the internal `oasis.pipeline_component_root.yaml` marker -> now points at the generic `ZONE_ROOT_MARKERS` seam and states the OSS build registers none by default. - component_from_func.py: 31 example `UPI` env-binding identifiers in the authoring-strip docstrings -> `TRAINING`. - trace.py: CompileError example `return publish_to_comet` -> `return train_model`. Widen `TestNoInternalReferences` to enforce the scrub and prevent regression: alongside the existing tangle_deploy/tangle-deploy substring check it now flags `areas-ml`/`areas/ml` substrings and the standalone words oasis/upi/comet (word-boundary, case-insensitive — so it won't false-positive on "deduping"). 745 passed; `uv lock --check` and `git diff --check` clean; no new ruff findings in touched files. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/tangle_cli/component_from_func.py | 44 +++++++++---------- .../src/tangle_cli/pipeline_compiler.py | 12 ----- .../tangle_cli/python_pipeline/registered.py | 6 ++- .../tangle_cli/python_pipeline/task_env.py | 14 +++--- .../src/tangle_cli/python_pipeline/trace.py | 2 +- tests/test_python_pipeline_dsl.py | 34 ++++++++++---- 6 files changed, 59 insertions(+), 53 deletions(-) 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 f267323..085a2cf 100644 --- a/packages/tangle-cli/src/tangle_cli/component_from_func.py +++ b/packages/tangle-cli/src/tangle_cli/component_from_func.py @@ -785,11 +785,11 @@ class AuthoringStripError(ValueError): authoring alias must be stripped, but line-deletion would take ``os`` with it); or - a ``@task(env=...)`` env binding entangled with runtime code — e.g. a - mixed ``from _envs import UPI, helper`` import whose ``helper`` is used at + mixed ``from _envs import TRAINING, helper`` import whose ``helper`` is used at runtime, or a collected env name referenced by the kept task body. Failing fast here is intentional: silently baking a broken ``import os`` / - ``from _envs import UPI`` / ``UPI = TaskEnv(...)`` would only surface as a + ``from _envs import TRAINING`` / ``TRAINING = TaskEnv(...)`` would only surface as a ``NameError`` / ``ImportError`` at container start. The message tells the author how to split the import or keep TaskEnv values authoring-only. """ @@ -889,9 +889,9 @@ def _env_keyword_binding_name(call: ast.Call) -> str | None: returns the name of the module-level binding that must also be stripped so the baked runtime program does not crash referencing an authoring-only name: - - ``env=UPI`` -> ``"UPI"`` (a module-level env *binding* to strip, either an - ``UPI = TaskEnv(...)`` assignment or a ``from _envs import UPI`` import); - - ``env=_envs.UPI`` -> ``"_envs"`` (the module-alias root, so the + - ``env=TRAINING`` -> ``"TRAINING"`` (a module-level env *binding* to strip, either an + ``TRAINING = TaskEnv(...)`` assignment or a ``from _envs import TRAINING`` import); + - ``env=_envs.TRAINING`` -> ``"_envs"`` (the module-alias root, so the ``import _envs`` line can be stripped); - ``env=TaskEnv(...)`` / ``env=tp.TaskEnv(...)`` (inline) -> ``None``: the whole decorator line range is already deleted, so there is no residual @@ -917,8 +917,8 @@ def _is_task_env_construction(value: ast.expr | None) -> bool: Matched by trailing call name (mirroring ``_decorator_called_name``), so both ``TaskEnv(image=...)`` and ``tp.TaskEnv(image=...)`` qualify. Used to - detect module-level env declarations like ``UPI = TaskEnv(...)`` regardless - of whether a ``@task(env=UPI)`` references them. + detect module-level env declarations like ``TRAINING = TaskEnv(...)`` regardless + of whether a ``@task(env=TRAINING)`` references them. """ return isinstance(value, ast.Call) and _decorator_called_name(value) == _AUTHORING_ENV_CLASS_NAME @@ -926,8 +926,8 @@ def _is_task_env_construction(value: ast.expr | None) -> bool: def _import_bound_names(node: ast.Import | ast.ImportFrom) -> dict[str, ast.alias]: """Map each name a top-level import binds into the namespace to its alias. - - ``from m import UPI`` -> ``{"UPI": alias}`` - - ``from m import UPI as U`` -> ``{"U": alias}`` + - ``from m import TRAINING`` -> ``{"TRAINING": alias}`` + - ``from m import TRAINING as U`` -> ``{"U": alias}`` - ``import _envs`` -> ``{"_envs": alias}`` (root of a dotted module path) - ``import a.b.c`` -> ``{"a": alias}`` - ``import envs as task_envs`` -> ``{"task_envs": alias}`` @@ -951,7 +951,7 @@ def _annotation_name_node_ids(tree: ast.AST) -> set[int]: (which runs AFTER ``_strip_authoring_constructs``), so a name that appears ONLY in an annotation is NOT a live runtime reference. Excluding these from the fail-fast reference scan prevents a false positive where an env name - used only as a parameter/return type annotation (``def f(x: UPI) -> UPI:``) + used only as a parameter/return type annotation (``def f(x: TRAINING) -> TRAINING:``) is mistaken for a kept runtime reference (FIX N1, §3.5). Annotation slots covered (matching ``_strip_type_hints_ast``): @@ -1029,17 +1029,17 @@ def _strip_authoring_constructs(source_code: str) -> str: TaskEnv authoring-strip hardening (``@task(env=...)``): an env declaration that exists ONLY to feed a stripped ``@task(env=...)`` decorator would otherwise crash the baked program (``NameError: TaskEnv`` for a - co-located ``UPI = TaskEnv(...)`` whose import was stripped, or - ``ImportError`` for a ``from _envs import UPI`` whose module is not in the + co-located ``TRAINING = TaskEnv(...)`` whose import was stripped, or + ``ImportError`` for a ``from _envs import TRAINING`` whose module is not in the runtime image). On top of the import/decorator strip this also removes, by line range: - every module-level ``X = TaskEnv(...)`` / ``X: TaskEnv = TaskEnv(...)`` declaration (direct ``TaskEnv(...)`` construction), and - module-level bindings (assignment OR import) of any name a stripped - ``@task(env=...)`` referenced — ``env=UPI`` collects ``UPI`` - (``UPI = TaskEnv(...)`` / ``UPI = make_task_env(...)`` / ``from _envs import - UPI``); ``env=_envs.UPI`` collects the module alias ``_envs`` + ``@task(env=...)`` referenced — ``env=TRAINING`` collects ``TRAINING`` + (``TRAINING = TaskEnv(...)`` / ``TRAINING = make_task_env(...)`` / ``from _envs import + TRAINING``); ``env=_envs.TRAINING`` collects the module alias ``_envs`` (``import _envs``). It is deliberately narrow: only names PROVEN to participate in a stripped @@ -1047,7 +1047,7 @@ def _strip_authoring_constructs(source_code: str) -> str: are removed. It is NOT a general unused-import cleaner. It raises :class:`AuthoringStripError` (fail-fast) rather than bake a broken program when an env binding is entangled with runtime code: a mixed - ``from _envs import UPI, helper`` whose ``helper`` is used at runtime, or a + ``from _envs import TRAINING, helper`` whose ``helper`` is used at runtime, or a collected env name still referenced by the kept task body. This intentionally operates on ``module_source_stripped`` ONLY. It must never @@ -1148,10 +1148,10 @@ def _strip_authoring_constructs(source_code: str) -> str: # Restricted to module-level statements (``tree.body``) so nested code is # never touched. Two kinds of statement are stripped: # 1. assignments that construct a TaskEnv directly (``X = TaskEnv(...)``) - # or whose target is a collected env name (``UPI = make_task_env(...)`` - # when ``@task(env=UPI)`` was seen), and + # or whose target is a collected env name (``TRAINING = make_task_env(...)`` + # when ``@task(env=TRAINING)`` was seen), and # 2. imports that bind a collected env name/module (``from _envs import - # UPI`` / ``import _envs``) when that name is env-only. + # TRAINING`` / ``import _envs``) when that name is env-only. # # We record each candidate's bound name(s) + line range, then verify (after # a reference scan) that removing it cannot break kept runtime code. @@ -1184,12 +1184,12 @@ def _strip_authoring_constructs(source_code: str) -> str: # Reference scan: every ``Name`` used in a ``Load`` context, mapped to the # 1-indexed lines it appears on. Attribute roots (``_envs`` in - # ``_envs.UPI``) are plain ``Name`` Load nodes too, so this covers them. + # ``_envs.TRAINING``) are plain ``Name`` Load nodes too, so this covers them. # # FIX N1 (§3.5): exclude ``Name`` nodes that live in a type-annotation slot # (param/return/AnnAssign). Annotations are stripped from the baked output by # ``_strip_type_hints`` (which runs later), so an env name used ONLY as a - # type annotation (``def f(x: UPI) -> UPI:``) is NOT a live runtime + # type annotation (``def f(x: TRAINING) -> TRAINING:``) is NOT a live runtime # reference and must not trip the body-ref fail-fast. A real body reference # (outside annotations) still records a Load and still fails fast. if env_assign_bindings or env_import_candidates: @@ -1234,7 +1234,7 @@ def _referenced_in_kept(name: str) -> bool: "statement with runtime name(s) " + ", ".join(used_others) + ". Split the import so TaskEnv env names are imported on " - "their own line (e.g. `from _envs import UPI` separate from " + "their own line (e.g. `from _envs import TRAINING` separate from " "`from _envs import helper`); env imports are authoring-only " "and stripped from the baked runtime program." ) diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py index eb9babb..10da5bf 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py @@ -1913,18 +1913,6 @@ def _pipeline_accepts_cfg(pipeline_fn: PipelineFn) -> bool: return getattr(annotation, "__origin__", None) is not In -def _resolve_cfg_path(pipeline_fn: PipelineFn, module_path: Path) -> Path: - """Resolve the cfg path declared by ``@pipeline(config=...)``. - - Relative to the file holding the decorated function. If the - decorator omits ``config=``, returns the module's directory / - ``config.yaml`` as the default. Thin wrapper over - :func:`_resolve_cfg_path_in_dir` for callers that have the module FILE - rather than its directory. - """ - return _resolve_cfg_path_in_dir(pipeline_fn, module_path.parent) - - def _resolve_cfg_path_in_dir(pipeline_fn: PipelineFn, base_dir: Path) -> Path: """Resolve ``@pipeline(config=...)`` relative to ``base_dir``. diff --git a/packages/tangle-cli/src/tangle_cli/python_pipeline/registered.py b/packages/tangle-cli/src/tangle_cli/python_pipeline/registered.py index cdb91e7..22b78b1 100644 --- a/packages/tangle-cli/src/tangle_cli/python_pipeline/registered.py +++ b/packages/tangle-cli/src/tangle_cli/python_pipeline/registered.py @@ -126,8 +126,10 @@ def registered( hydrate). - An explicit relative path is resolved against the nearest ancestor directory of the operation's source file that - contains the ``oasis.pipeline_component_root.yaml`` zone-root - marker. + contains a registered zone-root marker (see + ``pipeline_compiler.ZONE_ROOT_MARKERS``). The open-source + build registers no markers by default, so a relative path is + rejected unless a downstream distribution adds one. - When omitted, defaults to the NEAREST ancestor ``gen_config.yaml`` of the operation's source file. diff --git a/packages/tangle-cli/src/tangle_cli/python_pipeline/task_env.py b/packages/tangle-cli/src/tangle_cli/python_pipeline/task_env.py index ce7d3df..ed1325c 100644 --- a/packages/tangle-cli/src/tangle_cli/python_pipeline/task_env.py +++ b/packages/tangle-cli/src/tangle_cli/python_pipeline/task_env.py @@ -9,22 +9,22 @@ ``TaskEnv`` is **authoring-only**. ``@task(env=...)`` expands it at decoration time into the existing ``CallableRef._task_image`` / ``CallableRef._task_dependencies_from`` metadata. The compiler, hydrator, -and Oasis runner never see a ``TaskEnv`` object — no downstream component -learns the word ``env``. +and downstream runner never see a ``TaskEnv`` object — no downstream +component learns the word ``env``. Example:: from pathlib import Path from tangle_cli.python_pipeline import TaskEnv, task - UPI = TaskEnv( - image="us-docker.pkg.dev/.../areas-ml-upi:main", + TRAINING = TaskEnv( + image="python:3.12", dependencies_from=Path(__file__).parent / "pyproject.toml", ) - @task(env=UPI) - def publish_to_comet(...): - # docstring carries: Metadata / Name: [UPI Clustering] Publish To Comet + @task(env=TRAINING) + def train_model(...): + # docstring carries: Metadata / Name: Train Model ... The component name comes from the function's docstring ``Metadata: Name:`` diff --git a/packages/tangle-cli/src/tangle_cli/python_pipeline/trace.py b/packages/tangle-cli/src/tangle_cli/python_pipeline/trace.py index 5e871bd..1f46b87 100644 --- a/packages/tangle-cli/src/tangle_cli/python_pipeline/trace.py +++ b/packages/tangle-cli/src/tangle_cli/python_pipeline/trace.py @@ -280,7 +280,7 @@ def trace_pipeline( raise CompileError( "@pipeline declared -> Out[T] but the function did not return a " "TaskOutputProxy. Return the result of the final task call (e.g. " - "`return publish_to_comet`)." + "`return train_model`)." ) inner = _annotation_inner_type(return_anno) type_str = _python_type_to_tangle_type(inner) diff --git a/tests/test_python_pipeline_dsl.py b/tests/test_python_pipeline_dsl.py index 7988877..b693ab7 100644 --- a/tests/test_python_pipeline_dsl.py +++ b/tests/test_python_pipeline_dsl.py @@ -9,6 +9,7 @@ from __future__ import annotations import importlib +import re import sys from dataclasses import dataclass from pathlib import Path @@ -100,18 +101,28 @@ def test_import_is_light(self): class TestNoInternalReferences: """The OSS authoring + compile surface must be free of internal - ``tangle-deploy`` references, in either the ``tangle_deploy`` (module/import) - or the ``tangle-deploy`` (product/command) spelling. The dependency points + references — neither the downstream package (``tangle_deploy`` / + ``tangle-deploy``) nor the internal products/infra it was carved out of + (Oasis, UPI, Comet, ``areas-ml*`` image paths). The dependency points inward: a downstream package registers its own authoring path via - ``register_authoring_import_module`` — OSS never names it.""" + ``register_authoring_import_module`` — OSS never names it, and OSS + docstrings/examples never name an internal product. A leaked ref here + would surface in a user-facing CompileError, as one did before this + guard was widened.""" + + # Substring terms: unambiguous, safe to match anywhere in the text. + _INTERNAL_SUBSTRINGS = ("tangle_deploy", "tangle-deploy", "areas-ml", "areas/ml") + # Word-boundary terms: short/common enough that a bare substring match + # would false-positive (e.g. "upi" inside "deduping"), so match only as a + # standalone, case-insensitive word. + _INTERNAL_WORDS = ("oasis", "upi", "comet") def test_no_tangle_deploy_references_in_source(self): # Scan the whole vendored DSL package plus the sibling modules that make # up the OSS compile surface: the authoring-import strip driver # (``component_from_func.py``) and the ported compiler + its schema # validator + the ``pipelines`` facade/CLI. All must stay decoupled from - # the downstream package name (a leaked ref here would surface in a - # user-facing CompileError, as one did before this guard was widened). + # the downstream package name and from internal product names. scanned = list(_DSL_DIR.glob("*.py")) scanned += [ _DSL_DIR.parent / name @@ -123,12 +134,17 @@ def test_no_tangle_deploy_references_in_source(self): "pipelines_cli.py", ) ] - offenders = [] + word_re = re.compile( + r"\b(" + "|".join(self._INTERNAL_WORDS) + r")\b", re.IGNORECASE + ) + offenders = {} for path in scanned: text = path.read_text(encoding="utf-8") - if "tangle_deploy" in text or "tangle-deploy" in text: - offenders.append(path.name) - assert offenders == [], f"tangle-deploy referenced in: {offenders}" + hits = [term for term in self._INTERNAL_SUBSTRINGS if term in text] + hits += sorted({m.group(0).lower() for m in word_re.finditer(text)}) + if hits: + offenders[path.name] = hits + assert offenders == {}, f"internal references found: {offenders}" # ============================================================================ From 85c069b23fac9c560d7af669a06cba9061ad3bc2 Mon Sep 17 00:00:00 2001 From: Silin Gupta Date: Tue, 14 Jul 2026 15:48:18 -0400 Subject: [PATCH 4/6] Fix propagate_config broadcast through config-less intermediate + cover it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second independent review of #28 found a blocking correctness bug in the propagate_config broadcast subsystem, plus its total lack of test coverage. Both fixed here, with two doc nits. 1. BLOCKING — broadcast hard-failed through a config-less intermediate. In `_process_subpipeline_children`, an active broadcast (non-empty `ctx.broadcast_stack`) unconditionally called `_read_raw_cfg(child_cfg_path)` for every child edge, and that helper raises `CompileError: config file not found` on a missing file. A pure composing subpipeline (no `cfg` param, no `config=`) has no config.yaml on disk, so the canonical shape `root(propagate_config=True) -> config-less parent -> config-taking child` failed with a spurious "config file not found" for a file the author never wrote. Fix: in the broadcast path, treat a MISSING child config as `{}` (the child declares nothing to overlay; broadcast keys flow PAST it to descendants). A config-DECLARING child whose file is genuinely missing is still caught later, with guidance, in `_load_cfg_and_raw` (verified: the `cfg`-param error message is preserved). 2. Coverage: the entire broadcast subsystem had zero tests (`grep propagate_config tests/` was empty — which is why #1 shipped). Added `tests/fixtures/python_pipeline/broadcast_zone/` (a 3-level tree isolated in its own dir so the config-less intermediate genuinely has no config.yaml — the shared fixtures dir has one that would mask the bug) + a regression test asserting the compile succeeds and the root's broadcast value reaches the grandchild leaf, winning over the grandchild's own config (three distinct values make the source unambiguous). 3. Nit: the "content-hash" wording in a subpipeline test comment is really a compile-key hash (source path + qualname + name + config path + overrides fingerprint), not a hash of emitted content — corrected. 4. Nit: genericized a docstring example name (`options_standardization` / "Options Standardization") that echoed an internal pipeline -> `train_model` / "Train Model". 746 passed (+1); ruff clean on touched files; `uv lock --check` and `git diff --check` clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/tangle_cli/pipeline_compiler.py | 19 +++++-- .../broadcast_grandchild_config.yaml | 1 + .../broadcast_zone/broadcast_pipeline.py | 41 ++++++++++++++++ .../broadcast_zone/broadcast_root_config.yaml | 1 + tests/test_pipeline_compiler.py | 49 ++++++++++++++++++- 5 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 tests/fixtures/python_pipeline/broadcast_zone/broadcast_grandchild_config.yaml create mode 100644 tests/fixtures/python_pipeline/broadcast_zone/broadcast_pipeline.py create mode 100644 tests/fixtures/python_pipeline/broadcast_zone/broadcast_root_config.yaml diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py index 10da5bf..24cb35b 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py @@ -566,7 +566,18 @@ def _process_subpipeline_children( # when the child's OWN effective overrides are identical. ambient_passthrough: dict[str, Any] = {} if ctx.broadcast_stack or sub_ref.config_overrides: - child_raw = _read_raw_cfg(child_cfg_path) + # A config-less child (no ``cfg`` param, no ``config=``) has no + # ``config.yaml`` on disk. Under an active broadcast (or an + # explicit ``.override_config``) we still consult the child's + # declared keys to drive the same-name overlay and the ambient + # pass-through — but a MISSING file means the child simply declares + # nothing to overlay, so treat it as ``{}`` and let every broadcast + # key flow PAST it to descendants. Reading it strictly here would + # hard-fail a config-less intermediate purely because an ancestor + # broadcasts. A config-DECLARING child whose file is genuinely + # missing is still caught, with guidance, later in + # ``_load_cfg_and_raw`` (via the child's own ``_compile_pipeline_fn``). + child_raw = _read_raw_cfg(child_cfg_path) if child_cfg_path.exists() else {} effective = _effective_overrides_for_child( child_raw=child_raw, broadcast_stack=ctx.broadcast_stack, @@ -1682,9 +1693,9 @@ def _candidate_names(fn: PipelineFn) -> tuple[str, str]: """Return ``(function_name, display_name)`` for ``--pipeline`` matching. ``function_name`` is the decorated function's ``__name__`` (a clean, - shell-friendly identifier such as ``options_standardization``); - ``display_name`` is the ``@pipeline(name=...)`` value emitted as the - YAML ``name:`` (may contain spaces, e.g. ``"Options Standardization"``). + shell-friendly identifier such as ``train_model``); ``display_name`` is + the ``@pipeline(name=...)`` value emitted as the YAML ``name:`` (may + contain spaces, e.g. ``"Train Model"``). Selection matches the function name first, then the display name. """ func_name = getattr(fn.fn, "__name__", "") or "" diff --git a/tests/fixtures/python_pipeline/broadcast_zone/broadcast_grandchild_config.yaml b/tests/fixtures/python_pipeline/broadcast_zone/broadcast_grandchild_config.yaml new file mode 100644 index 0000000..e28a048 --- /dev/null +++ b/tests/fixtures/python_pipeline/broadcast_zone/broadcast_grandchild_config.yaml @@ -0,0 +1 @@ +shared_key: from-grandchild diff --git a/tests/fixtures/python_pipeline/broadcast_zone/broadcast_pipeline.py b/tests/fixtures/python_pipeline/broadcast_zone/broadcast_pipeline.py new file mode 100644 index 0000000..5aaf723 --- /dev/null +++ b/tests/fixtures/python_pipeline/broadcast_zone/broadcast_pipeline.py @@ -0,0 +1,41 @@ +"""Fixture: ``propagate_config`` broadcast through a config-less intermediate. + +The root sets ``propagate_config=True`` and declares ``shared_key`` in its own +config.yaml. ``Broadcast Middle`` is a pure composer — no ``cfg`` parameter and +no ``config=``, so it has NO config.yaml on disk. ``Broadcast Grandchild`` +declares ``shared_key`` in its own config and emits it as a task-argument +constant. + +The broadcast must flow PAST the config-less ``Middle`` and reach +``Grandchild``, where the root's broadcast value wins over the grandchild's own +config.yaml value (same key name). This is the canonical nesting shape; before +the broadcast path tolerated a missing intermediate config.yaml it hard-failed +here with a spurious "config file not found". +""" +from tangle_cli.python_pipeline import In, Out, pipeline, subpipeline, task + + +@task(image="python:3.12") +def leaf_task(shared_key: str = "from-task-default"): + """Emit the broadcast key. + + Metadata: + Name: Leaf Task + """ + print(shared_key) + + +@pipeline("Broadcast Grandchild", config="broadcast_grandchild_config.yaml") +def broadcast_grandchild(seed: In[str], cfg) -> Out[str]: + run = leaf_task(shared_key=cfg.shared_key, wait_for=seed) + return run + + +@pipeline("Broadcast Middle") # config-less composer: no cfg param, no config= +def broadcast_middle(seed: In[str]) -> Out[str]: + return subpipeline(broadcast_grandchild).named("Run Grandchild")(seed=seed) + + +@pipeline("Broadcast Root", config="broadcast_root_config.yaml", propagate_config=True) +def broadcast_root(parent_wait_token: In[str], cfg) -> Out[str]: + return subpipeline(broadcast_middle).named("Run Middle")(seed=parent_wait_token) diff --git a/tests/fixtures/python_pipeline/broadcast_zone/broadcast_root_config.yaml b/tests/fixtures/python_pipeline/broadcast_zone/broadcast_root_config.yaml new file mode 100644 index 0000000..2ed8eb9 --- /dev/null +++ b/tests/fixtures/python_pipeline/broadcast_zone/broadcast_root_config.yaml @@ -0,0 +1 @@ +shared_key: from-root-broadcast diff --git a/tests/test_pipeline_compiler.py b/tests/test_pipeline_compiler.py index 3113bbc..d3a9c7f 100644 --- a/tests/test_pipeline_compiler.py +++ b/tests/test_pipeline_compiler.py @@ -700,8 +700,8 @@ def test_compile_subpipeline_emits_child_sidecar(tmp_path): FIXTURES / "subpipeline_pipeline.py", out, pipeline_name="Parent Pipeline" ) - # Exactly one child sidecar, named after the child pipeline (content-hash - # suffix), living under the parent's .subgraphs/ directory. + # Exactly one child sidecar, named after the child pipeline (compile-key + # hash suffix), living under the parent's .subgraphs/ directory. assert len(result.subgraph_paths) == 1 child = result.subgraph_paths[0] assert child.exists() @@ -740,6 +740,51 @@ def test_compile_subpipeline_override_config_wins(tmp_path): assert task["arguments"]["message"] == "from-override" +# --------------------------------------------------------------------------- +# propagate_config broadcast. A pipeline with propagate_config=True broadcasts +# its own config.yaml deep into the subtree by key name; the broadcast must +# flow PAST config-less intermediate subpipelines to reach descendants that +# declare the key. + +_BROADCAST_ZONE = FIXTURES / "broadcast_zone" + + +def test_compile_propagate_config_broadcasts_through_configless_intermediate(tmp_path): + """``propagate_config=True`` broadcasts a config key deep into the subtree, + flowing PAST a config-less intermediate (no ``cfg``/``config=``, so no + config.yaml on disk) to a config-declaring grandchild, where the root's + broadcast value wins over the grandchild's own config.yaml (same key). + + Regression guard: before the broadcast path tolerated a missing + intermediate config.yaml, this compile hard-failed with a spurious + ``config file not found`` for the config-less ``Broadcast Middle``. The + grandchild leaf emits ``cfg.shared_key`` as a task-argument constant, so + the broadcast value is observable and can only be the root's — it is + neither the ``@task`` default (``from-task-default``) nor the grandchild's + own config value (``from-grandchild``).""" + out = tmp_path / "compiled.yaml" + result = compile_pipeline( + _BROADCAST_ZONE / "broadcast_pipeline.py", out, pipeline_name="Broadcast Root" + ) + # Two child sidecars: the config-less Middle and the Grandchild leaf. + assert len(result.subgraph_paths) == 2 + slugs = sorted(p.name.rsplit("-", 1)[0] for p in result.subgraph_paths) + assert slugs == ["broadcast-grandchild", "broadcast-middle"] + # The grandchild leaf task argument carries the root's broadcast value, + # proving the key flowed through the config-less intermediate and won over + # the grandchild's own config.yaml. + gc_path = next( + p for p in result.subgraph_paths if p.name.startswith("broadcast-grandchild-") + ) + gc = yaml.safe_load(gc_path.read_text()) + (leaf,) = gc["implementation"]["graph"]["tasks"].values() + assert leaf["arguments"]["shared_key"] == "from-root-broadcast" + # Parent + both child sidecars are structurally valid dehydrated pipelines. + validate_dehydrated_data(yaml.safe_load(out.read_text())) + for p in result.subgraph_paths: + validate_dehydrated_data(yaml.safe_load(p.read_text())) + + # --------------------------------------------------------------------------- # @registered gen_config resolution. @registered references an EXISTING # gen_config.yaml and generates no sidecar of its own; the task's componentRef From 4a19b60f04eeac73abe3ebfe8dec5bbe00a1745b Mon Sep 17 00:00:00 2001 From: Silin Gupta Date: Wed, 15 Jul 2026 12:53:59 -0400 Subject: [PATCH 5/6] Address review nits: drop internal design-doc citations, delete phantom docstring, extract cfg-load helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer follow-ups on the PR: * pipeline_compiler.py — replace ~36 internal design-doc citations (PROPAGATE_CONFIG_DESIGN §N, Decision A–M, NM1/NM2, Fix B, Finding 2, P2/D4/Phase N) with the rule stated inline, so every comment stands on its own in the public repo with no dangling pointer to a doc that isn't published. * schema_validation.py — delete the module-docstring sentence promising a `PipelineHydrator.validate_dehydrated_file` convenience that does not exist. * pipeline_compiler.py — extract the step-1 cfg resolve/load block (plus its warning/override-error branches) out of `_compile_pipeline_fn` into `_resolve_cfg_for_pipeline`, leaving the compile function a thin orchestrator whose remaining steps already delegate to named passes. Comment/docstring-only plus pure code-motion; behavior unchanged (746 tests pass, golden byte-identical fixtures included). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/tangle_cli/pipeline_compiler.py | 211 ++++++++++-------- .../src/tangle_cli/schema_validation.py | 4 +- 2 files changed, 121 insertions(+), 94 deletions(-) diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py index 24cb35b..e34936e 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py @@ -174,7 +174,7 @@ def compile_pipeline( try: # 2. Load the user pipeline module + select the ROOT PipelineFn. When # the file defines several pipelines, ``pipeline_name`` - # (``--pipeline``) chooses which to emit (Decision G). Imported + # (``--pipeline``) chooses which to emit. Imported # child pipelines are reachable to ``subpipeline(child)`` but are # never selectable compile targets; same-file nested children are # compiled via the ``subpipeline`` recursion below. @@ -188,9 +188,9 @@ def compile_pipeline( # 4. Build the recursive compile context. ALL artifacts (root + every # nested child sidecar) are traced, emitted, and rewritten IN MEMORY - # first; nothing is written until the whole bundle validates - # (Decision C). ``planned_files`` lets asset-policy validation accept - # refs to sidecars the compiler is about to write (Decision J). + # first; nothing is written until the whole bundle validates. + # ``planned_files`` lets asset-policy validation accept + # refs to sidecars the compiler is about to write. ctx = CompileContext( root_output_path=output_path, subgraph_dir=output_path.with_name(output_path.stem + ".subgraphs"), @@ -202,7 +202,7 @@ def compile_pipeline( # 5. Compile the root (and, recursively, all children) into in-memory # artifacts. The root cfg is resolved relative to the script's own # directory; each child's cfg is resolved relative to ITS OWN source - # directory inside ``_compile_pipeline_fn`` (Decision F). + # directory inside ``_compile_pipeline_fn``. root_artifact = _compile_pipeline_fn( pipeline_fn, output_path, @@ -214,14 +214,13 @@ def compile_pipeline( # 6. The full bundle: the root plus every deduped child in the registry. # Keying children by compile key means a child reached twice (or via - # a diamond) appears exactly once (Decision M). + # a diamond) appears exactly once. artifacts: list[SubgraphArtifact] = [root_artifact, *ctx.registry.values()] # 7. Validate EVERY artifact before writing ANY file. Per artifact: # validate_dehydrated_pipeline (top-level guard + schema + # no-template scan + semantic checks) -> dump -> reload -> validate - # again -> asset policy. On any failure nothing is written - # (Decision C / J). + # again -> asset policy. On any failure nothing is written. for artifact in artifacts: _validate_artifact(artifact, ctx) @@ -238,12 +237,12 @@ def compile_pipeline( ) finally: # Purge bundle-local modules so a subsequent in-process compile - # re-imports them fresh (P2). Runs on success AND on error. + # re-imports them fresh. Runs on success AND on error. _purge_bundle_local_modules(purge_dirs) # --------------------------------------------------------------------------- -# Recursive in-memory compile (Decision C) +# Recursive in-memory compile @contextmanager @@ -271,6 +270,72 @@ def _temp_sys_path(directory: Path) -> Iterator[None]: pass +def _resolve_cfg_for_pipeline( + pipeline_fn: PipelineFn, + base_dir: Path, + overrides: Mapping[str, Any], + ctx: CompileContext, + *, + is_root: bool, + output_path: Path, +) -> tuple[Cfg, dict[str, Any], Path]: + """Resolve + load a pipeline's cfg relative to its own source dir. + + Returns ``(cfg, raw_cfg, cfg_path)``. The config file is loaded when the + function takes a ``cfg`` parameter OR the pipeline is flagged + ``propagate_config`` (which must read its own config to broadcast it), and + otherwise skipped — a pipeline with neither tolerates a missing config.yaml. + A ``config=`` declared without a matching use appends an explanatory + warning; a root that received ``--override`` values it cannot consume is a + hard :class:`CompileError`. + """ + cfg_path = _resolve_cfg_path_in_dir(pipeline_fn, base_dir) + uses_cfg = _pipeline_accepts_cfg(pipeline_fn) + should_load_config = uses_cfg or pipeline_fn.propagate_config + if should_load_config: + _assert_config_output_path_is_separate( + cfg_path, + output_path, + pipeline_fn=pipeline_fn, + is_root=is_root, + ) + loaded_cfg, raw_cfg = _load_cfg_and_raw( + cfg_path, + overrides, + coerce=is_root, + pipeline_fn=pipeline_fn, + usage="cfg" if uses_cfg else "propagate_config", + ) + cfg = loaded_cfg if uses_cfg else Cfg({}) + if not uses_cfg and pipeline_fn.config_path: + ctx.warnings.append( + f"pipeline {pipeline_fn.name!r} declares config={pipeline_fn.config_path!r} " + "but its function has no `cfg` parameter. The config file is still " + "loaded because propagate_config=True broadcasts it to descendant " + "subpipelines. Add a `cfg` parameter if this pipeline also needs to " + "read the config directly." + ) + return cfg, raw_cfg, cfg_path + + if is_root and overrides: + keys = sorted(overrides) + raise CompileError( + f"pipeline {pipeline_fn.name!r} received --override values {keys!r}, " + "but its function has no `cfg` parameter and propagate_config is " + "not enabled. Add a `cfg` parameter to read compile-time config, " + "enable propagate_config=True to broadcast overrides to descendants, " + "or remove the unused override(s)." + ) + if pipeline_fn.config_path: + ctx.warnings.append( + f"pipeline {pipeline_fn.name!r} declares config={pipeline_fn.config_path!r} " + "but its function has no `cfg` parameter, so the config file was " + "not loaded. Remove `config=` when no compile-time config is needed, " + "or add a `cfg` parameter to use it." + ) + return Cfg({}), {}, cfg_path + + def _compile_pipeline_fn( pipeline_fn: PipelineFn, output_path: Path, @@ -291,12 +356,12 @@ def _compile_pipeline_fn( Steps: 1. Resolve + load cfg relative to ``base_dir`` (the pipeline's OWN - source dir — Decision F config isolation). + source dir — each pipeline loads its own config). 2. Trace in a fresh ``GraphBuilder`` and emit the dehydrated body. 3. Build (do NOT write) this artifact's ``@task`` ``local_from_python`` components sidecar and rewrite its ``@task`` refs to pure ``resolve://`` URLs. - 4. (Phase 4) compile any ``subpipeline(child)`` children recursively and + 4. Compile any ``subpipeline(child)`` children recursively and rewrite the parent task refs to pure ``file://`` URLs. Returns the planned artifact; validation + writing happen later in @@ -319,51 +384,14 @@ def _compile_pipeline_fn( # The ROOT coerces its CLI ``--override`` strings (yaml.safe_load); a # CHILD receives already-typed native ``effective`` overrides (broadcast # + explicit ``.override_config``) that must pass through unchanged. - cfg_path = _resolve_cfg_path_in_dir(pipeline_fn, base_dir) - uses_cfg = _pipeline_accepts_cfg(pipeline_fn) - should_load_config = uses_cfg or pipeline_fn.propagate_config - if should_load_config: - _assert_config_output_path_is_separate( - cfg_path, - output_path, - pipeline_fn=pipeline_fn, - is_root=is_root, - ) - loaded_cfg, raw_cfg = _load_cfg_and_raw( - cfg_path, - overrides, - coerce=is_root, - pipeline_fn=pipeline_fn, - usage="cfg" if uses_cfg else "propagate_config", - ) - cfg = loaded_cfg if uses_cfg else Cfg({}) - if not uses_cfg and pipeline_fn.config_path: - ctx.warnings.append( - f"pipeline {pipeline_fn.name!r} declares config={pipeline_fn.config_path!r} " - "but its function has no `cfg` parameter. The config file is still " - "loaded because propagate_config=True broadcasts it to descendant " - "subpipelines. Add a `cfg` parameter if this pipeline also needs to " - "read the config directly." - ) - else: - if is_root and overrides: - keys = sorted(overrides) - raise CompileError( - f"pipeline {pipeline_fn.name!r} received --override values {keys!r}, " - "but its function has no `cfg` parameter and propagate_config is " - "not enabled. Add a `cfg` parameter to read compile-time config, " - "enable propagate_config=True to broadcast overrides to descendants, " - "or remove the unused override(s)." - ) - if pipeline_fn.config_path: - ctx.warnings.append( - f"pipeline {pipeline_fn.name!r} declares config={pipeline_fn.config_path!r} " - "but its function has no `cfg` parameter, so the config file was " - "not loaded. Remove `config=` when no compile-time config is needed, " - "or add a `cfg` parameter to use it." - ) - cfg = Cfg({}) - raw_cfg = {} + cfg, raw_cfg, cfg_path = _resolve_cfg_for_pipeline( + pipeline_fn, + base_dir, + overrides, + ctx, + is_root=is_root, + output_path=output_path, + ) # 2. Trace + emit. The source dir is on sys.path so any trace-time # sibling imports inside the body resolve. ``emit_pipeline`` also @@ -456,22 +484,23 @@ def _compile_pipeline_fn( # and rewrite this artifact's subpipeline tasks to pure ``file://`` # refs. This artifact's key is on the active stack while children are # compiled so a child that reaches back here is detected as a cycle - # (Decision L). Subpipeline children appear only when NM1 recorded + # detected. Subpipeline children appear only when the trace recorded # them; root-only / @task-only compiles skip this entirely. # # When ``propagate_config`` is set, push a broadcast layer carrying - # THIS pipeline's OWN config (Decision §4: "broadcast my OWN config"). + # THIS pipeline's OWN config — a flagged pipeline broadcasts its own + # config, never a value handed to it from above. # The layer is ``raw_cfg`` overlaid with ``rebroadcast``. ``rebroadcast`` # is this pipeline's OWN re-broadcastable overrides: # * ROOT (``rebroadcast_overrides is None``) — its CLI ``--override`` # values, COERCED with the same ``yaml.safe_load`` coercion # ``load_cfg(coerce=True)`` applied to the root's own cfg, so the - # broadcast carries the typed value the root itself sees (Finding 2), + # broadcast carries the typed value the root itself sees, # not the raw CLI string. # * flagged CHILD (``rebroadcast_overrides == {}``) — nothing extra: # a flagged child broadcasts ONLY its own ``raw_cfg``. The explicit # ``.override_config`` values set on the edge INTO this child have - # their DEPTH governed by the CALLER's flag (Decision §4 line 122), + # their DEPTH governed by the CALLER's flag, # so the caller — not this child — pushes a per-edge layer for them # (see ``_process_subpipeline_children``). This keeps "nearest # flagged ancestor that DEFINES the key wins": an outer ancestor's @@ -518,7 +547,7 @@ def _process_subpipeline_children( * compute the child :class:`PipelineCompileKey`; * CYCLE check — if the key is already on ``ctx.active_stack`` raise a - :class:`CompileError` naming the full chain (Decision L); this also + :class:`CompileError` naming the full chain; this also covers self-reference (``A -> A``); * MAX-DEPTH guard — refuse chains deeper than ``ctx.max_depth``; * DEDUP — if the key is already compiled (``ctx.registry``) reuse that @@ -553,11 +582,11 @@ def _process_subpipeline_children( # entirely and use ``{}``. The fast path does NOT let a config-taking # child without a config.yaml compile — that child still raises # "config file not found" in its own ``_compile_pipeline_fn``. What it - # buys is: one fewer raw-cfg read, and exact preservation of Decision-F - # default isolation (empty effective overrides -> empty + # buys is: one fewer raw-cfg read, and exact preservation of the + # default per-pipeline config isolation (empty effective overrides -> empty # ``overrides_fingerprint`` -> the SAME dedup key the child gets with no # feature in play) whenever there is nothing to broadcast or override. - # Ambient PASS-THROUGH context (Fix B / §7): the resolved + # Ambient PASS-THROUGH context: the resolved # broadcast/override keys this child does NOT declare and therefore # flow PAST it, unchanged, to its descendants. It is folded into the # child's compile key so the SAME child reached under two different @@ -629,7 +658,7 @@ def _process_subpipeline_children( ) slug = _slugify(child_fn.name) child_output_path = ctx.subgraph_dir / f"{slug}-{child_key.hash8()}.yaml" - # Per-edge explicit-override depth (Decision §4 line 122-124): an + # Per-edge explicit-override depth: an # explicit ``.override_config`` set on THIS edge flows deep iff the # CALLER (this parent) is flagged. When it is, push a broadcast # layer carrying JUST this edge's explicit overrides as the INNERMOST @@ -650,7 +679,7 @@ def _process_subpipeline_children( # is resolved AFTER the whole broadcast tier in # ``_effective_overrides_for_child`` — so this edge's override # outranks even a NEARER flagged descendant's own-config - # broadcast of the same key (PROPAGATE_CONFIG_DESIGN §4). + # broadcast of the same key. ctx.broadcast_stack.append(BroadcastLayer(config=dict(sub_ref.config_overrides), explicit=True)) pushed_edge_override = True try: @@ -684,7 +713,7 @@ def _process_subpipeline_children( ) parent_tasks[task_id]["componentRef"] = {"url": f"file://{rel}"} - # INPUT interface validation (Decision E). The compiled child body + # INPUT interface validation. The compiled child body # is the source of truth for declared inputs; ``wait_for`` / # ``depends_on`` are NOT special — they must be declared child # In[...] inputs if passed. @@ -695,7 +724,7 @@ def _process_subpipeline_children( ) subpipeline_output_names[task_id] = _child_output_names(child_artifact.body) - # OUTPUT cross-file validation (Decision E / K) — a safety net beyond the + # OUTPUT cross-file validation — a safety net beyond the # strict SubpipelineOutputProxy: every ``taskOutput`` in the parent body # that targets a subpipeline task must name an output the child declares. _validate_subpipeline_output_refs(parent_artifact.body, subpipeline_output_names) @@ -741,7 +770,7 @@ def _validate_subpipeline_inputs( Rejects an UNKNOWN argument name (not a declared child input) and a MISSING REQUIRED child input (declared, non-optional, not supplied). Omitted optional/default child inputs are allowed. Raises a clear - :class:`CompileError` BEFORE any file is written (Decision E). + :class:`CompileError` BEFORE any file is written. """ declared, required = _child_input_specs(child_artifact.body) child_name = child_artifact.key.pipeline_name @@ -773,7 +802,7 @@ def _validate_subpipeline_output_refs( Scans both task ``arguments`` and graph ``outputValues``. This is the compiler-owned cross-file safety net that protects the serialized YAML - even if a proxy bug let an undeclared output slip through (Decision K). + even if a proxy bug let an undeclared output slip through. """ if not subpipeline_output_names: return @@ -817,7 +846,7 @@ def _pipeline_base_dir(pipeline_fn: PipelineFn, *, fallback: Path) -> Path: are relative to. Prefers the ``@pipeline`` decorator's captured ``caller_dir`` (the - child's own source file directory — Decision F), then the decorated + child's own source file directory), then the decorated function's source file, then ``fallback`` (the root bundle directory) for dynamically built pipelines with no on-disk source. """ @@ -851,8 +880,8 @@ def _compile_key_for( ) -> PipelineCompileKey: """Build the canonical :class:`PipelineCompileKey` for ``pipeline_fn``. - Paths are canonicalised per Decision B (repo-relative POSIX inside a git - repo, else resolved absolute POSIX). A dynamically built pipeline whose + Paths are canonicalised to repo-relative POSIX inside a git + repo, else resolved absolute POSIX. A dynamically built pipeline whose source cannot be located falls back to a qualname-derived sentinel so it still produces a stable, distinct key. @@ -862,7 +891,7 @@ def _compile_key_for( ``.override_config`` value surfaces as an actionable :class:`CompileError` rather than a bare ``TypeError``. - ``ambient_context`` is the AMBIENT PASS-THROUGH context (Fix B / §7): the + ``ambient_context`` is the AMBIENT PASS-THROUGH context: the active broadcast/override keys this node does NOT declare and therefore flow PAST it to its descendants. Folding it into the fingerprint keeps the same node distinct when it is reached under two different ancestor-broadcast @@ -872,7 +901,7 @@ def _compile_key_for( * **Empty / ``None`` ambient** (always true for the default-isolation compile) -> ``overrides_fingerprint(overrides)`` UNCHANGED, byte-for-byte - identical to before Fix B. + identical to the fingerprint with no ambient envelope. * **Non-empty ambient** -> a small envelope keyed by reserved sentinels (``"\\x00effective"`` / ``"\\x00ambient"``) that cannot collide with real config keys, routed through :func:`overrides_fingerprint` so its @@ -893,7 +922,7 @@ def _compile_key_for( "\x00ambient": dict(sorted(ambient_context.items())), } else: - # Byte-identical to today's key: the default-isolation guarantee (§7). + # Byte-identical to today's key: the default-isolation guarantee. fingerprint_input = overrides return PipelineCompileKey( source_path=source_canon, @@ -942,7 +971,7 @@ def _validate_artifact(artifact: SubgraphArtifact, ctx: CompileContext) -> None: prefix = "" if label is None else f"{label}: " raise CompileError(f"{prefix}compiled YAML failed re-validation after dump/reload: {e}") from e - # Asset policy (Decision J, extended to children in Phase 8). EVERY + # Asset policy. EVERY # artifact's relative local refs are validated relative to THAT # artifact's own output directory: # * the ROOT body relative to the root output dir (``label`` is None, @@ -953,7 +982,7 @@ def _validate_artifact(artifact: SubgraphArtifact, ctx: CompileContext) -> None: # sidecars and child ``@task`` components sidecars — are in # ``ctx.planned_files`` and count as present, so the compiler-managed # parent→child / child→child / child @task refs pass. A child's - # author-written relative leaf ref was relocated (NM2) to be relative to + # author-written relative leaf ref was relocated to be relative to # the child-sidecar dir, so it is validated against the real source-side # file via ``../``; a missing external leaf fails clearly here, before # any file is written. @@ -1548,8 +1577,8 @@ def _validate_local_component_refs_for_artifact( location, so a relative ``file://./x`` / ``resolve://./x`` ref is only resolvable if ``x`` sits next to that artifact. Generated bundle files (child sidecars, ``@task`` components sidecars) are passed in - ``planned_files`` — they validate as "present" before they are written - (Decision J). External relative refs must already exist on disk. + ``planned_files`` — they validate as "present" before they are written. + External relative refs must already exist on disk. Args: artifact_label: ``None`` for the ROOT (uses the legacy error wording @@ -1728,7 +1757,7 @@ def _load_pipeline_fn(module_path: Path, pipeline_name: str | None = None) -> Pi When the file defines exactly one root pipeline it is returned directly. When it defines several, ``pipeline_name`` selects which one to compile (matched against the function ``__name__`` first, then the - ``@pipeline`` display name — Decision G / ``--pipeline``). The selected + ``@pipeline`` display name, selected via ``--pipeline``). The selected pipeline's same-file nested children are still reachable to ``subpipeline(child)(...)`` and are compiled via the recursion driver, never via this candidate list. @@ -1782,8 +1811,8 @@ def _load_pipeline_fn(module_path: Path, pipeline_name: str | None = None) -> Pi # Root discovery selects the pipeline(s) DEFINED IN the target file. A # parent module that imports child pipelines (so it can wrap them with - # ``subpipeline(child)(...)``) must not count those imports as roots - # (Decision G). Compare each candidate's source file against + # ``subpipeline(child)(...)``) must not count those imports as roots. + # Compare each candidate's source file against # ``module_path``; imported children resolve to a DIFFERENT file and are # ignored as compile targets (they remain reachable to ``subpipeline``). # Several pipelines DEFINED in this one file are now allowed: when more @@ -1815,7 +1844,7 @@ def _load_pipeline_fn(module_path: Path, pipeline_name: str | None = None) -> Pi if pipeline_name is not None: # Explicit selection (``--pipeline``). Match against the function # name then the display name, across local AND undetermined-source - # candidates (D4 — an exec'd module still exposes ``fn.__name__``). + # candidates (an exec'd module still exposes ``fn.__name__``). # Imported children are never selectable as a root. searchable = [*local_candidates, *undetermined] matches = _select_by_name(searchable, pipeline_name) @@ -1833,7 +1862,7 @@ def _load_pipeline_fn(module_path: Path, pipeline_name: str | None = None) -> Pi ) # No explicit selection. Prefer locals; fall back to undetermined with - # the single-candidate dynamic auto-select (Decision G / D4): a single + # the single-candidate dynamic auto-select: a single # exec-built PipelineFn whose source cannot be resolved is still a valid # root, but two undetermined candidates cannot be told apart from # imports, so they fall through to the multiple-pipelines error. @@ -1861,7 +1890,7 @@ def _pipeline_source_path(pipeline_fn: PipelineFn) -> Path | None: source is UNDETERMINED — i.e. a dynamically built function whose source cannot be located. Used by :func:`_load_pipeline_fn` to distinguish locally defined root pipelines from imported child - pipelines, and to honour Decision G's single-candidate dynamic + pipelines, and to honour the single-candidate dynamic fallback. A source is treated as undetermined (``None``) when: @@ -1929,7 +1958,7 @@ def _resolve_cfg_path_in_dir(pipeline_fn: PipelineFn, base_dir: Path) -> Path: ``base_dir`` is the pipeline's OWN source directory — the root script's parent for the root, or the child ``PipelineFn``'s source directory for - a nested child (Decision F: each pipeline loads its own config relative + a nested child (each pipeline loads its own config relative to its own file). If the decorator omits ``config=``, defaults to ``/config.yaml``. """ @@ -2047,7 +2076,7 @@ def _resolve_broadcast_stack( key_filter: set[str] | None, ) -> dict[str, Any]: """Resolve the active broadcast stack into a single key/value map applying - the two precedence tiers from PROPAGATE_CONFIG_DESIGN §4 (lines 128-159). + the two precedence tiers (broadcast tier, then explicit tier). Tiers, lowest -> highest: @@ -2101,7 +2130,7 @@ def _effective_overrides_for_child( parent_task_id: str, ) -> dict[str, Any]: """Resolve a child's effective overrides across the two precedence tiers - plus the strict direct edge (PROPAGATE_CONFIG_DESIGN §4, lines 128-159). + plus the strict direct edge. Layered lowest -> highest precedence: @@ -2142,7 +2171,7 @@ def _resolve_ambient_context(broadcast_stack: list[BroadcastLayer]) -> dict[str, :func:`_effective_overrides_for_child` via the shared :func:`_resolve_broadcast_stack` helper, but with NO ``key_filter`` — every key present in the stack is folded in, regardless of what any single node - declares. Used by Fix B to compute the ambient PASS-THROUGH context (§7): + declares. Used to compute the ambient PASS-THROUGH context: the keys that flow PAST a node, unchanged, to its descendants. """ return _resolve_broadcast_stack(broadcast_stack, key_filter=None) diff --git a/packages/tangle-cli/src/tangle_cli/schema_validation.py b/packages/tangle-cli/src/tangle_cli/schema_validation.py index 5179f2a..774d058 100644 --- a/packages/tangle-cli/src/tangle_cli/schema_validation.py +++ b/packages/tangle-cli/src/tangle_cli/schema_validation.py @@ -28,9 +28,7 @@ Everything here is standalone — it never changes ``PipelineHydrator`` behavior. ``compile_pipeline`` uses :func:`validate_dehydrated_pipeline` -for its richer pre-write check; an explicit, read-only -``PipelineHydrator.validate_dehydrated_file`` convenience also delegates -here. +for its richer pre-write check. """ from __future__ import annotations From 53acb14785188d21029ceaece821c44423fc1049 Mon Sep 17 00:00:00 2001 From: Silin Gupta Date: Wed, 15 Jul 2026 15:41:52 -0400 Subject: [PATCH 6/6] Fix propagate_config cycle detection: thread parent-computed key into child Under propagate_config, a broadcast key a child does not declare flows past it as ambient pass-through context, which the parent folds into the child's cycle-check/registry key. The child then recomputed its OWN key (ambient-less) to push onto active_stack, so `child_key in active_stack` never matched: a real cycle escaped precise detection and degraded to the max-depth-32 guard with a misleading "Reduce nesting depth" message. Thread the parent-computed key into _compile_pipeline_fn as `precomputed_key` so the stacked, registry, and cycle-check keys are identical; the root still builds its own (ambient-less) key. Adds the suite's first cycle-detection tests: a direct self-reference and the propagate_config pass-through regression. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/tangle_cli/pipeline_compiler.py | 18 ++++- tests/test_pipeline_compiler.py | 67 +++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py index e34936e..87d0e91 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_compiler.py @@ -345,6 +345,7 @@ def _compile_pipeline_fn( is_root: bool, base_dir: Path, rebroadcast_overrides: Mapping[str, Any] | None = None, + precomputed_key: PipelineCompileKey | None = None, ) -> SubgraphArtifact: """Trace + emit ``pipeline_fn`` into an in-memory :class:`SubgraphArtifact`. @@ -402,8 +403,20 @@ def _compile_pipeline_fn( builder = trace_pipeline(pipeline_fn, cfg=cfg, inputs={}) body_dict, exempt_paths = emit_pipeline(builder) - # 3. Compute the canonical compile key for dedup / cycle detection. - key = _compile_key_for(pipeline_fn, cfg_path, overrides) + # 3. The canonical compile key for dedup / cycle detection. A child is + # compiled with the SAME key its parent already built for the cycle + # check and registry lookup (``precomputed_key``) — critically, that key + # folds in the ambient PASS-THROUGH context (broadcast keys the child + # does not declare). Recomputing it here would drop that ambient + # envelope, so the key pushed onto ``active_stack`` would no longer equal + # the key the parent tests with ``child_key in active_stack`` — a real + # cycle under ``propagate_config`` would then escape precise detection + # and degrade to the max-depth guard. The root has no parent (and no + # ambient), so it builds its own key from its own overrides. + if precomputed_key is not None: + key = precomputed_key + else: + key = _compile_key_for(pipeline_fn, cfg_path, overrides) # 4. @task local_from_python sidecar for THIS artifact (built, not # written). Each @task ref is rewritten to a pure @@ -691,6 +704,7 @@ def _process_subpipeline_children( is_root=False, base_dir=child_base_dir, rebroadcast_overrides={}, + precomputed_key=child_key, ) finally: if pushed_edge_override: diff --git a/tests/test_pipeline_compiler.py b/tests/test_pipeline_compiler.py index d3a9c7f..1c7ebf4 100644 --- a/tests/test_pipeline_compiler.py +++ b/tests/test_pipeline_compiler.py @@ -785,6 +785,73 @@ def test_compile_propagate_config_broadcasts_through_configless_intermediate(tmp validate_dehydrated_data(yaml.safe_load(p.read_text())) +# --------------------------------------------------------------------------- +# Cycle detection. A subpipeline that (transitively) reaches back to itself is +# a recursive Python pipeline, which cannot be compiled to a finite graph. It +# must be rejected with a precise "cycle detected" diagnostic — NOT allowed to +# recurse until the max-depth guard trips, whose "reduce nesting depth" advice +# is misleading for what is actually a cycle. + +_LOOP_SRC = ( + "from tangle_cli.python_pipeline import In, Out, pipeline, subpipeline\n" + "\n" + "@pipeline('Loop')\n" + "def loop(seed: In[str]) -> Out[str]:\n" + " # Self-reference => a 1-node cycle.\n" + " return subpipeline(loop).named('Recurse')(seed=seed)\n" +) + + +def test_compile_detects_self_referencing_cycle(tmp_path): + """A self-referencing subpipeline is reported as a cycle, precisely.""" + src = tmp_path / "loop_pipeline.py" + src.write_text(_LOOP_SRC) + + with pytest.raises(CompileError) as exc_info: + compile_pipeline(src, tmp_path / "compiled.yaml", pipeline_name="loop") + + msg = str(exc_info.value) + assert "nested pipeline cycle detected" in msg + assert "Loop" in msg + assert "max depth" not in msg # must not degrade to the depth guard + + +def test_compile_detects_cycle_under_propagate_config_passthrough(tmp_path): + """A cycle must still be caught precisely when the cyclic pipeline is + reached under a ``propagate_config`` ancestor that broadcasts a key the + cyclic pipeline does not declare (an ambient PASS-THROUGH key). + + Regression guard: the cycle-check key is built by the parent WITH the + ambient pass-through folded in, while a child used to push its OWN key + (ambient-less) onto ``active_stack``. Under a pass-through broadcast the two + keys diverged, so ``child_key in active_stack`` never matched and the SAME + cycle that is caught in isolation instead degraded to the max-depth guard + after 32 redundant levels. The fix threads the parent-computed key into the + child compile so the stacked, registry, and cycle-check keys are identical. + """ + src = tmp_path / "loop_pipeline.py" + src.write_text( + _LOOP_SRC + + "\n" + "@pipeline('Broadcast Root', config='root_config.yaml', propagate_config=True)\n" + "def broadcast_root(parent_wait_token: In[str], cfg) -> Out[str]:\n" + " # Broadcasts shared_key; `loop` doesn't declare it, so it flows\n" + " # PAST loop as ambient context at every self-edge.\n" + " return subpipeline(loop).named('Run Loop')(seed=parent_wait_token)\n" + ) + (tmp_path / "root_config.yaml").write_text("shared_key: broadcast-value\n") + + with pytest.raises(CompileError) as exc_info: + compile_pipeline( + src, tmp_path / "compiled.yaml", pipeline_name="broadcast_root" + ) + + msg = str(exc_info.value) + assert "nested pipeline cycle detected" in msg + assert "Broadcast Root" in msg and "Loop" in msg + assert "max depth" not in msg # the bug: cycle degraded to the depth guard + + # --------------------------------------------------------------------------- # @registered gen_config resolution. @registered references an EXISTING # gen_config.yaml and generates no sidecar of its own; the task's componentRef