fix: audit parameter-level enums (close silent-drift blind spot)#31
Merged
Conversation
amitray007
force-pushed
the
fix/audit-parameter-enum-blind-spot
branch
from
5c524f1 to
c4e322d
Compare
Contributor
Test Coverage ReportOverall: 100% (1672/1672 statements covered) Coverage by file
Updated by PR Tests |
…ip ci] The SDK audit went silent on a real spec change: Etsy removed 'Shipping' and 'Inventory' from the `includes` query-param enum on getListing and getListingsByListingIds. Neither the audit nor the diff caught it. Root causes (two blind spots in the maintenance tooling): - audit_sdk.get_spec_enums() only extracted enums from components/schemas, never from operation parameters — so all 18 parameter-level enums (includes, state, sort_on, holiday_id, ...) were never audited. Now walks paths/*/*/parameters, covering both schema.enum and schema.items.enum (array params). - diff_spec.diff_parameters() only compared schema.enum, missing schema.items.enum, so array-param enum changes read as "No changed endpoints". Now diffs items.enum too and reports added/removed values. Surfacing parameter enums also exposed a latent collision: scan_enum_values keyed SDK enums by class name alone, so Includes in ListingInventory.py (1 value) clobbered Includes in Listing.py (9 values). Now each name maps to a list of value-lists and compute_enum_findings picks the best-overlap candidate per spec enum. The now-visible Includes drift is intentional: SHIPPING/INVENTORY are kept for backward compatibility and remain valid on getListingsByShop (which shares the enum), so removing them would break callers. Documented inline in enums/Listing.py and suppressed in specs/audit-ignore.json. The param-side State.REMOVED and holiday_id findings mirror existing accepted decisions and are suppressed likewise. Baseline refreshed to the reviewed spec. Adds regression tests for parameter-enum extraction, the collision fix, and updates existing compute_enum_findings tests to the new sdk_enums shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
amitray007
force-pushed
the
fix/audit-parameter-enum-blind-spot
branch
from
c4e322d to
fb913f7
Compare
PR Tests only triggered on pull_request events, and GitHub short-circuits those before the workflow starts when the head commit carries a [skip ci] marker — so a skip-ci PR silently got no test run at all. workflow_dispatch is not subject to that short-circuit, so add it as a manual trigger to run tests on demand for any ref, including skip-ci PRs. - Add workflow_dispatch with an optional pr_number input. - Guard the coverage-report job so a manual run without a PR number still runs tests but skips the PR comment. - Resolve PR number and concurrency group from either the PR event or the dispatch input. Note: workflow_dispatch only becomes available once this file is on the default branch. Pairs with making Tests (Python 3.10/3.12) required status checks in the branch ruleset so every PR must have them pass (or be run on demand) before merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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
The SDK audit stopped reporting findings even though the Etsy spec changed. Investigation showed the maintenance tooling was structurally blind to the change, not that nothing changed.
What actually changed in the spec: Etsy removed
ShippingandInventoryfrom theincludesquery-parameter enum ongetListing(GET /v3/application/listings/{listing_id}) andgetListingsByListingIds(GET /v3/application/listings/batch). They remain valid ongetListingsByShop, which shares the same SDK enum.Why nothing caught it — two blind spots (Must Fix):
audit_sdk.get_spec_enums()only extracted enums fromcomponents/schemas, never from operation parameters. All 18 parameter-level enums (includes,state,sort_on,holiday_id, …) had never been audited. Now walkspaths/*/*/parameters, covering bothschema.enumandschema.items.enum(array params).diff_spec.diff_parameters()only comparedschema.enum, missingschema.items.enum, so this change reported as "No changed endpoints". Now diffsitems.enumtoo and reports added/removed values. (Side effect: the weekly maintenance workflow now escalates this class of change tomediumseverity instead of silently classifying itlow/cosmetic.)Latent bug exposed by the fix:
scan_enum_valueskeyed SDK enums by class name alone, soIncludesinListingInventory.py(1 value) clobberedIncludesinListing.py(9 values). Now each name maps to a list of value-lists andcompute_enum_findingspicks the best-overlap candidate per spec enum.Data decision (Should Fix → suppressed, non-breaking): The now-visible
Includesdrift is intentional —SHIPPING/INVENTORYare kept for backward compatibility and are still valid ongetListingsByShop. Removing them would break callers. Documented inline inenums/Listing.pyand suppressed inspecs/audit-ignore.json(mirroring the existingState.REMOVEDpattern). Param-sideState.REMOVEDandholiday_idfindings mirror existing accepted decisions and are suppressed likewise. Baseline refreshed to the reviewed spec.Findings tally: 2 Must Fix (tooling), 1 Should Fix (data, suppressed as intentional), 1 Informational (Etsy's spec is now internally inconsistent per-endpoint). All 7 prior suppressions re-verified as still valid; no stale ignores.
No SDK runtime behavior changes; no version bump (commit carries
[skip ci]).Test plan
pytest -q→ 340 passed (was 334; +6 regression tests)items.enum), the duplicate-class-name collision fix, and the updatedcompute_enum_findingscontract.diff_spec.pynow reportsincludes (enum values changed (removed ['Inventory', 'Shipping']))for both affected endpoints (previously "No changed endpoints").audit_sdk.pynow surfaces the realIncludesextras (inventory,shipping) and, after suppression, reports "All enum values are in sync" with no stale ignores.🤖 Generated with Claude Code