gabriel / musehub public
Closed #106
filed by gabriel human · 63 days ago

MWP-1: Make commit-graph generations authoritative (fixes clone-after-push staleness)

3 Anchors
305 Blast radius
19 Churn 30d
1 Proposals

MWP-1 — Make commit-graph generations authoritative (fixes clone-after-push staleness)

Sub-ticket of the MWP MVP master tracker: muse#58https://staging.musehub.ai/gabriel/muse/issues/58 (Formal issue links are not available yet; this issue and #58 cross-reference by URL.)

Background

muse clone has no dedicated endpoint. It is a full POST /fetch/mpack with have=[], served from the server-side prebuilt-mpack cache. The whole fetch path walks the commit DAG through one table — musehub_commit_graph — using a generation-bounded range scan, not the raw parent pointers. That makes the generation column load-bearing for correctness, not just for stats.

The bug

Both places that compute generations on push share the identical fallback:

musehub/services/musehub_wire_push.py:926 (inside wire_push_unpack_mpack, step 9b):

_gen = (max(_parent_gens) + 1) if _parent_gens else 0

musehub/services/musehub_wire_push.py:1188 (inside _build_commit_graph_from_raw):

gen = (max(parent_gens) + 1) if parent_gens else 0

This conflates two distinct cases:

  1. Genuine root commitparent_ids == []. Generation 0 is correct.
  2. Commit with parents whose generations could not be resolved — the parent is not in this mpack (it is already on the remote, excluded by the have set) and its musehub_commit_graph row was not returned by the lookup at musehub_wire_push.py:870-874 / :1170-1176. Generation 0 is wrong — it should be parent_generation + 1.

Case 2 silently writes a corrupt generation. An external parent can be missing from musehub_commit_graph for several real reasons: commits pushed before the graph table existed, a backfill gap, or a prior partial/failed push that wrote musehub_commits but not the graph row.

Why it breaks clone

The fetch walk _walk_commit_delta (musehub/services/musehub_wire_fetch.py:102-111):

.where(MusehubCommitGraph.generation > min_have_gen)
.where(MusehubCommitGraph.generation <= max_want_gen)

If the new tip was written with generation 0, then max_want_gen = 0, the scan excludes every real ancestor (generations 1..N), and the BFS truncates. The tip-snapshot resolver (musehub_wire_fetch.py:484-490, ORDER BY generation DESC LIMIT 1) then selects an older commit's snapshot. The assembled mpack carries the old tip's blobs but is cached against the new tip id. Clone returns HTTP 200 with a working tree missing the latest commits.

The fetch code already half-detects this — see the [BLOB-DEBUG] block at musehub_wire_fetch.py:495-509: "A mismatch means CommitGraph is stale for the newest push — the old snapshot's manifest_blob would be missing new blobs."

There is also no safety net: _walk_commit_delta pins if True: at musehub_wire_fetch.py:66, making the raw parent-pointer DAG walk (lines 159-184) dead code. A full DAG-walk fallback is tracked separately as MWP-2; this ticket makes generations authoritative at the source and repairs existing corruption, plus adds a minimal consistency guard.

Goal

  • A commit with non-empty parents never receives generation 0.
  • Both push-side generation computations resolve external parent generations authoritatively, backfilling musehub_commit_graph from musehub_commits (the source-of-truth DAG) when a parent row is missing.
  • Existing corrupt rows (generation 0 with non-empty parent_ids) are repaired.
  • A data-integrity invariant guards against regression.
  • The clone → push → clone staleness is proven gone by a non-skipped test.

Design

Distinguish root from unresolved (both sites)

if not pids:
    gen = 0                                  # genuine root — correct
else:
    resolved = _lookup_parent_gens(pids)     # inline_gen ∪ db_parent_gens
    if len(resolved) == len(pids):
        gen = max(resolved) + 1
    else:
        gen = await _resolve_generation_with_backfill(session, pids) + 1

_resolve_generation_with_backfill

For any parent whose generation is unknown, walk musehub_commits.parent_ids (the authoritative DAG) toward the root, computing generations bottom-up and upserting them into musehub_commit_graph. The walk is bounded: it stops at every commit that already has a graph generation (the common case after the first push), so steady-state cost is O(unresolved frontier), not O(history). If a parent commit does not exist in musehub_commits at all, raise a loud integrity error (a fast-forward push must not reference a commit the server lacks) rather than silently writing 0.

Minimal fetch-side guard (full fallback = MWP-2)

In the tip-snapshot resolver, when want_tip_snap_id (from the generation range-scan) disagrees with commit_rows[tip].snapshot_id (the authoritative row), log an error and repair the offending generation in-place before assembling the mpack, so a single stale row cannot ship a bad clone. The complete DAG-walk fallback for _walk_commit_delta is MWP-2.

