diff --git a/app/services/fundamentals_api_service.py b/app/services/fundamentals_api_service.py index f3c158e..05efb79 100644 --- a/app/services/fundamentals_api_service.py +++ b/app/services/fundamentals_api_service.py @@ -162,6 +162,10 @@ def _build_valuation(derived, subject_price, peer_derived, peer_price_by_cik, tw "shares_estimated": bool( market_cap is not None and derived.shares_outstanding_estimated ), + # A null P/E is ambiguous: no earnings data, or earnings we deliberately + # suppressed. Only the latter carries a caveat, so a split-contaminated + # TTM says why instead of looking like missing data. + "pe_caveat": derived.ttm_diluted_eps_caveat if pe is None else None, "pe_industry": pe_industry, "fcf_yield_industry": fcf_yield_industry, "price_date": _iso(price_date), diff --git a/app/services/fundamentals_derivation.py b/app/services/fundamentals_derivation.py index af0e18f..43a7610 100644 --- a/app/services/fundamentals_derivation.py +++ b/app/services/fundamentals_derivation.py @@ -46,7 +46,7 @@ _FLOW_FIELDS = ( _MERGED_FIELDS = ( *_FLOW_FIELDS, "cash_and_st_investments", "total_debt", "shares_outstanding", - "shares_outstanding_date", + "shares_outstanding_date", "weighted_avg_diluted_shares", # period_start is set alongside revenue by the parser, so it follows the same # fallback: a bare amendment reports neither and must not blank it. "period_start", diff --git a/app/services/sec_facts_parser.py b/app/services/sec_facts_parser.py index db1f943..6a6e8f9 100644 --- a/app/services/sec_facts_parser.py +++ b/app/services/sec_facts_parser.py @@ -310,6 +310,12 @@ def _period_identity( ends in early January (DPZ) shifts by one — they need to be unique, monotonic and YoY-aligned, which is all the derivation asks of them. Nothing outside the derivation reads these columns. + + Known limitation: ``fiscalYearEnd`` is the issuer's *current* calendar, so a + company that has changed its fiscal year end gets its historical periods + measured against the new one. The quarter tolerance shunts most of those to + the fy/fp fallback, and a same-key collision resolves newest-wins, so the + failure mode is a degraded old year rather than a scrambled current one. """ fy = _fiscal_year_of(meta.report_date, fiscal_year_end) if fy is None: diff --git a/app/services/sec_fundamentals_importer.py b/app/services/sec_fundamentals_importer.py index ccd0944..46aa2f3 100644 --- a/app/services/sec_fundamentals_importer.py +++ b/app/services/sec_fundamentals_importer.py @@ -345,8 +345,10 @@ class SecFundamentalsImporter: code="no_xbrl_filings", message=( f"{len(staged.no_xbrl_filings)} tracked issuer(s) resolved to a " - f"registrant with no XBRL 10-K/10-Q — pin the right CIK via the " - f"'{sec_universe.CIK_OVERRIDES_KEY}' setting: {named}" + f"registrant with no XBRL 10-K/10-Q. Either a successor shell " + f"(pin the real filer via the '{sec_universe.CIK_OVERRIDES_KEY}' " + f"setting) or a new registrant that has not filed its first " + f"10-K/10-Q yet, which needs nothing and clears itself: {named}" )[:4000], dedup_key=f"sec_facts:no_xbrl_filings:{run_id}", created_at=_now(), diff --git a/reports/fundamentals-parity-20260723-findings.md b/reports/fundamentals-parity-20260723-findings.md index 73eaf53..2f9eb51 100644 --- a/reports/fundamentals-parity-20260723-findings.md +++ b/reports/fundamentals-parity-20260723-findings.md @@ -801,6 +801,29 @@ data limitation and two operational steps that only run against production. 8. ~~**PPL basic-EPS fallback**~~ — **done**, ninth pass. 9. ~~**HAL null TTM EPS**~~ — **resolved**, ninth pass (already fixed; 2024 is a filer error). +## Review finding — two fixes on this branch silently interacted + +Caught in review, not by me. `_merge_amendments` (the per-field amendment fix, first pass) +builds the merged period from `_MERGED_FIELDS` + `_CARRIED_FIELDS` alone, so a column in +neither list is **absent** from the merged row, not merely stale — and every caller reads it +with `getattr(row, name, None)`, which quietly returns `None`. + +`weighted_avg_diluted_shares` (the market-cap fallback, seventh pass) was never added to +`_MERGED_FIELDS`. The failure needed both fixes to be present at once: a multi-class issuer +*and* a partial amendment on its latest period — META with a Part-III-only 10-K/A — would +silently lose market cap and FCF yield again, i.e. the seventh pass's fix undone by the +first pass's mechanism. I updated `_SNAPSHOT_COLS` in the importer when adding the column +but not `_MERGED_FIELDS` in the derivation. + +Fixed, with a regression test for the specific case. The more useful addition is a guard — +`test_merge_lists_cover_every_parser_field` asserts the two lists cover every `SnapshotRow` +field, so the *next* column added fails loudly instead of losing data quietly. Verified it +would have caught this one. + +Lesson worth keeping: a hand-maintained field list that reconstructs an object is a silent +data-loss footgun. `_SNAPSHOT_COLS` (importer) and `_MERGED_FIELDS` (derivation) must both +track the parser's `SnapshotRow`, and only one of them is now enforced by a test. + ## Genuinely unfixable from this data - **§1 part (b)** — a split post-dating the last filing (KLAC). No snapshot carries diff --git a/tests/unit/test_fundamentals_derivation.py b/tests/unit/test_fundamentals_derivation.py index 95e107d..77211de 100644 --- a/tests/unit/test_fundamentals_derivation.py +++ b/tests/unit/test_fundamentals_derivation.py @@ -291,6 +291,20 @@ def test_point_in_time_share_count_is_preferred_and_not_flagged(): assert d.shares_outstanding_estimated is False +def test_weighted_average_fallback_survives_a_partial_amendment(): + # A Part-III-only 10-K/A on a multi-class issuer's latest period: the merged + # row must keep the weighted-average count, or market cap silently vanishes. + rows = _two_years() + for row in rows: + row.shares_outstanding = None + row.weighted_avg_diluted_shares = 2_564_000_000.0 + part_iii_only = Snap(2026, "FY", date(2026, 9, 30), date(2026, 11, 1), + datetime(2027, 1, 1, tzinfo=UTC)) + d = fd.derive(rows + [part_iii_only]) + assert d.shares_outstanding == 2_564_000_000.0 + assert d.shares_outstanding_estimated is True + + def test_no_share_count_at_all_stays_none_and_unflagged(): rows = _two_years() for row in rows: @@ -298,3 +312,22 @@ def test_no_share_count_at_all_stays_none_and_unflagged(): d = fd.derive(rows) assert d.shares_outstanding is None assert d.shares_outstanding_estimated is False + + +def test_merge_lists_cover_every_parser_field(): + """_MERGED_FIELDS/_CARRIED_FIELDS are hand-maintained, and _merge_amendments + builds the merged row from them alone — so a parser field missing from both + is not merely stale on a merged period, it is *absent*, and callers using + getattr(row, name, None) read None. That is how weighted_avg_diluted_shares + silently lost market cap for multi-class issuers with a partial amendment. + Adding a column to SnapshotRow must fail here rather than lose data quietly. + """ + import dataclasses + + from app.services.sec_facts_parser import SnapshotRow + + parser_fields = {f.name for f in dataclasses.fields(SnapshotRow)} + covered = set(fd._MERGED_FIELDS) | set(fd._CARRIED_FIELDS) + assert not parser_fields - covered, ( + f"parser fields not merged or carried: {sorted(parser_fields - covered)}" + ) diff --git a/tests/unit/test_sec_facts_parser.py b/tests/unit/test_sec_facts_parser.py index c398559..3bc6f6b 100644 --- a/tests/unit/test_sec_facts_parser.py +++ b/tests/unit/test_sec_facts_parser.py @@ -463,3 +463,34 @@ def test_diluted_still_wins_over_basic_when_both_present(): datetime(2026, 5, 1, 10, tzinfo=UTC), "10-Q")} res = parse_snapshots(companyfacts, filings, {"X"}, fiscal_year_end="1231") assert res.rows[0].diluted_eps == 1.36 + + +def test_weighted_average_shares_prefers_the_shortest_span(): + # A 10-Q carries both the quarter's average and the YTD one. The shorter + # window sits closer to the current count, which is what market cap wants. + companyfacts = { + "cik": 1326801, + "facts": {"us-gaap": {"WeightedAverageNumberOfDilutedSharesOutstanding": { + "units": {"shares": [ + _dur("2026-01-01", "2026-09-30", 2_600_000_000, "X", fp="Q3"), # YTD + _dur("2026-07-01", "2026-09-30", 2_564_000_000, "X", fp="Q3"), # quarter + ]} + }}}, + } + filings = {"X": FilingMeta(date(2026, 9, 30), date(2026, 11, 1), + datetime(2026, 11, 1, 10, tzinfo=UTC), "10-Q")} + res = parse_snapshots(companyfacts, filings, {"X"}, fiscal_year_end="1231") + assert res.rows[0].weighted_avg_diluted_shares == 2_564_000_000 + + +def test_weighted_average_shares_falls_back_to_the_basic_and_diluted_concept(): + companyfacts = { + "cik": 1326801, + "facts": {"us-gaap": {"WeightedAverageNumberOfSharesOutstandingBasicAndDiluted": { + "units": {"shares": [_dur("2026-07-01", "2026-09-30", 500_000, "X", fp="Q3")]} + }}}, + } + filings = {"X": FilingMeta(date(2026, 9, 30), date(2026, 11, 1), + datetime(2026, 11, 1, 10, tzinfo=UTC), "10-Q")} + res = parse_snapshots(companyfacts, filings, {"X"}, fiscal_year_end="1231") + assert res.rows[0].weighted_avg_diluted_shares == 500_000