Guard PR bodies against premature payment-status wording (#1107)#1177
Guard PR bodies against premature payment-status wording (#1107)#1177yanyishuai wants to merge 1 commit into
Conversation
|
Warning Review limit reached
More reviews will be available in 56 minutes and 11 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds ChangesPayment Language Guard
Possibly related issues
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4ac2489e-7f1f-4882-9747-4afb086748a8
📒 Files selected for processing (5)
scripts/check_pr_payment_language.pyscripts/public_payment_language.pyscripts/submission_quality_gate.pytests/test_public_payment_language.pytests/test_submission_quality_gate.py
| pr = _load_pull_request(args.repo, args.pr) | ||
| text = "\n".join(str(pr.get(key) or "") for key in ("title", "body")) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check only the PR body in GitHub mode.
This CLI is described as a PR-body checker, but this path concatenates the title and body. A compliant PR body will still fail if the title says something like ban "paid" wording, which changes the gate's contract from the one described in this PR.
Suggested fix
- text = "\n".join(str(pr.get(key) or "") for key in ("title", "body"))
+ text = str(pr.get("body") or "")📝 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.
| pr = _load_pull_request(args.repo, args.pr) | |
| text = "\n".join(str(pr.get(key) or "") for key in ("title", "body")) | |
| pr = _load_pull_request(args.repo, args.pr) | |
| text = str(pr.get("body") or "") |
| def find_payment_language_violations(text: str) -> list[str]: | ||
| """Return human-readable violations for premature payment/status wording.""" | ||
| if not text or not text.strip(): | ||
| return [] | ||
|
|
||
| violations: list[str] = [] | ||
| if _PAYOUT_BOUNDARY_RE.search(text): | ||
| violations.append( | ||
| "deprecated 'Payout boundary' heading found; prefer neutral 'Submission status' wording" | ||
| ) | ||
| if _LEGACY_WITHDRAWABLE_RE.search(text): | ||
| violations.append( | ||
| "legacy 'not confirmed or withdrawable' phrasing found; use neutral submission status language" | ||
| ) | ||
|
|
||
| for line in text.splitlines(): | ||
| stripped = line.strip() | ||
| if not stripped or stripped.startswith("#"): | ||
| continue | ||
| if _line_is_allowlisted(line): | ||
| continue | ||
| for pattern in _RESERVED_STATUS_ASSERTION_RES: | ||
| if pattern.search(line): | ||
| violations.append( | ||
| "reserved payment/status wording used as a claim assertion: " | ||
| f"{stripped[:120]}" | ||
| ) | ||
| break |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run all payment-language rules through the same per-line classifier.
The initial whole-text search bypasses _line_is_allowlisted, so instructional lines like Do not write "not confirmed or withdrawable" still fail. Then stripped.startswith("#") skips headings entirely, so ## Claim is paid or ## Withdrawable status never reaches the reserved-assertion regexes. That gives this detector both false positives and false negatives.
Suggested fix
def find_payment_language_violations(text: str) -> list[str]:
"""Return human-readable violations for premature payment/status wording."""
if not text or not text.strip():
return []
violations: list[str] = []
- if _PAYOUT_BOUNDARY_RE.search(text):
- violations.append(
- "deprecated 'Payout boundary' heading found; prefer neutral 'Submission status' wording"
- )
- if _LEGACY_WITHDRAWABLE_RE.search(text):
- violations.append(
- "legacy 'not confirmed or withdrawable' phrasing found; use neutral submission status language"
- )
for line in text.splitlines():
stripped = line.strip()
- if not stripped or stripped.startswith("#"):
+ if not stripped:
continue
if _line_is_allowlisted(line):
continue
+ if _PAYOUT_BOUNDARY_RE.search(line):
+ violations.append(
+ "deprecated 'Payout boundary' heading found; prefer neutral 'Submission status' wording"
+ )
+ if _LEGACY_WITHDRAWABLE_RE.search(line):
+ violations.append(
+ "legacy 'not confirmed or withdrawable' phrasing found; use neutral submission status language"
+ )
for pattern in _RESERVED_STATUS_ASSERTION_RES:
if pattern.search(line):
violations.append(
"reserved payment/status wording used as a claim assertion: "
f"{stripped[:120]}"📝 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 find_payment_language_violations(text: str) -> list[str]: | |
| """Return human-readable violations for premature payment/status wording.""" | |
| if not text or not text.strip(): | |
| return [] | |
| violations: list[str] = [] | |
| if _PAYOUT_BOUNDARY_RE.search(text): | |
| violations.append( | |
| "deprecated 'Payout boundary' heading found; prefer neutral 'Submission status' wording" | |
| ) | |
| if _LEGACY_WITHDRAWABLE_RE.search(text): | |
| violations.append( | |
| "legacy 'not confirmed or withdrawable' phrasing found; use neutral submission status language" | |
| ) | |
| for line in text.splitlines(): | |
| stripped = line.strip() | |
| if not stripped or stripped.startswith("#"): | |
| continue | |
| if _line_is_allowlisted(line): | |
| continue | |
| for pattern in _RESERVED_STATUS_ASSERTION_RES: | |
| if pattern.search(line): | |
| violations.append( | |
| "reserved payment/status wording used as a claim assertion: " | |
| f"{stripped[:120]}" | |
| ) | |
| break | |
| def find_payment_language_violations(text: str) -> list[str]: | |
| """Return human-readable violations for premature payment/status wording.""" | |
| if not text or not text.strip(): | |
| return [] | |
| violations: list[str] = [] | |
| for line in text.splitlines(): | |
| stripped = line.strip() | |
| if not stripped: | |
| continue | |
| if _line_is_allowlisted(line): | |
| continue | |
| if _PAYOUT_BOUNDARY_RE.search(line): | |
| violations.append( | |
| "deprecated 'Payout boundary' heading found; prefer neutral 'Submission status' wording" | |
| ) | |
| if _LEGACY_WITHDRAWABLE_RE.search(line): | |
| violations.append( | |
| "legacy 'not confirmed or withdrawable' phrasing found; use neutral submission status language" | |
| ) | |
| for pattern in _RESERVED_STATUS_ASSERTION_RES: | |
| if pattern.search(line): | |
| violations.append( | |
| "reserved payment/status wording used as a claim assertion: " | |
| f"{stripped[:120]}" | |
| ) | |
| break |
qingfeng312
left a comment
qingfeng312
left a comment
There was a problem hiding this comment.
CI is currently failing on the new submission-quality gate test.
The implementation adds the payment_status_language check to evaluate_submission, so the result now contains one additional check entry. tests/test_submission_quality_gate.py::test_submission_quality_gate_passes_open_bounty_with_evidence still asserts the old exact set of six checks and fails because the left side includes {"payment_status_language": "pass"}.
Please update the expected check set in that test, and any related fixed-shape assertions, so the new payment-status-language guard is covered by the passing path.
Evidence: GitHub Actions run 28343359265, job 83961953872, 1 failed, 912 passed.
yanyishuai
force-pushed
the
fix/issue-1107-pr-payment-language
branch
from
fbde72f to
6d89af2
Compare
|
@qingfeng312 Addressed the failing quality-gate test.
Latest head: |
|
@qingfeng312 CI is green on the latest head ( |
1 similar comment
|
@qingfeng312 CI is green on the latest head ( |
qingfeng312
left a comment
qingfeng312
left a comment
There was a problem hiding this comment.
Follow-up on current head 6d89af2bd6df91cadfbf427a2e0987ae74ec6440.
The earlier quality-gate expectation blocker has been resolved. I rechecked the payment/status-language detector, the standalone PR-body checker, the submission quality gate integration, and the associated tests.
The updated test coverage now includes the new payment_status_language pass case, and the guard remains focused on reserved payment/status wording while allowing neutral lifecycle language.
Validation checked: GitHub CI Quality, readiness, docs, and image checks passed on run 28349183723; CodeRabbit status is success on this head. I did not find a remaining blocker for the scoped #1107 change.
|
@qingfeng312 Thanks again for the approval on #1107 — the payment-language guard remains green on Merge-ready whenever convenient. |
|
@qingfeng312 Follow-up on #1107 — payment-language guard still passes the full quality gate on Merge-ready whenever convenient. |
5 similar comments
|
@qingfeng312 Follow-up on #1107 — payment-language guard still passes the full quality gate on Merge-ready whenever convenient. |
|
@qingfeng312 Follow-up on #1107 — payment-language guard still passes the full quality gate on Merge-ready whenever convenient. |
|
@qingfeng312 Follow-up on #1107 — payment-language guard still passes the full quality gate on Merge-ready whenever convenient. |
|
@qingfeng312 Follow-up on #1107 — payment-language guard still passes the full quality gate on Merge-ready whenever convenient. |
|
@qingfeng312 Follow-up on #1107 — payment-language guard still passes the full quality gate on Merge-ready whenever convenient. |
2 participants
Summary
Adds a bounded public payment/status-language guard for MergeWork submission drafts and PR bodies.
scripts/public_payment_language.pyhelper detects deprecatedPayout boundaryheadings, legacy "not confirmed or withdrawable" phrasing, and reserved paid/settled/received/withdrawable claim assertions.scripts/check_pr_payment_language.pyCLI for standalone PR-body or text-file checks.scripts/submission_quality_gate.pynow fails when premature payment/status wording is present and suggests neutralSubmission statuslanguage instead.Why
Docs already reserve payment/status words for proof-backed states, but the quality gate did not enforce that rule. This closes the manual review gap for open PRs like #1072/#1075/#1077.
Tests
Wallet
Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpECloses #1107
Summary by CodeRabbit
New Features
Bug Fixes