Accept resolve and reference verbs in bounty checks#554
Conversation
📝 WalkthroughWalkthroughThis PR expands bounty-reference pattern recognition across two scripts. The ChangesBounty reference pattern expansion
Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Baijack-star
left a comment
Baijack-star
left a comment
There was a problem hiding this comment.
Reviewed current head ac7fadf3ecc30fc09b7c8a737a9af591f08662e0 for the reference-verb parser change.
Thanks for tightening these two local gates. The focused tests pass, but I found one remaining consistency gap with the stated goal of aligning these scripts with the webhook parser.
app/webhooks/github.py accepts the full GitHub-style close/fix/resolve verb families: close, closes, closed, fix, fixes, fixed, resolve, resolves, and resolved, plus reference(s). This PR only extends the script regexes to references? and resolves?, so both scripts still reject several forms the webhook would pay from. On this head:
Fix #319 gate=fail queue_missing=1 webhook=[319]
Fixed #319 gate=fail queue_missing=1 webhook=[319]
Close #319 gate=fail queue_missing=1 webhook=[319]
Closed #319 gate=fail queue_missing=1 webhook=[319]
Resolved #319 gate=fail queue_missing=1 webhook=[319]
Those are not obscure aliases: they are part of GitHub's normal closing keyword family. That means an agent can still pass the accepted-label webhook path but fail the pre-submission gate or appear as missing a bounty reference in queue health. Please mirror the webhook pattern for the same verb families in both scripts and add regression rows for the missed forms.
Validation run locally on this head:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_submission_quality_gate.py tests/test_pr_queue_health.py -q-> 34 passed./.venv/bin/python -m ruff check scripts/pr_queue_health.py scripts/submission_quality_gate.py tests/test_pr_queue_health.py tests/test_submission_quality_gate.py-> passed./.venv/bin/python -m ruff format --check scripts/pr_queue_health.py scripts/submission_quality_gate.py tests/test_pr_queue_health.py tests/test_submission_quality_gate.py-> already formattedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m mypy scripts/submission_quality_gate.py scripts/pr_queue_health.py-> success
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c3425413-210f-4793-b167-37807dafe3e3
📒 Files selected for processing (4)
scripts/pr_queue_health.pyscripts/submission_quality_gate.pytests/test_pr_queue_health.pytests/test_submission_quality_gate.py
| BOUNTY_REF_RE = re.compile( | ||
| r"\b(?:bounty|refs?|references?|fixes|closes|claims?|resolves?)\s+#(\d+)", | ||
| re.IGNORECASE, | ||
| ) |
There was a problem hiding this comment.
Update missing-reference guidance to match accepted verbs.
The parser now accepts reference(s) and resolve(s), but the missing-reference message still only advertises Bounty, Refs, and /claim. Please align the guidance string so users see the full valid set.
| BOUNTY_REF_RE = re.compile( | ||
| r"\b(?:bounty|refs?|references?|fixes|closes|claims?|resolves?)\s+#(\d+)", | ||
| re.IGNORECASE, | ||
| ) |
There was a problem hiding this comment.
Keep submission error text consistent with expanded regex.
BOUNTY_REF_RE now accepts reference(s) and resolve(s), but the failure hint still documents only Bounty, Refs, and /claim. Updating that message will prevent avoidable user confusion.
| def test_pr_queue_health_accepts_resolve_and_reference_words() -> None: | ||
| report = analyze_queue( | ||
| { | ||
| "bounties": [{"number": 310, "state": "OPEN", "awards_remaining": 1}], | ||
| "pull_requests": [ | ||
| { | ||
| "number": 8, | ||
| "title": "Harden bounty submission checks", | ||
| "body": "Resolves #310", | ||
| "merge_state": "clean", | ||
| "labels": [], | ||
| }, | ||
| { | ||
| "number": 9, | ||
| "title": "Harden bounty queue checks", | ||
| "body": "References #310", | ||
| "merge_state": "clean", | ||
| "labels": [], | ||
| }, | ||
| ], | ||
| } | ||
| ) | ||
|
|
||
| assert report["summary"]["missing_bounty_references"] == 0 | ||
| assert report["missing_bounty_references"] == [] | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add singular verb coverage for this regression test.
This test validates plural forms, but not singular Resolve #310 / `Reference `#310, which are also accepted by the regex. Add those cases to lock the contract.
As per coding guidelines, “tests/**/*.py: Do not request docstrings. Focus on whether tests prove the changed behavior and include negative, replay, boundary, or regression cases where relevant.”
| def test_submission_quality_gate_accepts_resolve_and_reference_words() -> None: | ||
| for text in ("Resolves #319", "Reference #319", "References #319"): | ||
| result = evaluate_submission( | ||
| { | ||
| "submission_text": f""" | ||
| Summary: | ||
| Harden the bounty reference parser. | ||
|
|
||
| {text} | ||
|
|
||
| Validation: | ||
| - pytest passed. | ||
| """, | ||
| "bounties": [{"number": 319, "state": "OPEN", "awards_remaining": 1}], | ||
| "pull_requests": [], | ||
| } | ||
| ) | ||
|
|
||
| assert result["status"] == "pass" | ||
| assert result["bounty_reference"] == 319 | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Include Resolve #319`` in the new keyword regression set.
You already cover Resolves and Reference(s). Add singular Resolve to fully pin the expanded regex behavior.
As per coding guidelines, “tests/**/*.py: Do not request docstrings. Focus on whether tests prove the changed behavior and include negative, replay, boundary, or regression cases where relevant.”
|
Reviewed PR #554 at Evidence checked:
Validation:
Assessment: no blocker found in this parser slice. The change is focused, covered on both local gates, and does not add private data, wallet material, production mutation, price, liquidity, exchange, bridge, off-ramp, or fabricated payout claims. |
|
Correction to my prior comment: after rechecking the current review thread, I agree the current head still has a merge blocker. My validation covers the newly added The remaining gap is the one already called out in review: |
eliasx45
left a comment
eliasx45
left a comment
There was a problem hiding this comment.
Reviewed current head ac7fadf3ecc30fc09b7c8a737a9af591f08662e0.
Verdict: request changes.
The behavioral change is small and reasonable: both queue-health and submission-quality parsing now recognize reference(s) and resolve(s) style bounty references, with focused tests for those cases. The branch itself is stale against current main, though, and cannot be merged cleanly as submitted.
Evidence:
- Current diff is scoped to
scripts/pr_queue_health.py,scripts/submission_quality_gate.py,tests/test_pr_queue_health.py, andtests/test_submission_quality_gate.py. PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .\.venv\Scripts\python.exe -m pytest tests\test_pr_queue_health.py tests\test_submission_quality_gate.py -q->34 passed..\.venv\Scripts\python.exe -m ruff check scripts\pr_queue_health.py scripts\submission_quality_gate.py tests\test_pr_queue_health.py tests\test_submission_quality_gate.py-> passed..\.venv\Scripts\python.exe -m ruff format --check scripts\pr_queue_health.py scripts\submission_quality_gate.py tests\test_pr_queue_health.py tests\test_submission_quality_gate.py-> already formatted.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> conflicts.
Blocking issue:
- The branch conflicts with current
mainin all four touched files:scripts/pr_queue_health.py,scripts/submission_quality_gate.py,tests/test_pr_queue_health.py, andtests/test_submission_quality_gate.py.
Required fix: rebase or merge current main, resolve those conflicts, and rerun the same focused tests/ruff checks on the resolved branch. Once the merge state is clean, the actual parser/test change should be straightforward to re-review.
4 participants
Summary
Resolve(s) #NandReference(s) #Nbounty refs in submission and queue health checksRefs #406
Validation
.\\.venv\\Scripts\\python.exe -m pytest tests\\test_submission_quality_gate.py tests\\test_pr_queue_health.py -q-> 34 passed.\\.venv\\Scripts\\python.exe -m ruff check scripts\\submission_quality_gate.py scripts\\pr_queue_health.py tests\\test_submission_quality_gate.py tests\\test_pr_queue_health.py-> passed.\\.venv\\Scripts\\python.exe -m ruff format --check scripts\\submission_quality_gate.py scripts\\pr_queue_health.py tests\\test_submission_quality_gate.py tests\\test_pr_queue_health.py-> 4 files already formatted.\\.venv\\Scripts\\python.exe -m mypy scripts\\submission_quality_gate.py scripts\\pr_queue_health.py-> success.\\.venv\\Scripts\\python.exe -m pytest -q-> 416 passedSummary by CodeRabbit
New Features
Tests