fix(code-intelligence): a failed git invocation is now distinguishable from empty history (#14114)

[mrveiss]

Closes #14114

Thinking Path

_run_git returned "" on any non-zero exit — the same value it returns for a genuinely empty window — and every caller (get_commits_in_range, get_file_history, get_commit_files, get_commit_file_sets) treated an empty string as "no history". A corrupt object store, a timeout, or a permissions problem therefore all read as "no coupling found" instead of an error. The CI evidence in the issue showed exactly this: a temporary repository lost its objects mid-test, git log exited 128, _run_git logged a warning and returned "", and a downstream test then failed on a bogus pair-count assertion that had nothing to do with the real defect.

Traced every _run_git caller (grep -rn "get_commit_file_sets\|GitHistoryCrawler") before changing behaviour. CoChangeAnalyzer.analyze() (code_intelligence/co_change.py) is not wired into any router or service today — its only callers are its own test file — so there is no production caller currently depending on "empty list on git failure" degrading gracefully. CodeEvolutionMiner (which wraps GitHistoryCrawler) IS production-facing, through api/code_intelligence.py and api/analytics_evolution.py; every one of those endpoints already wraps its call in a broad except Exception that converts it into a proper error response, so letting the new exception propagate out of analyze_evolution turns a silently-empty report into a correctly-reported failure, with no additional changes needed at the router layer.

Picked one approach: a GitCommandError raised by _run_git on any failure — subprocess-level (timeout, OSError) or non-zero exit. GitHistoryCrawler.__init__ is the one place that catches it, to keep "not a repository" a legitimate, non-crashing available = False state (unchanged public behaviour). Every other caller lets it propagate, since none of them can distinguish "the read failed" from "there is nothing" any other way.

What Changed

• autobot-backend/code_intelligence/code_evolution_miner.py
  • Added GitCommandError(RuntimeError).
  • _run_git now raises GitCommandError (with the git args, repo path and stderr/exception in the message) instead of returning "" on failure. A successful call with no matching commits still returns "" — that distinction is preserved.
  • GitHistoryCrawler.__init__ catches GitCommandError from the rev-parse --git-dir probe and sets available = False — "not a repository" still degrades rather than crashing.
  • get_commit_file_sets gained the same if not self.available: return [] guard the other three methods already had, so a non-repository path still degrades instead of raising through the new exception. All four _run_git callers now let a genuine failure on an available repo propagate as GitCommandError.
• autobot-backend/code_intelligence/git_history_crawler_test.py, autobot-backend/code_intelligence/co_change_test.py
  • New tests that corrupt an otherwise-valid repository's object store (rev-parse --git-dir still succeeds, git log/git show do not) and assert GitCommandError is raised, for get_commits_in_range, get_file_history, get_commit_files and get_commit_file_sets.
  • New tests confirming a genuinely empty window (a real repo, zero commits in range) still returns [] without raising.
  • Clarified the docstring on the pre-existing "not a repository" degradation test and the _pairs() test helper to name which branch each covers.

Scope boundary

The issue's second defect — /tmp/pytest-of-runner/pytest-0 being reused across concurrent jobs on the self-hosted runner, so one job's tmp retention can delete another job's live objects — is not fixed here. It is not a two-line basetemp change (it needs a per-job-unique tmp root, which touches CI job configuration, not this module), so per the issue's scope boundary it is left for a follow-up. Left a comment on #14114 noting this explicitly so it isn't lost.

Verification

Full code_intelligence suite, with the fix in place:

$ ./venv/bin/python -m pytest code_intelligence/ -q
================= 762 passed, 63 warnings in 52.18s ==================

Before/after evidence for the new behaviour — reproduced the CI failure directly against the unmodified module (repo passes rev-parse --git-dir, i.e. available: True, then its object store is corrupted):

available: True
get_commit_file_sets result (pre-fix): []
get_commits_in_range result (pre-fix): []

(git itself logged fatal: bad object HEAD, exit 128 — silently turned into [] by the old code)

New tests against the unmodified module (mutation test — reverted the production fix by file content, confirmed the file actually changed, then reran):

ImportError: cannot import name 'GitCommandError' from 'code_intelligence.code_evolution_miner'

(both new test files fail to even collect, since the type they assert on doesn't exist pre-fix)

With the fix restored, the same test files:

$ ./venv/bin/python -m pytest code_intelligence/git_history_crawler_test.py code_intelligence/co_change_test.py code_intelligence/code_evolution_miner_test.py -v
======================= 71 passed, 2 warnings in 11.22s ========================

ruff check on all three touched files: All checks passed!

Model Used

Sonnet 5

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[mrveiss]

Review response — findings 1 and 2 fixed in 21d21e53a

On the mutation evidence

The reviewer is right that reverting the whole file and getting ImportError: cannot import name 'GitCommandError' proved only that the tests import a new symbol. A collection failure is not a behavioural result. The mutant that matters keeps the interface and changes the behaviour, and both of the ones below are built that way.

1 — the __init__ probe swallowed every failure

except GitCommandError: in the constructor caught OSError, TimeoutExpired and ValueError alongside a non-zero exit, so a probe timeout or a missing git binary became available = False plus a log line asserting "is not a git repository — history is unavailable" — a false statement — after which every method returned [].

That mattered concretely because api/analytics_evolution.py:1132 pre-checks (repo_path / ".git").exists() before constructing the miner, so at that endpoint available = False could only mean a real git failure, and POST /analyze answered status: "success", commits_analyzed: 0 with HTTP 200. The defect this issue is about, moved from the method to the constructor.

GitCommandError now carries returncode. A None returncode means git never ran to a verdict, so the constructor re-raises it; only a real non-zero exit degrades to available = False. The distinction is documented on the exception itself, where the next reader will meet it.

2 — the exception never reached the router

My PR body claimed the endpoints' broad except Exception handled it, so no router change was needed. Wrong: _analyze_commit_patterns has its own try/except Exception around get_commit_files, which logged and moved on. A repo whose objects vanish mid-walk — a concurrent git gc --prune, or the tmp-retention race that motivated this issue — produced a well-formed empty report with commits_analyzed: 100 and HTTP 200.

except GitCommandError: raise now sits ahead of the broad handler, so an analysis that could not complete fails instead of reporting nothing found. PR body corrected.

3 — the router claim is now tested

test_analyze_evolution_propagates_a_git_failure_mid_walk drives analyze_evolution with get_commit_files failing mid-walk and asserts the error propagates. conftest.py:1093 stubs the miner as a MagicMock for everything outside code_intelligence/, which is why no api/ test could have caught this — the test lives beside the module it exercises.

4 and 5 — taken

The two duplicate "empty window" tests now assert crawler.available is True first, so [] from the degradation branch can no longer satisfy them — which is exactly finding 1's failure mode. And every match="git" became match="exited 128"; "git" appeared in the word, the argv and the repo path, so it could not fail.

Mutation evidence

Each mutation confirmed to have changed the file before running:

MUT-A  blanket except in __init__ (swallow subprocess failures again)
  -> FAILED test_a_construction_time_subprocess_failure_propagates
     (log shows: git ('rev-parse','--git-dir') ... timed out after 1 seconds)
  1 failed, 54 passed

MUT-B  let the broad except swallow GitCommandError again
  -> FAILED test_analyze_evolution_propagates_a_git_failure_mid_walk
  1 failed, 27 passed

restored -> 55 passed

$ pytest autobot-backend/code_intelligence/ -q
765 passed
$ black --check --line-length=120 autobot-backend/code_intelligence/
122 files would be left unchanged

Confirmed and left alone

Timeout handling was already correct and textually distinguishable; the fault injection is deterministic across git versions (rev-parse never reads the object store, log/show must); the available gate does not go stale in the dangerous direction; the if not self.available: return [] guard on get_commit_file_sets is load-bearing. Nothing deleted rather than adapted.

The second defect in #14114 — /tmp/pytest-of-runner/pytest-0 reused across concurrent self-hosted-runner jobs — is still not fixed here and is noted on the issue. It needs a per-job-unique tmp root, not a two-line change.