Add externalized, self-verifying audit suppression mechanism#30
Merged
Conversation
Contributor
Test Coverage ReportOverall: 100% (1672/1672 statements covered) Coverage by file
Updated by PR Tests |
…skip ci] Recurring audit false positives (deliberate deprecated-alias methods, intentionally partial holiday enums, the backward-compat State.REMOVED value) previously reappeared on every run. They are now declared in a data file, specs/audit-ignore.json — nothing to ignore is hard-coded in the audit script. scripts/audit_sdk.py now: - builds structured findings with stable (type, key) fingerprints - loads specs/audit-ignore.json (override: --ignore-file; absent = none) - suppresses a finding ONLY while it still occurs; for enums only the listed values are hidden, so newly added values still surface - reports hidden findings under "Suppressed (Verified)" with reasons and surfaces entries that match nothing under "Stale Ignores" - excludes suppressed findings from the active counts (clean report) Also fixes scan_string_concat_issues to match its documented intent: flag implicit string concatenation only inside [ ] list displays (a real missing-comma bug, e.g. in nullable/mandatory lists), not in parenthesised assignments or call arguments. This removes 8 long-standing false positives at the source without needing ignore entries, while the check now actually catches missing commas in list literals. The maintain-audit skill (Phase 2) now re-verifies suppressions on every run: it re-confirms each ignore's reason against the current code/spec, recommends removing stale or no-longer-valid entries (resurfacing the real finding), and flags newly surfaced enum values not covered by an ignore — so the agent, not just the script, keeps the ignore list honest each audit. Report section headers and Coverage Summary keys are preserved so format_pr_comment.py keeps working; counts now reflect active findings. Adds tests/test_audit_ignore.py (28 tests) covering ignore loading, suppression, enum value-subset verification, stale detection, enum finding computation, and the list-context detector fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
amitray007
force-pushed
the
feat/audit-ignore-mechanism
branch
from
8792807 to
1d56c23
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Recurring audit false positives (deliberate deprecated-alias methods, the intentionally partial holiday enums, the backward-compat
State.REMOVEDvalue) reappeared on everyaudit_sdk.pyrun. This adds an externalized, self-verifying suppression mechanism so they stop being noise — without hard-coding anything in the script. Accepted exceptions live entirely inspecs/audit-ignore.json.How it works
On every run,
audit_sdk.py:(type, key)fingerprints;specs/audit-ignore.json(override with--ignore-file; a missing file means "no suppressions");valuesare hidden, so a newly added value still surfaces;## Suppressed (Verified), and surfaces entries that match nothing under## Stale Ignoresso the list can't silently rot;This directly answers the requirement: don't hard-code anything; store what to ignore externally; re-check and verify the ignores on every audit.
Currently suppressed (7 reviewed entries)
extra_method×4get_listings_by_listings_ids,get_shop_payment_account_ledger_entry_payments,get_shop_receipt_transaction_by_shop,get_shop_shipping_profile_destination_by_shipping_profile(each delegates + warns)enum_stalenessShopHolidayPreference.holiday_id -> CA_HOLIDAYS[missing:*] — SDK intentionally names US/CA holidays and passes other IDs as integersenum_staleness×2ShopListing.state/ShopListingWithAssociations.state -> State[extra:removed] — backward-compat value, documented inlineBonus: detector precision fix
scan_string_concat_issuescontradicted its own docstring ("in list literals") by flagging all adjacent string literals. It now flags implicit concatenation only inside[ ]list displays — the real missing-comma bug class (e.g. innullable/mandatorylists) — and ignores parenthesised assignments / call arguments. This removes 8 long-standing false positives at the source (no ignore entries needed) while making the check genuinely useful.Test plan
pytest→ 334 passed (28 new intests/test_audit_ignore.py)Extra: 0, Code issues: 0, Suppressed: 7, "All enum values are in sync", "No stale ignores"4extra methods +3enum blocks resurface;0code issues (detector fix is independent)format_pr_comment.pystill parses the report (headers + Coverage Summary keys preserved; counts now reflect active findings)Note on versioning
Tooling/CI only —
scripts/,tests/, andspecs/are not part of the published package. To avoid a spurious version bump + identical PyPI republish, add[skip ci]to the squash-merge commit.🤖 Generated with Claude Code