Skip to content

fix(stocks): build INTERNAL quotes that carry the 52-week range - #157

Merged
MarketDataDev02 merged 2 commits into
mainfrom
fix/quote-52-week-fields
Oct 2, 2026
Merged

MarketDataDev02 merged 2 commits into
mainfrom
fix/quote-52-week-fields

Conversation

@MarketDataDev02

@MarketDataDev02 MarketDataDev02 commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #102

Summary

stocks.quotes(use_52_week=True) fails on OutputFormat.INTERNAL: the API adds 52weekHigh and 52weekLow (52 Week High and 52 Week Low under human=true), and neither quote model has a field for them. Each model now takes them in two optional money fields, named as in sdk-go:

model API keys fields
StockQuote 52weekHigh, 52weekLow fiftyTwoWeekHigh, fiftyTwoWeekLow
StockQuotesHumanReadable 52 Week High, 52 Week Low Fifty_Two_Week_High, Fifty_Two_Week_Low

Both are Decimal | None, and None when the call did not ask for them.

Version impact

minor (1.3.0 -> 1.4.0). The PR adds public names and removes or narrows none: the two fields on each quote model, StockQuote.api_names, fifty_two_week_fields on both models, and the optional omit_fields argument of no_data_result. 1.3.0 is the last release and the version in pyproject.toml on main. The next release is 2.0.0 either way, since main already carries breaking changes. The CHANGELOG entry stays under Fixed. What a caller sees change:

  • INTERNAL with use_52_week=True returns the quotes instead of raising.
  • The empty result of a no_data answer has the two columns in its DataFrame and CSV header only when the call asked for them. That is the shape a populated answer has in both cases. Before, a use_52_week=True call got an empty frame without them.
  • In a DataFrame the two columns are typed as prices, so a whole-number value is float64 where it was int64, as in every other price column.

Nothing changes for a call without use_52_week.

The change

  • output_types/stocks_quotes.py:
    • Both models get the two fields, last, in the order the API sends them.
    • Both map them through api_names, since no field can be named 52weekHigh.
    • StockQuote.from_dict renames the keys with _to_fields, as its human twin already did.
    • Each model lists the two fields in fifty_two_week_fields.
  • resources/base.py: no_data_result takes omit_fields, the fields the answer would not carry, and leaves their columns out of the empty DataFrame and CSV header. Its docstring now lists its arguments. Its two comments on the JSON branch lose their issue numbers and history, and each keeps the invariant it guards in one line.
  • resources/stocks/quotes.py: omits fifty_two_week_fields unless use_52_week is set. The column names stay in api_names only.
  • docs/stocks.md lists the four fields and says what use_52_week adds.

Out of scope: any other key the models do not declare still raises ParseError. That policy is #111.

Proof

What main does. The issue says the error is a bare TypeError. Since model_errors it is a ParseError, and it still fails. On main bc0619c, with a mocked answer that carries the two keys:

ParseError: Response body is not a valid answer of this resource (StockQuote.__init__() got an unexpected keyword argument '52weekHigh')

DataFrame output on main already worked. It had the two columns last, and 52weekLow was int64 for a whole-number value.

What the API sends, checked live on 2026-09-28 with the market open: the CSV headers at 14:51Z and the JSON keys at 14:52Z. With 52week=true the two keys come after updated (Date), in JSON and in CSV, plain and human. Without it, neither is sent. The CSV headers of AAPL:

?format=csv                           symbol,ask,askSize,bid,bidSize,mid,last,change,changepct,volume,updated
?52week=true&format=csv               symbol,ask,askSize,bid,bidSize,mid,last,change,changepct,volume,updated,52weekHigh,52weekLow
?human=true&format=csv                Symbol,Ask,Ask Size,Bid,Bid Size,Mid,Last,Change $,Change %,Volume,Date
?52week=true&human=true&format=csv    Symbol,Ask,Ask Size,Bid,Bid Size,Mid,Last,Change $,Change %,Volume,Date,52 Week High,52 Week Low

