Skip to content

ADR-011 review fixes: one alias per bytes, alias kinds, current names, verified repairs - #219

Merged
paddymul merged 13 commits into
feat/adr-011-stage-2from
fix/adr-011-duplicate-import
Sep 24, 2026
Merged

paddymul merged 13 commits into
feat/adr-011-stage-2from
fix/adr-011-duplicate-import

Conversation

@paddymul

@paddymul paddymul commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #218. The fixes from reviewing #217 and #218, each landed test first. #220 and #221 are merged into this branch (their own red/green runs are on those PRs), so they close as merged when this lands.

What changes

One set of bytes is one version under one alias (ADR-011 D1). A source entry's hash is an md5 of its bytes and reader options, so a second alias importing bytes another alias held landed on the same entry directory. _mint re-ran over it, rewriting the first alias's provenance and recipe, and a failure part-way through rmtree'd the entry the first alias pointed at. The import is now refused, at any version of any other alias. The error names the alias and version holding the bytes and the way to a second name: a catalog entry whose recipe is tracked_expr_from_alias('<that alias>'), which follows it on re-import and never shares its hash. A CSV read with other reader options is a different entry and still allowed (D12). The rule is per project: two projects importing one file each hold their own entry, snapshot and clone.

An alias's kind matches its entries' kind (from #220). set_alias refuses a catalog alias on a source entry and a source alias on a computed one, so catalog_alias(<source hash>, name) can no longer open a source version to catalog_revise. The companion's PUT /api/code/<alias> and promote-diff routes had no source-alias refusal at all: they built an entry, failed in set_alias and answered 500 with the entry left under no alias. They now refuse with the MCP tools' text (aliases.source_alias_refusal), 409, before building.

A version is named by the alias that holds it now (from #221). provenance records the name a version was imported as. The pin reason and the heal error used it as the current name, so after a rename the advised re-import minted a second source alias under the old name. scripts/rebuild_native_catalog.py replayed a renamed source under the old name too, so children reading the new name failed to rebuild and the versions lost their order.

A repair import heals and is verified. Re-importing a version whose snapshot is gone went through _mint, which rebuilt the entry, rewrote the manifest and took whatever it wrote as the recorded digest. A probe with a reader that drops a row went from 10 rows to 9 under the same hash with nothing recorded. The import now rewrites nothing of an existing entry: it restores the clone from the caller's bytes if needed and heals through ensure_materialized, which checks the recorded digest and records an unfaithful heal. An entry directory left without a manifest by a crash is written again rather than crashing the re-import.

Advice that names an import runs as written. The heal error's advised catalog_import_source(...) dropped a CSV's schema and reader options, so it named another entry and was refused. It now carries them (source_import.import_call). The pinned refusal says the reader options may be what differs, and D11's refusal offers pinned_expr_from_alias('<alias>-v<N>') instead of "a different alias", which D1 refuses.

Docs. ADR-011 (status, D1, the D3 table, D11, D12, Testing, a "Review fixes (PR #219)" implementation note, and two stale lines), docs/mcp-server.md, and the docstrings that described sharing.

TDD

Not here

🤖 Generated with Claude Code

paddymul and others added 4 commits September 24, 2026 05:50
Same bytes mint the same entry hash in both projects, but each holds its
own entry, snapshot and clone: a heal, a clone sweep or a re-import in one
never reaches the other, and the one-alias-per-bytes rule is per project.

Also pins the way to give a source a second name: a catalog entry whose
recipe reads it. Its hash is xorq's hash of the expression, not the md5 a
source entry is named by, so the two never coincide and nothing has to be
added to the recipe to force them apart.

These pass today; they guard the duplicate-import fix that follows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A second import of bytes another alias already holds minted nothing new
but re-ran _mint over the shared entry: it rewrote the first alias's
manifest and recipe with the second alias's name, and a failure in the
build step rmtree'd the entry the first alias points at.

- the second import raises, naming the alias and version that hold the
  bytes and the catalog_create way to a second name, and leaves the first
  entry byte-for-byte as it was
- the same holds for bytes of an older version of another alias
- catalog_import_source returns it as an error and records no revision
- a re-import that repairs a missing snapshot keeps the existing entry
  when the build step fails

Replaces test_two_aliases_over_identical_bytes_share_one_entry, which
pinned the behaviour being removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A source entry is named by its bytes and reader options, so a second
import of the same bytes under another alias landed on the first alias's
entry and re-ran _mint over it: the manifest's provenance and the recipe
were rewritten with the second alias's name, and a failure in the build
step rmtree'd the entry the first alias points at.

- update_and_depend refuses a mint whose entry is a version of another
  alias of the project, at any version. The error names that alias and
  version and the way to a second name: a catalog entry whose recipe is
  tracked_expr_from_alias(<that alias>). Keyed on the entry hash, so a CSV
  read two ways is still two imports (D12); per project.
- _mint removes the entry directory on failure only when it created it,
  so a re-import repairing a missing snapshot keeps the existing entry.
- ADR-011 D1 now says one set of bytes is one version under one alias,
  and records that it first said the opposite and why that changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The kind lives on the alias ("catalog" or "source" in aliases.jsonl), but
whether an entry is a source entry lives on the entry (its manifest carries
provenance), and nothing ties the two together. catalog_alias checks only the
kind of the name, so catalog_alias(<source entry hash>, "x") makes a catalog
alias whose head is an imported file. catalog_revise("x", ...) is then
allowed, which bypasses D1's "a source version has no recipe to revise".

Three tests pin the invariant:

- set_alias refuses a catalog alias onto a source entry;
- set_alias refuses a source alias onto a computed entry;
- MCP catalog_alias on a source entry returns an error naming the source
  alias-version (orders-v1) and steering to catalog_create over
  tracked_expr_from_alias, creates no alias, and the steer then works.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
manifest.provenance.alias and .version record the name a source version
was imported under, once. A rename carries the alias's history and kind to
the new name, and an unalias drops it, but three places still read the
import-time name as the current one:

- materialize.pinned_reason and the _heal_a_source error name the version
  a_src-v1 after a rename to renamed_src, and the heal error advises
  catalog_import_source(path, 'a_src', pinned_version=1). Following that
  advice mints a second source alias, a_src, over the same entry.
- The generated recipe's header reads "# a_src-v1: a source version", so the
  Code tab of renamed_src-v1 presents the old name as the entry's.
- scripts/rebuild_native_catalog.py replays a source entry under
  provenance["alias"] and orders its versions by that alias's history. After
  a rename the replay mints a_src, never makes renamed_src before the
  children build, and a child reading renamed_src fails; the versions lose
  their ordering edge and fall into hash order.

These tests pin the current name in both messages, advice that repairs the
renamed alias without minting a new one (through update_and_depend and
through the MCP tools), wording that does not send an unaliased version back
under its dead name, a recipe header that records the import name as
history, and a rebuild that replays a renamed source under its current name
and in version order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
paddymul and others added 8 commits September 24, 2026 06:45
set_alias now reads the manifest of the entry it is pointing at. A manifest
that records provenance is a source entry, a version of an imported file; any
other manifest is a computed entry. A catalog alias onto a source entry, or a
source alias onto a computed entry, raises AliasKindMismatch. The check lives
in set_alias because every route that names an entry goes through it, so
catalog_alias, a revise, a promoted diff, a recalc and an import all keep an
alias's kind and its entries' kind the same without each tool repeating it.

A hash with no readable manifest is not checked: the alias bookkeeping is used
and tested with hashes that name no entry. A passing test pins that boundary.

For a source entry the message names the source alias-version that holds it
(via version_of_hash) and gives the ADR-011 way to a second name:
catalog_create('<name>', "...expr = tracked_expr_from_alias('<source>')"), a
catalog entry that reads the source and follows it when it is imported again.
MCP catalog_alias returns that message as {"error": ...}.

The other set_alias callers only alias a hash they just built (catalog_create,
catalog_revise, promote_diff in MCP and the companion, the companion's PUT
/api/code, recalc, the rebuild and perf scripts) or just imported
(update_and_depend), so the new check cannot fire for them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
provenance.alias and .version stay what they are: the record of the import.
Everything that names a source version as it is now asks the alias store.

- source_import.source_version_in finds the source alias whose history holds
  an entry, trying the name it was imported as first. It is
  aliases.version_of_hash restricted to source aliases, because an import
  advances nothing else. current_source_version applies it to a project.
- materialize.pinned_reason and the _heal_a_source error name the version by
  that alias and add "imported as <old>-vN" when a rename changed it. The
  advised catalog_import_source(...) names that alias and version, so
  running it repairs the snapshot and mints nothing. When no source alias
  holds the entry, both messages say so, and the error says an import of the
  same bytes writes them again as a new version of whichever alias it names.
- The generated recipe's first line reads "Generated by catalog_import_source
  when the file was imported as a_src-v1", which stays true after a rename.
- scripts/rebuild_native_catalog.py orders a source's versions and replays
  each one under the source alias that holds it in the old catalog, pinned to
  its version there, so an out-of-order replay is an error. The re-point loop
  now skips only the alias the import pointed and sets any other holder with
  the kind it had; the pin needs a second source alias over the same bytes to
  be on that version before its next one replays.
- SourceProvenance's docstring says alias and version are the import name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… routes refuse source aliases (ADR-011)

Found reviewing #219.

- A re-import of a version whose snapshot is gone went through _mint: it
  rebuilt the entry, rewrote the manifest (provenance path, imported_at,
  result_digest, row_count) and took whatever it wrote as the new truth.
  A probe with a reader that drops a row showed 10 -> 9 rows under the
  same hash and no unfaithful_heal record, where ensure_materialized
  records one. The repair should rewrite nothing but the snapshot, and be
  verified like any heal.
- An entry directory with no manifest (a crash part-way through an
  import) could not be repaired by re-importing: the no-op branch read
  the manifest and raised.
- The heal error's advised catalog_import_source(...) dropped a CSV's
  schema and reader options, so running it as written was refused as
  "not orders-v1". The pinned-version refusal also blamed the file when
  only the reader options differed.
- PUT /api/code/<source alias> and POST /api/promote_diff onto a target
  name that is a source alias built an entry, failed to point the source
  alias at it and answered 500. Both should refuse before building, 409.

Replaces test_a_failed_repair_leaves_the_existing_entry_in_place, which
pinned the repair going through _mint.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
D11's refusal (bytes that match an older version of the same alias)
ends "or import these bytes under a different alias". Since #219 that
import is refused as well: bytes one alias holds cannot be imported
under another. The refusal should offer ways back that work, a reset or
reading the old version with pinned_expr_from_alias('<alias>-v<N>').

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…es refuse source aliases (ADR-011)

- update_and_depend never rewrites an entry that already exists. A no-op
  (or a new alias over an entry no alias holds) whose snapshot is gone
  restores the clone from the caller's bytes when it is gone too, then
  heals through ensure_materialized: from the clone, checked against the
  recorded result_digest, recorded as an unfaithful heal when the rows
  differ. It used to go through _mint, which rebuilt the entry, rewrote
  provenance and recorded whatever digest it wrote. An entry directory
  with no readable manifest is not an entry, and is written again.
- source_import.import_call prints the catalog_import_source call that
  imports a file the way its entry records it was read, with a CSV's
  schema= and reader_options=. The heal error's advice uses it, so the
  advice runs as written. The pinned refusal says the reader options may
  be what differs, and D11's refusal offers pinned_expr_from_alias
  instead of a second alias, which D1 refuses.
- The source-alias refusal text moves to aliases.source_alias_refusal.
  The companion's PUT /api/code and promote-diff routes refuse with it,
  409, before building; they used to build, fail in set_alias and 500
  with the entry left under no alias.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- ADR-011: status names PR #219; D1 gains the kind rule and says
  provenance keeps the name a version was imported as; D3's table gains
  the one-alias error and a note on what an import of an existing
  version does; D11 and D12 say "the same bytes read the same way" and
  that advice carries the reader options; Testing drops the
  import_once_and_depend bullet (not shipped) and lists the new cases;
  a "Review fixes (PR #219)" implementation note records what changed
  and why. Also fixes two stale lines: the arena holds snapshots, not
  ordered copies, and ADR-002's sources map goes rather than narrowing.
- docs/mcp-server.md: catalog_import_source's one-alias rule, the
  second-name recipe and the healing no-op; catalog_alias refuses a
  source entry.
- tests/conftest.py: orders_parquet's docstring no longer suggests
  importing it under another alias beside orders_src.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@paddymul paddymul changed the title ADR-011: importing bytes another alias already holds is an error ADR-011 review fixes: one alias per bytes, alias kinds, current names, verified repairs Sep 24, 2026
@paddymul

Copy link
Copy Markdown
Contributor Author

Review of #219: findings and how each was handled

  1. A repair re-import was not verified. It went through _mint, rewrote the manifest and recorded whatever it wrote. A probe with a reader that drops a row went from 10 to 9 rows under the same hash with no unfaithful_heal. Fixed in d3ef749: an import never rewrites an existing entry, and a missing snapshot heals through ensure_materialized. Red: dbde141.
  2. The heal error's advised import dropped a CSV's schema and reader options, so running it was refused. Found by the ADR-011: a source version is named by the alias that holds it now #221 agent. Fixed in d3ef749 (source_import.import_call). Red: dbde141.
  3. PUT /api/code/<source alias> built an entry, then answered 500 with the entry left under no alias. Found by the ADR-011: an alias's kind matches the entries it points at #220 agent. Fixed in d3ef749: refused before building, 409. Red: dbde141.
  4. The companion's promote-diff route did the same when its target name was a source alias. Fixed in d3ef749. Red: dbde141.
  5. D11's refusal advised "import these bytes under a different alias", which this PR refuses. Fixed in d3ef749: it offers pinned_expr_from_alias('<alias>-v<N>'). Red: 37567b2.
  6. scripts/rebuild_native_catalog.py replayed a renamed source under its old name. Fixed in 6aefd0f.
  7. Docs out of step with the code: D3's table lacked the new error row, Testing said "three errors", D11 said "the same bytes" where the code means bytes read one way, the status line didn't mention this PR, and the orders_parquet fixture, source_entry_hash and catalog_import_source docstrings were stale. Fixed in 9f46fc8.

Still open: whether the same CSV bytes under a second alias with other reader options should also be refused (D12 allows it today).

@paddymul
paddymul merged commit 968d502 into feat/adr-011-stage-2 Sep 24, 2026
3 checks passed
paddymul added a commit that referenced this pull request Sep 24, 2026
ADR-011 held back docs/architecture.md, caching.md, expression-lifecycle.md and
system-contract.md until this PR landed, so they would be rewritten once. Now
that the PR is rebased onto the merged ADR-011 stack (#217, #218, #219), they
describe what exists: a file enters only by catalog_import_source, as a source
entry under a source alias; a source entry's hash is an md5 of its bytes and
reader options; its snapshot is cache, healed from the clone under data/.cas,
and pinned once the clone is gone; recipes name aliases, never files or bare
hashes; alias kinds and the rule that an alias's kind matches its entries;
one set of bytes is one version under one alias; and staleness has one axis.

The ordered copy, the identity modes, manifest.sources, the source-digest
memo and the source axis are gone from every doc except where one says what
was removed. Known defects #197, #198, #207, #211 and #191, and the two
staleness defects with no issue, move to a note that they no longer apply.
reactive-recalc.md's walk-through now imports orders and advances it by a
re-import. mcp-server.md, installing.md (TALLYMAN_SOURCE_IDENTITY is gone), the
README and the code-derived sections of tallyman_explanation.md follow suit.

The #193 to #196 passages are unchanged; those fixes are being ported onto
this branch separately.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
paddymul added a commit that referenced this pull request Sep 24, 2026
ADR-011 held back docs/architecture.md, caching.md, expression-lifecycle.md and
system-contract.md until this PR landed, so they would be rewritten once. Now
that the PR is rebased onto the merged ADR-011 stack (#217, #218, #219), they
describe what exists: a file enters only by catalog_import_source, as a source
entry under a source alias; a source entry's hash is an md5 of its bytes and
reader options; its snapshot is cache, healed from the clone under data/.cas,
and pinned once the clone is gone; recipes name aliases, never files or bare
hashes; alias kinds and the rule that an alias's kind matches its entries;
one set of bytes is one version under one alias; and staleness has one axis.

The ordered copy, the identity modes, manifest.sources, the source-digest
memo and the source axis are gone from every doc except where one says what
was removed. Known defects #197, #198, #207, #211 and #191, and the two
staleness defects with no issue, move to a note that they no longer apply.
reactive-recalc.md's walk-through now imports orders and advances it by a
re-import. mcp-server.md, installing.md (TALLYMAN_SOURCE_IDENTITY is gone), the
README and the code-derived sections of tallyman_explanation.md follow suit.

The #193 to #196 passages are unchanged; those fixes are being ported onto
this branch separately.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant