fix(import): a failed import is recorded, names the user's file, and a column xorq cannot read is left out and named (#224, #225, #227, #234, #239)

[paddymul]

Stacked on #189: the base is feat/adr-007-009-cache-redesign, not main. Five issues in the ADR-011 import path: source_import.update_and_depend, which the catalog_import_source MCP tool calls.

Fixes #224
Fixes #225
Fixes #227
Fixes #234
Fixes #239

These close the issues only once #189 reaches main.

Terms: an import copies the user's file (the outside file) to a clone, data/.cas/<md5><suffix>, then writes the snapshot, the parquet file of its rows at compute_cache/result_cache/<hash>.parquet, then the entry directory with a generated recipe and a manifest. The recipe reads the snapshot with deferred_read_parquet. While a source version's clone is gone, its snapshot is pinned: the Cache page refuses to delete it, since nothing else can make it again.

What changed

#224: a column xorq has no type for is left out of the entry, and named

deferred_read_parquet asks xorq's DataFusion backend for the file's arrow schema and converts each field with PyArrowType.to_ibis, which has no entry for fixed_size_binary. A fixed_size_binary or UUID column failed there with a KeyError, after the clone and the snapshot were written.

update_and_depend now asks that backend for the file's schema before it digests anything, and runs the same conversion field by field. A top-level column with any field that fails is left out of the snapshot, not refused. meta goes when meta.key is a fixed_size_binary. The import says so in three places:

• The reader records it as omitted, [[column, why]]. The entry hash covers it, and a heal from the clone replays it, so the healed snapshot's digest matches.
• The generated recipe's header has one # not imported: line per column.
• The return value, and so the MCP reply, has omitted_columns and a warning:

/data/ids.parquet was imported without these 2 columns, which xorq, the reader of every entry, has no type for. Recipes over 'ids' do not see them:
  - 'id': 'id' is fixed_size_binary[16], cast it to binary, or, to see UUIDs as text, write each value as str(uuid.UUID(bytes=value))
  - 'meta': 'meta.key' is fixed_size_binary[4], cast it to binary
To import them, write the file again with those columns cast, and import that file.

The advice no longer says to cast a UUID to string: arrow refuses that cast (ArrowInvalid: Invalid UTF8 payload), since raw UUID bytes are not UTF-8. A file with no column left is still refused, and nothing is written.

These messages can be missed. A note that stays with the source and is shown in the UI is #254.

