Skip to content

Commit fa0a270

Browse files
paddymulclaude
andcommitted
fix(catalog): keep alias bookkeeping out of the xorq catalog repo (#48)
tallyman wrote aliases.json/alias_history.json under catalog_dir — the same directory that is the xorq catalog git repo — and checkpoint_catalog committed them. xorq's Catalog.assert_consistency() requires the repo's tracked files to be exactly its managed set (catalog.yaml + per-entry .zip/.metadata.yaml + alias symlinks), so the two extra tracked files made every Catalog.from_kwargs(..., init=False) raise a bare AssertionError. add_entry swallowed it, so every `xorq catalog add` after the first silently no-op'd and 17/18 recipes never reached git. Move the bookkeeping to artifacts/alias_state/ (outside the repo) and mirror its content into catalog.yaml as tallyman-owned keys (alias_map, alias_history), so reset_to still rolls alias state back now that git no longer tracks the files. checkpoint_catalog's _TRACKED drops to just catalog.yaml. Surface the previously-swallowed failure: build_and_persist exposes BuildResult.catalog_registered, and the failed-add path logs at error level instead of warning. - failing tests for this landed in 80d2e9b; this is the fix half of the split - test_failed_registration_is_observable: its premise (add fails on its own) was bug #48 itself; rewritten to inject the same inconsistency so it tests the catalog_registered signal rather than the bug - test_reset_rolls_back_alias_state: the deferred regression guard, shipping with the fix per the plan (it passes today, so it can't ride the failing-tests PR) - test_pack: alias file path updated to artifacts/alias_state/ Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6cb0b46 commit fa0a270

7 files changed

Lines changed: 117 additions & 24 deletions

File tree

src/tallyman_core/aliases.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
from pathlib import Path
1717

1818
from tallyman_core import xorq_catalog as _xcat
19-
from tallyman_core.paths import catalog_dir, ensure_project
19+
from tallyman_core.paths import alias_state_dir, ensure_project
2020

2121

2222
class AliasExists(ValueError):
@@ -28,11 +28,11 @@ class AliasNotFound(KeyError):
2828

2929

3030
def _aliases_path(project: str) -> Path:
31-
return catalog_dir(project) / "aliases.json"
31+
return alias_state_dir(project) / "aliases.json"
3232

3333

3434
def _history_path(project: str) -> Path:
35-
return catalog_dir(project) / "alias_history.json"
35+
return alias_state_dir(project) / "alias_history.json"
3636

3737

3838
def _read_json(p: Path, default):
@@ -57,6 +57,17 @@ def load_history(project: str) -> dict[str, list[str]]:
5757
return _read_json(_history_path(project), {})
5858

5959

60+
def write_state(project: str, aliases: dict[str, str], history: dict[str, list[str]]) -> None:
61+
"""Overwrite the alias bookkeeping files wholesale.
62+
63+
Used by ``catalog_state.materialize`` to reconstruct alias state from the
64+
catalog.yaml mirror on a ``reset_to``: the files live outside the git repo
65+
now (#48), so git can't roll them back — this does.
66+
"""
67+
_write_json(_aliases_path(project), aliases)
68+
_write_json(_history_path(project), history)
69+
70+
6071
def get_alias(project: str, name: str) -> str | None:
6172
return load_aliases(project).get(name)
6273

src/tallyman_core/catalog_state.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,15 @@
1111
entry_hashes: [content_hash, ...] # pointer to untracked entries/<hash>/
1212
result_cache: [relpath, ...] # pointer to untracked result cache
1313
compute_cache: [relpath, ...] # pointer to untracked compute cache
14-
15-
The catalog git repo tracks only ``catalog.yaml`` + the two alias files. The
14+
alias_map: {alias: latest_hash} # mirror of artifacts/alias_state/aliases.json
15+
alias_history: {alias: [hash, ...]} # mirror of artifacts/alias_state/alias_history.json
16+
17+
The catalog git repo tracks only ``catalog.yaml``. tallyman's alias bookkeeping
18+
(``aliases.json``/``alias_history.json``) lives outside the repo, under
19+
``artifacts/alias_state/`` — committing it inside made xorq's
20+
``assert_consistency`` reject the repo and every ``xorq catalog add`` after the
21+
first silently no-op (#48). Its content is mirrored into ``catalog.yaml`` above
22+
so ``reset_to`` still rolls alias state back (git no longer can). The
1623
heavy artifacts (entries/, the caches) are content-addressed, additive, and
1724
untracked — ``git reset`` can't roll them back, so ``reset_to`` reconciles them
1825
to the recorded pointer lists: evictions retire to the bullpen (not deleted),
@@ -40,6 +47,7 @@
4047

4148
import yaml
4249

50+
from tallyman_core import aliases as al
4351
from tallyman_core import charts, notebook
4452
from tallyman_core import display_configs as dc
4553
from tallyman_core import post_processing as pp
@@ -60,7 +68,10 @@
6068

6169
# git-level identity flags, so commits work without a global git config.
6270
_GIT_ID = ["-c", "user.email=tallyman@local", "-c", "user.name=tallyman"]
63-
_TRACKED = ("catalog.yaml", "aliases.json", "alias_history.json")
71+
# Only catalog.yaml is git-tracked in the catalog repo. The alias bookkeeping
72+
# files moved out to artifacts/alias_state/ (#48) — tracking them here made
73+
# xorq's assert_consistency reject the repo.
74+
_TRACKED = ("catalog.yaml",)
6475
_STEP_RE = re.compile(r"^step-(\d+)$")
6576
# Refs and labels are operator input that ends up as git arguments. A plain
6677
# tag-shaped name only: no leading dash (option injection), no revision
@@ -96,6 +107,8 @@ def read_tallyman_state(project: str) -> dict:
96107
"entry_hashes": raw.get("entry_hashes", []),
97108
"result_cache": raw.get("result_cache", []),
98109
"compute_cache": raw.get("compute_cache", []),
110+
"alias_map": raw.get("alias_map", {}),
111+
"alias_history": raw.get("alias_history", {}),
99112
}
100113

101114

@@ -154,6 +167,8 @@ def capture_tallyman_state(project: str) -> dict:
154167
"entry_hashes": sorted(c.name for c in ed.iterdir() if c.is_dir()) if ed.exists() else [],
155168
"result_cache": _list_cache_files(result_cache_dir(project)),
156169
"compute_cache": _list_cache_files(compute_cache_dir(project)),
170+
"alias_map": al.load_aliases(project),
171+
"alias_history": al.load_history(project),
157172
}
158173
write_tallyman_state(project, **state)
159174
return state
@@ -223,6 +238,8 @@ def materialize(project: str) -> None:
223238
_materialize_display_configs(project, raw["display_configs"])
224239
if "notebook" in raw:
225240
_materialize_notebook(project, raw["notebook"] or {"cells": []})
241+
if "alias_map" in raw:
242+
al.write_state(project, raw["alias_map"] or {}, raw.get("alias_history") or {})
226243

