Skip to content

fix: sync pr_state for stale-closed mirror PRs excluded from scoring - #1032

Closed
Dexterity104 wants to merge 1 commit into
entrius:testfrom
Dexterity104:fix/mirror-stale-closed-pr-state
Closed

fix: sync pr_state for stale-closed mirror PRs excluded from scoring#1032
Dexterity104 wants to merge 1 commit into
entrius:testfrom
Dexterity104:fix/mirror-stale-closed-pr-state

Conversation

@Dexterity104

Copy link
Copy Markdown

Summary

Stale CLOSED PRs from the mirror path now land in a storage-only bucket so the pull_requests row's pr_state flips from OPEN to CLOSED on the next evaluation. Before this change, the loader returned early without recording them, leaving phantom OPEN rows on dashboards for every mirror_enabled: true repo.

The fix mirrors PR #769, which solved the same problem on the legacy GraphQL path. The storage layer was already wired (stale_closed_pull_requests flows through store_pull_requests_bulk, and pr_state is in the DO UPDATE SET clause of BULK_UPSERT_PULL_REQUESTS), so the work is to add the matching mirror bucket and route it into the existing call.

Changes:

  • MirrorMinerEvaluation gains a stale_closed_prs field
  • _maybe_add_pr appends pre-lookback CLOSED PRs there instead of returning silently
  • MinerEvaluation gains a parallel mirror_stale_closed_prs slot
  • combine propagates the bucket onto the legacy evaluation
  • DatabaseStorage.store_evaluation concatenates the adapted mirror list onto the existing stale storage call (the four parallel store_pull_requests_bulk calls also get folded into a single loop over a pr_buckets table for clarity)
  • The mirror summary log now reports stale-closed count for observability

These PRs do not enter scoring or credibility math, so totals (total_closed_prs, merged_count for check_eligibility, etc.) stay correct.

Related Issues

Fixes #1031

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other (describe below)

Testing

  • Tests added/updated
  • Manually tested

Test coverage:

  • tests/validator/oss_contributions/mirror/test_load.py: the existing stale-closed test now also asserts the PR lands in stale_closed_prs and that total_closed_prs reflects only the scoring bucket
  • tests/validator/oss_contributions/mirror/test_combine.py: setup now seeds a stale entry, and test_aggregate_pr_count_properties_sum_both_paths proves it does not inflate total_closed_prs
  • tests/validator/utils/test_storage_mirror.py: new test_stale_closed_lists_concatenated covers legacy plus mirror concatenation, PullRequest adaptation, and identity propagation through the call at call_args_list[3]

Full suite: 602 passed, ruff clean.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Changes are documented (if applicable)

@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label May 6, 2026
@anderdc

anderdc commented May 9, 2026

Copy link
Copy Markdown
Collaborator

The validator's pull_requests table is a scoring artifact (who got credit and when), not the source of truth for current PR state. For mirror-enabled repos, das-github-mirror tracks live state continuously and is the canonical source — downstream consumers wanting current open/closed state should read from the mirror, not the validator table. Phantom OPEN rows are a non-issue if you don't treat the validator table as state-of-the-world. Closing.

@anderdc anderdc closed this May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mirror path drops stale-closed PRs without updating pr_state

2 participants