Flag non-live bounty references in queue health#660
Conversation
📝 WalkthroughWalkthroughThe PR extends queue health analysis to distinguish claimable bounties from non-claimable ones. It adds bounty liveness classification based on ChangesBounty Liveness Detection and Reporting
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 |
Gwani-28
left a comment
Gwani-28
left a comment
There was a problem hiding this comment.
Reviewed PR #660 current head a7dcf2d5d849592dc6f6f948f585cc7a9549dfd5 for #645.
Evidence checked:
- Confirmed
scripts/pr_queue_health.pykeeps the existing bounty-reference parser and adds a separate non-live bounty reference category instead of folding proposed-only issues into closed/exhausted or missing-reference buckets. - Confirmed live GitHub mode hydrates comments only for issue numbers referenced by open PRs, so the
Reserved on MergeWork:signal can be checked without fetching every bounty issue's comments. - Confirmed the liveness check requires both the
mrwk:bountylabel and a claims-openReserved on MergeWork:comment when comment metadata is present, while preserving older/offline fixture compatibility when comments are unavailable. - Confirmed text, Markdown, JSON summaries include non-live bounty counts and details, giving maintainers CI/report output without auto-labeling, auto-closing, accepting, or paying work.
- Confirmed regression tests cover a live bounty, proposed/non-live references, missing claims-open comments, and live-loader comment hydration.
- Confirmed no prior human review existed on #660 before this review; only the automated CodeRabbit in-progress comment was present.
Validation run locally:
.venv/bin/python -m pytest tests/test_pr_queue_health.py -q-> 18 passed..venv/bin/ruff check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> passed..venv/bin/ruff format --check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> 2 files already formatted..venv/bin/python -m mypy scripts/pr_queue_health.py-> success..venv/bin/python scripts/pr_queue_health.py --repo ramimbo/mergework --format markdown-> completed against the live queue; it reported zero non-live bounty references and identified only current dirty/needs-info/unstable queue state. The repository quality check was passing; CodeRabbit review was still pending at the time checked.
I did not find a code blocker in this pass. The implementation matches #645's requested maintainer-facing guard and keeps the change focused on queue-health reporting.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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: 0cb27991-980e-4f3a-b22d-66cdd75b8a6a
📒 Files selected for processing (2)
scripts/pr_queue_health.pytests/test_pr_queue_health.py
| if not labels and not comments: | ||
| return True, "open" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Edge case: empty metadata could mask non-live bounties.
When comments key is present but the list is empty (and labels is also empty), the function returns True, "open" rather than checking for the required signals. Per docs/bounty-lifecycle.md, an issue must have both the mrwk:bounty label and a "Reserved on MergeWork" comment to be claimable—empty metadata satisfies neither.
This could cause a PR referencing such an issue to skip non-live detection entirely. If this is intentional backwards compatibility for sparse live-fetched data, consider adding a clarifying comment. Otherwise, consider only applying this fallback when the comments key is absent.
Proposed clarification comment
if "comments" not in raw:
return True, "open"
+ # Minimal metadata (empty labels + empty comments) is treated as "open" to
+ # avoid false positives when live fetches return sparse data.
if not labels and not comments:
return True, "open"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not labels and not comments: | |
| return True, "open" | |
| # Minimal metadata (empty labels + empty comments) is treated as "open" to | |
| # avoid false positives when live fetches return sparse data. | |
| if not labels and not comments: | |
| return True, "open" |
| def _gh_bounty_issue(raw: dict[str, Any]) -> dict[str, Any]: | ||
| return { | ||
| "number": raw["number"], | ||
| "title": raw.get("title"), | ||
| "state": raw.get("state"), | ||
| "labels": raw.get("labels", []), | ||
| "awards_remaining": 1 if raw.get("state") == "OPEN" else 0, | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider case-insensitive state comparison for robustness.
_is_open_bounty lowercases state before comparison, but here the check is case-sensitive (== "OPEN"). If the GitHub CLI output format varies, this could incorrectly set awards_remaining: 0 for open issues.
Proposed fix
def _gh_bounty_issue(raw: dict[str, Any]) -> dict[str, Any]:
return {
"number": raw["number"],
"title": raw.get("title"),
"state": raw.get("state"),
"labels": raw.get("labels", []),
- "awards_remaining": 1 if raw.get("state") == "OPEN" else 0,
+ "awards_remaining": 1 if str(raw.get("state") or "").upper() == "OPEN" else 0,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _gh_bounty_issue(raw: dict[str, Any]) -> dict[str, Any]: | |
| return { | |
| "number": raw["number"], | |
| "title": raw.get("title"), | |
| "state": raw.get("state"), | |
| "labels": raw.get("labels", []), | |
| "awards_remaining": 1 if raw.get("state") == "OPEN" else 0, | |
| } | |
| def _gh_bounty_issue(raw: dict[str, Any]) -> dict[str, Any]: | |
| return { | |
| "number": raw["number"], | |
| "title": raw.get("title"), | |
| "state": raw.get("state"), | |
| "labels": raw.get("labels", []), | |
| "awards_remaining": 1 if str(raw.get("state") or "").upper() == "OPEN" else 0, | |
| } |
goodgoodclaw
left a comment
goodgoodclaw
left a comment
There was a problem hiding this comment.
Reviewed current head a7dcf2d5d849592dc6f6f948f585cc7a9549dfd5 for #645.
Blocker found:
scripts/pr_queue_health.pytreats an issue with live metadata shape but emptylabelsand emptycommentsas live/open. That conflicts with the PR description and #645 goal: when live metadata is available, a referenced bounty should require both themrwk:bountylabel and aReserved on MergeWork:claims-open comment before it is considered claimable.- Reproduction against current head:
- Input bounty:
{"number": 645, "state": "OPEN", "awards_remaining": 1, "labels": [], "comments": []}with a PR bodyRefs #645. - Observed summary:
live_bounties: 1,non_live_bounties: 0,non_live_bounty_references: 0. - Expected: non-live bounty and non-live reference, with missing
mrwk:bounty labeland missingReserved on MergeWork comment.
- Input bounty:
- Suggested fix: remove the
if not labels and not comments: return True, "open"fallback, or limit the compatibility fallback to the existing"comments" not in rawbranch only. Add a regression test for the empty-labels/empty-comments live metadata shape.
Evidence checked:
- Inspected
_comments,_bounty_liveness,analyze_queue,load_live_queue, and the new queue health tests. - Confirmed the live loader now fetches comments only for referenced bounty issue numbers and passes
commentsinto the bounty map. - Confirmed markdown/text/json report paths include
non_live_bounty_referenceswhen classification catches the issue. - Checked CodeRabbit's two actionable comments; the empty metadata case above is reproducible and affects the stated guard behavior. The
state == "OPEN"casing note is robustness polish, not a blocker for currentghoutput.
Validation run locally:
uv run --python 3.12 --with pytest python -m pytest tests/test_pr_queue_health.py -q-> 18 passed.uvx ruff check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> All checks passed.uvx ruff format --check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> 2 files already formatted.uv run --python 3.12 --with mypy python -m mypy scripts/pr_queue_health.py-> Success: no issues found in 1 source file.uv run --python 3.12 python scripts/pr_queue_health.py --repo ramimbo/mergework --format markdown-> completed; live queue reported 0 non-live references, 4 dirty/unstable PRs, and 2 needs-info PRs.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean tree9002ce380387fd4306061cd721d924afc22a827e.
No private data, credentials, wallet material, production mutation, price/exchange/bridge/off-ramp claims, or fabricated payout claims used.
|
Held for #645. This overlaps #675 and is not accepted for the single #645 award in this pass. Current blockers: requested changes found a reproducible liveness-classification problem, and the duplicate #645 lane needs one final implementation selected rather than paying or merging both PRs. Next useful step: update the implementation against the requested-changes review or coordinate with the #675 direction, then get fresh current-head review evidence. |
ramimbo
added
mrwk:rejected
4 participants
Bounty #645
Summary
scripts/pr_queue_health.pyto distinguish live bounty references from non-live proposed/pending bounty issues.mrwk:bountylabel and an actualReserved on MergeWork:claims-open comment when live GitHub metadata is available.Why
Proposed-only issues can look like open bounty work before they are actually claimable. This guard gives maintainers a repo-native way to spot PRs that cite pending, proposed-only, closed, exhausted, missing, or unsupported bounty references before those PRs muddy the queue.
Validation
.venv/bin/python -m pytest tests/test_pr_queue_health.py -q-> 18 passed..venv/bin/ruff check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> passed..venv/bin/ruff format --check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> 2 files already formatted..venv/bin/python -m mypy scripts/pr_queue_health.py-> success.git diff --check-> clean..venv/bin/python scripts/pr_queue_health.py --repo ramimbo/mergework --format markdown-> live queue report completed and identified the existing dirty/needs-info PR Add treasury proposal filters and links #639 without non-live bounty false positives.No private data, credentials, wallet material, production mutation, price/exchange/bridge/off-ramp claims, or fabricated payout claims used.
Summary by CodeRabbit