Phases (load-bearing; each must be green before the next)

Phase 0 — Reproduce (red)

  • MWP1_01 Unit: _build_commit_graph_from_raw with a child whose only parent is absent from both the mpack and musehub_commit_graph → assert the child's generation is parent_gen + 1, not 0. RED (assert 0 == 3).
  • MWP1_02 Unit: genuine root commit (parent_ids == []) → generation 0 (guards against over-correction). GREEN.
  • MWP1_03 Integration: seed musehub_commits with C1←C2←C3 but only C1/C2 in musehub_commit_graph; push C4 (child of C3, C3 in have) → musehub_commit_graph[C4].generation == graph[C3].generation + 1. RED (server log confirms external_parents=1 found=0gen_max=0).

Phase 0 complete (test file tests/test_mwp1_generation_authority.py): the two reproduction tests are red for the right reason at both generation sites and the root-case guard is green. No production code changed yet — the fix lands in Phases 1–2.

Phase 1 — Distinguish root from unresolved

  • MWP1_04 Both generation sites separate pids == [] from pids != [] but unresolved; the unresolved branch logs a loud warning and is the single seam later phases hook. No behavior change to the root case.
  • MWP1_05 Unit asserts the unresolved branch is reached (spy/log) for the Phase 0 scenario.

Phase 1 complete (commit sha256:9fa9f5bd7edf): both generation sites now separate the root case (pids == [], gen 0 correct) from the unresolved case (pids != [] but generation lookup failed). The unresolved branch logs [MWP1] unresolved parent generations at <site> ... at WARNING level — the seam Phase 2 hooks into. MWP1_04 and MWP1_05 are GREEN; MWP1_01 and MWP1_03 remain RED (Phase 2 is the fix).

Phase 2 — Authoritative backfill

  • MWP1_06 Implement _resolve_generation_with_backfill(session, pids): bounded walk of musehub_commits.parent_ids, bottom-up generation compute, upsert into musehub_commit_graph. Shared by both call sites.
  • MWP1_07 Unit: missing parent chain of depth N is backfilled with correct generations 0..N in one push.
  • MWP1_08 Unit: a parent commit absent from musehub_commits raises a loud integrity error (no silent 0).
  • MWP1_09 Stress: backfill over a 1,000-commit missing chain completes and is bounded (stops at the first commit that already has a generation).

Phase 2 complete (commit sha256:e7097592983d): _resolve_generation_with_backfill is implemented and wired at both sites. The function walks musehub_commits.parent_ids toward roots, stops at any commit already in musehub_commit_graph (the anchor), computes generations bottom-up via Kahn's topo-sort, and bulk-upserts the new rows. Raises ValueError([MWP1] integrity error) for phantom parents. All 8 tests GREEN — including MWP1_01 and MWP1_03 which were RED through Phase 1 (the original clone-staleness reproduction tests).

Phase 3 — Repair existing corruption

  • MWP1_10 Idempotent repair routine recomputes generations for every commit with generation == 0 AND parent_ids != '{}', in topological order. Ships as an alembic data migration or a maintenance routine.
  • MWP1_11 Integration: a repo with pre-seeded corrupt rows is fully corrected; re-running the repair is a no-op.

Phase 3 complete (commit sha256:667089b438359d): repair_corrupt_commit_generations(session) queries the graph for rows with generation=0 AND non-empty parents, fetches non-corrupt parent generations as anchors, topo-sorts the corrupt set, recomputes bottom-up, and bulk-upserts only the generation column. Idempotent — second call returns total_corrupt=0 immediately. All 10 tests GREEN.

Phase 4 — Invariant + guard

  • MWP1_12 Data-integrity invariant: no musehub_commit_graph row has generation == 0 AND parent_ids != '{}' (assertable in CI / verify).
  • MWP1_13 Fetch-side guard: when want_tip_snap_idcommit_rows[tip].snapshot_id, log an error and repair the generation before assembling the mpack (cross-references MWP-2 for the full DAG-walk fallback).

Phase 4 complete (commit sha256:2e0d3cd2563e7f): check_commit_graph_invariant(session) queries WHERE generation=0 AND cardinality(parent_ids)>0 and returns {valid: bool, violations: int}. The fetch-side guard in wire_fetch_mpack detects when want_tip_snap_id (CommitGraph max-gen) disagrees with commit_rows[tip].snapshot_id (authoritative MusehubCommit), calls repair_corrupt_commit_generations, then re-queries the tip snapshot before assembling the mpack. All 12 tests GREEN.

Phase 5 — Regression proof

  • MWP1_14 End-to-end: push C1→C2→C3, clone, push C4, clone again → the second clone contains C4 and its blobs. This is the acceptance gate. (Broad suite re-enable is MWP-6.)

