|
| 1 | +# Plan: failing integration tests for the catalog ↔ xorq coexistence and reset round-trips |
| 2 | + |
| 3 | +Status: **proposed — awaiting review.** Test-only PR. No production fix in this branch. |
| 4 | + |
| 5 | +## Why |
| 6 | + |
| 7 | +Issue #48 (aliases.json/alias_history.json committed into the xorq catalog repo → |
| 8 | +`assert_consistency` fails → every `xorq catalog add` after the first silently |
| 9 | +no-ops) shipped through all of V0 unnoticed. The reason it hid: the only |
| 10 | +integration test that exercises real `xorq catalog add` |
| 11 | +(`test_xorq_catalog_add_registers_after_genesis`) adds **exactly one** entry, so |
| 12 | +it never crosses the checkpoint that commits the two bookkeeping files. Every |
| 13 | +other reset/catalog test uses synthetic `entries/aaaa` dirs and never runs xorq |
| 14 | +at all. |
| 15 | + |
| 16 | +Reproduced both bugs against the real demo flow (`genesis → catalog_create → |
| 17 | +catalog_create → reset_to`), driven through the MCP tools so the checkpoint |
| 18 | +wrapper fires exactly as in production: |
| 19 | + |
| 20 | +1. **#48** — with two real entries, only entry 1's `entries/<hash>.zip` is |
| 21 | + git-tracked; entry 2's add silently no-ops; `Catalog.from_kwargs(init=False)` |
| 22 | + raises `AssertionError`. |
| 23 | +2. **reset ↔ xorq-catalog divergence (the second bug)** — a back→forward reset |
| 24 | + round-trips the *tallyman* `entries/<hash>/` dir through the bullpen, but the |
| 25 | + xorq `.zip` recipe is reconciled by a different mechanism (`git reset`) and is |
| 26 | + never regained. The two views of "what entries exist" drift apart, and |
| 27 | + `reset_to` never re-validates the xorq catalog. "Git-backed, recompute from |
| 28 | + history" is false for any entry after the first. |
| 29 | + |
| 30 | +``` |
| 31 | +[after both] dirs=[023fb..,49b47..] tracked_zips=[49b47..zip] head=2 |
| 32 | +[reset->s1] dirs=[49b47..] tracked_zips=[49b47..zip] head=1 |
| 33 | +[forward->s2] dirs=[023fb..,49b47..] tracked_zips=[49b47..zip] head=2 # entry2 recipe never durable |
| 34 | +``` |
| 35 | + |
| 36 | +## Scope |
| 37 | + |
| 38 | +- **Test-only.** This branch adds failing integration tests and nothing else. No |
| 39 | + edits to `src/`. The #48 fix is a separate future PR. |
| 40 | +- Every test marked `@pytest.mark.integration` so it runs in the CI `integration` |
| 41 | + job (visible failing on CI before any fix lands — satisfies the TDD rule). |
| 42 | +- Driven through the real surfaces: `tallyman_mcp.server.catalog_create` / |
| 43 | + `catalog_revise` (which apply the checkpoint wrapper, the actual #48 trigger), |
| 44 | + not synthetic dirs. |
| 45 | + |
| 46 | +## New file |
| 47 | + |
| 48 | +`tests/test_catalog_xorq_integration.py` — a cohesive suite. `test_reset_to_revision.py` |
| 49 | +is already 637 lines and its one related integration test |
| 50 | +(`test_xorq_catalog_add_registers_after_genesis`) stays where it is; the new |
| 51 | +multi-entry/reset-coexistence suite is more discoverable in its own file. Shared |
| 52 | +helpers (`_git`, `_open_xorq_catalog`, `_tracked_entry_zips`) live in the new file. |
| 53 | + |
| 54 | +## Helpers |
| 55 | + |
| 56 | +- `_git(project, *args)` — shell to `git -C <catalog_dir>` (the sanctioned test |
| 57 | + exception to the no-bare-git guard, same as the existing reset suite). |
| 58 | +- `_tracked_entry_zips(project) -> set[str]` — `git ls-files entries/*.zip`, |
| 59 | + stripped to bare hashes. The durable, content-addressed xorq artifact set. |
| 60 | +- `_open_xorq_catalog(project)` — `Catalog.from_kwargs(path=catalog_dir, init=False)`; |
| 61 | + returns the catalog or re-raises so a test can assert it opens. |
| 62 | +- `_make_entries(project, n)` — drive `catalog_create` n times with distinct |
| 63 | + aggregations over `orders_parquet`, returning the hashes/steps. |
| 64 | + |
| 65 | +## Tests — all fail today (this PR) |
| 66 | + |
| 67 | +### Group A — multi-entry catalog add (#48 core) |
| 68 | + |
| 69 | +1. `test_every_entry_registers_in_xorq_catalog` |
| 70 | + Create 3 real entries via `catalog_create`. Assert every entry hash is in |
| 71 | + `_tracked_entry_zips`. **Today:** only the first → fails. |
| 72 | + |
| 73 | +2. `test_catalog_openable_after_checkpoint` |
| 74 | + genesis + one `catalog_create` (which checkpoints). Assert |
| 75 | + `_open_xorq_catalog` does not raise. **Today:** `AssertionError` → fails. |
| 76 | + Directly pins the `assert_consistency` root cause. |
| 77 | + |
| 78 | +3. `test_bookkeeping_files_not_tracked_in_xorq_repo` |
| 79 | + Assert `git ls-files` contains neither `aliases.json` nor `alias_history.json`. |
| 80 | + **Today:** both tracked → fails. This is the precise #48 invariant; the fix |
| 81 | + (move them out of `catalog_dir`) makes it pass. |
| 82 | + |
| 83 | +### Group B — reset round-trips keep the xorq catalog consistent (second bug) |
| 84 | + |
| 85 | +4. `test_reset_keeps_xorq_catalog_consistent` |
| 86 | + Build 2 entries, `reset_to` step-1, assert `_open_xorq_catalog` opens. |
| 87 | + **Today:** unopenable → fails. |
| 88 | + |
| 89 | +5. `test_xorq_entry_set_matches_tallyman_pointers` |
| 90 | + At each step, assert `_tracked_entry_zips(project)` == set of |
| 91 | + `entry_hashes` in catalog.yaml. **Today:** diverges (1 zip vs 2 pointers) → fails. |
| 92 | + |
| 93 | +6. `test_back_then_forward_restores_xorq_recipe` |
| 94 | + 2 entries → reset back to step-1 → reset forward to step-2. Assert entry 2 is |
| 95 | + durably present in the xorq catalog (tracked `.zip`), not only as a bullpen |
| 96 | + dir copy. **Today:** zip never existed → fails. This is the divergence pinned |
| 97 | + end to end. |
| 98 | + |
| 99 | +### Group D — silent-swallow surfacing (#48 secondary) |
| 100 | + |
| 101 | +7. `test_failed_registration_is_observable` |
| 102 | + Force a failing `xorq catalog add` (run against the already-inconsistent |
| 103 | + catalog) and assert the failure is observable rather than reported as success. |
| 104 | + Proposed assertion: an ERROR-level log record is emitted (`caplog`) — matching |
| 105 | + the issue's "log it at error level" suggested fix. **Today:** only WARNING in |
| 106 | + the best-effort wrapper, and `build_and_persist` discards the return value → |
| 107 | + no ERROR → fails. |
| 108 | + *Open question for review:* keep this loose ("not silently swallowed" — assert |
| 109 | + `build_and_persist` exposes a registration-failed signal) or specific (ERROR |
| 110 | + log + `errors.jsonl` row)? The specific form is more fix-shaped; the loose |
| 111 | + form pins the property without dictating the API. Default: loose, asserting a |
| 112 | + `BuildResult.catalog_registered`-style signal exists and is `False` on failure. |
| 113 | + |
| 114 | +## Deferred to the fix PR (NOT in this branch) |
| 115 | + |
| 116 | +- `test_reset_rolls_back_alias_state` — **passes today** because `aliases.json` is |
| 117 | + git-tracked and `git reset --hard` rolls it back. Once the #48 fix moves the |
| 118 | + bookkeeping files out of the repo, reset stops rolling back alias state unless |
| 119 | + the fix reconciles it explicitly. So this is a regression-guard that must ship |
| 120 | + *with* the fix, per the rule that a test passing today is never bundled into a |
| 121 | + failing-tests commit. Noted here so the fix PR doesn't forget it. |
| 122 | + |
| 123 | +## Branch / PR / CI flow |
| 124 | + |
| 125 | +- Branch `test/catalog-xorq-integration` off `main`. |
| 126 | +- Run `uv run pytest -m integration tests/test_catalog_xorq_integration.py` |
| 127 | + locally first; confirm all Group A/B/D tests fail for the expected reasons |
| 128 | + (assertion text, not collection/import errors). |
| 129 | +- Commit the failing tests as a single commit (all bugs, one commit — they share |
| 130 | + the "seen failing on CI" axis). |
| 131 | +- Push `-u`, open PR, watch the CI `integration` job go red. |
| 132 | +- PR body: links #48, states this is the failing-test half of a TDD split, lists |
| 133 | + which assertion pins which bug, and names the deferred alias-rollback guard. |
| 134 | + |
| 135 | +## Cost / risk |
| 136 | + |
| 137 | +- Each test runs real xorq builds (~3–15s each per the integration marker doc). |
| 138 | + Entry counts kept at 2–3; ~7 tests → roughly 1–2 min added to the integration |
| 139 | + job. Acceptable; integration job is already separate and not on the fast path. |
| 140 | +- No `src/` changes, so the fast suite and lint are untouched. |
0 commit comments