metrics: handle distinct refs resolving to the same SHA in diff (#10429)

[jbbqqf]

Summary

dvc metrics diff --all <a> <b> returned {} when <a> and <b> were distinct ref names that resolved to the same commit (e.g. main and HEAD). The reporter on #10429 traced this to the interaction between dvc/repo/brancher.py (groups same-SHA revisions under a single comma-joined key like "main,HEAD") and dvc/repo/metrics/diff.py:_diff (looks each rev up by exact key, misses the composite, returns empty).

This PR adds a tiny _resolve_rev helper that falls back to membership lookup when the direct key isn't present. The same helper transparently fixes dvc params diff --all, which delegates to the same _diff.

Fixes #10429 — dvc metrics diff --all: on same branch is empty

Context

• Reproduce: any DVC repo with metrics, then dvc metrics diff --all $(git rev-parse --abbrev-ref HEAD) HEAD — both refs point at the same SHA, expected behavior is "all metrics with diff = 0.0".
• Root cause is in the brancher: dvc/repo/brancher.py:101 joins names of revs that resolve to the same SHA with , so the file-system swap is performed once per SHA. The metrics show result is therefore keyed by "main,HEAD". The reporter pinned this exact line in the issue body.

Changes

• dvc/repo/metrics/diff.py — extract a _resolve_rev(result, rev) helper that first tries result[rev], then falls back to scanning composite keys for the rev as a comma-separated member. The 5-line in-code comment explains why the second branch is needed (brancher group-by-SHA), so a reviewer reading the diff cold doesn't have to derive the reasoning. _diff switches to _resolve_rev.
• tests/func/metrics/test_diff.py — test_metrics_diff_same_commit_different_refs regression: builds a single-commit repo, asks for metrics.diff(a_rev=branch, b_rev="HEAD", all=True), asserts the all-zero diff comes back instead of {}.

dvc params diff is fixed transparently because it imports and calls the same _diff helper from dvc/repo/metrics/diff.py.

Reproduce BEFORE/AFTER yourself (copy-paste)

# --- one-time setup ---
git clone https://github.com/iterative/dvc.git /tmp/repro && cd /tmp/repro
python -m venv .venv && source .venv/bin/activate
pip install -e ".[tests]" >/dev/null

# --- BEFORE (origin/main) ---
git checkout origin/main
pytest tests/func/metrics/test_diff.py::test_metrics_diff_same_commit_different_refs -v 2>&1 | tail -5
# Expected: missing on main; bring in the new test from this branch and rerun:
git checkout origin/feat/10429-metrics-diff-same-rev -- tests/func/metrics/test_diff.py 2>/dev/null \
  || git fetch https://github.com/jbbqqf/dvc.git feat/10429-metrics-diff-same-rev \
  && git checkout FETCH_HEAD -- tests/func/metrics/test_diff.py
pytest tests/func/metrics/test_diff.py::test_metrics_diff_same_commit_different_refs -v 2>&1 | tail -5
# Expected: FAILED — assertion `dvc.metrics.diff(...) == {"diff": expected}` fails because origin/main
#           returns {} instead of the all-zero diff.

# --- AFTER (this PR) ---
git fetch https://github.com/jbbqqf/dvc.git feat/10429-metrics-diff-same-rev
git checkout FETCH_HEAD
pytest tests/func/metrics/test_diff.py::test_metrics_diff_same_commit_different_refs -v 2>&1 | tail -5
# Expected: PASSED.

What I ran locally

• pytest tests/func/metrics/ tests/func/params/ → 78 passed, 2 skipped (pre-existing skips). 21.85s. Includes the new regression and all existing diff tests on both metrics.diff and params.diff.
• BEFORE check: git stash -- dvc/repo/metrics/diff.py then rerun the new test → fails with the assertion error confirming the bug is exercised.
• ruff check dvc/repo/metrics/diff.py tests/func/metrics/test_diff.py → all checks passed.
• ruff format → applied to test file (formatter combined wrapped args).

Edge cases tested

#	Scenario	Input	Expected	Verified by
1	Distinct refs, same SHA	a_rev=branch_name, b_rev="HEAD", all=True	all-zero diff	test_metrics_diff_same_commit_different_refs (new)
2	Distinct refs, distinct SHAs	a_rev="HEAD~2", b_rev=workspace	actual diff	test_metrics_diff_simple, test_metrics_diff_yaml, test_metrics_diff_json (existing, unchanged)
3	Same string rev	a_rev=b_rev=HEAD	all-zero (already worked — single-key result)	exercised implicitly by test_metrics_diff_with_unchanged style
4	params.diff with same SHA refs	same as 1 but params.diff	all-zero	params.diff calls the same _diff; existing params tests pass
5	No commits / no metrics	empty repo, no metrics file	{}	test_no_commits, test_metrics_diff_no_metrics (existing, unchanged)

Risk / blast radius

• The new branch only activates when result[rev] is absent — i.e. when the brancher emitted a composite key. For all "normal" diffs (different SHAs) the direct lookup hits and behavior is byte-identical to before.
• The membership check uses rev in key.split(","), not rev in key — so a rev string containing commas (which is invalid as a git ref anyway) won't accidentally match.
• Both metrics.diff and params.diff benefit (params delegates to the same _diff). No other callers of _diff.

Release note

metrics/params: `dvc metrics diff --all` and `dvc params diff --all` now correctly produce an all-zero diff when both ref arguments resolve to the same commit (e.g. `main` and `HEAD`); previously they returned an empty result because the brancher groups same-SHA revs under a single composite key.

---

PR drafted with assistance from Claude Code. The reporter on #10429 had already pinpointed the exact lines (brancher.py:101 group-by-SHA, metrics/diff.py exact-key lookup) — this PR ships the minimal fix at the lookup site rather than touching the brancher contract. The reproducer block was used during development.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.97%. Comparing base (2431ec6) to head (299f06c).
⚠️ Report is 213 commits behind head on main.

Files with missing lines	Patch %	Lines
dvc/repo/metrics/diff.py	77.77%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11038      +/-   ##
==========================================
+ Coverage   90.68%   90.97%   +0.29%     
==========================================
  Files         504      505       +1     
  Lines       39795    41152    +1357     
  Branches     3141     3266     +125     
==========================================
+ Hits        36087    37440    +1353     
- Misses       3042     3072      +30     
+ Partials      666      640      -26     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[eeshsaxena]

The diagnosis is exactly right, and I confirmed it at the source. dvc/repo/brancher.py yields the composite key here:

for sha, names in found_revs.items():
    ...
    yield sha if sha_only else ",".join(names)

so when two refs resolve to the same commit they land under a single "main,HEAD" key, and the old result.get(old_rev, {}) / result.get(new_rev, {}) both miss it and silently produce an empty diff. Falling back to a membership check against the split key restores the lookup, and the {} default preserves the previous behaviour for a genuine miss. Nice regression test too.

One edge case worth being aware of (and it's inherited rather than introduced by this PR): git allows commas in ref names. I checked:

git check-ref-format --branch 'a,b'   -> accepted
git check-ref-format 'refs/heads/feat,x' -> accepted

So a branch literally named a,b is emitted by brancher as the key "a,b" - indistinguishable from the composite key for two refs named a and b. With this change, _resolve_rev(result, "a") would split "a,b" and match, returning metrics for the unrelated a,b branch.

That ambiguity really lives in brancher's ",".join(names) (the key format is lossy), so I don't think it blocks this PR - the fix is strictly better than the current silent-empty-diff. But it might be worth either a short comment noting the assumption, or, if you want it airtight, having brancher carry the names as structured data (a tuple/list) rather than a joined string, so consumers don't have to re-parse it. That would also let _resolve_rev disappear.

Deferring to the maintainers.