Skip to content

fix: add PR number context to merge-base compare API warnings #739

Closed
ebios-star wants to merge 1 commit intoentrius:testfrom
ebios-star:fix/merge-base-pr-context
Closed

fix: add PR number context to merge-base compare API warnings #739
ebios-star wants to merge 1 commit intoentrius:testfrom
ebios-star:fix/merge-base-pr-context

Conversation

@ebios-star
Copy link
Copy Markdown
Contributor

Summary

  • Adds optional pr_number argument to get_merge_base_sha so the four warning
    messages it emits identify the PR that failed.
  • Threads pr.number from fetch_file_contents_for_pr through the call.
  • Adds regression tests verifying PR# appears in warnings when provided and is
    absent otherwise.

Mirrors #711, which applied the same pattern to the downstream
fetch_file_contents_with_base warning.

Problem

When validator scoring processes many PRs per round, a warning like
Compare API for owner/repo failed after 3 attempts. Will use base_ref_oid.
can't be attributed to any specific PR, making transient compare-API
failures hard to diagnose.

Type of Change

  • Bug fix (observability)
  • New feature
  • Refactor
  • Documentation
  • Other

Testing

  • Tests added (test_warning_includes_pr_number_when_provided,
    test_warning_omits_pr_number_when_absent)
  • Updated existing assert_called_once_with to match new signature
  • uv run pre-commit run --all-files --hook-stage pre-push — all 431 tests pass

Threads an optional pr_number argument through get_merge_base_sha so
the four warning messages it emits identify the PR that failed. Without
this, a warning like "Compare API for owner/repo failed after 3 attempts"
is not attributable to any specific PR when multiple are scored in a
round.

Mirrors PR entrius#711, which applied the same pattern to the downstream
fetch_file_contents_with_base warning.
@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label Apr 23, 2026
@anderdc anderdc closed this Apr 24, 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.

2 participants