diff --git a/CLAUDE.md b/CLAUDE.md index ae5035d..89c868c 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. @@ -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,15 @@ 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 `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. **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 +895,46 @@ 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 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 +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 +1097,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 +1249,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 +1920,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 +2189,50 @@ 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. +- **`.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 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 + 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. +- **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 - **No `AGENTS.md` scaffolding** (spec §2 calls for an agent notes diff --git a/docs/api/assets.md b/docs/api/assets.md index 77dce6e..b584bd9 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(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. | ## What must stay true diff --git a/docs/api/container.md b/docs/api/container.md index 899f516..14a3256 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 af2bf0d..b5562db 100644 --- a/docs/api/crate.md +++ b/docs/api/crate.md @@ -39,6 +39,10 @@ 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. - **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 53df884..28da872 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 980eef4..93cf8e3 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 2dbe05b..a54cc75 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 af9a3bd..32081a2 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 db12a76..b290397 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 cb88423..43c28d0 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: @@ -191,13 +196,12 @@ 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) ) ``` -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 @@ -218,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) @@ -227,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 @@ -266,7 +270,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, @@ -281,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 @@ -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 39fa1d2..537ba98 100644 --- a/docs/user/glossary.md +++ b/docs/user/glossary.md @@ -46,15 +46,14 @@ 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. `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 @@ -65,8 +64,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 +89,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 +142,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 73f0b2a..e7e59f1 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 aa05abc..b605b41 100644 --- a/evals/prompt.md +++ b/evals/prompt.md @@ -30,11 +30,11 @@ 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 - `.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. + - Outputs land in `results/baseline/.`, each with a + `..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. @@ -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 diff --git a/evals/tasks/snae/astra.yaml b/evals/tasks/snae/astra.yaml index 41b9d64..cca7363 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 diff --git a/pyproject.toml b/pyproject.toml index ac861a0..d53fafc 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 40bdfd3..222f4a6 100644 --- a/src/lightcone/engine/assets.py +++ b/src/lightcone/engine/assets.py @@ -1,8 +1,10 @@ """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 +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. @@ -30,13 +32,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,25 +49,28 @@ 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(root: Path, universe_id: str, output_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 - 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: root: The project root. - universe_id: The universe the output was made under. - output_id: The output's id, qualified for a sub-analysis. + universe_id: The universe the output is made under. + output_id: The output's id. + 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 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. """ for label, value in (("universe", universe_id), ("output", output_id)): if not value or "/" in value or "\\" in value or value in {".", ".."}: @@ -77,7 +78,37 @@ def output_dir(root: Path, universe_id: str, output_id: str) -> Path: 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 "." 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 `{output_id}` declares the format {fmt!r}, which cannot name a " + f"file extension, so the output has nowhere to be written." + ) + 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 + 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 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. + + Returns: + The sidecar's path. + """ + return output.parent / f".{output.name.partition('.')[0]}{MANIFEST_SUFFIX}" # ============================================================================= @@ -90,16 +121,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 +150,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 +163,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 +344,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,31 +363,36 @@ 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: 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 -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 0a7b854..29eaf5c 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 78c10c9..e24ee21 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], ) @@ -347,35 +342,32 @@ 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} + 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 8b3d91e..be63832 100644 --- a/src/lightcone/engine/dataset.py +++ b/src/lightcone/engine/dataset.py @@ -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,32 @@ def annex_keys(directory: Path) -> dict[str, str]: return keys -def save(directory: Path, paths: Iterable[Path], message: str) -> bool: +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, 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. + + 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)results/**/*{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 +395,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,23 +415,31 @@ 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 — through ``_git_ok``, because a first + materialization has nothing to go back to and ``checkout`` exits + nonzero on a pathspec it cannot match. + + 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. - 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): - _git(["checkout", "-q", "HEAD", "--", rel], cwd=directory) + _git_ok(["checkout", "-q", "HEAD", "--", rel], cwd=directory) # ============================================================================= @@ -433,13 +479,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 6856744..55cd39d 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 331d7b3..9d441b4 100644 --- a/src/lightcone/engine/materialize.py +++ b/src/lightcone/engine/materialize.py @@ -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 @@ -564,9 +572,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 +603,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 +636,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 +648,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 +826,16 @@ 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 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: 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 +907,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 +1042,25 @@ def _graph( return graph, env_version, full +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.output_id}.*", + f":(glob){parent}/.{task.output_id}{assets.MANIFEST_SUFFIX}*", + ] + + def _name(key: Key) -> str: return f"{key[0]}/{key[1]}" diff --git a/src/lightcone/engine/plan.py b/src/lightcone/engine/plan.py index 154af1f..4606701 100644 --- a/src/lightcone/engine/plan.py +++ b/src/lightcone/engine/plan.py @@ -10,9 +10,11 @@ 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 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 @@ -38,17 +40,23 @@ class Task: universe_id: str output_id: str - output_dir: 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 +195,7 @@ def build(root: Path) -> Graph: declared_in[universe_id] = path for task in _tasks(root, universe_id, spec, universe): tasks[task.key] = task + return Graph(tasks=tasks) @@ -270,22 +279,38 @@ 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} + # 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 + # 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): + 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: + return assets.output_path( + root, + universe_id, + str(out.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 +327,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 +336,13 @@ def _tasks( Task( universe_id=universe_id, output_id=out.id, - output_dir=output_dir, + 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 c19325e..d317cda 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 feaf59d..0c91ba6 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 a5cb916..ffd1856 100644 --- a/src/lightcone/engine/templates/files/gitattributes.tmpl +++ b/src/lightcone/engine/templates/files/gitattributes.tmpl @@ -11,5 +11,5 @@ * filter=annex results/** annex.largefiles=anything data/** annex.largefiles=anything -**/.lightcone-manifest.json annex.largefiles=nothing .datalad/environments/*/image 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 736ed80..c0925bc 100644 --- a/src/lightcone/engine/templates/files/results-README.md.tmpl +++ b/src/lightcone/engine/templates/files/results-README.md.tmpl @@ -1,5 +1,9 @@ # 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. diff --git a/src/lightcone/engine/worker.py b/src/lightcone/engine/worker.py index 5d01922..ffffce7 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.output_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 e2ef5fe..cde3d7d 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,23 +115,52 @@ 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_dir(tmp_path, "baseline", bad) + assets.output_path(tmp_path, "baseline", bad, "csv") -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" - ) +@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_path(tmp_path, "baseline", "best_fit", bad) + + +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") + 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: @@ -237,35 +272,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 5010aed..ceb11a0 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 99a4872..3f80227 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,17 @@ 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, "make first", {"catalog": root / "data" / "catalog.csv"}, {}, @@ -93,9 +93,9 @@ def _graph(root: Path, universes: tuple[str, ...] = ("baseline",)) -> Graph: tasks[(universe_id, "second")] = Task( universe_id, "second", - root / "results" / universe_id / "second", + root / "results" / universe_id / "second.txt", "make second", - {"first": first_dir}, + {"first": first}, {"first": (universe_id, "first")}, {"method": "alpha"}, "sha256:def", @@ -103,7 +103,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 +133,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 +229,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 +322,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 +402,7 @@ def test_an_out_of_tree_input_publishes_no_checksum( task = Task( "baseline", "first", - project / "results/baseline/first", + project / "results/baseline/first.txt", "make first", {"catalog": catalog}, {}, diff --git a/tests/test_crate_smoke.py b/tests/test_crate_smoke.py index c7bf084..f1c71d0 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 9e4dc1c..8d50a93 100644 --- a/tests/test_dataset.py +++ b/tests/test_dataset.py @@ -71,29 +71,29 @@ 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_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 +109,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) @@ -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_identity.py b/tests/test_identity.py index 594703c..707a23c 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 5ff96e8..6a99c36 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() @@ -1010,14 +1019,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 +1039,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 +1056,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 +1074,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 +1119,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 +1181,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 +1204,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 0eb58be..48a994b 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: @@ -257,95 +260,17 @@ 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 - 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 - 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 / "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" +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", "") ) - 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_output_is_addressed_flat_and_qualified(tmp_path: Path) -> None: - """One addressing scheme and one place to look, whatever shape the - spec has.""" - 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" - - - - - - - - - - - - - - + with pytest.raises(ProjectError, match="format") as raised: + _build(root) + assert "fit" in str(raised.value) and "report" in str(raised.value) # ---- rendering a recipe ---------------------------------------------------- diff --git a/tests/test_sandbox_denial.py b/tests/test_sandbox_denial.py index 02ef15b..700ebab 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 93c86c6..179bf6e 100644 --- a/tests/test_sandbox_enforcement.py +++ b/tests/test_sandbox_enforcement.py @@ -382,28 +382,27 @@ 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_universes_results( 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* 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 / "results" / "robust" 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: - crossed = shell( - backend, policy, f"printf forged > {sibling / 'value.txt'}", cwd=project - ) - owned = shell(backend, policy, f"printf mine > {own / 'value.txt'}", cwd=project) + other.mkdir(parents=True) + (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 / 'first.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 / "first.txt").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 e05a492..0cfc260 100644 --- a/tests/test_sandbox_policy.py +++ b/tests/test_sandbox_policy.py @@ -59,37 +59,38 @@ 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.""" 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" + 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(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 f2f67ff..7fb782e 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -123,7 +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 "**/.lightcone-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: @@ -162,6 +162,21 @@ def test_an_opt_out_the_defaults_would_land_below_is_named() -> None: assert misplaced == "* 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 +212,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 d75ab01..1884170 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 ad715c8..8b392c2 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)