Suite. main bc0619c: 1954 passed. This branch: 1970 passed, the same 1954 plus the 16 new tests. Two existing tables gained the new names because they list every money field and every human column by hand: MONEY_FIELDS in test_money_decimal.py and API_HUMAN_COLUMNS in test_output_types.py. No other existing test changed.

The new tests fail on main. With main's three source files checked out on this branch, 10 of the 16 fail:

  • the four INTERNAL tests;
  • the four empty-frame tests with 52week;
  • the two empty-CSV tests with 52week.

The 6 that pass are the cases without 52week, which pin what does not change.

Mutation. quotes.py never omits the fields: 10 fail. That is the four cases of the existing test_every_resource_no_data_dataframe_has_the_shape_of_a_populated_one[stocks_quotes-*], plus the six new cases without 52week. Restored: all pass.

Live. src/tests/integration/test_stocks_live.py -k quotes on 2026-09-28 at 14:51Z: 3 passed, the two new ones included. The new test again on da96b09 at 21:41Z: 2 passed. Before the 09:30 ET open the API answers 52weekLow: 0 for AAPL while 52weekHigh keeps the year's value (MarketData-App/api#474): the test on f6857fc failed on that answer at 12:04Z on 2026-09-28, in attempt 2 of CI run 36158589796. Since da96b09 the test records that answer as an expected failure and checks every other answer in full, so its result does not depend on the time of day.

Tests

  • src/tests/test_stocks_quotes.py, plain and human:
    • test_internal_quotes_carry_the_52_week_range_when_asked_for sends 52week=true and gets Decimal values, a whole-number price included (169 is Decimal("169")).
    • test_internal_quotes_hold_no_52_week_range_when_not_asked_for gets None in both fields.
  • src/tests/test_status_mapping.py:
    • test_the_empty_quotes_frame_has_the_52_week_columns_only_when_asked_for, on pandas and polars, plain and human, with and without 52week: the empty frame has the index, columns, order and dtypes of a populated one, and has 52weekLow only when it was asked for.
    • test_the_empty_quotes_csv_has_the_52_week_columns_only_when_asked_for: the four headers, written by hand from the live answers above.
  • src/tests/integration/test_stocks_live.py: test_quotes_carry_the_52_week_range_when_asked_for, plain and human. INTERNAL output with use_52_week=True holds each bound as a positive Decimal or None, since the API answers null when it cannot derive the range, with low <= high when both are present. A 52weekLow of 0 is an xfail that names MarketData-App/api#474.

Review round 1

Measured on 2026-09-28, on f6857fc. No code changes in this round, so the head and its CI stay as they are.

795e86fe: the version impact is minor. Version impact now reads minor (1.3.0 -> 1.4.0). 1.3.0 is the last release (v1.3.0) and the version in pyproject.toml on main. The CHANGELOG entry stays under Fixed, as the review allows.

Proof timestamps. The live answers quoted in Proof were captured again on 2026-09-28 with the market open: the CSV headers at 14:51Z and the JSON keys at 14:52Z. The four CSV headers are the ones Proof quotes, and the JSON keys come in the order it describes. The live -k quotes run at 14:51Z: 3 passed.

The live test depends on the time of day until MarketData-App/api#474 is fixed. Before the open the API answers 52weekLow: 0 for AAPL, so 0 < low <= high fails. The times are under Live in Proof. The test is unchanged in this round.

Review round 2

Measured on 2026-09-28, on da96b09: one commit on top of f6857fc that changes only the live test.

7a4eaf48: the live test no longer depends on the time of day. In order, the test checks:

  • each bound is None or a Decimal. This comes first, so a mapping that returned a float or an int still fails at any hour;
  • a 52weekLow of 0, the API's answer before the open (MarketData-App/api#474), is an xfail;
  • each bound that is present is positive, and low <= high when both are.

