MinerEvaluationCache.get() does not isolate mirror PRs from finalize_miner_scores mutations

[carlos4s]

Description

MinerEvaluationCache._isolate_for_downstream in gittensor/classes.py returns an isolated snapshot for legacy PRs, but not for mirror PRs.

For legacy PR buckets, _isolate_for_downstream rebuilds each list with _pr_with_fresh_issues(...):

copy_eval.merged_pull_requests = [_pr_with_fresh_issues(pr) for pr in cached_eval.merged_pull_requests]
copy_eval.open_pull_requests = [_pr_with_fresh_issues(pr) for pr in cached_eval.open_pull_requests]
copy_eval.closed_pull_requests = [_pr_with_fresh_issues(pr) for pr in cached_eval.closed_pull_requests]

_pr_with_fresh_issues starts with copy.copy(pr), so downstream mutations to top-level PullRequest fields land on the returned snapshot, not on the object stored inside self._cache.

The mirror buckets do not have the equivalent treatment. _isolate_for_downstream starts with copy.copy(cached_eval), but that is only a shallow copy of the MinerEvaluation. As a result, the returned evaluation still shares the same mirror_merged_prs, mirror_open_prs, and mirror_closed_prs lists and the same ScoredMirrorPR instances with self._cache[uid].evaluation.

This matters because the cache fallback path returns the cached evaluation before final score finalization. gittensor/validator/oss_contributions/reward.py calls store_or_use_cached_evaluation(...), then calls finalize_miner_scores(...) on the same miner_evaluations dict:

cached_uids = self.store_or_use_cached_evaluation(miner_evaluations)
...
finalize_miner_scores(miner_evaluations)

finalize_miner_scores walks legacy and mirror PR lists together and mutates top-level PR scoring fields:

• scoring.py:318-320: writes collateral_score on open_pull_requests + mirror_open_prs
• scoring.py:347-350: writes open_pr_spam_multiplier, credibility_multiplier, and earned_score on merged_pull_requests + mirror_merged_prs
• scoring.py:273: writes pioneer_rank
• scoring.py:293-294: writes pioneer_dividend and updates earned_score

For legacy PRs, those mutations are isolated by the per-PR copy.copy(...) in _pr_with_fresh_issues. For mirror PRs, they currently mutate the ScoredMirrorPR objects stored in the cache itself. A later cache hit can therefore return a mirror PR that already contains scoring state from a previous fallback round.

#824 fixed the mirror file-blob memory leak by stripping ScoredMirrorPR.files at cache store time, but _isolate_for_downstream was not updated to isolate the mirror buckets when those buckets were added to _build_cache_entry.

Steps to Reproduce

1. Build a MinerEvaluation with one ScoredMirrorPR in mirror_merged_prs.
2. Call cache.store(evaluation).
3. Call first = cache.get(uid, hotkey, github_id).
4. Mutate first.mirror_merged_prs[0].earned_score = 999.0 to simulate a finalize_miner_scores write.
5. Call second = cache.get(uid, hotkey, github_id).
6. Observe that second.mirror_merged_prs[0].earned_score == 999.0.
7. Repeat the same shape of test against merged_pull_requests with a legacy PullRequest; the mutation does not persist because _pr_with_fresh_issues returns a fresh PullRequest object.

Expected Behavior

MinerEvaluationCache.get() should return mirror PR instances that are independent of the objects stored in self._cache[uid].evaluation.

finalize_miner_scores should be free to mutate returned ScoredMirrorPR scoring fields without contaminating the cached snapshot used by future fallback rounds.

Mirror PR behavior should match the existing legacy PR behavior: one fresh top-level PR object per returned PR.

Actual Behavior

MinerEvaluationCache.get() returns the same ScoredMirrorPR instances stored inside self._cache[uid].evaluation.

When the cache fallback path later runs finalize_miner_scores, it can write the following fields directly onto cached mirror PR objects:

• collateral_score
• open_pr_spam_multiplier
• credibility_multiplier
• earned_score
• pioneer_rank
• pioneer_dividend

Subsequent cache.get() calls can return those already-mutated mirror PR objects.

Environment

• OS: any
• Python version: project default, currently Python 3.12
• Commit/version: current test, post-fix: strip mirror file contents in MinerEvaluationCache #824

Suggested Fix

Add mirror PR top-level copy isolation in _isolate_for_downstream, mirroring the existing legacy bucket treatment:

copy_eval.mirror_merged_prs = [copy.copy(s) for s in cached_eval.mirror_merged_prs]
copy_eval.mirror_open_prs = [copy.copy(s) for s in cached_eval.mirror_open_prs]
copy_eval.mirror_closed_prs = [copy.copy(s) for s in cached_eval.mirror_closed_prs]

This is sufficient for the current production bug because the downstream writers after a cache hit mutate top-level ScoredMirrorPR scoring fields, not nested MirrorPullRequest metadata.

Nested mirror metadata such as MirrorPullRequest, MirrorReviewSummary, MirrorLabel, and MirrorLinkedIssue does not need fresh copies for this bug. Current gittensor/ code after a cache hit reads those fields but does not mutate them. This differs from legacy PullRequest.issues, where Issue has mutable discovery_* fields and therefore _pr_with_fresh_issues also refreshes the issues list.

Suggested Tests

Add cache-isolation tests that verify:

1. A mutation to first.mirror_merged_prs[0].earned_score after cache.get() does not appear in a second cache.get().
2. A mutation to first.mirror_open_prs[0].collateral_score after cache.get() does not appear in a second cache.get().
3. Legacy PR behavior remains unchanged.
4. ScoredMirrorPR.files is still stripped at store time, preserving the fix: strip mirror file contents in MinerEvaluationCache #824 memory fix.

Copying mirror_closed_prs is still recommended for symmetry and future safety, even though current finalization does not mutate closed mirror PR objects directly.

[anderdc]

The claimed leakage doesn't translate into observable behaviour — every field this issue lists as mutated is fresh-assigned in Phase 1 of finalize_miner_scores (scoring.py:319, 344-348) before Phase 2/3 read it. calculate_final_earned_score recomputes earned_score from base × multipliers, so any stale cached value is overwritten before observation. MinerEvaluationCache also lives entirely in the GitHub-PR-fetch fallback path being decommissioned with the move to mirror-only scoring (per the comment at classes.py:715). Closing — not worth fixing in a path being decommissioned.