Skip to content

fix(spreads,indicators): send required params and type the payloads production returns (#112) - #119

Merged
karlwaldman merged 1 commit into
mainfrom
fix/112-spreads-indicators-params
Sep 14, 2026
Merged

karlwaldman merged 1 commit into
mainfrom
fix/112-spreads-indicators-params

Conversation

@karlwaldman

Copy link
Copy Markdown
Member

Closes #112

What was wrong (re-verified 2026-09-13/14)

Checked against production (paid test account), oilpriceapi-api origin/main (config/routes.rb, V1::SpreadsController, app/services/calculated_metrics/), and python-sdk #146.

Defect Evidence
basis(), curveStructure(), priceContext(), annotations() could not send pair / commodity / code Production answers all four with 400 MISSING_PARAMETER; basis/historical and annotations/batch do the same
crack(), margin(), physicalPremium(), fuelSwitching(), storageAnalytics(), cftcPositioning() could not send type, crude, index, commodity, gas, location Only server defaults were reachable
Declared types described fields no route sends e.g. SpreadValue.type, components: string[]; margin sends margin_usd_bbl, not value; PriceContextIndicator.percentile is context.percentile_1y
all() returned undefined Lists are under spreads / commodities / margins / premiums / locations, not data
cftcPositioning() typed as an array Route returns one object; the list is /cftc-positioning/all
Not in the issue: priceContext() and annotations() returned a one-element array even with a code OilPriceAPI#shapeResponseData wraps any data with a top-level price in an array (the /v1/prices/latest convention). Proven by mutation below
congressionalTrades() 404 DATA_NOT_AVAILABLE, has never returned data (api#8478)

Silent-input behavior measured live, which is why validation happens locally:

  • start_date=banana → 200 with the default window
  • start_date after end_date → 200, count: 0
  • unknown or empty pair on basis/historical, unknown type on crack/historical → 200, count: 0
  • 21 codes to annotations/batchtotal_codes: 21, 20 annotated (already api#8448)

Fix

  • New src/resources/calculated-metrics.ts: one place for selector validation (reuses requireFilter from fix(ei): make the six by-* methods send their required filter and return the page (#105) #110), YYYY-MM-DD date and window checks, envelope unwrapping, named-collection unwrapping, and unexpected_response_shape with rawBody.
  • client.request gains an internal unshaped option that skips shapeResponseData; the metrics resources use it and validate the envelope themselves.
  • Types are taken from verbatim production bodies (33 fixtures in tests/fixtures/calculated-metrics/) and cross-checked against the Ruby serializers for conditional keys and nulls.
  • Known API defects are typed as they must be read: CrackSpread.data_stale?: true (absent = "not flagged", api#8477); CftcPositioningPoint.spec_net_pct_oi: number | null (a fabricated 0 today, api#8476).

Observed wire shapes (production, 2026-09-13)

Route data
spreads/crack {spread_type, crude_benchmark, value, unit, components:{crude, product | gasoline+diesel}, timestamp, changes, data_stale?, stale_warning?}
spreads/crack/historical {spread_type, crude_benchmark, period, coverage:{from,to,observations,complete}, data_revised_at, count, data:[{date,value,crude,product?|gasoline?,diesel?}]}
spreads/crack/all {crude_benchmark, spreads:[crack]}
spreads/gasoil-crack {spread_type, name, value, unit, components:{product,crude}:{code,contract_month,updated_at,price,unit}, conversion, timestamp, updated_at, data_stale?}
spreads/basis (pair required) {pair, spread_name, value, unit, components:{CODE:price}, signal, timestamp, percentile_1y|null, changes, negative_streak_days?, data_stale?}
spreads/basis/historical (pair required) {pair, period, count, data:[{date,value,code_a,code_b}]}
spreads/basis/all {spreads:[basis]}
spreads/curve-structure (commodity required) {commodity, display_name, structure, severity, term_slope_pct, spreads:{m1_m3?,m1_m6,m1_m12?}, front_month, back_month_6, curve_points, signal, timestamp}
spreads/curve-structure/all {commodities:[curve]}
spreads/margin {index, name, margin_usd_bbl, crude_input, product_basket:{name:{yield_pct,price,code}}, signal, percentile_1y|null, changes, timestamp}
spreads/margin/historical {index, period, count, data:[{date,margin,crude,revenue}]}
spreads/margin/all {margins:[margin]}
spreads/physical-premium {commodity, name, premium, premium_pct, unit, components:{spot,futures:{code,price,contract?}}, signal, elevated_streak_days, percentile_1y|null, timestamp, data_stale?}
spreads/physical-premium/historical {commodity, period, count, data:[{date,premium,premium_pct,spot,futures}]}
spreads/physical-premium/all {premiums:[premium]}
indicators/fuel-switching {oil_parity:{ratio_pct,threshold_pct,signal,parity_price,current_gas,headroom_pct}, components:{gas,crude}, energy_equivalent, historical_context:{}|{...}, timestamp}
indicators/fuel-switching/historical {gas_benchmark, crude_benchmark, period, count, data:[{date,ratio_pct,above_parity,gas_price,crude_price}]}
indicators/price-context (code required) {code, price, timestamp, context:{anomaly, anomaly_reason?, change_*?, high_52w?, low_52w?, percentile_1y?, percentile_5y?}, related_spreads?}
indicators/storage-analytics {location, name, current:{volume_mmbbl,utilization_pct,operational_capacity_mmbbl,data_date,timestamp}, draw_rate, seasonal, anomalies, range_52w, signal, trading_implication}
indicators/storage-analytics/all {locations:[storage]}
indicators/annotations (code required) {code, price, timestamp, annotation_count, annotations:[{type,severity,message,...}]}
indicators/annotations/batch (codes required) {annotated:[annotations], total_codes, codes_with_annotations}
indicators/cftc-positioning {commodity, name, report_date, positioning:{speculative:{net,long,short,net_pct_of_oi}, commercial:{net}, open_interest}, signal, percentile_1y, week_change, timestamp}
indicators/cftc-positioning/historical {commodity, period, count, data:[{date,spec_net,open_interest,spec_net_pct_oi}]}
indicators/cftc-positioning/all {commodities:[cftc]}

403 PREMIUM_REQUIRED: taken from V1::SpreadsController#check_analytics_accessrender_standard_error, same envelope as the live 400/404 bodies. Unverified live: every key available (test and smoke) holds the entitlement.

TDD evidence

Red: new tests against unmodified origin/main source

$ npx vitest run tests/resources/spreads.test.ts tests/resources/indicators.test.ts tests/calculated-metrics-types.test-d.ts
 Test Files  3 failed (3)
      Tests  97 failed | 16 passed (113)
Type Errors  9 failed

 FAIL  tests/resources/spreads.test.ts > #112 basis() > sends the required pair and returns the component map
AssertionError: expected {} to deeply equal { pair: 'BRENT_WTI' }
- Expected
+ Received
- {
-   "pair": "BRENT_WTI",
- }
+ {}

 FAIL  tests/resources/indicators.test.ts > #112 priceContext() > sends the required code and returns the object, not a one-element array
AssertionError: expected {} to deeply equal { code: 'BRENT_CRUDE_USD' }

The 16 that passed on old code are transport guards (429, timeout, abort) that must keep passing.

Mutation: remove only the unshaped opt-out

$ sed -i '' 's/{ unshaped: true }/{}/g' src/resources/calculated-metrics.ts && npx vitest run tests/resources/indicators.test.ts
      Tests  14 failed | 32 passed (46)
 FAIL  tests/resources/indicators.test.ts > #112 priceContext() > sends the required code and returns the object, not a one-element array
OilPriceAPIError: Unexpected response shape from /v1/indicators/price-context: expected a price context, received an array.

(file restored afterwards)

Green

$ npx vitest run
 Test Files  56 passed (56)
      Tests  904 passed | 1 skipped (905)
Type Errors  no errors

Baseline on 7fd722b was 812 passed | 1 skipped.

  • npx tsc --noEmit: 0
  • npm run lint: 0
  • npx tsc --noEmit -p tsconfig.typecheck.json: 0
  • npm run build: 0
  • npm run storefront:check: validated 40 Node public surfaces
  • npm run check:secrets: 0
  • npm run snippets:check: 4 passed, no type errors

No version bump and no CHANGELOG heading (release-readiness.test.ts); release-note text is below.

Breaking changes (for 2.0.0 notes)

client.spreads

  • Removed get(type), historical(type, options), all(type). They could not send the parameters the routes require. Use the named methods.
  • basis()basis(pair: string), required; returns BasisSpread.
  • curveStructure()curveStructure(commodity: string), required; returns CurveStructure.
  • crack()crack(options?: { type?, crude? }); returns CrackSpread (spread_type, not type; components is an object of priced legs).
  • margin()margin(options?: { index? }); returns RefineryMargin (margin_usd_bbl, no value).
  • physicalPremium()physicalPremium(options?: { commodity? }); returns PhysicalPremium (premium, premium_pct).
  • Added crackHistorical, crackAll, gasoilCrack, basisHistorical, basisAll, curveStructureAll, marginHistorical, marginAll, physicalPremiumHistorical, physicalPremiumAll.
  • SpreadValue is now a @deprecated union of CrackSpread | BasisSpread | CurveStructure | RefineryMargin | PhysicalPremium.
  • HistoricalSpreadValue is now a @deprecated union of the real row types.
  • HistoricalSpreadOptions is a @deprecated alias of MetricsDateRangeOptions; SpreadType is @deprecated (no method takes it).

client.indicators

  • Removed get(type) (untyped, could not send parameters).
  • Removed congressionalTrades() and type CongressionalTradeIndicator. The route has never returned data (api#8478).
  • priceContext()priceContext(code: string, options?: { relatedSpreads? }), required; returns PriceContext (metrics under context: percentile_1y, percentile_5y, high_52w...).
  • annotations()annotations(code: string), required; returns a MarketAnnotations object (was AnnotationIndicator[]).
  • cftcPositioning()cftcPositioning(options?: { commodity? }); returns one CftcPositioning object (was an array). The list is cftcPositioningAll().
  • fuelSwitching()fuelSwitching(options?: { gas?, crude? }); returns FuelSwitching (oil_parity.ratio_pct, oil_parity.signal).
  • storageAnalytics()storageAnalytics(options?: { location? }); returns StorageAnalytics (current.volume_mmbbl, draw_rate.days_to_depletion).
  • Added fuelSwitchingHistorical, storageAnalyticsAll, annotationsBatch, cftcPositioningHistorical, cftcPositioningAll, and the constant ANNOTATIONS_BATCH_MAX_CODES (20).
  • @deprecated aliases: FuelSwitchingIndicator = FuelSwitching, PriceContextIndicator = PriceContext, StorageAnalyticsIndicator = StorageAnalytics, AnnotationIndicator = MarketAnnotation (one annotation), CFTCPositioningIndicator = CftcPositioning.
  • IndicatorType is @deprecated and no longer includes "congressional-trades".

Behavior (both resources)

  • ValidationError before any request for a missing or blank required selector, a present-but-blank optional selector, a date that is not a real YYYY-MM-DD, startDate after endDate, or an annotationsBatch list that is empty, has a blank or comma-containing code, or has more than 20 codes.
  • A malformed 200 raises OilPriceAPIError with code: "unexpected_response_shape" and rawBody, instead of returning undefined fields.
  • 403 PREMIUM_REQUIRED surfaces as isEntitlementError (status 403, code PREMIUM_REQUIRED).

🤖 Generated with Claude Code

https://claude.ai/code/session_015ao5paex73xXvuM424Libo

…roduction returns (#112)

client.spreads and client.indicators could not send the parameters their
routes require (basis, curve-structure, price-context and annotations answer
HTTP 400 without them) and were typed against fields no route sends. The
/all routes nest their list under a named key, cftc-positioning returns one
object, and the core client's latest-price heuristic wrapped the
price-context and annotations payloads in a one-element array.

- Required selectors are required arguments; blank values, unparseable dates,
  inverted windows and >20 batch codes raise ValidationError before any
  request (the routes otherwise serve a default window or an empty 200).
- Every method returns a type taken from verbatim production bodies and
  raises unexpected_response_shape on a malformed 200.
- Adds the history, /all, gasoil-crack and annotations/batch routes.
- Removes get/historical/all, indicators.get and the never-populated trades
  route; old type names stay as @deprecated aliases.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5488e73f-84ad-4e1d-b90a-be451f3ce52f


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@karlwaldman
karlwaldman merged commit 26e5694 into main Sep 14, 2026
8 checks passed
@karlwaldman
karlwaldman deleted the fix/112-spreads-indicators-params branch September 14, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P2] spreads and indicators resources are typed against a shape the API does not send, and omit required parameters

1 participant