From c5663ceb282f8eaa5c4d24acd04c7f8fcbfa5ae7 Mon Sep 17 00:00:00 2001 From: EiffL Date: Tue, 25 Aug 2026 18:50:40 +0200 Subject: [PATCH 1/7] One output is one file, named by the spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An output was a directory a recipe filled, so nothing declared what was in it. A real agent-run project put chain.npz and run_config.json in one output, with the filename contract living only inside three consumer scripts; a recipe that exited 0 having written nothing committed a well-formed digest of an empty directory; and MySTRA had to guess which file in a figure's directory was the figure. ASTRA now declares the missing half (Output.format, astra-spec 0.0.14 / astra-tools 0.2.17), the counterpart of Input.source having always carried a path with an extension. So an output becomes exactly one file at a spec-derived path, and results live beside the astra.yaml that declares them: /results///. /results///..manifest.json An inline sub-analysis shares its parent's home and takes a scope directory; one declared with `path:` is a self-similar analysis with its own home and, where the parent universe names one, its own universe id. Only the path nests — graph keys and CLI targets stay the qualified id. The multiplicity had nowhere else to come from: there is no directory to compose a second name into, so `savefig(args.out)` writes exactly one file at exactly the declared path. Consumers are handed the upstream's file rather than a directory to search. Refusals over guesses, all before anything is spent: an output with no `format`, two tasks composing one path, a sub-analysis home escaping the project, a `universe:` naming a file that is not there, a payload that is not a regular file, and a manifest the attributes would route into the annex. `format` is framed into definition_version rather than left to arrive through the rendered {output} path — a recipe need not use that placeholder (evals/tasks/snae does not), and one that does not would keep its digest across a re-declared serialization while the sidecar, named from the id alone, did not move either. data_version drops the `file:` frame, so an output's digest is the number sha256sum prints and the one the crate publishes — one number instead of two. Only directory-valued declared inputs stay framed. The sandbox scope becomes the directory an output lands in, reversing the hardening pass's per-output narrowing: a mechanism can only grant creating a file through the directory that will hold it, and Landlock masks a non-directory grant to _FILE_ONLY_BITS. Outputs declared side by side are mutually writable again; data_version remains what answers for an output's bytes. `results/**` stays in the template though `**/results/**` subsumes it: a managed line dropped from the template stops being ranked, which blinds gitattributes_disorder to the trap it exists for. No migration. Nothing looks for .lightcone-manifest.json and nothing detects directory-shaped outputs from a previous engine. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PybSgBiP8pjZeWxxCxiAXi --- pyproject.toml | 2 +- src/lightcone/engine/assets.py | 121 +++++++++----- src/lightcone/engine/container.py | 6 +- src/lightcone/engine/crate.py | 61 +++---- src/lightcone/engine/dataset.py | 104 ++++++++++-- src/lightcone/engine/identity.py | 13 +- src/lightcone/engine/materialize.py | 126 +++++++++++--- src/lightcone/engine/plan.py | 158 ++++++++++++++++-- src/lightcone/engine/sandbox/denial.py | 10 +- src/lightcone/engine/sandbox/policy.py | 26 +-- .../engine/templates/files/gitattributes.tmpl | 9 +- .../templates/files/results-README.md.tmpl | 9 +- src/lightcone/engine/worker.py | 53 ++++-- tests/test_assets.py | 101 +++++++---- tests/test_container_smoke.py | 11 +- tests/test_crate.py | 80 +++++---- tests/test_crate_smoke.py | 6 +- tests/test_dataset.py | 43 +++-- tests/test_identity.py | 31 +++- tests/test_materialize.py | 132 +++++++++------ tests/test_plan.py | 106 +++++++++++- tests/test_sandbox_denial.py | 2 +- tests/test_sandbox_enforcement.py | 32 ++-- tests/test_sandbox_policy.py | 39 +++-- tests/test_templates.py | 34 +++- tests/test_venue.py | 8 +- tests/test_worker.py | 151 ++++++++++------- 27 files changed, 1061 insertions(+), 413 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ac861a07..d53fafc5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ classifiers = [ ] dependencies = [ - "astra-tools==0.2.16", + "astra-tools==0.2.17", "click>=8.0", "rich>=13.0", "git-annex>=10.2026", diff --git a/src/lightcone/engine/assets.py b/src/lightcone/engine/assets.py index 40bdfd37..fb753946 100644 --- a/src/lightcone/engine/assets.py +++ b/src/lightcone/engine/assets.py @@ -1,10 +1,16 @@ """What a materialized output *is*: where it lives, what it records, and whether it is still current. -An asset is a directory — ``results///`` — holding -whatever the recipe wrote, plus a manifest beside it. The manifest is the -only part lc writes itself, and it is kept out of the annex so it stays -readable on a clone that has fetched no content at all. +An asset is a single file — ``/results///.`` +— with a manifest sidecar beside it, ``..manifest.json``. The format +comes from the spec, so the path is derived rather than chosen by the +recipe, and one output can only ever be one file. The manifest is the only +part lc writes itself, and it is kept out of the annex so it stays readable +on a clone that has fetched no content at all. + +*home* is the directory holding the analysis's own ``astra.yaml`` — the +project root, or a sub-analysis's own directory — so results sit beside the +spec that declares them. The rule that classifies an output — ``current``, ``behind`` or ``stale`` — lives here too, next to the manifest it reads and the hashes @@ -19,7 +25,7 @@ import hashlib import json -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from dataclasses import asdict, dataclass, field from pathlib import Path from typing import TYPE_CHECKING, Any, Literal @@ -30,13 +36,9 @@ # A type only: history is git's, and this module runs no git. from lightcone.engine.dataset import LastWrite -MANIFEST_FILENAME = ".lightcone-manifest.json" +MANIFEST_SUFFIX = ".manifest.json" SCHEMA_VERSION = 1 -#: Excluded from the content hash: the manifest is written *after* the -#: hash it contains, so hashing it would be circular. -_HASH_EXCLUDE = frozenset({MANIFEST_FILENAME}) - #: The marker every annexed path carries, in both of the shapes an annexed #: file takes on disk — the pointer file's first bytes, and the locked #: symlink's target. Detecting the first is the same test git-annex's own @@ -51,33 +53,67 @@ class ContentNotFetchedError(ProjectError): """An annexed file whose content is not in this clone.""" -def output_dir(root: Path, universe_id: str, output_id: str) -> Path: - """Locate a ``(universe, output)`` pair's directory. +def output_path( + home: Path, universe_id: str, scope: Sequence[str], local_id: str, fmt: str +) -> Path: + """Locate the file one output is materialized to. - Path-addressed: the path in a rendered recipe is this path, with no + ``/results///.``. *home* is + the directory holding the declaring analysis's ``astra.yaml``, so a + sub-analysis keeps its results beside its own spec, and *scope* is the + inline sub-analyses descended through since that home. + + Path-addressed: this is the path a rendered recipe writes to, with no staging, scratch or relocation in between. Args: - root: The project root. - universe_id: The universe the output was made under. - output_id: The output's id, qualified for a sub-analysis. + home: The declaring analysis's directory. + universe_id: The universe the output is made under. + scope: Inline sub-analysis ids, outermost first. + local_id: The output's own id, unqualified. + fmt: The declared serialization, without a leading dot. Returns: - ``/results//``. + The output's path. Raises: - ProjectError: If either id is not a single path component. The - path is *composed* from them, so an empty one collapses it - onto a parent — ``results/`` itself, for two — and a worker - empties this directory before running a recipe in it. + ProjectError: If any part cannot name a single path component. The + path is *composed*, so a part carrying a separator or ``..`` + would place an output outside the tree the caller checked. """ - for label, value in (("universe", universe_id), ("output", output_id)): + named = [("universe", universe_id), *(("analysis", s) for s in scope), ("output", local_id)] + for label, value in named: if not value or "/" in value or "\\" in value or value in {".", ".."}: raise ProjectError( f"{label} id {value!r} is not a single path component, so it " f"cannot name a directory under results/." ) - return root / "results" / universe_id / output_id + if not fmt or "/" in fmt or "\\" in fmt or fmt.startswith("."): + raise ProjectError( + f"output `{local_id}` declares the format {fmt!r}, which cannot name a " + f"file extension, so the output has nowhere to be written." + ) + return home.joinpath("results", universe_id, *scope, f"{local_id}.{fmt}") + + +def manifest_path(output: Path) -> Path: + """The manifest sidecar beside *output*. + + ``..manifest.json``, named from the output's id alone and + never its format — so the manifest keeps its path, and therefore its + history, when a spec re-declares the output in another serialization. + + An id cannot contain a dot (``^[a-z][a-z0-9_]*$``) while a format can + (``tar.gz``), so the id is recovered by partitioning on the **first** + dot. ``Path.stem`` would answer ``x.tar`` for ``x.tar.gz``. + + Args: + output: The output's own path. + + Returns: + The sidecar's path. + """ + return output.parent / f".{output.name.partition('.')[0]}{MANIFEST_SUFFIX}" # ============================================================================= @@ -90,16 +126,18 @@ def data_version(path: Path) -> str: A directory hashes each file in sorted relative-path order with the relative path fed in beside the bytes, so a rename moves the digest. A - file hashes its own bytes. The two are framed apart, so a directory - holding one file cannot collide with that file alone. + file hashes its own bytes, unframed — so the digest is a plain sha256 + an outsider reproduces with ``sha256sum``, and the manifest agrees with + what the crate publishes for the same file. Only the directory side is + framed, which is enough to keep a directory holding one file from + colliding with that file alone. Never mtime or size: a content hash is what lets a byte-identical rebuild stop cascading, and what stops a file restored with an old timestamp passing as unchanged. Args: - path: A file or directory. The manifest is excluded from a - directory's digest, since it carries the result. + path: A file or directory. Returns: The digest, as ``sha256:``. @@ -117,7 +155,6 @@ def data_version(path: Path) -> str: h = hashlib.sha256() if path.is_file(): require_fetched(path) - h.update(b"file:") _feed(h, path) return f"sha256:{h.hexdigest()}" @@ -131,7 +168,7 @@ def data_version(path: Path) -> str: files = [ p for p in path.rglob("*") - if p.name not in _HASH_EXCLUDE and (p.is_file() or (p.is_symlink() and not p.exists())) + if p.is_file() or (p.is_symlink() and not p.exists()) ] for p in sorted(files, key=lambda x: x.relative_to(path).as_posix()): require_fetched(p) @@ -312,11 +349,16 @@ def as_dict(self) -> dict[str, Any]: return {"schema_version": data.pop("schema_version"), **data} -def read(directory: Path) -> Manifest | None: - """Read the manifest in *directory*. +def read(manifest: Path) -> Manifest | None: + """Read the manifest at *manifest*. + + Takes the sidecar's own path rather than the output's, so every caller + holding an output path has to say :func:`manifest_path` out loud. Both + are ``Path``, and a missing file answers ``None`` rather than raising, + so a caller that passed the wrong one would be wrong in silence. Args: - directory: An output directory. + manifest: The sidecar's path. Returns: The manifest, or ``None`` when it is absent or unparseable — which @@ -326,7 +368,7 @@ def read(directory: Path) -> Manifest | None: OSError: Deliberately not caught. A permission problem is a real fault and must not look like an output needing a rebuild. """ - path = directory / MANIFEST_FILENAME + path = manifest if not path.is_file(): return None try: @@ -336,21 +378,22 @@ def read(directory: Path) -> Manifest | None: return None -def write(directory: Path, manifest: Manifest) -> Path: - """Write *manifest* into *directory*, atomically. +def write(path: Path, manifest: Manifest) -> Path: + """Write *manifest* to *path*, atomically. The rename is the commit point: a reader sees the previous manifest or - this one, never half of either. + this one, never half of either. The temporary lands beside real outputs + now, so it is named off the sidecar — unique per output, and swept by + the same pathspec. Args: - directory: The output directory to write into. + path: The sidecar's path. manifest: The record to write. Returns: The path written. """ - path = directory / MANIFEST_FILENAME - temporary = directory / f"{MANIFEST_FILENAME}.tmp" + temporary = path.with_name(path.name + ".tmp") temporary.write_text(json.dumps(manifest.as_dict(), indent=2, sort_keys=False) + "\n") temporary.replace(path) return path diff --git a/src/lightcone/engine/container.py b/src/lightcone/engine/container.py index 0a7b8540..29eaf5c3 100644 --- a/src/lightcone/engine/container.py +++ b/src/lightcone/engine/container.py @@ -415,7 +415,7 @@ def converge(runtime: Runtime) -> list[str]: def policy_for( - runtime: Runtime, read_paths: list[Path], *, output_dir: Path | None = None + runtime: Runtime, read_paths: list[Path], *, write_dir: Path | None = None ) -> sandbox.Policy: """Build the exec policy for a resolved runtime. @@ -428,7 +428,7 @@ def policy_for( Args: runtime: The resolved runtime. read_paths: Declared inputs, as :func:`sandbox.exec_policy` takes. - output_dir: A recipe's own output directory; absent for a probe. + write_dir: The directory a recipe's output lands in; absent for a probe. Returns: The policy for this world. @@ -438,7 +438,7 @@ def policy_for( read_paths=read_paths, env_dir=runtime.env_dir, containerized=runtime.mode == "containerized", - output_dir=output_dir, + write_dir=write_dir, ) diff --git a/src/lightcone/engine/crate.py b/src/lightcone/engine/crate.py index 78c10c9b..f8318960 100644 --- a/src/lightcone/engine/crate.py +++ b/src/lightcone/engine/crate.py @@ -26,7 +26,6 @@ from __future__ import annotations -import bisect import hashlib import json import re @@ -80,7 +79,7 @@ def render( *, license: str, dsid: str, - writer: Callable[[Path], LastWrite], + writer: Callable[..., LastWrite], keys: Mapping[str, str], ) -> str: """Build the crate document for the project as it stands. @@ -124,7 +123,7 @@ def __init__( graph: Graph, license: str, dsid: str, - writer: Callable[[Path], LastWrite], + writer: Callable[..., LastWrite], keys: Mapping[str, str], ) -> None: from astra.helpers import load_yaml @@ -134,10 +133,6 @@ def __init__( self.license = license self.writer = writer self.keys = dict(keys) - #: Sorted once: each output selects its files by bisecting this, - #: not by rescanning the whole map — the map holds every annexed - #: file in the repository, data/ included. - self.sorted_keys = sorted(self.keys) self.crate = ROCrate() self.crate.metadata.extra_contexts.append(_WORKFLOW_RUN_CONTEXT) #: Every materialized task, sorted: the one iteration order. @@ -145,7 +140,7 @@ def __init__( ( (key, manifest) for key, task in graph.tasks.items() - if (manifest := assets.read(task.output_dir)) is not None + if (manifest := assets.read(task.manifest_path)) is not None ), key=lambda pair: pair[0], ) @@ -292,12 +287,21 @@ def _tool_id(self, output_id: str) -> str: # ----- the data entities ----- def _environment_files(self) -> None: - """The lock and its companions — what every run consumed.""" - universes = sorted( + """The lock and its companions — what every run consumed. + + Every analysis *home*, not just the root's: a sub-analysis keeps + its spec, its universes and its results together, so publishing + its outputs without the files that define them would leave the + crate describing half an analysis. + """ + homes = {self.root, *(task.home for task in self.graph.tasks.values())} + declaring = sorted( path.relative_to(self.root).as_posix() - for path in (self.root / "universes").glob("*.yaml") + for home in homes + for path in (*(home / "universes").glob("*.yaml"), home / SPEC_FILENAME) + if path.is_file() ) - for name in (*_ENVIRONMENT, *universes): + for name in (*_ENVIRONMENT, *declaring): if (self.root / name).is_file(): self._file(name) if (self.root / "README.md").is_file(): @@ -347,35 +351,34 @@ def _integrity(self, name: str) -> dict[str, str]: return {"contentSize": str(len(data)), "sha256": hashlib.sha256(data).hexdigest()} def _dataset_id(self, key: Key) -> str: - """One output directory's crate id — :func:`plan.declared_path`'s - answer, never a second spelling of the results layout.""" - return plan.declared_path(self.root, self.graph.tasks[key].output_dir) + "/" + """One output's crate id — :func:`plan.declared_path`'s answer, + never a second spelling of the results layout. No trailing slash: + an output is one file, so the entity is a ``File``.""" + return plan.declared_path(self.root, self.graph.tasks[key].output_path) def _dataset(self, key: Key, manifest: assets.Manifest) -> None: universe_id, output_id = key - dataset_id = self._dataset_id(key) - entity = self.crate.add_dataset(self.graph.tasks[key].output_dir, dataset_id) + task = self.graph.tasks[key] + # A `File`, not a `Dataset` with parts: an output is one file, so + # there is one annex key to look up by name and one checksum to + # publish. `data_version` is that same digest — the manifest and + # the crate no longer carry two numbers for the same bytes. + entity = self._file(self._dataset_id(key)) entity["name"] = f"{output_id} (universe {universe_id})" entity["description"] = f"output `{output_id}` materialized under `{universe_id}`" entity["version"] = manifest.data_version - manifest_file = self._file(f"{dataset_id}{assets.MANIFEST_FILENAME}") - manifest_file["about"] = {"@id": dataset_id} + if media := _FORMATS.get(f".{task.output_path.name.partition('.')[2]}", ""): + entity["encodingFormat"] = media + manifest_file = self._file(plan.declared_path(self.root, task.manifest_path)) + manifest_file["about"] = {"@id": entity.id} entity["subjectOf"] = {"@id": manifest_file.id} - # Every file the directory holds, each with the checksum its - # annex key already carries — the claim `sha256sum` can check - # after a `git archive` deposit, where `version` above is lc's - # own framed directory digest and deliberately is not that. - parts = [manifest_file] - lo = bisect.bisect_left(self.sorted_keys, dataset_id) - hi = bisect.bisect_left(self.sorted_keys, dataset_id + "\uffff") - parts += [self._file(name) for name in self.sorted_keys[lo:hi]] - entity["hasPart"] = [{"@id": part.id} for part in parts] # ----- the runs ----- def _action(self, key: Key, manifest: assets.Manifest) -> str: universe_id, output_id = key - write = self.writer(self.graph.tasks[key].output_dir) + task = self.graph.tasks[key] + write = self.writer(task.output_path, task.manifest_path) self.agents[key] = self._person(write.author, write.email) if write else "" properties: dict[str, Any] = { "@type": "CreateAction", diff --git a/src/lightcone/engine/dataset.py b/src/lightcone/engine/dataset.py index 8b3d91e4..dadf65e0 100644 --- a/src/lightcone/engine/dataset.py +++ b/src/lightcone/engine/dataset.py @@ -18,7 +18,7 @@ from __future__ import annotations -from collections.abc import Iterable +from collections.abc import Iterable, Sequence from dataclasses import dataclass from pathlib import Path @@ -259,8 +259,8 @@ def dataset_id(directory: Path) -> str: return found.stdout.strip() if found.returncode == 0 else "" -def last_writer(directory: Path, path: Path) -> LastWrite: - """Find the commit that last touched *path*. +def last_writer(directory: Path, *paths: Path | str) -> LastWrite: + """Find the commit that last touched any of *paths*. Run from the project root with a relative pathspec, so it answers about the project's own subdirectory even inside an enclosing @@ -273,15 +273,28 @@ def last_writer(directory: Path, path: Path) -> LastWrite: ``.git``, or a host without git — states such projects are actually in. + Several paths, because an output is a file *and* a manifest and a + hand edit to either contradicts the record: the payload alone would + miss a doctored manifest, and the manifest alone would miss the forged + result that is the whole reason to ask. The manifest also carries the + history across a re-declared format, where the payload's path is new. + Args: directory: The project root. - path: The path to ask about, absolute or repository-relative. + paths: What to ask about, absolute, repository-relative, or a git + pathspec. Returns: - The commit, falsy-empty when none has touched the path — or when - git cannot answer at all. + The commit, falsy-empty when none has touched them — or when git + cannot answer at all. """ - argv = ["log", "-1", "--format=%H%x00%s%x00%an%x00%ae%x00%as", "--", _rel(directory, path)] + argv = [ + "log", + "-1", + "--format=%H%x00%s%x00%an%x00%ae%x00%as", + "--", + *(_rel(directory, p) for p in paths), + ] out = _ask(argv, cwd=directory) if not (out := (out or "").strip("\n")): return LastWrite() @@ -318,7 +331,55 @@ def annex_keys(directory: Path) -> dict[str, str]: return keys -def save(directory: Path, paths: Iterable[Path], message: str) -> bool: +def largefiles(directory: Path, paths: Sequence[Path]) -> dict[str, str]: + """Ask ``.gitattributes`` where each of *paths* would be stored. + + ``check-attr`` answers about the *rules*, so it works on paths that do + not exist yet — which is what lets a run ask before it writes. + + Args: + directory: The repository root. + paths: What to ask about. + + Returns: + Repository-relative path → the ``annex.largefiles`` value, which + is ``"unspecified"`` where no line matches. + """ + if not paths: + return {} + relative = [_rel(directory, p) for p in paths] + out = _git(["check-attr", "-z", "annex.largefiles", "--", *relative], cwd=directory) + fields = out.split("\0") + return { + fields[i]: fields[i + 2] for i in range(0, len(fields) - 2, 3) if fields[i + 1] + } + + +def tracked_manifests(directory: Path) -> list[Path]: + """Every manifest sidecar git is tracking, anywhere in the tree. + + From the index rather than a directory walk: it is exact, it costs one + process, it never descends into ``.git`` or ``.venv``, and it still + finds the manifests under a sub-analysis that has just been deleted + from the spec — which is the case a walk driven by the current spec + would miss, in the very edit that orphans them. Untracked strays are + the dirty check's business, not this one's. + + Args: + directory: The repository root. + + Returns: + Absolute paths, unordered. + """ + # Imported here, not at module scope: `assets` reaches this module + # through `project`, so the name has to be fetched after both exist. + from lightcone.engine.assets import MANIFEST_SUFFIX + + listed = _git(["ls-files", "-z", "--", f":(glob)**/*{MANIFEST_SUFFIX}"], cwd=directory) + return [directory / name for name in listed.split("\0") if name] + + +def save(directory: Path, paths: Iterable[Path | str], message: str) -> bool: """Commit *paths*. A plain ``git add``: ``.gitattributes`` sets ``filter=annex``, so git's @@ -357,7 +418,7 @@ def save(directory: Path, paths: Iterable[Path], message: str) -> bool: Args: directory: The repository root. - paths: What to stage, absolute or repository-relative. + paths: What to stage — a path, or a git pathspec. message: The commit message. Returns: @@ -377,22 +438,28 @@ def save(directory: Path, paths: Iterable[Path], message: str) -> bool: return True -def restore(directory: Path, paths: Iterable[Path]) -> None: +def restore(directory: Path, paths: Iterable[Path | str]) -> None: """Put *paths* back the way the last commit had them. ``clean`` first for what a run wrote, then ``checkout`` for what it - deleted or truncated — and only when the path is in ``HEAD``, since a - first materialization has nothing to go back to. + deleted or truncated — and only when ``HEAD`` has something matching, + since a first materialization has nothing to go back to and + ``checkout`` errors on a pathspec it cannot match. + + ``ls-files --with-tree`` asks that question, not ``cat-file -e``: + a pathspec may be a glob, and ``cat-file`` takes a single path while + ``ls-tree`` refuses glob magic outright. Args: directory: The repository root. - paths: What to restore. Scoped to these and never the whole tree: - a failed run must not discard edits made while it ran. + paths: What to restore, as paths or git pathspecs. Scoped to these + and never the whole tree: a failed run must not discard edits + made while it ran. """ for path in paths: rel = _rel(directory, path) _git(["clean", "-qfdx", "--", rel], cwd=directory) - if _git_ok(["cat-file", "-e", f"HEAD:{rel}"], cwd=directory): + if _git(["ls-files", "--with-tree=HEAD", "--", rel], cwd=directory).strip(): _git(["checkout", "-q", "HEAD", "--", rel], cwd=directory) @@ -433,13 +500,18 @@ def _git_ok(argv: list[str], *, cwd: Path) -> bool: return bool(project._run(["git", *argv], cwd=cwd).returncode == 0) -def _rel(directory: Path, path: Path) -> str: +def _rel(directory: Path, path: Path | str) -> str: """*path* as a repository-relative POSIX pathspec. git pathspecs are ``/``-separated whatever the platform, and an absolute path would silently mean something else inside a repository reached through a symlink. + + A ``str`` passes through untouched: it is already a pathspec, and one + carrying magic (``:(glob)…``) is not a path to be normalised. """ + if isinstance(path, str): + return path resolved = Path(path) if resolved.is_absolute(): resolved = resolved.relative_to(directory.resolve()) diff --git a/src/lightcone/engine/identity.py b/src/lightcone/engine/identity.py index 68567443..55cd39d4 100644 --- a/src/lightcone/engine/identity.py +++ b/src/lightcone/engine/identity.py @@ -109,15 +109,23 @@ def env_version(root: Path) -> str: return f"sha256:{h.hexdigest()}" -def definition_version(*, recipe: str, decisions: Mapping[str, str]) -> str: +def definition_version(*, recipe: str, decisions: Mapping[str, str], fmt: str) -> str: """Compute what the spec says one output is. - ``sha256(recipe ‖ canonical decisions)``, length-framed. The + ``sha256(recipe ‖ canonical decisions ‖ format)``, length-framed. The environment is deliberately absent: see this module's docstring. + The format is framed in its own right rather than left to arrive + through the rendered recipe's ``{output}`` path, because a recipe need + not use that placeholder at all. One that does not would keep its + digest across a re-declared serialization while the manifest — whose + sidecar is named from the id alone, so it does not move either — went + on describing an output at a path that no longer exists. + Args: recipe: The rendered recipe command. decisions: The decisions this output declares, as id → option. + fmt: The declared serialization. Returns: The digest, as ``sha256:``. @@ -125,6 +133,7 @@ def definition_version(*, recipe: str, decisions: Mapping[str, str]) -> str: h = hashlib.sha256() _frame(h, "recipe", recipe.encode()) _frame(h, "decisions", _canonical(dict(decisions)).encode()) + _frame(h, "format", fmt.encode()) return f"sha256:{h.hexdigest()}" diff --git a/src/lightcone/engine/materialize.py b/src/lightcone/engine/materialize.py index 331d7b37..da2e0335 100644 --- a/src/lightcone/engine/materialize.py +++ b/src/lightcone/engine/materialize.py @@ -39,7 +39,7 @@ from collections.abc import Iterator, Sequence from contextlib import contextmanager from dataclasses import asdict, dataclass, field -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Any, Protocol from lightcone.engine import assets, container, dataset, identity, plan, project, venue, worker @@ -178,11 +178,11 @@ def _classified( classified = [] for key in graph.order(): task = graph.tasks[key] - manifest = assets.read(task.output_dir) + manifest = assets.read(task.manifest_path) # History is git's to answer, so it enters the one classification # rule as a value — computed here, where git lives, exactly as # the worker's driver computes it for a run. - foreign = None if manifest is None else _foreign_write(root, key) + foreign = None if manifest is None else _foreign_write(root, task) verdict = assets.classify( definition_version=task.definition_version, env_version=env_version, @@ -234,7 +234,9 @@ def _predicted( # no annex content the files are dangling symlinks, and hashing # them would report a different output and cascade a rebuild # over a project that is perfectly up to date. - manifest = None if upstream in would_run else assets.read(path) + manifest = ( + None if upstream in would_run else assets.read(assets.manifest_path(path)) + ) predicted[name] = manifest.data_version if manifest else None elif not path.exists(): predicted[name] = None @@ -426,13 +428,19 @@ def _crate_line(root: Path, newest: str) -> str: return "up to date with the outputs" -def _foreign_write(root: Path, key: Key) -> dataset.LastWrite | None: - """Find the commit that last touched *key*'s directory, unless it is +def _foreign_write(root: Path, task: Task) -> dataset.LastWrite | None: + """Find the commit that last touched *task*'s manifest, unless it is the output's own run record — then ``None``, the clean answer. The fact only: the verdict's prose is `classify`'s, like every other - why.""" - write = dataset.last_writer(root, assets.output_dir(root, *key)) - if not write or write.subject == datalad_run_subject(key): + why. + + Both paths: a forged *result* is the case the check exists for, and a + doctored *manifest* is the other half — while the manifest is also + what carries the history across a re-declared format, where the + payload's own path is new and would read as clean. + """ + write = dataset.last_writer(root, task.output_path, task.manifest_path) + if not write or write.subject == datalad_run_subject(task.key): return None return write @@ -513,6 +521,7 @@ def materialize( # only maintainer. _converge_crate(root, report, full, dsid) return report + _require_manifests_in_git(root, graph) _fetch_inputs(root, graph, report) # Before the runtime resolves, because the refusal must not cost an # image build: a containerized graph can span an allocation only if @@ -564,9 +573,7 @@ def materialize( # answer is dead — the output is remade regardless — and each ask is # a git process. foreign = { - key: _foreign_write(root, key) - if (task.output_dir / assets.MANIFEST_FILENAME).is_file() - else None + key: _foreign_write(root, task) if task.manifest_path.is_file() else None for key, task in graph.tasks.items() } outstanding: dict[Key, Task] = dict(graph.tasks) @@ -597,7 +604,7 @@ def materialize( # never to the whole tree, so edits made while the graph ran # survive. for task in outstanding.values(): - dataset.restore(root, [task.output_dir]) + dataset.restore(root, _owned(root, task)) # The tree was clean at the start-of-run refusal and save/restore # keeps `results/` clean, so anything dirty *now* was edited while # the graph ran — and every manifest records the starting commit, @@ -630,7 +637,7 @@ def _consume( report.notes.extend([f"{name}:", *lines]) if result.status == "ok": - dataset.save(root, [task.output_dir], run_record(root, task, dsid, runtime)) + dataset.save(root, _owned(root, task), run_record(root, task, dsid, runtime)) report.made.append(name) return @@ -642,7 +649,7 @@ def _consume( report.behind[name] = result.reason return - dataset.restore(root, [task.output_dir]) + dataset.restore(root, _owned(root, task)) getattr(report, result.status).append(name) report.warnings.append(f"{name}: {result.reason}") @@ -820,12 +827,18 @@ def _converge_crate(root: Path, report: MaterializeReport, full: Graph, dsid: st "publication view is maintained — declare one to enable it" ) return - for directory in sorted((root / "results").glob("*/*/")): - key = (directory.parent.name, directory.name) - if key not in full.tasks and assets.read(directory) is not None: + # A set difference, never a reconstruction: an external sub-analysis is + # filed under its own universe while its graph key carries the parent's, + # so no rule turns one of these paths back into a key. Enumerated from + # git rather than from the homes the graph knows, so a sub-analysis just + # deleted from the spec is still named — the edit that orphans a tree is + # the same edit that would drop it from any graph-derived root set. + expected = {task.manifest_path for task in full.tasks.values()} + for manifest in sorted(dataset.tracked_manifests(root)): + if manifest not in expected and assets.read(manifest) is not None: report.warnings.append( - f"{plan.declared_path(root, directory)} has a manifest but the " - "spec no longer declares it, so it is not in the publication view" + f"{plan.declared_path(root, manifest)} is a manifest the spec no " + "longer declares, so its output is not in the publication view" ) try: document = crate.render( @@ -897,7 +910,10 @@ def run_record(root: Path, task: Task, dsid: str, runtime: container.Runtime) -> "dsid": dsid, "exit": 0, "inputs": sorted(plan.declared_path(root, path) for path in task.inputs.values()), - "outputs": [plan.declared_path(root, task.output_dir)], + "outputs": [ + plan.declared_path(root, task.output_path), + plan.declared_path(root, task.manifest_path), + ], "pwd": ".", } if runtime.mode == "containerized": @@ -1029,6 +1045,58 @@ def _graph( return graph, env_version, full +def _require_manifests_in_git(root: Path, graph: Graph) -> None: + """Refuse before the graph runs if a manifest would reach the annex. + + ``dataset.save`` opts dot-named paths *out* of git-annex's stock + routing, so the sidecar's leading dot decides nothing on its own and + ``.gitattributes`` decides everything. Get that wrong and the run is + green here and broken everywhere else: a clone with no content fetched + reads a pointer file where the manifest should be, every output + classifies as never materialized, and nothing said so. + + One process for the whole graph — ``check-attr`` takes many paths — + and before anything executes, because the repo refuses before it + spends. + + Args: + root: The project root. + graph: The run's tasks. + + Raises: + ProjectError: If any manifest would be routed to the annex. + """ + if not (paths := sorted({task.manifest_path for task in graph.tasks.values()})): + return + routed = dataset.largefiles(root, paths) + if annexed := sorted(path for path, answer in routed.items() if answer != "nothing"): + raise ProjectError( + f"{plan.declared_path(root, Path(annexed[0]))} would be stored in the annex, " + "but a manifest has to stay a plain git blob — it is what a clone reads " + "before fetching any content. Check .gitattributes still carries " + "`**/results/**/.*.manifest.json annex.largefiles=nothing`, last." + ) + + +def _owned(root: Path, task: Task) -> list[str]: + """Every path one output owns, as git pathspecs. + + Globs rather than the two literal paths, and each earns its magic. The + payload's covers whatever format the *previous* run wrote, so a + re-declared serialization stages the old file's deletion instead of + leaving it behind untracked. The manifest's trailing ``*`` covers the + ``.tmp`` a failed write leaves, which nothing else would sweep. + + Never the parent directory: siblings share it, and under Dask they are + in flight — ``git clean`` on it would delete a running task's bytes. + """ + parent = plan.declared_path(root, task.output_path.parent) + return [ + f":(glob){parent}/{task.local_id}.*", + f":(glob){parent}/.{task.local_id}{assets.MANIFEST_SUFFIX}*", + ] + + def _name(key: Key) -> str: return f"{key[0]}/{key[1]}" @@ -1037,12 +1105,18 @@ def _dirty(root: Path, changes: Sequence[tuple[str, str]]) -> str: """The refusal, split by what the right remedy actually is. Two path classes, because they call for opposite actions: work the - researcher owns has to be committed, and anything under ``results/`` + researcher owns has to be committed, and anything under a ``results/`` is lc's to write, so a change there is wreckage to discard rather than a contribution to keep. + + The test is path-shaped rather than a lookup against the graph: this + refusal runs before the spec is read, deliberately, so that staged work + is protected before anything expensive happens. The cost is that a + directory called ``results`` anywhere in the tree reads as lc's — so + the remedy names the offending paths rather than sweeping a directory. """ - theirs = [c for c in changes if not c[1].startswith("results/")] - ours = [c for c in changes if c[1].startswith("results/")] + ours = [c for c in changes if "results" in PurePosixPath(c[1]).parts] + theirs = [c for c in changes if c not in ours] lines = [ f"uncommitted changes in {root} — every materialization is committed " @@ -1059,7 +1133,9 @@ def _dirty(root: Path, changes: Sequence[tuple[str, str]]) -> str: lines += [ "", " discard these (lc writes results/):", - " git restore --staged --worktree results/ && git clean -fd results/", + " git restore --staged --worktree -- " + + " ".join(sorted({path for _, path in ours})), + " git clean -fd -- " + " ".join(sorted({path for _, path in ours})), *(f" {code.strip() or '??'} {path}" for code, path in ours), ] return "\n".join(lines) diff --git a/src/lightcone/engine/plan.py b/src/lightcone/engine/plan.py index 154af1f5..90b82ca6 100644 --- a/src/lightcone/engine/plan.py +++ b/src/lightcone/engine/plan.py @@ -10,9 +10,15 @@ universe's decisions, resolves every output's inputs to what supplies them, drops the outputs whose ``when:`` does not hold, and renders the recipe grammar — so scoping, ``from:`` references and sub-analysis -nesting are read here rather than re-derived. A qualified output id -(``classification.accuracy``) is used verbatim as the directory name, so -one addressing scheme spans however deep the spec nests. +nesting are read here rather than re-derived. + +Where an output lands follows the spec's own shape: every analysis node +has a *home* — the directory holding its ``astra.yaml`` — and its outputs +materialize to ``/results///.``. +An external sub-analysis (``path:``) is a self-similar analysis with its +own home and, where it names one, its own universe; an inline one shares +its parent's home and disambiguates with a scope directory. Graph keys stay +the qualified id, so only the path nests, never the addressing. Nothing here schedules anything. Ordering is Dask's job at execution time and a topological walk's job in ``--check``; this module only says which @@ -21,9 +27,11 @@ from __future__ import annotations +import os from dataclasses import dataclass from graphlib import CycleError, TopologicalSorter from pathlib import Path +from typing import Any from lightcone.engine import assets, identity from lightcone.engine.project import SPEC_FILENAME, ProjectError @@ -37,18 +45,30 @@ class Task: """One output, in one universe: everything needed to make it.""" universe_id: str + #: The qualified id — ``classification.accuracy`` inside a sub-analysis. output_id: str - output_dir: Path + #: The id as declared, unqualified: what the file is named after. + local_id: str + #: The directory holding the astra.yaml that declares this output — + #: the project root, or a sub-analysis's own directory. + home: Path + #: The single file this output is. + output_path: Path #: The recipe with its placeholders substituted — a shell command. recipe: str #: Declared input name → the path it resolves to. Upstream outputs are - #: their directories; everything else is whatever ``source`` named. + #: their files; everything else is whatever ``source`` named. inputs: dict[str, Path] #: The subset of ``inputs`` another task produces, and which one. produced_by: dict[str, Key] decisions: dict[str, str] definition_version: str + @property + def manifest_path(self) -> Path: + """This output's manifest sidecar.""" + return assets.manifest_path(self.output_path) + @property def key(self) -> Key: """This task's identity within a run.""" @@ -187,6 +207,20 @@ def build(root: Path) -> Graph: declared_in[universe_id] = path for task in _tasks(root, universe_id, spec, universe): tasks[task.key] = task + + # Two tasks composing one path would have the second silently overwrite + # the first. The way in is real: an external sub-analysis is filed under + # its own universe, so two parent universes that both select it resolve + # to the same file while feeding it different inputs. + written_by: dict[Path, Key] = {} + for key, task in tasks.items(): + if (earlier := written_by.get(task.output_path)) is not None: + raise ProjectError( + f"`{earlier[0]}/{earlier[1]}` and `{key[0]}/{key[1]}` both materialize to " + f"{declared_path(root, task.output_path)}. Give the sub-analysis its own " + f"universe per parent universe, or the second would overwrite the first." + ) + written_by[task.output_path] = key return Graph(tasks=tasks) @@ -254,6 +288,83 @@ def _validate(spec_path: Path, universes: list[Path]) -> None: ) +@dataclass(frozen=True) +class _Placement: + """Where one analysis node's outputs land.""" + + #: The directory holding that node's ``astra.yaml``. + home: Path + #: The universe its results are filed under. + universe_id: str + #: The inline sub-analyses descended through since ``home``. + scope: tuple[str, ...] + + +def _placements( + root: Path, + spec: dict[str, Any], + universe: dict[str, Any], + universe_id: str, +) -> dict[tuple[str, ...], _Placement]: + """Locate every analysis node in the tree. + + Walks the spec and the universe file together, because only the + universe says which of a sub-analysis's own universes was picked. + + Args: + root: The project root — the root analysis's home. + spec: The analysis, external sub-analyses already resolved. + universe: The universe file's contents. + universe_id: The universe file's id. + + Returns: + Qualified scope → where its outputs land. + + Raises: + ProjectError: If a sub-analysis's home escapes the project, or it + selects a universe that is not there. + """ + found: dict[tuple[str, ...], _Placement] = {} + inside = root.resolve() + + def walk( + node: dict[str, Any], + chosen: dict[str, Any], + scope: tuple[str, ...], + place: _Placement, + ) -> None: + found[scope] = place + for sub_id, sub in (node.get("analyses") or {}).items(): + if not isinstance(sub, dict): + continue + here = (*scope, str(sub_id)) + picked = (chosen.get("analyses") or {}).get(str(sub_id)) or {} + declared = sub.get("path") + if declared: + home = Path(os.path.normpath(place.home / str(declared))) + if not home.resolve().is_relative_to(inside): + raise ProjectError( + f"sub-analysis `{'.'.join(here)}` has path `{declared}`, which is " + f"outside the project — its results could not be versioned here." + ) + selected = picked.get("universe") + if selected: + named = home / "universes" / f"{selected}.yaml" + if not named.is_file(): + raise ProjectError( + f"universe `{universe_id}` selects `{selected}` for " + f"sub-analysis `{'.'.join(here)}`, but {declared_path(root, named)} " + f"does not exist." + ) + nested = _Placement(home, str(selected) if selected else place.universe_id, ()) + else: + nested = _Placement(place.home, place.universe_id, (*place.scope, str(sub_id))) + walk(sub, picked, here, nested) + + walk(spec, universe, (), _Placement(root, universe_id, ())) + return found + + def _tasks( root: Path, universe_id: str, @@ -270,22 +381,41 @@ def _tasks( from astra.resolve import render_command, resolve_outputs resolved = resolve_outputs(spec, universe, root) - executable = {out.id for out in resolved if out.command} + where = _placements(root, dict(spec), dict(universe), universe_id) + # The whole resolved output, not just its id: an upstream input has to + # resolve to the *file* another task writes, which needs that output's + # home, universe, scope and format. A set of ids cannot say any of it. + executable = {out.id: out for out in resolved if out.command} + + if absent := sorted(out.id for out in executable.values() if not out.format): + raise ProjectError( + f"{len(absent)} output(s) declare no `format:`: {', '.join(absent)}. " + "lc names each output's file from it, so there is nowhere to write them — " + "add the artifact's file extension, e.g. `format: png`." + ) + + def file_of(out: object) -> Path: + place = where[out.scope] # type: ignore[attr-defined] + return assets.output_path( + place.home, + place.universe_id, + place.scope, + str(out.definition["id"]), # type: ignore[attr-defined] + str(out.format), # type: ignore[attr-defined] + ) tasks = [] for out in resolved: if not out.command: continue - output_dir = assets.output_dir(root, universe_id, out.id) + output_path = file_of(out) values: dict[str, str] = {} paths: dict[str, Path] = {} produced_by: dict[str, Key] = {} for declared in out.inputs: if declared.produced_by in executable: produced_by[declared.id] = (universe_id, declared.produced_by) - paths[declared.id] = assets.output_dir( - root, universe_id, declared.produced_by - ) + paths[declared.id] = file_of(executable[declared.produced_by]) elif declared.source: # An absolute `source:` wins over the join — pathlib's own # rule, and the one anyone writing one expects. @@ -302,7 +432,7 @@ def _tasks( out.command, inputs=values, decisions=out.decisions, - output=declared_path(root, output_dir), + output=declared_path(root, output_path), ) except ValueError as e: raise ProjectError(f"output `{out.id}`: {e}") from e @@ -311,13 +441,15 @@ def _tasks( Task( universe_id=universe_id, output_id=out.id, - output_dir=output_dir, + local_id=str(out.definition["id"]), + home=where[out.scope].home, + output_path=output_path, recipe=recipe, inputs=paths, produced_by=produced_by, decisions=out.decisions, definition_version=identity.definition_version( - recipe=recipe, decisions=out.decisions + recipe=recipe, decisions=out.decisions, fmt=str(out.format) ), ) ) diff --git a/src/lightcone/engine/sandbox/denial.py b/src/lightcone/engine/sandbox/denial.py index c19325ec..d317cdaf 100644 --- a/src/lightcone/engine/sandbox/denial.py +++ b/src/lightcone/engine/sandbox/denial.py @@ -184,11 +184,11 @@ def _render_write(path: Path) -> list[str]: return _message( f"cannot write {path}", [ - " a recipe writes only its own output directory ({output} in the", - " recipe); a probe writes results/. The rest of the tree is", - " read-only, so the environment a run starts with is the one it", - " ends with. For anything that is not output, write somewhere", - " scratch:", + " a recipe writes its output ({output} in the recipe) and the", + " directory that holds it; a probe writes results/. The rest of", + " the tree is read-only, so the environment a run starts with is", + " the one it ends with. For anything that is not output, write", + " somewhere scratch:", " import tempfile; tempfile.mkdtemp() # or $TMPDIR", ], ) diff --git a/src/lightcone/engine/sandbox/policy.py b/src/lightcone/engine/sandbox/policy.py index feaf59d9..0c91ba68 100644 --- a/src/lightcone/engine/sandbox/policy.py +++ b/src/lightcone/engine/sandbox/policy.py @@ -181,18 +181,18 @@ def exec_policy( read_paths: Sequence[Path] = (), env_dir: Path | None = None, containerized: bool = False, - output_dir: Path | None = None, + write_dir: Path | None = None, ) -> Policy: """Build what a sandboxed command may touch. - The tree is read-only apart from the write scope: a recipe writes its - own output directory and nothing else in the tree, so a concurrent - task cannot land bytes in a sibling's directory before that sibling - hashes — the one corruption ``data_version`` could never see, because - the manifest it produces is self-consistent and wrong. A probe has no - output, so ``lc run`` gets ``results/`` whole; the probe→recipe - promise therefore excludes exactly the commands that write outside - their own output directory, which is the accident being prevented. + The tree is read-only apart from the write scope: the directory an + output's file lands in, and nothing else. An output is one file now, + and a mechanism can only grant *creating* it through the directory + that will hold it — so outputs declared side by side are mutually + writable again, which the narrower per-output directory used to + prevent. Recorded rather than papered over: what answers whether an + output's bytes are its own is ``data_version``, never the sandbox. A + probe has no analysis node, so ``lc run`` gets ``results/`` whole. The containerized shape is the same policy with the host stripped out: the *image* is the OS baseline and the exec set — everything @@ -207,8 +207,10 @@ def exec_policy( ``.venv``. containerized: Build the mount-shaped policy instead of the host one. - output_dir: The one in-tree directory a recipe may write; absent - for a probe, which gets ``results/`` whole. + write_dir: The one in-tree directory a recipe may write — the + directory holding its output file, shared with the siblings + declared beside it. Absent for a probe, which has no analysis + node and gets the project's own ``results/`` whole. Returns: The policy. The in-tree write scope is granted only if it exists — @@ -231,7 +233,7 @@ def exec_policy( for sub in _HOME_LAYOUT.values(): (tmp_home / sub).mkdir(parents=True, exist_ok=True) - in_tree_write = output_dir if output_dir is not None else project / "results" + in_tree_write = write_dir if write_dir is not None else project / "results" if containerized: # Declared spellings, not realpaths — the one shape that keeps # its paths unresolved. These become mount *destinations*, and a diff --git a/src/lightcone/engine/templates/files/gitattributes.tmpl b/src/lightcone/engine/templates/files/gitattributes.tmpl index a5cb916b..b9bde9be 100644 --- a/src/lightcone/engine/templates/files/gitattributes.tmpl +++ b/src/lightcone/engine/templates/files/gitattributes.tmpl @@ -5,11 +5,18 @@ # decides what counts as content: the default is nothing, and only outputs # and declared inputs opt out. Last matching line wins. # +# An analysis keeps its results beside its own astra.yaml, so a results +# tree sits under a sub-analysis directory as readily as at the root. +# `results/**` is kept alongside the broader rule: a managed line dropped +# from this template stops being ranked, and the ordering check that keeps +# the defaults above the opt-outs would go blind to it. +# # Manifests are exempted back out because they must be readable on a clone # that has fetched no annex content. * annex.largefiles=nothing * filter=annex results/** annex.largefiles=anything data/** annex.largefiles=anything -**/.lightcone-manifest.json annex.largefiles=nothing .datalad/environments/*/image annex.largefiles=anything +**/results/** annex.largefiles=anything +**/results/**/.*.manifest.json annex.largefiles=nothing diff --git a/src/lightcone/engine/templates/files/results-README.md.tmpl b/src/lightcone/engine/templates/files/results-README.md.tmpl index 736ed80c..20e5ea6e 100644 --- a/src/lightcone/engine/templates/files/results-README.md.tmpl +++ b/src/lightcone/engine/templates/files/results-README.md.tmpl @@ -1,5 +1,10 @@ # results/ -Materialized outputs land here, one directory per universe and output: +Materialized outputs land here, one file per universe and output, named +from the output's id and the `format:` its spec declares: - results/// + results//. + +Beside each one is a `..manifest.json` recording how it was +made. A sub-analysis declared with `path:` keeps its own results tree, +beside its own `astra.yaml`. diff --git a/src/lightcone/engine/worker.py b/src/lightcone/engine/worker.py index 5d019224..8dede9d1 100644 --- a/src/lightcone/engine/worker.py +++ b/src/lightcone/engine/worker.py @@ -28,7 +28,6 @@ from __future__ import annotations import functools -import shutil import sys from collections.abc import Mapping from dataclasses import asdict, dataclass @@ -171,7 +170,7 @@ def _materialize( name: live[key] if (key := task.produced_by.get(name)) else context.versions.of(path) for name, path in task.inputs.items() } - manifest = assets.read(task.output_dir) + manifest = assets.read(task.manifest_path) verdict = assets.classify( definition_version=task.definition_version, env_version=context.env_version, @@ -225,18 +224,24 @@ def execute( if moved := _gate(root, context.env_version): return TaskResult(task.key, "failed", reason=moved) - # The whole directory, not a list of expected files: a recipe declares - # an output id rather than filenames, and a previous run that crashed - # can have left anything in here — which would otherwise survive into - # this run's `data_version` as though the recipe had written it. The - # path is `results//` and `output_dir` refuses an id - # that could widen it. - if task.output_dir.exists(): - shutil.rmtree(task.output_dir) - task.output_dir.mkdir(parents=True) + # Never the parent directory: siblings share it now, and under Dask + # they are being written concurrently. What this output owns is its + # manifest and any file named after its id — the glob rather than the + # one declared path, so a spec that re-declares the output in another + # format leaves no stale payload behind. `.` needs a literal + # dot straight after the whole id, and an id cannot contain one, so it + # cannot reach a sibling, a longer id, another output's sidecar, or a + # scope directory of the same name. + task.output_path.parent.mkdir(parents=True, exist_ok=True) + task.manifest_path.unlink(missing_ok=True) + for stale in task.output_path.parent.glob(f"{task.local_id}.*"): + if stale.is_file() or stale.is_symlink(): + stale.unlink() read_paths = [p for p in task.inputs.values() if p.exists()] - policy = container.policy_for(context.runtime, read_paths, output_dir=task.output_dir) + policy = container.policy_for( + context.runtime, read_paths, write_dir=task.output_path.parent + ) started_at = _now() with sandbox.scope(policy): outcome = sandbox.run( @@ -262,11 +267,27 @@ def execute( # Guarded separately from the boundary catch above it, because these # two failures deserve different words: "your recipe failed" and "your # recipe worked and we could not record it" are different problems. + if not task.output_path.is_file(): + # `data_version` would answer for a directory — the `dir:` branch + # hashes it happily — so a recipe that ran `mkdir` on its output, + # or wrote some other name, would commit a well-formed digest of + # something that is not the output at all. + found = "a directory" if task.output_path.is_dir() else "nothing" + return TaskResult( + task.key, + "failed", + reason=( + f"the recipe exited 0 but left {found} at " + f"{plan.declared_path(root, task.output_path)}" + ), + notes=outcome.notes, + ) + try: sha, remote = context.head - data_version = assets.data_version(task.output_dir) + data_version = assets.data_version(task.output_path) assets.write( - task.output_dir, + task.manifest_path, assets.Manifest( output_id=task.output_id, universe_id=task.universe_id, @@ -425,10 +446,10 @@ def _from_disk(task: Task) -> dict[str, str]: versions: dict[str, str] = {} for name, path in task.inputs.items(): if task.produced_by.get(name) is not None: - if (manifest := assets.read(path)) is None: + if (manifest := assets.read(assets.manifest_path(path))) is None: raise ProjectError( f"the input `{name}` has never been materialized — there is no " - f"manifest in {path}. Run `lc materialize` instead." + f"manifest beside {path}. Run `lc materialize` instead." ) versions[name] = manifest.data_version else: diff --git a/tests/test_assets.py b/tests/test_assets.py index e2ef5fed..93a7ff0a 100644 --- a/tests/test_assets.py +++ b/tests/test_assets.py @@ -17,13 +17,14 @@ from __future__ import annotations +import hashlib import json from pathlib import Path import pytest from lightcone.engine import assets, dataset -from lightcone.engine.assets import Manifest, classify, data_version, output_dir +from lightcone.engine.assets import Manifest, classify, data_version, output_path from lightcone.engine.project import ProjectError @@ -52,7 +53,9 @@ def _manifest(**overrides: object) -> Manifest: def test_an_asset_is_addressed_by_its_path(tmp_path: Path) -> None: """The path in a rendered recipe is the path on disk — no staging, no scratch, no relocation.""" - assert output_dir(tmp_path, "baseline", "best_fit") == tmp_path / "results/baseline/best_fit" + assert output_path(tmp_path, "baseline", (), "best_fit", "csv") == ( + tmp_path / "results/baseline/best_fit.csv" + ) # ---- content identity ------------------------------------------------------ @@ -91,17 +94,20 @@ def test_touching_a_file_does_not_move_it(tmp_path: Path) -> None: def test_the_manifest_is_not_part_of_its_own_hash(tmp_path: Path) -> None: - """It carries the hash, so hashing it would be circular — and the - driver commits both together, so the two must agree.""" - (tmp_path / "fit.csv").write_text("x,y\n") - before = data_version(tmp_path) - assets.write(tmp_path, _manifest()) - assert data_version(tmp_path) == before + """It carries the hash, so hashing it would be circular. Structural + now rather than an exclusion: the manifest is a sidecar beside the + output, so hashing one file cannot reach it.""" + output = tmp_path / "fit.csv" + output.write_text("x,y\n") + before = data_version(output) + assets.write(assets.manifest_path(output), _manifest()) + assert data_version(output) == before def test_a_file_and_a_directory_holding_it_are_different(tmp_path: Path) -> None: - """Framed apart deliberately: a declared input can be either, and the - two must never collide.""" + """A declared input can be either, and the two must never collide. + Only the directory side is framed now — enough to keep them apart, + while a file's digest stays a plain sha256 of its own bytes.""" (tmp_path / "one").mkdir() (tmp_path / "one" / "fit.csv").write_text("x,y\n") (tmp_path / "fit.csv").write_text("x,y\n") @@ -109,25 +115,56 @@ def test_a_file_and_a_directory_holding_it_are_different(tmp_path: Path) -> None @pytest.mark.parametrize("bad", ["", "/", "..", ".", "a/b", "results/../.."]) -def test_output_dir_refuses_an_id_that_is_not_one_path_component( +def test_output_path_refuses_a_part_that_is_not_one_path_component( tmp_path: Path, bad: str ) -> None: - """The path is composed from the two ids, and a worker empties it - before running a recipe — so an id that collapses it onto a parent - would take every other universe's outputs with it.""" + """The path is composed, so a part carrying a separator would put an + output outside the tree every guard above it checked.""" with pytest.raises(ProjectError): - assets.output_dir(tmp_path, bad, "best_fit") + assets.output_path(tmp_path, bad, (), "best_fit", "csv") + with pytest.raises(ProjectError): + assets.output_path(tmp_path, "baseline", (), bad, "csv") + with pytest.raises(ProjectError): + assets.output_path(tmp_path, "baseline", (bad,), "best_fit", "csv") + + +@pytest.mark.parametrize("bad", ["", "/", "a/b", ".hidden"]) +def test_output_path_refuses_a_format_that_cannot_be_an_extension( + tmp_path: Path, bad: str +) -> None: + """A leading dot would make the output look like its own sidecar; a + separator would move it entirely.""" with pytest.raises(ProjectError): - assets.output_dir(tmp_path, "baseline", bad) + assets.output_path(tmp_path, "baseline", (), "best_fit", bad) -def test_output_dir_is_two_components_below_results(tmp_path: Path) -> None: - """The shape everything else in the layer addresses by.""" - assert assets.output_dir(tmp_path, "baseline", "best_fit") == ( - tmp_path / "results" / "baseline" / "best_fit" +def test_an_inline_sub_analysis_nests_below_its_universe(tmp_path: Path) -> None: + """Scope becomes directories, so one addressing scheme spans however + deep the spec nests without the filename carrying the structure.""" + assert assets.output_path(tmp_path, "baseline", ("hod", "mass"), "fit", "npz") == ( + tmp_path / "results" / "baseline" / "hod" / "mass" / "fit.npz" ) +def test_the_manifest_is_named_from_the_id_never_the_format(tmp_path: Path) -> None: + """A format may contain dots, and `Path.stem` would answer `x.tar` for + `x.tar.gz`. Naming the sidecar from the id alone also keeps its path — + and so its history — across a re-declared serialization.""" + packed = assets.output_path(tmp_path, "baseline", (), "chain", "tar.gz") + plain = assets.output_path(tmp_path, "baseline", (), "chain", "npz") + assert assets.manifest_path(packed).name == ".chain.manifest.json" + assert assets.manifest_path(packed) == assets.manifest_path(plain) + + +def test_a_file_hashes_its_own_bytes_and_nothing_else(tmp_path: Path) -> None: + """Unframed, so the digest is one an outsider reproduces with + `sha256sum` — and the same number the crate publishes for that file.""" + output = tmp_path / "fit.csv" + body = b"x,y\n" + output.write_bytes(body) + assert data_version(output) == f"sha256:{hashlib.sha256(body).hexdigest()}" + + def test_an_unfetched_annexed_file_is_refused_not_hashed(tmp_path: Path) -> None: """`filter=annex` leaves a pointer file where the content would be, so the path *exists* and is readable. Hashing it would be a well-formed @@ -237,35 +274,39 @@ def test_the_memo_does_not_confuse_two_inputs(tmp_path: Path) -> None: def test_a_manifest_round_trips(tmp_path: Path) -> None: written = _manifest() - assets.write(tmp_path, written) - assert assets.read(tmp_path) == written + sidecar = assets.manifest_path(tmp_path / "best_fit.csv") + assets.write(sidecar, written) + assert assets.read(sidecar) == written def test_the_manifest_is_readable_json_with_the_schema_first(tmp_path: Path) -> None: """It stays out of the annex precisely so a clone with no content fetched can read it — including with a plain `grep`.""" - assets.write(tmp_path, _manifest()) - text = (tmp_path / assets.MANIFEST_FILENAME).read_text() + sidecar = assets.manifest_path(tmp_path / "best_fit.csv") + assets.write(sidecar, _manifest()) + text = sidecar.read_text() assert next(iter(json.loads(text))) == "schema_version" assert "best_fit" in text def test_no_manifest_reads_as_none(tmp_path: Path) -> None: - assert assets.read(tmp_path) is None + assert assets.read(assets.manifest_path(tmp_path / "best_fit.csv")) is None def test_an_unparseable_manifest_reads_as_none(tmp_path: Path) -> None: """The safe direction: an unreadable record means make it again, not trust it.""" - (tmp_path / assets.MANIFEST_FILENAME).write_text("{not json") - assert assets.read(tmp_path) is None + sidecar = assets.manifest_path(tmp_path / "best_fit.csv") + sidecar.write_text("{not json") + assert assets.read(sidecar) is None def test_writing_a_manifest_replaces_the_previous_one_whole(tmp_path: Path) -> None: - assets.write(tmp_path, _manifest()) - assets.write(tmp_path, _manifest(data_version="sha256:second")) + sidecar = assets.manifest_path(tmp_path / "best_fit.csv") + assets.write(sidecar, _manifest()) + assets.write(sidecar, _manifest(data_version="sha256:second")) - manifest = assets.read(tmp_path) + manifest = assets.read(sidecar) assert manifest is not None and manifest.data_version == "sha256:second" assert not list(tmp_path.glob("*.tmp")) diff --git a/tests/test_container_smoke.py b/tests/test_container_smoke.py index 5010aed9..ceb11a07 100644 --- a/tests/test_container_smoke.py +++ b/tests/test_container_smoke.py @@ -42,8 +42,9 @@ outputs: - id: sums type: metric + format: txt recipe: - command: echo "2+2" | bc > {output}/sum.txt + command: echo "2+2" | bc > {output} """ @@ -218,10 +219,10 @@ def test_materialize_end_to_end_in_the_image(runtime: str, cproject: Path) -> No assert report.ok, report.warnings assert report.made == ["baseline/sums"] - assert (cproject / "results/baseline/sums/sum.txt").read_text() == "4\n" + assert (cproject / "results/baseline/sums.txt").read_text() == "4\n" assert not dataset.status(cproject) - manifest = assets.read(cproject / "results/baseline/sums") + manifest = assets.read(cproject / "results/baseline/.sums.manifest.json") assert manifest is not None assert manifest.hermeticity["mechanism"] == runtime assert manifest.hermeticity["network"] == "allowed" @@ -259,7 +260,7 @@ def test_a_rerun_on_a_clone_fetches_the_archive_and_reproduces( monkeypatch.setattr(engine, "_engine_requirement", lambda: f"lightcone-cli=={version}") report = engine.materialize(cproject, []) assert report.ok, report.warnings - original = assets.read(cproject / "results/baseline/sums") + original = assets.read(cproject / "results/baseline/.sums.manifest.json") assert original is not None clone = tmp_path / "clone" @@ -280,7 +281,7 @@ def test_a_rerun_on_a_clone_fetches_the_archive_and_reproduces( ) assert proc.returncode == 0, proc.stderr - rerun = assets.read(clone / "results/baseline/sums") + rerun = assets.read(clone / "results/baseline/.sums.manifest.json") assert rerun is not None assert rerun.data_version == original.data_version assert not dataset.status(clone) diff --git a/tests/test_crate.py b/tests/test_crate.py index 99a4872c..4b6edec6 100644 --- a/tests/test_crate.py +++ b/tests/test_crate.py @@ -22,7 +22,7 @@ _DSID = "4b7b5c1e-0000-4000-8000-000000000000" -Writer = Callable[[Path], LastWrite] +Writer = Callable[..., LastWrite] @pytest.fixture @@ -50,11 +50,11 @@ def _made( inputs: dict[str, str] | None = None, finished_at: str = "2026-08-19T10:05:00.000+00:00", ) -> Path: - directory = root / "results" / universe_id / output_id - directory.mkdir(parents=True) - (directory / "out.txt").write_text(f"{universe_id}/{output_id}\n") + output = root / "results" / universe_id / f"{output_id}.txt" + output.parent.mkdir(parents=True, exist_ok=True) + output.write_text(f"{universe_id}/{output_id}\n") assets.write( - directory, + assets.manifest_path(output), assets.Manifest( output_id=output_id, universe_id=universe_id, @@ -73,17 +73,19 @@ def _made( image=image, ), ) - return directory + return output def _graph(root: Path, universes: tuple[str, ...] = ("baseline",)) -> Graph: tasks: dict[Key, Task] = {} for universe_id in universes: - first_dir = root / "results" / universe_id / "first" + first = root / "results" / universe_id / "first.txt" tasks[(universe_id, "first")] = Task( universe_id, "first", - first_dir, + "first", + root, + first, "make first", {"catalog": root / "data" / "catalog.csv"}, {}, @@ -93,9 +95,11 @@ def _graph(root: Path, universes: tuple[str, ...] = ("baseline",)) -> Graph: tasks[(universe_id, "second")] = Task( universe_id, "second", - root / "results" / universe_id / "second", + "second", + root, + root / "results" / universe_id / "second.txt", "make second", - {"first": first_dir}, + {"first": first}, {"first": (universe_id, "first")}, {"method": "alpha"}, "sha256:def", @@ -103,7 +107,7 @@ def _graph(root: Path, universes: tuple[str, ...] = ("baseline",)) -> Graph: return Graph(tasks) -def _writer(path: Path) -> LastWrite: +def _writer(*paths: Path) -> LastWrite: return LastWrite("a" * 40, "irrelevant", "Ada Lovelace", "ada@example.org", "2026-08-19") @@ -133,7 +137,7 @@ def test_rendering_twice_at_the_same_state_is_byte_identical(project: Path) -> N _made(project, "baseline", "second", git_sha="aaa111") graph = _graph(project) - keys = {"results/baseline/first/out.txt": "SHA256E-s24--" + "c" * 64 + ".txt"} + keys = {"results/baseline/first.txt": "SHA256E-s24--" + "c" * 64 + ".txt"} first = crate.render(project, graph, license="MIT", dsid=_DSID, writer=_writer, keys=keys) second = crate.render(project, graph, license="MIT", dsid=_DSID, writer=_writer, keys=keys) @@ -229,20 +233,20 @@ def test_an_action_chains_its_inputs_and_its_environment(project: Path) -> None: second = actions["run of `second` in universe `baseline`"] first_objects = {ref["@id"] for ref in first["object"]} assert {"uv.lock", ".python-version", "pyproject.toml", "data/catalog.csv"} <= first_objects - assert "results/baseline/first/" in {ref["@id"] for ref in second["object"]} - assert second["result"] == [{"@id": "results/baseline/second/"}] + assert "results/baseline/first.txt" in {ref["@id"] for ref in second["object"]} + assert second["result"] == [{"@id": "results/baseline/second.txt"}] assert second["description"] == "make second" - assert entities["results/baseline/second/"]["version"] == "sha256:baseline-second" + assert entities["results/baseline/second.txt"]["version"] == "sha256:baseline-second" def test_the_manifest_is_in_the_crate_and_about_its_dataset(project: Path) -> None: _made(project, "baseline", "first", git_sha="aaa111") entities = _entities(_render(project, _graph(project))) - manifest = entities["results/baseline/first/.lightcone-manifest.json"] - assert manifest["about"] == {"@id": "results/baseline/first/"} - assert entities["results/baseline/first/"]["subjectOf"] == { - "@id": "results/baseline/first/.lightcone-manifest.json" + manifest = entities["results/baseline/.first.manifest.json"] + assert manifest["about"] == {"@id": "results/baseline/first.txt"} + assert entities["results/baseline/first.txt"]["subjectOf"] == { + "@id": "results/baseline/.first.manifest.json" } @@ -322,34 +326,40 @@ def test_the_license_is_a_local_entity_never_a_minted_url(project: Path) -> None # ---- per-file integrity ---------------------------------------------------- -def test_output_files_carry_checksums_from_their_annex_keys(project: Path) -> None: +def test_an_output_carries_the_checksum_from_its_annex_key(project: Path) -> None: """The keys are repository state, so a bytes-free clone renders the same claims — and the hex in a SHA256E key is the raw sha256 an - archive can verify with `sha256sum` after a `git archive` deposit, - where the dataset's `version` is lc's framed digest and cannot be.""" + archive can verify with `sha256sum` after a `git archive` deposit.""" _made(project, "baseline", "first", git_sha="aaa111") digest = "d" * 64 - keys = {"results/baseline/first/out.txt": f"SHA256E-s21--{digest}.txt"} + keys = {"results/baseline/first.txt": f"SHA256E-s21--{digest}.txt"} entities = _entities(_render(project, _graph(project), keys=keys)) - part = entities["results/baseline/first/out.txt"] - assert part["sha256"] == digest - assert part["contentSize"] == "21" - parts = {ref["@id"] for ref in entities["results/baseline/first/"]["hasPart"]} - assert parts == { - "results/baseline/first/.lightcone-manifest.json", - "results/baseline/first/out.txt", - } + output = entities["results/baseline/first.txt"] + assert output["@type"] == "File" + assert output["sha256"] == digest + assert output["contentSize"] == "21" + + +def test_an_output_is_one_file_not_a_dataset_of_parts(project: Path) -> None: + """One output is one file, so there is nothing to enumerate — and the + manifest hangs off it as `subjectOf` rather than as a part of it.""" + _made(project, "baseline", "first", git_sha="aaa111") + entities = _entities(_render(project, _graph(project))) + + output = entities["results/baseline/first.txt"] + assert "hasPart" not in output + assert output["subjectOf"] == {"@id": "results/baseline/.first.manifest.json"} def test_a_non_sha256_key_yields_size_and_no_digest(project: Path) -> None: """`annex.backend` is the researcher's to set, and a wrong checksum is worse than none — the publish-neither discipline.""" _made(project, "baseline", "first", git_sha="aaa111") - keys = {"results/baseline/first/out.txt": "MD5E-s21--" + "e" * 32 + ".txt"} + keys = {"results/baseline/first.txt": "MD5E-s21--" + "e" * 32 + ".txt"} entities = _entities(_render(project, _graph(project), keys=keys)) - part = entities["results/baseline/first/out.txt"] + part = entities["results/baseline/first.txt"] assert part["contentSize"] == "21" assert "sha256" not in part @@ -396,7 +406,9 @@ def test_an_out_of_tree_input_publishes_no_checksum( task = Task( "baseline", "first", - project / "results/baseline/first", + "first", + project, + project / "results/baseline/first.txt", "make first", {"catalog": catalog}, {}, diff --git a/tests/test_crate_smoke.py b/tests/test_crate_smoke.py index c7bf0847..f1c71d08 100644 --- a/tests/test_crate_smoke.py +++ b/tests/test_crate_smoke.py @@ -58,15 +58,17 @@ outputs: - id: first type: metric + format: txt decisions: [method] recipe: - command: echo {decisions.method} > {output}/value.txt + command: echo {decisions.method} > {output} - id: second type: report + format: txt inputs: [first] recipe: - command: cat {inputs.first}/value.txt > {output}/copy.txt + command: cat {inputs.first} > {output} decisions: method: diff --git a/tests/test_dataset.py b/tests/test_dataset.py index 9e4dc1c6..d5fb254b 100644 --- a/tests/test_dataset.py +++ b/tests/test_dataset.py @@ -71,29 +71,44 @@ def test_save_puts_result_bytes_in_the_annex_and_the_manifest_in_git(repo: Path) """The whole storage policy, exercised end to end. The manifest has to stay a plain git blob: `lc` reads it on clones that have fetched no annex content at all.""" - output = repo / "results" / "baseline" / "best_fit" - output.mkdir(parents=True) - (output / "fit.csv").write_text("a,b\n1,2\n") - (output / ".lightcone-manifest.json").write_text('{"data_version": "abc"}\n') + results = repo / "results" / "baseline" + results.mkdir(parents=True) + (results / "best_fit.csv").write_text("a,b\n1,2\n") + (results / ".best_fit.manifest.json").write_text('{"data_version": "abc"}\n') - assert dataset.save(repo, [output], "materialize best_fit") + assert dataset.save(repo, [results], "materialize best_fit") - assert _annexed(repo, output / "fit.csv") - assert not _annexed(repo, output / ".lightcone-manifest.json") + assert _annexed(repo, results / "best_fit.csv") + assert not _annexed(repo, results / ".best_fit.manifest.json") assert not dataset.status(repo) +def test_a_sub_analysis_results_tree_is_routed_the_same_way(repo: Path) -> None: + """An analysis keeps its results beside its own astra.yaml, so the + routing rule cannot be anchored at the project root — a sub-analysis's + payload would land in git as a plain blob, invisibly.""" + results = repo / "hod" / "results" / "fast" + results.mkdir(parents=True) + (results / "mass_function.npz").write_bytes(b"binned\n" * 64) + (results / ".mass_function.manifest.json").write_text("{}\n") + + assert dataset.save(repo, [results], "materialize the sub-analysis") + + assert _annexed(repo, results / "mass_function.npz") + assert not _annexed(repo, results / ".mass_function.manifest.json") + + def test_a_plain_git_add_annexes_content_by_itself(repo: Path) -> None: """`filter=annex` is what makes git's own add route content, which is what lets lc — and everyone else — never run a git-annex command.""" - output = repo / "results" / "baseline" / "best_fit" - output.mkdir(parents=True) - (output / "fit.csv").write_text("a,b\n1,2\n") + results = repo / "results" / "baseline" + results.mkdir(parents=True) + (results / "best_fit.csv").write_text("a,b\n1,2\n") dataset._git(["add", "-A", "--", "results"], cwd=repo) dataset._git(["commit", "-q", "-m", "plain git"], cwd=repo) - assert _annexed(repo, output / "fit.csv") + assert _annexed(repo, results / "best_fit.csv") def test_dot_paths_follow_the_storage_policy_not_annex_defaults(repo: Path) -> None: @@ -109,16 +124,16 @@ def test_dot_paths_follow_the_storage_policy_not_annex_defaults(repo: Path) -> N archive = repo / ".datalad" / "environments" / "lc-env-abc" / "image" archive.parent.mkdir(parents=True) archive.write_bytes(b"pretend image bytes\n" * 64) - output = repo / "results" / "baseline" / "fit" + output = repo / "results" / "baseline" output.mkdir(parents=True) (output / ".cache.h5").write_bytes(b"intermediate\n" * 64) - (output / ".lightcone-manifest.json").write_text("{}\n") + (output / ".fit.manifest.json").write_text("{}\n") dataset.save(repo, [archive.parent, output], "routed") assert _annexed(repo, archive) assert _annexed(repo, output / ".cache.h5") - assert not _annexed(repo, output / ".lightcone-manifest.json") + assert not _annexed(repo, output / ".fit.manifest.json") plain = repo / ".datalad" / "environments" / "lc-env-def" / "image" plain.parent.mkdir(parents=True) diff --git a/tests/test_identity.py b/tests/test_identity.py index 594703c9..707a23cf 100644 --- a/tests/test_identity.py +++ b/tests/test_identity.py @@ -171,19 +171,34 @@ def test_fields_cannot_shift_into_one_another(root: Path) -> None: # ---- definition_version ---------------------------------------------------- -def test_definition_version_follows_both_its_terms() -> None: +def test_definition_version_follows_all_three_terms() -> None: recipe, decisions = "python fit.py {output}", {"method": "mcmc"} - original = definition_version(recipe=recipe, decisions=decisions) + original = definition_version(recipe=recipe, decisions=decisions, fmt="csv") - assert definition_version(recipe=recipe + " -v", decisions=decisions) != original - assert definition_version(recipe=recipe, decisions={"method": "nested"}) != original + assert definition_version(recipe=recipe + " -v", decisions=decisions, fmt="csv") != original + assert ( + definition_version(recipe=recipe, decisions={"method": "nested"}, fmt="csv") != original + ) + assert definition_version(recipe=recipe, decisions=decisions, fmt="parquet") != original + + +def test_the_format_moves_it_even_when_the_recipe_never_names_the_output() -> None: + """A recipe need not use `{output}` at all, so the format cannot be + left to arrive through the rendered path. One that did would keep its + digest across a re-declared serialization — and the sidecar, named from + the id alone, does not move either, so the manifest would go on + describing an output at a path that no longer exists.""" + fixed = "python fit.py --out results/baseline/fit.csv" + assert definition_version(recipe=fixed, decisions={}, fmt="csv") != definition_version( + recipe=fixed, decisions={}, fmt="parquet" + ) def test_decision_order_does_not_matter() -> None: """Decisions are a mapping, not a sequence — two spellings of the same choices define the same output.""" - a = definition_version(recipe="r", decisions={"x": "1", "y": "2"}) - b = definition_version(recipe="r", decisions={"y": "2", "x": "1"}) + a = definition_version(recipe="r", decisions={"x": "1", "y": "2"}, fmt="csv") + b = definition_version(recipe="r", decisions={"y": "2", "x": "1"}, fmt="csv") assert a == b @@ -191,10 +206,10 @@ def test_the_environment_is_not_part_of_what_an_output_is(root: Path) -> None: """The load-bearing separation. `env_version` is recorded beside an output and compared to say it is *behind*; folding it in here would make one added dependency remake a project's every result.""" - before = definition_version(recipe="r", decisions={}) + before = definition_version(recipe="r", decisions={}, fmt="csv") (root / ".python-version").write_text("3.12.9\n") assert env_version(root) # the environment did move - assert definition_version(recipe="r", decisions={}) == before + assert definition_version(recipe="r", decisions={}, fmt="csv") == before # ---- the lock scan --------------------------------------------------------- diff --git a/tests/test_materialize.py b/tests/test_materialize.py index 5ff96e8d..49c7d6ab 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -40,15 +40,17 @@ outputs: - id: first type: metric + format: txt decisions: [method] recipe: - command: echo {decisions.method} > {output}/value.txt + command: echo {decisions.method} > {output} - id: second type: report + format: txt inputs: [first] recipe: - command: cat {inputs.first}/value.txt > {output}/copy.txt + command: cat {inputs.first} > {output} decisions: method: @@ -91,7 +93,7 @@ def test_every_output_is_made_and_committed(root: Path, inline: None) -> None: assert report.made == ["baseline/first", "baseline/second"] assert report.ok and not report.up_to_date - assert (root / "results/baseline/second/copy.txt").read_text() == "alpha\n" + assert (root / "results/baseline/second.txt").read_text() == "alpha\n" assert _commits(root) == before + 2 assert not dataset.status(root) @@ -105,8 +107,8 @@ def test_an_output_and_its_manifest_land_in_one_commit(root: Path, inline: None) ["show", "--name-only", "--format=", "HEAD"], cwd=root ).split() assert sorted(committed) == [ - "results/baseline/first/.lightcone-manifest.json", - "results/baseline/first/value.txt", + "results/baseline/.first.manifest.json", + "results/baseline/first.txt", ] @@ -118,8 +120,8 @@ def test_the_bytes_go_to_the_annex_and_the_manifest_to_git(root: Path, inline: N def blob(rel: str) -> str: return dataset._git(["cat-file", "-p", f"HEAD:{rel}"], cwd=root) - assert blob("results/baseline/first/value.txt").startswith("/annex/objects/") - assert blob("results/baseline/first/.lightcone-manifest.json").startswith("{") + assert blob("results/baseline/first.txt").startswith("/annex/objects/") + assert blob("results/baseline/.first.manifest.json").startswith("{") def test_a_second_run_does_nothing_and_commits_nothing(root: Path, inline: None) -> None: @@ -159,7 +161,7 @@ def test_a_moved_environment_is_reported_and_nothing_is_remade( so it is reported and left where it is.""" engine.materialize(root, []) after_first = _commits(root) - first = (root / "results/baseline/first/value.txt").read_text() + first = (root / "results/baseline/first.txt").read_text() _move_the_environment(root) report = engine.materialize(root, []) @@ -169,7 +171,7 @@ def test_a_moved_environment_is_reported_and_nothing_is_remade( assert "earlier environment" in report.behind["baseline/first"] assert report.up_to_date, "behind is not out of date" assert _commits(root) == after_first + 1, "only the environment edit" - assert (root / "results/baseline/first/value.txt").read_text() == first + assert (root / "results/baseline/first.txt").read_text() == first def test_refresh_remakes_what_is_behind_and_commits_it(root: Path, inline: None) -> None: @@ -184,7 +186,7 @@ def test_refresh_remakes_what_is_behind_and_commits_it(root: Path, inline: None) assert set(report.made) == {"baseline/first", "baseline/second"} assert report.behind == {} assert _commits(root) == before + 2 - manifest = assets.read(root / "results/baseline/first") + manifest = assets.read(root / "results/baseline/.first.manifest.json") assert manifest is not None assert manifest.env_version == identity.env_version(root) @@ -198,7 +200,7 @@ def test_the_manifest_records_the_uv_that_converged_the_environment( engine.materialize(root, ["first"]) - manifest = assets.read(root / "results/baseline/first") + manifest = assets.read(root / "results/baseline/.first.manifest.json") assert manifest is not None assert manifest.uv_version == project.uv_version(root) assert manifest.uv_version.count(".") >= 1, "a real version token, not prose" @@ -293,7 +295,7 @@ def test_status_does_not_mind_a_dirty_tree(root: Path, inline: None) -> None: """It reads. Refusing here would make the one verb that tells you what state you are in unavailable exactly when you need it.""" engine.materialize(root, []) - (root / "results/baseline/first/value.txt").write_text("edited by hand\n") + (root / "results/baseline/first.txt").write_text("edited by hand\n") assert engine.status(root).counts["current"] == 2 @@ -314,7 +316,7 @@ def test_a_changed_decision_remakes_the_output_and_its_dependents( report = engine.materialize(root, []) assert report.made == ["baseline/first", "baseline/second"] - assert (root / "results/baseline/second/copy.txt").read_text() == "beta\n" + assert (root / "results/baseline/second.txt").read_text() == "beta\n" def test_the_previous_bytes_are_still_there_at_the_previous_commit( @@ -327,11 +329,11 @@ def test_the_previous_bytes_are_still_there_at_the_previous_commit( (root / "universes" / "baseline.yaml").write_text("id: baseline\ndecisions:\n method: beta\n") dataset.save(root, [root], "switch method") engine.materialize(root, ["first"]) - assert (root / "results/baseline/first/value.txt").read_text() == "beta\n" + assert (root / "results/baseline/first.txt").read_text() == "beta\n" - dataset._git(["checkout", original, "--", "results/baseline/first"], cwd=root) + dataset._git(["checkout", original, "--", "results/baseline/first.txt"], cwd=root) - assert (root / "results/baseline/first/value.txt").read_text() == "alpha\n" + assert (root / "results/baseline/first.txt").read_text() == "alpha\n" # ---- check mode ------------------------------------------------------------ @@ -350,7 +352,7 @@ def test_check_writes_nothing_and_commits_nothing(root: Path) -> None: engine.check(root, []) - assert not (root / "results/baseline/first").exists() + assert not (root / "results/baseline/first.txt").exists() assert _commits(root) == before @@ -388,14 +390,14 @@ def test_a_dirty_tree_refuses_and_says_what_to_do_about_each_path( committed, and anything under `results/` is lc's to write.""" engine.materialize(root, ["first"]) (root / "notes.md").write_text("in progress\n") - (root / "results/baseline/first/stray.txt").write_text("by hand\n") + (root / "results/baseline/stray.txt").write_text("by hand\n") with pytest.raises(ProjectError) as raised: engine.materialize(root, []) message = str(raised.value) assert "commit these" in message and "notes.md" in message - assert "discard these" in message and "results/baseline/first/stray.txt" in message + assert "discard these" in message and "results/baseline/stray.txt" in message def _consuming(source: str) -> str: @@ -568,7 +570,7 @@ def test_a_failing_recipe_commits_nothing_and_leaves_the_tree_clean( """The invariant that makes the dirty-tree refusal survivable: the next run must not tell the user to commit truncated, manifest-less garbage.""" - spec = _SPEC.replace("echo {decisions.method} > {output}/value.txt", "exit 1") + spec = _SPEC.replace("echo {decisions.method} > {output}", "exit 1") root = analysis(spec, universes={"baseline": _UNIVERSE}) before = _commits(root) @@ -587,7 +589,7 @@ def test_a_run_in_which_everything_failed_is_not_up_to_date( """`made` stays empty when every recipe fails, so `up_to_date` alone read "nothing to do" over a list of failures — and it is the second key of the JSON report, which is what an agent branches on.""" - spec = _SPEC.replace("echo {decisions.method} > {output}/value.txt", "exit 1") + spec = _SPEC.replace("echo {decisions.method} > {output}", "exit 1") root = analysis(spec, universes={"baseline": _UNIVERSE}) report = engine.materialize(root, []) @@ -604,7 +606,7 @@ def test_a_rebuild_that_fails_puts_the_previous_output_back( ) -> None: engine.materialize(root, ["first"]) (root / "astra.yaml").write_text( - _SPEC.replace("echo {decisions.method} > {output}/value.txt", "exit 1") + _SPEC.replace("echo {decisions.method} > {output}", "exit 1") ) dataset.save(root, [root], "break the recipe") at_break = _commits(root) @@ -612,7 +614,7 @@ def test_a_rebuild_that_fails_puts_the_previous_output_back( report = engine.materialize(root, ["first"]) assert report.failed == ["baseline/first"] - assert (root / "results/baseline/first/value.txt").read_text() == "alpha\n" + assert (root / "results/baseline/first.txt").read_text() == "alpha\n" assert _commits(root) == at_break assert not dataset.status(root) @@ -666,8 +668,11 @@ def test_the_run_record_is_what_datalad_reads( "uv run --no-project --with 'lightcone-cli==1.2.3' -- " "python -m lightcone.engine.worker baseline/second" ) - assert info["inputs"] == ["results/baseline/first"] - assert info["outputs"] == ["results/baseline/second"] + assert info["inputs"] == ["results/baseline/first.txt"] + assert info["outputs"] == [ + "results/baseline/second.txt", + "results/baseline/.second.manifest.json", + ] assert info["dsid"] == "4b7b5c1e-0000-4000-8000-000000000000" assert info["chain"] == [] and info["pwd"] == "." @@ -708,9 +713,10 @@ def test_the_record_names_the_declared_input_not_the_annex_object( outputs: - id: fit type: metric + format: txt inputs: [catalog] recipe: - command: cat {inputs.catalog} > {output}/seen.txt + command: cat {inputs.catalog} > {output} """ root = analysis(spec, files={"data/catalog.txt": "measured\n"}) dataset.save(root, [root / "data"], "the catalog") @@ -732,7 +738,9 @@ def test_every_manifest_of_one_run_names_the_same_commit(root: Path, inline: Non engine.materialize(root, []) shas = { - assets.read(root / "results/baseline" / name).git_sha # type: ignore[union-attr] + assets.read( # type: ignore[union-attr] + root / "results/baseline" / f".{name}.manifest.json" + ).git_sha for name in ("first", "second") } assert len(shas) == 1 @@ -746,7 +754,7 @@ def test_check_agrees_with_a_run_on_a_clone_with_no_annex_content( than the pointer file sitting in its place.""" engine.materialize(root, []) clone = _clone(root, tmp_path) - pointer = (clone / "results/baseline/first/value.txt").read_text() + pointer = (clone / "results/baseline/first.txt").read_text() assert pointer.startswith("/annex/objects/") # content really is absent assert engine.check(clone, []).planned == {} @@ -781,9 +789,10 @@ def _clone(root: Path, into: Path) -> Path: outputs: - id: copy type: metric + format: txt inputs: [catalog] recipe: - command: cat {inputs.catalog} > {output}/copy.txt + command: cat {inputs.catalog} > {output} """ @@ -881,7 +890,7 @@ def test_the_recorded_command_reproduces_the_output( # working tree — the code actually under test. monkeypatch.setattr(engine, "_engine_requirement", lambda: f"lightcone-cli=={version}") engine.materialize(root, ["first"]) - original = assets.read(root / "results/baseline/first") + original = assets.read(root / "results/baseline/.first.manifest.json") assert original is not None proc = subprocess.run( @@ -893,10 +902,10 @@ def test_the_recorded_command_reproduces_the_output( ) assert proc.returncode == 0, proc.stderr - rerun = assets.read(root / "results/baseline/first") + rerun = assets.read(root / "results/baseline/.first.manifest.json") assert rerun is not None assert rerun.data_version == original.data_version - assert rerun.data_version == assets.data_version(root / "results/baseline/first") + assert rerun.data_version == assets.data_version(root / "results/baseline/first.txt") assert not dataset.status(root) @@ -916,7 +925,7 @@ def test_the_recorded_command_holds_on_a_fresh_clone( version, dist = engine_dist monkeypatch.setattr(engine, "_engine_requirement", lambda: f"lightcone-cli=={version}") engine.materialize(root, ["first"]) - original = assets.read(root / "results/baseline/first") + original = assets.read(root / "results/baseline/.first.manifest.json") assert original is not None clone = _clone(root, tmp_path) @@ -931,7 +940,7 @@ def test_the_recorded_command_holds_on_a_fresh_clone( ) assert proc.returncode == 0, proc.stderr - rerun = assets.read(clone / "results/baseline/first") + rerun = assets.read(clone / "results/baseline/.first.manifest.json") assert rerun is not None assert rerun.data_version == original.data_version assert (clone / ".venv").exists() @@ -988,6 +997,29 @@ def test_the_report_is_json_ready(root: Path, inline: None) -> None: # ---- the foreign-write fact ------------------------------------------------ +def test_a_manifest_the_attributes_would_annex_refuses_before_the_graph_runs( + root: Path, inline: None +) -> None: + """The silent failure this guard exists for: `save` opts dot-paths out + of git-annex's stock routing, so the leading dot decides nothing and + `.gitattributes` decides everything. Annexed, a clone with no content + reads a pointer where the manifest should be and reports every output + as never materialized — with nothing said. Refused up front, before + any recipe has spent anything.""" + attributes = root / ".gitattributes" + attributes.write_text( + attributes.read_text().replace( + "**/results/**/.*.manifest.json annex.largefiles=nothing", "" + ) + ) + dataset.save(root, [attributes], "route manifests into the annex") + + with pytest.raises(ProjectError, match="plain git blob"): + engine.materialize(root, []) + + assert not (root / "results" / "baseline").exists(), "it refused before running anything" + + def _forge(path: Path, text: str) -> None: """Overwrite a committed result the way a hand edit would — unlinking first, because results are committed thin: an in-place truncate would @@ -1010,14 +1042,14 @@ def test_a_foreign_write_is_stale_and_names_its_commit(root: Path, inline: None) so a directory last written by anything but its own run record is a *contradiction*, and contradiction is what `stale` means.""" engine.materialize(root, []) - forged = root / "results" / "baseline" / "first" / "value.txt" + forged = root / "results" / "baseline" / "first.txt" _forge(forged, "curated by hand\n") - dataset.save(root, [forged.parent], "tweak colors") + dataset.save(root, [forged], "tweak colors") outputs = {o.output: o for o in engine.status(root).outputs} assert outputs["baseline/first"].status == "stale" - forged_sha = dataset.last_writer(root, root / "results/baseline/first").sha + forged_sha = dataset.last_writer(root, root / "results/baseline/first.txt").sha assert outputs["baseline/first"].foreign_write == forged_sha assert "tweak colors" in outputs["baseline/first"].why assert "git show" in outputs["baseline/first"].why @@ -1030,9 +1062,9 @@ def test_check_plans_the_remake_of_a_foreign_written_output( """Status and `--check` answer from one walk, so they cannot disagree about a foreign write — and the gate exits nonzero over it.""" engine.materialize(root, []) - forged = root / "results" / "baseline" / "first" / "value.txt" + forged = root / "results" / "baseline" / "first.txt" _forge(forged, "curated by hand\n") - dataset.save(root, [forged.parent], "tweak colors") + dataset.save(root, [forged], "tweak colors") report = engine.check(root, []) @@ -1047,9 +1079,9 @@ def test_the_foreign_write_fact_survives_a_bytes_free_clone( the fact needs no annex content — where a rehash would have nothing to hash.""" engine.materialize(root, []) - forged = root / "results" / "baseline" / "first" / "value.txt" + forged = root / "results" / "baseline" / "first.txt" _forge(forged, "curated by hand\n") - dataset.save(root, [forged.parent], "tweak colors") + dataset.save(root, [forged], "tweak colors") clone = _clone(root, tmp_path) outputs = {o.output: o for o in engine.status(clone).outputs} @@ -1065,9 +1097,9 @@ def test_the_next_run_remakes_a_foreign_written_output(root: Path, inline: None) refusal's path split — so a committed hand edit is remade, and the rebuild's own run record becomes the last writer again.""" engine.materialize(root, []) - forged = root / "results" / "baseline" / "first" / "value.txt" + forged = root / "results" / "baseline" / "first.txt" _forge(forged, "curated by hand\n") - dataset.save(root, [forged.parent], "tweak colors") + dataset.save(root, [forged], "tweak colors") report = engine.materialize(root, []) @@ -1110,7 +1142,7 @@ def test_a_licensed_materialize_converges_the_crate_and_commits_it( graph = json.loads(crate_path.read_text())["@graph"] types = {e["@id"]: e["@type"] for e in graph} assert "OrganizeAction" in types.values() - assert types["results/baseline/first/"] == "Dataset" + assert types["results/baseline/first.txt"] == "File" def test_an_idempotent_rerun_commits_nothing(root: Path, inline: None) -> None: @@ -1172,11 +1204,11 @@ def test_status_sees_the_crate_lag_a_rerun_leaves(root: Path, inline: None) -> N _declare_license(root) engine.materialize(root, []) - directory = root / "results/baseline/second" - manifest = assets.read(directory) + sidecar = root / "results/baseline/.second.manifest.json" + manifest = assets.read(sidecar) assert manifest is not None - assets.write(directory, replace(manifest, finished_at="2027-01-01T00:00:00.000+00:00")) - dataset.save(root, [directory], "a rerun-shaped manifest rewrite") + assets.write(sidecar, replace(manifest, finished_at="2027-01-01T00:00:00.000+00:00")) + dataset.save(root, [sidecar], "a rerun-shaped manifest rewrite") assert engine.status(root).crate.startswith("behind") @@ -1195,6 +1227,6 @@ def test_an_output_the_spec_dropped_is_excluded_and_named(root: Path, inline: No report = engine.materialize(root, []) - assert any("results/baseline/second" in w for w in report.warnings) + assert any(".second.manifest.json" in w for w in report.warnings) document = (root / "ro-crate-metadata.json").read_text() - assert "results/baseline/second/" not in document + assert "results/baseline/second.txt" not in document diff --git a/tests/test_plan.py b/tests/test_plan.py index 0eb58be8..c4271d23 100644 --- a/tests/test_plan.py +++ b/tests/test_plan.py @@ -30,6 +30,7 @@ outputs: - id: fit type: metric + format: json inputs: [catalog] decisions: [method] recipe: @@ -37,6 +38,7 @@ - id: report type: report + format: md inputs: [fit] recipe: command: python src/report.py {inputs.fit} {output} @@ -113,10 +115,11 @@ def test_an_output_that_ignores_a_decision_is_not_moved_by_it(tmp_path: Path) -> assert a.tasks[key].definition_version == b.tasks[key].definition_version -def test_an_output_addresses_its_own_directory(tmp_path: Path) -> None: +def test_an_output_addresses_its_own_file(tmp_path: Path) -> None: task = _build(_project(tmp_path)).tasks[("baseline", "fit")] - assert task.output_dir == tmp_path / "results" / "baseline" / "fit" - assert "results/baseline/fit" in task.recipe + assert task.output_path == tmp_path / "results" / "baseline" / "fit.json" + assert task.manifest_path == tmp_path / "results" / "baseline" / ".fit.manifest.json" + assert "results/baseline/fit.json" in task.recipe def test_a_declared_input_resolves_to_its_source(tmp_path: Path) -> None: @@ -274,6 +277,7 @@ def test_no_universe_is_a_clean_error(tmp_path: Path) -> None: - id: summary type: report + format: md inputs: [mass_function] recipe: command: python summarize.py {inputs.mass_function} {output} @@ -295,6 +299,7 @@ def test_no_universe_is_a_clean_error(tmp_path: Path) -> None: outputs: - id: mass_function type: metric + format: npz inputs: [catalog] decisions: [binning] recipe: @@ -311,6 +316,7 @@ def test_no_universe_is_a_clean_error(tmp_path: Path) -> None: def _tree(root: Path) -> Path: + root.mkdir(parents=True, exist_ok=True) (root / "astra.yaml").write_text(textwrap.dedent(_PARENT)) (root / "universes").mkdir() # The sub-analysis's universe is named explicitly: ASTRA has no @@ -325,18 +331,72 @@ def _tree(root: Path) -> Path: return root -def test_a_sub_analysis_output_is_addressed_flat_and_qualified(tmp_path: Path) -> None: - """One addressing scheme and one place to look, whatever shape the - spec has.""" +def test_a_sub_analysis_is_addressed_by_its_qualified_id(tmp_path: Path) -> None: + """One addressing scheme whatever shape the spec has: only the path + nests, never the key.""" graph = _build(_tree(tmp_path)) assert sorted(graph.tasks) == [("baseline", "hod.mass_function"), ("baseline", "summary")] - task = graph.tasks[("baseline", "hod.mass_function")] - assert task.output_dir == tmp_path / "results" / "baseline" / "hod.mass_function" +def test_an_external_sub_analysis_keeps_results_beside_its_own_spec(tmp_path: Path) -> None: + """A sub-analysis with a `path:` is a self-similar analysis — its own + astra.yaml, its own universes, and so its own results tree.""" + root = _tree(tmp_path) + task = _build(root).tasks[("baseline", "hod.mass_function")] + assert task.output_path == root / "analyses/hod/results/baseline/mass_function.npz" + assert task.manifest_path == ( + root / "analyses/hod/results/baseline/.mass_function.manifest.json" + ) + + +def test_a_consumer_is_handed_the_upstream_file_not_its_directory(tmp_path: Path) -> None: + """The filename contract the spec now carries: a downstream recipe is + given the actual file, so nothing has to know what is inside a + directory it was handed.""" + root = _tree(tmp_path) + recipe = _build(root).tasks[("baseline", "summary")].recipe + assert "analyses/hod/results/baseline/mass_function.npz" in recipe + + +def test_a_re_export_resolves_to_what_actually_makes_the_bytes(tmp_path: Path) -> None: + """`summary` names the parent's re-export, which carries no recipe of + its own — so its input has to be the sub-analysis's file.""" + root = _tree(tmp_path) + task = _build(root).tasks[("baseline", "summary")] + assert task.produced_by == {"mass_function": ("baseline", "hod.mass_function")} + assert task.inputs["mass_function"] == ( + root / "analyses/hod/results/baseline/mass_function.npz" + ) + +def test_a_sub_analysis_reaching_outside_the_project_is_refused(tmp_path: Path) -> None: + """Its results would land outside the repository, where nothing could + version or fetch them. The sub-analysis is real and resolves — only + where it sits is wrong.""" + root = _tree(tmp_path / "project") + (root / "astra.yaml").write_text( + textwrap.dedent(_PARENT).replace("path: ./analyses/hod", "path: ../hod") + ) + outside = tmp_path / "hod" + (outside / "universes").mkdir(parents=True) + (outside / "astra.yaml").write_text(textwrap.dedent(_SUB)) + (outside / "universes" / "baseline.yaml").write_text( + "id: baseline\ndecisions:\n binning: log\n" + ) + with pytest.raises(ProjectError, match="outside the project"): + _build(root) +def test_a_sub_analysis_universe_that_is_not_there_is_refused(tmp_path: Path) -> None: + """astra logs a warning and settles the sub-analysis from the parent's + decisions instead, so without this lc would file an artifact under a + universe it never loaded.""" + root = _tree(tmp_path) + (root / "universes" / "baseline.yaml").write_text( + "id: baseline\ndecisions: {}\nanalyses:\n hod:\n universe: absent\n" + ) + with pytest.raises(ProjectError, match="absent"): + _build(root) @@ -348,6 +408,36 @@ def test_a_sub_analysis_output_is_addressed_flat_and_qualified(tmp_path: Path) - + + + + + +def test_an_output_without_a_format_is_refused_by_name(tmp_path: Path) -> None: + """lc names the file from it, so there is nowhere to write the output. + Every offender at once: a spec is fixed in one pass, not one run per + missing key.""" + root = _project(tmp_path) + (root / "astra.yaml").write_text( + textwrap.dedent(_SPEC).replace(" format: json\n", "").replace(" format: md\n", "") + ) + with pytest.raises(ProjectError, match="format") as raised: + _build(root) + assert "fit" in str(raised.value) and "report" in str(raised.value) + + +def test_two_tasks_that_would_write_one_file_are_refused(tmp_path: Path) -> None: + """An external sub-analysis is filed under its own universe, so two + parent universes selecting the same one resolve to the same path while + feeding it different inputs. The second would overwrite the first.""" + root = _tree(tmp_path / "project") + (root / "universes" / "robust.yaml").write_text( + "id: robust\ndecisions: {}\nanalyses:\n hod:\n universe: baseline\n" + ) + with pytest.raises(ProjectError, match="both materialize to"): + _build(root) + + # ---- rendering a recipe ---------------------------------------------------- diff --git a/tests/test_sandbox_denial.py b/tests/test_sandbox_denial.py index 02ef15b2..700ebab7 100644 --- a/tests/test_sandbox_denial.py +++ b/tests/test_sandbox_denial.py @@ -97,7 +97,7 @@ def test_an_in_tree_write_is_its_own_kind_of_denial(policy: Policy, project: Pat stderr = "PermissionError: [Errno 13] Permission denied: 'astra.yaml'\n" joined = "\n".join(denial.explain(stderr, policy, cwd=project)) assert "cannot write" in joined - assert "its own output directory" in joined + assert "directory that holds it" in joined assert "inputs:" not in joined diff --git a/tests/test_sandbox_enforcement.py b/tests/test_sandbox_enforcement.py index 93c86c67..2e7b16ab 100644 --- a/tests/test_sandbox_enforcement.py +++ b/tests/test_sandbox_enforcement.py @@ -382,28 +382,30 @@ def test_results_can_be_written(backend: sandbox.Backend, project: Path) -> None assert (project / "results" / "out.csv").read_text() == "out" -def test_a_recipe_cannot_write_a_sibling_output_directory( +def test_a_recipe_cannot_write_another_analysis_results_tree( backend: sandbox.Backend, project: Path ) -> None: - """The cross-write closure, at the kernel: a recipe granted its own - output directory cannot land bytes in a sibling's — the corruption - that would otherwise enter the sibling's digest as though its recipe - wrote it. Both writes target user-owned paths, so only the boundary - can refuse the first; the second is the mutation check in-place.""" - own = project / "results" / "baseline" / "first" - sibling = project / "results" / "baseline" / "second" + """The scope is the directory the output lands in, so outputs declared + beside each other are mutually writable — `data_version` is what + answers for an output's bytes, not the boundary. What the kernel does + still refuse is every *other* results tree: another universe, and + another analysis's own. Both writes target user-owned paths, so only + the boundary can refuse the first; the second is the mutation check + in-place.""" + own = project / "results" / "baseline" + other = project / "hod" / "results" / "fast" own.mkdir(parents=True) - sibling.mkdir(parents=True) - (sibling / "value.txt").write_text("theirs\n") - with sandbox.scope(sandbox.exec_policy(project, output_dir=own)) as policy: + other.mkdir(parents=True) + (other / "mass_function.npz").write_text("theirs\n") + with sandbox.scope(sandbox.exec_policy(project, write_dir=own)) as policy: crossed = shell( - backend, policy, f"printf forged > {sibling / 'value.txt'}", cwd=project + backend, policy, f"printf forged > {other / 'mass_function.npz'}", cwd=project ) - owned = shell(backend, policy, f"printf mine > {own / 'value.txt'}", cwd=project) + owned = shell(backend, policy, f"printf mine > {own / 'first.txt'}", cwd=project) assert crossed.returncode != 0 - assert (sibling / "value.txt").read_text() == "theirs\n", "the file changed anyway" + assert (other / "mass_function.npz").read_text() == "theirs\n", "the file changed anyway" assert owned.returncode == 0, owned.stderr - assert (own / "value.txt").read_text() == "mine" + assert (own / "first.txt").read_text() == "mine" def test_a_declared_input_is_read_only( diff --git a/tests/test_sandbox_policy.py b/tests/test_sandbox_policy.py index e05a492b..4c5a9a70 100644 --- a/tests/test_sandbox_policy.py +++ b/tests/test_sandbox_policy.py @@ -59,37 +59,40 @@ def test_results_is_writable_for_a_probe(tmp_path: Path) -> None: assert built.grants(project / "results" / "out.csv", built.write) -def test_a_recipe_is_narrowed_to_its_own_output_directory(tmp_path: Path) -> None: - """The cross-write closure: a concurrent task landing bytes in a - sibling's directory before the sibling hashes produces a manifest - that is self-consistent and wrong — no checksum can ever see it, so - prevention is the only fix. Same nested-writable shape, one level - deeper.""" +def test_a_recipe_is_narrowed_to_the_directory_its_output_lands_in(tmp_path: Path) -> None: + """An output is one file, and a mechanism can only grant *creating* it + through the directory that will hold it — so the scope is that + directory, and outputs declared side by side are mutually writable. + Recorded rather than papered over: `data_version` is what answers + whether an output's bytes are its own. What the scope does still + exclude is every other universe and every other analysis.""" project = tmp_path / "proj" - own = project / "results" / "baseline" / "first" - sibling = project / "results" / "baseline" / "second" - own.mkdir(parents=True) - sibling.mkdir(parents=True) - with scope(policy_module.exec_policy(project, output_dir=own)) as built: - assert built.grants(own / "out.csv", built.write) - assert not built.grants(sibling / "out.csv", built.write) + own = project / "results" / "baseline" + elsewhere = project / "results" / "robust" + sub = project / "hod" / "results" / "fast" + for path in (own, elsewhere, sub): + path.mkdir(parents=True) + with scope(policy_module.exec_policy(project, write_dir=own)) as built: + assert built.grants(own / "first.txt", built.write) + assert not built.grants(elsewhere / "first.txt", built.write) + assert not built.grants(sub / "mass_function.npz", built.write) assert not built.grants(project / "results", built.write) - assert built.grants(sibling / "out.csv", built.read), ( + assert built.grants(elsewhere / "first.txt", built.read), ( "an upstream output is still a readable input" ) -def test_the_containerized_recipe_mounts_only_its_own_output_directory( +def test_the_containerized_recipe_mounts_only_that_directory( tmp_path: Path, ) -> None: - """The mount table derives from the write set, so the narrowing must + """The mount table derives from the write set, so the scope must survive into the containerized shape untranslated.""" project = tmp_path / "proj" - own = project / "results" / "baseline" / "first" + own = project / "results" / "baseline" own.mkdir(parents=True) with scope( policy_module.exec_policy( - project, containerized=True, env_dir=project / ".lightcone/venv", output_dir=own + project, containerized=True, env_dir=project / ".lightcone/venv", write_dir=own ) ) as built: assert own in built.write diff --git a/tests/test_templates.py b/tests/test_templates.py index f2f67ff3..fd9c6593 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -123,7 +123,8 @@ def test_gitattributes_routes_content_to_the_annex_and_everything_else_to_git() assert entries[0] == "* annex.largefiles=nothing" assert "results/** annex.largefiles=anything" in entries assert "data/** annex.largefiles=anything" in entries - assert "**/.lightcone-manifest.json annex.largefiles=nothing" in entries + assert "**/results/** annex.largefiles=anything" in entries + assert "**/results/**/.*.manifest.json annex.largefiles=nothing" in entries def test_gitattributes_exceptions_come_after_the_default() -> None: @@ -162,6 +163,35 @@ def test_an_opt_out_the_defaults_would_land_below_is_named() -> None: assert misplaced == "* annex.largefiles=nothing" +def test_an_analysis_keeps_its_results_beside_its_own_spec() -> None: + """A sub-analysis is a self-similar analysis in its own directory, so a + results tree is not root-anchored. The narrow `results/**` is kept + beside the broad rule deliberately: a managed line dropped from the + template stops being ranked, which would blind the ordering check to + the very file `test_an_opt_out_the_defaults_would_land_below_is_named` + exists to catch.""" + entries = templates.entries("gitattributes.tmpl") + assert "results/** annex.largefiles=anything" in entries + assert entries.index("**/results/** annex.largefiles=anything") < entries.index( + "**/results/**/.*.manifest.json annex.largefiles=nothing" + ) + + +def test_a_gitattributes_written_by_an_earlier_template_still_converges() -> None: + """Repair only appends, at end of file, carrying each line's template + rank — so a managed line added anywhere but last strands every file + written before it, permanently, since append-only repair cannot + reorder. This is such a file, and it must converge.""" + earlier = ( + "* annex.largefiles=nothing\n" + "* filter=annex\n" + "results/** annex.largefiles=anything\n" + "data/** annex.largefiles=anything\n" + ".datalad/environments/*/image annex.largefiles=anything\n" + ) + assert templates.gitattributes_disorder(earlier) == "" + + def test_a_hand_written_file_already_in_the_right_order_needs_nothing() -> None: """Judged on meaning, not on who wrote it — and only lines setting the *same* attribute can be out of order with each other, so the @@ -197,7 +227,7 @@ def test_index_md_renders_the_title_and_keeps_myst_roles() -> None: def test_results_readme_explains_the_output_layout() -> None: """`results/` starts empty and git carries no empty directories, so the README is the only thing a clone shows for it.""" - assert "results///" in templates.read("results-README.md.tmpl") + assert "results//." in templates.read("results-README.md.tmpl") def test_data_readme_explains_where_declared_inputs_go() -> None: diff --git a/tests/test_venue.py b/tests/test_venue.py index d75ab01c..1884170c 100644 --- a/tests/test_venue.py +++ b/tests/test_venue.py @@ -33,15 +33,17 @@ outputs: - id: first type: metric + format: txt decisions: [method] recipe: - command: echo {decisions.method} > {output}/value.txt + command: echo {decisions.method} > {output} - id: second type: report + format: txt inputs: [first] recipe: - command: cat {inputs.first}/value.txt > {output}/copy.txt + command: cat {inputs.first} > {output} decisions: method: @@ -229,7 +231,7 @@ def test_a_run_spans_the_allocation( report = engine.materialize(root, []) assert report.made == ["baseline/first", "baseline/second"] - assert (root / "results/baseline/second/copy.txt").read_text() == "alpha\n" + assert (root / "results/baseline/second.txt").read_text() == "alpha\n" assert not dataset.status(root) diff --git a/tests/test_worker.py b/tests/test_worker.py index ad715c86..8b392c2a 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -37,14 +37,16 @@ outputs: - id: first type: metric + format: txt recipe: - command: echo one > {output}/value.txt + command: echo one > {output} - id: second type: report + format: txt inputs: [first] recipe: - command: cat {inputs.first}/value.txt > {output}/copy.txt + command: cat {inputs.first} > {output} """ @@ -96,10 +98,10 @@ def _age(root: Path, output_id: str) -> None: the recipe actually runs under has to stay the real one, or the mid-run gate refuses the execution before any of this is exercised. """ - directory = root / "results/baseline" / output_id - manifest = assets.read(directory) + sidecar = root / "results/baseline" / f".{output_id}.manifest.json" + manifest = assets.read(sidecar) assert manifest is not None - assets.write(directory, replace(manifest, env_version="sha256:an-earlier-environment")) + assets.write(sidecar, replace(manifest, env_version="sha256:an-earlier-environment")) # ---- executing a recipe ---------------------------------------------------- @@ -109,8 +111,8 @@ def test_a_recipe_runs_and_its_output_is_recorded(root: Path) -> None: result = _make(root, "first") assert result.status == "ok" - assert (root / "results/baseline/first/value.txt").read_text() == "one\n" - assert result.data_version == assets.data_version(root / "results/baseline/first") + assert (root / "results/baseline/first.txt").read_text() == "one\n" + assert result.data_version == assets.data_version(root / "results/baseline/first.txt") def test_the_manifest_is_complete_before_anything_is_saved(root: Path) -> None: @@ -119,9 +121,9 @@ def test_the_manifest_is_complete_before_anything_is_saved(root: Path) -> None: in that commit.""" _make(root, "first") - manifest = assets.read(root / "results/baseline/first") + manifest = assets.read(root / "results/baseline/.first.manifest.json") assert manifest is not None - assert manifest.data_version == assets.data_version(root / "results/baseline/first") + assert manifest.data_version == assets.data_version(root / "results/baseline/first.txt") assert manifest.definition_version == _task(root, "first").definition_version assert manifest.env_version == identity.env_version(root) assert manifest.git_sha == _HEAD[0] and manifest.git_remote == _HEAD[1] @@ -141,7 +143,7 @@ def test_the_recipe_runs_under_the_boundary(root: Path) -> None: _make(root, "first") - manifest = assets.read(root / "results/baseline/first") + manifest = assets.read(root / "results/baseline/.first.manifest.json") assert manifest is not None assert manifest.hermeticity["mechanism"] == sandbox.detect().capability.kind @@ -161,10 +163,10 @@ def test_a_skip_returns_the_recorded_digest_rather_than_rehashing(root: Path) -> """On a clone that has fetched no annex content the files are dangling symlinks, so a recompute would quietly report a different output.""" _make(root, "first") - output = root / "results/baseline/first" - manifest = assets.read(output) + output = root / "results/baseline/first.txt" + manifest = assets.read(assets.manifest_path(output)) assert manifest is not None - (output / "value.txt").unlink() + output.unlink() assert _make(root, "first").data_version == manifest.data_version @@ -174,7 +176,7 @@ def test_a_moved_environment_leaves_the_output_alone(root: Path) -> None: define exactly this output, so it is reported and kept — remaking it would spend the compute that a rewritten `uv.lock` never justified.""" made = _make(root, "first") - (root / "results/baseline/first/value.txt").write_text("untouched\n") + (root / "results/baseline/first.txt").write_text("untouched\n") _age(root, "first") again = _make(root, "first") @@ -182,20 +184,20 @@ def test_a_moved_environment_leaves_the_output_alone(root: Path) -> None: assert again.status == "behind" assert "earlier environment" in again.reason assert again.data_version == made.data_version - assert (root / "results/baseline/first/value.txt").read_text() == "untouched\n" + assert (root / "results/baseline/first.txt").read_text() == "untouched\n" def test_refresh_remakes_what_is_only_behind(root: Path) -> None: """And the recipe really runs: the file the previous assertion left in place is overwritten, so this cannot pass by skipping too.""" _make(root, "first") - (root / "results/baseline/first/value.txt").write_text("untouched\n") + (root / "results/baseline/first.txt").write_text("untouched\n") _age(root, "first") again = _make(root, "first", refresh=True) assert again.status == "ok" - assert (root / "results/baseline/first/value.txt").read_text() == "one\n" + assert (root / "results/baseline/first.txt").read_text() == "one\n" def test_refresh_does_not_remake_what_is_current(root: Path) -> None: @@ -217,7 +219,7 @@ def test_a_behind_upstream_still_feeds_its_dependents(root: Path) -> None: second = _make(root, "second", behind) assert second.status == "ok" - manifest = assets.read(root / "results/baseline/second") + manifest = assets.read(root / "results/baseline/.second.manifest.json") assert manifest is not None assert manifest.input_versions == {"first": first.data_version} @@ -231,7 +233,7 @@ def test_a_task_whose_upstream_did_not_finish_is_blocked(root: Path) -> None: assert result.status == "blocked" assert "baseline/first" in result.reason - assert not (root / "results/baseline/second").exists() + assert not (root / "results/baseline/second.txt").exists() def test_a_downstream_task_takes_its_upstreams_answer(root: Path) -> None: @@ -239,7 +241,7 @@ def test_a_downstream_task_takes_its_upstreams_answer(root: Path) -> None: second = _make(root, "second", first) assert second.status == "ok" - manifest = assets.read(root / "results/baseline/second") + manifest = assets.read(root / "results/baseline/.second.manifest.json") assert manifest is not None assert manifest.input_versions == {"first": first.data_version} @@ -247,42 +249,70 @@ def test_a_downstream_task_takes_its_upstreams_answer(root: Path) -> None: # ---- the output directory the recipe owns ---------------------------------- -def test_a_stale_file_does_not_survive_a_rebuild(root: Path) -> None: - """It would otherwise land in the content hash and be committed as part - of an output that never produced it.""" +def test_a_payload_from_an_earlier_format_does_not_survive_a_rebuild(root: Path) -> None: + """The reset takes every file named after the output, not just the one + the spec declares today — so re-declaring the serialization leaves no + orphan behind for the publication view to trip over.""" _make(root, "first") - output = root / "results/baseline/first" - (output / "leftover.txt").write_text("from a previous run\n") + earlier = root / "results/baseline/first.json" + earlier.write_text("from a run that declared another format\n") worker.execute(root, _task(root, "first"), {}, _context(root)) - assert not (output / "leftover.txt").exists() - assert (output / "value.txt").exists() + assert not earlier.exists() + assert (root / "results/baseline/first.txt").exists() + + +def test_a_sibling_output_is_left_alone_by_the_reset(root: Path) -> None: + """Outputs share a directory now, and under Dask they are written + concurrently — so the reset may only ever touch what its own id + names.""" + _make(root, "first") + sibling = root / "results/baseline/second.txt" + sibling.parent.mkdir(parents=True, exist_ok=True) + sibling.write_text("a neighbour's bytes\n") + + worker.execute(root, _task(root, "first"), {}, _context(root)) + + assert sibling.read_text() == "a neighbour's bytes\n" def test_a_failing_recipe_records_no_manifest(root: Path) -> None: - spec = _SPEC.replace("echo one > {output}/value.txt", "echo one > {output}/value.txt && false") + spec = _SPEC.replace("echo one > {output}", "echo one > {output} && false") (root / "astra.yaml").write_text(spec) result = _make(root, "first") assert result.status == "failed" assert "exited 1" in result.reason - assert assets.read(root / "results/baseline/first") is None + assert assets.read(root / "results/baseline/.first.manifest.json") is None -def test_a_recipe_that_removes_its_output_directory_fails_on_any_host(root: Path) -> None: - """The two mechanisms disagree about whether the removal is even - allowed — Landlock follows POSIX and refuses it, because unlinking the - directory needs write on `results/`, which is not granted; Seatbelt's - subpath grant covers the directory node itself and permits it. The - *contract* is the same either way, so that is what this asserts: a - `failed` result, never a raise into the driver.""" +def test_a_recipe_that_writes_nothing_fails(root: Path) -> None: + """Exit 0 is not evidence of an output. Without this the run would + commit a manifest for a file that is not there.""" (root / "astra.yaml").write_text( - _SPEC.replace("echo one > {output}/value.txt", "rm -rf {output}") + _SPEC.replace("echo one > {output}", "echo nowhere > /dev/null") ) - assert _make(root, "first").status == "failed" + result = _make(root, "first") + + assert result.status == "failed" + assert "left nothing" in result.reason + assert assets.read(root / "results/baseline/.first.manifest.json") is None + + +def test_a_recipe_that_writes_a_directory_fails(root: Path) -> None: + """`data_version` hashes a directory perfectly happily, so `mkdir` on + the output — which is what every recipe written for the old layout + does — would otherwise commit a well-formed digest of something that + is not the output at all.""" + (root / "astra.yaml").write_text(_SPEC.replace("echo one > {output}", "mkdir -p {output}")) + + result = _make(root, "first") + + assert result.status == "failed" + assert "a directory" in result.reason def test_an_output_that_cannot_be_recorded_fails_rather_than_raises( @@ -293,12 +323,10 @@ def test_an_output_that_cannot_be_recorded_fails_rather_than_raises( where reporting one failure and letting the rest finish is the whole point of owning the loop. Exercised without a mechanism, because that is the host where a recipe really can delete what it was given.""" - from lightcone.engine import sandbox + def refuse(*args: object, **kwargs: object) -> None: + raise OSError("no space left on device") - monkeypatch.setattr(sandbox, "detect", Unavailable) - (root / "astra.yaml").write_text( - _SPEC.replace("echo one > {output}/value.txt", "rm -rf {output}") - ) + monkeypatch.setattr(assets, "write", refuse) result = _make(root, "first") @@ -318,24 +346,26 @@ def test_an_environment_that_moved_under_the_run_is_refused(root: Path) -> None: assert result.status == "failed" assert "environment changed" in result.reason - assert assets.read(root / "results/baseline/first") is None + assert assets.read(root / "results/baseline/.first.manifest.json") is None # ---- what a recipe may touch ----------------------------------------------- def test_a_recipe_cannot_write_outside_the_results_tree(root: Path) -> None: - """One policy for probes and recipes, so `results/` is the whole of a - recipe's in-tree write scope. Sibling outputs are *not* carved out — - the manifest's content hash is what says whether an output's bytes are - its own, and a second mechanism for one guarantee is one more than can - be kept honest.""" + """The tree is read-only apart from the directory the output lands in. + Sibling outputs inside that directory are *not* carved out — an output + is one file, and a mechanism can only grant creating it through the + directory that will hold it; the manifest's content hash is what says + whether an output's bytes are its own.""" from lightcone.engine import sandbox if sandbox.detect().capability.kind == "none": pytest.skip("no sandbox mechanism on this host") (root / "astra.yaml").write_text( - _SPEC.replace("echo one > {output}/value.txt", "echo tampered > src/injected.py") + _SPEC.replace( + "echo one > {output}", "echo tampered > src/injected.py && echo one > {output}" + ) ) assert _make(root, "first").status == "failed" @@ -352,7 +382,9 @@ def test_that_write_would_have_succeeded_unsandboxed( monkeypatch.setattr(sandbox, "detect", Unavailable) (root / "src").mkdir(exist_ok=True) (root / "astra.yaml").write_text( - _SPEC.replace("echo one > {output}/value.txt", "echo tampered > src/injected.py") + _SPEC.replace( + "echo one > {output}", "echo tampered > src/injected.py && echo one > {output}" + ) ) assert _make(root, "first").status == "ok" @@ -378,9 +410,10 @@ def test_a_recipe_can_read_an_annexed_input(analysis: Callable[..., Path]) -> No outputs: - id: fit type: metric + format: txt inputs: [catalog] recipe: - command: cat {inputs.catalog} > {output}/seen.txt + command: cat {inputs.catalog} > {output} """ root = analysis(spec, files={"data/catalog.txt": "measured\n"}) dataset.save(root, [root / "data"], "the catalog") @@ -388,7 +421,7 @@ def test_a_recipe_can_read_an_annexed_input(analysis: Callable[..., Path]) -> No result = _make(root, "fit") assert result.status == "ok", result.reason - assert (root / "results/baseline/fit/seen.txt").read_text() == "measured\n" + assert (root / "results/baseline/fit.txt").read_text() == "measured\n" # ---- the entry point the run record names ---------------------------------- @@ -420,7 +453,7 @@ def test_the_module_runs_one_task_and_commits_nothing(root: Path) -> None: ) assert proc.returncode == 0, proc.stderr - assert (root / "results/baseline/first/value.txt").read_text() == "one\n" + assert (root / "results/baseline/first.txt").read_text() == "one\n" assert dataset.status(root) @@ -431,10 +464,10 @@ def test_the_module_reruns_unconditionally( a staleness check would answer a question nobody asked.""" monkeypatch.chdir(root) _make(root, "first") - (root / "results/baseline/first/value.txt").unlink() + (root / "results/baseline/first.txt").unlink() assert worker.main(["baseline/first"]) == 0 - assert (root / "results/baseline/first/value.txt").exists() + assert (root / "results/baseline/first.txt").exists() def test_the_module_converges_the_environment( @@ -452,7 +485,7 @@ def test_the_module_converges_the_environment( assert worker.main(["baseline/first"]) == 0 assert (root / ".venv").exists() - assert (root / "results/baseline/first/value.txt").read_text() == "one\n" + assert (root / "results/baseline/first.txt").read_text() == "one\n" def test_the_module_refuses_an_argument_it_cannot_use(root: Path) -> None: @@ -494,8 +527,8 @@ def test_a_declared_input_that_is_not_there_names_itself( """`data_version` reports an absent path with the OS's own exception, which would unwind as a traceback at whoever is reading a rerun.""" spec = _SPEC.replace( - " recipe:\n command: echo one > {output}/value.txt", - " inputs: [catalog]\n recipe:\n command: echo one > {output}/value.txt", + " recipe:\n command: echo one > {output}", + " inputs: [catalog]\n recipe:\n command: echo one > {output}", 1, ) root = analysis(spec) From 4488262ffc461a459c7166959197340b5e0ce7e1 Mon Sep 17 00:00:00 2001 From: EiffL Date: Tue, 25 Aug 2026 18:50:49 +0200 Subject: [PATCH 2/7] Document one-output-one-file, and record what it reverses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docs track the layout: `{output}` is a file, a sub-analysis keeps its results beside its own spec, data_version is a plain sha256, an output is a crate `File` rather than a Dataset of parts. The eval seed leaves `format:` out deliberately — completing the spec is the task — and its prompt no longer tells the agent under test to write artifact files inside a directory. CLAUDE.md records the decision and two things that are not obvious from the code: that the sandbox narrowing was reversed by mechanism rather than by choice, and that `results/**` is kept in the template only to keep the ordering guard from going blind. Also written down, pre-existing and unfixed: a malformed astra.yaml tracebacks out of `lc status` and `lc materialize --check` as a raw ScannerError, which contradicts the invariant it now sits under; and a sub-analysis's declared `source:` still resolves against the project root rather than its own home. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PybSgBiP8pjZeWxxCxiAXi --- CLAUDE.md | 196 ++++++++++++++++++++++++++++------- docs/api/assets.md | 5 +- docs/api/container.md | 3 +- docs/api/crate.md | 8 ++ docs/api/dataset.md | 2 +- docs/api/plan.md | 2 +- docs/api/worker.md | 16 ++- docs/architecture.md | 2 +- docs/user/concepts.md | 4 +- docs/user/getting-started.md | 21 ++-- docs/user/glossary.md | 32 +++--- docs/user/troubleshooting.md | 4 +- evals/prompt.md | 32 +++--- 13 files changed, 240 insertions(+), 87 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ae5035d5..55eafc59 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -714,8 +714,14 @@ must not discard edits made elsewhere while the graph was running. ## Key Invariants (layer 2) **Two hashes, and they answer different questions** (`identity.py`). -`definition_version = sha256(recipe ‖ canonical decisions)` is what the -spec says an output *is*. `env_version = sha256(uv.lock bytes ‖ +`definition_version = sha256(recipe ‖ canonical decisions ‖ format)` is +what the spec says an output *is*. The format is framed in its own right +rather than left to arrive through the rendered `{output}` path: a recipe +need not use that placeholder at all (`evals/tasks/snae` does not), and +one that does not would keep its digest across a re-declared +serialization — while the sidecar, named from the id alone, does not move +either, so the manifest would go on describing an output at a path that +no longer exists. `env_version = sha256(uv.lock bytes ‖ .python-version bytes ‖ canonical install-settings JSON)` is what it ran under. @@ -862,13 +868,37 @@ missing decision or an unresolvable input — blaming the run for a fault in the file, far from the line at fault. It caught three of lc's own test fixtures the first time it ran. -**The layout is flat and path-addressed.** `results///`, -`data/` for declared inputs, and the path in a rendered recipe *is* the -path on disk — no staging, no scratch, no relocation. The `output_id` is -ASTRA's **qualified** id, so a sub-analysis output lands at -`results//./` and one addressing scheme spans -however deep the spec nests. Nesting is not capped: the dot separator is -unambiguous because ASTRA ids match `^[a-z][a-z0-9_]*$`. +**One output is one file, and the spec names it.** An output is +`/results///.` — never a +directory a recipe fills — and the path in a rendered recipe *is* the path +on disk: no staging, no scratch, no relocation. `format` comes from ASTRA +(`Output.format`, astra-spec 0.0.14), so lc composes the filename rather +than a recipe choosing it, and the whole contents of every `results/` tree +are a pure function of the spec. `plan.build` **refuses** a spec whose +executable outputs omit it, naming every one: it is only *recommended* +until ASTRA 0.1.0, but lc has nowhere to write an output without it. + +**Results live beside the spec that declares them.** Every analysis node +has a *home* — the directory holding its `astra.yaml` — and a universe, +both derived by `plan._placements` walking the spec and the universe file +together: the root's home is the project root; an inline sub-analysis +shares its parent's home and disambiguates with a scope directory; an +external one (`path:`) is a self-similar analysis with its own home and, +where the parent universe names one, **its own universe id**. Two +refusals ride on that walk — a home escaping the project root (its +results could not be versioned) and a `universe: X` naming a file that is +not there (astra logs a warning and settles from the parent's decisions +instead, so lc would file an artifact under a universe it never loaded). + +Only the *path* nests. Graph keys stay `(universe_id, qualified_id)` and +CLI targets stay dotted, because `Graph.resolve` splits on +`rpartition("/")` and a slash-form target would mis-split the universe. + +**No task may share a path with another.** An external sub-analysis is +filed under its own universe, so two parent universes selecting the same +one resolve to the same file while feeding it different inputs — the +second would overwrite the first, silently. `build` refuses, naming both, +the same shape as the duplicate-universe-id refusal. **One rule names a path, and both the recipe and the run record use it** (`plan.declared_path`). Project-relative inside the tree, absolute @@ -887,26 +917,43 @@ the layer makes, and saying so is the whole obligation — the same treatment `sdist_built` gets. **Two universes cannot share an id.** The id names a directory under -`results/`, and the graph is keyed on `(universe_id, output_id)` — so the +a `results/`, and the graph is keyed on `(universe_id, output_id)` — so the second file simply replaced the first and one universe's outputs went missing with nothing said. The way in is the natural one: copy `baseline.yaml`, edit the decisions, forget the id inside. `build` refuses, naming both files. -**Because the path is composed, `output_dir` refuses an id that is not one -path component.** An empty universe or output id collapses -`results//` onto a *parent* — `results/` itself, for two — and the -worker empties that directory before running a recipe in it, so the -consequence of an unchecked id is deleting every other universe's -outputs. A `/`, `\`, `.` or `..` is refused for the same reason. This is -the guard that lets the reset stay a whole-directory operation. - -**The reset takes the whole directory, and cannot take a named list.** A -recipe declares an output *id*, never filenames, so there is no set of -"expected files" to remove — and a previous run that crashed can have left -anything in there, which would otherwise survive into this run's -`data_version` as though the recipe had written it. What bounds the blast -radius is the guard above, not a narrower delete. +**Because the path is composed, `assets.output_path` refuses any part +that is not one path component** — an empty or `..`-bearing universe id, +scope segment or output id would place a file outside the tree every +guard above it checked — and a `format` that could not be an extension +(empty, a separator, or leading-dot, which would make the output look +like its own sidecar). + +**The reset takes what the output's id names, never the directory.** +Outputs share a directory now and Dask writes them concurrently, so a +whole-directory delete would take a neighbour's bytes. It unlinks the +sidecar and globs `.*`: an id cannot contain a dot, so the glob +cannot reach a sibling, a longer id, another output's sidecar or a scope +directory of the same name — and it *does* reach a payload left by a run +that declared another `format`, which is what stops one orphaning. + +**A payload that is not a regular file fails the task.** `data_version` +branches on `is_file()` and hashes a directory happily, so `mkdir +{output}` — what every recipe written for the old layout does — would +otherwise commit a well-formed digest of something that is not the +output, plus a crate `File` with no `sha256`. One check covers absent, +directory and wrong-name, and it is what makes exit 0 stop being +evidence that anything was written. + +**The manifest is a sidecar, `..manifest.json`, named from the +id alone and never the format.** So it keeps its path — and therefore its +history — across a re-declared serialization, which is what lets +`_foreign_write` still answer for an output whose payload path is new. +Decomposition is safe by partitioning on the **first** dot (ids carry +none; formats may, `tar.gz`), never `Path.stem`. The old +`_HASH_EXCLUDE` is gone with the directory that made it necessary: the +manifest cannot be inside the thing it describes any more. **Dask owns the ordering.** Every task is submitted with its upstream futures as arguments, so the dependency order, the parallelism, and the @@ -1069,6 +1116,19 @@ raised anywhere inside astra's validation or resolution was reported as a bad target — a rerun misdiagnosing itself, at the one place nobody is watching. +**Known violation, unfixed: a malformed `astra.yaml` tracebacks out of +`lc status` and `lc materialize --check`** as a raw +`yaml.scanner.ScannerError`. `plan._validate` is the gate that turns a +bad spec into ASTRA's own errors, but it only catches what *validation* +reports — a spec that does not parse raises inside +`validate_analysis_schema` → `astra.helpers.load_yaml` before there is +anything to validate. (`lc materialize` usually masks it by refusing a +dirty tree first, which is not a fix.) The shape of the fix is a +`yaml.YAMLError` catch in `_validate` naming the file and the line, the +same way the validator's own errors are rendered. Pre-existing and +independent of the one-output-one-file change; written down here because +it contradicts the invariant directly above it. + **`git_sha` in a manifest is the commit the run *started* at**, not the commit the run went on to create. It is the code that produced the output. A test that reads `dataset.head()` after materializing and expects a match @@ -1208,20 +1268,21 @@ impurity lives in policy construction, once.) **There is one policy, `exec_policy`, and it differs between a recipe and a probe by one keyword.** The tree is read-only apart from the -in-tree write scope: a recipe's is **its own output directory** -(`output_dir=`, handed down from the worker's task), a probe's is -`results/` whole, because a probe has no output id. This narrowing is -the hardening pass reversing an earlier decision (see Recorded -decisions): it exists as leak *prevention*, closing the cross-write -residue — a concurrent task landing bytes in a sibling's directory -before the sibling hashes produced a manifest that was self-consistent -and wrong, which no checksum could ever see, so prevention was the only -possible fix. The probe→recipe promise ("a command that works under -`lc run` works as a recipe") now excludes exactly the commands that -write outside their own output directory — which is the accident being -prevented, not a loophole in the promise. Integrity-answering is still -`data_version`'s job alone; the sandbox prevents the write, it does not -attest the bytes. +in-tree write scope: a recipe's is **the directory its output lands in** +(`write_dir=`, handed down from the worker's task), a probe's is the +project's own `results/` whole, because a probe has no analysis node. + +That is a *directory* and not the output file, by mechanism rather than +by choice: `policy._declared`/`_existing` drop paths that do not exist, +so a file scope would leave a recipe with no in-tree write at all, and +the Landlock shim masks a non-directory grant to `_FILE_ONLY_BITS`, +stripping `MAKE_REG` — a file grant could not create the file. So the +one-output-one-file change reverses the hardening pass's narrowing: +outputs declared side by side are mutually writable again. Recorded, not +papered over — what answers whether an output's bytes are its own is +`data_version`, never the sandbox. What the scope still excludes is +every *other* results tree: another universe, and another analysis's +own. **`cluster_for_run()` is the seam, and it is two methods wide.** `submit(fn, *args, key=…)` and `completed(handles)`. That is all the @@ -1878,7 +1939,13 @@ JSON-LD drops on expansion — the pre-rebuild exporter's silent failure. The committed archive is one entity, `["File", "ContainerImage"]`, identity (`sha256` = config-blob id) and payload together. -**A published `sha256` is always a raw digest an outsider can verify** — +**A published `sha256` is always a raw digest an outsider can verify**, and +since the one-output-one-file change the manifest agrees with it: an +output is a file, `data_version` drops the `file:` frame, and the +manifest's digest, the crate's `sha256` and `sha256sum` are one number. +Only directory-valued declared inputs stay framed (`dir:`), which is +enough to keep the two from colliding and is the one entry in a manifest +no standard tool can check. Historically — since the hardening pass, never lc's framed hash. Every file in an output directory is a `File` under its dataset's `hasPart`, with `sha256` and `contentSize` parsed from its SHA256E annex key @@ -2141,6 +2208,57 @@ unlinks before writing; a new tampering test should too. materialize runs for hours) arrives with the submission-model venue. +### Recorded decision: one output, one file (post-hardening) + +`Output.format` landed in astra-spec 0.0.14 / astra-tools 0.2.17, and lc +took it as the missing half of a long-standing asymmetry: `Input.source` +always declared a path with an extension, while an output declared +nothing and `{output}` was documented as "the path the artifact will be +written to" — a path the schema never defined. What changed, and why: + +- **An output is one file, not a directory.** The multiplicity was born + the moment a recipe was handed a directory and picked a name inside it. + There is no directory now, so the natural idiom (`savefig(args.out)`) + produces exactly one file at exactly the declared path. The cost, + stated: a figure plus its data, a zarr store, a checkpoint directory + must become separate outputs or a container format (`.h5`, `.zip`, + `.tar.gz`). No flag re-opens it. +- **The filename contract between producer and consumer is gone.** + `{inputs.X}` renders to the upstream's *file*, so nothing has to know + what is inside a directory it was handed. +- **A results tree per analysis home**, so `results/` stopped being + root-anchored. `.gitattributes` broadened to `**/results/**` — with the + accepted collateral that a user's own `notebooks/results/` now annexes, + cheaper than generating spec-dependent attribute lines repair could + never remove. +- **`results/**` is kept in the template even though `**/results/**` + subsumes it.** A managed line dropped from the template gets rank + `None` and is *skipped* by `gitattributes_disorder` — which would blind + the guard to the exact trap it exists for (a user file carrying the + `results/` opt-out without the `*` defaults). Measured both ways. +- **A `check-attr` preflight refuses a run whose manifests would annex.** + `dataset.save` passes `annex.dotfiles=true`, so the sidecar's leading + dot decides nothing and `.gitattributes` decides everything. Annexed, + the run is green locally and every clone reports the whole project as + never materialized, silently. Probed once for the whole graph, before + anything executes. +- **The orphan walk is a set difference, never a reconstruction.** An + external sub-analysis is filed under its own universe while its graph + key carries the parent's, so no rule turns such a path back into a key. + Expected sidecars come from `Task.manifest_path`; present ones from + `git ls-files`, which still finds the tree of a sub-analysis just + deleted from the spec — the edit that orphans it is the same edit that + would drop it from any graph-derived root set. +- **No migration, by decision.** Nothing looks for `.lightcone-manifest.json` + and nothing detects directory-shaped outputs from a previous engine; + they are invisible to every walk. `git rm -r results/` and + re-materialize. +- **Known residue**: a declared input's `source:` is still resolved + relative to the *project root*, not the declaring analysis's home, so a + sub-analysis cannot name a file beside its own `astra.yaml` the way it + names its results. Pre-existing; whether `source:` should be + home-relative is astra's question before it is lc's. + ### Recorded deviations from the spec - **No `AGENTS.md` scaffolding** (spec §2 calls for an agent notes diff --git a/docs/api/assets.md b/docs/api/assets.md index 77dce6ef..b8d608b5 100644 --- a/docs/api/assets.md +++ b/docs/api/assets.md @@ -16,8 +16,9 @@ Source: `src/lightcone/engine/assets.py`. | `Verdict.calls_for_a_remake(refresh=)` | The one place a state becomes an action: `stale` always, `behind` only when asked. | | `data_version(path)` | Content hash of a directory or file — computed in the worker, before anything is annexed. | | `Versions` | Per-run memo so a shared declared input hashes once, not once per dependent. | -| `read(dir)` / `write(...)` | The manifest, `.lightcone-manifest.json`. | -| `output_dir(root, u, o)` | The path, guarded: an id that is not one path component is refused — this guard is what lets the worker's reset stay a whole-directory delete. | +| `read(sidecar)` / `write(...)` | The manifest, `..manifest.json`. Both take the sidecar's own path, so a caller holding an output path has to say `manifest_path` out loud. | +| `output_path(home, u, scope, id, fmt)` | The output's file, guarded: any part that is not a single path component is refused, and so is a format that could not be an extension. | +| `manifest_path(output)` | The sidecar beside it, named from the id alone — so it keeps its path, and its history, across a re-declared format. | | `ContentNotFetchedError` | An annexed file whose content is not in this clone, in either shape it takes. | ## What must stay true diff --git a/docs/api/container.md b/docs/api/container.md index 899f5164..14a32564 100644 --- a/docs/api/container.md +++ b/docs/api/container.md @@ -46,7 +46,8 @@ Sources: `src/lightcone/engine/image.py`, after the graph resolves** — a refusal over a typo must not cost a minutes-long build, and `dataset.save` commits the whole index. - **The mount table is the mechanism** (`sandbox/oci.py`): project - `:ro`, `results/` `:rw`, declared inputs `:ro`, private HOME, + `:ro`, the write scope `:rw` — the directory a recipe's output lands + in, or `results/` for a probe — declared inputs `:ro`, private HOME, `--tmpfs /tmp`, over a `--read-only` rootfs — without that flag a stray write *succeeds* into the ephemeral layer and vanishes while the attestation claims `fs: declared`. Mounts are resolved source, diff --git a/docs/api/crate.md b/docs/api/crate.md index af2bf0d3..08ca530f 100644 --- a/docs/api/crate.md +++ b/docs/api/crate.md @@ -39,6 +39,14 @@ Source: `src/lightcone/engine/crate.py` (converged by - **The `Person` is the author of the output's *saving* commit** (via `writer`), never the manifest's `git_sha` — that is the commit the run *started* at and can be someone else's. +- **An output is a `File`, not a `Dataset` of parts.** It is one file, + so there is one annex key to look up and one `sha256` to publish — + the same number its manifest records as `data_version`, and the one + `sha256sum` prints. +- **Every analysis home's spec and universes are in the crate**, not + just the root's: a sub-analysis declared with `path:` keeps its own, + and publishing its outputs without them would describe half an + analysis. - **The manifest is not transliterated.** `env_version`, `definition_version` and `hermeticity` get no invented schema.org spelling — the manifest itself is in the crate as a `File`, diff --git a/docs/api/dataset.md b/docs/api/dataset.md index 53df884d..28da872a 100644 --- a/docs/api/dataset.md +++ b/docs/api/dataset.md @@ -17,7 +17,7 @@ Source: `src/lightcone/engine/dataset.py` (+ | `restore(root, paths)` | `git clean` always; `git checkout HEAD --` only when HEAD has the path. Never `-- .`. | | `status(root)` | The dirty question, scoped to the project (`-- .`, prefix-stripped) so a project inside a larger repository works. | | `head(root)` | The commit a run started at — read once per run, by the driver. | -| `last_writer(root, dir)` | Who last touched an output's directory — the foreign-write question. Answers "cannot say" as empty, never an error. | +| `last_writer(root, *paths)` | Who last touched an output or its manifest — the foreign-write question. Answers "cannot say" as empty, never an error. | | `require_committer(root)` | Refuses a repository with no git identity, before any recipe spends time. Asked as `git var`, the question a commit itself asks. | | `dataset_id(root)` | The DataLad dataset UUID, read via `git config -f`. | | `set_annex_filter_required(root)` | Set `filter.annex.required=true`, so a `git add` that cannot reach git-annex fails loudly instead of staging raw bytes. | diff --git a/docs/api/plan.md b/docs/api/plan.md index 980eef4a..c2681ba8 100644 --- a/docs/api/plan.md +++ b/docs/api/plan.md @@ -32,7 +32,7 @@ Source: `src/lightcone/engine/plan.py`. resolution answers what a *valid* spec means and does not re-check that it is one. - **The layout is flat and path-addressed.** - `results///`, and the path in a + `/results///.`, and the path in a rendered recipe *is* the path on disk — no staging, no relocation. - **`declared_path` is lexical, never `resolve()`d.** A declared input under `data/` is an annex symlink; resolving it writes diff --git a/docs/api/worker.md b/docs/api/worker.md index 2dbe05b0..40e96db7 100644 --- a/docs/api/worker.md +++ b/docs/api/worker.md @@ -20,7 +20,7 @@ Source: `src/lightcone/engine/worker.py`. | Symbol | Role | |---|---| -| `materialize(task, versions, ...)` | The unit: classify → reset the directory → sandbox → recipe → hash → manifest. Returns a `TaskResult`, always. | +| `materialize(task, versions, ...)` | The unit: classify → reset → sandbox → recipe → check the payload → hash → manifest. Returns a `TaskResult`, always. | | `TaskResult` | `ok` / `current` / `behind` / `failed` / `blocked`, the output's `data_version`, and the attestation. `.usable` is what dependents check. | | `main(argv)` | The rerun entry point: guards, converges the project environment from the commit's own lock, resolves its own HEAD and runtime, executes. | | `lc_version()` | The engine version every manifest records. | @@ -37,10 +37,16 @@ Source: `src/lightcone/engine/worker.py`. `git annex find` records `sha256([])` for everything, silently, with green tests — and couples the digest to the annex backend, which is deliberately not pinned. -- **The reset takes the whole directory** — a crashed previous run can - have left anything there, and there is no "expected file list" to - delete by. The `output_dir` guard bounds the blast radius, not a - narrower delete. +- **The reset takes what the output's id names, never the directory** — + outputs share a directory and Dask writes them concurrently, so a + whole-directory delete would take a neighbour's bytes with it. The + glob is `.*` plus the sidecar: an id cannot contain a dot, + so it cannot reach a sibling, and it *does* reach a payload left by a + run that declared another `format`. +- **A payload that is not a regular file fails the task.** `data_version` + hashes a directory perfectly happily, so `mkdir {output}` would + otherwise commit a well-formed digest of something that is not the + output — and exit 0 is not evidence that anything was written. - **No git in here.** The driver commits; a worker that asked git would race the index lock and could read a HEAD this same run moved. - **`main`'s "no output ``" message covers the task lookup only.** diff --git a/docs/architecture.md b/docs/architecture.md index af9a3bd3..32081a20 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -87,7 +87,7 @@ callers: the worker (live input digests) and the read-only walk change"). That single value is the entire difference between run and check, which is what keeps `--check` honest. `behind` does not propagate; `stale` wins when both apply; and a foreign write (an -output's directory last touched by a commit that is not its own run +output whose file or manifest was last touched by a commit that is not its own run record) classifies stale through the same rule, as one more input value. diff --git a/docs/user/concepts.md b/docs/user/concepts.md index db12a764..b290397a 100644 --- a/docs/user/concepts.md +++ b/docs/user/concepts.md @@ -35,7 +35,7 @@ Recipes import from the locked environment and nothing else — a stray ## An output has an identity, and three facts about it Every materialized output records, in its -`.lightcone-manifest.json`: +`..manifest.json`: 1. **What it is** — a hash of its recipe and the decision values that shaped it (its *definition*). @@ -108,7 +108,7 @@ How recipes execute is never configured — it is read off the project: - **Direct mode** (the default): recipes run on your machine, in the project's `.venv`, under an OS sandbox — Landlock on Linux, Seatbelt on macOS. The project tree is read-only except each recipe's own - output directory; undeclared tools don't execute. + directory its output lands in; undeclared tools don't execute. - **Containerized mode**: declaring a `[tool.lightcone.image]` table in `pyproject.toml` *is* the switch. Recipes then run inside a content-addressed image built from that declaration — and the image diff --git a/docs/user/getting-started.md b/docs/user/getting-started.md index cb884231..fd66297d 100644 --- a/docs/user/getting-started.md +++ b/docs/user/getting-started.md @@ -102,6 +102,7 @@ inputs: outputs: - id: fit type: metric + format: json description: "Slope and intercept of the least-squares line" inputs: [points] decisions: [outliers] @@ -110,6 +111,7 @@ outputs: - id: fit_plot type: figure + format: png description: "The points and the fitted line" inputs: [points, fit] recipe: @@ -136,9 +138,12 @@ A few things to notice: changes. - Recipes reference those dependencies through placeholders — `{inputs.points}`, `{decisions.outliers}`, `{output}` — which are - expanded at execution time. `{output}` is the output's own results - directory, `results///`; the engine creates it - before the recipe runs. + expanded at execution time. `{output}` is the output's own file, + `results//.`; the engine creates the + directory before the recipe runs, and the recipe writes that one path. +- Each output declares a `format` — the extension its artifact is + written with. It is what names the file, so a consumer knows what an + output *is* from the spec alone, and one output is always one file. - The decision's options aren't hardcoded anywhere in code; the scripts will take them as command-line arguments. @@ -153,7 +158,7 @@ decisions: ``` Each universe is one complete selection of decision values; its results -materialize to `results///`. +materialize to `results//.`. Check the spec is well-formed: @@ -197,7 +202,7 @@ out = Path(args.output) ) ``` -Then `src/plot.py` — reads the upstream output's directory, makes the +Then `src/plot.py` — reads the upstream output's file, makes the figure: ```python @@ -266,7 +271,8 @@ uncommitted edits (it wouldn't be able to say what code ran). Then: ``` (We'll come back to that license line in step 7.) Each output landed in -`results/baseline//` next to a `.lightcone-manifest.json` — +`results/baseline/.` next to a +`..manifest.json` — a manifest recording the recipe, the decisions, the input hashes, the environment, and the commit — and was committed with a run record that `datalad rerun` can replay. Look at `git log`: the build wrote history, @@ -359,7 +365,8 @@ repository you already have. - The scripts take decision values as plain command-line arguments, so nothing methodological is hardcoded. - `lc materialize` ran each recipe in the project's locked environment, - sandboxed — free to write its own output directory and nothing else — + sandboxed — free to write the directory its output lands in, and + nothing else — and committed every output with a manifest and a re-runnable run record. - `lc status` and `lc materialize --check` read those manifests — they diff --git a/docs/user/glossary.md b/docs/user/glossary.md index 39fa1d23..4181a305 100644 --- a/docs/user/glossary.md +++ b/docs/user/glossary.md @@ -46,15 +46,19 @@ their `options`, and their `rationale`. One specific selection of decision values. Universes live as YAML files in `universes/` (e.g. `universes/baseline.yaml`, `universes/robust.yaml`). Each universe materializes its results -to its own directory: `results///`. +to its own directory: `results//.`. ## Sub-analysis A nested ASTRA analysis with its own inputs, outputs, and decisions, -referenced from a parent's `analyses:` section. A sub-analysis output's -directory uses its qualified id — -`results//./` — so one addressing scheme -spans however deep the spec nests. +referenced from a parent's `analyses:` section. Results follow the +spec's shape: one declared inline lands under a scope directory, +`results///.`, while one declared +with `path:` is a self-similar analysis — its own `astra.yaml`, its own +`universes/`, and so its own results tree beside them, +`/results//.`. Addressing does not +nest with the path: an output is always named by its qualified id, +`.`. ## Materialize @@ -65,8 +69,8 @@ says so and touches nothing. ## Manifest -The per-output sidecar JSON file -(`/.lightcone-manifest.json`) recording what produced the +The per-output sidecar JSON file, `..manifest.json` beside +the output itself, recording what produced the output: the recipe, the decisions, `definition_version`, `env_version`, `data_version`, `input_versions`, the git commit the run started at, the engine version, what enforcement actually ran @@ -90,10 +94,13 @@ output is `behind`, reported and left alone. ## data_version -A content hash over the files in an output's directory (or of a -declared input). This is what flows downstream: a dependent is remade -when an input's `data_version` changed, and a rebuild that comes out -byte-identical stops the cascade right there. +A content hash of an output's bytes (or of a declared input). For a +file it is a plain sha256 — the number `sha256sum` prints, and the one +the RO-Crate publishes; a directory-valued declared input is hashed +tree-wise and framed, so the two can never collide. This is what flows +downstream: a dependent is remade when an input's `data_version` +changed, and a rebuild that comes out byte-identical stops the cascade +right there. ## input_versions @@ -140,7 +147,8 @@ down). The isolation every recipe and every `lc run` command executes under — Landlock on Linux, Seatbelt on macOS, the container boundary in containerized mode. The project tree is read-only apart from the -output directory being made; undeclared tools don't execute. Each +directory the output being made lands in; undeclared tools don't +execute. Each manifest's `hermeticity` field records what was actually enforced, and a host with no mechanism says so rather than pretending. diff --git a/docs/user/troubleshooting.md b/docs/user/troubleshooting.md index 73f0b2a2..e7e59f1c 100644 --- a/docs/user/troubleshooting.md +++ b/docs/user/troubleshooting.md @@ -56,7 +56,7 @@ environment ``` Recipes run in the project's locked environment, with the tree -read-only apart from their own output directory. The common cases: +read-only apart from the directory their output lands in. The common cases: - **`ModuleNotFoundError`** — the package isn't in the project's lock. `uv add `, commit, re-run. (Installing it on the host with @@ -64,7 +64,7 @@ read-only apart from their own output directory. The common cases: - **Reading a file outside the project** — declare it as an ASTRA input; declared inputs are readable and their content becomes part of the output's provenance. -- **Writing outside the output directory** — a recipe's product +- **Writing outside that directory** — a recipe's product belongs in `{output}`; for true scratch files, use `tempfile.mkdtemp()`, which lands in the writable temp area. diff --git a/evals/prompt.md b/evals/prompt.md index aa05abcf..50386375 100644 --- a/evals/prompt.md +++ b/evals/prompt.md @@ -30,7 +30,7 @@ This project is driven by two CLIs — use them rather than improvising: `uv run`: `lc run python scripts/fit.py --output /tmp/x`, never a single quoted shell string; for shell syntax use `lc run bash -c '...'`. - - Outputs land in `results/baseline//`, each with a + - Outputs land in `results/baseline/.`, each with a `.lightcone-manifest.json` manifest written and committed by the engine. Never write into `results/` yourself: a hand-placed file has no run record, and the engine detects the foreign write and @@ -43,13 +43,14 @@ This project is driven by two CLIs — use them rather than improvising: A recipe's `command` is a template. The engine substitutes these placeholders before invoking it: -- `{output}` — the directory the output is materialized into - (`results///`). The engine creates it; your script - must write its artifact file(s) inside it. +- `{output}` — the file the output is materialized to, + `results//.`, where `format` is the one the + output declares. Your script must write exactly that path, and nothing + else: an output is one file. The engine creates the directory; a recipe + that writes a directory there, or writes some other name, fails. - `{inputs.}` — the named input's resolved path: an analysis-level `Input`'s `source` (e.g. a file under `data/`), or, for an upstream - output, that output's results directory (your script reads the file(s) - inside it). + output, that output's own file — so your script opens it directly. - `{inputs}` — space-separated paths of all declared inputs, in declaration order. - `{decisions.}` — the active option ID for the named decision in the @@ -67,8 +68,8 @@ is how the engine orders the build. Recipes run in the project's own locked environment (`pyproject.toml` + `uv.lock` + `.venv`), sandboxed: the project tree is read-only apart from -each recipe's own output directory under `results/`, and only declared -tools are executable. +the directory each recipe's output lands in, and only declared tools are +executable. - The project is managed by uv and starts with **no dependencies**. Every package a recipe script imports must be declared before @@ -86,18 +87,21 @@ tools are executable. `astra.yaml` is the single source of truth: inputs, outputs, recipes, and methodological decisions all live there — read it first. The seed spec is -deliberately incomplete: recipe commands do not yet pass their inputs, -decisions, or output directory, and outputs may be missing entries in -their `inputs:` / `decisions:` contracts. Completing the spec is part of +deliberately incomplete: outputs declare no `format:`, recipe commands do +not yet pass their inputs, decisions, or output path, and outputs may be +missing entries in their `inputs:` / `decisions:` contracts. Completing the spec is part of the task. For each output: -1. Complete the recipe `command` so it references `{output}` and the +1. Declare the output's `format:` — the file extension its artifact is + written with, without the leading dot (`png`, `csv`, `json`, …). lc + names the file from it and refuses a spec that omits it. +2. Complete the recipe `command` so it references `{output}` and the `{inputs.}` / `{decisions.}` the computation needs, and declare everything it references in that output's `inputs:` / `decisions:` lists. -2. Write the script at the path the command names, parameterizing every +3. Write the script at the path the command names, parameterizing every decision via argparse — never hardcode option values. -3. Commit your edits, then run `lc materialize` (or +4. Commit your edits, then run `lc materialize` (or `lc materialize `) to build through the engine. Build iteratively from upstream outputs to downstream. `lc status` shows From 2ba87600824acf60bbc4ba87a52ed632acbedf3d Mon Sep 17 00:00:00 2001 From: EiffL Date: Tue, 25 Aug 2026 19:59:34 +0200 Subject: [PATCH 3/7] Name the manifest by its new pattern in the two docs that missed it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidecar rename left `.lightcone-manifest.json` in two places the rest of the change updated: the eval prompt, whose preceding line had already moved to the new file-shaped layout, and the layer-1 storage invariant, which named a `.gitattributes` pattern the template no longer carries — contradicting this branch's own Recorded decision. The third mention stays: the no-migration decision names that file precisely because nothing looks for it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PybSgBiP8pjZeWxxCxiAXi --- CLAUDE.md | 4 ++-- evals/prompt.md | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 55eafc59..58b11efa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -478,8 +478,8 @@ Without it `filter=annex` routes *everything* into the annex, analysis code included. `tests/test_dataset.py::test_analysis_code_stays_in_git_and_stays_writable` pins it against a real annex. -**Manifests stay in git, deliberately.** `**/.lightcone-manifest.json` is -exempted back out of the annex so it is readable on a clone that has +**Manifests stay in git, deliberately.** `**/results/**/.*.manifest.json` +is exempted back out of the annex so it is readable on a clone that has fetched no annex content at all — which is what lets `lc materialize --check` classify a whole project on a laptop that holds none of the bytes. diff --git a/evals/prompt.md b/evals/prompt.md index 50386375..b605b41f 100644 --- a/evals/prompt.md +++ b/evals/prompt.md @@ -31,10 +31,10 @@ This project is driven by two CLIs — use them rather than improvising: single quoted shell string; for shell syntax use `lc run bash -c '...'`. - Outputs land in `results/baseline/.`, each with a - `.lightcone-manifest.json` manifest written and committed by the - engine. Never write into `results/` yourself: a hand-placed file - has no run record, and the engine detects the foreign write and - remakes the output. + `..manifest.json` manifest beside it, written and + committed by the engine. Never write into `results/` yourself: a + hand-placed file has no run record, and the engine detects the + foreign write and remakes the output. - When a recipe fails, `lc materialize` reports which output failed and why; fix the script or the spec, commit, and re-run. From 4ab214202540102dcd3a8417170458f6d8206225 Mon Sep 17 00:00:00 2001 From: EiffL Date: Tue, 25 Aug 2026 20:05:00 +0200 Subject: [PATCH 4/7] Stop the eval seed's spec version drifting from the installed astra-spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lc init` writes the version the installed astra-spec provides, and the eval harness then overlays a seed whose version is pinned by hand — so bumping astra-tools to 0.2.17 (astra-spec 0.0.14) left every `astra validate` call in the eval warning about a 0.0.13 declaration, training the agent under test to read past version warnings. The seed is bumped, and the harness now puts the scaffold's answer back after the overlay so the same drift cannot return on the next release. The remaining warning in the eval is the intentional one: the seed declares no `format:` because supplying it is part of the task. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PybSgBiP8pjZeWxxCxiAXi --- .github/workflows/eval.yml | 7 +++++++ evals/tasks/snae/astra.yaml | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 949205c9..843cb2e8 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -80,8 +80,15 @@ jobs: project="$RUNNER_TEMP/eval-project" echo "PROJECT=$project" >> "$GITHUB_ENV" lc init "$project" + # `lc init` writes the spec version the installed astra-spec + # provides; the task seed is a committed fixture pinned by hand, + # so it drifts on every astra-spec release and `astra validate` + # then warns on every call — training the agent under test to + # read past version warnings. Keep the scaffold's answer. + spec_version="$(grep -m1 '^version:' "$project/astra.yaml")" # Overlay the task seed (astra.yaml, data/) on the scaffold cp -r evals/tasks/snae/. "$project"/ + sed -i "0,/^version:/s|^version:.*|$spec_version|" "$project/astra.yaml" cd "$project" # Regenerate baseline.yaml from the task astra.yaml's defaults — # the scaffolded one matches the boilerplate spec, not the task's diff --git a/evals/tasks/snae/astra.yaml b/evals/tasks/snae/astra.yaml index 41b9d647..cca7363a 100644 --- a/evals/tasks/snae/astra.yaml +++ b/evals/tasks/snae/astra.yaml @@ -1,7 +1,7 @@ # ASTRA Analysis Specification # Documentation: https://github.com/LightconeResearch/ASTRA -version: "0.0.13" +version: "0.0.14" name: "snae" description: | Fit the Union2.1 Type Ia supernova distance modulus vs redshift data From 37c4f6c5b7f28bfdf5ff047633f8c56d58d5aa0b Mon Sep 17 00:00:00 2001 From: EiffL Date: Tue, 25 Aug 2026 20:16:08 +0200 Subject: [PATCH 5/7] Call it the analysis root, not its home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every analysis node has a directory holding its own `astra.yaml`, which is what its results hang off. "Home" was a coinage; "analysis root" says the same thing in the vocabulary the rest of the codebase already uses. Not bare `root`: that already means the *project* root in every signature this sits beside, and `{self.root, *(task.root for …)}` in the crate would read as two different roots in one expression. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PybSgBiP8pjZeWxxCxiAXi --- CLAUDE.md | 21 +++++++++--------- docs/api/assets.md | 2 +- docs/api/crate.md | 2 +- docs/api/plan.md | 2 +- src/lightcone/engine/assets.py | 14 ++++++------ src/lightcone/engine/crate.py | 11 ++++++---- src/lightcone/engine/plan.py | 40 +++++++++++++++++++--------------- 7 files changed, 50 insertions(+), 42 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 58b11efa..65de5617 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -869,7 +869,7 @@ in the file, far from the line at fault. It caught three of lc's own test fixtures the first time it ran. **One output is one file, and the spec names it.** An output is -`/results///.` — never a +`/results///.` — never directory a recipe fills — and the path in a rendered recipe *is* the path on disk: no staging, no scratch, no relocation. `format` comes from ASTRA (`Output.format`, astra-spec 0.0.14), so lc composes the filename rather @@ -879,13 +879,14 @@ executable outputs omit it, naming every one: it is only *recommended* until ASTRA 0.1.0, but lc has nowhere to write an output without it. **Results live beside the spec that declares them.** Every analysis node -has a *home* — the directory holding its `astra.yaml` — and a universe, +has an *analysis root* — the directory holding its `astra.yaml` — and a +universe, both derived by `plan._placements` walking the spec and the universe file -together: the root's home is the project root; an inline sub-analysis -shares its parent's home and disambiguates with a scope directory; an -external one (`path:`) is a self-similar analysis with its own home and, +together: the root analysis's is the project root; an inline sub-analysis +shares its parent's and disambiguates with a scope directory; an +external one (`path:`) is a self-similar analysis with its own root and, where the parent universe names one, **its own universe id**. Two -refusals ride on that walk — a home escaping the project root (its +refusals ride on that walk — an analysis root escaping the project (its results could not be versioned) and a `universe: X` naming a file that is not there (astra logs a warning and settles from the parent's decisions instead, so lc would file an artifact under a universe it never loaded). @@ -2226,7 +2227,7 @@ written to" — a path the schema never defined. What changed, and why: - **The filename contract between producer and consumer is gone.** `{inputs.X}` renders to the upstream's *file*, so nothing has to know what is inside a directory it was handed. -- **A results tree per analysis home**, so `results/` stopped being +- **A results tree per analysis root**, so `results/` stopped being root-anchored. `.gitattributes` broadened to `**/results/**` — with the accepted collateral that a user's own `notebooks/results/` now annexes, cheaper than generating spec-dependent attribute lines repair could @@ -2254,10 +2255,10 @@ written to" — a path the schema never defined. What changed, and why: they are invisible to every walk. `git rm -r results/` and re-materialize. - **Known residue**: a declared input's `source:` is still resolved - relative to the *project root*, not the declaring analysis's home, so a + against the *project* root, not the analysis root that declares it, so a sub-analysis cannot name a file beside its own `astra.yaml` the way it - names its results. Pre-existing; whether `source:` should be - home-relative is astra's question before it is lc's. + names its results. Pre-existing; which of the two a `source:` means is + astra's question before it is lc's (issue #201). ### Recorded deviations from the spec diff --git a/docs/api/assets.md b/docs/api/assets.md index b8d608b5..916e5c80 100644 --- a/docs/api/assets.md +++ b/docs/api/assets.md @@ -17,7 +17,7 @@ Source: `src/lightcone/engine/assets.py`. | `data_version(path)` | Content hash of a directory or file — computed in the worker, before anything is annexed. | | `Versions` | Per-run memo so a shared declared input hashes once, not once per dependent. | | `read(sidecar)` / `write(...)` | The manifest, `..manifest.json`. Both take the sidecar's own path, so a caller holding an output path has to say `manifest_path` out loud. | -| `output_path(home, u, scope, id, fmt)` | The output's file, guarded: any part that is not a single path component is refused, and so is a format that could not be an extension. | +| `output_path(analysis_root, u, scope, id, fmt)` | The output's file, guarded: any part that is not a single path component is refused, and so is a format that could not be an extension. | | `manifest_path(output)` | The sidecar beside it, named from the id alone — so it keeps its path, and its history, across a re-declared format. | | `ContentNotFetchedError` | An annexed file whose content is not in this clone, in either shape it takes. | diff --git a/docs/api/crate.md b/docs/api/crate.md index 08ca530f..c8055154 100644 --- a/docs/api/crate.md +++ b/docs/api/crate.md @@ -43,7 +43,7 @@ Source: `src/lightcone/engine/crate.py` (converged by so there is one annex key to look up and one `sha256` to publish — the same number its manifest records as `data_version`, and the one `sha256sum` prints. -- **Every analysis home's spec and universes are in the crate**, not +- **Every analysis root's spec and universes are in the crate**, not just the root's: a sub-analysis declared with `path:` keeps its own, and publishing its outputs without them would describe half an analysis. diff --git a/docs/api/plan.md b/docs/api/plan.md index c2681ba8..6bba5929 100644 --- a/docs/api/plan.md +++ b/docs/api/plan.md @@ -32,7 +32,7 @@ Source: `src/lightcone/engine/plan.py`. resolution answers what a *valid* spec means and does not re-check that it is one. - **The layout is flat and path-addressed.** - `/results///.`, and the path in a + `/results///.`, and the path in a rendered recipe *is* the path on disk — no staging, no relocation. - **`declared_path` is lexical, never `resolve()`d.** A declared input under `data/` is an annex symlink; resolving it writes diff --git a/src/lightcone/engine/assets.py b/src/lightcone/engine/assets.py index fb753946..43eaf1dd 100644 --- a/src/lightcone/engine/assets.py +++ b/src/lightcone/engine/assets.py @@ -1,14 +1,14 @@ """What a materialized output *is*: where it lives, what it records, and whether it is still current. -An asset is a single file — ``/results///.`` +An asset is a single file — ``/results///.`` — with a manifest sidecar beside it, ``..manifest.json``. The format comes from the spec, so the path is derived rather than chosen by the recipe, and one output can only ever be one file. The manifest is the only part lc writes itself, and it is kept out of the annex so it stays readable on a clone that has fetched no content at all. -*home* is the directory holding the analysis's own ``astra.yaml`` — the +The *analysis root* is the directory holding that analysis's own ``astra.yaml`` — the project root, or a sub-analysis's own directory — so results sit beside the spec that declares them. @@ -54,20 +54,20 @@ class ContentNotFetchedError(ProjectError): def output_path( - home: Path, universe_id: str, scope: Sequence[str], local_id: str, fmt: str + analysis_root: Path, universe_id: str, scope: Sequence[str], local_id: str, fmt: str ) -> Path: """Locate the file one output is materialized to. - ``/results///.``. *home* is + ``/results///.``. The *analysis root* is the directory holding the declaring analysis's ``astra.yaml``, so a sub-analysis keeps its results beside its own spec, and *scope* is the - inline sub-analyses descended through since that home. + inline sub-analyses descended through since that analysis root. Path-addressed: this is the path a rendered recipe writes to, with no staging, scratch or relocation in between. Args: - home: The declaring analysis's directory. + analysis_root: The declaring analysis's directory. universe_id: The universe the output is made under. scope: Inline sub-analysis ids, outermost first. local_id: The output's own id, unqualified. @@ -93,7 +93,7 @@ def output_path( f"output `{local_id}` declares the format {fmt!r}, which cannot name a " f"file extension, so the output has nowhere to be written." ) - return home.joinpath("results", universe_id, *scope, f"{local_id}.{fmt}") + return analysis_root.joinpath("results", universe_id, *scope, f"{local_id}.{fmt}") def manifest_path(output: Path) -> Path: diff --git a/src/lightcone/engine/crate.py b/src/lightcone/engine/crate.py index f8318960..b3f25fae 100644 --- a/src/lightcone/engine/crate.py +++ b/src/lightcone/engine/crate.py @@ -289,16 +289,19 @@ def _tool_id(self, output_id: str) -> str: def _environment_files(self) -> None: """The lock and its companions — what every run consumed. - Every analysis *home*, not just the root's: a sub-analysis keeps + Every analysis root, not just the project's: a sub-analysis keeps its spec, its universes and its results together, so publishing its outputs without the files that define them would leave the crate describing half an analysis. """ - homes = {self.root, *(task.home for task in self.graph.tasks.values())} + analysis_roots = {self.root, *(task.analysis_root for task in self.graph.tasks.values())} declaring = sorted( path.relative_to(self.root).as_posix() - for home in homes - for path in (*(home / "universes").glob("*.yaml"), home / SPEC_FILENAME) + for analysis_root in analysis_roots + for path in ( + *(analysis_root / "universes").glob("*.yaml"), + analysis_root / SPEC_FILENAME, + ) if path.is_file() ) for name in (*_ENVIRONMENT, *declaring): diff --git a/src/lightcone/engine/plan.py b/src/lightcone/engine/plan.py index 90b82ca6..37b7496b 100644 --- a/src/lightcone/engine/plan.py +++ b/src/lightcone/engine/plan.py @@ -13,11 +13,11 @@ nesting are read here rather than re-derived. Where an output lands follows the spec's own shape: every analysis node -has a *home* — the directory holding its ``astra.yaml`` — and its outputs -materialize to ``/results///.``. +has an *analysis root* — the directory holding its ``astra.yaml`` — and its +materialize to ``/results///.``. An external sub-analysis (``path:``) is a self-similar analysis with its -own home and, where it names one, its own universe; an inline one shares -its parent's home and disambiguates with a scope directory. Graph keys stay +own analysis root and, where it names one, its own universe; an inline one +shares its parent's and disambiguates with a scope directory. Graph keys stay the qualified id, so only the path nests, never the addressing. Nothing here schedules anything. Ordering is Dask's job at execution time @@ -51,7 +51,7 @@ class Task: local_id: str #: The directory holding the astra.yaml that declares this output — #: the project root, or a sub-analysis's own directory. - home: Path + analysis_root: Path #: The single file this output is. output_path: Path #: The recipe with its placeholders substituted — a shell command. @@ -292,11 +292,11 @@ def _validate(spec_path: Path, universes: list[Path]) -> None: class _Placement: """Where one analysis node's outputs land.""" - #: The directory holding that node's ``astra.yaml``. - home: Path + #: The directory holding that node's own ``astra.yaml``. + analysis_root: Path #: The universe its results are filed under. universe_id: str - #: The inline sub-analyses descended through since ``home``. + #: The inline sub-analyses descended through since ``analysis_root``. scope: tuple[str, ...] @@ -312,7 +312,7 @@ def _placements( universe says which of a sub-analysis's own universes was picked. Args: - root: The project root — the root analysis's home. + root: The project root — the root analysis's own analysis root. spec: The analysis, external sub-analyses already resolved. universe: The universe file's contents. universe_id: The universe file's id. @@ -321,7 +321,7 @@ def _placements( Qualified scope → where its outputs land. Raises: - ProjectError: If a sub-analysis's home escapes the project, or it + ProjectError: If a sub-analysis's analysis root escapes the project, or it selects a universe that is not there. """ found: dict[tuple[str, ...], _Placement] = {} @@ -341,24 +341,28 @@ def walk( picked = (chosen.get("analyses") or {}).get(str(sub_id)) or {} declared = sub.get("path") if declared: - home = Path(os.path.normpath(place.home / str(declared))) - if not home.resolve().is_relative_to(inside): + analysis_root = Path(os.path.normpath(place.analysis_root / str(declared))) + if not analysis_root.resolve().is_relative_to(inside): raise ProjectError( f"sub-analysis `{'.'.join(here)}` has path `{declared}`, which is " f"outside the project — its results could not be versioned here." ) selected = picked.get("universe") if selected: - named = home / "universes" / f"{selected}.yaml" + named = analysis_root / "universes" / f"{selected}.yaml" if not named.is_file(): raise ProjectError( f"universe `{universe_id}` selects `{selected}` for " f"sub-analysis `{'.'.join(here)}`, but {declared_path(root, named)} " f"does not exist." ) - nested = _Placement(home, str(selected) if selected else place.universe_id, ()) + nested = _Placement( + analysis_root, str(selected) if selected else place.universe_id, () + ) else: - nested = _Placement(place.home, place.universe_id, (*place.scope, str(sub_id))) + nested = _Placement( + place.analysis_root, place.universe_id, (*place.scope, str(sub_id)) + ) walk(sub, picked, here, nested) walk(spec, universe, (), _Placement(root, universe_id, ())) @@ -384,7 +388,7 @@ def _tasks( where = _placements(root, dict(spec), dict(universe), universe_id) # The whole resolved output, not just its id: an upstream input has to # resolve to the *file* another task writes, which needs that output's - # home, universe, scope and format. A set of ids cannot say any of it. + # analysis root, universe, scope and format. A set of ids cannot say it. executable = {out.id: out for out in resolved if out.command} if absent := sorted(out.id for out in executable.values() if not out.format): @@ -397,7 +401,7 @@ def _tasks( def file_of(out: object) -> Path: place = where[out.scope] # type: ignore[attr-defined] return assets.output_path( - place.home, + place.analysis_root, place.universe_id, place.scope, str(out.definition["id"]), # type: ignore[attr-defined] @@ -442,7 +446,7 @@ def file_of(out: object) -> Path: universe_id=universe_id, output_id=out.id, local_id=str(out.definition["id"]), - home=where[out.scope].home, + analysis_root=where[out.scope].analysis_root, output_path=output_path, recipe=recipe, inputs=paths, From 202f26c13ce3c3ac6702b8399a2ce8d406f8f695 Mon Sep 17 00:00:00 2001 From: EiffL Date: Wed, 26 Aug 2026 08:39:54 +0200 Subject: [PATCH 6/7] Materialize a flat analysis, and only that MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one-file layout arrived carrying support for sub-analyses: an analysis-root walk, scope directories, a results tree per external sub-analysis, and the guards each of those needed — a path-collision refusal, a missing-universe-file refusal, an escape refusal, a broadened `**/results/**` routing rule with its own collateral, a path-shaped dirty-tree split, and a multi-root crate walk. None of that is required by the change this PR is for. An output is `results//.`, one results tree, and the whole of it composed from the spec. `output_path` refuses an output id carrying a dot: the sidecar is recovered by partitioning the filename on the first one, so a dotted id would name a manifest that could not be told from the payload. ASTRA's id grammar carries no dot, so this only fires on an id lc did not compose — which is what makes a nested spec unbuildable today, recorded as #202 with what deciding it would take. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PybSgBiP8pjZeWxxCxiAXi --- CLAUDE.md | 80 +++------ docs/api/crate.md | 4 - docs/api/plan.md | 2 +- docs/user/glossary.md | 11 +- src/lightcone/engine/assets.py | 49 +++--- src/lightcone/engine/crate.py | 20 +-- src/lightcone/engine/materialize.py | 24 +-- src/lightcone/engine/plan.py | 115 +----------- .../engine/templates/files/gitattributes.tmpl | 9 +- .../templates/files/results-README.md.tmpl | 3 +- src/lightcone/engine/worker.py | 4 +- tests/test_assets.py | 26 ++- tests/test_crate.py | 6 - tests/test_dataset.py | 15 -- tests/test_materialize.py | 2 +- tests/test_plan.py | 165 ------------------ tests/test_sandbox_enforcement.py | 19 +- tests/test_sandbox_policy.py | 6 +- tests/test_templates.py | 17 +- 19 files changed, 96 insertions(+), 481 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 65de5617..5bf5c14a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -869,38 +869,15 @@ in the file, far from the line at fault. It caught three of lc's own test fixtures the first time it ran. **One output is one file, and the spec names it.** An output is -`/results///.` — never -directory a recipe fills — and the path in a rendered recipe *is* the path -on disk: no staging, no scratch, no relocation. `format` comes from ASTRA +`results//.` — never a directory a recipe +fills — and the path in a rendered recipe *is* the path on disk: no +staging, no scratch, no relocation. `format` comes from ASTRA (`Output.format`, astra-spec 0.0.14), so lc composes the filename rather -than a recipe choosing it, and the whole contents of every `results/` tree -are a pure function of the spec. `plan.build` **refuses** a spec whose +than a recipe choosing it, and the whole contents of `results/` are a +pure function of the spec. `plan.build` **refuses** a spec whose executable outputs omit it, naming every one: it is only *recommended* until ASTRA 0.1.0, but lc has nowhere to write an output without it. -**Results live beside the spec that declares them.** Every analysis node -has an *analysis root* — the directory holding its `astra.yaml` — and a -universe, -both derived by `plan._placements` walking the spec and the universe file -together: the root analysis's is the project root; an inline sub-analysis -shares its parent's and disambiguates with a scope directory; an -external one (`path:`) is a self-similar analysis with its own root and, -where the parent universe names one, **its own universe id**. Two -refusals ride on that walk — an analysis root escaping the project (its -results could not be versioned) and a `universe: X` naming a file that is -not there (astra logs a warning and settles from the parent's decisions -instead, so lc would file an artifact under a universe it never loaded). - -Only the *path* nests. Graph keys stay `(universe_id, qualified_id)` and -CLI targets stay dotted, because `Graph.resolve` splits on -`rpartition("/")` and a slash-form target would mis-split the universe. - -**No task may share a path with another.** An external sub-analysis is -filed under its own universe, so two parent universes selecting the same -one resolve to the same file while feeding it different inputs — the -second would overwrite the first, silently. `build` refuses, naming both, -the same shape as the duplicate-universe-id refusal. - **One rule names a path, and both the recipe and the run record use it** (`plan.declared_path`). Project-relative inside the tree, absolute outside it, never resolved. A declared input may name an absolute @@ -925,16 +902,19 @@ missing with nothing said. The way in is the natural one: copy refuses, naming both files. **Because the path is composed, `assets.output_path` refuses any part -that is not one path component** — an empty or `..`-bearing universe id, -scope segment or output id would place a file outside the tree every -guard above it checked — and a `format` that could not be an extension -(empty, a separator, or leading-dot, which would make the output look -like its own sidecar). +that is not one path component** — an empty or `..`-bearing universe or +output id would place a file outside the tree every guard above it +checked — and a `format` that could not be an extension (empty, a +separator, or leading-dot, which would make the output look like its own +sidecar). It also refuses an output id carrying a **dot**: the sidecar is +recovered by partitioning the filename on the first one, so a dotted id +would name a manifest for something else. ASTRA's own id grammar carries +no dot, so this only ever fires on an id lc did not compose. **The reset takes what the output's id names, never the directory.** Outputs share a directory now and Dask writes them concurrently, so a whole-directory delete would take a neighbour's bytes. It unlinks the -sidecar and globs `.*`: an id cannot contain a dot, so the glob +sidecar and globs `.*`: an id cannot contain a dot, so the glob cannot reach a sibling, a longer id, another output's sidecar or a scope directory of the same name — and it *does* reach a payload left by a run that declared another `format`, which is what stops one orphaning. @@ -947,7 +927,7 @@ output, plus a crate `File` with no `sha256`. One check covers absent, directory and wrong-name, and it is what makes exit 0 stop being evidence that anything was written. -**The manifest is a sidecar, `..manifest.json`, named from the +**The manifest is a sidecar, `..manifest.json`, named from the id alone and never the format.** So it keeps its path — and therefore its history — across a re-declared serialization, which is what lets `_foreign_write` still answer for an output whose payload path is new. @@ -2227,38 +2207,26 @@ written to" — a path the schema never defined. What changed, and why: - **The filename contract between producer and consumer is gone.** `{inputs.X}` renders to the upstream's *file*, so nothing has to know what is inside a directory it was handed. -- **A results tree per analysis root**, so `results/` stopped being - root-anchored. `.gitattributes` broadened to `**/results/**` — with the - accepted collateral that a user's own `notebooks/results/` now annexes, - cheaper than generating spec-dependent attribute lines repair could - never remove. -- **`results/**` is kept in the template even though `**/results/**` - subsumes it.** A managed line dropped from the template gets rank - `None` and is *skipped* by `gitattributes_disorder` — which would blind - the guard to the exact trap it exists for (a user file carrying the - `results/` opt-out without the `*` defaults). Measured both ways. - **A `check-attr` preflight refuses a run whose manifests would annex.** `dataset.save` passes `annex.dotfiles=true`, so the sidecar's leading dot decides nothing and `.gitattributes` decides everything. Annexed, the run is green locally and every clone reports the whole project as never materialized, silently. Probed once for the whole graph, before anything executes. -- **The orphan walk is a set difference, never a reconstruction.** An - external sub-analysis is filed under its own universe while its graph - key carries the parent's, so no rule turns such a path back into a key. +- **The orphan walk is a set difference, never a reconstruction.** Expected sidecars come from `Task.manifest_path`; present ones from - `git ls-files`, which still finds the tree of a sub-analysis just - deleted from the spec — the edit that orphans it is the same edit that - would drop it from any graph-derived root set. + `git ls-files`, which still finds a manifest whose output the spec has + just dropped — the edit that orphans it is the same edit that would + drop it from anything derived from the graph. - **No migration, by decision.** Nothing looks for `.lightcone-manifest.json` and nothing detects directory-shaped outputs from a previous engine; they are invisible to every walk. `git rm -r results/` and re-materialize. -- **Known residue**: a declared input's `source:` is still resolved - against the *project* root, not the analysis root that declares it, so a - sub-analysis cannot name a file beside its own `astra.yaml` the way it - names its results. Pre-existing; which of the two a `source:` means is - astra's question before it is lc's (issue #201). +- **A nested spec is not buildable.** lc materializes a flat analysis: + ASTRA qualifies an output declared in a sub-analysis as `.`, and + `output_path` refuses a dotted id because the sidecar could not be told + from the payload. Supporting nesting means deciding where a nested + output's file goes, which is its own change (issues #201, #202). ### Recorded deviations from the spec diff --git a/docs/api/crate.md b/docs/api/crate.md index c8055154..b5562db3 100644 --- a/docs/api/crate.md +++ b/docs/api/crate.md @@ -43,10 +43,6 @@ Source: `src/lightcone/engine/crate.py` (converged by so there is one annex key to look up and one `sha256` to publish — the same number its manifest records as `data_version`, and the one `sha256sum` prints. -- **Every analysis root's spec and universes are in the crate**, not - just the root's: a sub-analysis declared with `path:` keeps its own, - and publishing its outputs without them would describe half an - analysis. - **The manifest is not transliterated.** `env_version`, `definition_version` and `hermeticity` get no invented schema.org spelling — the manifest itself is in the crate as a `File`, diff --git a/docs/api/plan.md b/docs/api/plan.md index 6bba5929..93cf8e35 100644 --- a/docs/api/plan.md +++ b/docs/api/plan.md @@ -32,7 +32,7 @@ Source: `src/lightcone/engine/plan.py`. resolution answers what a *valid* spec means and does not re-check that it is one. - **The layout is flat and path-addressed.** - `/results///.`, and the path in a + `results//.`, and the path in a rendered recipe *is* the path on disk — no staging, no relocation. - **`declared_path` is lexical, never `resolve()`d.** A declared input under `data/` is an annex symlink; resolving it writes diff --git a/docs/user/glossary.md b/docs/user/glossary.md index 4181a305..537ba98f 100644 --- a/docs/user/glossary.md +++ b/docs/user/glossary.md @@ -51,14 +51,9 @@ to its own directory: `results//.`. ## Sub-analysis A nested ASTRA analysis with its own inputs, outputs, and decisions, -referenced from a parent's `analyses:` section. Results follow the -spec's shape: one declared inline lands under a scope directory, -`results///.`, while one declared -with `path:` is a self-similar analysis — its own `astra.yaml`, its own -`universes/`, and so its own results tree beside them, -`/results//.`. Addressing does not -nest with the path: an output is always named by its qualified id, -`.`. +referenced from a parent's `analyses:` section. `lc` materializes a flat +analysis: an output id it cannot name a file from is refused, so a +nested spec is not buildable today. ## Materialize diff --git a/src/lightcone/engine/assets.py b/src/lightcone/engine/assets.py index 43eaf1dd..542a3152 100644 --- a/src/lightcone/engine/assets.py +++ b/src/lightcone/engine/assets.py @@ -25,7 +25,7 @@ import hashlib import json -from collections.abc import Mapping, Sequence +from collections.abc import Mapping from dataclasses import asdict, dataclass, field from pathlib import Path from typing import TYPE_CHECKING, Any, Literal @@ -53,59 +53,58 @@ class ContentNotFetchedError(ProjectError): """An annexed file whose content is not in this clone.""" -def output_path( - analysis_root: Path, universe_id: str, scope: Sequence[str], local_id: str, fmt: str -) -> Path: +def output_path(root: Path, universe_id: str, output_id: str, fmt: str) -> Path: """Locate the file one output is materialized to. - ``/results///.``. The *analysis root* is - the directory holding the declaring analysis's ``astra.yaml``, so a - sub-analysis keeps its results beside its own spec, and *scope* is the - inline sub-analyses descended through since that analysis root. - - Path-addressed: this is the path a rendered recipe writes to, with no - staging, scratch or relocation in between. + ``/results//.``. Path-addressed: + this is the path a rendered recipe writes to, with no staging, scratch + or relocation in between. Args: - analysis_root: The declaring analysis's directory. + root: The project root. universe_id: The universe the output is made under. - scope: Inline sub-analysis ids, outermost first. - local_id: The output's own id, unqualified. + output_id: The output's id. fmt: The declared serialization, without a leading dot. Returns: The output's path. Raises: - ProjectError: If any part cannot name a single path component. The - path is *composed*, so a part carrying a separator or ``..`` - would place an output outside the tree the caller checked. + ProjectError: If either id cannot name a single path component, or + the format could not be an extension. The path is *composed*, + so an unchecked part would place a file outside the tree the + caller checked — and an id carrying a dot would make + :func:`manifest_path` recover the wrong name. """ - named = [("universe", universe_id), *(("analysis", s) for s in scope), ("output", local_id)] - for label, value in named: + for label, value in (("universe", universe_id), ("output", output_id)): if not value or "/" in value or "\\" in value or value in {".", ".."}: raise ProjectError( f"{label} id {value!r} is not a single path component, so it " f"cannot name a directory under results/." ) + if "." in output_id: + raise ProjectError( + f"output id {output_id!r} contains a dot, so the manifest beside it " + f"could not be told from the output's own name." + ) if not fmt or "/" in fmt or "\\" in fmt or fmt.startswith("."): raise ProjectError( - f"output `{local_id}` declares the format {fmt!r}, which cannot name a " + f"output `{output_id}` declares the format {fmt!r}, which cannot name a " f"file extension, so the output has nowhere to be written." ) - return analysis_root.joinpath("results", universe_id, *scope, f"{local_id}.{fmt}") + return root / "results" / universe_id / f"{output_id}.{fmt}" def manifest_path(output: Path) -> Path: """The manifest sidecar beside *output*. - ``..manifest.json``, named from the output's id alone and + ``..manifest.json``, named from the output's id alone and never its format — so the manifest keeps its path, and therefore its history, when a spec re-declares the output in another serialization. - An id cannot contain a dot (``^[a-z][a-z0-9_]*$``) while a format can - (``tar.gz``), so the id is recovered by partitioning on the **first** - dot. ``Path.stem`` would answer ``x.tar`` for ``x.tar.gz``. + An id carries no dot (:func:`output_path` refuses one) while a format + may (``tar.gz``), so the id is recovered by partitioning on the + **first** dot. ``Path.stem`` would answer ``x.tar`` for ``x.tar.gz``. Args: output: The output's own path. diff --git a/src/lightcone/engine/crate.py b/src/lightcone/engine/crate.py index b3f25fae..2bb38536 100644 --- a/src/lightcone/engine/crate.py +++ b/src/lightcone/engine/crate.py @@ -287,24 +287,12 @@ def _tool_id(self, output_id: str) -> str: # ----- the data entities ----- def _environment_files(self) -> None: - """The lock and its companions — what every run consumed. - - Every analysis root, not just the project's: a sub-analysis keeps - its spec, its universes and its results together, so publishing - its outputs without the files that define them would leave the - crate describing half an analysis. - """ - analysis_roots = {self.root, *(task.analysis_root for task in self.graph.tasks.values())} - declaring = sorted( + """The lock and its companions — what every run consumed.""" + universes = sorted( path.relative_to(self.root).as_posix() - for analysis_root in analysis_roots - for path in ( - *(analysis_root / "universes").glob("*.yaml"), - analysis_root / SPEC_FILENAME, - ) - if path.is_file() + for path in (self.root / "universes").glob("*.yaml") ) - for name in (*_ENVIRONMENT, *declaring): + for name in (*_ENVIRONMENT, *universes): if (self.root / name).is_file(): self._file(name) if (self.root / "README.md").is_file(): diff --git a/src/lightcone/engine/materialize.py b/src/lightcone/engine/materialize.py index da2e0335..e869f834 100644 --- a/src/lightcone/engine/materialize.py +++ b/src/lightcone/engine/materialize.py @@ -39,7 +39,7 @@ from collections.abc import Iterator, Sequence from contextlib import contextmanager from dataclasses import asdict, dataclass, field -from pathlib import Path, PurePosixPath +from pathlib import Path from typing import Any, Protocol from lightcone.engine import assets, container, dataset, identity, plan, project, venue, worker @@ -1074,7 +1074,7 @@ def _require_manifests_in_git(root: Path, graph: Graph) -> None: f"{plan.declared_path(root, Path(annexed[0]))} would be stored in the annex, " "but a manifest has to stay a plain git blob — it is what a clone reads " "before fetching any content. Check .gitattributes still carries " - "`**/results/**/.*.manifest.json annex.largefiles=nothing`, last." + "`results/**/.*.manifest.json annex.largefiles=nothing`, last." ) @@ -1092,8 +1092,8 @@ def _owned(root: Path, task: Task) -> list[str]: """ parent = plan.declared_path(root, task.output_path.parent) return [ - f":(glob){parent}/{task.local_id}.*", - f":(glob){parent}/.{task.local_id}{assets.MANIFEST_SUFFIX}*", + f":(glob){parent}/{task.output_id}.*", + f":(glob){parent}/.{task.output_id}{assets.MANIFEST_SUFFIX}*", ] @@ -1105,18 +1105,12 @@ def _dirty(root: Path, changes: Sequence[tuple[str, str]]) -> str: """The refusal, split by what the right remedy actually is. Two path classes, because they call for opposite actions: work the - researcher owns has to be committed, and anything under a ``results/`` + researcher owns has to be committed, and anything under ``results/`` is lc's to write, so a change there is wreckage to discard rather than a contribution to keep. - - The test is path-shaped rather than a lookup against the graph: this - refusal runs before the spec is read, deliberately, so that staged work - is protected before anything expensive happens. The cost is that a - directory called ``results`` anywhere in the tree reads as lc's — so - the remedy names the offending paths rather than sweeping a directory. """ - ours = [c for c in changes if "results" in PurePosixPath(c[1]).parts] - theirs = [c for c in changes if c not in ours] + theirs = [c for c in changes if not c[1].startswith("results/")] + ours = [c for c in changes if c[1].startswith("results/")] lines = [ f"uncommitted changes in {root} — every materialization is committed " @@ -1133,9 +1127,7 @@ def _dirty(root: Path, changes: Sequence[tuple[str, str]]) -> str: lines += [ "", " discard these (lc writes results/):", - " git restore --staged --worktree -- " - + " ".join(sorted({path for _, path in ours})), - " git clean -fd -- " + " ".join(sorted({path for _, path in ours})), + " git restore --staged --worktree results/ && git clean -fd results/", *(f" {code.strip() or '??'} {path}" for code, path in ours), ] return "\n".join(lines) diff --git a/src/lightcone/engine/plan.py b/src/lightcone/engine/plan.py index 37b7496b..14b7154d 100644 --- a/src/lightcone/engine/plan.py +++ b/src/lightcone/engine/plan.py @@ -27,11 +27,9 @@ from __future__ import annotations -import os from dataclasses import dataclass from graphlib import CycleError, TopologicalSorter from pathlib import Path -from typing import Any from lightcone.engine import assets, identity from lightcone.engine.project import SPEC_FILENAME, ProjectError @@ -45,13 +43,7 @@ class Task: """One output, in one universe: everything needed to make it.""" universe_id: str - #: The qualified id — ``classification.accuracy`` inside a sub-analysis. output_id: str - #: The id as declared, unqualified: what the file is named after. - local_id: str - #: The directory holding the astra.yaml that declares this output — - #: the project root, or a sub-analysis's own directory. - analysis_root: Path #: The single file this output is. output_path: Path #: The recipe with its placeholders substituted — a shell command. @@ -208,19 +200,6 @@ def build(root: Path) -> Graph: for task in _tasks(root, universe_id, spec, universe): tasks[task.key] = task - # Two tasks composing one path would have the second silently overwrite - # the first. The way in is real: an external sub-analysis is filed under - # its own universe, so two parent universes that both select it resolve - # to the same file while feeding it different inputs. - written_by: dict[Path, Key] = {} - for key, task in tasks.items(): - if (earlier := written_by.get(task.output_path)) is not None: - raise ProjectError( - f"`{earlier[0]}/{earlier[1]}` and `{key[0]}/{key[1]}` both materialize to " - f"{declared_path(root, task.output_path)}. Give the sub-analysis its own " - f"universe per parent universe, or the second would overwrite the first." - ) - written_by[task.output_path] = key return Graph(tasks=tasks) @@ -288,87 +267,6 @@ def _validate(spec_path: Path, universes: list[Path]) -> None: ) -@dataclass(frozen=True) -class _Placement: - """Where one analysis node's outputs land.""" - - #: The directory holding that node's own ``astra.yaml``. - analysis_root: Path - #: The universe its results are filed under. - universe_id: str - #: The inline sub-analyses descended through since ``analysis_root``. - scope: tuple[str, ...] - - -def _placements( - root: Path, - spec: dict[str, Any], - universe: dict[str, Any], - universe_id: str, -) -> dict[tuple[str, ...], _Placement]: - """Locate every analysis node in the tree. - - Walks the spec and the universe file together, because only the - universe says which of a sub-analysis's own universes was picked. - - Args: - root: The project root — the root analysis's own analysis root. - spec: The analysis, external sub-analyses already resolved. - universe: The universe file's contents. - universe_id: The universe file's id. - - Returns: - Qualified scope → where its outputs land. - - Raises: - ProjectError: If a sub-analysis's analysis root escapes the project, or it - selects a universe that is not there. - """ - found: dict[tuple[str, ...], _Placement] = {} - inside = root.resolve() - - def walk( - node: dict[str, Any], - chosen: dict[str, Any], - scope: tuple[str, ...], - place: _Placement, - ) -> None: - found[scope] = place - for sub_id, sub in (node.get("analyses") or {}).items(): - if not isinstance(sub, dict): - continue - here = (*scope, str(sub_id)) - picked = (chosen.get("analyses") or {}).get(str(sub_id)) or {} - declared = sub.get("path") - if declared: - analysis_root = Path(os.path.normpath(place.analysis_root / str(declared))) - if not analysis_root.resolve().is_relative_to(inside): - raise ProjectError( - f"sub-analysis `{'.'.join(here)}` has path `{declared}`, which is " - f"outside the project — its results could not be versioned here." - ) - selected = picked.get("universe") - if selected: - named = analysis_root / "universes" / f"{selected}.yaml" - if not named.is_file(): - raise ProjectError( - f"universe `{universe_id}` selects `{selected}` for " - f"sub-analysis `{'.'.join(here)}`, but {declared_path(root, named)} " - f"does not exist." - ) - nested = _Placement( - analysis_root, str(selected) if selected else place.universe_id, () - ) - else: - nested = _Placement( - place.analysis_root, place.universe_id, (*place.scope, str(sub_id)) - ) - walk(sub, picked, here, nested) - - walk(spec, universe, (), _Placement(root, universe_id, ())) - return found - - def _tasks( root: Path, universe_id: str, @@ -385,10 +283,9 @@ def _tasks( from astra.resolve import render_command, resolve_outputs resolved = resolve_outputs(spec, universe, root) - where = _placements(root, dict(spec), dict(universe), universe_id) # The whole resolved output, not just its id: an upstream input has to # resolve to the *file* another task writes, which needs that output's - # analysis root, universe, scope and format. A set of ids cannot say it. + # format. A set of ids cannot say it. executable = {out.id: out for out in resolved if out.command} if absent := sorted(out.id for out in executable.values() if not out.format): @@ -399,12 +296,10 @@ def _tasks( ) def file_of(out: object) -> Path: - place = where[out.scope] # type: ignore[attr-defined] return assets.output_path( - place.analysis_root, - place.universe_id, - place.scope, - str(out.definition["id"]), # type: ignore[attr-defined] + root, + universe_id, + str(out.id), # type: ignore[attr-defined] str(out.format), # type: ignore[attr-defined] ) @@ -445,8 +340,6 @@ def file_of(out: object) -> Path: Task( universe_id=universe_id, output_id=out.id, - local_id=str(out.definition["id"]), - analysis_root=where[out.scope].analysis_root, output_path=output_path, recipe=recipe, inputs=paths, diff --git a/src/lightcone/engine/templates/files/gitattributes.tmpl b/src/lightcone/engine/templates/files/gitattributes.tmpl index b9bde9be..ffd18568 100644 --- a/src/lightcone/engine/templates/files/gitattributes.tmpl +++ b/src/lightcone/engine/templates/files/gitattributes.tmpl @@ -5,12 +5,6 @@ # decides what counts as content: the default is nothing, and only outputs # and declared inputs opt out. Last matching line wins. # -# An analysis keeps its results beside its own astra.yaml, so a results -# tree sits under a sub-analysis directory as readily as at the root. -# `results/**` is kept alongside the broader rule: a managed line dropped -# from this template stops being ranked, and the ordering check that keeps -# the defaults above the opt-outs would go blind to it. -# # Manifests are exempted back out because they must be readable on a clone # that has fetched no annex content. * annex.largefiles=nothing @@ -18,5 +12,4 @@ results/** annex.largefiles=anything data/** annex.largefiles=anything .datalad/environments/*/image annex.largefiles=anything -**/results/** annex.largefiles=anything -**/results/**/.*.manifest.json annex.largefiles=nothing +results/**/.*.manifest.json annex.largefiles=nothing diff --git a/src/lightcone/engine/templates/files/results-README.md.tmpl b/src/lightcone/engine/templates/files/results-README.md.tmpl index 20e5ea6e..c0925bc6 100644 --- a/src/lightcone/engine/templates/files/results-README.md.tmpl +++ b/src/lightcone/engine/templates/files/results-README.md.tmpl @@ -6,5 +6,4 @@ from the output's id and the `format:` its spec declares: results//. Beside each one is a `..manifest.json` recording how it was -made. A sub-analysis declared with `path:` keeps its own results tree, -beside its own `astra.yaml`. +made. diff --git a/src/lightcone/engine/worker.py b/src/lightcone/engine/worker.py index 8dede9d1..ffffce73 100644 --- a/src/lightcone/engine/worker.py +++ b/src/lightcone/engine/worker.py @@ -228,13 +228,13 @@ def execute( # they are being written concurrently. What this output owns is its # manifest and any file named after its id — the glob rather than the # one declared path, so a spec that re-declares the output in another - # format leaves no stale payload behind. `.` needs a literal + # format leaves no stale payload behind. `.` needs a literal # dot straight after the whole id, and an id cannot contain one, so it # cannot reach a sibling, a longer id, another output's sidecar, or a # scope directory of the same name. task.output_path.parent.mkdir(parents=True, exist_ok=True) task.manifest_path.unlink(missing_ok=True) - for stale in task.output_path.parent.glob(f"{task.local_id}.*"): + for stale in task.output_path.parent.glob(f"{task.output_id}.*"): if stale.is_file() or stale.is_symlink(): stale.unlink() diff --git a/tests/test_assets.py b/tests/test_assets.py index 93a7ff0a..cde3d7d7 100644 --- a/tests/test_assets.py +++ b/tests/test_assets.py @@ -53,7 +53,7 @@ def _manifest(**overrides: object) -> Manifest: def test_an_asset_is_addressed_by_its_path(tmp_path: Path) -> None: """The path in a rendered recipe is the path on disk — no staging, no scratch, no relocation.""" - assert output_path(tmp_path, "baseline", (), "best_fit", "csv") == ( + assert output_path(tmp_path, "baseline", "best_fit", "csv") == ( tmp_path / "results/baseline/best_fit.csv" ) @@ -121,11 +121,9 @@ def test_output_path_refuses_a_part_that_is_not_one_path_component( """The path is composed, so a part carrying a separator would put an output outside the tree every guard above it checked.""" with pytest.raises(ProjectError): - assets.output_path(tmp_path, bad, (), "best_fit", "csv") + assets.output_path(tmp_path, bad, "best_fit", "csv") with pytest.raises(ProjectError): - assets.output_path(tmp_path, "baseline", (), bad, "csv") - with pytest.raises(ProjectError): - assets.output_path(tmp_path, "baseline", (bad,), "best_fit", "csv") + assets.output_path(tmp_path, "baseline", bad, "csv") @pytest.mark.parametrize("bad", ["", "/", "a/b", ".hidden"]) @@ -135,23 +133,23 @@ def test_output_path_refuses_a_format_that_cannot_be_an_extension( """A leading dot would make the output look like its own sidecar; a separator would move it entirely.""" with pytest.raises(ProjectError): - assets.output_path(tmp_path, "baseline", (), "best_fit", bad) + assets.output_path(tmp_path, "baseline", "best_fit", bad) -def test_an_inline_sub_analysis_nests_below_its_universe(tmp_path: Path) -> None: - """Scope becomes directories, so one addressing scheme spans however - deep the spec nests without the filename carrying the structure.""" - assert assets.output_path(tmp_path, "baseline", ("hod", "mass"), "fit", "npz") == ( - tmp_path / "results" / "baseline" / "hod" / "mass" / "fit.npz" - ) +def test_output_path_refuses_an_id_carrying_a_dot(tmp_path: Path) -> None: + """The sidecar is the id with a leading dot and `.manifest.json` after + it, recovered by partitioning on the first dot — so an id carrying one + of its own would name a manifest for something else.""" + with pytest.raises(ProjectError, match="dot"): + assets.output_path(tmp_path, "baseline", "fit.plot", "png") def test_the_manifest_is_named_from_the_id_never_the_format(tmp_path: Path) -> None: """A format may contain dots, and `Path.stem` would answer `x.tar` for `x.tar.gz`. Naming the sidecar from the id alone also keeps its path — and so its history — across a re-declared serialization.""" - packed = assets.output_path(tmp_path, "baseline", (), "chain", "tar.gz") - plain = assets.output_path(tmp_path, "baseline", (), "chain", "npz") + packed = assets.output_path(tmp_path, "baseline", "chain", "tar.gz") + plain = assets.output_path(tmp_path, "baseline", "chain", "npz") assert assets.manifest_path(packed).name == ".chain.manifest.json" assert assets.manifest_path(packed) == assets.manifest_path(plain) diff --git a/tests/test_crate.py b/tests/test_crate.py index 4b6edec6..3f802279 100644 --- a/tests/test_crate.py +++ b/tests/test_crate.py @@ -83,8 +83,6 @@ def _graph(root: Path, universes: tuple[str, ...] = ("baseline",)) -> Graph: tasks[(universe_id, "first")] = Task( universe_id, "first", - "first", - root, first, "make first", {"catalog": root / "data" / "catalog.csv"}, @@ -95,8 +93,6 @@ def _graph(root: Path, universes: tuple[str, ...] = ("baseline",)) -> Graph: tasks[(universe_id, "second")] = Task( universe_id, "second", - "second", - root, root / "results" / universe_id / "second.txt", "make second", {"first": first}, @@ -406,8 +402,6 @@ def test_an_out_of_tree_input_publishes_no_checksum( task = Task( "baseline", "first", - "first", - project, project / "results/baseline/first.txt", "make first", {"catalog": catalog}, diff --git a/tests/test_dataset.py b/tests/test_dataset.py index d5fb254b..308b3b97 100644 --- a/tests/test_dataset.py +++ b/tests/test_dataset.py @@ -83,21 +83,6 @@ def test_save_puts_result_bytes_in_the_annex_and_the_manifest_in_git(repo: Path) assert not dataset.status(repo) -def test_a_sub_analysis_results_tree_is_routed_the_same_way(repo: Path) -> None: - """An analysis keeps its results beside its own astra.yaml, so the - routing rule cannot be anchored at the project root — a sub-analysis's - payload would land in git as a plain blob, invisibly.""" - results = repo / "hod" / "results" / "fast" - results.mkdir(parents=True) - (results / "mass_function.npz").write_bytes(b"binned\n" * 64) - (results / ".mass_function.manifest.json").write_text("{}\n") - - assert dataset.save(repo, [results], "materialize the sub-analysis") - - assert _annexed(repo, results / "mass_function.npz") - assert not _annexed(repo, results / ".mass_function.manifest.json") - - def test_a_plain_git_add_annexes_content_by_itself(repo: Path) -> None: """`filter=annex` is what makes git's own add route content, which is what lets lc — and everyone else — never run a git-annex command.""" diff --git a/tests/test_materialize.py b/tests/test_materialize.py index 49c7d6ab..fb33982b 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -1009,7 +1009,7 @@ def test_a_manifest_the_attributes_would_annex_refuses_before_the_graph_runs( attributes = root / ".gitattributes" attributes.write_text( attributes.read_text().replace( - "**/results/**/.*.manifest.json annex.largefiles=nothing", "" + "results/**/.*.manifest.json annex.largefiles=nothing", "" ) ) dataset.save(root, [attributes], "route manifests into the annex") diff --git a/tests/test_plan.py b/tests/test_plan.py index c4271d23..48a994b0 100644 --- a/tests/test_plan.py +++ b/tests/test_plan.py @@ -260,159 +260,6 @@ def test_no_universe_is_a_clean_error(tmp_path: Path) -> None: _build(tmp_path) -# ---- sub-analyses ---------------------------------------------------------- - -_PARENT = """ -version: "0.0.13" -name: parent - -inputs: - - id: catalog - type: data - source: data/catalog.fits - -outputs: - - id: mass_function - from: hod.mass_function - - - id: summary - type: report - format: md - inputs: [mass_function] - recipe: - command: python summarize.py {inputs.mass_function} {output} - -analyses: - hod: - path: ./analyses/hod -""" - -_SUB = """ -version: "0.0.13" -name: hod - -inputs: - - id: catalog - type: data - from: ../catalog - -outputs: - - id: mass_function - type: metric - format: npz - inputs: [catalog] - decisions: [binning] - recipe: - command: python hod.py {inputs.catalog} --bins {decisions.binning} {output} - -decisions: - binning: - label: Binning - default: log - options: - log: {label: log} - linear: {label: linear} -""" - - -def _tree(root: Path) -> Path: - root.mkdir(parents=True, exist_ok=True) - (root / "astra.yaml").write_text(textwrap.dedent(_PARENT)) - (root / "universes").mkdir() - # The sub-analysis's universe is named explicitly: ASTRA has no - # implicit "same id" fallback, and lc no longer invents one. - (root / "universes" / "baseline.yaml").write_text( - "id: baseline\ndecisions: {}\nanalyses:\n hod:\n universe: baseline\n" - ) - sub = root / "analyses" / "hod" - (sub / "universes").mkdir(parents=True) - (sub / "astra.yaml").write_text(textwrap.dedent(_SUB)) - (sub / "universes" / "baseline.yaml").write_text("id: baseline\ndecisions:\n binning: log\n") - return root - - -def test_a_sub_analysis_is_addressed_by_its_qualified_id(tmp_path: Path) -> None: - """One addressing scheme whatever shape the spec has: only the path - nests, never the key.""" - graph = _build(_tree(tmp_path)) - assert sorted(graph.tasks) == [("baseline", "hod.mass_function"), ("baseline", "summary")] - - -def test_an_external_sub_analysis_keeps_results_beside_its_own_spec(tmp_path: Path) -> None: - """A sub-analysis with a `path:` is a self-similar analysis — its own - astra.yaml, its own universes, and so its own results tree.""" - root = _tree(tmp_path) - task = _build(root).tasks[("baseline", "hod.mass_function")] - assert task.output_path == root / "analyses/hod/results/baseline/mass_function.npz" - assert task.manifest_path == ( - root / "analyses/hod/results/baseline/.mass_function.manifest.json" - ) - - -def test_a_consumer_is_handed_the_upstream_file_not_its_directory(tmp_path: Path) -> None: - """The filename contract the spec now carries: a downstream recipe is - given the actual file, so nothing has to know what is inside a - directory it was handed.""" - root = _tree(tmp_path) - recipe = _build(root).tasks[("baseline", "summary")].recipe - assert "analyses/hod/results/baseline/mass_function.npz" in recipe - - -def test_a_re_export_resolves_to_what_actually_makes_the_bytes(tmp_path: Path) -> None: - """`summary` names the parent's re-export, which carries no recipe of - its own — so its input has to be the sub-analysis's file.""" - root = _tree(tmp_path) - task = _build(root).tasks[("baseline", "summary")] - assert task.produced_by == {"mass_function": ("baseline", "hod.mass_function")} - assert task.inputs["mass_function"] == ( - root / "analyses/hod/results/baseline/mass_function.npz" - ) - - -def test_a_sub_analysis_reaching_outside_the_project_is_refused(tmp_path: Path) -> None: - """Its results would land outside the repository, where nothing could - version or fetch them. The sub-analysis is real and resolves — only - where it sits is wrong.""" - root = _tree(tmp_path / "project") - (root / "astra.yaml").write_text( - textwrap.dedent(_PARENT).replace("path: ./analyses/hod", "path: ../hod") - ) - outside = tmp_path / "hod" - (outside / "universes").mkdir(parents=True) - (outside / "astra.yaml").write_text(textwrap.dedent(_SUB)) - (outside / "universes" / "baseline.yaml").write_text( - "id: baseline\ndecisions:\n binning: log\n" - ) - with pytest.raises(ProjectError, match="outside the project"): - _build(root) - - -def test_a_sub_analysis_universe_that_is_not_there_is_refused(tmp_path: Path) -> None: - """astra logs a warning and settles the sub-analysis from the parent's - decisions instead, so without this lc would file an artifact under a - universe it never loaded.""" - root = _tree(tmp_path) - (root / "universes" / "baseline.yaml").write_text( - "id: baseline\ndecisions: {}\nanalyses:\n hod:\n universe: absent\n" - ) - with pytest.raises(ProjectError, match="absent"): - _build(root) - - - - - - - - - - - - - - - - def test_an_output_without_a_format_is_refused_by_name(tmp_path: Path) -> None: """lc names the file from it, so there is nowhere to write the output. Every offender at once: a spec is fixed in one pass, not one run per @@ -426,18 +273,6 @@ def test_an_output_without_a_format_is_refused_by_name(tmp_path: Path) -> None: assert "fit" in str(raised.value) and "report" in str(raised.value) -def test_two_tasks_that_would_write_one_file_are_refused(tmp_path: Path) -> None: - """An external sub-analysis is filed under its own universe, so two - parent universes selecting the same one resolve to the same path while - feeding it different inputs. The second would overwrite the first.""" - root = _tree(tmp_path / "project") - (root / "universes" / "robust.yaml").write_text( - "id: robust\ndecisions: {}\nanalyses:\n hod:\n universe: baseline\n" - ) - with pytest.raises(ProjectError, match="both materialize to"): - _build(root) - - # ---- rendering a recipe ---------------------------------------------------- diff --git a/tests/test_sandbox_enforcement.py b/tests/test_sandbox_enforcement.py index 2e7b16ab..179bf6ee 100644 --- a/tests/test_sandbox_enforcement.py +++ b/tests/test_sandbox_enforcement.py @@ -382,28 +382,25 @@ def test_results_can_be_written(backend: sandbox.Backend, project: Path) -> None assert (project / "results" / "out.csv").read_text() == "out" -def test_a_recipe_cannot_write_another_analysis_results_tree( +def test_a_recipe_cannot_write_another_universes_results( backend: sandbox.Backend, project: Path ) -> None: """The scope is the directory the output lands in, so outputs declared beside each other are mutually writable — `data_version` is what answers for an output's bytes, not the boundary. What the kernel does - still refuse is every *other* results tree: another universe, and - another analysis's own. Both writes target user-owned paths, so only - the boundary can refuse the first; the second is the mutation check - in-place.""" + still refuse is every *other* universe. Both writes target user-owned + paths, so only the boundary can refuse the first; the second is the + mutation check in-place.""" own = project / "results" / "baseline" - other = project / "hod" / "results" / "fast" + other = project / "results" / "robust" own.mkdir(parents=True) other.mkdir(parents=True) - (other / "mass_function.npz").write_text("theirs\n") + (other / "first.txt").write_text("theirs\n") with sandbox.scope(sandbox.exec_policy(project, write_dir=own)) as policy: - crossed = shell( - backend, policy, f"printf forged > {other / 'mass_function.npz'}", cwd=project - ) + crossed = shell(backend, policy, f"printf forged > {other / 'first.txt'}", cwd=project) owned = shell(backend, policy, f"printf mine > {own / 'first.txt'}", cwd=project) assert crossed.returncode != 0 - assert (other / "mass_function.npz").read_text() == "theirs\n", "the file changed anyway" + assert (other / "first.txt").read_text() == "theirs\n", "the file changed anyway" assert owned.returncode == 0, owned.stderr assert (own / "first.txt").read_text() == "mine" diff --git a/tests/test_sandbox_policy.py b/tests/test_sandbox_policy.py index 4c5a9a70..0cfc2605 100644 --- a/tests/test_sandbox_policy.py +++ b/tests/test_sandbox_policy.py @@ -65,17 +65,15 @@ def test_a_recipe_is_narrowed_to_the_directory_its_output_lands_in(tmp_path: Pat directory, and outputs declared side by side are mutually writable. Recorded rather than papered over: `data_version` is what answers whether an output's bytes are its own. What the scope does still - exclude is every other universe and every other analysis.""" + exclude is every other universe.""" project = tmp_path / "proj" own = project / "results" / "baseline" elsewhere = project / "results" / "robust" - sub = project / "hod" / "results" / "fast" - for path in (own, elsewhere, sub): + for path in (own, elsewhere): path.mkdir(parents=True) with scope(policy_module.exec_policy(project, write_dir=own)) as built: assert built.grants(own / "first.txt", built.write) assert not built.grants(elsewhere / "first.txt", built.write) - assert not built.grants(sub / "mass_function.npz", built.write) assert not built.grants(project / "results", built.write) assert built.grants(elsewhere / "first.txt", built.read), ( "an upstream output is still a readable input" diff --git a/tests/test_templates.py b/tests/test_templates.py index fd9c6593..7fb782eb 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -123,8 +123,7 @@ def test_gitattributes_routes_content_to_the_annex_and_everything_else_to_git() assert entries[0] == "* annex.largefiles=nothing" assert "results/** annex.largefiles=anything" in entries assert "data/** annex.largefiles=anything" in entries - assert "**/results/** annex.largefiles=anything" in entries - assert "**/results/**/.*.manifest.json annex.largefiles=nothing" in entries + assert "results/**/.*.manifest.json annex.largefiles=nothing" in entries def test_gitattributes_exceptions_come_after_the_default() -> None: @@ -163,20 +162,6 @@ def test_an_opt_out_the_defaults_would_land_below_is_named() -> None: assert misplaced == "* annex.largefiles=nothing" -def test_an_analysis_keeps_its_results_beside_its_own_spec() -> None: - """A sub-analysis is a self-similar analysis in its own directory, so a - results tree is not root-anchored. The narrow `results/**` is kept - beside the broad rule deliberately: a managed line dropped from the - template stops being ranked, which would blind the ordering check to - the very file `test_an_opt_out_the_defaults_would_land_below_is_named` - exists to catch.""" - entries = templates.entries("gitattributes.tmpl") - assert "results/** annex.largefiles=anything" in entries - assert entries.index("**/results/** annex.largefiles=anything") < entries.index( - "**/results/**/.*.manifest.json annex.largefiles=nothing" - ) - - def test_a_gitattributes_written_by_an_earlier_template_still_converges() -> None: """Repair only appends, at end of file, carrying each line's template rank — so a managed line added anywhere but last strands every file From 0007e2019ab98a4edf007bb9ffbe4fe8a76e6a86 Mon Sep 17 00:00:00 2001 From: EiffL Date: Wed, 26 Aug 2026 09:04:26 +0200 Subject: [PATCH 7/7] Take the review's cuts, and fix what the docs claimed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review comments, in order: - The `check-attr` preflight and `dataset.largefiles` are gone. `.gitattributes` is convergence's file: its repair appends the exemption and its disorder guard blocks what it cannot fix, so a second answer in the run path is one more place for the two to disagree. The residue is recorded. - The eval harness no longer rewrites the seed's spec version. The seed is bumped and that is the whole of it. - `docs/api/assets.md` documented `output_path(analysis_root, u, scope, id, fmt)`; the signature has been `(root, universe_id, output_id, fmt)` since the flat layout landed. And what running the tutorial turned up — the docs rule is to verify every block by executing it, which is exactly what had not been done: - `src/fit.py` and `src/plot.py` still wrote into `{output}` as a directory, so the whole walkthrough raised `NotADirectoryError` on step one. Both now write the file they are given. - The `lc status` sample was missing the `crate:` line a real run prints. Verified by building the tutorial from the doc's own blocks: it produces the fit and a 960x720 PNG, and is idempotent. Two fixes the review found that were mine: - `restore` guarded its checkout with `ls-files --with-tree=HEAD`, which also lists what is staged but unborn — exactly where `checkout HEAD --` then exits nonzero. An interrupted save would make the `finally` restore raise over whatever interrupted it. The checkout goes through `_git_ok` instead, with no guard: a pathspec that matches nothing is a no-op. - `tracked_manifests` globbed the whole repository, so any tracked file named `*.manifest.json` reached `assets.read`, which raised `AttributeError` on a top-level array and `UnicodeDecodeError` on bytes. Scoped to `results/`, and `read` now answers `None` for anything that is not a manifest object. Plus stale prose the simplification left: the `plan` and `assets` module docstrings still described analysis roots and scope directories, the orphan walk's comment justified itself with a case that no longer exists, `docs/api/worker.md` named the removed `local_id`, `CLAUDE.md` carried two spellings of the manifest attribute line, and the crate's `encodingFormat` line could never differ from what `_file()` already set (`_FORMATS` has no multi-dot key — checked). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PybSgBiP8pjZeWxxCxiAXi --- .github/workflows/eval.yml | 7 ---- CLAUDE.md | 17 ++++++--- docs/api/assets.md | 2 +- docs/api/worker.md | 2 +- docs/user/getting-started.md | 8 ++-- src/lightcone/engine/assets.py | 22 +++++------ src/lightcone/engine/crate.py | 2 - src/lightcone/engine/dataset.py | 57 +++++++++-------------------- src/lightcone/engine/materialize.py | 44 ++-------------------- src/lightcone/engine/plan.py | 10 ++--- tests/test_dataset.py | 44 ++++++++++++++++++++++ tests/test_materialize.py | 23 ------------ 12 files changed, 97 insertions(+), 141 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 843cb2e8..949205c9 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -80,15 +80,8 @@ jobs: project="$RUNNER_TEMP/eval-project" echo "PROJECT=$project" >> "$GITHUB_ENV" lc init "$project" - # `lc init` writes the spec version the installed astra-spec - # provides; the task seed is a committed fixture pinned by hand, - # so it drifts on every astra-spec release and `astra validate` - # then warns on every call — training the agent under test to - # read past version warnings. Keep the scaffold's answer. - spec_version="$(grep -m1 '^version:' "$project/astra.yaml")" # Overlay the task seed (astra.yaml, data/) on the scaffold cp -r evals/tasks/snae/. "$project"/ - sed -i "0,/^version:/s|^version:.*|$spec_version|" "$project/astra.yaml" cd "$project" # Regenerate baseline.yaml from the task astra.yaml's defaults — # the scaffolded one matches the boilerplate spec, not the task's diff --git a/CLAUDE.md b/CLAUDE.md index 5bf5c14a..89c868cb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -478,7 +478,7 @@ Without it `filter=annex` routes *everything* into the annex, analysis code included. `tests/test_dataset.py::test_analysis_code_stays_in_git_and_stays_writable` pins it against a real annex. -**Manifests stay in git, deliberately.** `**/results/**/.*.manifest.json` +**Manifests stay in git, deliberately.** `results/**/.*.manifest.json` is exempted back out of the annex so it is readable on a clone that has fetched no annex content at all — which is what lets `lc materialize --check` classify a whole project on a laptop that holds none of the @@ -2207,12 +2207,17 @@ written to" — a path the schema never defined. What changed, and why: - **The filename contract between producer and consumer is gone.** `{inputs.X}` renders to the upstream's *file*, so nothing has to know what is inside a directory it was handed. -- **A `check-attr` preflight refuses a run whose manifests would annex.** +- **`.gitattributes` is convergence's to get right, not a run's.** `dataset.save` passes `annex.dotfiles=true`, so the sidecar's leading - dot decides nothing and `.gitattributes` decides everything. Annexed, - the run is green locally and every clone reports the whole project as - never materialized, silently. Probed once for the whole graph, before - anything executes. + dot decides nothing and the attributes decide everything — annexed, a + run is green locally while every clone reports the project as never + materialized. A `check-attr` preflight in `materialize` was written and + removed: `lc init` owns that file, its repair appends the exemption and + its disorder guard blocks a file it cannot fix, and a second answer in + the run path is one more place for the two to disagree. The residue, + stated: a project scaffolded before the exemption existed annexes its + manifests until someone converges it — which this change already + requires, since every output needs a `format:` added by hand. - **The orphan walk is a set difference, never a reconstruction.** Expected sidecars come from `Task.manifest_path`; present ones from `git ls-files`, which still finds a manifest whose output the spec has diff --git a/docs/api/assets.md b/docs/api/assets.md index 916e5c80..b584bd97 100644 --- a/docs/api/assets.md +++ b/docs/api/assets.md @@ -17,7 +17,7 @@ Source: `src/lightcone/engine/assets.py`. | `data_version(path)` | Content hash of a directory or file — computed in the worker, before anything is annexed. | | `Versions` | Per-run memo so a shared declared input hashes once, not once per dependent. | | `read(sidecar)` / `write(...)` | The manifest, `..manifest.json`. Both take the sidecar's own path, so a caller holding an output path has to say `manifest_path` out loud. | -| `output_path(analysis_root, u, scope, id, fmt)` | The output's file, guarded: any part that is not a single path component is refused, and so is a format that could not be an extension. | +| `output_path(root, u, id, fmt)` | The output's file, guarded: any part that is not a single path component is refused, and so is a format that could not be an extension. | | `manifest_path(output)` | The sidecar beside it, named from the id alone — so it keeps its path, and its history, across a re-declared format. | | `ContentNotFetchedError` | An annexed file whose content is not in this clone, in either shape it takes. | diff --git a/docs/api/worker.md b/docs/api/worker.md index 40e96db7..a54cc751 100644 --- a/docs/api/worker.md +++ b/docs/api/worker.md @@ -40,7 +40,7 @@ Source: `src/lightcone/engine/worker.py`. - **The reset takes what the output's id names, never the directory** — outputs share a directory and Dask writes them concurrently, so a whole-directory delete would take a neighbour's bytes with it. The - glob is `.*` plus the sidecar: an id cannot contain a dot, + glob is `.*` plus the sidecar: an id cannot contain a dot, so it cannot reach a sibling, and it *does* reach a payload left by a run that declared another `format`. - **A payload that is not a regular file fails the task.** `data_version` diff --git a/docs/user/getting-started.md b/docs/user/getting-started.md index fd66297d..43c28d04 100644 --- a/docs/user/getting-started.md +++ b/docs/user/getting-started.md @@ -196,8 +196,7 @@ if args.outliers == "clip": x, y = x[mask], y[mask] slope, intercept = np.polyfit(x, y, 1) -out = Path(args.output) -(out / "fit.json").write_text( +Path(args.output).write_text( json.dumps({"slope": slope, "intercept": intercept, "n_used": len(x)}, indent=2) ) ``` @@ -223,7 +222,7 @@ parser.add_argument("--output", required=True) args = parser.parse_args() x, y = np.loadtxt(args.points, delimiter=",", skiprows=1, unpack=True) -fit = json.loads((Path(args.fit) / "fit.json").read_text()) +fit = json.loads(Path(args.fit).read_text()) fig, ax = plt.subplots() ax.scatter(x, y, s=12) @@ -232,7 +231,7 @@ ax.plot(xs, fit["slope"] * xs + fit["intercept"], color="C1") ax.set_xlabel("x") ax.set_ylabel("y") ax.set_title(f"slope = {fit['slope']:.3f}") -fig.savefig(Path(args.output) / "fit_plot.png", dpi=150) +fig.savefig(args.output, dpi=150) ``` Both scripts import from the project's locked environment, so declare @@ -287,6 +286,7 @@ lc status ``` mode: direct sandbox: landlock (fs: declared, network: allowed) + crate: not maintained — declare [project].license to enable it · current baseline/fit a3f1f11 · current baseline/fit_plot a3f1f11 diff --git a/src/lightcone/engine/assets.py b/src/lightcone/engine/assets.py index 542a3152..222f4a69 100644 --- a/src/lightcone/engine/assets.py +++ b/src/lightcone/engine/assets.py @@ -1,16 +1,12 @@ """What a materialized output *is*: where it lives, what it records, and whether it is still current. -An asset is a single file — ``/results///.`` -— with a manifest sidecar beside it, ``..manifest.json``. The format -comes from the spec, so the path is derived rather than chosen by the -recipe, and one output can only ever be one file. The manifest is the only -part lc writes itself, and it is kept out of the annex so it stays readable -on a clone that has fetched no content at all. - -The *analysis root* is the directory holding that analysis's own ``astra.yaml`` — the -project root, or a sub-analysis's own directory — so results sit beside the -spec that declares them. +An asset is a single file — ``results//.`` — +with a manifest sidecar beside it, ``..manifest.json``. The +format comes from the spec, so the path is derived rather than chosen by +the recipe, and one output can only ever be one file. The manifest is the +only part lc writes itself, and it is kept out of the annex so it stays +readable on a clone that has fetched no content at all. The rule that classifies an output — ``current``, ``behind`` or ``stale`` — lives here too, next to the manifest it reads and the hashes @@ -373,7 +369,11 @@ def read(manifest: Path) -> Manifest | None: try: data = json.loads(path.read_text()) return Manifest(**{k: v for k, v in data.items() if k in _FIELDS}) - except (json.JSONDecodeError, TypeError): + except (json.JSONDecodeError, TypeError, AttributeError, UnicodeDecodeError, ValueError): + # Anything that is not a manifest object reads as "no manifest": + # a top-level array, bytes that are not text, a mapping whose + # fields do not fit. The staleness rule takes that as "make it + # again", which is the safe direction for all of them. return None diff --git a/src/lightcone/engine/crate.py b/src/lightcone/engine/crate.py index 2bb38536..e24ee21a 100644 --- a/src/lightcone/engine/crate.py +++ b/src/lightcone/engine/crate.py @@ -358,8 +358,6 @@ def _dataset(self, key: Key, manifest: assets.Manifest) -> None: entity["name"] = f"{output_id} (universe {universe_id})" entity["description"] = f"output `{output_id}` materialized under `{universe_id}`" entity["version"] = manifest.data_version - if media := _FORMATS.get(f".{task.output_path.name.partition('.')[2]}", ""): - entity["encodingFormat"] = media manifest_file = self._file(plan.declared_path(self.root, task.manifest_path)) manifest_file["about"] = {"@id": entity.id} entity["subjectOf"] = {"@id": manifest_file.id} diff --git a/src/lightcone/engine/dataset.py b/src/lightcone/engine/dataset.py index dadf65e0..be638329 100644 --- a/src/lightcone/engine/dataset.py +++ b/src/lightcone/engine/dataset.py @@ -18,7 +18,7 @@ from __future__ import annotations -from collections.abc import Iterable, Sequence +from collections.abc import Iterable from dataclasses import dataclass from pathlib import Path @@ -331,39 +331,14 @@ def annex_keys(directory: Path) -> dict[str, str]: return keys -def largefiles(directory: Path, paths: Sequence[Path]) -> dict[str, str]: - """Ask ``.gitattributes`` where each of *paths* would be stored. - - ``check-attr`` answers about the *rules*, so it works on paths that do - not exist yet — which is what lets a run ask before it writes. - - Args: - directory: The repository root. - paths: What to ask about. - - Returns: - Repository-relative path → the ``annex.largefiles`` value, which - is ``"unspecified"`` where no line matches. - """ - if not paths: - return {} - relative = [_rel(directory, p) for p in paths] - out = _git(["check-attr", "-z", "annex.largefiles", "--", *relative], cwd=directory) - fields = out.split("\0") - return { - fields[i]: fields[i + 2] for i in range(0, len(fields) - 2, 3) if fields[i + 1] - } - - def tracked_manifests(directory: Path) -> list[Path]: """Every manifest sidecar git is tracking, anywhere in the tree. From the index rather than a directory walk: it is exact, it costs one - process, it never descends into ``.git`` or ``.venv``, and it still - finds the manifests under a sub-analysis that has just been deleted - from the spec — which is the case a walk driven by the current spec - would miss, in the very edit that orphans them. Untracked strays are - the dirty check's business, not this one's. + process, and it never descends into ``.git`` or ``.venv``. Scoped to + ``results/``, which is the only place lc writes one — elsewhere the + name is the user's to use. Untracked strays are the dirty check's + business, not this one's. Args: directory: The repository root. @@ -375,7 +350,9 @@ def tracked_manifests(directory: Path) -> list[Path]: # through `project`, so the name has to be fetched after both exist. from lightcone.engine.assets import MANIFEST_SUFFIX - listed = _git(["ls-files", "-z", "--", f":(glob)**/*{MANIFEST_SUFFIX}"], cwd=directory) + listed = _git( + ["ls-files", "-z", "--", f":(glob)results/**/*{MANIFEST_SUFFIX}"], cwd=directory + ) return [directory / name for name in listed.split("\0") if name] @@ -442,13 +419,16 @@ def restore(directory: Path, paths: Iterable[Path | str]) -> None: """Put *paths* back the way the last commit had them. ``clean`` first for what a run wrote, then ``checkout`` for what it - deleted or truncated — and only when ``HEAD`` has something matching, - since a first materialization has nothing to go back to and - ``checkout`` errors on a pathspec it cannot match. + deleted or truncated — through ``_git_ok``, because a first + materialization has nothing to go back to and ``checkout`` exits + nonzero on a pathspec it cannot match. - ``ls-files --with-tree`` asks that question, not ``cat-file -e``: - a pathspec may be a glob, and ``cat-file`` takes a single path while - ``ls-tree`` refuses glob magic outright. + No guard in front of it: a pathspec may be a glob, and nothing cheap + answers "is this in HEAD" for one. ``cat-file -e`` takes a single + path, ``ls-tree`` refuses glob magic, and ``ls-files --with-tree`` + also lists what is staged but *unborn* — so it says yes exactly where + ``checkout`` then says no, which is the state an interrupted save + leaves behind. Args: directory: The repository root. @@ -459,8 +439,7 @@ def restore(directory: Path, paths: Iterable[Path | str]) -> None: for path in paths: rel = _rel(directory, path) _git(["clean", "-qfdx", "--", rel], cwd=directory) - if _git(["ls-files", "--with-tree=HEAD", "--", rel], cwd=directory).strip(): - _git(["checkout", "-q", "HEAD", "--", rel], cwd=directory) + _git_ok(["checkout", "-q", "HEAD", "--", rel], cwd=directory) # ============================================================================= diff --git a/src/lightcone/engine/materialize.py b/src/lightcone/engine/materialize.py index e869f834..9d441b44 100644 --- a/src/lightcone/engine/materialize.py +++ b/src/lightcone/engine/materialize.py @@ -521,7 +521,6 @@ def materialize( # only maintainer. _converge_crate(root, report, full, dsid) return report - _require_manifests_in_git(root, graph) _fetch_inputs(root, graph, report) # Before the runtime resolves, because the refusal must not cost an # image build: a containerized graph can span an allocation only if @@ -827,12 +826,10 @@ def _converge_crate(root: Path, report: MaterializeReport, full: Graph, dsid: st "publication view is maintained — declare one to enable it" ) return - # A set difference, never a reconstruction: an external sub-analysis is - # filed under its own universe while its graph key carries the parent's, - # so no rule turns one of these paths back into a key. Enumerated from - # git rather than from the homes the graph knows, so a sub-analysis just - # deleted from the spec is still named — the edit that orphans a tree is - # the same edit that would drop it from any graph-derived root set. + # A set difference rather than a walk driven by the spec: what is on + # disk is exactly what the *previous* spec declared, so the edit that + # orphans a manifest is the same edit that drops it from the graph. + # Enumerated from git, which answers for the tree as committed. expected = {task.manifest_path for task in full.tasks.values()} for manifest in sorted(dataset.tracked_manifests(root)): if manifest not in expected and assets.read(manifest) is not None: @@ -1045,39 +1042,6 @@ def _graph( return graph, env_version, full -def _require_manifests_in_git(root: Path, graph: Graph) -> None: - """Refuse before the graph runs if a manifest would reach the annex. - - ``dataset.save`` opts dot-named paths *out* of git-annex's stock - routing, so the sidecar's leading dot decides nothing on its own and - ``.gitattributes`` decides everything. Get that wrong and the run is - green here and broken everywhere else: a clone with no content fetched - reads a pointer file where the manifest should be, every output - classifies as never materialized, and nothing said so. - - One process for the whole graph — ``check-attr`` takes many paths — - and before anything executes, because the repo refuses before it - spends. - - Args: - root: The project root. - graph: The run's tasks. - - Raises: - ProjectError: If any manifest would be routed to the annex. - """ - if not (paths := sorted({task.manifest_path for task in graph.tasks.values()})): - return - routed = dataset.largefiles(root, paths) - if annexed := sorted(path for path, answer in routed.items() if answer != "nothing"): - raise ProjectError( - f"{plan.declared_path(root, Path(annexed[0]))} would be stored in the annex, " - "but a manifest has to stay a plain git blob — it is what a clone reads " - "before fetching any content. Check .gitattributes still carries " - "`results/**/.*.manifest.json annex.largefiles=nothing`, last." - ) - - def _owned(root: Path, task: Task) -> list[str]: """Every path one output owns, as git pathspecs. diff --git a/src/lightcone/engine/plan.py b/src/lightcone/engine/plan.py index 14b7154d..46067016 100644 --- a/src/lightcone/engine/plan.py +++ b/src/lightcone/engine/plan.py @@ -12,13 +12,9 @@ recipe grammar — so scoping, ``from:`` references and sub-analysis nesting are read here rather than re-derived. -Where an output lands follows the spec's own shape: every analysis node -has an *analysis root* — the directory holding its ``astra.yaml`` — and its -materialize to ``/results///.``. -An external sub-analysis (``path:``) is a self-similar analysis with its -own analysis root and, where it names one, its own universe; an inline one -shares its parent's and disambiguates with a scope directory. Graph keys stay -the qualified id, so only the path nests, never the addressing. +Where an output lands is composed, never chosen: an output is the single +file ``results//.``, and the format comes from the +spec — so the whole contents of ``results/`` are a pure function of it. Nothing here schedules anything. Ordering is Dask's job at execution time and a topological walk's job in ``--check``; this module only says which diff --git a/tests/test_dataset.py b/tests/test_dataset.py index 308b3b97..8d50a93c 100644 --- a/tests/test_dataset.py +++ b/tests/test_dataset.py @@ -289,6 +289,21 @@ def test_save_stages_what_a_rebuild_deleted(repo: Path) -> None: # ---- leaving the tree as clean as it was found ----------------------------- +def test_tracked_manifests_are_only_the_ones_under_results(repo: Path) -> None: + """`results/` is the only place lc writes one; elsewhere the name is + the user's to use, and handing such a file to `assets.read` would ask + it to parse something that was never a manifest.""" + ours = repo / "results" / "baseline" / ".fit.manifest.json" + ours.parent.mkdir(parents=True) + ours.write_text("{}\n") + theirs = repo / "data" / ".vendor.manifest.json" + theirs.parent.mkdir(parents=True, exist_ok=True) + theirs.write_text("[1, 2, 3]\n") + dataset.save(repo, [repo / "results", repo / "data"], "both") + + assert dataset.tracked_manifests(repo) == [ours] + + def test_restore_undoes_a_half_written_rebuild(repo: Path) -> None: """The invariant that makes the dirty-tree refusal survivable: a recipe that truncates a committed output and then fails must not leave the @@ -322,6 +337,35 @@ def test_restore_of_a_never_committed_output_is_not_an_error(repo: Path) -> None assert not (output / "fit.csv").exists() +def test_restore_of_a_staged_but_unborn_output_does_not_raise(repo: Path) -> None: + """The state an interrupted `save` leaves: staged, never committed. + `ls-files --with-tree=HEAD` calls such a path known while + `checkout HEAD --` then exits nonzero on it — so a guard built on the + former makes the restore in materialize's `finally` raise over + whatever interrupted the run. Unstaging is not restore's job (git's + own `clean` leaves an index entry alone); not raising is.""" + output = repo / "results" / "baseline" / "best_fit.csv" + output.parent.mkdir(parents=True) + output.write_text("staged, never committed\n") + dataset._git(["add", "--", "results"], cwd=repo) + + dataset.restore(repo, [":(glob)results/baseline/best_fit.*"]) + + +def test_restore_takes_a_glob_pathspec(repo: Path) -> None: + """An output owns whatever its id names, so the pathspec is a glob — + which is what rules out `cat-file -e` and `ls-tree` as guards.""" + output = repo / "results" / "baseline" / "best_fit.csv" + output.parent.mkdir(parents=True) + output.write_text("committed\n") + dataset.save(repo, [output], "the first version") + output.unlink() + + dataset.restore(repo, [":(glob)results/baseline/best_fit.*"]) + + assert output.read_text() == "committed\n" + + def test_restore_is_scoped_to_the_paths_it_is_given(repo: Path) -> None: """Never `git checkout HEAD -- .`: a failed task must not discard edits made elsewhere while the graph was running.""" diff --git a/tests/test_materialize.py b/tests/test_materialize.py index fb33982b..6a99c36a 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -997,29 +997,6 @@ def test_the_report_is_json_ready(root: Path, inline: None) -> None: # ---- the foreign-write fact ------------------------------------------------ -def test_a_manifest_the_attributes_would_annex_refuses_before_the_graph_runs( - root: Path, inline: None -) -> None: - """The silent failure this guard exists for: `save` opts dot-paths out - of git-annex's stock routing, so the leading dot decides nothing and - `.gitattributes` decides everything. Annexed, a clone with no content - reads a pointer where the manifest should be and reports every output - as never materialized — with nothing said. Refused up front, before - any recipe has spent anything.""" - attributes = root / ".gitattributes" - attributes.write_text( - attributes.read_text().replace( - "results/**/.*.manifest.json annex.largefiles=nothing", "" - ) - ) - dataset.save(root, [attributes], "route manifests into the annex") - - with pytest.raises(ProjectError, match="plain git blob"): - engine.materialize(root, []) - - assert not (root / "results" / "baseline").exists(), "it refused before running anything" - - def _forge(path: Path, text: str) -> None: """Overwrite a committed result the way a hand edit would — unlinking first, because results are committed thin: an in-place truncate would