Skip to content

[P1][bug] normalize_futures_slug silently rewrites 18 live catalog codes onto a DIFFERENT instrument #112

Closed
Closed
[P1][bug] normalize_futures_slug silently rewrites 18 live catalog codes onto a DIFFERENT instrument#112
Labels
bugSomething isn't workingpriority: criticalMust be fixed immediatelytechnical-debtTechnical debt that should be addressed

Description

@karlwaldman
karlwaldman
opened on Sep 13, 2026

Summary

normalize_futures_slug (oilpriceapi/resources/_futures_slug.py) resolves an unrecognised code by splitting on the first separator and stripping trailing digits, then looking the remaining leading token up in CONTRACT_CODE_TO_SLUG. The geography and currency tokens — the two fields that distinguish these instruments from one another — are discarded before the lookup.

The result is that 18 real catalog codes are silently rewritten onto a different instrument and the call succeeds, returning the wrong commodity as if it were correct.

Confirmed-live 2026-09-13.

Evidence

Measured 2026-09-13 by executing normalize_futures_slug against all 604 codes returned by the live catalog (GET https://api.oilpriceapi.com/v1/commodities, authenticated, 2026-09-13):

total=604  refused with ValueError=586  silently rewritten=18

586/604 raise a loud ValueError — that part is fine. The 18 that do not:

Requested code Returned slug What changed
LNG_NW_EUROPE_EUR lng-jkm NW Europe LNG in EUR → Japan/Korea Marker in USD
LNG_SOUTH_EUROPE_EUR lng-jkm South Europe LNG in EUR → JKM in USD
LNG_EU_AVERAGE_EUR lng-jkm EU average LNG in EUR → JKM in USD
WTI_MIDLAND_USD ice-wti Permian basis grade → Cushing WTI
WTI_SPOT_CUSHING_USD ice-wti spot → futures curve
WTI_SPOT_USD ice-wti spot → futures curve
WTI_USD ice-wti spot → futures curve
BRENT_SPOT_EUROPE_USD ice-brent spot → futures curve
BRENT_SPOT_USD ice-brent spot → futures curve
BRENT_CRUDE_USD ice-brent spot → futures curve
GASOIL_USD ice-gasoil spot → futures curve
JKM_LNG_USD lng-jkm spot assessment → futures curve
BRENT_FUTURES ice-brent (benign alias)
WTI_FUTURES ice-wti (benign alias)
TTF_FUTURES ttf-gas (benign alias)
JKM_FUTURES lng-jkm (benign alias)
BRENT_FUTURES_CONTINUOUS ice-brent drops the continuous convention
WTI_FUTURES_CONTINUOUS ice-wti drops the continuous convention

The three LNG rows are the worst: a different delivery region and a different currency. WTI_MIDLAND_USD → ice-wti is a different physical delivery point. Neither is a typo correction — both are real, currently-listed catalog codes being answered with a different contract.

Repro (no network needed for the resolver itself):

import importlib.util
spec = importlib.util.spec_from_file_location("fs", "oilpriceapi/resources/_futures_slug.py")
m = importlib.util.module_from_spec(spec); spec.loader.exec_module(m)
m.normalize_futures_slug("LNG_NW_EUROPE_EUR")  # -> 'lng-jkm'
m.normalize_futures_slug("WTI_MIDLAND_USD")    # -> 'ice-wti'

Reachability

_futures_slug.py:78-92 is the fallback path. It is reached from every futures method — oilpriceapi/resources/futures.py lines 51, 87, 122, 152, 205, 250 — so futures.get, curve, historical, ohlc, intraday and spread_history all inherit it.

Same bug class as mcp-server#90

OilpriceAPI/mcp-server#90 ("stop substituting a different commodity for a real code", merged, released as v3.3.0 today) is the same defect in a different resolver: a local mapping rewriting a real code onto a different commodity. The fix there should be applied here.

Recommendation

Remove the prefix-fallback entirely. normalize_futures_slug should accept the canonical slugs, the continuous slugs, and an exact entry in CONTRACT_CODE_TO_SLUG, and raise ValueError for everything else — including the 18 above. A loud failure is correct; the caller asked for an instrument we do not serve on the futures route, and guessing at a neighbouring contract is not a better answer.

Where an alias is genuinely wanted (BRENT_FUTURES → ice-brent), add it as an explicit full-string key, not as a prefix guess.

Related, same repo — decide whether to fold in or link

oilpriceapi/resources/prices.py:50:

"commodity": price_data.get("code", commodity),

When the API response omits code, the SDK stamps the requested code onto the response object. That manufactures the appearance of a match and would defeat any consumer-side "did I get back what I asked for?" check — including a check someone might add as a mitigation for the slug bug above. prices.py:159 does the right thing (price_data.get("code", "")). Recommend prices.py:50 be made to match, or that the key be left absent rather than fabricated.

Activity

  1. karlwaldman
    added
    bugSomething isn't working
    technical-debtTechnical debt that should be addressed
    priority: criticalMust be fixed immediately
    on Sep 13, 2026

  2. karlwaldman commented on Sep 13, 2026

    @karlwaldman
    karlwaldman
    on Sep 13, 2026
    MemberAuthor

    Verified fixed by 6e1a66c — not closing, leaving that to Karl

    Re-verified by execution on 2026-09-13 against a fresh clone of main (fcc4edf), CPython 3.14.7. The resolver was loaded twice in one process — once from 31b8d80 (the commit before the fix) and once from main — and all 18 codes from the issue table were run through both.

    code parent 31b8d80 6e1a66c (main)
    LNG_NW_EUROPE_EUR lng-jkm ValueError
    LNG_SOUTH_EUROPE_EUR lng-jkm ValueError
    LNG_EU_AVERAGE_EUR lng-jkm ValueError
    WTI_MIDLAND_USD wti ValueError
    WTI_SPOT_CUSHING_USD wti ValueError
    WTI_SPOT_USD wti ValueError
    WTI_USD wti ValueError
    BRENT_SPOT_EUROPE_USD brent ValueError
    BRENT_SPOT_USD brent ValueError
    BRENT_CRUDE_USD brent ValueError
    GASOIL_USD gasoil ValueError
    JKM_LNG_USD lng-jkm ValueError
    BRENT_FUTURES brent ValueError
    WTI_FUTURES wti ValueError
    TTF_FUTURES ttf-gas ValueError
    JKM_FUTURES lng-jkm ValueError
    BRENT_FUTURES_CONTINUOUS brent ValueError
    WTI_FUTURES_CONTINUOUS wti ValueError 
    silently rewritten before: 18     silently rewritten after: 0

    Canonical inputs are unaffected — ice-brent, ice-wti, CL, BZ, CL1!, continuous/brent all still resolve. The fix took the recommendation in this issue: the prefix fallback now only fires when the discarded tail is a month/order marker, so an unrecognised code raises rather than guessing at a neighbouring contract.

    Two things from this issue are still open.

    1. The raise is a bare builtin ValueError, outside OilPriceAPIError. Turning 18 codes from a wrong answer into a refusal is only a win if the refusal is catchable through the documented base class — except OilPriceAPIError: fall_back() currently does not catch it, so this went from "silently wrong" to "uncaught crash". Filed as [P2][bug] normalize_futures_slug raises bare ValueError, outside OilPriceAPIError - #111 made that path far more common #122 and fixed in fix(url,retry,futures): un-break today's three regressions (#119, #118, #122) #125 (FuturesContractError(ValidationError, ValueError) — catchable, and still a ValueError for existing callers).

    2. The prices.py residual noted at the bottom of this issue is NOT fixed. oilpriceapi/resources/prices.py:41 on main today:

      commodity=cast(str, price_data.get("code", fallback_code)),

      When the API response omits code, the SDK still stamps the requested code onto the response object, manufacturing the appearance of a match and defeating any consumer-side "did I get back what I asked for?" check. prices.py:207 does the right thing (price_data.get("code", "")). Unfixed as of fcc4edf.

    3. Low, same file: _futures_slug.py still strips trailing digits unconditionally after the new month-suffix guard declines to split — WTI2026 -> wti, BRENT1 -> brent, LNG1 -> lng-jkm, CL0 -> wti. Verified today. Same "guess rather than refuse" shape, no live catalog code matches it, and removing it risks the TradingView CL1! handling, so it wants its own change and its own catalog re-run.

    Suggested disposition: close this one on the strength of the table above, and let #122 / #125 carry item 1. Items 2 and 3 need somewhere to live — happy to file them if you want them tracked separately.

  3. karlwaldman commented on Sep 13, 2026

    @karlwaldman
    karlwaldman
    on Sep 13, 2026
    MemberAuthor

    Follow-up to my verification comment above: the two residuals noted in this issue are now filed separately, both re-confirmed by execution against main at 9823588 (post-#124/#125/#126):

    The main defect this issue reports — 18 live catalog codes rewritten onto a different instrument — remains verified fixed by 6e1a66c per the table above, and the uncatchable-refusal follow-on (#122) shipped in #125. Still leaving this one open for Karl to close.

  4. karlwaldman
    added a commit that references this issue on Sep 13, 2026

    fix(prices): stop stamping the requested code onto a response that om…

    380b238

  5. karlwaldman commented on Sep 13, 2026

    @karlwaldman
    karlwaldman
    on Sep 13, 2026
    MemberAuthor

    Closing — verified fixed and shipped.

    Fixed by 6e1a66c (#111), verified by loading the resolver from 31b8d80 and from main in one process: 18 codes rewritten before, 0 after, canonical inputs unaffected.

    Shipped to users in v1.14.0 (PyPI, 2026-09-13) — not just merged.

    The two residuals noted in this thread are tracked separately and are not closed by this:

  6. karlwaldman
    closed this as completedon Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: criticalMust be fixed immediatelytechnical-debtTechnical debt that should be addressed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions