gabriel / musehub public
mwp-1-generation-authority.md markdown
247 lines 11.8 KB
Raw
sha256:0032d6cfa33bc3c8367436ad768e7dd0e339b4332153160247da8266cb5fa352 Merge branch 'task/version-tags-phase3-server' into dev Human 17 days ago

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

Sub-ticket of the MWP MVP master tracker: muse#58 — https://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)

  • [x] 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).
  • [x] MWP1_02 Unit: genuine root commit (parent_ids == []) → generation 0 (guards against over-correction). GREEN.
  • [x] 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

  • [x] 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.
  • [x] 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

  • [x] 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.
  • [x] MWP1_07 Unit: missing parent chain of depth N is backfilled with correct generations 0..N in one push.
  • [x] MWP1_08 Unit: a parent commit absent from musehub_commits raises a loud integrity error (no silent 0).
  • [x] 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

  • [x] 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.
  • [x] 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

  • [x] MWP1_12 Data-integrity invariant: no musehub_commit_graph row has generation == 0 AND parent_ids != '{}' (assertable in CI / verify).
  • [x] 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

  • [x] 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

  • [x] No commit with non-empty parents is ever written with generation 0 (MWP1_04, MWP1_06, MWP1_12).
  • [x] Pushing a child of a commit whose graph row is missing backfills the chain and assigns the correct generation (MWP1_03, MWP1_07).
  • [x] Existing corrupt generations are repaired idempotently (MWP1_10, MWP1_11).
  • [x] Clone → push → clone returns the new commits and blobs every time (MWP1_14).
  • [x] 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)
File History 1 commit
sha256:0032d6cfa33bc3c8367436ad768e7dd0e339b4332153160247da8266cb5fa352 Merge branch 'task/version-tags-phase3-server' into dev Human 17 days ago