The check reads the file it names. register_parquet takes a glob pattern, not a path. So ids[v2].parquet matched nothing, the schema came back empty, and the check passed a fixed_size_binary column. ids[12].parquet matched the sibling ids1.parquet and checked that file instead. The backend is now given a symlink to the file, with a plain name, in a temporary directory. The same glob reading makes every entry read as empty when the project path holds [, * or ?. That is #255, not fixed here.

The clone is checked again. The check reads the caller's file before it is digested. A file rewritten in between was checked on its old bytes and minted from its new ones. _mint now takes the schema of the verified clone. If its left-out columns differ from those recorded, the import is refused and the call to run again is printed.

Deviation: the check reads DataFusion's schema, not pq.read_schema. The issue suggested converting pq.read_schema(src). The two schemas differ on extension types. DataFusion gives a top-level extension column its storage type and keeps a nested one:

column	pq.read_schema type	to_ibis of it	the read (DataFusion)
top-level JSON	extension<arrow.json>	KeyError	string, imports
top-level bool8	extension<arrow.bool8>	KeyError	int8, imports
top-level fixed_shape_tensor	extension<...>	KeyError	array<float64>, imports
top-level UUID	extension<arrow.uuid>	KeyError	fixed_size_binary[16], KeyError
JSON field in a struct	struct<j: extension<arrow.json>>	KeyError	KeyError

A check over pq.read_schema would leave out the first three, which import today. So the check asks the backend the read asks, through the same call xorq_datafusion.Backend.read_parquet makes. test_the_type_check_passes_every_type_the_read_takes covers the first three rows and every other type the issue lists as importing. The check executes nothing, so it adds no execution site for #118.

#225: a failed import removes what it wrote, and only that

_mint notes which of the clone and the snapshot were absent when it started. One failure handler removes those, plus the entry directory if _mint created it. The project lock, which update_and_depend holds, keeps anything else from writing those paths meanwhile. Two things stay:

• A clone that was already there. A CSV read with two sets of reader options is two entries over one clone (ADR-011 D12, reader options are fixed at import).
• A snapshot that was already there, such as one a reset left. The entry hash fixes its rows (a failed build deletes the snapshot already on disk for its hash — a reset-retired or crashed entry's only copy is lost #193).

The repair branch, for an entry that already exists, restores a missing clone (#239) and keeps it even if the heal after it fails. That clone is named by a live entry, so it is not an orphan.

#227: every failure is recorded, and the message is about the user's call

• A reader failure while the snapshot is written becomes a SourceImportError: polars' ValueError, TypeError, NoDataError and ComputeError, parsy's ParseError, and the schema DSL's own errors. It names the outside file and catalog_import_source, not the clone or tallyman_read_csv. Only those types are wrapped (_reader_errors), and a TypeError only when the call passed reader options. Any other exception, a bug in tallyman's own writer, keeps its type, so the tool reports it with its class name.
• CloneDigestMismatch, from ADR-011 D9 (ingest verifies what it wrote), becomes one too. It ends with the call to run again.
• The tool catches every exception, not three types. So each failure gets an error_id, an errors.jsonl record with the traceback, a build_error event and a build_failed notification, as _run_and_record does for builds. An exception that is not one of the three expected types keeps its class name in the message. The conversion of a list schema is inside the handler too, and schema=[1] gets a message that says what a schema is.
• The steps after the import (the alias_set event, the notebook, carrying the entry's config forward, the recalc) are handled as well. The head has already advanced by then, so the import happened: the reply is the import's, with after_import_error: {error, error_id} recorded as above. The dispatch checkpoint still commits the advance. Before, a failure there skipped the checkpoint and left the head advance uncommitted.

A CSV the inference ladder cannot parse, before and after:

Error calling tool 'catalog_import_source': tallyman_read_csv: the schema does not parse 25ecdf2ab1b2e352c713b40a15adbe77.csv: could not parse `x` as dtype `i64` at column 'a' (column number 1)
...
- increasing `infer_schema_length` (e.g. `infer_schema_length=10000`),
- specifying correct dtype with the `schema_overrides` argument
... Suggested schema (whole-file inference): schema=(('a', 'string'), ('b', 'int64'))

catalog_import_source could not parse /data/orders.csv: could not parse `x` as dtype `i64` at column 'a' (column number 1). Suggested schema (whole-file inference), as the import to run: catalog_import_source('/data/orders.csv', 'orders', schema=[['a', 'string'], ['b', 'int64']]).

The retry comes from import_call, so it carries reader_options and pinned_version when the call had them. The test runs it as written, through the client, and it imports.

Deviation: all of polars' advice is dropped, not only the lines about refused options. The message keeps polars' first paragraph, which names the value and the column. The advice after it is written as scan_csv keywords, not the tool's arguments. Besides the two options the import refuses, it suggests ignore_errors=True, which would turn the bad values into nulls without saying so, and null_values, which the caller can pass in reader_options.

io.py is unchanged, to stay clear of the _polars_dtype and timestamp work on #231. source_import reads the ladder's message with a regex (_LADDER_FAILURE) and strips the tallyman_read_csv: prefix from the others. The cost is that the schema DSL's own messages still show their examples as tuples, as in schema=(("Date", "date"), ("&rest", "infer")). Those are examples of the positional form, not a retry call.

#234: the append-only refusal does not send the user to a command that does not exist

It first said to reset with tallyman reset or catalog_reset_to, and neither exists. It no longer advises a reset at all. tallyman reset-to <step> exists, but it moves the whole catalog back, every alias and entry with it, to put one source on an older version. The refusal now offers only pinned_expr_from_alias('<alias>-v<N>'), which reads the old version without moving the head. Moving one alias back on its own is #256, which also amends ADR-011 D11.

test_every_catalog_name_a_message_in_src_quotes_is_a_tool_or_a_python_name is the guard the issue suggested. It covers string literals in src/, but not docstrings, since dependents.py's docstring names the deleted catalog_parents and catalog_dag as history. It does not cover README.md, whose catalog_load_parquet mentions #216 rewrites.

#239: a re-import restores a lost clone whenever it is missing

The existing-entry branch calls ensure_cas_path every time. That function writes the clone only when it is missing and verifies it against the digest (D9). The clone goes to the path the entry recorded: ensure_cas_path now takes the suffix. The same bytes imported from orders.pq for an entry minted from orders.parquet have the same entry hash. Before, a repair from such a file wrote a second clone under .pq and left the entry's clone missing.

Tests

In tests/test_source_import.py, a new section at the end, and one guard in tests/test_mcp_tool.py:

• importing a parquet file with a fixed_size_binary column fails with a KeyError traceback that names no column #224: a file with a top-level fixed_size_binary, a struct-nested one and a UUID column imports with only n. It returns omitted_columns and a warning naming all three, with a cast that works. The recipe header and the recorded reader name them, the alias reads through tracked_expr_from_alias, and a deleted snapshot heals with the same columns. The same, for files named ids[v2].parquet, ids[12].parquet (beside a decoy ids1.parquet), ids*.parquet and ids?.parquet. A file rewritten between the check and the digest is refused and leaves nothing.
• a failed import leaves its clone in data/.cas and its snapshot in result_cache/ — nothing ever deletes the clone #225: an import that fails in its generated recipe leaves both directories as they were. So does a CSV that fails to parse.
• catalog_import_source lets a CSV reader error escape unrecorded — the message names the clone and tallyman_read_csv #227: through fastmcp.Client (in memory), seven CSV failures each return {error, error_id}, with one errors.jsonl record, one build_error event and a build_failed notification. The seven are a pinned-type mismatch, a ragged row, invalid UTF-8, a dtype that does not parse, separator="ab", an unknown reader option, and an empty file. The message names the file and the tool, not tallyman_read_csv or the clone. For the mismatch, the advised retry is parsed and run as written. A clone write that fails its digest check is recorded the same way and leaves no clone. schema=[1] is recorded the same way. A failure after the head advanced is recorded as after_import_error, and the step still moves. A KeyError from the snapshot writer keeps its type and leaves nothing.
• the append-only import error names reset commands that do not exist (tallyman reset, catalog_reset_to) #234: the refusal advises no reset and offers the pinned read. Also the catalog_* guard above.
• re-importing a version whose clone is lost does not restore the clone while its snapshot exists, so the version stays pinned #239: with the clone deleted and the snapshot present, a pinned re-import restores the clone and pinned_reason returns None, from a file with the same suffix and from one with another.
• Guards that pass before and after, in the fix commits: the types the read takes and a file with no column xorq reads (importing a parquet file with a fixed_size_binary column fails with a KeyError traceback that names no column #224), and a failed import over a clone another entry uses (a failed import leaves its clone in data/.cas and its snapshot in result_cache/ — nothing ever deletes the clone #225).

test_suggested_schema_recovery_is_pasteable_with_reserved_column now takes the suggestion out of the advised call, since the message no longer ends in schema=<tuple>. test_recon_cas_path_raises_instead_of_serving_drifted_live_bytes is deleted with recon_cas_path, which nothing in src/ called, and which named the clone by the live file's suffix (the bug #239 fixed for the repair path).

How it was built

1. b284b6d adds the 16 failing tests. CI run 36017772166: ruff passed, and the fast suite had 16 failed and 954 passed. The 16 failures were exactly the new tests, each for the reason its issue gives: a BuildError carrying the KeyError (importing a parquet file with a fixed_size_binary column fails with a KeyError traceback that names no column #224), leftover files (a failed import leaves its clone in data/.cas and its snapshot in result_cache/ — nothing ever deletes the clone #225), is_error with no record (catalog_import_source lets a CSV reader error escape unrecorded — the message names the clone and tallyman_read_csv #227), catalog_reset_to in the text (the append-only import error names reset commands that do not exist (tallyman reset, catalog_reset_to) #234), a clone still missing (re-importing a version whose clone is lost does not restore the clone while its snapshot exists, so the version stays pinned #239).
2. e6447b9 fixes importing a parquet file with a fixed_size_binary column fails with a KeyError traceback that names no column #224. b8acc06 fixes the other four. CI run 36020422748 on the tip: ruff, the fast suite (972 passed, and vitest 15 passed) and the integration suite (7 passed) all pass.
3. After review (the review comment below lists each finding), 667557d adds 10 failing tests and 026148f changes the the append-only import error names reset commands that do not exist (tallyman reset, catalog_reset_to) #234 test. CI runs 36041713861 (10 failed, 984 passed, exactly the new tests) and 36043124024 (11 failed, 983 passed). e0e91b9 reworks importing a parquet file with a fixed_size_binary column fails with a KeyError traceback that names no column #224 and fixes the rest. CI run 36043866014: ruff, the fast suite (993 passed, and vitest 15 passed) and the integration suite (7 passed) all pass.

Checked locally before each push, with TALLYMAN_HOME and TALLYMAN_COMPANION_URL pointed at scratch values:

• On the base, the 16 new tests failed for those reasons.
• At the fix tip, the fast suite had 965 passed and 6 skipped. test_fouc::test_unknown_api_path_404s failed because it needs the built React app, as on fix(xorq): a failed build keeps the snapshot already on disk for its hash (ADR-011 port of #213) #222 and fix(cache): a pin survives a reset and a dismissed error banner (#194, #195, #196), on the ADR-011 stack #223.
• At the fix tip, the integration suite had 7 passed, and uvx ruff check passed.
• The importing a parquet file with a fixed_size_binary column fails with a KeyError traceback that names no column #224 commit alone passed its two tests, run from an export of the index.

The repo has no .pre-commit-config.yaml, and the files this PR edits are not ruff-formatted as a whole. So ruff format was applied to the lines this PR touches and nowhere else.

Not in this PR

• a source imported without some columns carries no permanent note saying so #254: a note on the source, shown in the UI, that some columns were not imported.

• every entry reads as empty, with no error, when the project path holds a glob character #255: with [, * or ? in the project path, every entry reads as empty, with no error.

• no way to move one source alias back to an older version #256: moving one source alias back to an older version.

• An alias write that fails after _mint returns leaves the new entry, snapshot and clone on disk with no alias. Found in review, and deferred.

• an unfaithful heal of a source version is blamed on the recipe (#83), and a corrupt clone is healed from and pinned #232: a corrupt clone is still trusted, since ensure_cas_path writes only a missing one.

• a writer killed mid-write leaves a .tmp file in result_cache/ that the Cache page never lists and nothing deletes #226: temp files a killed writer leaves in data/.cas/ and result_cache/.

• For a ragged row, the suggested schema does not fix the file. That was already so, and tallyman_read_csv: ragged/short CSV rows are silently null-filled instead of raising #142 covers the short-row side.

• A failed import can write a clone that a different entry has lost (the same bytes, read with other reader options) and then remove it, since it was absent when the import started. That entry stays pinned until its own re-import restores the clone (re-importing a version whose clone is lost does not restore the clone while its snapshot exists, so the version stays pinned #239).

🤖 Generated with Claude Code

[paddymul]

Review of b8acc06, and what was done about each finding

/code-review found ten issues. Failing tests went in 667557d and 026148f, and the fixes in e0e91b9.

1. The importing a parquet file with a fixed_size_binary column fails with a KeyError traceback that names no column #224 type check read a glob, not the file (source_import.py:267). register_parquet takes a pattern, so ids[v2].parquet matched nothing and passed a fixed_size_binary column, and ids[12].parquet read the sibling ids1.parquet. Fixed in e0e91b9: the backend reads a symlink with a plain name. The same glob reading empties every entry when the project path holds [, * or ?. That is filed as every entry reads as empty, with no error, when the project path holds a glob character #255.
2. The UUID advice didn't work. Casting a UUID or fixed_size_binary(16) to string raises ArrowInvalid: Invalid UTF8 payload. Fixed in e0e91b9: the advice is binary, or each value formatted with str(uuid.UUID(bytes=value)). importing a parquet file with a fixed_size_binary column fails with a KeyError traceback that names no column #224 itself changed too. Such a column is now left out of the entry and named, where before the whole file was refused. The reader, the recipe header and the reply record it, and a note on the source shown in the UI is a source imported without some columns carries no permanent note saying so #254.
3. Every non-OSError from _write_snapshot became "could not read your file". Fixed in e0e91b9: only the reader's own error types are wrapped (_reader_errors), so a bug in tallyman's writer keeps its type and class name.
4. A failed set_alias leaves an orphan entry (:887). Deferred at Paddy's call. It is listed under "Not in this PR".
5. Not every failure of the tool was recorded (server.py:607). Fixed in e0e91b9. The schema conversion is inside the handler, with a message that says what a schema is. A failure after the head advanced is recorded as after_import_error, and the checkpoint still commits the advance.
6. The check read the file before the digest, so a rewrite in between was not checked. Fixed in e0e91b9: _mint checks the verified clone again and refuses when its left-out columns differ.
7. _read_failure regex-parses io.py's English message. Declined for now. A structured exception means changing io._materialize_ordered, which a tz-aware timestamp in an import schema reads offset-less text as UTC and converts it, contrary to ADR-005 D9(a) #231 is changing. It can follow once a tz-aware timestamp in an import schema reads offset-less text as UTC and converts it, contrary to ADR-005 D9(a) #231 lands.
8. Every parquet import builds a xorq backend just to read a schema. Declined. The left-out columns are now part of the entry hash, so the check has to run before the hash, on no-op imports too. It asks the same backend the read asks so the two cannot disagree, and it costs about 6 ms.
9. recon_cas_path built the clone path by hand, with the live file's suffix. Fixed in e0e91b9: nothing called it, so it is deleted, along with its test and LostSourceVersion. catalog_state.py:248 builds the .cas directory, not a clone's path, so it stays.
10. _mint worked out the clone path twice. Fixed in e0e91b9: it uses the path _clone_verified returns.

Also changed, at Paddy's direction: the #234 refusal no longer advises tallyman reset-to, which moves the whole catalog back to put one alias on an older version. It offers the pinned read, and moving one alias back on its own is #256.

CI: red runs 36041713861 (10 failed, 984 passed) and 36043124024 (11 failed, 983 passed), in both cases exactly the new tests. Fix run 36043866014: ruff, the fast suite (993 passed) and the integration suite (7 passed) all pass.

🤖 Generated with Claude Code

[paddymul]

Review of #247 (2026-09-28)

A /code-review pass found ten things. Each was reproduced by a test that CI saw fail first (2597de8, 1a15939) or was split out into its own issue.

Fixed here:

• Steps after the import shared one try. A failed config carry-forward skipped the new_entry notification and the recalc, so dependents of the source stayed on the old version's rows. Each step now runs on its own. e1c8891
• A failure after a successful import was recorded as a failed build. It was a build_error event with no hash, so the Log showed a failed build for a committed import. It is now an after_import_error event. The message says which version was imported and what failed after it, and the error record carries the entry's hash so stale dependents tie back to it. build_failed is still sent, since it is what reloads the companion's error banner. e1c8891
• The the append-only import error names reset commands that do not exist (tallyman reset, catalog_reset_to) #234 guard test counted any variable, parameter or attribute as a defined name, so a stale tool name spelled like a variable passed. It now counts only modules, functions and classes. 42a64d4

Split out:

• a source's entry hash covers the printed arrow type of each left-out column, so the same bytes mint a new version after an upgrade that prints types differently #274: the entry hash covers the printed arrow type of each left-out column, so the same bytes mint a new version after an upgrade that prints types differently. What the hash should cover is still open.
• _mint reports a ValueError from tallyman's own snapshot writer as the user's file being unreadable #269: _reader_errors wraps a ValueError from tallyman's own snapshot writer as the user's file being unreadable.
• two import paths skip #227's error wrapping: the clone's second type check and the repair branch's heal #270: two import paths skip the catalog_import_source lets a CSV reader error escape unrecorded — the message names the clone and tallyman_read_csv #227 wrapping: the clone's second type check and the repair branch's heal.
• #224's type check reads DataFusion's schema of the source, but the snapshot is written from pyarrow's: a nested JSON field without ARROW:schema passes the check and fails the read #273: the importing a parquet file with a fixed_size_binary column fails with a KeyError traceback that names no column #224 check reads DataFusion's schema of the source, but the snapshot is written from pyarrow's. A nested JSON field written without ARROW:schema passes the check, then fails the read.
• _schema_the_read_sees builds a new DataFusion backend per call, twice per parquet import #271 (performance): _schema_the_read_sees builds a DataFusion backend per call.
• _mint computes the clone path twice, once for the cleanup list and once for the write #272: _mint computes the clone path twice.
• The symlink in the type check doesn't fix glob paths on the real read path. That was already every entry reads as empty, with no error, when the project path holds a glob character #255.

🤖 Generated with Claude Code