gabriel / musehub public
fix BREAKING fix/proposal-snapshot-commit-naming #1 / 1
AI Agent gabriel · 6 hours ago · Sep 9, 2026 · Diff

fix: proposal from/to_snapshot_id held commit IDs, not snapshot IDs

Found while reviewing MP #9 against musehub#144: fromSnapshotId in the proposal API response is actually a commit ID -- confirmed by #0049's own docstring ('captures the HEAD commit ID of each branch') despite naming the columns from_snapshot_id/to_snapshot_id. musehub_proposals.create_proposal has always populated them from branch.head_commit_id. This is also why proposal_detail.html's 'from'/'to' anchor links to /commits/{id} worked at all -- they were unknowingly relying on the bug.

Fix: add from_commit_id/to_commit_id (holding what from/to_snapshot_id always actually held), and correct from_snapshot_id/to_snapshot_id to hold the real Snapshot (manifest) ID looked up from the anchor commit. Migration 0075 backfills the new columns from the existing (mislabeled) ones, then corrects the snapshot columns in place via a join against musehub_commits -- existing rows whose commit no longer resolves get a null snapshot_id rather than a stale commit ID masquerading as one.

Updated proposal_detail.html's anchor links to use from/to_commit_id (the correct field now that from/to_snapshot_id hold real snapshot IDs). ui_proposals.py's own from_snapshot_id/to_snapshot_id are unrelated local variables computed live from branch heads -- confirmed unaffected, left as-is. docs_muse_wire.html's from/to_snapshot_id + from/to_commit_id already correctly documents the unrelated MPatch format, which was already using this exact from/to_X_id + commit_id pairing convention correctly -- good precedent for this fix's naming.

Rewrote tests/test_proposal_snapshot_anchors.py: renamed assertions to fromCommitId/toCommitId matching corrected semantics, added coverage for the new snapshot lookup (both the real-commit hit case and the fixture-commit-doesn't-resolve miss case). 69 tests across proposals and snapshot-anchors pass with zero regressions.

sha256:8d9ff9953480a93c194c2c4dba18752f490344ba971a98d03c6250db172bebc3 sha
+23 ~7 −5 symbols
sha256:6b7f42ebb3dd66e11828caa034f40ee333113d1240769dab7eac880c6ae4b8d7 snapshot
+23
symbols added
~7
symbols modified
−5
symbols removed
0
dead code introduced
Semantic Changes 35 symbols
+ docs/
+ src/
+ tests/
+ tools/
+ branch_labels variable variable branch_labels L33–33
+ depends_on variable variable depends_on L34–34
+ down_revision variable variable down_revision L32–32
+ downgrade function function downgrade L79–86
+ Sequence import import Sequence L24–24
+ Union import import Union L24–24
+ annotations import import annotations L22–22
+ op import import op L26–26
+ sa import import sa L27–27
+ revision variable variable revision L31–31
+ upgrade function function upgrade L37–76
~ musehub/models/musehub.py .py 1 symbol modified
~ tests/test_proposal_snapshot_anchors.py .py 12 symbols added, 5 symbols removed, 3 symbols modified
test_from_snapshot_null_when_from_branch_empty function async_function test_from_snapshot_null_when_from_branch_empty L102–119
test_snapshot_anchors_frozen_at_creation_time function async_function test_snapshot_anchors_frozen_at_creation_time L215–250
test_snapshot_anchors_stored_when_both_branches_have_heads function async_function test_snapshot_anchors_stored_when_both_branches_have_heads L77–94
test_snapshot_fields_always_present_in_response function async_function test_snapshot_fields_always_present_in_response L152–168
test_to_snapshot_null_when_to_branch_empty function async_function test_to_snapshot_null_when_to_branch_empty L127–144
+ _SNAPSHOT_A variable variable _SNAPSHOT_A L117–117
+ _SNAPSHOT_B variable variable _SNAPSHOT_B L118–118
+ _push_branch_with_real_commit function async_function _push_branch_with_real_commit L90–112
+ MusehubCommit import import MusehubCommit L47–47
+ datetime import import datetime L41–41
+ timezone import import timezone L41–41
+ test_anchor_fields_always_present_in_response function async_function test_anchor_fields_always_present_in_response L237–255
+ test_commit_anchors_frozen_at_creation_time function async_function test_commit_anchors_frozen_at_creation_time L306–341
+ test_commit_anchors_stored_when_both_branches_have_heads function async_function test_commit_anchors_stored_when_both_branches_have_heads L126–146
+ test_from_commit_null_when_from_branch_empty function async_function test_from_commit_null_when_from_branch_empty L185–203
+ test_snapshot_ids_resolved_from_real_anchor_commits function async_function test_snapshot_ids_resolved_from_real_anchor_commits L154–177
+ test_to_commit_null_when_to_branch_empty function async_function test_to_commit_null_when_to_branch_empty L211–229
← Older Oldest on fix/proposal-snapshot-commit-naming
All commits
Newer → Latest on fix/proposal-snapshot-commit-naming

0 comments

No comments yet. Be the first to start the discussion.

To add a comment, use the Muse CLI: muse hub commit comment sha256:8d9ff9953480a93c194c2c4dba18752f490344ba971a98d03c6250db172bebc3 --body "your comment"