Use API award capacity in queue health#472
Conversation
|
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)
📝 WalkthroughWalkthroughAdds MergeWork API fetching to pr_queue_health: defines DEFAULT_API_HOST, implements _load_api_bounties(repo, api_host), integrates API data into load_live_queue (overriding bounty state and awards_remaining), and adds tests for API capacity, non-UTF8 fallback, and duplicate-row handling. ChangesAPI bounty metadata integration
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 |
er1c-cartman
left a comment
er1c-cartman
left a comment
There was a problem hiding this comment.
Reviewed current head a8319aad13366ff4fbffb596be60f329041463a6 for the #447 review bounty.
I inspected scripts/pr_queue_health.py, tests/test_pr_queue_health.py, and the existing queue analysis path. The change is scoped: live mode still gathers PRs/issues through gh, then overlays only the bounty state and award capacity from the public MergeWork API when that API is available. If the API is unavailable or malformed, the existing GitHub-only heuristic is preserved.
Checks I walked through:
- API
statusvalues are lowercase (open) while GitHub issue state is uppercase (OPEN);_is_open_bounty()normalizes both. - API
awards_remaining: 0flows throughanalyze_queue()and marks a referenced bounty as not payable, which is the behavior this patch needs. - API failures intentionally fall back to the previous GitHub issue-state behavior.
- Live
_load_api_bounties("ramimbo/mergework")returned current rows for #406, #447, and #459, including current award capacity.
Validation:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ~/.local/bin/uv run --extra dev python -m pytest tests/test_pr_queue_health.py -q->10 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ~/.local/bin/uv run --extra dev ruff check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ~/.local/bin/uv run --extra dev ruff format --check scripts/pr_queue_health.py tests/test_pr_queue_health.py->2 files already formattedgit diff --check origin/main...HEAD-> clean- live API readback: #406 open/15 awards, #447 open/8 awards, #459 open/10 awards, 76 repo rows
Note: I disabled external pytest plugin autoload because this workstation has a ROS Humble pytest plugin on the global path; the project test itself passed after isolating third-party pytest plugins.
No blocker found in this scoped review.
![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: f060d644-00ae-4bf7-9e31-256f5f86a337
📒 Files selected for processing (2)
scripts/pr_queue_health.pytests/test_pr_queue_health.py
adliebe
left a comment
adliebe
left a comment
There was a problem hiding this comment.
Reviewed PR #472 at current head a8319aad13366ff4fbffb596be60f329041463a6.
I found one fallback-path blocker before this should merge. The PR intends to preserve the GitHub-only fallback when the MergeWork API is unavailable or malformed, but _load_api_bounties() currently does not wrap UnicodeDecodeError from response.read().decode("utf-8"). That means a bad/non-UTF-8 API response bypasses the RuntimeError contract and load_live_queue() does not reach its except RuntimeError: api_bounties = {} fallback.
Concrete repro:
class BadUtf8Response:
def __enter__(self): return self
def __exit__(self, *args): return None
def read(self): return b"\xff\xfe"
pr_queue_health.urlopen = lambda url, timeout: BadUtf8Response()
pr_queue_health._load_api_bounties("ramimbo/mergework")
Observed result:
UnicodeDecodeError
'utf-8' codec can't decode byte 0xff in position 0: invalid start byte
The same bad response through load_live_queue("ramimbo/mergework") also raises UnicodeDecodeError, so the fallback is not preserved for that malformed-response class. A simulated URLError("temporary outage") does fall back correctly and produced closed_bounty_references == 0, open_bounties == 1 in my local harness.
Recommended fix: catch UnicodeDecodeError alongside OSError, URLError, and json.JSONDecodeError, then add a regression test for API fallback behavior. Keep the test fixture using Refs #406; a backticked Refs \#406`form does not match the currentBOUNTY_REF_RE`.
Validation I ran on the current head:
.venv\Scripts\python.exe -m pytest tests\test_pr_queue_health.py -q-> 10 passed.venv\Scripts\ruff.exe check scripts\pr_queue_health.py tests\test_pr_queue_health.py-> All checks passed.venv\Scripts\ruff.exe format --check scripts\pr_queue_health.py tests\test_pr_queue_health.py-> 2 files already formatted.venv\Scripts\python.exe -m mypy app scripts\pr_queue_health.py-> Success, no issues found in 31 source files.venv\Scripts\python.exe -m pytest -q-> 415 passedgit diff --check origin/main...HEAD-> clean
The successful API overlay path looks good; the failure/fallback boundary just needs the decode case covered.
GHX5T-SOL
left a comment
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Reviewed current head a8319aad13366ff4fbffb596be60f329041463a6.
I found one additional blocker separate from the existing UTF-8/fallback comments. _load_api_bounties() collapses API rows into a dict by issue_number, but it overwrites an existing entry every time it sees the same issue. The live endpoint currently returns newest rows first (limit=10 starts with ids 76, 75, 74...), so if the API ever contains two bounty rows for the same repo issue, an older/stale row later in the response wins over the newer/current row. That can make load_live_queue() report an open/payable bounty as closed or exhausted.
Local repro against the submitted code:
p.urlopen = lambda url, timeout: response_with([
{"repo":"ramimbo/mergework","issue_number":406,"title":"new","status":"open","awards_remaining":5},
{"repo":"ramimbo/mergework","issue_number":406,"title":"old","status":"paid","awards_remaining":0},
])
print(p._load_api_bounties("ramimbo/mergework"))
Observed output:
{406: {'number': 406, 'title': 'old', 'state': 'paid', 'awards_remaining': 0}}
That is the stale row, even though the current/newer row appeared first. I would either keep the first row for a repo/issue pair after filtering, or explicitly compare a stable API field such as id/created_at if this endpoint does not guarantee ordering. A regression test with duplicate API rows would catch the current overwrite behavior.
Other validation I ran on this head:
/home/kali/money/mergework/.venv/bin/python -m pytest tests/test_pr_queue_health.py -q-> 10 passed/home/kali/money/mergework/.venv/bin/python -m ruff check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> passed/home/kali/money/mergework/.venv/bin/python -m ruff format --check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> 2 files already formattedgit diff --check upstream/main...HEAD-> clean- diff gitleaks scan -> no leaks found
- live
scripts/pr_queue_health.py --repo ramimbo/mergework --format jsoncompleted; current API scan returned 76 rows and no duplicate(repo, issue_number)pairs at review time
GHX5T-SOL
left a comment
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Reviewed current head a8319aad13366ff4fbffb596be60f329041463a6.
I am requesting changes because the new API fallback is incomplete. The PR says live queue health preserves the GitHub-only fallback when the MergeWork API is unavailable, and load_live_queue() only falls back when _load_api_bounties() raises RuntimeError. However _load_api_bounties() decodes the response before json.loads, and UnicodeDecodeError is not caught or wrapped. A malformed/non-UTF-8 API response therefore aborts live queue collection instead of falling back to GitHub-derived bounty state.
Local reproduction against this head:
UnicodeDecodeError 'utf-8' codec can't decode byte 0xff in position 0: invalid start byte
The repro monkeypatched urlopen() to return b'\xff' and monkeypatched the gh calls to return one open PR referencing open bounty #406. Expected behavior for an unavailable/unusable API response is the same GitHub-only fallback used for URLError; actual behavior is an uncaught decode exception.
Suggested fix: include UnicodeDecodeError in the _load_api_bounties() wrapper and add a regression test for the fallback path. A focused test should prove that when the API raises URLError or returns undecodable bytes, load_live_queue() still returns the GitHub-derived open bounty with no closed-bounty reference.
Validation I ran:
uv run --extra dev python -m pytest tests/test_pr_queue_health.py -q
# 10 passed
uv run --extra dev python -m pytest -q
# 415 passed
uv run --extra dev python scripts/docs_smoke.py
# docs smoke ok
uv run --extra dev ruff check scripts/pr_queue_health.py tests/test_pr_queue_health.py
# All checks passed!
uv run --extra dev ruff format --check scripts/pr_queue_health.py tests/test_pr_queue_health.py
# 2 files already formatted
uv run --extra dev python -m mypy app scripts/pr_queue_health.py
# Success: no issues found in 31 source files
git diff --check origin/main...HEAD
# clean
gitleaks detect --no-banner --redact --source . --log-opts origin/main..HEAD --exit-code 1
# no leaks found
Live readback using this branch also confirmed the success path is wired to the public API: #406 reads as open with 15 awards remaining, #447 as open with 8, and #459 as open with 10. The blocker is specifically the fallback/error path.
jtc268
force-pushed
the
codex/b406-small-fix-191629
branch
from
a8319aa to
89b5f78
Compare
|
Updated current head to address the review blockers:
Validation on current head:
|
GHX5T-SOL
left a comment
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Approved current head 89b5f78b344d8f110a5f9af766bf558b96a77ce7.
This is a current-head follow-up to my earlier changes-requested reviews on old head a8319aad13366ff4fbffb596be60f329041463a6, not a new bounty claim.
I verified both blockers are fixed:
_load_api_bounties()now wrapsUnicodeDecodeError, so malformed/non-UTF-8 API responses preserve the GitHub-only fallback path instead of abortingload_live_queue().- Duplicate API rows for the same
(repo, issue_number)now keep the first/current row instead of allowing a later stale row to overwrite open/payable state.
Current-head validation from /home/kali/money/mergework-pr472-review:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /home/kali/money/mergework/.venv/bin/python -m pytest tests/test_pr_queue_health.py -q->12 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /home/kali/money/mergework/.venv/bin/python -m pytest -q->417 passed/home/kali/money/mergework/.venv/bin/python -m ruff check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> passed/home/kali/money/mergework/.venv/bin/python -m ruff format --check scripts/pr_queue_health.py tests/test_pr_queue_health.py->2 files already formattedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /home/kali/money/mergework/.venv/bin/python -m mypy app scripts/pr_queue_health.py-> successPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /home/kali/money/mergework/.venv/bin/python scripts/docs_smoke.py->docs smoke okgit diff --check upstream/main...HEAD-> cleangit diff --no-ext-diff upstream/main...HEAD | gitleaks stdin --no-banner --redact --exit-code 1-> no leaks foundgh pr checks 472 --repo ramimbo/mergework-> CodeRabbit pass/skipped and quality workflow pass
I also ran the live queue-health command against public GitHub/API reads and it completed with a JSON report. No wallet material, secrets, authenticated production mutation, or private data was used.
4 participants
Refs #406
Summary
scripts/pr_queue_health.py --repo ...read MergeWork bounty capacity from the public bounty API.awards_remainingis0.Why
The runbook says queue health reports closed or exhausted bounty references before maintainer batches. Before this change, live mode only looked at GitHub issue state and treated every open bounty issue as having one payable slot, so an open but exhausted bounty could be missed.
Evidence
GET https://api.mrwk.ltclab.site/api/v1/bounties/66returned issue#406, statusopen,awards_remaining: 15,available_mrwk: "750".GET /api/v1/bounties/66/attempts?include_expired=falsereturned no warnings and no active attempts./claim #Nreference parsing.Validation
uv run --extra dev python -m pytest tests/test_pr_queue_health.py -q-> 10 passeduv run --extra dev ruff check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> passeduv run --extra dev ruff format --check scripts/pr_queue_health.py tests/test_pr_queue_health.py-> passeduv run --extra dev python -m mypy app scripts/pr_queue_health.py-> successgit diff --check-> cleanSummary by CodeRabbit
New Features
Bug Fixes
Tests