chore: pay down mypy typing-debt + param audit + live demo contract tests#33
Conversation
…ests Task 1 — typing-debt paid down: fixed the real type errors in client, async_client, cli, telemetry, prices, alerts, analytics, data_sources, webhooks and the package __init__ with proper annotations (not type: ignore), then removed all 10 TODO(typing-debt) per-module mypy overrides. The only remaining narrow override is models.py import-untyped (optional dateutil stub, not a real typing bug). `mypy oilpriceapi/ --ignore-missing-imports` is clean with the overrides gone. Task 2 — param-correctness audit vs the real controllers: - analytics: SDK sent commodity/commodity1/commodity2/days; the v1 analytics controller requires code/code1/code2/period. Fixed correlation (the Node SDK bug class), plus statistics/trend/forecast. performance now maps to the controller's `range` param; spread now targets a named spread (the endpoint takes a spread name, not two codes) and a new available_spreads() helper was added. Same fixes applied to the async analytics resource. - webhooks: controller permits `status` (active/inactive/paused), not boolean `enabled`. create/update now map enabled -> status (sync + async). - data_sources: controller requires params nested under `data_source` and uses `status` + `scraper_config`, not flat `enabled` + `config`. create/update now nest and map correctly (sync + async). - alerts already nested correctly under price_alert with the right keys. Added wire-param unit tests asserting the exact params sent. Task 3 — live demo contract tests: new DemoResource (client.demo, also usable standalone with no API key) hitting GET /v1/demo/prices and /v1/demo/commodities. Live tests under tests/integration (marked `live`, excluded from the unit gate) assert the real envelope: 9 free-tier prices incl BRENT_CRUDE_USD ~$80, 442 commodities, meta.free_commodities. Mocked unit tests cover the accessor. Gates: mypy clean, ruff clean (oilpriceapi/), 249 unit tests pass, cov 57.38% (>=50). Version not bumped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 15 minutes and 43 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…ests (#33) Task 1 — typing-debt paid down: fixed the real type errors in client, async_client, cli, telemetry, prices, alerts, analytics, data_sources, webhooks and the package __init__ with proper annotations (not type: ignore), then removed all 10 TODO(typing-debt) per-module mypy overrides. The only remaining narrow override is models.py import-untyped (optional dateutil stub, not a real typing bug). `mypy oilpriceapi/ --ignore-missing-imports` is clean with the overrides gone. Task 2 — param-correctness audit vs the real controllers: - analytics: SDK sent commodity/commodity1/commodity2/days; the v1 analytics controller requires code/code1/code2/period. Fixed correlation (the Node SDK bug class), plus statistics/trend/forecast. performance now maps to the controller's `range` param; spread now targets a named spread (the endpoint takes a spread name, not two codes) and a new available_spreads() helper was added. Same fixes applied to the async analytics resource. - webhooks: controller permits `status` (active/inactive/paused), not boolean `enabled`. create/update now map enabled -> status (sync + async). - data_sources: controller requires params nested under `data_source` and uses `status` + `scraper_config`, not flat `enabled` + `config`. create/update now nest and map correctly (sync + async). - alerts already nested correctly under price_alert with the right keys. Added wire-param unit tests asserting the exact params sent. Task 3 — live demo contract tests: new DemoResource (client.demo, also usable standalone with no API key) hitting GET /v1/demo/prices and /v1/demo/commodities. Live tests under tests/integration (marked `live`, excluded from the unit gate) assert the real envelope: 9 free-tier prices incl BRENT_CRUDE_USD ~$80, 442 commodities, meta.free_commodities. Mocked unit tests cover the accessor. Gates: mypy clean, ruff clean (oilpriceapi/), 249 unit tests pass, cov 57.38% (>=50). Version not bumped. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Hardens the Python SDK across three fronts: paying down mypy typing-debt, auditing request params against the real Rails controllers, and adding live demo-endpoint contract tests. No version bump.
Task 1 — mypy typing-debt paid down
Removed all 10
TODO(typing-debt)per-module mypy overrides by fixing the underlying type errors with proper annotations (not# type: ignore):oilpriceapi(__init__)api_key: Optional[str] = NoneclientOptional[list]/tuplereturn,viz/last_exceptiontypes,Headersargasync_clientOptional[list],self._client = Nonetyped None,response["data"]on union, untypedAsyncGeneratorOptional[httpx.AsyncClient], narrowed withisinstance,AsyncGenerator[List[HistoricalPrice], None]climaindef +Callable.commandmain, exit-at-import fallback, typedcommodity_listtelemetry_events, no_implicit_reexport import,kwargsdictversionmodulepricesget_multipleresultassert isinstance(prices, list)alerts,analytics,data_sources,webhooksDict[str, str]inferred dictsDict[str, Any]annotationsOverrides removed:
oilpriceapi,client,async_client,cli,telemetry,prices,alerts,analytics,data_sources,webhooks.Overrides remaining (and why):
oilpriceapi.*— package-wide annotation-completeness relaxation (no-untyped-def,no-any-return,no-untyped-call). This is the legitimate "SDK not yet fully annotated" debt, not real-bug codes. Out of scope to fully annotate every method here.oilpriceapi.modelsimport-untyped—models.pyoptionally importsdateutil; the stub package isn't a runtime/CI dep, so this is a missing-stub finding, not a typing bug.Task 2 — param-correctness audit (same bug class as the Node SDK)
Audited SDK params/paths against
oilpriceapi-api/app/controllers/. Findings + fixes:commodity/commodity1/commodity2/days, butanalytics_controller.rbreadscode/code1/code2/period. Fixedcorrelation(code1/code2/period),statistics/trend/forecast(code/period),performance(maps to controllerrange), andspread(the endpoint takes a named spread, not two codes — also addedavailable_spreads()). Applied to the async analytics resource too.status(active/inactive/paused), not booleanenabled.create/updatenow mapenabled→status(sync + async).params.require(:data_source).permit(:status, scraper_config: {})— SDK sent flat body withenabled/config. Now nests underdata_sourceand mapsenabled→status,config→scraper_config(sync + async).price_alertwithcommodity_code/condition_operator/condition_value/enabled. No change.config/routes.rb.Added wire-param unit tests asserting the exact params sent (regression guards against the old keys).
Task 3 — live demo contract tests (no key needed)
New
DemoResource(exposed asclient.demo, also usable standalone with no API key) hittingGET /v1/demo/pricesand/v1/demo/commodities.Live tests live in
tests/integration/test_demo_contract.py(markedlive, excluded from the default unit gate via--ignore=tests/integration). They assert the real envelope{status, data:{prices|commodities, meta}}: 9 free-tier prices inclBRENT_CRUDE_USD≈ $80, 442 commodities (meta.total== flattened catalog), andmeta.free_commodities. Mocked unit tests cover the accessor in the default gate. Verified passing against the live API.Gates (all green)
mypy oilpriceapi/ --ignore-missing-imports→ Success (40 files)ruff check oilpriceapi/→ cleanpytest tests/ --ignore=tests/integration --ignore=tests/contract -m 'not slow' --cov=oilpriceapi→ 249 passed, 9 skipped, coverage 57.38% (≥50)🤖 Generated with Claude Code