Ignore native bounty ids in queue refs#547
Conversation
Bounty ramimbo#427: scripts/pr_queue_health.py
Bounty ramimbo#427: tests/test_pr_queue_health.py
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughExpanded the bounty-reference regex to detect more keywords and optional native prefixes; adjusted extraction to deduplicate and ignore native-prefixed bounty mentions; extended the missing-bounty message wording; added tests for boolean refs, native vs explicit refs, and markdown formatting. ChangesBounty Reference Parsing
Possibly Related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c075eaa7-7daa-426e-b4ea-dea56fe7d5a7
📒 Files selected for processing (2)
scripts/pr_queue_health.pytests/test_pr_queue_health.py
Baijack-star
left a comment
Baijack-star
left a comment
There was a problem hiding this comment.
I found one correctness issue that still leaves the advertised native-id case noisy.
The new pattern only suppresses live bounty #... and MRWK bounty #..., but the PR title and summary describe ignoring native bounty ids in general. A body that uses the natural wording native bounty #66 / issue #406 still matches bounty #66, so analyze_queue emits unknown_bounty_reference even though the GitHub issue ref is present.
Repro I ran on this branch:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python - <<'PY'
from scripts.pr_queue_health import analyze_queue
fixture = {
"bounties": [{"number": 406, "state": "OPEN", "awards_remaining": 16}],
"pull_requests": [{
"number": 547,
"title": "Refs #406: Ignore native bounty ids",
"body": "Evidence: native bounty #66 / issue #406 preflight returned status=open.",
"merge_state": "clean",
"labels": [],
}],
}
report = analyze_queue(fixture)
print(report["summary"])
print(report["closed_bounty_references"])
PYObserved:
{'pull_requests': 1, 'open_bounties': 1, 'closed_or_exhausted_bounties': 0, 'closed_bounty_references': 1, 'missing_bounty_references': 0, 'dirty_or_unstable_merge_state': 0, 'needs_info': 0, 'duplicate_scope_groups': 0}
[{'pull_request': 547, 'title': 'Refs #406: Ignore native bounty ids', 'url': None, 'reason': 'unknown_bounty_reference', 'detail': 'Referenced bounty #66 was not in input'}]
The focused suite itself passes (PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_pr_queue_health.py -q, 10 passed), so this likely needs an added regression case and either a broader native-id prefix list or parsing that prefers explicit issue/refs/fixes/closes/claims references over generic native bounty evidence text.
|
Addressed the requested-change case in the updated PR head What changed:
Repro from the review now returns no queue-health finding: Validation after the update:
|
Baijack-star
left a comment
Baijack-star
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the requested-change case on head db9e5e9fafbf22fc9426a8ac78773a31e740cd24; the native/internal wording that failed before is now covered and resolves cleanly to the GitHub issue reference.
Validation run locally:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_pr_queue_health.py -q-> 10 passed./.venv/bin/python -m ruff check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> passed./.venv/bin/python -m ruff format --check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> passedgit diff --check origin/main...HEAD-> clean
I also reran the ad hoc parser smoke for these bodies and each produced zero closed_bounty_references and zero missing_bounty_references:
native bounty #66 / issue #406internal bounty #66, issue #406MRWK bounty #66 / issues #406live bounty #66 / issue #406
No remaining blocker from my earlier requested-change finding.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e058d6a8-16b4-4053-8c2b-2c85bdb0a281
📒 Files selected for processing (2)
scripts/pr_queue_health.pytests/test_pr_queue_health.py
|
Addressed the latest CodeRabbit edge-case comments in PR head What changed:
Extra parser sanity check: Validation:
|
|
Addressed the CodeRabbit whitespace/native-only follow-up on the updated PR branch. What changed:
Validation:
|
|
Correction/remote-branch verification: I fetched the exact TinyOps branch currently backing this PR and verified head The branch content handles the CodeRabbit edge cases: Validation on the fetched remote branch:
Note: my previous follow-up used the local patch worktree count before fetching the exact branch state; the fetched remote branch evidence above is the authoritative verification. |
eliasx45
left a comment
eliasx45
left a comment
There was a problem hiding this comment.
Current-head review for PR #547 at ae621671071caf3cb44f4bae03e3ea2c3946a3f5.
The queue-health behavior looks fixed for the earlier native-id cases, but I found a merge-readiness blocker in the typed validation path: mypy scripts/pr_queue_health.py fails on the new _bounty_refs() implementation.
refs is first inferred as list[int] in the explicit bounty_refs branch:
refs = [item for item in explicit if isinstance(item, int)]The same local name is then reassigned to set() in the text-parsing branch and used with .add(...):
refs = set()
...
refs.add(int(match.group("number")))That makes the script fail the project's typed check:
.\.venv\Scripts\python.exe -m mypy scripts\pr_queue_health.py
scripts\pr_queue_health.py:62: error: Incompatible types in assignment (expression has type "set[Never]", variable has type "list[int]") [assignment]
scripts\pr_queue_health.py:66: error: "list[int]" has no attribute "add"; maybe "append" or "extend"? [attr-defined]
Found 2 errors in 1 file (checked 1 source file)
The runtime behavior and formatting checks I ran are otherwise good:
.\.venv\Scripts\python.exe -m pytest tests\test_pr_queue_health.py -q
# 11 passed
.\.venv\Scripts\python.exe -m ruff check scripts\pr_queue_health.py tests\test_pr_queue_health.py
# All checks passed!
.\.venv\Scripts\python.exe -m ruff format --check scripts\pr_queue_health.py tests\test_pr_queue_health.py
# 2 files already formatted
git diff --check origin/main...HEAD
# clean
Suggested fix: avoid reusing the refs local with two incompatible container types, e.g. name the explicit list separately and use refs: set[int] = set() for the parsed branch before returning sorted(refs).
|
Addressed the mypy failure reported on the latest PR #547 review in head What changed:
Validation on the updated branch:
|
eliasx45
left a comment
eliasx45
left a comment
There was a problem hiding this comment.
Follow-up current-head review for PR #547 at 918d029145865f5f1dcf253c49655b5df6d3e51a.
My earlier requested-change finding is resolved. The _bounty_refs() implementation no longer reuses refs for both a list[int] and a set[...]; it now keeps explicit refs in explicit_refs and declares the parsed branch as refs: set[int] = set(), so the typed check passes while preserving the native-bounty-id parser behavior.
Validation I ran on the fetched PR head:
.\.venv\Scripts\python.exe -m mypy scripts\pr_queue_health.py
# Success: no issues found in 1 source file
.\.venv\Scripts\python.exe -m pytest tests\test_pr_queue_health.py -q
# 11 passed
.\.venv\Scripts\python.exe -m ruff check scripts\pr_queue_health.py tests\test_pr_queue_health.py
# All checks passed!
.\.venv\Scripts\python.exe -m ruff format --check scripts\pr_queue_health.py tests\test_pr_queue_health.py
# 2 files already formatted
git diff --check origin/main...HEAD
# clean
GitHub still showed CodeRabbit as pending when I checked, so this approval is scoped to the current diff, the prior mypy blocker, focused queue-health validation, formatting, and diff hygiene.
|
Addressed the remaining CodeRabbit boolean edge case on PR head What changed:
Validation:
|
eliasx45
left a comment
eliasx45
left a comment
There was a problem hiding this comment.
Follow-up current-head review for PR #547 at dcf8b6b6dcd0c5edc134e95e008c63282185f9ee.
The remaining boolean edge case from CodeRabbit is fixed in this head. Explicit bounty_refs now filters with isinstance(item, int) and not isinstance(item, bool), so True and False are not interpreted as refs 1 and 0, while real integer refs still count. The new regression covers boolean-only refs and mixed boolean plus real integer refs.
Validation I ran on the fetched PR head:
.\.venv\Scripts\python.exe -m mypy scripts\pr_queue_health.py
# Success: no issues found in 1 source file
.\.venv\Scripts\python.exe -m pytest tests\test_pr_queue_health.py -q
# 12 passed
.\.venv\Scripts\python.exe -m ruff check scripts\pr_queue_health.py tests\test_pr_queue_health.py
# All checks passed!
.\.venv\Scripts\python.exe -m ruff format --check scripts\pr_queue_health.py tests\test_pr_queue_health.py
# 2 files already formatted
git diff --check origin/main...HEAD
# clean
CodeRabbit was still processing when I checked, so this approval is scoped to the resolved boolean/ref parsing edge case, the prior mypy blocker, focused tests, formatting, and diff hygiene on this head.
|
Actionable comments posted: 0 |
barnacleagent-svg
left a comment
barnacleagent-svg
left a comment
There was a problem hiding this comment.
Verdict: APPROVED
Scope: Updates BOUNTY_REF_RE regex to ignore "live", "mrwk", "native", "internal" prefixed bounty references. Also fixes explicit_refs type check to filter out bool instances. Prevents false bounty ref detection from native bounty ID mentions.
Checklist:
- Diff: focused regex change + type fix
- Tests cover prefix and non-prefix cases
- Type check fix prevents bool-passing-as-int issues
- Follows same pattern as PR #548
Conclusion: Well-implemented. Ready to merge.
4 participants
Bounty #406
Summary:
live bounty #66/MRWK bounty #66while still acceptingissue #406,Refs #406, and/claim #406Evidence:
scripts/pr_queue_health.py --repo ramimbo/mergework --format jsonreported open MRWK bounty: useful bug reports and small fixes, round 5 #406/MRWK bounty: site UX and functional improvements #427 PRs asunknown_bounty_referencefor native bounty ids such as Prefill linked wallet on claim form #66 and Handle malformed bounty API JSON #73 when PR text included evidence likelive bounty #66 / issue #406Verification:
Summary by CodeRabbit
New Features
Improvements
Tests