Phase 5 complete (commit sha256:1aca297eaa71ac): test_mwp1_14_end_to_end_push_clone_push_clone proves the staleness is gone. Setup: push C1/C2/C3 individually, delete C3 from CommitGraph (simulates the RC-1 backfill gap), push C4. With the fix, Phase 2 backfills C3=gen 2 and C4=gen 3. Clone with want=[C4], have=[] asserts all 4 commits (C1–C4) and C4's blob are in the assembled mpack. Without the fix, the range scan (gen ≤ 0) misses C2 and C3 — the assertion on C1/C2/C3 fails. All 13 MWP1 tests GREEN.

Acceptance criteria

  • No commit with non-empty parents is ever written with generation 0 (MWP1_04, MWP1_06, MWP1_12).
  • Pushing a child of a commit whose graph row is missing backfills the chain and assigns the correct generation (MWP1_03, MWP1_07).
  • Existing corrupt generations are repaired idempotently (MWP1_10, MWP1_11).
  • Clone → push → clone returns the new commits and blobs every time (MWP1_14).
  • A regression test (not skipped) codifies the clone-after-push contract.

Testing tiers

Tier Coverage
Unit generation math, root vs unresolved, backfill bounds
Integration DB-backed graph with missing/stale rows; repair idempotence
End-to-end push → clone → push → clone via the wire
Stress deep missing-parent chain backfill
Data integrity no-gen-0-with-parents invariant

Out of scope

  • The full DAG-walk fallback in _walk_commit_delta (removing the if True: dead code) — MWP-2.
  • Job-enqueue dedup on back-to-back pushes — MWP-3.
  • Prebuild/index ordering — MWP-4.
  • Client 503 retry — MWP-5.
  • Broad re-enable of the skipped wire suite — MWP-6.

Anchors

  • musehub/services/musehub_wire_push.py::wire_push_unpack_mpack (step 9b, line 926)
  • musehub/services/musehub_wire_push.py::_build_commit_graph_from_raw (line 1188)
  • musehub/services/musehub_wire_fetch.py::_walk_commit_delta (range-scan consumer)
Activity5
gabriel opened this issue 63 days ago
gabriel 63 days ago

Phase 0 complete (RED). Failing tests landed in tests/test_mwp1_generation_authority.py on branch task/mwp-1-generation-authority:

  • MWP1_01 RED — _build_commit_graph_from_raw assigns generation 0 instead of 3 when the parent is absent from both the mpack and the commit graph (assert 0 == 3).
  • MWP1_02 GREEN — genuine root commit correctly stays at generation 0 (guard).
  • MWP1_03 RED — wire_push_unpack_mpack step 9b assigns generation 0 to the new tip. Server log pinpoints the bug path: [step 9a] external_parents=1 found=0[step 9b] gen_max=0.

Both generation sites are now reproduced. No production code changed. Proceeding to Phase 1 (distinguish genuine root from unresolved parent) only on your go.

gabriel 63 days ago

Phase 1 complete

Commit: sha256:9fa9f5bd7edf

Both generation sites in musehub_wire_push.py now carry an explicit seam separating the two cases that the original else 0 conflated:

  • Genuine root (pids == []) → generation 0, no warning, no change.
  • Commit with parents but unresolved generations (pids != [] but lookup failed) → still falls back to 0 (Phase 2 fixes this), but now logs a loud [MWP1] unresolved parent generations at <site> WARNING identifying the commit id and the unresolved parent ids.

The seam is the precise hook Phase 2 will replace with _resolve_generation_with_backfill.

Test results:

  • MWP1_02 GREEN (root guard — not over-corrected)
  • MWP1_04 GREEN (_build_commit_graph_from_raw seam: root → no warning, unresolved → warning)
  • MWP1_05 GREEN (wire_push_unpack_mpack seam: push-path warning fires)
  • MWP1_01 RED (generation still 0 — Phase 2 is the fix)
  • MWP1_03 RED (backfill not yet implemented — Phase 2)
gabriel 63 days ago

Phase 2 complete

Commit: sha256:e7097592983d

What was added

