Make proposed-work payment source selectable#806
Conversation
📝 WalkthroughWalkthroughThe PR extends the proposed work triage script to load payment bounty data from multiple configurable GitHub issue numbers instead of a single hard-coded issue. A new repeatable ChangesConfigurable payment bounty intake
Possibly related issues
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 |
There was a problem hiding this comment.
Reviewed current head e57160e against base 6d324f0.
I checked the CLI argument flow, payment-bounty issue loading, report payment indexing, the new #722 pending-payout fixture, and the admin-runbook guidance. The change keeps the historical #649 default via the fallback path and only switches payment sources when --payment-bounty-issue is provided, including repeatable values during round transitions.
Validation I ran:
python -m pytest tests/test_proposed_work_triage.py-> 14 passedpython -m ruff format --check scripts/proposed_work_triage.py tests/test_proposed_work_triage.pypython -m ruff check scripts/proposed_work_triage.py tests/test_proposed_work_triage.pypython scripts/docs_smoke.pygit diff --check origin/main...HEADgit merge-treeagainst currentorigin/mainproduced no conflicts
Hosted Quality, readiness, docs, and image checks is also passing. No blockers from this 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: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 422ad93d-2449-49d9-ada3-1b87bd908f8a
📒 Files selected for processing (3)
docs/admin-runbook.mdscripts/proposed_work_triage.pytests/test_proposed_work_triage.py
keilogic
left a comment
keilogic
left a comment
There was a problem hiding this comment.
Follow-up review on current head 20434485fb195480e954d2bebeb408e8354b1219 after the 6c3702c aggregation-test commit and the rebase onto current origin/main 424c014ce0e4c80cdb9965e392012f89d25cca58.
I checked the delta from the earlier reviewed e57160e head. The new test covers repeated --payment-bounty-issue 649 / --payment-bounty-issue 722 inputs and asserts both paid and pending submissions contribute to payment_counts, while the base update brings in the merged bounty-issue filter from #808. I did not find a blocker in the current head.
Validation:
python -m pytest tests/test_proposed_work_triage.py -q->16 passedpython -m ruff check scripts/proposed_work_triage.py tests/test_proposed_work_triage.pypython -m ruff format --check scripts/proposed_work_triage.py tests/test_proposed_work_triage.pypython scripts/docs_smoke.pygit diff --check origin/main...HEADgit merge-tree --write-tree origin/main HEAD-> clean treef4b6c483c875ef7de6cc8aa80874a570171101c1- Direct public API reads for bounty issues #649 and #722 returned current rows; #722 still shows pending payout proposals including #791.
Hosted Quality, readiness, docs, and image checks is successful on this head. GitHub currently reports mergeStateStatus=UNSTABLE with the secondary status context unresolved/empty, but I did not see a code blocker in the changed files.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/proposed_work_triage.py (1)
505-528:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
--payment-bounty-issueis silently ignored in offline (--input) mode.When
--inputis supplied, payment state comes from the fixture andargs.payment_bounty_issueis never read. An operator running--input fixture.json --payment-bounty-issue 722during a transition gets no signal that the flag had no effect. Consider erroring (or warning) when both are passed.Proposed guard in main()
args = parser.parse_args(argv) + + if args.input and args.payment_bounty_issue: + parser.error("--payment-bounty-issue is only valid in live (--repo) mode")
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 98f869e5-2a2e-4cde-a266-48f7886b8f65
📒 Files selected for processing (2)
scripts/proposed_work_triage.pytests/test_proposed_work_triage.py
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: a994df51-4f9b-48a6-a38b-c52669ec71ac
📒 Files selected for processing (2)
scripts/proposed_work_triage.pytests/test_proposed_work_triage.py
| def test_proposed_work_triage_rejects_payment_bounty_issue_in_offline_mode( | ||
| tmp_path, capsys | ||
| ) -> None: | ||
| fixture = tmp_path / "fixture.json" | ||
| fixture.write_text(json.dumps({"issues": []}), encoding="utf-8") | ||
|
|
||
| with pytest.raises(SystemExit) as excinfo: | ||
| main(["--input", str(fixture), "--payment-bounty-issue", "722"]) | ||
|
|
||
| assert excinfo.value.code == 2 | ||
| assert "--payment-bounty-issue is only valid in live --repo mode" in capsys.readouterr().err | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add a boundary test for --payment-bounty-issue validation.
The new offline-mode rejection is covered, but _positive_issue_number's reject paths (non-integer and <= 0) are untested. A small case asserting SystemExit(2) for --payment-bounty-issue 0 and a non-numeric value would lock in the positive-int contract from the PR objective. As per coding guidelines, tests should "include negative, replay, boundary, or regression cases where relevant."
Proposed test
`@pytest.mark.parametrize`("value", ["0", "-1", "abc"])
def test_proposed_work_triage_rejects_invalid_payment_bounty_issue(value, capsys) -> None:
with pytest.raises(SystemExit) as excinfo:
main(["--repo", "ramimbo/mergework", "--payment-bounty-issue", value])
assert excinfo.value.code == 2
assert "payment bounty issue must be" in capsys.readouterr().err
keilogic
left a comment
keilogic
left a comment
There was a problem hiding this comment.
Follow-up review on current head 42ae352d1797ca5a15149f0fff8214659bff79f2 after the offline-mode guard commit.
I checked the delta from the previously reviewed 20434485fb195480e954d2bebeb408e8354b1219 head. The new code rejects --input ... --payment-bounty-issue ... instead of silently ignoring the payment-source flag in offline fixture mode, and the new regression test covers that path. This addresses the prior CodeRabbit issue about offline-mode behavior.
CodeRabbit's current boundary-test note for _positive_issue_number is useful polish, but I do not see it as a blocker: direct probes for --payment-bounty-issue 0 and --payment-bounty-issue abc already fail through argparse with the expected positive/integer validation errors.
Validation:
python -m pytest tests/test_proposed_work_triage.py -q->17 passedpython -m ruff check scripts/proposed_work_triage.py tests/test_proposed_work_triage.py-> passedpython -m ruff format --check scripts/proposed_work_triage.py tests/test_proposed_work_triage.py-> passedpython scripts/docs_smoke.py-> docs smoke okgit diff --check origin/main...HEAD-> cleangit merge-tree --write-tree origin/main HEAD-> clean treebc5a52d44390e8eff4b2a01c772a125557637f90- Hosted
Quality, readiness, docs, and image checksand CodeRabbit are successful on this head; GitHub reportsmergeStateStatus=CLEAN.
No blockers found in the current head.
2 participants
Summary
--payment-bounty-issueoption toscripts/proposed_work_triage.pyso live reports can load payment state from the current accepted-proposed-work round.Evidence
scripts/proposed_work_triage.py,tests/test_proposed_work_triage.py,docs/admin-runbook.md.Live read-only smoke:
Test Evidence
./.venv/bin/python scripts/check_agents.py./.venv/bin/python -m pytest./.venv/bin/python -m ruff format --check ../.venv/bin/python -m ruff check ../.venv/bin/python -m mypy app./.venv/bin/python scripts/docs_smoke.pygit diff --checkMRWK
Maintainer infrastructure PR; no bounty requested. Related issue: #802.
Summary by CodeRabbit
New Features
Documentation
Tests