diff --git a/docs/mcp-server.md b/docs/mcp-server.md index cf5d8a4..781371f 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -189,18 +189,26 @@ a version of `alias` is minted. Recipes then read `tracked_expr_from_alias(alias)`; the original path is provenance and is never read again. - **Params:** `outside_path` (required) — any path, `data/` is not special; - `alias` (required) — the source alias, which may not name a catalog entry; + `alias` (required) — the source alias, which may not name a catalog alias; `pinned_version` — the version you claim the file is, checked rather than assumed; `prompt` — optional intent / cell markdown; `schema` and `reader_options` — CSV column types and `polars.scan_csv` options, recorded on the entry and never re-derived (ADR-011 D12). - **Returns:** `hash`, `alias`, `version`, `created`, `row_count`, `schema`, `path`, `digest`, `url`, plus `recalc` when advancing the alias cascaded. - `{error, error_id}` for a missing path, an alias collision, or any row of the - ADR-011 D3 table that is an error. + `{error, error_id}` for a missing path, an alias collision, bytes another + alias already holds (the error names that alias and version), or any row of + the ADR-011 D3 table that is an error. - Re-running it on **unchanged** bytes is a no-op that returns the current - version, so it is safe in a script. Re-running it on **changed** bytes mints - the next version and cascades like a revise. + version, so it is safe in a script. If that version's snapshot is gone, the + no-op heals it from the clone and checks it against the recorded digest; it + never rewrites the version. Re-running it on **changed** bytes mints the next + version and cascades like a revise. +- **One set of bytes is one version under one alias** (ADR-011 D1). To give a + source a second name, `catalog_create` an entry whose recipe is + `tracked_expr_from_alias("")`; it follows the source when it is + re-imported. A CSV read with other reader options is a different entry and + may be imported under its own alias (ADR-011 D12). ### `catalog_create(name, code, prompt="") -> dict` Execute and persist as a **named** entry (alias) and append a notebook cell. @@ -239,7 +247,9 @@ cell. Used after a `catalog_run` when a scratch entry earns a permanent name. - **Params:** `hash` (required) — must name an on-disk entry; `name` (required) — must be free. - **Returns:** `{hash, alias, version, url}`. `{error}` if the hash has no entry - or the name is taken. + or the name is taken, and if the entry is a source version: a source entry + belongs to its source alias, and the error names the `catalog_create` over + `tracked_expr_from_alias` that gives the source a second name (ADR-011 D1). ### `catalog_rename(old_name, new_name) -> dict` Rename an alias, preserving its full version history and notebook position. diff --git a/plans/ADR-011-sources-are-aliases.md b/plans/ADR-011-sources-are-aliases.md index 54ed5be..c83c5ad 100644 --- a/plans/ADR-011-sources-are-aliases.md +++ b/plans/ADR-011-sources-are-aliases.md @@ -5,7 +5,12 @@ decisions of the same day folded into D1: the snapshot is written by pyarrow in the pinned layout, and it is cache that `ensure_materialized` re-creates from the clone. Stage 2 — D6, D8 and the rewrite of every call site that - authored a raw file read — is PR #218, stacked on it. Amends + authored a raw file read — is PR #218, stacked on it. The fixes from reviewing + both are PR #219, stacked on #218: one set of bytes is one version under one + alias, an alias's kind matches its entries' kind, a version is named by the + alias that holds it now, and a re-import that finds a version already there + rewrites nothing but a missing snapshot, verified like any heal (D1, D3, D11, + D12 and the PR #219 implementation notes). Amends `plans/ADR-002-source-identity-content-hash.md` (its modes, its `sources` map and its reconstruction caveat go; the clone store stays), `plans/ADR-005-intelligent-csv-import.md` (its reader runs at import, not in @@ -45,8 +50,8 @@ `src/tallyman_mcp/server.py` (`catalog_load_parquet`), `docs/system-contract.md`. - **Related ADRs:** `plans/ADR-002-source-identity-content-hash.md` (the `.cas` - clone store this builds on; its `off` and `salt` modes go, and its `sources` - map narrows to a retention record), + clone store this builds on; its `off` and `salt` modes go, and so does its + `sources` map, D6), `plans/ADR-005-intelligent-csv-import.md` (reader options and the suggestion contract, which move to import time), `plans/ADR-008-row-order-of-reads.md` (its D2 and D7 — every source enters @@ -64,8 +69,8 @@ - **Source entry:** one version of a source alias, stored as an ordinary entry with a content hash, a recipe and a manifest. - **The arena:** the files tallyman owns — the clone store under `data/.cas/` - and the ordered copies under `compute_cache/`. A file outside the arena is - never read by a build. + and the snapshots under `compute_cache/result_cache/`. A file outside the arena + is never read by a build. - **Import:** the explicit act of copying a file into the arena and pointing a source alias at the resulting entry. - **Provenance path:** the outside path a version was imported from, recorded @@ -137,16 +142,30 @@ Everything below follows from those two sentences. ### D1. A raw input is an alias whose versions are entries Importing a file mints an entry and points a source alias at it. The entry is -ordinary: a content hash, a recipe, a manifest, an ordered copy carrying +ordinary: a content hash, a recipe, a manifest, a snapshot carrying `__row_order`. The alias lives in the same `aliases.jsonl` as catalog aliases, with the same head-plus-history shape, so `orders-v2` resolves through the existing `VERSION_REF_RE` and `resolve_version_ref` with no new syntax. A source alias is a distinct kind. `catalog_revise` is refused on one (there is -no recipe to revise), as is promoting a diff onto it. Rename and unalias behave -as they do for catalog aliases. The kind is recorded in the alias store so +no recipe to revise), as is promoting a diff onto it, on every surface that +offers those (the MCP tools and the companion's code-edit and promote-diff +routes, which refuse before building anything). Rename and unalias behave as +they do for catalog aliases. The kind is recorded in the alias store so `catalog_list` can separate inputs from computations. +**An alias's kind matches the kind of the entries it points at.** A catalog +alias never points at a source entry (one whose manifest records `provenance`), +and a source alias never points at a computed one. `set_alias` enforces it, so +it holds on every route that names an entry: `catalog_alias`, a revise, a +promoted diff, a recalc, an import. + +A source version's `provenance` keeps the name it was **imported as**. A rename +moves the alias's history to a new name and an unalias drops it, and neither +rewrites that record, so anything that names the version as it is now (the pin +reason, the heal error and the import it advises, the rebuild's replay) asks +the alias store which source alias holds it. + Consequence: a child of a source records `{hash: , ref: "orders", follow: True}` — the same edge shape as any catalog parent. The DAG gains its roots. Source versions become visible in the UI, diffable against @@ -177,8 +196,24 @@ missing clone and the re-import that repairs it. A source version is therefore the one kind of entry whose "cache or data" answer depends on a second file being present, which is the price of not storing the bytes three times. -Two imports of identical bytes under different aliases produce the same entry -hash and share one file, with both aliases pointing at it. +**One set of bytes, read one way, is one source version under one alias.** +Importing bytes that another alias of the project already holds, at any version, +is an error naming that alias and version. The likely way to get there is not +knowing the bytes are already in the project, and letting two aliases share the +entry meant one entry directory carrying one alias's name in its provenance and +recipe while another alias pointed at it. (This document first said two such +imports share one entry. PR #219 reversed that after the #217/#218 review found +the second import rewrote the first alias's manifest, and a failure part-way +deleted its entry.) + +A second name for a source is a catalog entry whose recipe reads it: +`catalog_create("orders_eu", "... expr = tracked_expr_from_alias('orders')")`. +Nothing has to be added to force the hashes apart, because a source entry's hash +is an md5 of its bytes and a recipe's is xorq's hash of the expression, and the +new entry follows `orders`, so a re-import advances it. The rule keys on the entry +hash, so a CSV read two ways is still two imports under two aliases (D12), and +it is per project: two projects importing one file each hold their own entry, +snapshot and clone under the same hash. ### D2. Files enter only by an explicit import @@ -210,6 +245,17 @@ The official, and only, way to advance a source alias: | `pinned_version` beyond head+1 | error: versions cannot be skipped | | `pinned_version=N` < head, digest matches | return vN; the head does not move | | bytes match a version older than the head | error: see D11 | +| bytes are a version of another alias | error: one set of bytes, one alias (D1) | + +"Bytes" in this table means the bytes read with the given reader options, which +is what the entry hash covers (D12). A row that ends on a version that already +exists (the no-ops, and a new alias over an entry no alias holds any more) never +rewrites that entry: its recipe, build and manifest are the record of the import +that minted it. If its snapshot is gone, the import heals it the way +`ensure_materialized` does, from the clone (restored from the caller's bytes +first when it is gone too) and checked against the recorded `result_digest`, so +a reader that now parses the bytes differently is recorded as an unfaithful heal +rather than becoming the version's rows. **Considered and not shipped: `import_once_and_depend(outside_path, alias)`.** An idempotent form for a standalone script that wants the same data wherever it @@ -297,8 +343,11 @@ existing alias is an error, and under this ADR it mints the next version. ### D11. Version history is append-only and monotonic An import whose bytes match a version older than the head is an error, naming -`reset_to` as the way back. Two version numbers never denote the same bytes, and -a version number never moves backwards while history is intact. +`reset_to` as the way back, and `pinned_expr_from_alias("-v")` as the +way to read the old version without moving the head. Importing the bytes under +a different alias is not offered, because D1 refuses it. Two version numbers +never denote the same bytes read the same way, and a version number never moves +backwards while history is intact. The case is unlikely — it takes a source file that was edited and then restored exactly — and the error keeps the alternative (a v3 whose content equals v1's, @@ -308,7 +357,15 @@ or a head that jumps backwards) out of the model. A CSV's delimiter, schema overrides and inference settings are named once, in the import call, and recorded on the source entry. Two recipes cannot read one -file two ways; import it twice under two aliases. +file two ways; import it twice under two aliases. That is the one case in which +the same bytes are under two aliases, and it is two entries, because the entry +hash covers the reader options. + +Because the hash covers them, an import that leaves them out names another +entry. Every message that advises an import therefore prints the recorded +options with it (`source_import.import_call`, in the MCP tool's `schema=` and +`reader_options=` shapes), and a pinned import refused for naming another entry +says the reader options may be what differs. This removes the hash fork found in the #215 review, where a function-valued reader option's `repr` carries a memory address and re-derives a new copy key on @@ -319,14 +376,27 @@ every build. Options are evaluated once, at import, and stored. - A child of a source alias is stale after an import advances it, and clear after recalc. The permanently-stale case of Problem cannot be constructed, because D5 refuses the recipe. -- `import_once_and_depend` returns v1 after the alias has advanced to v3, and - does not read the outside file. -- Every row of D3's `update_and_depend` table, including the three errors. +- Every row of D3's `update_and_depend` table, including the four errors. (The + `import_once_and_depend` test this list first named went with the function, + which D3 records as considered and not shipped.) +- The same bytes under a second alias are refused, naming the alias and version + that hold them, and the first alias's entry is left byte for byte as it was. +- Two projects importing one file each hold their own entry, snapshot and + clone; a heal, a clone sweep or a re-import in one never reaches the other. +- `catalog_alias` of a source entry is refused, as is a source alias pointed at + a computed entry. +- After a rename, the pin reason and the heal error name the alias the version + is under now, and running the advised import repairs that version. +- A re-import of a version whose snapshot is gone rebuilds nothing, rewrites + nothing of the entry, and records an unfaithful heal when the rows differ. +- The advised import for a CSV carries its schema and reader options and, run + as written, repairs the version. - An authored recipe calling `read_project_file` fails to build, and the error names the import call. - An authored recipe passing a bare content hash to `pinned_expr_from_alias` fails, and the error names `-v`. -- A source alias refuses `catalog_revise` and refuses a promoted diff. +- A source alias refuses `catalog_revise` and refuses a promoted diff, through + the MCP tools and through the companion's routes alike. - `reset_to` rewinds a source alias to v1 and a reset forward restores v2 from the bullpen. - Editing the outside file after import changes nothing: the same build, the @@ -374,8 +444,7 @@ D5, D9, D10, D12.** What the code does that this document did not say: - **A source entry's content hash is `md5("source||")`, truncated to xorq's 12 hex.** It cannot come from `build_expr`, because the generated recipe reads the snapshot and the snapshot is named by the hash. The - bytes and the reader options are the whole identity, which is what makes two - imports of one file under two aliases mint one entry (`source_import.py`). + bytes and the reader options are the whole identity (`source_import.py`). - **The generated recipe still calls `read_project_file`**, and a contextvar (`_SOURCE_ENTRY`) is what makes that call legal and resolves it to the entry's own snapshot. `result_cache._recipe_expr` sets the same contextvar when it @@ -426,6 +495,8 @@ D5, D9, D10, D12.** What the code does that this document did not say: for one, so `catalog_state._live_source_digests` had to learn to keep a clone alive from `provenance.digest` as well — otherwise a reset retires the only copy of the imported bytes. + Its `alias` and `version` are the name the version was imported as (D1): a + rename or unalias does not rewrite them. - **`entry_staleness` skips axis 2 for a source entry** rather than reporting it unknown. A small piece of D6, forced: every source entry would otherwise carry a permanent "source axis unknown". @@ -489,6 +560,35 @@ code does that this document did not say: failure mode that deleting `manifest.sources` could otherwise have caused silently. +**Review fixes (PR #219, 2026-09-24).** Found reviewing #217 and #218; each +landed test first. What the code does that the decisions above did not say: + +- **Duplicate bytes were a shared entry.** Stage 1 let a second alias import + bytes another alias held, on the grounds that the hash made it one entry. The + second import re-ran `_mint` over that entry, rewriting the first alias's + provenance and recipe, and a failure part-way deleted the entry directory the + first alias pointed at. D1 now refuses the import (`_refuse_bytes_held_elsewhere`). +- **The kind rule lives in `set_alias`** (D1), not in each tool, after + `catalog_alias` put a catalog name on a source entry and opened it to + `catalog_revise`. The companion's `PUT /api/code` and promote-diff routes had + no refusal at all: they built the entry, failed to point the source alias at + it and answered 500 with the entry left under no alias. They now refuse with + the tools' text (`aliases.source_alias_refusal`) before building, answering 409. +- **A version is named by the alias that holds it now** + (`source_import.current_source_version`). Naming it by `provenance.alias` + made the heal advice mint a second alias under the old name after a rename, + and made `scripts/rebuild_native_catalog.py` replay a renamed source under the + old name: children reading the new name failed to rebuild, and the order of + the source's versions was lost. +- **A repair import heals** (D3). It used to go through `_mint`, which rebuilt + the entry, rewrote the manifest and recorded whatever digest it wrote: a + probe with a reader that drops a row went from 10 rows to 9 under the same + hash with nothing recorded. An entry directory without a manifest, which a + crash part-way through an import leaves, is not an entry, and a re-import + writes it again. +- **Advised imports carry the reader options** (D12). Without them the heal + error's advice for a CSV named another entry and was refused. + ## Open questions 1. ~~**Does a source version keep its raw bytes as well as its snapshot?**~~ diff --git a/scripts/rebuild_native_catalog.py b/scripts/rebuild_native_catalog.py index 66b8235..055417e 100644 --- a/scripts/rebuild_native_catalog.py +++ b/scripts/rebuild_native_catalog.py @@ -23,7 +23,9 @@ file) is not re-exec'd at all — its generated recipe reads the very snapshot the rebuild just wiped, and a raw file read is a build error anywhere else. It is replayed through ``update_and_depend``, from the provenance path when that still - holds the imported bytes and from the clone under ``data/.cas`` when it does not. + holds the imported bytes and from the clone under ``data/.cas`` when it does not, + under the source alias that holds it now (a rename leaves ``provenance.alias`` + naming the alias it was imported as) and pinned to its version there. Its hash is a function of the bytes and the reader options, so it is preserved; * recipes that chain by ALIAS (``tracked_expr_from_alias("citibike")``) survive untouched — the alias is re-pointed at the rebuilt parent before the child is built; @@ -198,6 +200,7 @@ def parse_deps( history: dict[str, list[str]], known: set[str], provenance: dict | None = None, + kinds: dict[str, str] | None = None, ) -> set[str]: """The old hashes this recipe depends on, as the build resolved them. @@ -208,13 +211,17 @@ def parse_deps( this very entry, a false self-cycle); otherwise it is the alias's current hash. A SOURCE entry reads nothing, but its versions are minted in order — ``update_and_depend`` - refuses to skip one — so it depends on the previous version of its own alias. + refuses to skip one — so it depends on the previous version of the source alias that holds it + (``source_version_in``). That is not always ``provenance["alias"]``, the name it was imported as: + a rename moves the history to a new name and leaves the provenance behind. """ + from tallyman_xorq.source_import import source_version_in # noqa: PLC0415 + out: set[str] = set() if provenance: - hist = history.get(provenance["alias"], []) - if self_hash in hist and hist.index(self_hash) > 0: - out.add(hist[hist.index(self_hash) - 1]) + held = source_version_in(history, kinds, self_hash, provenance["alias"]) + if held is not None and held[1] > 1: + out.add(history[held[0]][held[1] - 2]) for ref in _FROM_CAT_RE.findall(expr_text): if ref in aliases: hist = history.get(ref, []) @@ -235,12 +242,13 @@ def toposort( aliases: dict[str, str], history: dict[str, list[str]] | None = None, provenance: dict[str, dict] | None = None, + kinds: dict[str, str] | None = None, ) -> list[str]: """Dependency order (parents before children) over the tracked_expr_from_alias graph.""" history = history or {} provenance = provenance or {} known = set(recipes) - dmap = {h: parse_deps(t, h, aliases, history, known, provenance.get(h)) for h, t in recipes.items()} + dmap = {h: parse_deps(t, h, aliases, history, known, provenance.get(h), kinds) for h, t in recipes.items()} order: list[str] = [] placed: set[str] = set() while len(placed) < len(recipes): @@ -266,7 +274,14 @@ def sub(m: re.Match) -> str: return _HASH_RE.sub(sub, expr_text) -def replay_import(project: str, provenance: dict, *, prompt: str | None = None, log=print) -> str: +def replay_import( + project: str, + provenance: dict, + *, + into: tuple[str, int] | None = None, + prompt: str | None = None, + log=print, +) -> str: """Re-import a source version and return its content hash (ADR-011). A source entry cannot be re-exec'd: its generated recipe reads its own snapshot, which the rebuild @@ -279,12 +294,26 @@ def replay_import(project: str, provenance: dict, *, prompt: str | None = None, ``data/.cas``, which survives the rebuild because it lives under ``data/`` and only the catalog bookkeeping is wiped. Taking the clone changes the provenance path the rebuilt entry records (it then names the clone), which is logged; it changes nothing else. + + *into* is ``(alias, version)``: the source alias that holds the version in the old catalog and + its place in that alias's history (``source_version_in``). The import goes under that alias, + which after a rename is not ``provenance["alias"]`` (the name it was imported as), and is pinned + to that version, so a replay out of order is an error instead of one version's bytes minted as + another's number. None when no source alias holds it: it goes under the name it was imported + as, unpinned, and the final alias write keeps that name only if the old catalog has it. """ from tallyman_core.paths import data_dir # noqa: PLC0415 from tallyman_xorq import source_identity as si # noqa: PLC0415 from tallyman_xorq.ordered_copy import _spec_from_json # noqa: PLC0415 from tallyman_xorq.source_import import update_and_depend # noqa: PLC0415 + imported = f"{provenance['alias']}-v{provenance['version']}" + alias, version = into if into is not None else (provenance["alias"], None) + if into is None: + name = f"the version imported as {imported}, which no source alias holds" + else: + name = f"{alias}-v{version}" + ("" if f"{alias}-v{version}" == imported else f" (imported as {imported})") + digest, suffix = provenance["digest"], provenance.get("suffix", "") original = Path(provenance["path"]) clone = data_dir(project) / ".cas" / f"{digest}{suffix}" @@ -292,11 +321,11 @@ def replay_import(project: str, provenance: dict, *, prompt: str | None = None, src = original elif clone.is_file(): src = clone - log(f" {provenance['alias']}-v{provenance['version']}: {original} is gone; re-importing from {clone.name}") + log(f" {name}: {original} no longer holds the imported bytes; re-importing from {clone.name}") else: raise RuntimeError( - f"cannot rebuild {provenance['alias']}-v{provenance['version']}: {original} no longer has the " - f"imported bytes and their clone {clone} is missing, so there is nothing to import" + f"cannot rebuild {name}: {original} no longer has the imported bytes and their clone {clone} is " + "missing, so there is nothing to import" ) reader = provenance["reader"] @@ -304,7 +333,8 @@ def replay_import(project: str, provenance: dict, *, prompt: str | None = None, options = dict(reader.get("scan_kwargs") or {}) if reader["kind"] == "csv" else {} out = update_and_depend( src, - provenance["alias"], + alias, + version, project=project, prompt=prompt, schema=schema, @@ -330,11 +360,12 @@ def rebuild_project(project: str, *, dry_run: bool = False, log=print) -> dict[s from tallyman_core.post_processing import write_post_processing # noqa: PLC0415 from tallyman_core.summary_stats import write_stat # noqa: PLC0415 from tallyman_xorq import build_and_persist # noqa: PLC0415 + from tallyman_xorq.source_import import source_version_in # noqa: PLC0415 oc = read_old_catalog(project) if not oc.recipes: raise RuntimeError(f"no rebuildable entries (expr.py + manifest.json) found in project {project!r}") - order = toposort(oc.recipes, oc.aliases, oc.history, oc.provenance) + order = toposort(oc.recipes, oc.aliases, oc.history, oc.provenance, oc.kinds) log( f"project {project!r}: {len(oc.recipes)} entries ({len(oc.provenance)} imported sources), " f"{len(oc.aliases)} aliases, {len(oc.charts)} charts, " @@ -381,8 +412,11 @@ def rebuild_project(project: str, *, dry_run: bool = False, log=print) -> dict[s prompts = oc.prompts.get(old_hash, []) first = prompts[0].get("prompt") if prompts else None provenance = oc.provenance.get(old_hash) + imported_into = None # the alias an import pointed at the entry itself if provenance is not None: - new_hash = replay_import(project, provenance, prompt=first, log=log) + held = source_version_in(oc.history, oc.kinds, old_hash, provenance["alias"]) + new_hash = replay_import(project, provenance, into=held, prompt=first, log=log) + imported_into = held[0] if held is not None else provenance["alias"] else: recipe = oc.recipes[old_hash].replace("${TALLYMAN_PROJECT_ROOT}", root) recipe = rewrite_hash_refs(recipe, remap) # fix literal parent-hash refs @@ -391,10 +425,11 @@ def rebuild_project(project: str, *, dry_run: bool = False, log=print) -> dict[s tag = "" if new_hash == old_hash else f" (rehashed -> {new_hash})" log(f" built {old_hash}{tag}") for name in revises_at.get(old_hash, []): - # An import already pointed its own alias, with the source kind; re-pointing it as a - # catalog alias would be refused (aliases.AliasKindMismatch). - if provenance is None: - al.set_alias(project, name, new_hash) + # The import already pointed the alias it went under. Any other alias that holds the entry + # is pointed here, with the kind it had: a second source alias over the same bytes has to + # be on this version before its own next version replays, pinned to follow it. + if name != imported_into: + al.set_alias(project, name, new_hash, kind=oc.kinds.get(name, al.CATALOG_KIND)) if len(prompts) > 1: # build wrote only the first; carry the rest p = prompts_path(project, new_hash) p.parent.mkdir(parents=True, exist_ok=True) diff --git a/src/tallyman_companion/app.py b/src/tallyman_companion/app.py index 641bb2a..54530e7 100644 --- a/src/tallyman_companion/app.py +++ b/src/tallyman_companion/app.py @@ -1323,6 +1323,12 @@ def _resolve(v: int): column_config_overrides = compute_column_config_overrides(a_expr.schema(), b_expr.schema(), keys) target_alias = f"diff_{alias}_v{a_idx}_v{b_idx}" + # The generated name can already be a source alias: refuse before building the diff, as the MCP tool does. + from tallyman_core.aliases import source_alias_refusal # noqa: PLC0415 + + refusal = source_alias_refusal(project, target_alias, "the target of a promoted diff") + if refusal: + raise HTTPException(409, refusal) keys_repr = repr(keys) code = textwrap.dedent(f"""\ # auto-generated — diff of {alias} V{a_idx} → V{b_idx} @@ -1657,6 +1663,12 @@ async def put_code(project: str, alias: str, payload: dict): prev_hash = _get_alias(project, alias) if prev_hash is None: raise HTTPException(404, f"alias {alias!r} not found") + # A source alias's versions are imported files: refuse before building, as catalog_revise does (ADR-011 D1). + from tallyman_core.aliases import source_alias_refusal as _source_alias_refusal # noqa: PLC0415 + + refusal = _source_alias_refusal(project, alias, "revised") + if refusal: + raise HTTPException(409, refusal) code = payload.get("code", "") if not code.strip(): raise HTTPException(400, "code is empty") diff --git a/src/tallyman_core/aliases.py b/src/tallyman_core/aliases.py index 5cc7686..4cab14b 100644 --- a/src/tallyman_core/aliases.py +++ b/src/tallyman_core/aliases.py @@ -13,6 +13,12 @@ with the same head-plus-history shape, so ``orders-v2`` resolves through the one ``VERSION_REF_RE``, and a name is one kind or the other but never both. +The kind is recorded here, on the alias; whether an entry is a *source entry* (a +version of an imported file) is recorded on the entry, by its manifest carrying +``provenance``. ``set_alias`` ties the two together: **an alias's kind matches the +kind of the entries it points at**, so a catalog name never heads an import and a +source name never heads a recipe's result, whichever route set it. + All three live in a tracked ``aliases.jsonl`` in the catalog repo — one line per alias, ``{"alias", "latest", "history": [...], "kind"}``. The native store tracks the file directly, so alias state clones and versions with the catalog and @@ -32,7 +38,8 @@ import re from tallyman_core.fsutil import atomic_write_text -from tallyman_core.paths import catalog_dir, ensure_project +from tallyman_core.manifest import read_manifest +from tallyman_core.paths import catalog_dir, ensure_project, entry_dir class AliasExists(ValueError): @@ -44,7 +51,8 @@ class AliasNotFound(KeyError): class AliasKindMismatch(ValueError): - """A name already belongs to the other kind of alias (ADR-011 D1).""" + """An alias of one kind aimed at the other kind: a name that already belongs to the other kind of alias, or an + entry whose kind is not the alias's (ADR-011 D1).""" # A catalog alias names a computation; a source alias names an imported dataset. @@ -220,6 +228,63 @@ def previous_version(project: str, content_hash: str, alias: str | None = None) return hist[idx] if 0 <= idx < len(hist) else None +def source_alias_refusal(project: str, name: str, doing: str) -> str | None: + """The error for an operation that would give *name* a computed version, or None when *name* is not a source alias. + + A source alias names an imported dataset (ADR-011 D1). Its versions are files, not recipes, so there is nothing + to revise and nothing to promote a diff onto, and a catalog alias may not take its name either. Every surface + that can aim such an operation at a name (the MCP tools, the companion's routes) refuses with this text before it + builds anything, so no entry is left behind under no alias. *doing* completes "it cannot be …". + """ + if alias_kind(project, name) != SOURCE_KIND: + return None + return ( + f"{name!r} is a source alias — its versions are imported files, not recipes — so it cannot be {doing}. " + f"To give it new data, import the file again with catalog_import_source(, {name!r}). To build on it, " + f"write a recipe over tracked_expr_from_alias({name!r}) under a different name." + ) + + +def _entry_kind(project: str, content_hash: str) -> str | None: + """The kind of alias the entry *content_hash* may take, or None when there is no readable manifest to tell. + + ``SOURCE_KIND`` for a source entry (its manifest records ``provenance``: it is a version of an imported file), + ``CATALOG_KIND`` for any other entry, which a recipe computed. None when the entry directory or its manifest is + missing or unreadable, which ``set_alias`` treats as "do not check": the alias bookkeeping is used, and tested, + with hashes that name no entry. + """ + try: + manifest = read_manifest(entry_dir(project, content_hash)) + except (OSError, ValueError): # no entry, no manifest yet, or one that does not parse + return None + return SOURCE_KIND if manifest.provenance is not None else CATALOG_KIND + + +def _entry_kind_refusal(project: str, name: str, content_hash: str, entry_kind: str) -> str: + """The ``AliasKindMismatch`` message for pointing *name* at an entry of the other kind, *entry_kind*.""" + if entry_kind == SOURCE_KIND: + held = version_of_hash(project, content_hash) + if held is None: # imported, but its source alias has since been removed + return ( + f"{content_hash!r} is a source entry (a version of an imported file) that no source alias holds, so " + f"the catalog alias {name!r} cannot point at it: a source version has no recipe to revise. To name " + f"it, import its file again with catalog_import_source(, {name!r})." + ) + src, version = held + steer = f"from tallyman_xorq.io import tracked_expr_from_alias\nexpr = tracked_expr_from_alias({src!r})" + return ( + f"{content_hash!r} is {src}-v{version}, a version of the source alias {src!r} (an imported file, not a " + f"computation), so the catalog alias {name!r} cannot point at it: a source version has no recipe to " + f"revise. To give the source a second name, create a catalog entry that reads it: " + f"catalog_create({name!r}, {steer!r}). That entry follows {src!r} when it is imported again." + ) + return ( + f"{content_hash!r} is a computed entry (a recipe's result), so the source alias {name!r} cannot point at it: " + f"a source alias's versions are imported files, and it advances only by catalog_import_source(, " + f"{name!r}). To name this entry, give it a catalog alias under another name." + ) + + def set_alias( project: str, name: str, @@ -239,6 +304,15 @@ def set_alias( default, so every existing caller keeps its meaning) or ``SOURCE_KIND`` for a version of an imported dataset. A name that already belongs to the other kind raises ``AliasKindMismatch`` — the collision is refused both ways (ADR-011 D1). + + So does an entry of the other kind: a catalog alias may not point at a source + entry (one whose manifest records ``provenance``), since it would then offer + ``catalog_revise`` on a version with no recipe, and a source alias may not + point at a computed entry, since its versions are imported files. The check + is here rather than in each tool so that every route that names an entry + (``catalog_alias``, a revise, a promoted diff, a recalc, an import) + keeps the alias's kind and its entries' kind the same. A hash whose manifest + cannot be read is not checked. """ if kind not in _KINDS: raise ValueError(f"alias kind {kind!r}; expected one of {_KINDS}") @@ -255,6 +329,9 @@ def set_alias( f"{name!r} is a {kinds.get(name, CATALOG_KIND)} alias and cannot be set as a {kind} alias; " "a name is one kind or the other" ) + entry_kind = _entry_kind(project, content_hash) + if entry_kind is not None and entry_kind != kind: + raise AliasKindMismatch(_entry_kind_refusal(project, name, content_hash, entry_kind)) aliases[name] = content_hash kinds[name] = kind diff --git a/src/tallyman_core/manifest.py b/src/tallyman_core/manifest.py index fc4969d..77f21aa 100644 --- a/src/tallyman_core/manifest.py +++ b/src/tallyman_core/manifest.py @@ -36,6 +36,10 @@ class SourceProvenance(BaseModel): the imported bytes, which names their clone in ``data/.cas/`` and, with ``reader``, determines the entry's content hash. ``reader`` is the reader options the import used (``{"kind": "parquet"}``, or a CSV's schema spec and ``scan_csv`` options): fixed at import and never re-derived at build time. + + ``alias`` and ``version`` are the name the version was **imported as**, which is history too: a rename moves the + alias's history to a new name and an unalias drops it, and neither rewrites this record. Anything that names the + version as it is now asks the alias store (``source_import.current_source_version``). """ alias: str diff --git a/src/tallyman_mcp/server.py b/src/tallyman_mcp/server.py index aaba96c..c78803f 100644 --- a/src/tallyman_mcp/server.py +++ b/src/tallyman_mcp/server.py @@ -14,6 +14,7 @@ from tallyman_core import ( AliasExists, + AliasKindMismatch, AliasNotFound, CellNotFound, ChartSpecError, @@ -256,18 +257,11 @@ def _auto_recalc_after_head_advance(project: str, name: str, *, tool: str) -> di def _source_alias_refusal(project: str, name: str, doing: str) -> str | None: """The error for an operation aimed at a source alias, or None when *name* is not one (ADR-011 D1). - A source alias names an imported dataset. Its versions are files, not recipes, so there is nothing to revise and - nothing to promote a diff onto, and a catalog alias may not take its name either. + The text lives in ``tallyman_core.aliases.source_alias_refusal`` so the companion's routes refuse with it too. """ - from tallyman_core.aliases import SOURCE_KIND, alias_kind # noqa: PLC0415 + from tallyman_core.aliases import source_alias_refusal # noqa: PLC0415 - if alias_kind(project, name) != SOURCE_KIND: - return None - return ( - f"{name!r} is a source alias — its versions are imported files, not recipes — so it cannot be {doing}. " - f"To give it new data, import the file again with catalog_import_source(, {name!r}). To build on it, " - f"write a recipe over tracked_expr_from_alias({name!r}) under a different name." - ) + return source_alias_refusal(project, name, doing) @mcp.tool() @@ -583,7 +577,10 @@ def catalog_import_source( Args: outside_path: Any path to a parquet or CSV file. It does not have to live under the project's `data/` directory. - alias: The source alias. Must not already name a catalog entry. + alias: The source alias. Must not already name a catalog alias, and the + bytes must not already be a version of another alias — the error + names that alias. For a second name, catalog_create an entry whose + recipe is `tracked_expr_from_alias()`. pinned_version: The version you claim this file is. Given, it is checked: if the file is that version the call is a no-op, and if it is not you get an error instead of an accidental new version. Use it in a script @@ -831,6 +828,12 @@ def catalog_alias(hash: str, name: str) -> dict: Use this when an entry was created via `catalog_run` and you decide post-hoc that it deserves a name. Errors if the alias already exists. + + The entry must be a computed one. A source entry (a version of an imported + file) belongs to its source alias and cannot take a catalog name (ADR-011 + D1); to give a source a second name, `catalog_create` an entry whose recipe + is `tracked_expr_from_alias("")`, which follows the source + when it is imported again. """ project = _resolve_active_project() refusal = _source_alias_refusal(project, name, "given to another entry") @@ -840,7 +843,10 @@ def catalog_alias(hash: str, name: str) -> dict: return {"error": f"no catalog entry for hash {hash!r}"} if get_alias(project, name) is not None: return {"error": f"alias {name!r} already exists"} - info = set_alias(project, name, hash, expect_exists=False) + try: + info = set_alias(project, name, hash, expect_exists=False) + except AliasKindMismatch as exc: # a source entry: the message names its source alias-version and the steer + return {"error": str(exc)} notebook.append(project, name) _notify("alias_changed", content_hash=hash, alias=name, version=info["version"]) _notify("notebook_changed") diff --git a/src/tallyman_xorq/materialize.py b/src/tallyman_xorq/materialize.py index 727bb84..7ae3d8f 100644 --- a/src/tallyman_xorq/materialize.py +++ b/src/tallyman_xorq/materialize.py @@ -279,6 +279,19 @@ def _source_provenance(project: str, content_hash: str): return None +def _source_names(project: str, content_hash: str, provenance) -> tuple[tuple[str, int] | None, str]: + """``(held, imported)``: how a message names a source version (ADR-011 D1). + + ``held`` is ``(alias, version)`` as the alias store has it now, or None when no source alias holds the entry + (``source_import.current_source_version``). ``imported`` is the ``-v`` the version was imported as, + from ``provenance``: history, which a rename or an unalias leaves behind. A message names the version by + ``held``, and mentions ``imported`` only where the two differ. + """ + from tallyman_xorq.source_import import current_source_version + + return current_source_version(project, content_hash, provenance), f"{provenance.alias}-v{provenance.version}" + + def _heal_a_source(project: str, content_hash: str) -> bool: """Re-create a source version's snapshot from the clone of the bytes it was imported from (ADR-011 D1). @@ -289,13 +302,17 @@ def _heal_a_source(project: str, content_hash: str) -> bool: re-created snapshot. Only a clone that is gone as well makes the version unrecoverable, and then the error names the file that is - missing and the re-import that repairs it. Returns whether this entry is a source version, so the general path - can stop. + missing and the re-import that repairs it: the alias and version that hold the entry now (``_source_names``), so + the advised call re-imports into the version it names rather than minting an alias under the name the version + was imported as, and the reader options the entry recorded (``source_import.import_call``), without which a CSV + names another entry. When no source alias holds it there is no version to re-import into, and the error says what + an import of the same bytes would do instead. Returns whether this entry is a source version, so the general + path can stop. """ from tallyman_core.catalog_state import project_lock from tallyman_xorq.build import BuildError from tallyman_xorq.result_cache import _verify_self_heal - from tallyman_xorq.source_import import rewrite_source_snapshot, source_clone_path + from tallyman_xorq.source_import import import_call, rewrite_source_snapshot, source_clone_path provenance = _source_provenance(project, content_hash) if provenance is None: @@ -305,12 +322,23 @@ def _heal_a_source(project: str, content_hash: str) -> bool: return True clone = source_clone_path(project, provenance) if not clone.is_file(): + held, imported = _source_names(project, content_hash, provenance) + lost = ( + f"cannot be made again: {snapshot_path(project, content_hash)} is not on disk and neither is the " + f"clone of the imported bytes, {clone}." + ) + if held is None: + raise BuildError( + f"the data imported as {imported} (entry {content_hash}), which no source alias holds now, " + f"{lost} Importing the same bytes again, " + f"{import_call(provenance.path, None, provenance.reader)}, writes these rows again as a new " + "version of ." + ) + alias, version = held + also = "" if f"{alias}-v{version}" == imported else f", imported as {imported}" raise BuildError( - f"the data of {provenance.alias}-v{provenance.version} (entry {content_hash}) cannot be made " - f"again: {snapshot_path(project, content_hash)} is not on disk and neither is the clone of the " - f"imported bytes, {clone}. Import them again with " - f"catalog_import_source({provenance.path!r}, {provenance.alias!r}, " - f"pinned_version={provenance.version})." + f"the data of {alias}-v{version} (entry {content_hash}{also}) {lost} Import them again with " + f"{import_call(provenance.path, alias, provenance.reader, pinned_version=version)}." ) digest = rewrite_source_snapshot(project, content_hash, provenance) perf_log.debug("ensure_materialized re-imported %s from %s", content_hash, clone.name) @@ -358,8 +386,8 @@ def pinned_reason(project: str, content_hash: str) -> str | None: A snapshot is pinned when it cannot be made again faithfully: the recipe is not reproducible (two runs at create time gave different digests), a heal already produced different rows than were built, or it is a source version whose clone of the imported bytes is gone (ADR-011 D1 — with the clone it is ordinary cache, made again - from those bytes). The Cache page's delete leaves such a file alone and says why. ``compute_cache/`` as a whole - is still deletable by definition. + from those bytes). The Cache page's delete leaves such a file alone and says why, naming a source version by the + alias that holds it now (``_source_names``). ``compute_cache/`` as a whole is still deletable by definition. """ from tallyman_core import read_manifest from tallyman_core.errors import list_errors @@ -373,9 +401,14 @@ def pinned_reason(project: str, content_hash: str) -> str | None: if manifest is not None and manifest.provenance is not None: clone = source_clone_path(project, manifest.provenance) if not clone.is_file(): + held, imported = _source_names(project, content_hash, manifest.provenance) + if held is None: + name = f"the source version imported as {imported}, which no source alias holds now" + else: + current = f"{held[0]}-v{held[1]}" + name = f"the source version {current}" + ("" if current == imported else f" (imported as {imported})") return ( - f"this file is the last copy of the source version {manifest.provenance.alias}-v" - f"{manifest.provenance.version}: the clone of the bytes imported from " + f"this file is the last copy of {name}: the clone of the bytes imported from " f"{manifest.provenance.path} is gone from {clone}, so nothing can make it again and it is kept" ) if manifest is not None and manifest.reproducible is False: diff --git a/src/tallyman_xorq/source_import.py b/src/tallyman_xorq/source_import.py index a8e31c1..d85232f 100644 --- a/src/tallyman_xorq/source_import.py +++ b/src/tallyman_xorq/source_import.py @@ -18,8 +18,9 @@ records the outside path, the digest and the reader options; 4. points the source alias at it, appending a version. -The **content hash of a source entry is a function of its bytes and its reader options**, and of nothing else, so two -imports of identical bytes under two aliases mint one entry and share one file. +The **content hash of a source entry is a function of its bytes and its reader options**, and of nothing else. One +such entry belongs to one alias: importing bytes another alias already holds is an error that names that alias, and a +second name for a source is a catalog entry whose recipe reads it (``_refuse_bytes_held_elsewhere``). That snapshot is **cache** in the sense of ADR-007 D13 — a file is cache if ``ensure_materialized`` can re-create it — because the clone holds the bytes and the entry holds the reader options. A deleted one is written again from the @@ -74,7 +75,7 @@ def source_entry_hash(digest: str, reader: dict) -> str: """The content hash of the entry an import mints: its bytes and its reader options, and nothing else. Same 12-hex shape as xorq's build hash, so nothing downstream (entry directories, URLs, aliases) notices the - difference. Two imports of the same bytes under the same reader are the same entry. + difference. The same bytes under the same reader are the same entry, which is why only one alias may hold them. """ from tallyman_xorq.ordered_copy import _reader_signature @@ -82,6 +83,17 @@ def source_entry_hash(digest: str, reader: dict) -> str: return hashlib.md5(payload.encode()).hexdigest()[:HASH_LEN] # noqa: S324 — an identity, not a credential +def _readable_manifest(project: str, content_hash: str): + """The entry's manifest, or None when there is no entry or its manifest cannot be read.""" + from tallyman_core import read_manifest + from tallyman_core.paths import entry_dir + + try: + return read_manifest(entry_dir(project, content_hash)) + except (OSError, ValueError): + return None + + def is_source_entry(project: str, content_hash: str) -> bool: """Whether the entry is a version of a source alias — i.e. whether its manifest records an import.""" from tallyman_core import read_manifest @@ -93,6 +105,37 @@ def is_source_entry(project: str, content_hash: str) -> bool: return False +def source_version_in( + history: dict[str, list[str]], kinds: dict[str, str] | None, content_hash: str, imported_as: str +) -> tuple[str, int] | None: + """The source alias whose history holds *content_hash*, and its 1-based version there, or None when none does. + + ``provenance.alias`` is the name a version was **imported as**: recorded once, at import, and never updated. A + rename carries the history and the kind to a new name (``aliases.rename_alias``) and an unalias drops them, so + that name is only a hint, tried first because it is usually still right. This is ``aliases.version_of_hash`` + restricted to source aliases — an import advances nothing else, so a version held only by a catalog alias has no + alias an import could repair it under. *kinds* None means the kinds are not known, and every alias counts. + """ + from tallyman_core.aliases import SOURCE_KIND + + for name in (imported_as, *history): + hashes = history.get(name, []) + if content_hash in hashes and (kinds is None or kinds.get(name) == SOURCE_KIND): + return name, hashes.index(content_hash) + 1 + return None + + +def current_source_version(project: str, content_hash: str, provenance) -> tuple[str, int] | None: + """``(alias, version)`` of the source entry *content_hash* as the project's alias store has it now, or None. + + What a message that names a source version, or advises the import that repairs one, should say: the name in + *provenance* is where it came from, and this is where it is (``source_version_in``). + """ + from tallyman_core.aliases import load_history, load_kinds + + return source_version_in(load_history(project), load_kinds(project), content_hash, provenance.alias) + + def source_entry_context() -> tuple[str, str] | None: """The ``(project, content_hash)`` whose generated recipe is running on this stack, or ``None``. @@ -164,7 +207,8 @@ def _plan_version(alias: str, history: list[str], content_hash: str, pinned_vers """Decide what this import does: ``(mint, version)``, or raise. The full case table of ADR-011 D3, plus D11's rule that history is append-only and monotonic — two version - numbers never denote the same bytes, and a version number never moves backwards while history is intact. + numbers never denote the same bytes read the same way, and a version number never moves backwards while history + is intact. """ head = len(history) # the head's 1-based version; 0 when the alias is absent already = history.index(content_hash) + 1 if content_hash in history else None @@ -177,7 +221,8 @@ def _plan_version(alias: str, history: list[str], content_hash: str, pinned_vers f"these bytes are already {alias}-v{already}, and {alias} is at v{head}. Version history is " f"append-only: a v{head + 1} whose rows equal v{already}'s would make the version number meaningless. " f"To put {alias} back on v{already}, reset the catalog to the revision before v{already + 1} " - f"(tallyman reset / catalog_reset_to), or import these bytes under a different alias." + f"(tallyman reset / catalog_reset_to). To read v{already} in a recipe without moving the head, use " + f"pinned_expr_from_alias('{alias}-v{already}')." ) return True, head + 1 @@ -193,18 +238,42 @@ def _plan_version(alias: str, history: list[str], content_hash: str, pinned_vers if claimed == content_hash: return False, pinned_version # the file IS that version; the head does not move raise SourceImportError( - f"this file is not {alias}-v{pinned_version}: that version is entry {claimed}, and these bytes are " - f"{content_hash}. Import without pinned_version to mint the next version, or point at the file " - f"{alias}-v{pinned_version} was imported from." + f"this import is not {alias}-v{pinned_version}: that version is entry {claimed}, and this import would " + f"be entry {content_hash}. An entry is named by the bytes and the reader options together (ADR-011 D12), " + f"so either this is not the file {alias}-v{pinned_version} was imported from, or it is read with other " + f"reader options than that import used; the header of {alias}-v{pinned_version}'s recipe records them. " + f"Import without pinned_version to mint the next version." ) if already is not None: # pinned at head+1, but these bytes are already a version (D11) raise SourceImportError( f"these bytes are already {alias}-v{already}, so they cannot also be v{pinned_version}. " - f"Two version numbers never denote the same bytes." + f"Two version numbers never denote the same bytes read the same way." ) return True, pinned_version +def _refuse_bytes_held_elsewhere(project: str, alias: str, content_hash: str) -> None: + """Raise when another alias of *project* already has *content_hash* as one of its versions. + + One set of bytes, read one way, is one source version under one alias. The likely way to get here is not knowing + the bytes are already in the project, so the error says where they are and how to give them a second name. The + check keys on the entry hash, so a CSV read two ways is still two entries (D12), and it is per project. + """ + from tallyman_core.aliases import load_history + + for other, hashes in sorted(load_history(project).items()): + if other == alias or content_hash not in hashes: + continue + held = f"{other}-v{hashes.index(content_hash) + 1}" + raise SourceImportError( + f"these bytes are already {held} in project {project!r}, so they cannot also be imported as {alias!r}: " + f"one set of bytes is one source version under one alias. Read them with " + f"tracked_expr_from_alias({other!r}). To give them a second name, create a catalog entry over the " + f"source — catalog_create({alias!r}, \"from tallyman_xorq.io import tracked_expr_from_alias\\n" + f"expr = tracked_expr_from_alias({other!r})\") — which follows {other!r} when it is re-imported." + ) + + # --------------------------------------------------------------------------- # writing a source entry # --------------------------------------------------------------------------- @@ -321,6 +390,28 @@ def source_clone_path(project: str, provenance) -> Path: return data_dir(project) / ".cas" / f"{provenance.digest}{provenance.suffix}" +def import_call(path: str, alias: str | None, reader: dict, pinned_version: int | None = None) -> str: + """The ``catalog_import_source(...)`` call that imports *path* the way *reader* records it was read. + + What an error prints when the way out is an import. A CSV's entry hash covers its reader options (D12), so a call + without them names another entry and a pinned one is refused; the call therefore carries the schema and the + ``scan_csv`` options the entry recorded, in the MCP tool's own argument shapes. *alias* None prints ````, + for a version no alias holds, where the caller has to choose the name. + """ + args = [repr(path), "" if alias is None else repr(alias)] + if pinned_version is not None: + args.append(f"pinned_version={pinned_version}") + if reader.get("kind") == "csv": + spec = reader.get("schema") + if spec is not None: + cells = [[name, dtype] for name, dtype in spec["cells"]] + schema = dict(cells) if spec["form"] == "named" else cells + args.append(f"schema={schema!r}") + if reader.get("scan_kwargs"): + args.append(f"reader_options={reader['scan_kwargs']!r}") + return f"catalog_import_source({', '.join(args)})" + + def rewrite_source_snapshot(project: str, content_hash: str, provenance) -> str: """Write the snapshot of the source entry *content_hash* again, from its clone; return the digest written. @@ -338,9 +429,12 @@ def _recipe(outside_path: Path, alias: str, version: int, digest: str, reader: d The read resolves to this entry's own snapshot (``_SOURCE_ENTRY``), not to the path in the call — the path is provenance and is never read again. This is the one recipe in which a raw read is allowed (ADR-011 D2). + + The recipe is written once, so the alias in its header is the one it was imported as, and says so: after a + rename the entry is a version of another name (``current_source_version``). """ return ( - f"# {alias}-v{version}: a source version, generated by catalog_import_source. Do not edit.\n" + f"# Generated by catalog_import_source when the file was imported as {alias}-v{version}. Do not edit.\n" "# A source version is data: its rows are the bytes imported from the path below, ordered and\n" "# numbered in __row_order. The path is provenance — the build reads tallyman's own copy of it.\n" f"# imported from: {outside_path}\n" @@ -387,8 +481,8 @@ def _mint( # 1. The bytes, as imported, into the arena. ensure_cas_path digests what it wrote (ADR-011 D9). clone = si.ensure_cas_path(project, outside_path, digest) - # 2. The one snapshot, named by the entry hash. An existing one (a second alias over the same bytes) is kept: - # the hash is a function of the bytes and the reader, so it already holds exactly these rows. + # 2. The one snapshot, named by the entry hash. An existing one is kept: the hash is a function of the bytes and + # the reader, so it already holds exactly these rows. snapshot = snapshot_path(project, content_hash) if snapshot.exists(): from tallyman_xorq.digest import content_digest @@ -412,6 +506,9 @@ def _mint( if expr is None: raise BuildError(f"the generated recipe of {alias}-v{version} bound no 'expr'") target = entry_dir(project, content_hash) + # A re-import that repairs a missing snapshot writes into an entry that already exists; a failure must not + # take that entry with it. + created = not target.exists() target.mkdir(parents=True, exist_ok=True) try: from xorq.ibis_yaml.compiler import build_expr @@ -462,7 +559,8 @@ def _mint( ), ) except Exception: - shutil.rmtree(target, ignore_errors=True) + if created: + shutil.rmtree(target, ignore_errors=True) raise finally: import sys @@ -506,12 +604,14 @@ def update_and_depend( ``pinned_version`` beyond head+1 error: versions cannot be skipped ``pinned_version=N`` < head, digest matches return vN; the head does not move bytes match a version older than the head error: history is append-only (D11) + bytes are a version of another alias error: one set of bytes, one alias ================================================== ========================================= Args: outside_path: Any path to a parquet or CSV file. ``data/`` is not special — a file is imported from wherever it is, and after the import the path is provenance and is never read again. alias: The source alias. It may not already name a catalog alias, and a catalog alias may not later take it. + Bytes another alias of the project already holds are refused; the error names that alias. pinned_version: The version the caller claims this file is, when they want the claim checked. project: Project name override (defaults to the active project). prompt: Optional human-readable description, recorded on the entry. @@ -521,7 +621,9 @@ def update_and_depend( Returns: ``{"alias", "version", "hash", "created", "path", "digest", "row_count", "schema"}``. ``created`` is False - when the import was a no-op. + when the import was a no-op. An import of bytes that are already an entry rewrites nothing of that entry; if + its snapshot is gone it is healed from the clone and checked against the recorded digest, exactly as + ``ensure_materialized`` heals one, so a repair is never a way to change a version's rows. Raises: SourceImportError: for every row of the table above that is an error, and for a path that is not an importable @@ -563,12 +665,13 @@ def update_and_depend( with project_lock(proj): history = history_for(proj, alias) mint, version = _plan_version(alias, history, content_hash, pinned_version) - from tallyman_core.paths import entry_dir + if mint: + _refuse_bytes_held_elsewhere(proj, alias, content_hash) from tallyman_xorq.materialize import snapshot_path - # A no-op whose entry is gone is repaired here, and so is a missing snapshot: the caller has handed us the - # bytes, which is cheaper than going through the clone, and an entry directory is not cache at all. - if mint or not (entry_dir(proj, content_hash).is_dir() and snapshot_path(proj, content_hash).exists()): + existing = _readable_manifest(proj, content_hash) + if existing is None: + # No entry, or a directory a crash left without a manifest, which is not an entry either: write it. written = _mint( proj, src, @@ -580,10 +683,21 @@ def update_and_depend( prompt=prompt, ) else: - from tallyman_core import entry_schema_path, read_manifest + # The entry exists, and its recipe, build and manifest are the record of the import that minted it: an + # import of the same bytes, as a repair or under a new alias, rewrites none of them. A missing snapshot + # is healed the way ensure_materialized heals any source snapshot, from the clone and checked against + # the recorded result_digest, so a reader that now parses the bytes differently is recorded as an + # unfaithful heal instead of becoming the version's rows. The caller's bytes restore the clone first + # when it is gone too; ensure_cas_path verifies them against the digest (D9). + if not snapshot_path(proj, content_hash).exists(): + from tallyman_xorq.materialize import ensure_materialized + + si.ensure_cas_path(proj, src, digest) + ensure_materialized(proj, content_hash) + from tallyman_core import entry_schema_path written = { - "row_count": read_manifest(entry_dir(proj, content_hash)).row_count, + "row_count": existing.row_count, "schema": json.loads(entry_schema_path(proj, content_hash).read_text()), } if mint: diff --git a/tests/conftest.py b/tests/conftest.py index 115e92c..f826bea 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -49,7 +49,8 @@ def orders_parquet(project: str) -> Path: """Generate a deterministic shoe-orders fixture under project/data/. The file only. A recipe cannot read it (ADR-011 D2) — depend on ``orders_src`` for that, and use - this where the test is about the file itself: editing it, importing it under another alias, or + this where the test is about the file itself: editing it, importing it under an alias of the test's own (not + alongside ``orders_src``, since one set of bytes goes under one alias), or asserting on the bytes. """ return write_shoe_orders(data_dir(project) / "orders.parquet", n_rows=200, seed=0) diff --git a/tests/test_aliases.py b/tests/test_aliases.py index 731a7f4..a569be2 100644 --- a/tests/test_aliases.py +++ b/tests/test_aliases.py @@ -429,6 +429,51 @@ def test_set_alias_refuses_to_change_an_alias_kind(project: str): set_alias(project, "orders", "bbbbbbbbbbbb") +# An alias's kind matches the kind of the entries it points at. The kind lives on the alias (aliases.jsonl); whether an +# entry is a *source entry* — a version of an imported file, recognised by its manifest carrying ``provenance`` — lives +# on the entry. set_alias is the one writer every route goes through, so it is where the two are tied together. + + +def test_set_alias_refuses_a_catalog_alias_onto_a_source_entry(project: str, tmp_path: Path, monkeypatch): + """A catalog name over an import would let catalog_revise "revise" a version that has no recipe (D1).""" + from tallyman_core.aliases import AliasKindMismatch, alias_kind + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + imported = _import(project, tmp_path, "orders") + + with pytest.raises(AliasKindMismatch) as exc: + set_alias(project, "looks_like_catalog", imported["hash"]) + + assert "orders" in str(exc.value) + assert get_alias(project, "looks_like_catalog") is None + assert alias_kind(project, "looks_like_catalog") is None + + +def test_set_alias_refuses_a_source_alias_onto_a_computed_entry(project: str, tmp_path: Path, monkeypatch): + """A source alias advances only by an import, so its head is always an imported file, never a recipe's result.""" + from tallyman_core.aliases import SOURCE_KIND, AliasKindMismatch + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + imported = _import(project, tmp_path, "orders") + computed = build_and_persist(project, _over_source("orders", project)) + + with pytest.raises(AliasKindMismatch): + set_alias(project, "orders", computed.content_hash, kind=SOURCE_KIND) + + assert history_for(project, "orders") == [imported["hash"]] + + +def test_set_alias_does_not_check_a_hash_with_no_entry(project: str): + """With no manifest there is nothing to match the kind against, so either kind is accepted.""" + from tallyman_core.aliases import SOURCE_KIND, alias_kind + + set_alias(project, "a_catalog", "aaaaaaaaaaaa") + set_alias(project, "a_source", "bbbbbbbbbbbb", kind=SOURCE_KIND) + + assert alias_kind(project, "a_catalog") == "catalog" + assert alias_kind(project, "a_source") == SOURCE_KIND + + def test_renaming_a_source_alias_keeps_its_kind(project: str, tmp_path: Path, monkeypatch): """Rename and unalias behave as they do for catalog aliases (D1).""" from tallyman_core.aliases import SOURCE_KIND, alias_kind diff --git a/tests/test_code_edit.py b/tests/test_code_edit.py index 652842f..e0352b1 100644 --- a/tests/test_code_edit.py +++ b/tests/test_code_edit.py @@ -162,3 +162,26 @@ def test_entry_detail_omits_edit_button_in_serve_mode(project: str, orders_src: c = TestClient(app) r = c.put(f"/{project}/api/code/shoe_sales", json={"code": _filter(project)}) assert r.status_code == 403 + + +def test_put_code_refuses_a_source_alias(fresh_companion_app, project: str, orders_src: str, monkeypatch): + """A source alias's versions are imported files with no recipe to revise (ADR-011 D1), so the route refuses. + + It refuses before building, as ``catalog_revise`` does. It used to build the entry, fail to point the source + alias at it, and answer 500 with the entry left behind under no alias. + """ + from tallyman_core import entry_dir + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + catalog_create("shoe_sales", _agg(project)) + head = get_alias(project, orders_src) + entries = sorted(p.name for p in entry_dir(project, head).parent.iterdir()) + code = "from tallyman_xorq.io import tracked_expr_from_alias\nexpr = tracked_expr_from_alias('shoe_sales')\n" + + c = TestClient(fresh_companion_app, raise_server_exceptions=False) + r = c.put(f"/{project}/api/code/{orders_src}", json={"code": code}) + + assert r.status_code == 409, r.text + assert "catalog_import_source" in r.json()["detail"] + assert get_alias(project, orders_src) == head + assert sorted(p.name for p in entry_dir(project, head).parent.iterdir()) == entries diff --git a/tests/test_promote_diff.py b/tests/test_promote_diff.py index eeb15b7..59672e8 100644 --- a/tests/test_promote_diff.py +++ b/tests/test_promote_diff.py @@ -328,6 +328,37 @@ def test_http_promote_diff_creates_entry(fresh_companion_app, project: str, orde assert body["row_count"] > 0 +def test_http_promote_diff_refuses_a_source_alias_as_its_target( + fresh_companion_app, project: str, orders_src: str, tmp_path: Path, monkeypatch +): + """The generated target name can already be a source alias; the route refuses before building, as the tool does. + + It used to build the diff entry, fail to point the source alias at it, and answer 500 with the entry left behind. + """ + import pandas as pd + + from tallyman_core import entry_dir, get_alias + from tallyman_xorq.source_import import update_and_depend + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + catalog_create("shoe_sales", _agg_code(project)) + catalog_revise("shoe_sales", _filter_code(project)) + other = tmp_path / "outside" / "other.parquet" + other.parent.mkdir(parents=True) + pd.DataFrame({"x": [1, 2, 3]}).to_parquet(other) + update_and_depend(str(other), "diff_shoe_sales_v1_v2", project=project) + head = get_alias(project, "diff_shoe_sales_v1_v2") + entries = sorted(p.name for p in entry_dir(project, head).parent.iterdir()) + + c = TestClient(fresh_companion_app, raise_server_exceptions=False) + r = c.post(f"/{project}/api/promote_diff/shoe_sales/1/2") + + assert r.status_code == 409, r.text + assert "source alias" in r.json()["detail"] + assert get_alias(project, "diff_shoe_sales_v1_v2") == head + assert sorted(p.name for p in entry_dir(project, head).parent.iterdir()) == entries + + def test_http_promote_diff_sets_display_config(fresh_companion_app, project: str, orders_src: str, monkeypatch): from tallyman_core.display_configs import get_display_config diff --git a/tests/test_rebuild_native_catalog.py b/tests/test_rebuild_native_catalog.py index 511abd2..9694889 100644 --- a/tests/test_rebuild_native_catalog.py +++ b/tests/test_rebuild_native_catalog.py @@ -231,3 +231,73 @@ def test_rebuild_replays_a_source_entry_as_an_import(project, tmp_path): pointers = set(cs.read_tallyman_state(project)["entry_hashes"]) assert pointers == set(remap.values()) catalog.assert_catalog_consistent(project, pointers) + + +def test_toposort_orders_a_renamed_sources_versions_by_the_alias_that_holds_them(project): + """A source version follows the previous version of the alias that holds it now (ADR-011). + + ``provenance["alias"]`` is the name the version was imported under. After a rename no alias has that name, so + ordering by it gave v1 and v2 no edge and left them in hash order. Here v2's hash sorts first, and hash order + would replay v2's bytes as the renamed alias's v1. + """ + rb = _load_rebuild() + v1, v2 = "bbbbbbbbbbbb", "aaaaaaaaaaaa" + recipes = {v1: "# a source version\n", v2: "# a source version\n"} + provenance = {v1: {"alias": "a_src", "version": 1}, v2: {"alias": "a_src", "version": 2}} + history = {"renamed_src": [v1, v2]} + + assert rb.toposort(recipes, {"renamed_src": v2}, history, provenance) == [v1, v2] + + +def test_rebuild_replays_a_renamed_source_under_the_name_it_has_now(project, tmp_path): + """A renamed source is re-imported under its alias now, so a child reading that alias rebuilds (ADR-011). + + Replaying under ``provenance["alias"]`` minted the import-time name, ``a_src``, and never made ``renamed_src`` + until the final alias write, after every child had tried to build. + """ + import pandas as pd + import pytest + + from tallyman_core.aliases import SOURCE_KIND, load_kinds, rename_alias + from tallyman_core.manifest import read_manifest + from tallyman_core.paths import entry_dir + from tallyman_xorq.source_import import update_and_depend + + rb = _load_rebuild() + cs.genesis(project) + outside = tmp_path / "outside" + outside.mkdir(parents=True, exist_ok=True) + src = outside / "orders.parquet" + pd.DataFrame({"region": ["n", "s", "n"], "price": [1.0, 2.0, 3.0]}).to_parquet(src) + v1 = update_and_depend(src, "a_src", project=project) + pd.DataFrame({"region": ["n", "s", "e", "e"], "price": [1.0, 2.0, 3.0, 4.0]}).to_parquet(src) + v2 = update_and_depend(src, "a_src", project=project) + rename_alias(project, "a_src", "renamed_src") + child = build_and_persist( + project, + "from tallyman_xorq.io import tracked_expr_from_alias\n" + f"t = tracked_expr_from_alias('renamed_src', {project!r})\n" + "expr = t.group_by('region').aggregate(n=t.count())\n", + prompt="count by region", + ) + al.set_alias(project, "regions", child.content_hash) + cs.checkpoint_catalog(project, "corpus") + logged: list[str] = [] + + try: + remap = rb.rebuild_project(project, log=logged.append) + except Exception as exc: # noqa: BLE001 + pytest.fail(f"the rebuild failed after a rename: {exc!r}") + + assert remap[v1["hash"]] == v1["hash"] and remap[v2["hash"]] == v2["hash"] + assert load_kinds(project) == {"renamed_src": SOURCE_KIND, "regions": "catalog"} + assert al.history_for(project, "renamed_src") == [v1["hash"], v2["hash"]] + provenance = read_manifest(entry_dir(project, v1["hash"])).provenance + assert (provenance.alias, provenance.version) == ("renamed_src", 1) + # v1's outside path now holds v2's bytes, so v1 came from its clone, and the log says under which name. + assert any("renamed_src-v1" in line for line in logged), logged + assert len(cached_result_expr(project, remap[child.content_hash]).execute()) == 3 + + pointers = set(cs.read_tallyman_state(project)["entry_hashes"]) + assert pointers == set(remap.values()) + catalog.assert_catalog_consistent(project, pointers) diff --git a/tests/test_source_import.py b/tests/test_source_import.py index ddcf32c..de912b7 100644 --- a/tests/test_source_import.py +++ b/tests/test_source_import.py @@ -227,6 +227,32 @@ def test_bytes_matching_an_older_version_error_names_reset(project: str, tmp_pat assert "reset" in message.lower() +def test_the_append_only_refusal_does_not_advise_a_second_alias(project: str, tmp_path: Path, monkeypatch): + """D11's refusal offers ways back that work: a reset, or reading the old version pinned. + + It used to offer "import these bytes under a different alias", which is refused too, because bytes an alias + already holds cannot be imported under another (ADR-011 D1). + """ + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + src = _outside(tmp_path) / "orders.parquet" + _write_parquet(src, 10) + source_import.update_and_depend(str(src), "orders") + _write_parquet(src, 20) + source_import.update_and_depend(str(src), "orders") + _write_parquet(src, 10) # back to v1's bytes + + with pytest.raises(source_import.SourceImportError) as exc: + source_import.update_and_depend(str(src), "orders") + + message = str(exc.value) + assert "different alias" not in message, message + assert "pinned_expr_from_alias('orders-v1')" in message, message + with pytest.raises(source_import.SourceImportError, match="orders-v1"): + source_import.update_and_depend(str(src), "orders_again") + + def test_absent_alias_with_a_skipped_pin_errors(project: str, tmp_path: Path, monkeypatch): """A brand-new source alias starts at v1; a pin of v2 would leave version 1 undefined forever.""" from tallyman_xorq import source_import @@ -323,20 +349,138 @@ def test_the_snapshot_is_the_ordered_copy(project: str, tmp_path: Path, monkeypa assert read_manifest(entry_dir(project, out["hash"])).cache_worthy is True -def test_two_aliases_over_identical_bytes_share_one_entry(project: str, tmp_path: Path, monkeypatch): - """The entry hash is a function of the bytes and the reader options, so one file serves both aliases.""" +def test_identical_bytes_under_a_second_alias_is_an_error(project: str, tmp_path: Path, monkeypatch): + """One set of bytes is one source version under one alias; importing it again under another name is refused. + + The likeliest way to get here is not knowing the bytes are already in the project, so the error names the alias + and version that hold them and the way to give them a second name. The first alias's entry is untouched: a second + import used to rewrite its manifest and recipe, and a failure part-way deleted the entry outright. + """ from tallyman_xorq import source_import monkeypatch.setenv("TALLYMAN_PROJECT", project) src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + a = source_import.update_and_depend(str(src), "orders") + entry = entry_dir(project, a["hash"]) + manifest_before = (entry / "manifest.json").read_bytes() + recipe_before = (entry / "expr.py").read_bytes() + copy = _write_parquet(_outside(tmp_path) / "copy_of_orders.parquet", 10) + assert copy.read_bytes() == src.read_bytes() + + with pytest.raises(source_import.SourceImportError) as info: + source_import.update_and_depend(str(copy), "orders_again") + + message = str(info.value) + assert "orders-v1" in message + assert "tracked_expr_from_alias('orders')" in message + assert "catalog_create" in message + assert get_alias(project, "orders_again") is None + assert (entry / "manifest.json").read_bytes() == manifest_before + assert (entry / "expr.py").read_bytes() == recipe_before + + +def test_bytes_of_an_older_version_of_another_alias_are_refused(project: str, tmp_path: Path, monkeypatch): + """The bytes need not be another alias's head: any version of any other alias holds them.""" + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + src = _outside(tmp_path) / "orders.parquet" + _write_parquet(src, 10) + first_bytes = src.read_bytes() + source_import.update_and_depend(str(src), "orders") + _write_parquet(src, 20) + source_import.update_and_depend(str(src), "orders") + old = _outside(tmp_path) / "old_orders.parquet" + old.write_bytes(first_bytes) + + with pytest.raises(source_import.SourceImportError, match="orders-v1"): + source_import.update_and_depend(str(old), "archive") + + assert get_alias(project, "archive") is None + +def test_a_repair_import_writes_the_snapshot_and_nothing_else(project: str, tmp_path: Path, monkeypatch): + """Re-importing a version whose snapshot is gone restores the snapshot and leaves the entry alone. + + The entry's recipe, build and manifest are the record of the import, and a repair is not an import: it goes + through the heal ``ensure_materialized`` uses. So nothing is rebuilt (a failing ``build_expr`` does not matter), + and repairing from another path does not move the recorded provenance path or its time. + """ + import xorq.ibis_yaml.compiler as compiler + + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) a = source_import.update_and_depend(str(src), "orders") - b = source_import.update_and_depend(str(src), "orders_again") + entry = entry_dir(project, a["hash"]) + record = {name: (entry / name).read_bytes() for name in ("manifest.json", "expr.py")} + snapshot = snapshot_path(project, a["hash"]) + before = snapshot.read_bytes() + snapshot.unlink() + moved = _outside(tmp_path) / "moved" / "orders.parquet" + moved.parent.mkdir() + moved.write_bytes(src.read_bytes()) - assert a["hash"] == b["hash"] - assert get_alias(project, "orders") == get_alias(project, "orders_again") - snaps = sorted(p.name for p in snapshot_path(project, a["hash"]).parent.glob("*.parquet")) - assert snaps == [f"{a['hash']}.parquet"] + def rebuild(*args, **kwargs): + raise RuntimeError("a repair rebuilt the entry") + + monkeypatch.setattr(compiler, "build_expr", rebuild) + out = source_import.update_and_depend(str(moved), "orders") + + assert out["created"] is False + assert snapshot.read_bytes() == before + assert {name: (entry / name).read_bytes() for name in record} == record + + +def test_a_repair_import_is_verified_like_any_heal(project: str, tmp_path: Path, monkeypatch): + """A repair that writes other rows under the recorded hash is an unfaithful heal, and is recorded as one. + + The recorded digest and row count stay the build-time ones. A repair used to take whatever it wrote as the new + truth, so a child built on the old rows read the new ones with nothing to say they had changed. + """ + from tallyman_core.errors import list_errors + from tallyman_xorq import source_import + from tallyman_xorq.digest import content_digest + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + a = source_import.update_and_depend(str(src), "orders") + recorded = read_manifest(entry_dir(project, a["hash"])) + snapshot_path(project, a["hash"]).unlink() + write = source_import._write_snapshot + + def one_row_short(clone, reader, dest): # a reader that now parses the same bytes into other rows + write(clone, reader, dest) + table = pq.read_table(dest) + pq.write_table(table.slice(0, table.num_rows - 1), dest) + return content_digest(dest) + + monkeypatch.setattr(source_import, "_write_snapshot", one_row_short) + source_import.update_and_depend(str(src), "orders") + + after = read_manifest(entry_dir(project, a["hash"])) + assert (after.result_digest, after.row_count) == (recorded.result_digest, recorded.row_count) + codes = [r.get("code") for r in list_errors(project, limit=1000) if r.get("hash") == a["hash"]] + assert "unfaithful_heal" in codes, codes + + +def test_an_entry_whose_manifest_is_gone_is_written_again_by_a_re_import(project: str, tmp_path: Path, monkeypatch): + """An entry directory with no readable manifest, which a crash part-way through an import leaves, is not an entry. + + A re-import of its bytes writes it again rather than failing to read it. + """ + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + a = source_import.update_and_depend(str(src), "orders") + (entry_dir(project, a["hash"]) / "manifest.json").unlink() + + out = source_import.update_and_depend(str(src), "orders") + + assert (out["hash"], out["created"]) == (a["hash"], False) + assert read_manifest(entry_dir(project, a["hash"])).provenance.alias == "orders" def test_a_source_version_keeps_its_raw_bytes_in_the_clone_store(project: str, tmp_path: Path, monkeypatch): @@ -764,6 +908,218 @@ def test_a_source_snapshot_whose_clone_is_gone_is_pinned(project: str, tmp_path: assert ".cas" in reason, reason # the pin is the missing clone, not the fact that this is a source +# ``manifest.provenance.alias`` and ``.version`` are the name the import was made under, recorded once. A rename +# carries an alias's history and kind to the new name (``aliases.rename_alias``) and an unalias drops it, so a +# message that names a source version, or tells the user which import repairs it, has to ask the alias store who +# holds the entry now. + + +def _advised_import(message: str) -> tuple[list, dict]: + """The arguments of the ``catalog_import_source(...)`` call that ends *message*, parsed as Python literals.""" + import ast + + call = ast.parse(message[message.index("catalog_import_source(") :].rstrip("."), mode="eval").body + assert isinstance(call, ast.Call), message + return [ast.literal_eval(a) for a in call.args], {k.arg: ast.literal_eval(k.value) for k in call.keywords} + + +def test_a_pinned_source_snapshot_is_named_by_the_alias_it_was_renamed_to(project: str, tmp_path: Path, monkeypatch): + """The pin names the version as the catalog knows it now; the name it was imported under is history.""" + from tallyman_core.aliases import rename_alias + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + from tallyman_xorq.materialize import pinned_reason + + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + out = source_import.update_and_depend(str(src), "a_src") + rename_alias(project, "a_src", "renamed_src") + _clone_of(project, out).unlink() + + reason = pinned_reason(project, out["hash"]) + assert reason is not None + assert "renamed_src-v1" in reason, reason + assert "source version a_src-v1" not in reason, reason + + +def test_the_re_import_advice_names_the_alias_a_source_was_renamed_to(project: str, tmp_path: Path, monkeypatch): + """The heal error names the version by its alias now, and so does the import it advises.""" + from tallyman_core.aliases import rename_alias + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + from tallyman_xorq.build import BuildError + from tallyman_xorq.materialize import ensure_materialized + + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + out = source_import.update_and_depend(str(src), "a_src") + rename_alias(project, "a_src", "renamed_src") + snapshot_path(project, out["hash"]).unlink() + _clone_of(project, out).unlink() + + with pytest.raises(BuildError) as exc: + ensure_materialized(project, out["hash"]) + + message = str(exc.value) + assert "renamed_src-v1" in message, message + assert f"catalog_import_source({str(src)!r}, 'renamed_src', pinned_version=1)" in message, message + assert "'a_src'" not in message, message + + +def test_following_the_re_import_advice_after_a_rename_repairs_the_version(project: str, tmp_path: Path, monkeypatch): + """The advice is a call the user runs as written, so it has to repair the renamed alias's version. + + Advice that named the import-time alias minted a second source alias, ``a_src``, over the same entry, and the + renamed one's snapshot came back only by accident. + """ + from tallyman_core.aliases import load_kinds, rename_alias + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + from tallyman_xorq.build import BuildError + from tallyman_xorq.materialize import ensure_materialized + + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + out = source_import.update_and_depend(str(src), "a_src") + before = snapshot_path(project, out["hash"]).read_bytes() + rename_alias(project, "a_src", "renamed_src") + snapshot_path(project, out["hash"]).unlink() + _clone_of(project, out).unlink() + with pytest.raises(BuildError) as exc: + ensure_materialized(project, out["hash"]) + args, kwargs = _advised_import(str(exc.value)) + + repaired = source_import.update_and_depend(*args, **kwargs) + + assert load_kinds(project) == {"renamed_src": "source"} + assert (repaired["alias"], repaired["version"], repaired["created"]) == ("renamed_src", 1, False) + assert repaired["hash"] == out["hash"] + assert history_for(project, "renamed_src") == [out["hash"]] + assert snapshot_path(project, out["hash"]).read_bytes() == before + + +def test_catalog_rename_then_the_advised_catalog_import_source_repairs_the_version( + project: str, tmp_path: Path, monkeypatch +): + """The same, through the tools a user has: the advice is an MCP call, run against the MCP tool.""" + monkeypatch.setenv("TALLYMAN_PROJECT", project) + from tallyman_core.aliases import load_kinds + from tallyman_mcp.server import catalog_import_source, catalog_rename + from tallyman_xorq.build import BuildError + from tallyman_xorq.materialize import ensure_materialized + + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + out = catalog_import_source(str(src), "a_src") + assert "error" not in catalog_rename("a_src", "renamed_src") + snapshot_path(project, out["hash"]).unlink() + _clone_of(project, out).unlink() + with pytest.raises(BuildError) as exc: + ensure_materialized(project, out["hash"]) + args, kwargs = _advised_import(str(exc.value)) + + repaired = catalog_import_source(*args, **kwargs) + + assert "error" not in repaired, repaired + assert load_kinds(project) == {"renamed_src": "source"} + assert snapshot_path(project, out["hash"]).is_file() + + +def test_a_source_version_no_alias_holds_is_not_advised_back_under_its_old_name( + project: str, tmp_path: Path, monkeypatch +): + """After an unalias no alias holds the version, and the messages say so instead of naming the dead alias. + + Re-importing under the old name would not restore anything that exists: it would mint a new alias of that name. + """ + from tallyman_core.aliases import remove_alias + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + from tallyman_xorq.build import BuildError + from tallyman_xorq.materialize import ensure_materialized, pinned_reason + + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + out = source_import.update_and_depend(str(src), "a_src") + remove_alias(project, "a_src") + _clone_of(project, out).unlink() + + reason = pinned_reason(project, out["hash"]) + assert reason is not None + assert "no source alias" in reason, reason + assert "imported as a_src-v1" in reason, reason + + snapshot_path(project, out["hash"]).unlink() + with pytest.raises(BuildError) as exc: + ensure_materialized(project, out["hash"]) + + message = str(exc.value) + assert "no source alias" in message, message + assert "'a_src'" not in message, message + assert "pinned_version" not in message, message + + +def test_following_the_re_import_advice_for_a_csv_repairs_the_version(project: str, tmp_path: Path, monkeypatch): + """A CSV's entry hash covers its reader options (D12), so the advised import has to carry them. + + Without them the same file hashes to another entry, and the advised call is refused as "not orders-v1". + """ + from tallyman_mcp.server import catalog_import_source + from tallyman_xorq.build import BuildError + from tallyman_xorq.materialize import ensure_materialized + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + src = _write_csv(_outside(tmp_path) / "orders.csv", [("north", 1), ("south", 2)], sep=";") + out = catalog_import_source( + str(src), "orders", schema={"region": "string", "n": "int64"}, reader_options={"separator": ";"} + ) + assert "error" not in out, out + snapshot_path(project, out["hash"]).unlink() + _clone_of(project, out).unlink() + with pytest.raises(BuildError) as exc: + ensure_materialized(project, out["hash"]) + args, kwargs = _advised_import(str(exc.value)) + + repaired = catalog_import_source(*args, **kwargs) + + assert "error" not in repaired, repaired + assert (repaired["hash"], repaired["created"]) == (out["hash"], False) + assert snapshot_path(project, out["hash"]).is_file() + + +def test_a_pinned_import_read_another_way_says_the_reader_options_differ(project: str, tmp_path: Path, monkeypatch): + """The same bytes under other reader options are another entry, and the refusal has to say so. + + "This file is not orders-v1" is false about the file; what differs is how it is read. + """ + from tallyman_mcp.server import catalog_import_source + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + src = _write_csv(_outside(tmp_path) / "orders.csv", [("north", 1), ("south", 2)], sep=";") + assert "error" not in catalog_import_source(str(src), "orders", reader_options={"separator": ";"}) + + out = catalog_import_source(str(src), "orders", pinned_version=1) + + assert "reader options" in out.get("error", ""), out + + +def test_a_source_recipe_names_its_alias_as_the_one_it_was_imported_under(project: str, tmp_path: Path, monkeypatch): + """The generated recipe is written once, at import, so the Code tab must not present that name as the entry's. + + After a rename the entry is ``renamed_src-v1``; a header reading ``# a_src-v1: a source version`` says otherwise. + """ + from tallyman_core.aliases import rename_alias + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + out = source_import.update_and_depend(str(src), "a_src") + rename_alias(project, "a_src", "renamed_src") + + header = (entry_dir(project, out["hash"]) / "expr.py").read_text().splitlines()[0] + assert "imported as a_src-v1" in header, header + assert not header.startswith("# a_src-v1:"), header + + def test_a_reset_keeps_the_clone_of_an_imported_source_alive(project: str, tmp_path: Path, monkeypatch): """``gc_cas`` walks the retention closure; an imported version's clone is in it via the entry's provenance.""" from tallyman_xorq import source_import @@ -925,6 +1281,22 @@ def test_catalog_import_source_reports_an_error_as_a_dict(project: str, tmp_path assert "nope.parquet" in out["error"] +def test_catalog_import_source_reports_a_duplicate_import_as_an_error(project: str, tmp_path: Path, monkeypatch): + monkeypatch.setenv("TALLYMAN_PROJECT", project) + from tallyman_core.catalog_state import list_revisions + from tallyman_mcp.server import catalog_import_source + + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + catalog_import_source(str(src), "orders") + before = len(list_revisions(project)) + + out = catalog_import_source(str(src), "orders_again") + + assert "orders-v1" in out.get("error", ""), out + assert get_alias(project, "orders_again") is None + assert len(list_revisions(project)) == before + + def test_catalog_revise_refuses_a_source_alias(project: str, tmp_path: Path, monkeypatch): """There is no recipe to revise: a source alias advances only by an import.""" monkeypatch.setenv("TALLYMAN_PROJECT", project) @@ -955,6 +1327,37 @@ def test_promote_diff_refuses_a_source_alias_as_its_target(project: str, tmp_pat assert "source alias" in out["error"] +def test_catalog_alias_refuses_a_source_entry(project: str, tmp_path: Path, monkeypatch): + """A source version cannot take a catalog name: the alias's kind must match its entry's. + + catalog_alias checked only the kind of the *name*, so ``catalog_alias(, "x")`` made a catalog alias + whose head was an imported file, and ``catalog_revise("x", ...)`` was then allowed on it. The refusal names the + source alias-version that holds the entry and steers to a catalog entry that reads it, which follows the source + when it is imported again. + """ + monkeypatch.setenv("TALLYMAN_PROJECT", project) + from tallyman_core.aliases import alias_kind + from tallyman_mcp.server import catalog_alias, catalog_create, catalog_import_source + + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + imported = catalog_import_source(str(src), "orders") + + out = catalog_alias(imported["hash"], "x") + + assert "error" in out, out + assert "orders-v1" in out["error"] + assert "catalog_create" in out["error"] + assert get_alias(project, "x") is None + assert alias_kind(project, "x") is None + + steer = "from tallyman_xorq.io import tracked_expr_from_alias\nexpr = tracked_expr_from_alias('orders')" + followed = catalog_create("x", steer) + + assert "error" not in followed, followed + assert followed["hash"] != imported["hash"] + assert alias_kind(project, "x") == "catalog" + + # --------------------------------------------------------------------------- # D6 — the ordered-copy store is subsumed into the source entry's snapshot # --------------------------------------------------------------------------- @@ -996,3 +1399,148 @@ def test_the_ordered_copy_store_is_gone(project: str, tmp_path: Path, monkeypatc assert snapshot_path(project, out["hash"]).is_file() assert [d.name for d in sorted(compute_cache_dir(project).iterdir())] == [RESULT_CACHE_DIRNAME] + + +# --------------------------------------------------------------------------- +# a second name for a source is an expression over it, not a second import +# --------------------------------------------------------------------------- + + +def test_a_second_name_for_a_source_is_a_catalog_entry_over_it(project: str, tmp_path: Path, monkeypatch): + """What the duplicate-import error tells the user to do instead, and why it needs nothing to force the hash apart. + + A source entry's hash is an md5 of its bytes and reader options; a recipe's hash is xorq's hash of the expression. + An expression that only reads the source is therefore a different entry already, with a followed parent edge, so a + re-import of the source advances it like any other follower. + """ + from tallyman_mcp.server import catalog_create + from tallyman_xorq import source_import + + monkeypatch.setenv("TALLYMAN_PROJECT", project) + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 10) + source = source_import.update_and_depend(str(src), "orders") + + out = catalog_create( + "orders_eu", "from tallyman_xorq.io import tracked_expr_from_alias\nexpr = tracked_expr_from_alias('orders')\n" + ) + + assert "error" not in out, out + assert out["hash"] != source["hash"] + parents = read_manifest(entry_dir(project, out["hash"])).parents + assert [(p.ref, p.hash, p.follow) for p in parents] == [("orders", source["hash"], True)] + + +# --------------------------------------------------------------------------- +# two projects importing one file share nothing +# --------------------------------------------------------------------------- + + +@pytest.fixture +def two_projects(isolated_home: Path) -> tuple[str, str]: + from tallyman_core import ensure_project, set_active_project + + for name in ("alpha", "beta"): + ensure_project(name) + set_active_project("alpha") + return "alpha", "beta" + + +def test_two_projects_importing_one_file_each_get_their_own_copy(two_projects, tmp_path: Path): + """Same bytes, same entry hash, but each project holds its own entry, snapshot and clone.""" + from tallyman_xorq import source_import + from tallyman_xorq.source_import import source_clone_path + + alpha, beta = two_projects + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 12) + + a = source_import.update_and_depend(str(src), "orders", project=alpha) + b = source_import.update_and_depend(str(src), "orders", project=beta) + + assert a["hash"] == b["hash"] + assert a["created"] and b["created"], "the second project's import is not a no-op on the first's entry" + for proj in (alpha, beta): + manifest = read_manifest(entry_dir(proj, a["hash"])) + assert manifest.project == proj + assert snapshot_path(proj, a["hash"]).is_file() + assert source_clone_path(proj, manifest.provenance).is_file() + assert get_alias(proj, "orders") == a["hash"] + assert snapshot_path(alpha, a["hash"]) != snapshot_path(beta, b["hash"]) + + +def test_the_duplicate_import_check_is_per_project(two_projects, tmp_path: Path): + """One file under different alias names in two projects is two unrelated imports, not a duplicate.""" + from tallyman_xorq import source_import + + alpha, beta = two_projects + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 12) + + source_import.update_and_depend(str(src), "orders", project=alpha) + out = source_import.update_and_depend(str(src), "sales", project=beta) + + assert out["created"] is True + assert history_for(beta, "sales") == [out["hash"]] + assert history_for(beta, "orders") == [] + + +def test_deleting_and_healing_one_projects_snapshot_leaves_the_others_alone(two_projects, tmp_path: Path): + from tallyman_xorq import source_import + from tallyman_xorq.build import build_and_persist + from tallyman_xorq.digest import content_digest + from tallyman_xorq.materialize import ensure_materialized + from tallyman_xorq.result_cache import cached_result_expr + + alpha, beta = two_projects + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 12) + h = source_import.update_and_depend(str(src), "orders", project=alpha)["hash"] + source_import.update_and_depend(str(src), "orders", project=beta) + child_a = build_and_persist(alpha, _child_code("orders", alpha)).content_hash + child_b = build_and_persist(beta, _child_code("orders", beta)).content_hash + beta_snapshot = snapshot_path(beta, h) + beta_digest, beta_mtime = content_digest(beta_snapshot), beta_snapshot.stat().st_mtime_ns + + snapshot_path(alpha, h).unlink() + ensure_materialized(alpha, h) + + assert snapshot_path(alpha, h).is_file() + assert (content_digest(beta_snapshot), beta_snapshot.stat().st_mtime_ns) == (beta_digest, beta_mtime) + assert len(cached_result_expr(alpha, child_a).execute()) == 3 + assert len(cached_result_expr(beta, child_b).execute()) == 3 + + +def test_retiring_one_projects_clones_leaves_the_others(two_projects, tmp_path: Path): + """``.cas`` lives under each project's ``data/``; a sweep in one project never reaches another's.""" + from tallyman_xorq import source_identity as si + from tallyman_xorq import source_import + from tallyman_xorq.source_import import source_clone_path + + alpha, beta = two_projects + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 12) + h = source_import.update_and_depend(str(src), "orders", project=alpha)["hash"] + source_import.update_and_depend(str(src), "orders", project=beta) + beta_clone = source_clone_path(beta, read_manifest(entry_dir(beta, h)).provenance) + + assert si.gc_cas(alpha, set(), bullpen=tmp_path / "bullpen") == 1 + + assert beta_clone.is_file() + + +def test_a_re_import_in_one_project_does_not_stale_the_other(two_projects, tmp_path: Path): + from tallyman_core.aliases import set_alias + from tallyman_xorq import source_import + from tallyman_xorq.build import build_and_persist + from tallyman_xorq.staleness import scan + + alpha, beta = two_projects + src = _write_parquet(_outside(tmp_path) / "orders.parquet", 12) + source_import.update_and_depend(str(src), "orders", project=alpha) + source_import.update_and_depend(str(src), "orders", project=beta) + child_a = build_and_persist(alpha, _child_code("orders", alpha)).content_hash + child_b = build_and_persist(beta, _child_code("orders", beta)).content_hash + for proj, child in ((alpha, child_a), (beta, child_b)): + set_alias(proj, "totals", child) # the scan judges live alias heads (#154) + + _write_parquet(src, 20) + source_import.update_and_depend(str(src), "orders", project=beta) + + assert scan(alpha)[child_a].stale is False + assert scan(beta)[child_b].stale is True