_resolve_generation_with_backfill(session, pids, *, inline_gen, db_gen) in musehub_wire_push.py:

  1. Starts from inline_gen ∪ db_gen (already-known generations).
  2. BFS frontier = pids not yet in known.
  3. At each frontier step: check musehub_commit_graph — commits found there become anchors (stop the walk).
  4. For remaining frontier: fetch parent_ids from musehub_commits. Any commit absent from musehub_commits raises ValueError([MWP1] integrity error) — the fast-forward invariant requires the server to already have every referenced commit.
  5. Expand frontier with newly discovered parents.
  6. After walk: topo-sort (Kahn's algorithm) the discovered commits, compute generations bottom-up, bulk-upsert into musehub_commit_graph.
  7. Return max(gen for pid in pids). Caller adds +1.

Both call sites now await this instead of silently writing 0.

Test results — all 8 GREEN

Test Status What it proves
MWP1_01 ✅ GREEN (was RED) _build_commit_graph_from_raw backfills correctly
MWP1_02 ✅ GREEN root guard still untouched
MWP1_03 ✅ GREEN (was RED) push path: C3 backfilled to 2, C4=3
MWP1_04 ✅ GREEN seam warning still fires for unresolved case
MWP1_05 ✅ GREEN push-path seam warning fires
MWP1_07 ✅ GREEN 5-commit chain fully backfilled in one push
MWP1_08 ✅ GREEN absent parent raises ValueError
MWP1_09 ✅ GREEN 1,000-commit chain: C999=999, stops at C0 anchor

The RC-1 clone-staleness bug (generation 0 written for commits with parents) is now fixed at both generation sites.

gabriel 63 days ago

Phase 3 complete

Commit: sha256:667089b438359d

What was added

repair_corrupt_commit_generations(session) in musehub_wire_push.py:

Algorithm (5 steps):

  1. SELECT commit_id, parent_ids FROM musehub_commit_graph WHERE generation=0 AND cardinality(parent_ids)>0 — finds all RC-1 artefacts.
  2. Identifies external parents (parents NOT in the corrupt set) and fetches their graph generations as anchors.
  3. Falls back to _resolve_generation_with_backfill for any external parent missing from the graph entirely (shouldn't happen post-Phase-2, handled gracefully).
  4. Kahn's topo-sort on the corrupt set (corrupt parents before corrupt children). Recomputes correct generations bottom-up.
  5. Bulk-upsert with ON CONFLICT DO UPDATE SET generation=excluded.generation — only touches the generation column, all other fields preserved.

Idempotent: second call hits WHERE generation=0 AND cardinality(parent_ids)>0 — finds zero rows after the first repair — returns immediately.

Test results — all 10 GREEN

Test Phase Status
MWP1_01 0 ✅ GREEN
MWP1_02 0 ✅ GREEN
MWP1_03 0 ✅ GREEN
MWP1_04 1 ✅ GREEN
MWP1_05 1 ✅ GREEN
MWP1_07 2 ✅ GREEN
MWP1_08 2 ✅ GREEN
MWP1_09 2 ✅ GREEN
MWP1_10 3 ✅ GREEN — C2/C3/C4 (all gen=0, non-empty parents) repaired to 1/2/3; C1 (true root) untouched
MWP1_11 3 ✅ GREEN — second call returns total_corrupt=0, repaired=0

Phases 0–3 are now complete. Next: Phase 4 (invariant + fetch-side guard).

gabriel 63 days ago

All 6 phases complete and landed on dev + main.

What shipped:

  • Phase 0 (MWP1_01–05): Failing reproduction tests for the RC-1 staleness bug — RED gates.
  • Phase 1 (MWP1_06): Split the else 0 fallback into three distinct branches — root (genuine gen=0), all-resolved (max+1), and unresolved-seam (parent gen unknown, warns, defers to Phase 2).
  • Phase 2 (MWP1_07–09): _resolve_generation_with_backfill — bounded DAG walk of musehub_commits.parent_ids, Kahn's topo-sort, bulk upsert to musehub_commit_graph. Called whenever push encounters an unresolved parent.
  • Phase 3 (MWP1_10–11): repair_corrupt_commit_generations — idempotent repair of pre-existing RC-1 artefacts (rows with generation=0 and non-empty parent_ids).
  • Phase 4 (MWP1_12–13): check_commit_graph_invariant + fetch-side guard in wire_fetch_mpack — detects tip snapshot mismatch, calls repair in-place, re-queries corrected tip before assembling the clone mpack.
  • Phase 5 (MWP1_14): End-to-end regression proof — push C1→C2→C3, simulate RC-1 gap on C3, push C4 (backfill fires), clone with have=[] → all 4 commits + C4's blob in cloned mpack.

All 13 tests (MWP1_01–MWP1_14, MWP1_06 tracked by 07/08/09) GREEN at merge.

Landing commit: sha256:da49a05fd62cda46a7d73ec53a8d0adc5835d2070f6f0c51b12233a673c2e109 (local + staging dev + main).

closed this issue 63 days ago
Intelligence
7 direct dependents · 225 transitive
gravity: 0.32% of codebase
top callers: test_object_store_invariant_phase1.py AGENTS.md test_object_store_invariant_phase5.py test_mpack_byte_range.py backfill_loop.sh +227
17 modifications · 1 author · last touched Jun 13, 2026 gabriel