fix(xbrl,find): four structural defects — dead stitching path, dropped query option, mutated fact cache, X-ticker routing - #1183
Merged
Conversation
…ement dict was expected (#1173) `XBRLS.get_statement(use_optimal_periods=False)` appended the (statements, role, statement_type) tuple returned by `XBRL.find_statement()` to the list handed to `StatementStitcher`, which then called `.get()` on it. The whole branch was dead: it raised `AttributeError: 'tuple' object has no attribute 'get'` before stitching a single period. It now calls `get_statement_by_type()` — the accessor the optimal-periods branch already uses, and the one that returns a dict carrying 'periods' and 'data' — and skips filings that lack the statement type, matching the StatementNotFoundError guard added for #683. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN2NaNcXEuv5YvcKFntcaZ
…fact cache (#1172, #1175) #1172: `XBRLS.query()` stored the option as `standardize` while `StitchedFactQuery.__init__` read `standard`, so the value never reached `get_statement()` and standardization stayed on no matter what the caller asked for. Both spellings are now accepted. #1175: `FactQuery.transform()` and `.scale()` wrote transformed values back into the row dictionaries returned by `get_facts()`. Those rows come from the facts view's shared cache, so scaling scaled the cache: a second identical query returned values divided by a million, and an unrelated untransformed query saw the scaled numbers. Rows are copied before transformation now. `StitchedFactQuery` carried its own copy of the same loop and is fixed with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN2NaNcXEuv5YvcKFntcaZ
…1178) The ordinary-ticker branch of `find()` matched `^[A-WYZ]{1,5}([.-][A-Z])?$`. The character class excludes `X` in every position, but the thing it was guarding is the mutual-fund convention of a *trailing* X — so XOM, AXP and FIX fell through to `find_company()` and came back as `CompanySearchResults` instead of the `Company` that `Company(ticker)` resolves, changing the return type and the operations available to the caller. The `^[A-Z]{4}X$` fund pattern is now tested first, which is what the exclusion was standing in for, and the ticker pattern admits the full alphabet. Fund routing is unchanged: no X-containing input reached the ticker branch before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN2NaNcXEuv5YvcKFntcaZ
The regression-test gate requires an explicit issue, PR or bead link in the module docstring so a test can always be traced to the bug it guards. The three new modules referenced their issues by number only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN2NaNcXEuv5YvcKFntcaZ
This was referenced Aug 31, 2026
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
1 participant
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.
Fixes #1173, #1172, #1175 and #1178 — four reproducible defects, each verified against the source before fixing and each gated by a regression test confirmed to fail without its fix.
The fixes
#1173 —
XBRLS.get_statement(use_optimal_periods=False)crashed. The non-optimal branch appended the(statements, role, statement_type)tuple fromXBRL.find_statement()to the list handed toStatementStitcher, which then called.get()on it. Those tuple entries are index records carrying neitherperiodsnordata, so the branch was dead code that raisedAttributeError: 'tuple' object has no attribute 'get'before stitching a single period. It now callsget_statement_by_type()— the accessor the optimal branch already uses, which itself wrapsfind_statement()— with theStatementNotFoundErrorskip added for #683.#1172 —
XBRLS.query(standardize=False)still standardized.XBRLS.query()stored the option asstandardize;StitchedFactQuery.__init__readstandard. The value never reachedget_statement(), so standardization stayed on regardless. Both spellings are accepted now, keeping the base-classstandard=callers working.#1175 —
.transform()/.scale()mutated the shared fact cache. BothFactQueryandStitchedFactQuerywrote transformed values back into the row dictionaries returned byget_facts(), which come from the facts view's cache..scale(1000)therefore scaled the cache itself: a second identical query returned values divided by a million, and an unrelated untransformed query on the same view saw the scaled numbers. Rows are copied before transformation.#1178 —
find()did not recognize tickers containingX. The ordinary-ticker pattern^[A-WYZ]{1,5}([.-][A-Z])?$excludedXin every position, but the thing it was standing in for is the mutual-fund convention of a trailing X.find("XOM"),find("AXP")andfind("FIX")fell through tofind_company()and returnedCompanySearchResultsinstead of theCompanythatCompany(ticker)resolves. The^[A-Z]{4}X$fund pattern is now tested first and the ticker class admits the full alphabet.Verification
tests/issues/regression/. Each was confirmed to be a real gate by reverting the source and watching it fail — including reproducing the reporter's exactAttributeErrorforXBRLS.get_statement(use_optimal_periods=False)crashes because a tuple is passed toStatementStitcher#1173.tests/issues: 2230 passed, 1 xfailed. No new failures.Notes for review
find()fails to recognize valid ordinary tickers containingX#1178 is provably unchanged: under the old ordering no X-containing string could reach the ticker branch at all, so everything that previously matched^[A-Z]{4}X$still does. The tests pinVFIAX/SPHIX/FXAIXto the fund branch.find()fails to recognize valid ordinary tickers containingX#1178 is a behavior change infind()'s return type for X-containing tickers. It matches the documented contract, but anyone handlingCompanySearchResultsback fromfind("XOM")now gets anEntity.@pytest.mark.networkcase intest_issue_1178_find_ticker_with_x.py(assertingfind("XOM").cik == 34088) has not been run.XBRLS.from_filings()silently swallows unexpected parsing exceptions and returns partial results #1174, Stitchedshow_date_range=Trueaccessor option is ignored by normal Rich/reprrendering #1176, EntityFacts income and cash-flow statements ignoreperiod_length#1177, [DATA]discrete_quarters=Truecan relabel full-year cash-flow values as Q4 when a shorter-period concept is missing #1179, [DATA] EntityFacts quarterly cash-flow statement returns a nine-month YTD value as Q3 #1180, [PERFORMANCE]FactQuery.to_dataframe(*columns)materializes the full fact width before applying column projection #1181) are untouched and the data-correctness ones still need ground-truth checking against real filings.🤖 Generated with Claude Code