Fix: days_to_expiration sent under wrong query param name (dte)#31
Open
MarketDataDev03 wants to merge 2 commits into
Open
Fix: days_to_expiration sent under wrong query param name (dte)#31MarketDataDev03 wants to merge 2 commits into
days_to_expiration sent under wrong query param name (dte)#31MarketDataDev03 wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 52 52
Lines 2281 2281
=========================================
Hits 2281 2281
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Fix:
days_to_expirationsent under wrong query param name (dte)Closes #30
Summary
The
days_to_expirationfilter onclient.options.chain(...)was silentlyignored by the API. The URL builder serializes input models with
by_alias=True(src/marketdata/resources/base.py:45), but thedays_to_expirationfield had no Pydanticalias, so it was emitted to thewire under its Python name (
days_to_expiration=30) instead of the parameterthe API expects (
dte=30). The backend doesn't recognize that key, so thefilter was dropped without error and the full chain was returned.
Changes
1. The fix —
src/marketdata/input_types/options.pyAdded
alias="dte"to thedays_to_expirationfield, matching the existingconvention used by every other renamed field in the model (
strikeLimit,minBid,from/to, etc.). BecauseBaseModelConfigsetspopulate_by_name=True, callers keep usingdays_to_expiration=...while theemitted query parameter is now
dte.2. Targeted regression test —
src/tests/test_options_chain.pytest_options_chain_input_days_to_expiration_alias_on_wireasserts that a realrequest carries
dte=30on the wire and neverdays_to_expiration, mirroringthe existing
from_date/to_datealias test.3. Preventive test for the whole bug class —
src/tests/test_input_types.pyA parametrized test (
test_snake_case_input_fields_have_api_alias)auto-discovers every
BaseInputTypemodel in the SDK and asserts that any fieldwhose Python name contains an underscore is sent under an explicit
alias(or is exempt via
GLOBAL_EXCLUDED_PARAMS, likeoutput_format/filename,which never reach the query string). A guard test ensures discovery isn't
silently empty. This catches the next field that forgets its alias, not just
this one.
Audit
I audited every input model across
options.py,funds.py,markets.py,stocks.py, andbase.py.days_to_expirationwas the only field missingan alias — all other renamed fields (
strike_limit,min_bid/max_bid,min_ask/max_ask,max_bid_ask_spread[_pct],min_open_interest,min_volume,from_date/to_date,use_52_week,adjust_splits,report_type,date_format,add_headers,use_human_readable) alreadycarry the correct alias. The new preventive test now enforces this invariant.
Testing
alias="dte"fix is reverted,confirming it isn't a false green.