harden(spec-drift): stop the recurring #467/#470 spam + close drift-test false-negatives

[TexasCoding]

Summary

The weekly (#470) and nightly (#467) spec-drift issues recur because Kalshi's OpenAPI is a fast-moving, loosely-versioned upstream (~1 change every ~5 days; content changes land without an info.version bump) and our reporting/test mechanics amplified the churn. I confirmed the current drift is real, not a CI artifact (live spec fetched 3× → byte-identical SHA256; /exchange/announcements genuinely removed; upstream now at 3.24.0). This PR fixes the mechanics so real drift is reported once, benign drift stops redding CI, and previously-invisible drift gets caught. It does not reconcile the SDK to 3.24.0 — that's a separate follow-up (the announcements removal is being handled as a soft-deprecate pending a confirming fetch).

Changes

CI reporting (fe9ec5a)

• Nightly: re-comments only when the failing-test-ID set changes (fingerprint), not every failing night; a spec-drift-ack/snoozed label silences noise during an in-flight reconcile; a new report-success job auto-closes the tracker on a green scheduled run.
• Weekly: title now leads with a content signal — [content-changed] (paths 91→90, fp 9834160a) — instead of a bare 3.23.0 → 3.23.0 that read as a false alarm; older weekly snapshots are superseded so exactly one stays open (scoped to spec-sync-bot; the nightly tracker is never touched).

Drift tests (4f68ce5)

• Less brittle: additive drift is split — a spec-required field the SDK lacks still hard-fails; a spec-optional field becomes a non-blocking AdditiveOptionalDriftWarning (subclasses Warning, not UserWarning, so -W error::UserWarning can't promote benign drift). +3 regression tests. test_schema_coverage now batches all misses into one failure.
• Closes false-negatives: test_every_spec_endpoint_is_mapped asserts every spec operation (core+perps+perps_scm) is mapped — catching ADDED endpoints/operations the schema/path-count diff misses. kwarg_rename exclusions are now spec-anchored so a stale rename can't mask an upstream param removal.

Test plan

• uv run pytest tests/test_contracts.py -q — 660 passed on the pinned 3.23.0 baseline
• uv run ruff check . — clean
• uv run mypy kalshi/ — clean (157 files)
• Full suite collects cleanly (4457 tests; changes isolated to tests/test_contracts.py + 2 workflow YAMLs)
• CI workflow bash validated locally (YAML parse, shellcheck, fingerprint/dedup/title/supersede logic on sample data)
• Verified against live 3.24.0: optional additive drift downgrades to a warning; required additions/removals still hard-fail; the coverage test catches the two new /margin/* endpoints and a new GET on an existing /communications/rfqs/.../quotes path (which the weekly path diff reported as "Added endpoints: (none)")

Notes for reviewers

Non-obvious design decisions, called out deliberately:

• AdditiveOptionalDriftWarning must not subclass UserWarning — a regression test locks this, because that property is what keeps the nightly gate from re-brittling.
• Response-side field REMOVAL is intentionally NOT gated. The vendored spec is titled "Kalshi Trade API Manual Endpoints" — a curated subset — so SDK response models are a superset by design (client reshapes like Orderbook.yes/no, plus real-but-undocumented fields). A hard-fail removal gate would red CI on every intentional SDK field, re-introducing the exact churn this PR removes. Request-body drift already gates removals (_check_model_drift), which is where a rename's dropped old name is caught. The one residual gap — a response-only optional rename — surfaces as an AdditiveOptionalDriftWarning and is reconciled on the next sync.
• Deferred to follow-ups: (1) the actual 3.24.0 reconcile + announcements soft-deprecate; (2) response-side type / enum-value drift for REST (reuse the WS _sdk_type_kind/_spec_property_kind helpers) — deferred because it needs careful type-equivalence handling to avoid a churny baseline break; (3) a maintainer-gated workflow_dispatch reconcile-PR workflow.
• Not in the shipped wheel — only tests/ and .github/ change, so no CHANGELOG entry.

Issue links

Refs #467, #470

[claude]

Review: harden(spec-drift) — PR #471

Solid, well-scoped PR. The mechanics fix (fingerprint-gated nightly comments, ack/snooze, auto-close, content-signal titles, weekly supersede) and the drift-test improvements (required-vs-optional additive split, batched schema-coverage failures, endpoint-completeness check, spec-anchored kwarg_rename staleness check) are all implemented cleanly, and the new regression tests (TestAdditiveDriftClassification) correctly lock the load-bearing property that AdditiveOptionalDriftWarning is not a UserWarning. Scope matches the stated diff (only tests/test_contracts.py + 2 workflow YAMLs) — no CHANGELOG entry needed, per CLAUDE.md conventions.

Potential bug: fingerprint blind spot for ERROR-outcome failures

.github/workflows/spec-drift.yml, the new fingerprint step:

```bash
grep -oE '^FAILED [^ ]+' /tmp/drift.log | awk '{print $2}' | sort -u > /tmp/fail_ids.txt
```

This only captures pytest's FAILED <nodeid> summary lines. It misses ERROR <nodeid> lines, which is exactly what pytest emits when an autouse fixture raises — e.g. _load (self.spec = _load_spec()) at tests/test_contracts.py:809/997/1317/1630 etc. If upstream ever publishes a spec that fails to parse (malformed YAML, or any other exception during yaml.safe_load), every test in the affected class errors out instead of failing, so fail_ids.txt ends up empty — the same empty-file SHA256 you'd get on a clean run. Two different "spec totally broke" nights (different root causes) would hash identically and get silently deduped as "failing set unchanged," which defeats the fingerprint's purpose for precisely the catastrophic-drift case this PR is trying to make more visible, not less. Since this workflow is explicitly designed around a "fast-moving, loosely-versioned upstream," a malformed/broken spec fetch isn't a far-fetched scenario.

Suggest also matching ^ERROR (or at minimum falling back to a hash of the whole drift.tail when no FAILED/ERROR lines are found, so a total collection blowup doesn't collapse to the "nothing new" fingerprint).

Test coverage

• TestAdditiveDriftClassification nicely locks the optional/required split and the UserWarning non-subclassing property.
• The two other new pieces of test-suite logic — test_every_spec_endpoint_is_mapped (tests/test_contracts.py:1241) and the kwarg_rename spec-anchor staleness check in test_exclusion_map_is_current (tests/test_contracts.py:1853) — only get exercised against the real, current spec/EXCLUSIONS content, with no synthetic unit test analogous to TestAdditiveDriftClassification. Given these two are specifically meant to catch future upstream changes, a small synthetic-spec test for each (mirroring the Fake-model pattern already used) would guard against a logic regression that wouldn't otherwise surface until the next real drift event — somewhat ironic given that's the exact failure mode both checks exist to prevent.

Minor / nice-to-have

• _UNIMPLEMENTED_ENDPOINTS (tests/test_contracts.py:1233) has no staleness guard analogous to the one just added for kwarg_rename exclusions — if upstream ever removes /margin/large_trader_positions outright, the entry just becomes permanently inert with nothing flagging it as dead. Not urgent (small allowlist, one entry today), but worth keeping in mind as it grows.
• Nice touch scoping the weekly supersede and dedup logic to spec-sync-bot-tagged bodies only, so it can never touch the nightly tracker — and using Warning (not UserWarning) for the new soft-drift class is exactly the right call given the nightly -W error::UserWarning gate; the regression test for it is a good defensive addition.

Security

No concerns — permissions stay least-privilege per job (report-success correctly gets no checkout/SHA-pinning since it does nothing but call gh against trusted repo state), and the new printf/--body-file paths keep upstream-sourced content out of shell expansion, consistent with the existing threat model in the workflow header comments.

Nothing here should block merge; the fingerprint gap is the one item I'd actually want addressed (or explicitly accepted) before relying on this to silence nightly noise for real incidents.