docs: plan failing integration tests for catalog↔xorq coexistence + reset

plans/catalog-xorq-integration-tests.md

@@ -0,0 +1,140 @@
+# Plan: failing integration tests for the catalog ↔ xorq coexistence and reset round-trips
+
+Status: **proposed — awaiting review.** Test-only PR. No production fix in this branch.
+
+## Why
+
+Issue #48 (aliases.json/alias_history.json committed into the xorq catalog repo →
+`assert_consistency` fails → every `xorq catalog add` after the first silently
+no-ops) shipped through all of V0 unnoticed. The reason it hid: the only
+integration test that exercises real `xorq catalog add`
+(`test_xorq_catalog_add_registers_after_genesis`) adds **exactly one** entry, so
+it never crosses the checkpoint that commits the two bookkeeping files. Every
+other reset/catalog test uses synthetic `entries/aaaa` dirs and never runs xorq
+at all.
+
+Reproduced both bugs against the real demo flow (`genesis → catalog_create →
+catalog_create → reset_to`), driven through the MCP tools so the checkpoint
+wrapper fires exactly as in production:
+
+1. **#48** — with two real entries, only entry 1's `entries/<hash>.zip` is
+   git-tracked; entry 2's add silently no-ops; `Catalog.from_kwargs(init=False)`
+   raises `AssertionError`.
+2. **reset ↔ xorq-catalog divergence (the second bug)** — a back→forward reset
+   round-trips the *tallyman* `entries/<hash>/` dir through the bullpen, but the
+   xorq `.zip` recipe is reconciled by a different mechanism (`git reset`) and is
+   never regained. The two views of "what entries exist" drift apart, and
+   `reset_to` never re-validates the xorq catalog. "Git-backed, recompute from
+   history" is false for any entry after the first.
+
+```
+[after both]   dirs=[023fb..,49b47..]  tracked_zips=[49b47..zip]   head=2
+[reset->s1]    dirs=[49b47..]          tracked_zips=[49b47..zip]   head=1
+[forward->s2]  dirs=[023fb..,49b47..]  tracked_zips=[49b47..zip]   head=2   # entry2 recipe never durable
+```
+
+## Scope
+
+- **Test-only.** This branch adds failing integration tests and nothing else. No
+  edits to `src/`. The #48 fix is a separate future PR.
+- Every test marked `@pytest.mark.integration` so it runs in the CI `integration`
+  job (visible failing on CI before any fix lands — satisfies the TDD rule).
+- Driven through the real surfaces: `tallyman_mcp.server.catalog_create` /
+  `catalog_revise` (which apply the checkpoint wrapper, the actual #48 trigger),
+  not synthetic dirs.
+
+## New file
+
+`tests/test_catalog_xorq_integration.py` — a cohesive suite. `test_reset_to_revision.py`
+is already 637 lines and its one related integration test
+(`test_xorq_catalog_add_registers_after_genesis`) stays where it is; the new
+multi-entry/reset-coexistence suite is more discoverable in its own file. Shared
+helpers (`_git`, `_open_xorq_catalog`, `_tracked_entry_zips`) live in the new file.
+
+## Helpers
+
+- `_git(project, *args)` — shell to `git -C <catalog_dir>` (the sanctioned test
+  exception to the no-bare-git guard, same as the existing reset suite).
+- `_tracked_entry_zips(project) -> set[str]` — `git ls-files entries/*.zip`,
+  stripped to bare hashes. The durable, content-addressed xorq artifact set.
+- `_open_xorq_catalog(project)` — `Catalog.from_kwargs(path=catalog_dir, init=False)`;
+  returns the catalog or re-raises so a test can assert it opens.
+- `_make_entries(project, n)` — drive `catalog_create` n times with distinct
+  aggregations over `orders_parquet`, returning the hashes/steps.
+
+## Tests — all fail today (this PR)
+
+### Group A — multi-entry catalog add (#48 core)
+
+1. `test_every_entry_registers_in_xorq_catalog`
+   Create 3 real entries via `catalog_create`. Assert every entry hash is in
+   `_tracked_entry_zips`. **Today:** only the first → fails.
+
+2. `test_catalog_openable_after_checkpoint`
+   genesis + one `catalog_create` (which checkpoints). Assert
+   `_open_xorq_catalog` does not raise. **Today:** `AssertionError` → fails.
+   Directly pins the `assert_consistency` root cause.
+
+3. `test_bookkeeping_files_not_tracked_in_xorq_repo`
+   Assert `git ls-files` contains neither `aliases.json` nor `alias_history.json`.
+   **Today:** both tracked → fails. This is the precise #48 invariant; the fix
+   (move them out of `catalog_dir`) makes it pass.
+
+### Group B — reset round-trips keep the xorq catalog consistent (second bug)
+
+4. `test_reset_keeps_xorq_catalog_consistent`
+   Build 2 entries, `reset_to` step-1, assert `_open_xorq_catalog` opens.
+   **Today:** unopenable → fails.
+
+5. `test_xorq_entry_set_matches_tallyman_pointers`
+   At each step, assert `_tracked_entry_zips(project)` == set of
+   `entry_hashes` in catalog.yaml. **Today:** diverges (1 zip vs 2 pointers) → fails.
+
+6. `test_back_then_forward_restores_xorq_recipe`
+   2 entries → reset back to step-1 → reset forward to step-2. Assert entry 2 is
+   durably present in the xorq catalog (tracked `.zip`), not only as a bullpen
+   dir copy. **Today:** zip never existed → fails. This is the divergence pinned
+   end to end.
+
+### Group D — silent-swallow surfacing (#48 secondary)
+
+7. `test_failed_registration_is_observable`
+   Force a failing `xorq catalog add` (run against the already-inconsistent
+   catalog) and assert the failure is observable rather than reported as success.
+   Proposed assertion: an ERROR-level log record is emitted (`caplog`) — matching
+   the issue's "log it at error level" suggested fix. **Today:** only WARNING in
+   the best-effort wrapper, and `build_and_persist` discards the return value →
+   no ERROR → fails.
+   *Open question for review:* keep this loose ("not silently swallowed" — assert
+   `build_and_persist` exposes a registration-failed signal) or specific (ERROR
+   log + `errors.jsonl` row)? The specific form is more fix-shaped; the loose
+   form pins the property without dictating the API. Default: loose, asserting a
+   `BuildResult.catalog_registered`-style signal exists and is `False` on failure.
+
+## Deferred to the fix PR (NOT in this branch)
+
+- `test_reset_rolls_back_alias_state` — **passes today** because `aliases.json` is
+  git-tracked and `git reset --hard` rolls it back. Once the #48 fix moves the
+  bookkeeping files out of the repo, reset stops rolling back alias state unless
+  the fix reconciles it explicitly. So this is a regression-guard that must ship
+  *with* the fix, per the rule that a test passing today is never bundled into a
+  failing-tests commit. Noted here so the fix PR doesn't forget it.
+
+## Branch / PR / CI flow
+
+- Branch `test/catalog-xorq-integration` off `main`.
+- Run `uv run pytest -m integration tests/test_catalog_xorq_integration.py`
+  locally first; confirm all Group A/B/D tests fail for the expected reasons
+  (assertion text, not collection/import errors).
+- Commit the failing tests as a single commit (all bugs, one commit — they share
+  the "seen failing on CI" axis).
+- Push `-u`, open PR, watch the CI `integration` job go red.
+- PR body: links #48, states this is the failing-test half of a TDD split, lists
+  which assertion pins which bug, and names the deferred alias-rollback guard.
+
+## Cost / risk
+
+- Each test runs real xorq builds (~3–15s each per the integration marker doc).
+  Entry counts kept at 2–3; ~7 tests → roughly 1–2 min added to the integration
+  job. Acceptable; integration job is already separate and not on the fast path.
+- No `src/` changes, so the fast suite and lint are untouched.

tests/test_catalog_xorq_integration.py

@@ -0,0 +1,209 @@
+"""catalog ↔ xorq coexistence across multi-entry adds and reset round-trips.
+
+Companion to ``test_reset_to_revision.py``. That suite drives synthetic
+``entries/aaaa`` dirs and never runs xorq; its one real-xorq integration test
+(``test_xorq_catalog_add_registers_after_genesis``) adds exactly *one* entry, so
+it never crosses the checkpoint that commits the bookkeeping files into the
+xorq catalog repo. That blind spot is how #48 shipped through all of V0.
+
+These tests reproduce the real demo flow (genesis → catalog_create →
+catalog_create → reset_to) through the production MCP surface, so the
+dispatch-boundary checkpoint wrapper fires exactly as it does in production —
+the actual #48 trigger. Two distinct bugs surface:
+
+1. **#48** — the checkpoint commits ``aliases.json``/``alias_history.json`` into
+   the xorq catalog repo, ``assert_consistency`` then fails, and every
+   ``xorq catalog add`` after the first silently no-ops (Group A, D).
+2. **reset ↔ xorq-catalog divergence** — a reset round-trips the tallyman
+   ``entries/<hash>/`` dir through the bullpen, but the durable xorq ``.zip``
+   recipe is reconciled by ``git reset`` and never regained. The two views of
+   "what entries exist" drift apart and ``reset_to`` never re-validates the
+   xorq catalog (Group B).
+
+All tests here fail today. They are the failing-test half of a TDD split; the
+#48 fix is a separate PR. See ``plans/catalog-xorq-integration-tests.md``.
+
+Like the reset suite, these shell out to git directly to assert on the catalog
+repo — the sanctioned exception to the no-bare-subprocess-git guard.
+"""
+
+from __future__ import annotations
+
+import subprocess
+
+import pytest
+
+from tallyman_core import catalog_state as cs
+from tallyman_core import paths
+from tallyman_xorq import build_and_persist
+
+# Distinct aggregations → distinct exprs → distinct xorq build hashes, so each
+# entry is a genuinely separate catalog add (not a content-hash re-run no-op).
+_AGGS = [
+    "total=t.price.sum(), n=t.count()",
+    "avg=t.price.mean()",
+    "mx=t.price.max()",
+    "mn=t.price.min()",
+]
+
+
+def _agg_code(parquet_path, agg: str) -> str:
+    return (
+        "import xorq.api as xo\n"
+        f"t = xo.deferred_read_parquet({str(parquet_path)!r})\n"
+        f"expr = t.group_by('region').aggregate({agg})\n"
+    )
+
+
+def _git(project: str, *args: str) -> str:
+    cd = paths.catalog_dir(project)
+    return subprocess.run(["git", "-C", str(cd), *args], capture_output=True, text=True).stdout.strip()
+
+
+def _tracked_entry_zips(project: str) -> set[str]:
+    """Bare hashes of the git-tracked ``entries/<hash>.zip`` recipes.
+
+    The durable, content-addressed xorq artifact set — what survives a clone or
+    a ``git reset``, as opposed to the untracked ``entries/<hash>/`` working
+    dirs the tallyman bullpen reconciles.
+    """
+    out = _git(project, "ls-files", "entries")
+    return {line[len("entries/") : -len(".zip")] for line in out.split() if line.endswith(".zip")}
+
+
+def _open_xorq_catalog(project: str):
+    """Open the project's xorq catalog with consistency checking on.
+
+    ``from_kwargs`` defaults ``check_consistency=True``, so this runs
+    ``assert_consistency`` — the exact check #48 breaks. Returns the catalog or
+    re-raises, letting a test assert it opens.
+    """
+    from xorq.catalog.catalog import Catalog
+
+    return Catalog.from_kwargs(path=paths.catalog_dir(project), init=False)
+
+
+def _make_entries(project: str, orders_parquet, n: int) -> list[str]:
+    """Drive ``catalog_create`` n times through the real MCP tool (so the
+    checkpoint wrapper fires), returning the content hashes in order."""
+    from tallyman_mcp.server import catalog_create
+
+    cs.genesis(project)  # branch-pinned, xorq-keyed catalog.yaml baseline
+    hashes: list[str] = []
+    for i in range(n):
+        res = catalog_create(name=f"agg{i}", code=_agg_code(orders_parquet, _AGGS[i]))
+        assert "error" not in res, f"catalog_create #{i} errored: {res}"
+        hashes.append(res["hash"])
+    return hashes
+
+
+# ---------------------------------------------------------------------------
+# Group A — multi-entry catalog add (#48 core)
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.integration
+def test_every_entry_registers_in_xorq_catalog(project, orders_parquet):
+    """Every entry's recipe must land in the xorq catalog, not just the first.
+
+    Today only entry 1's ``.zip`` is tracked: entry 1's checkpoint commits the
+    bookkeeping files, ``assert_consistency`` then fails, and entries 2+ add
+    silently no-op."""
+    hashes = _make_entries(project, orders_parquet, 3)
+    tracked = _tracked_entry_zips(project)
+    missing = [h for h in hashes if h not in tracked]
+    assert not missing, f"entries never registered in xorq catalog: {missing} (tracked: {tracked})"
+
+
+@pytest.mark.integration
+def test_catalog_openable_after_checkpoint(project, orders_parquet):
+    """The xorq catalog must stay openable after a checkpoint. Pins the
+    ``assert_consistency`` root cause: genesis + one ``catalog_create`` (which
+    checkpoints) already leaves it unopenable today."""
+    _make_entries(project, orders_parquet, 1)
+    try:
+        _open_xorq_catalog(project)
+    except AssertionError as exc:
+        pytest.fail(f"xorq catalog unopenable after checkpoint (#48 assert_consistency): {exc!r}")
+
+
+@pytest.mark.integration
+def test_bookkeeping_files_not_tracked_in_xorq_repo(project, orders_parquet):
+    """The precise #48 invariant: tallyman's alias bookkeeping must not be
+    git-tracked inside the xorq catalog repo. The checkpoint's ``_TRACKED`` set
+    commits both today, which is what breaks ``assert_consistency``. The fix
+    (move them out of ``catalog_dir``) makes this pass."""
+    _make_entries(project, orders_parquet, 1)
+    tracked = set(_git(project, "ls-files").split())
+    offenders = tracked & {"aliases.json", "alias_history.json"}
+    assert not offenders, f"tallyman bookkeeping tracked in xorq catalog repo: {sorted(offenders)}"
+
+
+# ---------------------------------------------------------------------------
+# Group B — reset round-trips keep the xorq catalog consistent (second bug)
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.integration
+def test_reset_keeps_xorq_catalog_consistent(project, orders_parquet):
+    """A reset must leave the xorq catalog openable. Today the reset target
+    (step-1) was itself checkpointed with the bookkeeping files tracked, so
+    ``git reset --hard`` restores an inconsistent tree and ``reset_to`` never
+    re-validates it."""
+    _make_entries(project, orders_parquet, 2)
+    cs.reset_to(project, 1)
+    try:
+        _open_xorq_catalog(project)
+    except AssertionError as exc:
+        pytest.fail(f"xorq catalog unopenable after reset_to(step-1): {exc!r}")
+
+
+@pytest.mark.integration
+def test_xorq_entry_set_matches_tallyman_pointers(project, orders_parquet):
+    """The durable xorq recipe set must match tallyman's ``entry_hashes``
+    pointers. Today they diverge — 1 tracked ``.zip`` against 2 pointers — the
+    visible signature of "git-backed, recompute from history" being false for
+    every entry after the first."""
+    _make_entries(project, orders_parquet, 2)
+    pointers = set(cs.read_tallyman_state(project)["entry_hashes"])
+    tracked = _tracked_entry_zips(project)
+    assert tracked == pointers, f"xorq tracked zips {tracked} diverge from tallyman pointers {pointers}"
+
+
+@pytest.mark.integration
+def test_back_then_forward_restores_xorq_recipe(project, orders_parquet):
+    """Reset back then forward must restore entry 2's *durable* xorq recipe,
+    not merely its bullpen dir copy. Today the ``.zip`` never existed (its add
+    no-op'd), so a forward reset cannot regain it — the divergence pinned end
+    to end."""
+    hashes = _make_entries(project, orders_parquet, 2)
+    cs.reset_to(project, 1)  # back to entry-1-only
+    cs.reset_to(project, 2)  # forward to both
+    entry2 = hashes[1]
+    assert entry2 in _tracked_entry_zips(project), (
+        f"entry 2 recipe {entry2} not durable in xorq catalog after back→forward reset"
+    )
+
+
+# ---------------------------------------------------------------------------
+# Group D — silent-swallow surfacing (#48 secondary)
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.integration
+def test_failed_registration_is_observable(project, orders_parquet):
+    """A failed xorq catalog add must be observable, not reported as success.
+
+    Entry 1 registers and (via the committed bookkeeping files) leaves the
+    catalog inconsistent; the next add then fails ``assert_consistency``. Today
+    ``add_entry``'s failure is swallowed in a best-effort wrapper and its bool
+    return is discarded, so ``build_and_persist`` exposes no signal. The loose,
+    fix-shaped property: a ``catalog_registered``-style flag exists and is
+    ``False`` on failure."""
+    _make_entries(project, orders_parquet, 1)
+    result = build_and_persist(project, _agg_code(orders_parquet, _AGGS[1]))
+    registered = getattr(result, "catalog_registered", None)
+    assert registered is False, (
+        "failed xorq catalog add must be observable: build_and_persist should expose "
+        f"catalog_registered=False, got {registered!r}"
+    )