gabriel / muse public
Closed #84 Bug
filed by gabriel human · 108 days ago

bug: muse blame attributes lines to a commit that never touched the file

0 Anchors
— Blast radius
— Churn 30d
0 Proposals

Summary

muse blame <file> --range attributes lines to a commit whose own files_modified list does not include that file, and whose blobs for the file are byte-identical to its parent — i.e. the blamed commit provably never touched the file.

Repro

cd ~/ecosystem/musehub
muse blame src/scss/_layout.scss --range 38-41 --json

Returns attribution to sha256:659f09b2a6570076b0ec01897c95ce391467b1a11fb8fa6d3c12e88b2e0e1c77 ("fix: update all list_domains/list_repos callers to cursor pagination API").

muse read sha256:659f09b2a6570076b0ec01897c95ce391467b1a11fb8fa6d3c12e88b2e0e1c77 --json \
  | jq '.files_modified'

[
  "musehub/api/routes/musehub/ui_domains.py",
  "musehub/mcp/dispatcher.py",
  "musehub/mcp/resources.py",
  "musehub/services/musehub_mcp_executor.py"
]

src/scss/_layout.scss is not in that list. Confirming the file content is identical between the blamed commit and its parent:

diff <(muse cat src/scss/_layout.scss --at sha256:659f09b2a6570076b0ec01897c95ce391467b1a11fb8fa6d3c12e88b2e0e1c77) \
     <(muse cat src/scss/_layout.scss --at sha256:9a6e44e84b66174d5e1a275f1d011bb9b100a3bbafc102f7620b36e8db53d1df)
# zero output — byte-identical

Expected

muse blame should attribute each line to the nearest ancestor commit that actually changed it (per its manifest / diff), never a commit whose files_modified excludes the file entirely.

Impact

Blame output cannot be trusted for archaeology (e.g. "why does this rule exist, when was it introduced") — it silently returns plausible-looking but false commit history, which is worse than an error since it's not obviously wrong without independent verification.

◎ Activity1
●
gabriel opened this issue 108 days ago
○
gabriel 2 days ago

Fixed. Root cause was in the core attribution algorithm, not the specific file.

Root cause

blame_file() (muse/core/blame.py) walked iter_ancestors()'s flattened BFS order over the full commit DAG and diffed adjacent entries of that list as if they were real parent/child pairs. That's only valid for a strictly linear chain — the moment history has a merge commit, a commit's BFS-neighbor in the flattened list is frequently some unrelated commit reached via a completely different branch, not its real parent.

Confirmed exactly, against your repro: the blamed commit 659f09b2's real parent (parent_commit_id) is 9a6e44e8 — content-identical, confirming it never touched the file, exactly as you found. But iter_ancestors's BFS order placed an unrelated commit (6d327a10) immediately after 659f09b2 in the flattened list. blame_file compared 659f09b2 against that unrelated BFS-neighbor instead of its real parent, found a content difference, and permanently misattributed the line — never discovering that walking the real parent edge would have shown the file unchanged and continued the search further back.

The fix

blame_file() now walks each commit's actual parent_commit_id / parent2_commit_id edges, in reverse-topological order: a commit's own outgoing edges are only explored once every real child of it (within the search's reachable ancestor set) has already had a chance to push attribution into it. This guarantees every content comparison uses a genuine commit-graph edge — never an arbitrary BFS-neighbor pair — and that each commit is visited exactly once with complete information, regardless of how many branches converge on it. The existing object_id-equality fast path (skip reading file bytes entirely when two real-parent commits share the same content) is preserved, now made properly lazy so it doesn't read bytes it doesn't need.

Regression test

test_blame_never_attributes_to_commit_that_never_touched_file reproduces your exact DAG shape: base → feature (the only commit that changes the file) → merge (an unrelated base-side commit + feature, content identical to feature, never touches the file) → final (also untouched). Confirmed genuine RED first: the old algorithm attributed a line to the merge commit itself — the exact symptom shape you reported (attributed to a commit absent from files_modified, with content byte-identical to its real parent).

Live-verified against your exact repro

muse blame src/scss/_layout.scss --range 38-41 --json

Line 38 now attributes to fbd6a93ed2655 ("refactor: move frontend source from static/js+scss to src/ts+scss") and lines 39-41 to c1d980625041d ("fix(#139): app-shell for issue detail..."). Confirmed both commits genuinely differ from their respective real parents for this exact file (direct content diff, not just files_modified presence — matching your own verification method). Runtime actually improved slightly (3.45s vs 4.2s original), likely because the DAG-correct walk no longer wastes time comparing against unrelated BFS neighbors whose diffs could never produce useful attribution anyway.

Targeted regression suite — test_core_blame.py (15, including the new test), test_cmd_blame.py (the only direct caller of blame_file per muse code impact) — all pass (78 passed), zero regressions.

Merged to dev → main, pushed to local/staging/production for both branches.

✓
closed this issue 2 days ago