deleted-symbols gate: multi-line waiver for large deletions, and say which action re-runs it

[jaylfc]

CARD TITLE (intent, not commit subject): deleted-symbols gate: multi-line waiver for large deletions, and say which action re-runs it

Autonomous build of board card tsk-xuuzct.

Files:
scripts/check_deleted_symbols.py | 10 +++--
tests/test_deleted_symbols.py | 83 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 3 deletions(-)

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Warning

Review limit reached

@jaylfc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 24 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 reviews.

How do review 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 refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6f19e870-396c-4c9c-bbc8-601bc122d5ea

📥 Commits

Reviewing files that changed from the base of the PR and between 0f0a9f8 and 301d452.

📒 Files selected for processing (2)

• scripts/check_deleted_symbols.py
• tests/test_deleted_symbols.py

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gitar-bot]

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

_Gitar

[kilo-code-bot]

SUGGESTION: Instruction message is grammatically awkward and confusing

The message reads: "add this trailer to the PR body and edit the PR body, do not use Re-run job:". The phrase "edit the PR body" is redundant (adding the trailer IS editing the PR body), and the lack of punctuation makes the "do not use Re-run job" clause read as a run-on. Consider: "If these deletions are intentional, add this trailer to the PR body. Do not use Re-run job:"

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
SUGGESTION	1

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
scripts/check_deleted_symbols.py	233	Instruction message is grammatically awkward and confusing

Files Reviewed (2 files)

• scripts/check_deleted_symbols.py - 1 issue
• tests/test_deleted_symbols.py

Fix these issues in Kilo Cloud

---

_{Reviewed by step-3.7-flash · Input: 53.7K · Output: 6.5K · Cached: 189.6K}

[jaylfc]

My card (tsk-xuuzct). The functional half is correct and I proved it by round-tripping rather than reading, which is the same method that cleared #240 and #243. 27/27 tests pass, and independently:

n=6, no waiver                    rc 1   (control: the gate can go red)
printed 6 trailer lines, pasted
  back VERBATIM as the PR body    rc 0   (the hint actually works)
waive 3 of the 6                  rc 1   (the #240 property still holds)
n=5 boundary                      1 line, single-line form, round-trips to rc 0

Worth stating for the record, because it is the thing that could have been wrong and was not: the parser already accepted multi-line trailers (_waived_symbols iterates every line and unions into a set, lines 141-149). So this PR correctly changes only the HINT, which previously offered a form the parser accepted but the message never advertised. That is exactly the shape of the defect #243 fixed, one level down, and the fix is in the right place. The >5 threshold is a sensible split and the tests pin both sides of it.

One fix before merge, and it is the half the card actually asked for.

The new sentence reads:

If these deletions are intentional, add this trailer to the PR body and edit the PR body, do not use Re-run job:

Two problems. It is self-redundant, since adding the trailer to the body IS editing the body, so the instruction appears to ask for two actions when there is one. More importantly it DROPPED the old clause "and the gate will re-run automatically", which was true and was the reassurance that stops someone reaching for Re-run job in the first place. The card asked the message to say which action re-runs it; this version says only which action does not.

Suggested replacement:

If these deletions are intentional, add this trailer to the PR body.
Editing the body re-runs this gate automatically. Do not use "Re-run job":
it replays the old body and will fail again.

The final clause is the bus-2440 finding, and it is the reason the whole card exists: taOS shipped a gate whose documented waiver was unreachable for weeks because re-running the job replays the stale payload. Naming that consequence is what makes the warning stick rather than read as arbitrary.

Verified separately and not a defect here: deleted-symbols-gate.yml does carry types: [..., edited], so "editing re-runs it automatically" is a true statement and safe to put back in the message.

Nothing else. Fix the wording and this is good to merge.

[jaylfc]

Revision card filed: tsk-hyqmvc.

This PR is not being abandoned and it stays open. The card's STEP 0 squash-merges this branch so the work carries forward.

The reason it needs a new card rather than a revision of this one's card is mechanical, and I checked it in source today rather than assuming the tooling had landed: next_card.py:32 excludes any card whose exec/* PR is open, so the original card is unclaimable while this PR exists, and executor.sh still sets BASE per repo (master for taosmd) with no per-card override, so a plain replacement card would rebuild from master and discard this branch entirely.

STEP 0 uses git merge --squash, not git merge, because executor.sh:584 bounces any card whose branch contains a merge commit. The acceptance line requires git rev-list --merges to be empty before push.

Findings are unchanged from my review above; the card summarises them and points back here as the authoritative detail.

[jaylfc]

Superseding card: tsk-zln2uf replaces tsk-hyqmvc.

Same fix, and the do-not-regress evidence is carried over unchanged. What differs is the mechanism: the new card uses the per-card BASE: exec/tsk-xuuzct override that @taOS-dev installed today, and drops the STEP 0 squash entirely. Squashing preserves the work but not the ancestry, so a squashed revision can never contain this branch's head; cutting from the branch makes it an ancestor by construction, and the executor now asserts that at runtime.

This card is also the deliberate first end-to-end trial of that override, chosen because it was unclaimed (so no lane lost work) and because the failure mode it guards against would be invisible here: if the cut silently fell back to master, the entire multi-line waiver work would vanish and the PR would still open clean and green.

[jaylfc]

Closing: this PR is now a REVERT of what already landed.

Its revision, PR #259, was cut from this branch with the BASE: override and merged as 8b20e09. Master therefore already contains this branch's work plus the wording fix. What remains on this branch is only the OLD text, so merging it now would undo the fix:

git diff origin/master origin/exec/tsk-xuuzct -- scripts/check_deleted_symbols.py

-            "If these deletions are intentional, edit the PR body to add this trailer. "
-            "Editing the PR body re-runs this gate automatically; Re-run job does not "
-            "work because it replays the stale payload with the old body:"
+            "If these deletions are intentional, add this trailer to the PR body and "
+            "edit the PR body, do not use Re-run job:"

Note the branch still reports 1 commit ahead of master, because #259 was squash-merged and the commit objects did not transfer. A stale branch after a squash-merge looks identical to a branch with genuine unmerged work, which is why this is worth stating rather than leaving for someone to infer from the ahead-count.

Nothing is lost: the functional half (the multi-line waiver) and the improved hint are both on master.