Skip to content

deleted-symbols gate: the failure message never tells you the waiver exists - #243

Merged
jaylfc merged 1 commit into
masterfrom
exec/tsk-gkmvcp
Aug 13, 2026
Merged

deleted-symbols gate: the failure message never tells you the waiver exists#243
jaylfc merged 1 commit into
masterfrom
exec/tsk-gkmvcp

Conversation

@jaylfc

@jaylfc jaylfc commented Aug 13, 2026

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): deleted-symbols gate: the failure message never tells you the waiver exists

Autonomous build of board card tsk-gkmvcp.

The waiver trailer is documented only in a workflow comment and the
module docstring, neither of which a contributor reads while CI is red.
Print a ready-to-paste trailer line built from the symbols that actually
failed right in the FAIL message so the escape hatch is discoverable.

Add tests asserting the hint appears on failure and is absent from the
clean output.

Files:
scripts/check_deleted_symbols.py | 9 +++++++++
tests/test_deleted_symbols.py | 25 +++++++++++++++++++++++++
2 files changed, 34 insertions(+)

…lure output

The waiver trailer is documented only in a workflow comment and the
module docstring, neither of which a contributor reads while CI is red.
Print a ready-to-paste trailer line built from the symbols that actually
failed right in the FAIL message so the escape hatch is discoverable.

Add tests asserting the hint appears on failure and is absent from the
clean output.
@qodo-code-review

Copy link
Copy Markdown

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

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 23 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: 5ec0646a-e809-436e-a465-c89dddb81277

📥 Commits

Reviewing files that changed from the base of the PR and between f6b5a17 and 4143614.

📒 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

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

@gitar-bot

gitar-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

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

kilo-code-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • scripts/check_deleted_symbols.py
  • tests/test_deleted_symbols.py

Reviewed by step-3.7-flash · Input: 73.6K · Output: 19.3K · Cached: 154.4K

@jaylfc

jaylfc commented Aug 13, 2026

Copy link
Copy Markdown
Owner Author

Approved. The waiver round-trips, proven at the case the tests do not cover

This is my card (tsk-gkmvcp), so I reviewed it against the thing that made me file it: #240
shipped a waiver that was documented only in a YAML comment and a docstring, and taOS shipped a
sibling gate whose documented waiver was unreachable for weeks (bus 2440). A hint that prints the
wrong format would be the same bug wearing a fix.

The two tests here cover one deleted symbol. The interesting case is the one where the hint is
actually constructed, ", ".join(...) over several violations, so I ran that:

=== 1 deleted symbol(s) ===
  rc without waiver         : 1  (expect 1)
  hint the gate prints      : Removes-Intentionally: taosmd/service.py:sym_0
  rc with that hint pasted  : 0  PASS
  CONTROL wrong symbol      : 1  good, still fails

=== 3 deleted symbol(s) ===
  rc without waiver         : 1  (expect 1)
  hint the gate prints      : Removes-Intentionally: taosmd/service.py:sym_0, taosmd/service.py:sym_1, taosmd/service.py:sym_2
  rc with that hint pasted  : 0  PASS
  CONTROL wrong symbol      : 1  good, still fails
  CONTROL partial waiver    : 1  good, still fails for the rest

The round trip is real in both shapes: the exact string the gate prints, pasted back verbatim,
turns rc 1 into rc 0. The comma-joined form matches the parser, which splits on , at
check_deleted_symbols.py:145. Both controls matter, because without them a rc=0 proves nothing:
a wrong symbol still fails, and waiving one of three still fails for the other two, which is the
#240 property (46 down to 45 and still red).

I also checked the claim the new message makes, since it is an assertion about CI rather than
about this script. .github/workflows/deleted-symbols-gate.yml triggers on
types: [opened, synchronize, reopened, edited], so "the gate will re-run automatically" on a
body edit is true. That is the exact trap from bus 2440, and this repo is on the right side of it.

22/22 pass. test, deleted-symbols-gate, CodeRabbit and Gitar all green, 0 commits behind
master. Merging.

One follow-up, not a blocker

The parser accumulates across lines, so multiple Removes-Intentionally: lines work as well as
one comma-joined line. For a large deletion (the incident that motivated #240 named 46 symbols)
the single joined line gets unwieldy. Printing one trailer per line above a threshold would read
better and parses identically.

Worth considering too: telling the reader NOT to use "re-run job" after adding the trailer. Per
@taOS-dev at bus 2440, a re-run replays the stale payload with the old body, so it fails again and
looks like the waiver is broken. The body edit is what re-triggers it. Since this PR exists to
make the waiver discoverable, saying which action works is in scope for a later pass.

@jaylfc
jaylfc merged commit 53d147b into master Aug 13, 2026
5 checks passed
@jaylfc
jaylfc deleted the exec/tsk-gkmvcp branch August 13, 2026 17:07
jaylfc added a commit that referenced this pull request Aug 17, 2026
…e container, and three merged PRs still unlogged (#256)

* carry forward the tsk-twljv4 work

* fix: deduplicate /version CHANGELOG entry, reword README container sentence, log #240 #243 #245
jaylfc added a commit that referenced this pull request Aug 17, 2026
…lure output (#243)

The waiver trailer is documented only in a workflow comment and the
module docstring, neither of which a contributor reads while CI is red.
Print a ready-to-paste trailer line built from the symbols that actually
failed right in the FAIL message so the escape hatch is discoverable.

Add tests asserting the hint appears on failure and is absent from the
clean output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant