test_mwp2_walk_fallback.py
python
sha256:4dd2a937f66f8e36d9aa59bd1bf3cb880ca1d503fef67300a0cba3b482784081
test(mwp2): Phase 0 RED — reproduction tests for _walk_comm…
Sonnet 4.6
26 days ago
| 1 | """MWP-2 Phase 0 (RED) — _walk_commit_delta correctness fallback. |
| 2 | |
| 3 | Sub-ticket: musehub#107 — https://staging.musehub.ai/gabriel/musehub/issues/107 |
| 4 | Master: muse#58 — https://staging.musehub.ai/gabriel/muse/issues/58 |
| 5 | Predecessor: musehub#106 (MWP-1, generation authority) — closed. |
| 6 | |
| 7 | Root cause (RC-2): ``_walk_commit_delta`` (musehub_wire_fetch.py:49) pins |
| 8 | ``if True:`` at line 66, making the authoritative MusehubCommit DAG walk (lines |
| 9 | 159–184) dead code. The live path is a generation-bounded range scan: |
| 10 | |
| 11 | generation > min_have_gen AND generation <= max_want_gen |
| 12 | |
| 13 | This is correct only when every generation in ``musehub_commit_graph`` is |
| 14 | correct. When a generation is wrong or a graph row is missing, the range scan |
| 15 | either excludes real ancestors (corrupt low generation causes max_want_gen to |
| 16 | understate the range) or the BFS dead-ends (missing graph row yields empty |
| 17 | parents from ``graph_map.get(cid, ([], None, 0))``). Either way |
| 18 | ``_walk_commit_delta`` silently returns a truncated commit set, the assembled |
| 19 | mpack is missing commits and blobs, and clone exits 0 with a stale working tree. |
| 20 | |
| 21 | MWP-1 (musehub#106) fixed the write side: no new commit is ever written with a |
| 22 | wrong generation, and ``repair_corrupt_commit_generations`` heals pre-existing |
| 23 | artefacts. MWP-2 fixes the read side: ``_walk_commit_delta`` must detect that |
| 24 | its BFS is incomplete and fall back to the authoritative parent-pointer walk, |
| 25 | so the *current* response is always complete regardless of graph state. |
| 26 | |
| 27 | Expected Phase 0 status: |
| 28 | - MWP2_01 RED — corrupt generation excludes ancestors; truncated result |
| 29 | - MWP2_02 RED — missing graph row dead-ends BFS; missing commits in result |
| 30 | - MWP2_03 GREEN — fully consistent graph uses fast path; no fallback triggered |
| 31 | |
| 32 | The fix (Phases 1–2) adds a ``graph_incomplete`` flag to the BFS and calls a |
| 33 | promoted ``_walk_commit_delta_dag`` helper when the flag is set. |
| 34 | """ |
| 35 | from __future__ import annotations |
| 36 | |
| 37 | import datetime |
| 38 | import logging |
| 39 | from unittest.mock import AsyncMock, MagicMock, patch |
| 40 | |
| 41 | import pytest |
| 42 | from sqlalchemy import delete as _sa_delete, select |
| 43 | from sqlalchemy.ext.asyncio import AsyncSession |
| 44 | |
| 45 | from muse.core.ids import hash_snapshot |
| 46 | from muse.core.types import blob_id |
| 47 | from musehub.db.musehub_repo_models import ( |
| 48 | MusehubCommit, |
| 49 | MusehubCommitGraph, |
| 50 | ) |
| 51 | from musehub.services.musehub_wire_fetch import _walk_commit_delta |
| 52 | |
| 53 | _OWNER = "gabriel" |
| 54 | |
| 55 | |
| 56 | # --------------------------------------------------------------------------- |
| 57 | # Helpers — mirrors test_mwp1_generation_authority.py conventions |
| 58 | # --------------------------------------------------------------------------- |
| 59 | |
| 60 | |
| 61 | def _cid(seed: str) -> str: |
| 62 | """Deterministic sha256: commit id from a seed string.""" |
| 63 | return blob_id(f"mwp2-commit-{seed}".encode()) |
| 64 | |
| 65 | |
| 66 | def _sid(seed: str) -> str: |
| 67 | """Deterministic snapshot id.""" |
| 68 | return hash_snapshot({f"{seed}.txt": blob_id(seed.encode())}) |
| 69 | |
| 70 | |
| 71 | def _now() -> datetime.datetime: |
| 72 | return datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc) |
| 73 | |
| 74 | |
| 75 | async def _seed_commit( |
| 76 | session: AsyncSession, |
| 77 | cid: str, |
| 78 | parent_ids: list[str], |
| 79 | snap_id: str, |
| 80 | ) -> None: |
| 81 | """Insert a musehub_commits row (authoritative DAG) with no graph row.""" |
| 82 | session.add(MusehubCommit( |
| 83 | commit_id=cid, |
| 84 | branch="main", |
| 85 | message=f"commit {cid[:12]}", |
| 86 | author=_OWNER, |
| 87 | timestamp=_now(), |
| 88 | parent_ids=parent_ids, |
| 89 | snapshot_id=snap_id, |
| 90 | )) |
| 91 | await session.flush() |
| 92 | |
| 93 | |
| 94 | async def _seed_graph( |
| 95 | session: AsyncSession, |
| 96 | cid: str, |
| 97 | parent_ids: list[str], |
| 98 | generation: int, |
| 99 | snap_id: str, |
| 100 | ) -> None: |
| 101 | """Insert a musehub_commit_graph row with an explicit (possibly corrupt) generation.""" |
| 102 | session.add(MusehubCommitGraph( |
| 103 | commit_id=cid, |
| 104 | parent_ids=parent_ids, |
| 105 | generation=generation, |
| 106 | snapshot_id=snap_id, |
| 107 | created_at=_now(), |
| 108 | )) |
| 109 | await session.flush() |
| 110 | |
| 111 | |
| 112 | # --------------------------------------------------------------------------- |
| 113 | # MWP2_01 — corrupt generation excludes ancestors from range scan (RED) |
| 114 | # --------------------------------------------------------------------------- |
| 115 | |
| 116 | |
| 117 | @pytest.mark.asyncio |
| 118 | async def test_mwp2_01_corrupt_generation_truncates_walk( |
| 119 | db_session: AsyncSession, |
| 120 | ) -> None: |
| 121 | """RED: a missing interior graph row dead-ends the BFS, dropping ancestors. |
| 122 | |
| 123 | Chain: C1 <- C2 <- C3 <- C4 (all in musehub_commits, authoritative). |
| 124 | Graph: C1(gen=0), C2(gen=1), C4(gen=3) — C3 has NO graph row. |
| 125 | want=[C4], have=[]. |
| 126 | |
| 127 | BFS: C4 is in graph_map (gen=3, parents=[C3]). C3 → graph_map.get(C3) |
| 128 | returns default ([], None, 0) → BFS treats C3 as a leaf and stops. |
| 129 | C2 and C1 are never added to the reachable set. |
| 130 | |
| 131 | Result without fix: {C4, C3} — C2 and C1 missing. |
| 132 | Result with fix: {C4, C3, C2, C1}. |
| 133 | |
| 134 | This is the canonical post-MWP-1 RC-2 scenario: the tip has a correct high |
| 135 | generation, but an interior ancestor's graph row was never written (a push |
| 136 | that wrote musehub_commits but not the graph row, or a pre-backfill gap). |
| 137 | MWP2_02 covers the case where the *start tip itself* is absent from the graph. |
| 138 | """ |
| 139 | c1 = _cid("01-c1") |
| 140 | c2 = _cid("01-c2") |
| 141 | c3 = _cid("01-c3") |
| 142 | c4 = _cid("01-c4") |
| 143 | s1, s2, s3, s4 = _sid("01-s1"), _sid("01-s2"), _sid("01-s3"), _sid("01-s4") |
| 144 | |
| 145 | # Authoritative DAG — all four commits exist in musehub_commits. |
| 146 | await _seed_commit(db_session, c1, [], s1) |
| 147 | await _seed_commit(db_session, c2, [c1], s2) |
| 148 | await _seed_commit(db_session, c3, [c2], s3) |
| 149 | await _seed_commit(db_session, c4, [c3], s4) |
| 150 | |
| 151 | # Graph: C1, C2 present with correct generations; C3 absent (the backfill |
| 152 | # gap that produces the BFS dead-end); C4 present with correct generation |
| 153 | # relative to the true chain (gen=3). |
| 154 | await _seed_graph(db_session, c1, [], 0, s1) |
| 155 | await _seed_graph(db_session, c2, [c1], 1, s2) |
| 156 | # C3 deliberately NOT seeded in graph. |
| 157 | await _seed_graph(db_session, c4, [c3], 3, s4) |
| 158 | await db_session.commit() |
| 159 | |
| 160 | result = await _walk_commit_delta(db_session, want=[c4], have=[]) |
| 161 | |
| 162 | assert c4 in result, "C4 (want tip) must always be in result" |
| 163 | assert c3 in result, ( |
| 164 | f"C3 must be in result — it is a direct parent of C4. " |
| 165 | f"It is missing because graph_map.get(C3) returned empty parents " |
| 166 | f"(C3 has no graph row), so the BFS dead-ended at C3. " |
| 167 | f"Got result keys: {sorted(result.keys())}" |
| 168 | ) |
| 169 | assert c2 in result, ( |
| 170 | f"C2 must be in result — ancestor of C4 reachable via C3. " |
| 171 | f"Got result keys: {sorted(result.keys())}" |
| 172 | ) |
| 173 | assert c1 in result, ( |
| 174 | f"C1 must be in result — root ancestor of C4. " |
| 175 | f"Got result keys: {sorted(result.keys())}" |
| 176 | ) |
| 177 | |
| 178 | |
| 179 | # --------------------------------------------------------------------------- |
| 180 | # MWP2_02 — missing graph row for the start tip dead-ends BFS (RED) |
| 181 | # --------------------------------------------------------------------------- |
| 182 | |
| 183 | |
| 184 | @pytest.mark.asyncio |
| 185 | async def test_mwp2_02_missing_start_tip_graph_row( |
| 186 | db_session: AsyncSession, |
| 187 | ) -> None: |
| 188 | """RED: when the start tip (a want commit) has no graph row at all, the |
| 189 | BFS dead-ends immediately and returns an empty or incomplete set. |
| 190 | |
| 191 | Setup: |
| 192 | musehub_commits: C1(root) <- C2 <- C3 |
| 193 | musehub_commit_graph: C1(gen=0), C2(gen=1) — C3 has NO graph row. |
| 194 | want=[C3], have=[]. |
| 195 | |
| 196 | max_want_gen: C3 not in graph → query returns None → max_want_gen = 0. |
| 197 | Range scan: gen > -1 AND gen <= 0 → only C1 (gen=0) returned in graph_map. |
| 198 | BFS: starts from C3; C3 not in graph_map → treated as having empty parents; |
| 199 | BFS adds C3 to reachable but never follows its parents (C2, C1). |
| 200 | Result without fix: {C3} only. Missing: C2, C1. |
| 201 | Result with fix: {C3, C2, C1}. |
| 202 | """ |
| 203 | c1 = _cid("02-c1") |
| 204 | c2 = _cid("02-c2") |
| 205 | c3 = _cid("02-c3") |
| 206 | s1, s2, s3 = _sid("02-s1"), _sid("02-s2"), _sid("02-s3") |
| 207 | |
| 208 | # Authoritative DAG. |
| 209 | await _seed_commit(db_session, c1, [], s1) |
| 210 | await _seed_commit(db_session, c2, [c1], s2) |
| 211 | await _seed_commit(db_session, c3, [c2], s3) |
| 212 | |
| 213 | # Graph: C1 and C2 present; C3 (the want tip) absent. |
| 214 | await _seed_graph(db_session, c1, [], 0, s1) |
| 215 | await _seed_graph(db_session, c2, [c1], 1, s2) |
| 216 | await db_session.commit() |
| 217 | |
| 218 | result = await _walk_commit_delta(db_session, want=[c3], have=[]) |
| 219 | |
| 220 | assert c3 in result, "C3 (want tip) must be in result" |
| 221 | assert c2 in result, ( |
| 222 | f"C2 must be in result — parent of C3 (the want tip). " |
| 223 | f"C3 had no graph row, so max_want_gen=0 and the range scan only " |
| 224 | f"returned C1. BFS from C3 found no parents in graph_map. " |
| 225 | f"Got result keys: {sorted(result.keys())}" |
| 226 | ) |
| 227 | assert c1 in result, ( |
| 228 | f"C1 must be in result — root ancestor reachable from C3. " |
| 229 | f"Got result keys: {sorted(result.keys())}" |
| 230 | ) |
| 231 | |
| 232 | |
| 233 | # --------------------------------------------------------------------------- |
| 234 | # MWP2_03 — consistent graph uses fast path, no fallback invoked (GREEN) |
| 235 | # --------------------------------------------------------------------------- |
| 236 | |
| 237 | |
| 238 | @pytest.mark.asyncio |
| 239 | async def test_mwp2_03_consistent_graph_uses_fast_path( |
| 240 | db_session: AsyncSession, |
| 241 | ) -> None: |
| 242 | """GREEN guard: a fully consistent graph must return the complete ancestor |
| 243 | closure via the fast path, and ``_walk_commit_delta_dag`` must NOT be called. |
| 244 | |
| 245 | This test pins the happy-path contract so later phases cannot accidentally |
| 246 | make the fast path degrade into always calling the fallback (which would be |
| 247 | correct but slow). |
| 248 | |
| 249 | After Phase 2 the ``_walk_commit_delta_dag`` helper will exist; this test |
| 250 | patches it to detect if it is invoked. Before Phase 1 it doesn't exist, so |
| 251 | we patch ``musehub.graph.walk.walk_dag_async`` (the underlying function the |
| 252 | legacy walk calls) as a proxy — if the fast path is truly live and exclusive, |
| 253 | walk_dag_async should never be called during _walk_commit_delta. |
| 254 | |
| 255 | Note: after Phase 1 promotes _walk_commit_delta_dag to a real named function, |
| 256 | this test should be updated to patch that directly. For Phase 0 we patch |
| 257 | walk_dag_async as the canary. |
| 258 | |
| 259 | Setup: |
| 260 | musehub_commits: C1(root) <- C2 <- C3 |
| 261 | musehub_commit_graph: C1(gen=0), C2(gen=1), C3(gen=2) — fully consistent. |
| 262 | want=[C3], have=[]. |
| 263 | |
| 264 | Expected: result contains C1, C2, C3 and walk_dag_async is not called. |
| 265 | """ |
| 266 | c1 = _cid("03-c1") |
| 267 | c2 = _cid("03-c2") |
| 268 | c3 = _cid("03-c3") |
| 269 | s1, s2, s3 = _sid("03-s1"), _sid("03-s2"), _sid("03-s3") |
| 270 | |
| 271 | await _seed_commit(db_session, c1, [], s1) |
| 272 | await _seed_commit(db_session, c2, [c1], s2) |
| 273 | await _seed_commit(db_session, c3, [c2], s3) |
| 274 | |
| 275 | await _seed_graph(db_session, c1, [], 0, s1) |
| 276 | await _seed_graph(db_session, c2, [c1], 1, s2) |
| 277 | await _seed_graph(db_session, c3, [c2], 2, s3) |
| 278 | await db_session.commit() |
| 279 | |
| 280 | with patch( |
| 281 | "musehub.graph.walk.walk_dag_async", |
| 282 | wraps=None, |
| 283 | new_callable=MagicMock, |
| 284 | ) as mock_dag_walk: |
| 285 | # Make it an AsyncMock that returns an empty async iterator if called. |
| 286 | async def _never_called(*args, **kwargs): |
| 287 | pytest.fail( |
| 288 | "_walk_commit_delta invoked the DAG walk (walk_dag_async) on a " |
| 289 | "fully consistent graph — the fast path must be used exclusively." |
| 290 | ) |
| 291 | |
| 292 | mock_dag_walk.side_effect = _never_called |
| 293 | |
| 294 | result = await _walk_commit_delta(db_session, want=[c3], have=[]) |
| 295 | |
| 296 | assert c3 in result, f"C3 must be in result; got {sorted(result.keys())}" |
| 297 | assert c2 in result, f"C2 must be in result; got {sorted(result.keys())}" |
| 298 | assert c1 in result, f"C1 must be in result; got {sorted(result.keys())}" |
File History
1 commit
sha256:4dd2a937f66f8e36d9aa59bd1bf3cb880ca1d503fef67300a0cba3b482784081
test(mwp2): Phase 0 RED — reproduction tests for _walk_comm…
Sonnet 4.6
26 days ago