fix: SDK audit — correctness fixes and improved drift detection - #15
Merged
Conversation
…ation - Fix holiday_id enum not extracted in URL path (HolidayPreferences) - Fix production_partner_ids type from int to List[int] (CreateDraftListingRequest) - Fix image_ids type from List[str] to List[int] (UpdateListingRequest) - Add variation_images to mandatory list (UpdateVariationImagesRequest) - Update spec baseline Closes #14 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Change MOCK_HOLIDAY_ID from string to integer to match API contract - Add test for enum path param extraction in update_holiday_preferences - Add negative test for UpdateVariationImagesRequest mandatory validation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Auto-classify drift as high/medium/low based on audit findings - Title format: "audit: Spec Drift [date] — [severity]" - Add severity labels (high, medium, low) with auto-creation - Assign issues to @amitray007 - Show actionable items extracted from audit, not just raw reports - Include structured SDK impact section (missing endpoints, body drift, etc.) - Severity-specific action guidance in issue body Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Test Coverage ReportOverall: 93% (1533/1643 statements covered) Coverage by file
Updated by PR Tests |
There was a problem hiding this comment.
Pull request overview
This PR applies a set of SDK correctness fixes identified during a spec/SDK audit, adds targeted regression tests, and refactors the GitHub maintenance-check workflow to generate drift issues with severity classification.
Changes:
- Fix path-param interpolation for holiday enum IDs and add a regression test.
- Correct several request-model field types / validation rules and add a mandatory-field regression test.
- Update the maintenance-check workflow to classify drift severity and create structured issues/labels/assignees.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
etsy_python/v3/resources/HolidayPreferences.py |
Extract enum .value for holiday_id before URL interpolation. |
etsy_python/v3/models/Listing.py |
Fix request model typings and enforce variation_images as mandatory. |
tests/test_remaining_resources.py |
Add regression test ensuring enum holiday IDs interpolate correctly in endpoint path. |
tests/test_listing_models.py |
Add regression test for mandatory validation on UpdateVariationImagesRequest. |
tests/fixtures/responses.py |
Make holiday fixture data more realistic (numeric ID, updated name). |
tests/conftest.py |
Update MOCK_HOLIDAY_ID to numeric ID to match API expectations. |
specs/baseline.json |
Sync baseline spec text with latest spec (description formatting). |
.github/workflows/maintenance-check.yml |
Add severity classification, label management, and structured issue creation/update logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,4 +1,5 @@ | |||
| from dataclasses import dataclass | |||
| from enum import Enum | |||
| from typing import Optional, Union | |||
There was a problem hiding this comment.
Optional is no longer used in this module after removing Optional[...] from the holiday_id type hint. Please drop the unused import to avoid lint/type-check noise.
Suggested change
| from typing import Optional, Union | |
| from typing import Union |
| if [ -f "$DIFF_FILE" ]; then | ||
| # Count sections that have content (not just "No ... endpoints/changes") | ||
| if ! grep -q "No new endpoints" "$DIFF_FILE"; then | ||
| NEW_ENDPOINTS=$(grep -c '^\- \*\*' "$DIFF_FILE" 2>/dev/null || echo 0) |
There was a problem hiding this comment.
The NEW_ENDPOINTS counter is computed by grepping for - ** across the entire diff report, which will also count bullets in other sections (Removed Endpoints, Schema Changes, Deprecations). This can inflate the “new endpoints” count and summary; consider scoping the count to just the “## New Endpoints” section (e.g., by extracting that section first).
Suggested change
| NEW_ENDPOINTS=$(grep -c '^\- \*\*' "$DIFF_FILE" 2>/dev/null || echo 0) | |
| NEW_ENDPOINTS=$(sed -n '/^## New Endpoints$/,/^## /p' "$DIFF_FILE" | grep -c '^\- \*\*' 2>/dev/null || echo 0) |
| if [ "$REMOVED_ENDPOINTS" -gt 0 ] || [ "$MISSING_ENDPOINTS" -gt 0 ] || [ "$BODY_DRIFT" -gt 0 ]; then | ||
| SEVERITY="high" | ||
| # Medium: new endpoints, changed endpoints, param drift, schema changes, real code issues | ||
| elif [ "$NEW_ENDPOINTS" -gt 0 ] || [ "$CHANGED_ENDPOINTS" -gt 0 ] || [ "$PARAM_DRIFT" -gt 0 ] || [ "$SCHEMA_CHANGES" -gt 0 ]; then |
There was a problem hiding this comment.
The severity logic comment says medium includes “real code issues”, but CODE_ISSUES is never used when computing SEVERITY. If code issues should bump severity, include a CODE_ISSUES > 0 check (or update the comment to reflect the intended behavior).
Suggested change
| elif [ "$NEW_ENDPOINTS" -gt 0 ] || [ "$CHANGED_ENDPOINTS" -gt 0 ] || [ "$PARAM_DRIFT" -gt 0 ] || [ "$SCHEMA_CHANGES" -gt 0 ]; then | |
| elif [ "$NEW_ENDPOINTS" -gt 0 ] || [ "$CHANGED_ENDPOINTS" -gt 0 ] || [ "$PARAM_DRIFT" -gt 0 ] || [ "$SCHEMA_CHANGES" -gt 0 ] || [ "$CODE_ISSUES" -gt 0 ]; then |
Comment on lines
+188
to
+200
| # Body drift (if any) | ||
| if ! grep -q "No request body drift" "$AUDIT_FILE"; then | ||
| echo "#### Request Body Drift" | ||
| sed -n '/## Request Body Drift/,/^## /p' "$AUDIT_FILE" | head -30 | ||
| echo "" | ||
| fi | ||
|
|
||
| # Param drift (if any) | ||
| if ! grep -q "No query/path parameter drift" "$AUDIT_FILE"; then | ||
| echo "#### Parameter Drift" | ||
| sed -n '/## Query\/Path Parameter Drift/,/^## /p' "$AUDIT_FILE" | head -30 | ||
| echo "" | ||
| fi |
There was a problem hiding this comment.
These sed -n '/## Request Body Drift/,/^## /p' (and similar) ranges will stop immediately because the end pattern ^## matches the same header line as the start pattern. As a result the issue body will include only the section header, not the section content. Use an end pattern that matches the next specific section header (or a range that excludes the first ^## match).
Comment on lines
+220
to
+229
| # Extra SDK methods | ||
| EXTRA_SECTION=$(sed -n '/## Extra SDK Methods/,/^## /p' "$AUDIT_FILE") | ||
| if ! echo "$EXTRA_SECTION" | grep -q "no matching OAS operation" || echo "$EXTRA_SECTION" | grep -q "\*\*"; then | ||
| echo "<details><summary>Extra SDK Methods (no OAS match)</summary>" | ||
| echo "" | ||
| echo "$EXTRA_SECTION" | head -20 | ||
| echo "" | ||
| echo "</details>" | ||
| echo "" | ||
| fi |
There was a problem hiding this comment.
The EXTRA_SECTION extraction uses sed -n '/## Extra SDK Methods/,/^## /p', which will also stop on the start header and likely capture only a single line. This prevents the <details> block from containing the actual list; extract until the next known section header (e.g., “## Missing Exports”) instead.
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
update_holiday_preferences()now extracts.valuefrom enum types before URL interpolationproduction_partner_idstype fromOptional[int]toOptional[List[int]]inCreateDraftListingRequestimage_idstype fromOptional[List[str]]toOptional[List[int]]inUpdateListingRequestvariation_imagesto mandatory list inUpdateVariationImagesRequestMOCK_HOLIDAY_IDfrom string to integer for realistic testingseverity: high/medium/low), assignee, and structured issue bodiesCloses #14
Test plan
severity: high,severity: medium,severity: lowlabels created on first workflow run🤖 Generated with Claude Code