None is accepted because the API answers null when it cannot derive the range. Run against a stub client in both parametrizations, the range (345.34, 242.7592), (None, None) and a single None pass; a 0 low and (0, 0) xfail; low > high, a negative bound, a float 0.0 low and float bounds fail. The body on f6857fc fails the 0 low case. Live at 21:41Z: 2 passed.

bf4373dc: no sibling SDK builds its own empty result for stock quotes, so none has this defect. Read on each default branch on 2026-09-28. On no_data each one returns the API's own body (CSV and HTML) or an empty typed result, and none builds CSV columns from its model:

  • Go 901e855: stocks.CSVService.Quote and Quotes go through FetchFormatted, which wraps the raw body; the typed Quotes returns an empty slice with NoData.
  • JS 7995952: CSV returns the API's Blob as it came (no_data is an empty Blob); JSON returns [] with the no_data flag.
  • PHP ec5181b: CSV and HTML return the body under ->csv and ->html; the JSON Quote keeps status = 'no_data' with null fields.
  • Java d6f1077: StocksCsvResource.quote and quotes return the API's text as a CsvResponse.
  • C# cb83a0f: GetQuoteCsvAsync returns the raw payload through CreateCsvResponse.

Each typed quote model already has the two 52-week fields. No sibling issue is needed.

Checks on da96b09. uv run pytest -n 4: 1970 passed; the live suite is opt-in, so the count does not change. ./lint.sh clean. git merge-tree is clean against main, #154, #158 and #160. Against #159 it conflicts in StockQuote.from_dict, which #159 now builds with _split_fields and _with_extra, and in the collections.abc import line of resources/base.py; both conflicts are already there on f6857fc.

StockQuote and StockQuotesHumanReadable take the API's 52weekHigh /
52weekLow (52 Week High / 52 Week Low) as fiftyTwoWeekHigh /
fiftyTwoWeekLow (Fifty_Two_Week_High / Fifty_Two_Week_Low), Decimal
money, None when the call did not ask for them. The empty result of a
no_data answer carries the two columns only under use_52_week=True, the
shape a populated answer has.
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (bc0619c) to head (da96b09).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #157   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           62        62           
  Lines         3196      3204    +8     
=========================================
+ Hits          3196      3204    +8     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marketdata-code-review

marketdata-code-review Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

✅ Ready — Code review

This fixes INTERNAL stock quotes with use_52_week=True by adding optional Decimal fields and preserving the matching empty DataFrame and CSV shapes. Version impact: minor (1.3.0 -> 1.4.0). The sibling SDKs do not use the affected model-derived empty-result path, so the same defect does not apply there.

Rating patch 6/6 (exemplary) · proof 6/6 (comprehensive) · overall 6/6

Since last review

  • Resolved: 7a4eaf48, bf4373dc
  • 0 still open, 0 new

Reviewed da96b09 · revision 2 · codex (gpt-5.6-sol) · claude unavailable · rules: default, sdk

… low

The live check takes each bound as a positive Decimal or None, and
checks low <= high only when both are present: the API answers null
when it cannot derive the range. A 52weekLow of 0, which the API
answers before the open (MarketData-App/api#474), is recorded as an
expected failure instead of failing the job.
@MarketDataDev02
MarketDataDev02 merged commit 87bf817 into main Oct 2, 2026
20 of 24 checks passed
@MarketDataDev02
MarketDataDev02 deleted the fix/quote-52-week-fields branch October 2, 2026 14:45
MarketDataDev02 added a commit that referenced this pull request Oct 2, 2026
#157 conflicted in two places: the imports of resources/base.py keep
both csv and Collection, and StockQuote.from_dict keeps this branch's
_split_fields and _with_extra, with its docstring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stocks.quotes(use_52_week=True) fails on INTERNAL output with a bare TypeError: the models have no 52-week fields

2 participants