227244

228245
# ---------------------------------------------------------------------------

src/tallyman_core/paths.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
├── buckaroo_sessions.json # global Buckaroo session map
88
└── projects/<name>/
99
├── artifacts/ # everything the system produces
10-
│ ├── catalog/entries/<hash>/
11-
│ ├── catalog/aliases.json, alias_history.json, chart_specs/
10+
│ ├── catalog/entries/<hash>/ # the xorq catalog git repo
11+
│ ├── alias_state/aliases.json, alias_history.json
1212
│ ├── post_processing/<name>.py
1313
│ ├── stats/<name>.py
1414
│ ├── exports/... # marimo .py, screenshots, CSVs
@@ -119,6 +119,20 @@ def catalog_dir(project: str) -> Path:
119119
return artifacts_dir(project) / "catalog"
120120

121121

122+
def alias_state_dir(project: str) -> Path:
123+
"""Tallyman's alias bookkeeping — deliberately OUTSIDE the xorq catalog repo.
124+
125+
``aliases.json`` / ``alias_history.json`` are tallyman-owned, not
126+
xorq-managed. They used to live under ``catalog_dir``, so ``checkpoint_catalog``
127+
committed them into the xorq catalog git repo, and xorq's
128+
``assert_consistency`` then refused to open it — every ``xorq catalog add``
129+
after the first silently no-op'd (#48). They live here instead; their content
130+
is mirrored into ``catalog.yaml`` (tallyman-owned keys), so ``reset_to`` still
131+
rolls alias state back even though git no longer tracks the files.
132+
"""
133+
return artifacts_dir(project) / "alias_state"
134+
135+
122136
def entries_dir(project: str) -> Path:
123137
return catalog_dir(project) / "entries"
124138

