refactor(analytics): move analyze_diff's halves out so the file can take its security fix (#16186, #16179)

[mrveiss]

Thinking Path

analyze_diff was 117 body lines against the 65 threshold, and the
Function Length Check (#620) is whole-file — so it blocked every change to
analytics_code_review.py, including the two-line security fix this PR carries.
Same wall as #15950's _execute_claimed, which is the second instance and the
evidence behind #16191.

The file also sat at 972 lines against a recorded ceiling of 972, zero
headroom. Splitting a function within the file would have made it longer, and a
ratchet may shrink but never grow. So the two halves leave the file entirely
rather than moving inside it — that satisfies the length guard and the size
ratchet with one change.

What Changed

Security (#16179, the last of seven unscrubbed git call sites).
get_git_diff now goes through autobot_shared.git_probe.start_git instead of a
bare asyncio.create_subprocess_exec, so the git environment is scrubbed —
GIT_DIR outranks both -C and cwd, so an inherited value silently redirects
this diff at another repository. strict=True is the default and is correct here:
this is a git diff on local refs, not a transport call.

The argv is built without "git" in it, because start_git prepends it. A
list carrying "git" would run git git diff the first time a later edit passed
the whole list.

analytics_code_review_diff.py (new) receives two blocks verbatim:

• collect_file_comments — the per-file analysis loop
• persist_review — the Redis result/history write

Ceiling lowered 972 → 929 in both scripts/python_file_size_known_large.py
and repo_tests/python_file_size_ratchet_baseline.py, as the audit instructed:
"an unlowered ceiling re-licenses the lines just cut."

No function in the file now exceeds 65 lines.

Verification

Given what the equivalent split shipped in #15950 — three control-flow defects,
all four linters green on every one — this one was checked differently:

• Statement-level comparison of both extracted blocks against the originals,
  captured before the cut: 21/21 and 28/28 statements accounted for, 0 missing.
• A recursive fall-through audit over both files — every function's declared
  return type against every path that can reach its end. 0 findings. That auditor
  was itself validated against a reproduction of Enforce work claims: task executor and base agent acquire declared scopes #15950's two review findings and
  catches both; its first version caught neither, because a substring test for
  "None" skipped str | None and tuple[str, dict | None] — the same blind
  spot that let the original defect past mypy.
• One real defect the extraction introduced and this caught: the persist block
  landed at module level rather than inside persist_review, leaving the
  function an empty shell. Found by ast structure inspection, not by a linter.

By exit code, not piped output: flake8 0, isort --check-only 0 with CI's exact
settings, check_python_file_size --audit-ceilings 0, check_undefined_names --audit 0.

Model Used

Claude Opus 5 (1M context).

Single-issue rationale

Two issues in one PR, which is the batching default: the extraction exists only to
unblock the file, and the security fix is what needed unblocking. Splitting them
would mean landing a refactor with no reason attached, then a two-line fix behind
a second full CI run.

Closes #16186
Completes #16179

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 90a92f76-80ec-42cf-9e63-0d08554afe84

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.