Add treasury proposal filters and links#639
Conversation
📝 WalkthroughWalkthroughThe PR adds ChangesTreasury Proposal URLs and List Filtering
Possibly Related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 282c478b-cf97-4b0e-b9a3-b1247988d0b7
📒 Files selected for processing (6)
app/treasury.pyapp/treasury_routes.pydocs/admin-runbook.mddocs/agent-guide.mddocs/api-examples.mdtests/test_treasury_proposals.py
| TREASURY_PROPOSAL_STATUSES = {"pending", "executed", "blocked"} | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider moving TREASURY_PROPOSAL_STATUSES to treasury.py for consistency.
TREASURY_ACTIONS is defined in treasury.py and imported here. Colocating both constants in the domain module would make the allowed values easier to maintain and discover.
|
Maintainer triage 2026-05-30T05:55Z: CI is green and this appears within #597 scope, but I am holding it for at least two useful current-head human reviews before merge. CodeRabbit’s current thread is a low-value nit, not a blocker here. |
yui-stingray
left a comment
There was a problem hiding this comment.
Reviewed current head b3c0054911cb077a9f3c010c34169b320848d82f.
I found one blocker in the new treasury proposal filters. _normalized_filter() strips and lowercases the raw query value before validating membership, so a control-character-padded value can be accepted as valid after normalization, for example status=%09pending or action=%C2%85create_bounty.
That looks inconsistent with the recent control-character hardening pattern in this repository: raw control characters should be rejected before trim/lower normalization, especially on newly introduced public query filters. Please add coverage that these new status and action filters return a bounded 400 for raw control characters before accepting this path.
Evidence checked: public diff for app/treasury_routes.py and tests/test_treasury_proposals.py, current head SHA above, mergeable metadata, and hosted Quality, readiness, docs, and image checks reporting success. I did not run local tests. CodeRabbit’s current inline note is about constant placement and does not cover this input-normalization boundary.
|
Addressed the current-head filter validation review in commit eb86b41. What changed:
Verification:
|
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)
app/treasury_routes.py (1)
1-1:⚠️ Potential issue | 🟠 MajorBlocker: required checks for this Python change aren’t runnable here (mypy/pytest missing)
ruff format --check .✅ (87 files already formatted; all checks passed)ruff check .✅ (all checks passed)mypy app❌ (python: No module named mypy)pytest❌ (pytest: command not found)Please ensure
mypy appandpytestrun successfully in the PR/CI environment per the repo guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 27179010-e8aa-40ce-a7eb-cff099261a17
📒 Files selected for processing (2)
app/treasury_routes.pytests/test_treasury_proposals.py
chinook1001
left a comment
There was a problem hiding this comment.
Reviewed current head eb86b41ca74fa046cbe02435ee2044ec01693741 after the follow-up commits.
I approve this updated version. The earlier filter-normalization concern is addressed on the current head: _normalized_filter() now checks CONTROL_CHAR_RE against the raw query value before strip().lower(), so control-character-padded filters are rejected instead of normalized into accepted values.
Evidence checked:
- Inspected
app/treasury_routes.py,app/treasury.py,tests/test_treasury_proposals.py,docs/admin-runbook.md,docs/agent-guide.md, anddocs/api-examples.md. - Verified
statusandactionfilters are restricted to explicit allowlists and compose withlimitwhile preserving newest-first ordering. - Verified raw control-character test coverage for
status=%09pendingandaction=%C2%85create_bounty, plus invalid enum coverage for unsupported status/action values. - Verified proposal/challenge dictionaries now expose detail URLs, and executed proposals expose
executed_ledger_urlonly when an executed ledger sequence exists. - Checked hosted PR metadata: current head matches the reviewed commit, PR is mergeable/clean, and
Quality, readiness, docs, and image checksis green.
Local validation:
./.venv/bin/python -m pytest tests/test_treasury_proposals.py tests/test_docs_public_urls.py -q-> 57 passed, 1 Starlette/httpx deprecation warning../.venv/bin/python scripts/docs_smoke.py-> docs smoke ok../.venv/bin/python -m ruff check app/treasury.py app/treasury_routes.py tests/test_treasury_proposals.py-> passed../.venv/bin/python -m ruff format --check app/treasury.py app/treasury_routes.py tests/test_treasury_proposals.py-> already formatted.git diff --check origin/main...HEAD-> clean.
Verdict: APPROVED — the current head fixes the raw-control-character filter issue and the API/docs additions are covered by targeted tests. No private data, credentials, wallet material, production mutation, price/exchange/bridge claims, or fabricated payout claims used.
xingxi0614-cpu
left a comment
There was a problem hiding this comment.
Reviewed current head eb86b41ca74fa046cbe02435ee2044ec01693741.
I am requesting changes for mergeability rather than for the filter implementation itself. The follow-up commit appears to address the earlier raw-control-character concern by checking CONTROL_CHAR_RE before strip().lower(), and the PR includes regression coverage for status=%09pending and action=%C2%85create_bounty.
Evidence checked:
- Inspected the current PR diff for
app/treasury.py,app/treasury_routes.py,tests/test_treasury_proposals.py,docs/admin-runbook.md,docs/agent-guide.md, anddocs/api-examples.md. - Verified the current PR metadata reports
mergeable=CONFLICTING/mergeStateStatus=DIRTY. - Reproduced the current-base merge conflict with a local merge-tree against current
origin/main; the conflict is indocs/api-examples.md. - Verified hosted checks are green on the PR head:
Quality, readiness, docs, and image checkspassed, and CodeRabbit passed/skipped without a current blocker.
Required follow-up:
- Please rebase or merge current
main, resolve thedocs/api-examples.mdconflict, and rerun the relevant docs/API checks on the rebased head.
Verdict: REQUEST CHANGES until the current-base conflict is resolved. I did not run local tests because the branch does not currently merge cleanly onto main. No private data, credentials, wallet material, production mutation, price/exchange/bridge claims, or fabricated payout claims used.
|
Needs current-base refresh before maintainer review. This branch is currently conflicting/dirty against |
|
Reviewed current head The implementation checks out locally, but the branch still needs a rebase/merge refresh before it is mergeable. I reproduced the conflict with:
The conflict is in the treasury examples block: current Local checks run on the current PR head before attempting any merge resolution:
So my requested follow-up is narrow: rebase or merge current |
Review: Add treasury proposal filters and links (PR #639)Files inspected: Behavior checked:
CI status: Mergeable state is Finding: Verdict: Clean, focused PR. Small scope, well-tested. The dirty merge state is the only blocker — a rebase should fix it. |
Gwani-28
left a comment
There was a problem hiding this comment.
Reviewed PR #639 at current head eb86b41ca74fa046cbe02435ee2044ec01693741.
Blocker: this PR is currently not mergeable against origin/main because the merge tree conflicts in two docs files:
docs/admin-runbook.mddocs/api-examples.md
Evidence:
- GitHub reports the PR merge state as
DIRTY. git merge-tree --write-tree origin/main HEADexits non-zero and reports content conflicts indocs/admin-runbook.mdanddocs/api-examples.md.- The code-side changes look scoped to proposal filters/links:
proposal_url,executed_ledger_url,status/actionfilters, and validation for raw control characters. - I inspected
app/treasury.py,app/treasury_routes.py, the docs updates, andtests/test_treasury_proposals.py.
Validation on the PR head still passes:
.venv/bin/python -m pytest tests/test_treasury_proposals.py tests/test_bounty_api.py tests/test_api_mcp.py -q-> 128 passed, 1 warning..venv/bin/ruff check app/treasury.py app/treasury_routes.py tests/test_treasury_proposals.py-> passed..venv/bin/ruff format --check app/treasury.py app/treasury_routes.py tests/test_treasury_proposals.py-> passed, 3 files already formatted..venv/bin/python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.
Suggested maintainer action: rebase/merge current main and resolve the two documentation conflicts, then rerun the focused treasury proposal tests and docs smoke. I would not merge this PR in its current state because the conflict blocks integration even though the PR head itself tests cleanly.
Add a read-only recipient filter so agents and maintainers can reconcile pending pay_bounty proposals for one account without mixing unrelated payout rows from busy rounds. Keep limit semantics after payload filtering so recipient-specific pages do not drop older matching rows behind newer unrelated proposals, and stream rows instead of materializing the unbounded filtered candidate set. Constraint: Source issue ramimbo#680 is accepted in the ramimbo#649 proposed-work intake for ramimbo#647, while bounty ramimbo#95 still has available awards. Rejected: Client-side filtering only | it keeps the public API gap from ramimbo#680 and fails account-specific reconciliation. Confidence: high Scope-risk: narrow Directive: If PR ramimbo#639 lands first, rebase and keep this PR focused on the to_account recipient filter plus docs/tests. Tested: PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests/test_treasury_proposals.py -k "recipient or treasury_proposals_list_honors_limit"; PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests/test_treasury_proposals.py; python -m ruff check app/treasury_routes.py tests/test_treasury_proposals.py; python -m ruff format app/treasury_routes.py tests/test_treasury_proposals.py; python scripts/docs_smoke.py; git diff --check Not-tested: Production API behavior against live pending payout data.
JONASXZB
left a comment
There was a problem hiding this comment.
Reviewed current head eb86b41ca74fa046cbe02435ee2044ec01693741 against current origin/main 0eb12a384da80cacf669b0fb98dc06afefbbd2e3.
Requesting changes because the branch is still not mergeable and the PR still needs a live-scope/payment-path refresh before maintainers can safely act on it. Evidence:
- GitHub reports
mergeable_state=dirtyfor the current head. git merge-tree --write-tree origin/main HEADexits non-zero with content conflicts indocs/admin-runbook.mdanddocs/api-examples.md. The non-lossy docs resolution should keep currentmain'shttps://api.mrwk.onlineexamples,/api/v1/treasury/statuscoverage, executor/finalization/admin-token warnings, and availability metadata text while also retaining this PR's proposalstatus/actionfilter examples plusproposal_url/executed_ledger_urlresponse-field note.- The PR body still references
/claim #597. A live bounty preflight on 2026-06-01 shows source issue #597 isavailability_state=pending_payouts_full,effective_awards_remaining=0,effective_available_mrwk=0, with 6 pending payout proposals. That matches the maintainer's request to confirm a live scope/payment path before review/merge; please clarify or refresh the bounty reference after rebasing so the PR does not present an effectively unavailable bounty as claimable.
Implementation-side checks look good on the PR head before the merge conflict:
- inspected
app/treasury.py,app/treasury_routes.py,tests/test_treasury_proposals.py,docs/admin-runbook.md,docs/agent-guide.md, anddocs/api-examples.md; - verified
proposal_url/executed_ledger_urlserialization, boundedstatusandactionallowlists, raw control-character rejection before trim/lower normalization, and no payout/ledger mutation path changes beyond response metadata/filtering; - ran
MERGEWORK_DATABASE_URL=sqlite:////tmp/mergework-pr639-review-test.sqlite python -m pytest tests/test_treasury_proposals.py -q-> 33 passed, 1 warning; - ran
python scripts/docs_smoke.py, targeted Ruff check/format,python -m mypy app/treasury.py app/treasury_routes.py, and a local treasury proposal filter/link smoke; - hosted status is green for this head (
CodeRabbit: success;Quality, readiness, docs, and image checks: success).
So the remaining blocker is current-base refresh plus bounty-path clarification, not the core filter/link implementation.
Jorel97
left a comment
There was a problem hiding this comment.
Reviewed current head eb86b41ca74fa046cbe02435ee2044ec01693741 against current origin/main 2a69dd04c541272d2d2cbd4d54f3b6531a87b175 as a non-author.
The PR-head implementation still looks focused and its targeted checks pass, but I am requesting changes because this branch is now materially stale against current main and the original bounty path is no longer live.
Current blockers:
- GitHub reports
mergeStateStatus: DIRTY. git merge-tree --write-tree origin/main HEADexits non-zero against current main and reports content conflicts in all six touched files:app/treasury.py,app/treasury_routes.py,docs/admin-runbook.md,docs/agent-guide.md,docs/api-examples.md, andtests/test_treasury_proposals.py.- This is broader than the earlier docs-only conflict state. A refresh needs to preserve current main's newer treasury proposal filtering/recipient/bounty-id behavior and finalization/admin docs while retaining this PR's useful proposal URL / executed ledger URL response additions if maintainers still want them.
- The PR body still says
Refs #597and/claim #597, but a live bounty lookup now showsramimbo/mergework#597isstatus=paid,availability_state=paid,awards_remaining=0, andeffective_awards_remaining=0. Please refresh or remove the stale claim path so the PR does not present an exhausted bounty as currently claimable.
Implementation-side evidence on the PR head before merge resolution:
- Inspected
app/treasury.py,app/treasury_routes.py,docs/admin-runbook.md,docs/agent-guide.md,docs/api-examples.md, andtests/test_treasury_proposals.py. - Verified the PR-head code adds
proposal_url,executed_ledger_url,status/actionfilters, raw control-character rejection before trim/lower normalization, and focused treasury proposal coverage. - Hosted Quality/readiness/docs/image and CodeRabbit are successful for this head.
Validation run locally on this head:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests/test_treasury_proposals.py tests/test_docs_public_urls.py -q-> 57 passed, 1 existing Starlette/httpx warning.ruff check app/treasury.py app/treasury_routes.py tests/test_treasury_proposals.py-> passed.ruff format --check app/treasury.py app/treasury_routes.py tests/test_treasury_proposals.py-> 3 files already formatted.python -m mypy app/treasury.py app/treasury_routes.py-> success.python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.
No private data, credentials, wallet material, production mutation, payout/proof/ledger execution, exchange, bridge, cash-out, price behavior, or fabricated payout claims were used or changed.
vondutchi
left a comment
There was a problem hiding this comment.
Reviewed current head eb86b41ca74fa046cbe02435ee2044ec01693741 as a non-author against refreshed origin/main 6d324f048319a84cf4b9972d31e6cb81cd4ba036 (merge-base 1e424d21e0921d5fa56829d6d1d305262ccee46d).
The treasury proposal filter/link implementation still validates on the PR head, but this branch is substantially out of date against current main.
Evidence checked:
- Inspected the current six-file diff:
app/treasury.py,app/treasury_routes.py,docs/admin-runbook.md,docs/agent-guide.md,docs/api-examples.md, andtests/test_treasury_proposals.py. .venv/bin/python -m pytest tests/test_treasury_proposals.py -q->33 passed, 1 warning in 1.45s..venv/bin/python scripts/docs_smoke.py->docs smoke ok..venv/bin/ruff check app/treasury.py app/treasury_routes.py tests/test_treasury_proposals.py-> all checks passed..venv/bin/ruff format --check app/treasury.py app/treasury_routes.py tests/test_treasury_proposals.py->3 files already formatted..venv/bin/mypy app/treasury.py app/treasury_routes.py->Success: no issues found in 2 source files.git diff --check origin/main...HEADpassed.- Current-base mergeability is blocked:
git merge-tree --write-tree origin/main HEADexits non-zero with content conflicts inapp/treasury.py,app/treasury_routes.py,docs/admin-runbook.md,docs/agent-guide.md, anddocs/api-examples.md; merge-tree output included tree58a3d80e8588b9f023bd8d11b8db023a94c56f0b. GitHub reportsmergeStateStatus=DIRTY. - Hosted status rollup shows
Quality, readiness, docs, and image checkssuccessful and CodeRabbit successful on the PR head.
Requested follow-up: rebase/merge current main, resolve the treasury/docs conflicts, and rerun focused treasury proposal tests plus docs smoke before merge. No private data, wallet material, payout execution, treasury mutation, exchange, bridge, cash-out, or price behavior was used.
|
Maintainer queue pass 2026-06-02: closing as stale/superseded. |
Summary
proposal_urlandexecuted_ledger_urlto treasury proposal responses, plusproposal_urlon challenge responsesstatusandactionfilters onGET /api/v1/treasury/proposalsRefs #597
/claim #597
Tests
./.venv/bin/ruff check app/treasury.py app/treasury_routes.py tests/test_treasury_proposals.py./.venv/bin/python -m pytest tests/test_treasury_proposals.py./.venv/bin/python scripts/docs_smoke.py./.venv/bin/python -m pytestSummary by CodeRabbit