src/tallyman_core/xorq_catalog.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ def add_entry(
6363
def _run(d: Path) -> bool:
6464
r = _xorq(project, "add", "--no-sync", str(d), *alias_flags)
6565
if r.returncode != 0:
66-
log.warning("xorq catalog add failed for %s: %s", d, r.stderr.strip())
66+
# error, not warning: a failed add means the recipe never reached git
67+
# and is silently lost (#48). The caller surfaces this on BuildResult.
68+
log.error("xorq catalog add failed for %s: %s", d, r.stderr.strip())
6769
return False
6870
return True
6971

src/tallyman_xorq/build.py

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ class BuildResult:
5151
row_count: int
5252
execute_seconds: float
5353
schema: dict
54+
# Whether the entry's recipe landed in the xorq catalog git repo. None on the
55+
# re-run path (entry already on disk, registration not re-attempted); True/False
56+
# on a fresh build. False means the recipe is NOT durable — see #48.
57+
catalog_registered: bool | None = None
5458

5559

5660
class BuildError(RuntimeError):
@@ -369,22 +373,32 @@ def build_and_persist(
369373
except OSError:
370374
pass
371375

372-
# Best-effort: register with the xorq catalog git repo.
376+
# Best-effort by design (a missing/misconfigured catalog repo must not break
377+
# the build), but the outcome is surfaced on the result instead of swallowed:
378+
# a failed add means the recipe never reached git and is not durable (#48).
379+
import logging
380+
381+
catalog_registered: bool | None = None
373382
try:
374383
from tallyman_core.xorq_catalog import add_entry as _xcat_add
375384

376-
_xcat_add(project, xorq_build_dir, entry_name=content_hash)
385+
catalog_registered = _xcat_add(project, xorq_build_dir, entry_name=content_hash)
377386
except Exception as exc:
378-
import logging
379-
380-
logging.getLogger(__name__).warning("xorq catalog add skipped: %s", exc)
387+
catalog_registered = False
388+
logging.getLogger(__name__).error("xorq catalog add raised for %s: %s", content_hash, exc)
389+
if catalog_registered is False:
390+
logging.getLogger(__name__).error(
391+
"xorq catalog add failed for %s — entry recipe is not durable in the catalog repo (#48)",
392+
content_hash,
393+
)
381394

382395
return BuildResult(
383396
content_hash=content_hash,
384397
entry_path=target,
385398
row_count=row_count,
386399
execute_seconds=execute_seconds,
387400
schema=schema_doc,
401+
catalog_registered=catalog_registered,
388402
)
389403

390404

tests/test_catalog_xorq_integration.py

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -194,16 +194,51 @@ def test_back_then_forward_restores_xorq_recipe(project, orders_parquet):
194194
def test_failed_registration_is_observable(project, orders_parquet):
195195
"""A failed xorq catalog add must be observable, not reported as success.
196196
197-
Entry 1 registers and (via the committed bookkeeping files) leaves the
198-
catalog inconsistent; the next add then fails ``assert_consistency``. Today
199-
``add_entry``'s failure is swallowed in a best-effort wrapper and its bool
200-
return is discarded, so ``build_and_persist`` exposes no signal. The loose,
201-
fix-shaped property: a ``catalog_registered``-style flag exists and is
202-
``False`` on failure."""
197+
Pre-fix this failure happened on its own (entry 1's checkpoint committed the
198+
bookkeeping files, leaving the catalog inconsistent so the next add fails) —
199+
that was bug #48 itself, and ``add_entry`` swallowed the failure with no
200+
signal. The fix removes that spontaneous failure, so to test the *signal* we
201+
inject the same kind of inconsistency a stray tracked file gives, then assert
202+
``build_and_persist`` reports ``catalog_registered is False`` rather than
203+
claiming success."""
203204
_make_entries(project, orders_parquet, 1)
205+
206+
# A tracked file outside xorq's managed set is exactly the shape #48 created:
207+
# assert_consistency then rejects the repo and the next add fails.
208+
cd = paths.catalog_dir(project)
209+
(cd / "stray.txt").write_text("not xorq-managed\n")
210+
_git(project, "add", "stray.txt")
211+
subprocess.run(
212+
["git", "-C", str(cd), "-c", "user.email=t@t", "-c", "user.name=t", "commit", "-q", "-m", "stray"],
213+
capture_output=True,
214+
text=True,
215+
)
216+
204217
result = build_and_persist(project, _agg_code(orders_parquet, _AGGS[1]))
205-
registered = getattr(result, "catalog_registered", None)
206-
assert registered is False, (
218+
assert result.catalog_registered is False, (
207219
"failed xorq catalog add must be observable: build_and_persist should expose "
208-
f"catalog_registered=False, got {registered!r}"
220+
f"catalog_registered=False, got {result.catalog_registered!r}"
221+
)
222+
223+
224+
# ---------------------------------------------------------------------------
225+
# Group E — reset rolls back alias state (regression guard shipping WITH the fix)
226+
# ---------------------------------------------------------------------------
227+
228+
229+
@pytest.mark.integration
230+
def test_reset_rolls_back_alias_state(project, orders_parquet):
231+
"""A reset must roll alias bookkeeping back, even though the files left the repo.
232+
233+
Pre-fix, ``aliases.json`` was git-tracked, so ``git reset --hard`` rolled it
234+
back for free. The #48 fix moves the bookkeeping out of the catalog repo, so
235+
``reset_to`` has to reconcile alias state from the catalog.yaml mirror
236+
instead — this guards that it does (it would silently regress otherwise)."""
237+
from tallyman_core import aliases as al
238+
239+
_make_entries(project, orders_parquet, 2) # creates aliases agg0, agg1
240+
assert set(al.load_aliases(project)) == {"agg0", "agg1"}
241+
cs.reset_to(project, 1) # back to the one-entry step
242+
assert set(al.load_aliases(project)) == {"agg0"}, (
243+
f"reset_to(step-1) did not roll alias state back: {al.load_aliases(project)}"
209244
)

tests/test_pack.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def test_pack_creates_tgz(project: str, orders_parquet: Path, isolated_home: Pat
3636

3737
with tarfile.open(out_path) as tar:
3838
names = tar.getnames()
39-
assert f"{project}/artifacts/catalog/aliases.json" in names
39+
assert f"{project}/artifacts/alias_state/aliases.json" in names
4040
assert f"{project}/data/orders.parquet" in names
4141
assert any(f"{project}/artifacts/catalog/entries/{h}/" in n for n in names)
4242

0 commit comments

Comments
 (0)