From 8cc9c5b93863fdc013b861ab956e96194e5f37d5 Mon Sep 17 00:00:00 2001 From: Volv G Date: Thu, 24 Sep 2026 21:44:16 -0700 Subject: [PATCH] Author editor layout from Python: with_position and flow_direction (v0.1.22) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pipeline editor stores graph layout in ordinary annotations, so a Python author can already write it — as a hand-serialized JSON string under a key they have to remember. This adds the typed spelling: `.with_position(x, y)` on task and subpipeline handles, and `@pipeline(flow_direction=...)` for the root. Both are pure sugar: they route through `with_annotations` / the `annotations` mapping, so the compiled bundle is byte-identical to the hand-written form and the last write to a key wins regardless of which spelling made it. The key names, value format and validation live in a new stdlib-only `tangle_cli.editor_layout`, shared with `pipelines layout` and the runner's auto-layout gate so the three cannot drift. It sits outside `python_pipeline` because importing that package pulls in the authoring/codegen stack, which the layout and submit paths must not require; callers inject their own `error_cls`. --- README.md | 31 + .../tangle-cli/src/tangle_cli/__init__.py | 2 +- .../src/tangle_cli/editor_layout.py | 83 +++ .../src/tangle_cli/pipeline_runner.py | 3 +- .../tangle-cli/src/tangle_cli/pipelines.py | 3 +- .../src/tangle_cli/python_pipeline/errors.py | 9 + .../tangle_cli/python_pipeline/pipeline.py | 21 +- .../src/tangle_cli/python_pipeline/ref.py | 34 +- .../tangle_cli/python_pipeline/subpipeline.py | 30 +- pyproject.toml | 2 +- tests/test_editor_layout.py | 536 ++++++++++++++++++ tests/test_packaging.py | 2 +- uv.lock | 2 +- 13 files changed, 748 insertions(+), 10 deletions(-) create mode 100644 packages/tangle-cli/src/tangle_cli/editor_layout.py create mode 100644 tests/test_editor_layout.py diff --git a/README.md b/README.md index 5be41d2..595025d 100644 --- a/README.md +++ b/README.md @@ -603,6 +603,37 @@ The rules live in one place, `tangle_cli.schema_validation`: `check_annotations( A distribution that reads these annotations from its own config file should call `check_annotations(mapping, policy=CALLER_ANNOTATION_POLICY, error_cls=...)` at config-parse time — passing its own error type and adding the config path and key to the message — so one user mistake produces one diagnostic instead of two competing ones. The compiler's own call is then the backstop for anything arriving by another route. +##### Editor layout (`editor.*`) + +The pipeline editor stores graph layout in ordinary annotations, so layout is authorable from Python. `.with_position(x, y)` on a task or subpipeline handle writes `editor.position`, and `@pipeline(flow_direction=...)` writes the root `editor.flow-direction`: + +```python +@pipeline("Daily Pulse", flow_direction="left-to-right") +def daily_pulse() -> Out[str]: + scrape = SCRAPE.named("Scrape").with_position(0, 0)() + judge = JUDGE.named("Judge").with_position(300, 0)(rows=scrape.rows) + return judge.report +``` + +Both are pure sugar over annotations you can still write by hand, and they compile to byte-identical output: + +```python +# Equivalent, and what the sugar emits: +SCRAPE.with_annotations({"editor.position": '{"x": 0, "y": 0}'}) +@pipeline("Daily Pulse", annotations={"editor.flow-direction": "left-to-right"}) +``` + +- **Value format is the editor's, not ours.** `editor.position` is a JSON object *string* (`'{"x": 300, "y": 120}'`) because the editor writes `JSON.stringify` and reads `JSON.parse`. Coordinates may be negative. Optional `width=` / `height=` keywords add the node-size fields the editor also reads; they are omitted entirely when not given. +- **`flow_direction`** accepts `"left-to-right"` (what the editor writes, and what it renders today) and `"top-to-bottom"` (legacy, accepted so existing documents stay expressible). There is no default: omitting the keyword emits no annotation. +- **Last write wins**, one rule for both spellings. `.with_position(...)` *is* a `.with_annotations({"editor.position": ...})` call, so whichever runs last sets the value, and unrelated annotations are untouched. Within a single `@pipeline(...)`, the typed `flow_direction` keyword is applied after the `annotations` mapping and therefore wins on that key. +- **Validated without echoing values.** A non-numeric, `bool`, `NaN` or infinite coordinate, or an unknown flow direction, raises `InvalidEditorLayoutError` (a `CompileError`) at the call, before anything is written. Diagnostics name the coordinate or list the allowed directions. +- **Positions are descriptive.** They are task annotations, not part of any `componentRef`, so they never change component digests, compile identity, or cache behaviour. On a subpipeline handle the position applies to the parent task; the child sidecar is byte-identical either way. +- **Auto-layout interaction.** At submit time the runner lays a graph out only when no task carries a *non-zero* position, so an explicit `.with_position(...)` suppresses auto-layout and `--force-layout` overrides that. A graph positioned entirely at `(0, 0)` still counts as unpositioned and is laid out. `tangle sdk pipelines layout` writes the same canonical value this sugar writes. + +Graph **inputs** and **outputs** carry `editor.position` in the same way, and the editor reads it there, but there is no Python authoring surface for a graph input/output object yet — inputs come from the `In[T]` signature and outputs from the return annotation, neither of which has a place to hang layout. That is follow-up work tied to a public `graph_input()` / `graph_output()` API; until then, position inputs by editing the YAML or by using the editor. + +The key names, the serialized format, and the validation live in `tangle_cli.editor_layout` (`POSITION_ANNOTATION`, `FLOW_DIRECTION_ANNOTATION`, `FLOW_DIRECTIONS`, `position_annotation_value`, `validate_flow_direction`), which the CLI's own `pipelines layout` and the runner's auto-layout gate share, so a tool emitting layout cannot drift from what the editor reads. That module is stdlib-only and lives outside `python_pipeline` on purpose, so layout and submit paths do not pull in the authoring/codegen stack; it raises whatever `error_cls` the caller injects, and the authoring surfaces inject `InvalidEditorLayoutError`. + ##### Conditional task execution Pipeline inputs used as conditions are ordinary `In[str]` values; there is no special conditional input annotation. Pass the value through the reserved task-call metadata keyword `is_enabled=`: diff --git a/packages/tangle-cli/src/tangle_cli/__init__.py b/packages/tangle-cli/src/tangle_cli/__init__.py index ceba2aa..69f5486 100644 --- a/packages/tangle-cli/src/tangle_cli/__init__.py +++ b/packages/tangle-cli/src/tangle_cli/__init__.py @@ -14,6 +14,6 @@ try: __version__ = metadata_version("tangle-cli") except PackageNotFoundError: - __version__ = "0.1.21" + __version__ = "0.1.22" __all__ = ["TangleDynamicDiscoveryClient", "__version__"] diff --git a/packages/tangle-cli/src/tangle_cli/editor_layout.py b/packages/tangle-cli/src/tangle_cli/editor_layout.py new file mode 100644 index 0000000..d1985a0 --- /dev/null +++ b/packages/tangle-cli/src/tangle_cli/editor_layout.py @@ -0,0 +1,83 @@ +"""Canonical ``editor.*`` layout keys, value formats and validation. + +Layout is descriptive: it never affects execution, component digests or the +cache. Stdlib-only and outside :mod:`tangle_cli.python_pipeline` so the +layout command and the submit-time gate import it without the authoring +stack; callers inject their own ``error_cls``. +""" +from __future__ import annotations + +import json +import math +from typing import Any + +#: Task / graph-input / graph-output coordinates. The value is a JSON object +#: STRING (the editor stringifies and parses it), ``{"x", "y"}`` plus optional +#: ``"width"``/``"height"``; coordinates may be negative. +POSITION_ANNOTATION = "editor.position" + +#: Root ``metadata.annotations`` rendering direction. +FLOW_DIRECTION_ANNOTATION = "editor.flow-direction" + +#: Accepted ``editor.flow-direction`` values; ``top-to-bottom`` is legacy. +FLOW_DIRECTIONS: tuple[str, ...] = ("left-to-right", "top-to-bottom") + + +def position_annotation_value( + x: float, + y: float, + *, + width: float | None = None, + height: float | None = None, + error_cls: type[Exception] = ValueError, +) -> str: + """Return the canonical ``editor.position`` value for ``x`` / ``y``. + + Key order is ``x, y, width, height``; absent dimensions are omitted + rather than written as null. + + Raises: + error_cls: If a coordinate is not a finite real number. Messages name + the coordinate and its type, never the value. + """ + + position: dict[str, float] = { + "x": _finite_number("x", x, error_cls), + "y": _finite_number("y", y, error_cls), + } + if width is not None: + position["width"] = _finite_number("width", width, error_cls) + if height is not None: + position["height"] = _finite_number("height", height, error_cls) + return json.dumps(position) + + +def validate_flow_direction( + value: Any, *, error_cls: type[Exception] = ValueError +) -> str: + """Return ``value`` if it is a supported ``editor.flow-direction``. + + Raises: + error_cls: If it is not one of :data:`FLOW_DIRECTIONS`. + """ + + if value in FLOW_DIRECTIONS: + return str(value) + allowed = ", ".join(repr(direction) for direction in FLOW_DIRECTIONS) + raise error_cls( + f"flow_direction must be one of {allowed}; got " + f"{type(value).__name__}." + ) + + +def _finite_number(name: str, value: Any, error_cls: type[Exception]) -> float: + # bool is an int subclass, and True would silently serialize as 1. + if isinstance(value, bool) or not isinstance(value, (int, float)): + raise error_cls( + f"position coordinate {name!r} must be a real number; got " + f"{type(value).__name__}." + ) + if not math.isfinite(value): + # json.dumps emits bare NaN / Infinity, which JSON.parse rejects. + raise error_cls(f"position coordinate {name!r} must be finite.") + return value diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_runner.py b/packages/tangle-cli/src/tangle_cli/pipeline_runner.py index 3132cec..240a8a0 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_runner.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_runner.py @@ -16,6 +16,7 @@ from pathlib import Path from typing import Any, Mapping +from .editor_layout import POSITION_ANNOTATION from .pipeline_run_manager import ( PipelineRunContext, PipelineRunError, @@ -124,7 +125,7 @@ def has_layout(self, pipeline_spec: Mapping[str, Any]) -> bool: if not isinstance(task, Mapping): continue annotations = task.get("annotations", {}) - position = annotations.get("editor.position") if isinstance(annotations, Mapping) else None + position = annotations.get(POSITION_ANNOTATION) if isinstance(annotations, Mapping) else None if isinstance(position, str): try: import json diff --git a/packages/tangle-cli/src/tangle_cli/pipelines.py b/packages/tangle-cli/src/tangle_cli/pipelines.py index 5339dc1..0c6eb05 100644 --- a/packages/tangle-cli/src/tangle_cli/pipelines.py +++ b/packages/tangle-cli/src/tangle_cli/pipelines.py @@ -17,6 +17,7 @@ import yaml +from .editor_layout import POSITION_ANNOTATION from .pipeline_spec_utils import _extract_task_output_refs from .pipeline_validation import ( PipelineValidationError, @@ -31,8 +32,6 @@ if TYPE_CHECKING: from .pipeline_compiler import CompileResult -POSITION_ANNOTATION = "editor.position" - __all__ = [ "HydrateResult", "LayoutResult", diff --git a/packages/tangle-cli/src/tangle_cli/python_pipeline/errors.py b/packages/tangle-cli/src/tangle_cli/python_pipeline/errors.py index 66f3bf6..354c42d 100644 --- a/packages/tangle-cli/src/tangle_cli/python_pipeline/errors.py +++ b/packages/tangle-cli/src/tangle_cli/python_pipeline/errors.py @@ -29,6 +29,15 @@ class InvalidArgumentTypeError(CompileError): """Raised on an argument value with no supported emit dispatch.""" +class InvalidEditorLayoutError(CompileError): + """Raised on a malformed value passed to the typed layout sugar. + + Covers ``.with_position`` coordinates and + ``@pipeline(flow_direction=...)`` only; hand-written + ``.with_annotations`` values still pass through unchecked. + """ + + class InvalidPipelineAnnotationsError(CompileError): """Raised on a malformed caller-supplied ``pipeline_annotations`` mapping. diff --git a/packages/tangle-cli/src/tangle_cli/python_pipeline/pipeline.py b/packages/tangle-cli/src/tangle_cli/python_pipeline/pipeline.py index dd7e81a..064860d 100644 --- a/packages/tangle-cli/src/tangle_cli/python_pipeline/pipeline.py +++ b/packages/tangle-cli/src/tangle_cli/python_pipeline/pipeline.py @@ -13,7 +13,13 @@ from pathlib import Path from typing import Any, Callable +from tangle_cli.editor_layout import ( + FLOW_DIRECTION_ANNOTATION, + validate_flow_direction, +) + from . import emit +from .errors import InvalidEditorLayoutError from .graph import GraphBuilder @@ -130,6 +136,7 @@ def pipeline( config: str | None = None, annotations: dict[str, Any] | None = None, task_annotations: dict[str, Any] | None = None, + flow_direction: str | None = None, output_name: str = "wait_for_output", propagate_config: bool = False, ) -> Callable[[Callable[..., Any]], PipelineFn]: @@ -149,6 +156,10 @@ def pipeline( time so ``--override key=value`` pairs can merge in. annotations: ``metadata.annotations`` block (e.g. ``version``, ``author``). + flow_direction: Editor rendering direction, written as the + ``editor.flow-direction`` root annotation + (``"left-to-right"`` / ``"top-to-bottom"``). Sugar over + ``annotations``; applied last, so it wins on that key. task_annotations: Per-task default annotations applied to every task in the pipeline. Accepted for API completeness but not wired through in MVP — the PoC sets per-task annotations @@ -168,12 +179,20 @@ def decorator(fn: Callable[..., Any]) -> PipelineFn: # Tests inside generated modules may not have a real file. caller_dir = None + merged_annotations = dict(annotations or {}) + if flow_direction is not None: + # Assigned after the mapping: the typed keyword wins, and an + # existing key keeps its position in key order. + merged_annotations[FLOW_DIRECTION_ANNOTATION] = validate_flow_direction( + flow_direction, error_cls=InvalidEditorLayoutError + ) + return PipelineFn( fn=fn, name=name, description=description, config_path=config, - annotations=dict(annotations or {}), + annotations=merged_annotations, task_annotations=dict(task_annotations or {}), caller_dir=caller_dir, output_name=output_name, diff --git a/packages/tangle-cli/src/tangle_cli/python_pipeline/ref.py b/packages/tangle-cli/src/tangle_cli/python_pipeline/ref.py index e86b55c..0276d92 100644 --- a/packages/tangle-cli/src/tangle_cli/python_pipeline/ref.py +++ b/packages/tangle-cli/src/tangle_cli/python_pipeline/ref.py @@ -15,7 +15,9 @@ from pathlib import Path from typing import Any -from .errors import CompileError +from tangle_cli.editor_layout import POSITION_ANNOTATION, position_annotation_value + +from .errors import CompileError, InvalidEditorLayoutError from .graph import EXECUTION_OPTIONS_UNSET, IS_ENABLED_UNSET _UNWRAPPED_KEY_RE = re.compile(r"^[A-Za-z0-9_-]+$") @@ -222,6 +224,36 @@ def with_annotations(self, ann: dict[str, Any]) -> "CallableRef": merged[k] = v # type: ignore[assignment] return self._replace(annotations=merged) + def with_position( + self, + x: float, + y: float, + *, + width: float | None = None, + height: float | None = None, + ) -> "CallableRef": + """Return a new CallableRef carrying the ``editor.position`` + annotation for ``x``/``y``, with optional node ``width``/``height``. + + Sugar over :meth:`with_annotations`, which is what makes the last + write to ``editor.position`` win regardless of spelling. + + Raises: + InvalidEditorLayoutError: On a non-numeric or non-finite + coordinate. + """ + return self.with_annotations( + { + POSITION_ANNOTATION: position_annotation_value( + x, + y, + width=width, + height=height, + error_cls=InvalidEditorLayoutError, + ) + } + ) + # ------------------------------------------------------------------ # @task codegen — materialize() writes the component YAML. diff --git a/packages/tangle-cli/src/tangle_cli/python_pipeline/subpipeline.py b/packages/tangle-cli/src/tangle_cli/python_pipeline/subpipeline.py index 823f953..2345754 100644 --- a/packages/tangle-cli/src/tangle_cli/python_pipeline/subpipeline.py +++ b/packages/tangle-cli/src/tangle_cli/python_pipeline/subpipeline.py @@ -36,7 +36,9 @@ from dataclasses import dataclass, field, replace from typing import TYPE_CHECKING, Any -from .errors import CompileError +from tangle_cli.editor_layout import POSITION_ANNOTATION, position_annotation_value + +from .errors import CompileError, InvalidEditorLayoutError if TYPE_CHECKING: # pragma: no cover from .pipeline import PipelineFn @@ -90,6 +92,32 @@ def with_annotations(self, ann: dict[str, Any]) -> "SubpipelineRef": merged[k] = v # type: ignore[assignment] return replace(self, annotations=merged) + def with_position( + self, + x: float, + y: float, + *, + width: float | None = None, + height: float | None = None, + ) -> "SubpipelineRef": + """Return a new handle carrying ``editor.position`` for ``x``/``y``. + + Positions the PARENT task in the enclosing graph — the same scope + :meth:`with_annotations` writes to; child layout is authored inside + the child pipeline. Otherwise as :meth:`CallableRef.with_position`. + """ + return self.with_annotations( + { + POSITION_ANNOTATION: position_annotation_value( + x, + y, + width=width, + height=height, + error_cls=InvalidEditorLayoutError, + ) + } + ) + def override_config(self, **kwargs: Any) -> "SubpipelineRef": """Return a new handle with compile-time cfg overrides for the direct child merged in (later calls win on conflict). diff --git a/pyproject.toml b/pyproject.toml index 013e106..53f9a69 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "tangle-cli" -version = "0.1.21" +version = "0.1.22" description = "CLI for Tangle, the open-source ML pipeline orchestration platform" readme = "README.md" authors = [ diff --git a/tests/test_editor_layout.py b/tests/test_editor_layout.py new file mode 100644 index 0000000..efd5092 --- /dev/null +++ b/tests/test_editor_layout.py @@ -0,0 +1,536 @@ +"""Typed sugar for the ``editor.*`` layout annotations. + +``.with_position(x, y)`` and ``@pipeline(flow_direction=...)`` restate what +an author can already hand-write as annotations. The contract pinned here: + +* pure sugar, proven by BYTE identity of the whole compiled bundle against + the hand-written form — the property a YAML→Python decompiler needs; +* chainable, immutable, and last-write-wins on ``editor.position`` + regardless of spelling (the sugar routes through ``with_annotations``); +* validation refuses what the editor could not parse back, and never echoes + the rejected value; +* layout is descriptive: componentRefs, sidecar bytes and the cache are + untouched; +* the runner's auto-layout gate only counts a NON-ZERO position as layout. +""" + +from __future__ import annotations + +import json +import textwrap +from pathlib import Path + +import pytest +import yaml + +from tangle_cli.editor_layout import ( + FLOW_DIRECTION_ANNOTATION, + FLOW_DIRECTIONS, + POSITION_ANNOTATION, + position_annotation_value, + validate_flow_direction, +) +from tangle_cli.pipeline_compiler import compile_pipeline +from tangle_cli.pipeline_runner import PipelineRunnerHooks +from tangle_cli.pipelines import POSITION_ANNOTATION as PIPELINES_POSITION_ANNOTATION +from tangle_cli.python_pipeline import pipeline, ref +from tangle_cli.python_pipeline.errors import CompileError, InvalidEditorLayoutError + + +# --------------------------------------------------------------------------- +# Compile helpers. Each compile runs in its own directory: the loader caches +# modules by name, and output paths are embedded in the bundle. + + +_TASK_SOURCE = ''' +from tangle_cli.python_pipeline import Out, pipeline, task + + +@task(image="python:3.12") +def greet(greeting: str = "hi"): + """Write a greeting. + + Metadata: + Name: Greet + """ + print(greeting) + + +@pipeline(__DECORATOR_ARGS__) +def laid_out() -> Out[str]: + run_greet = greet__TASK_SUFFIX__() + return run_greet +''' + + +def _source(*, decorator_args: str = '"Laid Out"', task_suffix: str = "") -> str: + return textwrap.dedent( + _TASK_SOURCE.replace("__DECORATOR_ARGS__", decorator_args).replace( + "__TASK_SUFFIX__", task_suffix + ) + ) + + +def _compile( + tmp_path: Path, source: str, case: str, *, pipeline_name: str | None = None +) -> Path: + """Compile ``source`` in its own directory; return the output path.""" + case_dir = tmp_path / case + case_dir.mkdir(parents=True, exist_ok=True) + script = case_dir / "pipeline.py" + script.write_text(source, encoding="utf-8") + out = case_dir / "compiled.yaml" + compile_pipeline(script, out, pipeline_name=pipeline_name) + return out + + +def _bundle(out: Path) -> dict[str, bytes]: + """Every file the compile wrote, keyed by name relative to its directory.""" + return { + p.name: p.read_bytes() + for p in sorted(out.parent.rglob("*")) + if p.is_file() and p.suffix in {".yaml", ".yml"} + } + + +def _task_annotations(out: Path, task_id: str = "Run Greet") -> dict: + data = yaml.safe_load(out.read_text(encoding="utf-8")) + return data["implementation"]["graph"]["tasks"][task_id].get("annotations", {}) + + +def _root_annotations(out: Path) -> dict: + data = yaml.safe_load(out.read_text(encoding="utf-8")) + return data.get("metadata", {}).get("annotations", {}) + + +# --------------------------------------------------------------------------- +# Pure sugar: byte identity with the hand-written annotation. + + +def test_a_position_compiles_to_the_hand_written_annotation(tmp_path): + """Byte identity across the bundle, not just an equal parsed value.""" + sugar = _compile( + tmp_path, _source(task_suffix=".with_position(300, 120)"), "sugar" + ) + manual = _compile( + tmp_path, + _source( + task_suffix=( + ".with_annotations({\"editor.position\": " + "'{\"x\": 300, \"y\": 120}'})" + ) + ), + "manual", + ) + + assert _bundle(sugar) == _bundle(manual) + assert _task_annotations(sugar) == {"editor.position": '{"x": 300, "y": 120}'} + + +def test_a_flow_direction_compiles_to_the_hand_written_annotation(tmp_path): + """Same for the root keyword, including its place in key order.""" + sugar = _compile( + tmp_path, + _source( + decorator_args=( + '"Laid Out", annotations={"sdk": "x"}, ' + 'flow_direction="left-to-right"' + ) + ), + "sugar", + ) + manual = _compile( + tmp_path, + _source( + decorator_args=( + '"Laid Out", annotations={"sdk": "x", ' + '"editor.flow-direction": "left-to-right"}' + ) + ), + "manual", + ) + + assert _bundle(sugar) == _bundle(manual) + assert list(_root_annotations(sugar)) == ["sdk", "editor.flow-direction"] + + +def test_the_position_value_matches_the_corpus_byte_for_byte(): + """The serialized form itself: a JSON object STRING, ``x`` then ``y``, + ``json.dumps`` spacing — what ``pipelines layout`` and the corpus use.""" + assert position_annotation_value(300, 120) == '{"x": 300, "y": 120}' + assert position_annotation_value(-10, 80) == '{"x": -10, "y": 80}' + assert position_annotation_value(1.5, 2.5) == '{"x": 1.5, "y": 2.5}' + assert ( + position_annotation_value(10, 20, width=250, height=100) + == '{"x": 10, "y": 20, "width": 250, "height": 100}' + ) + # Optional dimensions are OMITTED, not emitted as null: the editor reads + # a missing width as "use the default", but a null would be a number-ish + # field it has to reject. + assert "width" not in position_annotation_value(10, 20) + assert json.loads(position_annotation_value(10, 20)) == {"x": 10, "y": 20} + + +def test_the_shared_module_stays_importable_without_the_authoring_stack(): + """Importing ``python_pipeline`` pulls the codegen stack and its optional + dependencies, which a minimal install does not have — and the layout + command and submit-time gate both read these constants.""" + import subprocess + import sys + + result = subprocess.run( + [ + sys.executable, + "-c", + "import sys; import tangle_cli.editor_layout as m; " + "assert not [n for n in sys.modules " + "if n.startswith('tangle_cli.python_pipeline')], " + "sorted(n for n in sys.modules if n.startswith('tangle_cli')); " + "print(m.position_annotation_value(1, 2))", + ], + capture_output=True, + text=True, + ) + + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == '{"x": 1, "y": 2}' + + +def test_the_shared_helpers_default_to_the_stdlib_error(): + """``error_cls`` injection keeps the shared module independent of the + authoring error hierarchy.""" + with pytest.raises(ValueError): + position_annotation_value("x", 0) + with pytest.raises(ValueError): + validate_flow_direction("sideways") + + +def test_the_authoring_surfaces_inject_the_precise_error_type(): + with pytest.raises(InvalidEditorLayoutError): + ref(url="file://./greet.yaml").with_position("x", 0) + + with pytest.raises(InvalidEditorLayoutError): + + @pipeline("P", flow_direction="sideways") + def a_pipeline(): + return None + + +def test_the_key_names_have_one_definition(): + """``pipelines`` (CLI layout) and ``pipeline_runner`` (auto-layout gate) + read the same constant the sugar writes, so the three cannot drift.""" + assert POSITION_ANNOTATION == "editor.position" + assert FLOW_DIRECTION_ANNOTATION == "editor.flow-direction" + assert PIPELINES_POSITION_ANNOTATION is POSITION_ANNOTATION + + +# --------------------------------------------------------------------------- +# Handle semantics: chaining, immutability, collisions. + + +def test_position_chains_with_the_other_combinators(tmp_path): + """Order does not matter, and the task ID still comes from ``.named``.""" + out = _compile( + tmp_path, + _source(task_suffix='.named("Greet Step").with_position(40, 50)'), + "chain", + ) + reversed_order = _compile( + tmp_path, + _source(task_suffix='.with_position(40, 50).named("Greet Step")'), + "chain_reversed", + ) + + assert _task_annotations(out, "Greet Step") == { + "editor.position": '{"x": 40, "y": 50}' + } + assert _bundle(out) == _bundle(reversed_order) + + +def test_position_returns_a_new_handle_and_leaves_the_original_alone(): + """Immutable composition: a shared base ref can be positioned twice.""" + base = ref(url="file://./greet.yaml") + left = base.with_position(0, 0) + right = base.with_position(100, 200) + + assert base.annotations in (None, {}) + assert left is not base and right is not base + assert left.annotations == {"editor.position": '{"x": 0, "y": 0}'} + assert right.annotations == {"editor.position": '{"x": 100, "y": 200}'} + + +def test_the_last_write_to_the_position_key_wins_either_way(): + """Neither spelling shadows the other; they are simply ordered.""" + handle = ref(url="file://./greet.yaml") + + sugar_last = handle.with_annotations( + {"editor.position": '{"x": 1, "y": 1}'} + ).with_position(2, 2) + manual_last = handle.with_position(2, 2).with_annotations( + {"editor.position": '{"x": 1, "y": 1}'} + ) + + assert sugar_last.annotations["editor.position"] == '{"x": 2, "y": 2}' + assert manual_last.annotations["editor.position"] == '{"x": 1, "y": 1}' + + +def test_a_position_leaves_unrelated_annotations_alone(): + handle = ref(url="file://./greet.yaml").with_annotations( + {"cloud-pipelines.net/launchers/generic/resources.memory": "50Gi"} + ) + + positioned = handle.with_position(10, 20) + + assert positioned.annotations == { + "cloud-pipelines.net/launchers/generic/resources.memory": "50Gi", + "editor.position": '{"x": 10, "y": 20}', + } + + +def test_the_typed_flow_direction_wins_over_the_same_key_in_annotations(): + """Applied last. Not an error: a shared annotations dict plus an explicit + keyword means the explicit one.""" + + @pipeline( + "P", + annotations={"editor.flow-direction": "top-to-bottom"}, + flow_direction="left-to-right", + ) + def a_pipeline(): + return None + + assert a_pipeline.annotations == {"editor.flow-direction": "left-to-right"} + + +def test_flow_direction_is_optional_and_absent_by_default(): + """No keyword means no annotation; inventing a default would change the + emitted document for every existing pipeline.""" + + @pipeline("P") + def a_pipeline(): + return None + + assert a_pipeline.annotations == {} + + +# --------------------------------------------------------------------------- +# Validation. A rejected value never appears in the diagnostic. + + +@pytest.mark.parametrize( + "kwargs", + [ + {"x": "300", "y": 120}, + {"x": 300, "y": "120"}, + {"x": None, "y": 0}, + {"x": True, "y": 0}, + {"x": 0, "y": False}, + {"x": [300], "y": 120}, + {"x": {"x": 1}, "y": 120}, + {"x": float("nan"), "y": 0}, + {"x": 0, "y": float("inf")}, + {"x": 0, "y": float("-inf")}, + ], +) +def test_an_unusable_coordinate_is_refused_without_echoing_it(kwargs): + """Each of these is unparseable JSON or silently misread (``True`` -> 1).""" + with pytest.raises(InvalidEditorLayoutError) as exc: + position_annotation_value(**kwargs, error_cls=InvalidEditorLayoutError) + + message = str(exc.value) + assert "'x'" in message or "'y'" in message + # A type NAME is fine ("got NoneType"); the VALUE never appears. + for value in kwargs.values(): + if isinstance(value, (str, list, dict)): + assert str(value) not in message + + +def test_an_unusable_dimension_is_refused(): + with pytest.raises(InvalidEditorLayoutError) as exc: + position_annotation_value( + 0, 0, width="wide", error_cls=InvalidEditorLayoutError + ) + + assert "'width'" in str(exc.value) + assert "wide" not in str(exc.value) + + +def test_a_bad_coordinate_fails_before_anything_is_written(tmp_path): + """Validation is at the handle call, not at emit: no partial bundle.""" + case_dir = tmp_path / "bad" + case_dir.mkdir() + script = case_dir / "pipeline.py" + script.write_text(_source(task_suffix='.with_position("300", 120)'), "utf-8") + + with pytest.raises(InvalidEditorLayoutError): + compile_pipeline(script, case_dir / "out" / "compiled.yaml") + + assert not (case_dir / "out").exists() + + +@pytest.mark.parametrize("direction", FLOW_DIRECTIONS) +def test_every_documented_flow_direction_is_accepted(direction): + assert validate_flow_direction(direction) == direction + + +@pytest.mark.parametrize( + "direction", ["LEFT-TO-RIGHT", "left_to_right", "diagonal", "", None, 1] +) +def test_an_unknown_flow_direction_is_refused_with_the_allowed_values(direction): + with pytest.raises(InvalidEditorLayoutError) as exc: + validate_flow_direction(direction, error_cls=InvalidEditorLayoutError) + + message = str(exc.value) + assert "'left-to-right'" in message and "'top-to-bottom'" in message + if isinstance(direction, str) and direction: + assert direction not in message + + +def test_layout_errors_are_compile_errors(): + """Existing ``CompileError`` handlers keep working.""" + assert issubclass(InvalidEditorLayoutError, CompileError) + + +# --------------------------------------------------------------------------- +# Subpipelines. + + +def test_a_subpipeline_task_can_be_positioned(tmp_path): + """The position lands on the PARENT task; the child sidecar's name and + bytes are untouched. + + Both compiles reuse the SAME source and output paths: the sidecar name + hashes compile identity, which includes the source path, so separate + directories would differ for reasons unrelated to the position. + """ + child_and_parent = ''' +from tangle_cli.python_pipeline import Out, pipeline, subpipeline, task + + +@task(image="python:3.12") +def greet(greeting: str = "hi"): + """Write a greeting. + + Metadata: + Name: Greet + """ + print(greeting) + + +@pipeline("Child") +def child() -> Out[str]: + run_greet = greet() + return run_greet + + +@pipeline("Parent") +def parent() -> Out[str]: + run_child = subpipeline(child)__SUFFIX__() + return run_child +''' + + script = tmp_path / "pipeline.py" + out = tmp_path / "out" / "compiled.yaml" + + script.write_text( + textwrap.dedent(child_and_parent).replace("__SUFFIX__", ""), encoding="utf-8" + ) + compile_pipeline(script, out, pipeline_name="parent") + plain_children = { + name: data for name, data in _bundle(out).items() if name.startswith("child-") + } + assert plain_children, "expected a child sidecar to compare" + + script.write_text( + textwrap.dedent(child_and_parent).replace( + "__SUFFIX__", ".with_position(500, 60)" + ), + encoding="utf-8", + ) + compile_pipeline(script, out, pipeline_name="parent") + + assert _task_annotations(out, "Run Child") == { + "editor.position": '{"x": 500, "y": 60}' + } + positioned_children = { + name: data for name, data in _bundle(out).items() if name.startswith("child-") + } + assert positioned_children == plain_children + + +def test_a_position_does_not_change_the_component_reference(tmp_path): + """Only the task's own annotations differ between the two compiles.""" + positioned = _compile( + tmp_path, _source(task_suffix=".with_position(300, 120)"), "with_pos" + ) + plain = _compile(tmp_path, _source(), "without_pos") + + def _task(out: Path) -> dict: + data = yaml.safe_load(out.read_text(encoding="utf-8")) + return data["implementation"]["graph"]["tasks"]["Run Greet"] + + positioned_task, plain_task = _task(positioned), _task(plain) + assert positioned_task["componentRef"] == plain_task["componentRef"] + assert {k: v for k, v in positioned_task.items() if k != "annotations"} == { + k: v for k, v in plain_task.items() if k != "annotations" + } + assert "annotations" not in plain_task + + +# --------------------------------------------------------------------------- +# Auto-layout interaction. + + +def test_an_explicit_position_suppresses_auto_layout(tmp_path): + """The runner relayouts only a graph it considers unpositioned.""" + hooks = PipelineRunnerHooks() + positioned = yaml.safe_load( + _compile( + tmp_path, _source(task_suffix=".with_position(300, 120)"), "auto_pos" + ).read_text(encoding="utf-8") + ) + plain = yaml.safe_load( + _compile(tmp_path, _source(), "auto_plain").read_text(encoding="utf-8") + ) + + assert hooks.has_layout(positioned) is True + assert hooks.has_layout(plain) is False + + common = { + "pipeline_path": "p.yaml", + "effective_path": None, + "skip_layout": False, + "layout_algorithm": None, + } + assert hooks.should_apply_layout(positioned, force_layout=False, **common) is False + assert hooks.should_apply_layout(plain, force_layout=False, **common) is True + # Explicit force still wins: the author asked for a relayout. + assert hooks.should_apply_layout(positioned, force_layout=True, **common) is True + + +def test_a_position_at_the_origin_does_not_count_as_layout(tmp_path): + """Pre-existing runner semantics, pinned because the sugar makes ``(0, 0)`` + easy to write: an all-origin graph is still auto-laid-out.""" + hooks = PipelineRunnerHooks() + origin = yaml.safe_load( + _compile( + tmp_path, _source(task_suffix=".with_position(0, 0)"), "auto_origin" + ).read_text(encoding="utf-8") + ) + + assert hooks.has_layout(origin) is False + + +def test_the_cli_layout_writes_what_the_sugar_writes(tmp_path): + """A laid-out document must round-trip back through Python.""" + from tangle_cli.pipelines import layout_pipeline_spec + + spec = yaml.safe_load( + _compile(tmp_path, _source(), "for_layout").read_text(encoding="utf-8") + ) + layout_pipeline_spec(spec) + + written = spec["implementation"]["graph"]["tasks"]["Run Greet"]["annotations"][ + POSITION_ANNOTATION + ] + assert written == position_annotation_value(0, 0) diff --git a/tests/test_packaging.py b/tests/test_packaging.py index 4c91c75..93dc39c 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -183,7 +183,7 @@ def test_tangle_cli_wheel_supports_expert_no_deps_import_path_without_tangle_api requires_dist = [line for line in metadata.splitlines() if line.startswith("Requires-Dist: ")] assert not any(name.startswith("tangle_api/") for name in names) assert "tangle_cli/openapi/openapi.json" not in names - assert "Version: 0.1.21" in metadata + assert "Version: 0.1.22" in metadata assert "Requires-Dist: tangle-api==0.1.1" in requires_dist assert not any("extra == 'native'" in line for line in requires_dist) assert "Provides-Extra: native" in metadata diff --git a/uv.lock b/uv.lock index 55cefa8..7dc3fc4 100644 --- a/uv.lock +++ b/uv.lock @@ -2083,7 +2083,7 @@ requires-dist = [{ name = "pydantic", specifier = ">=2.0" }] [[package]] name = "tangle-cli" -version = "0.1.21" +version = "0.1.22" source = { editable = "." } dependencies = [ { name = "cloud-pipelines" },