dennisthiessen/signal-platform
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(sec): A3 slice-2b review — no index cap, full discrepancy + malformed gate

Browse Source 
1. Removed the 45-day index-walk cap: it discarded the older part of a long
   outage while still advancing source_max_date, permanently losing filings.
   The walk now covers every unprocessed date (a large gap is one-time cost).
2. Discrepancy detection meets the immutability contract: it compares ALL source
   snapshot fields (not five), read-only during stage/validate, reports the
   differing accessions + fields in validation_json, and promote emits a warning
   system event (in-transaction) — never mutating the stored row.
3. Malformed companyfacts (missing facts/units structure) are recorded separately
   and FAIL validation, instead of silently degrading to skipped rows that the
   50% backfill coverage floor could still pass.

Also corrected the stale "sum share classes" / DEI-only wording in the snapshot
model docstring and the A3 design doc to describe the us-gaap fallback.

Tests: +4 regressions (>45-day gap loses nothing, newly-added issuer backfills
without filing, malformed payload fails, shares discrepancy detected + evented).
23 passed, 1 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
dennisthiessen
2026-07-22 19:55:58 +02:00
co-authored by Claude Opus 4.8
parent f7ce85a33e
commit 8dcdcac2a6
4 changed files with 194 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/services/sec_fundamentals_importer.py
+72 -32
View File
@@ -36,6 +36,7 @@ from sqlalchemy import func, select
from app.database import insert_for_session
from app.models.data_import_run import DataImportRun
from app.models.fundamental_snapshot import FundamentalSnapshot
from app.models.system_event import SystemEvent
from app.services import sec_facts_parser as parser
from app.services import sec_universe
from app.services.data_import import STATUS_PROMOTED, ValidationResult
@@ -50,9 +51,6 @@ _XBRL_FORMS = {"10-K", "10-Q", "10-K/A", "10-Q/A"}
# On the one-time backfill, require this fraction of tracked issuers to yield at
# least one snapshot (guards a broken fetch/parse from promoting a hollow table).
MIN_BACKFILL_COVERAGE = 0.5
# Bound how far back the incremental index walk goes if the job hasn't run in a
# while (each day = one small request); older gaps are logged, not silently lost.
_MAX_INDEX_WALK_DAYS = 45
_SNAPSHOT_COLS = (
"cik", "accession", "form", "filed_date", "accepted_at", "period_start",
@@ -61,8 +59,9 @@ _SNAPSHOT_COLS = (
"cash_and_st_investments", "total_debt", "shares_outstanding",
"shares_outstanding_date",
)
# Fields compared to flag a differing existing accession (immutable → report, not mutate).
_DISCREPANCY_COLS = ("period_end", "fiscal_year", "fiscal_period", "revenue", "net_income")
# Compare ALL source fields (every column except the accession key) to flag a
# differing existing accession — immutable, so we report, never mutate.
_COMPARE_COLS = tuple(c for c in _SNAPSHOT_COLS if c != "accession")
@dataclass
@@ -74,6 +73,9 @@ class StagedFundamentals:
field_issues: list[dict[str, str]] = field(default_factory=list)
skipped_non_xbrl: list[dict[str, str]] = field(default_factory=list)
missing_xbrl: list[dict[str, str]] = field(default_factory=list)
invalid_payloads: list[dict[str, str]] = field(default_factory=list)
existing_accessions: set[str] = field(default_factory=set)
discrepancies: list[dict[str, Any]] = field(default_factory=list)
backfill: bool = False
issuers_fetched: int = 0
issuers_with_rows: int = 0
@@ -146,10 +148,30 @@ class SecFundamentalsImporter:
for cik in sorted(backfill_ciks | incremental_ciks):
is_backfill = cik in backfill_ciks
await self._stage_issuer(client, cik, is_backfill, filed_by_cik, staged)
# Read-only discrepancy detection: an accession we reconstructed that is
# already stored, differing in ANY source field (immutable → report in
# validation, event on promote, never mutate). Also gives promote the
# existing set so its insert count is dialect-independent.
if staged.rows:
existing = await self._existing_by_accession(db, [r.accession for r in staged.rows])
staged.existing_accessions = set(existing)
for row in staged.rows:
old = existing.get(row.accession)
if old is not None:
fields = _diff_fields(row, old)
if fields:
staged.discrepancies.append({"accession": row.accession, "fields": fields})
return staged
async def _stage_issuer(self, client, cik, is_backfill, filed_by_cik, staged) -> None:
cf = await client.companyfacts(cik)
if not isinstance(cf, dict) or not isinstance(cf.get("facts"), dict):
# Malformed payload (missing facts/units structure) — record separately
# and fail validation, rather than letting it degrade to skipped rows.
staged.invalid_payloads.append({"cik": cik10(cik), "reason": "missing facts structure"})
staged.issuers_fetched += 1
return
sub = await client.submissions(cik, include_history=is_backfill)
xbrl_meta, nonxbrl = _filing_meta(sub)
@@ -191,6 +213,12 @@ class SecFundamentalsImporter:
f"{len(staged.missing_xbrl)} tracked XBRL filing(s) not yet in "
"Company Facts (index/facts lag) — retry"
)
# Malformed companyfacts payloads must fail, not degrade to skipped rows.
if staged.invalid_payloads:
messages.append(
f"{len(staged.invalid_payloads)} issuer(s) returned a malformed "
"companyfacts payload (missing facts structure)"
)
accns = [r.accession for r in staged.rows]
if len(accns) != len(set(accns)):
@@ -213,7 +241,11 @@ class SecFundamentalsImporter:
"field_issues": len(staged.field_issues),
"skipped_non_xbrl": len(staged.skipped_non_xbrl),
"missing_xbrl": len(staged.missing_xbrl),
"invalid_payloads": staged.invalid_payloads,
"cik_updates": len(staged.resolved.cik_updates),
# differing existing accessions (immutable — kept, reported here)
"discrepancies": staged.discrepancies[:50],
"discrepancy_count": len(staged.discrepancies),
}
return ValidationResult(
ok=not messages,
@@ -224,33 +256,37 @@ class SecFundamentalsImporter:
async def promote(self, db, staged: StagedFundamentals, run_id: int) -> dict[str, int]:
inserted = 0
discrepancies = 0
if staged.rows:
existing = await self._existing_by_accession(db, [r.accession for r in staged.rows])
for row in staged.rows:
old = existing.get(row.accession)
if old is not None:
if _differs(row, old):
discrepancies += 1
logger.warning(
"sec_facts: accession %s reconstructed differently than "
"stored (immutable — not overwriting)", row.accession
)
continue # ON CONFLICT DO NOTHING (below) leaves it untouched
stmt = insert_for_session(db, FundamentalSnapshot).values(
**_row_values(row, run_id)
)
stmt = stmt.on_conflict_do_nothing(index_elements=["accession"])
await db.execute(stmt)
inserted += 1
for row in staged.rows:
if row.accession in staged.existing_accessions:
continue # immutable — keep the original row
stmt = insert_for_session(db, FundamentalSnapshot).values(**_row_values(row, run_id))
stmt = stmt.on_conflict_do_nothing(index_elements=["accession"]) # race belt-and-suspenders
await db.execute(stmt)
inserted += 1
# Warn (in-transaction, so it commits atomically with the promotion) when
# any existing accession reconstructed differently — kept immutable.
if staged.discrepancies:
accns = ", ".join(d["accession"] for d in staged.discrepancies[:10])
db.add(SystemEvent(
severity="warning",
source="sec_facts",
code="snapshot_discrepancy",
message=(
f"{len(staged.discrepancies)} stored accession(s) reconstructed "
f"differently; kept immutable: {accns}"
)[:4000],
dedup_key=f"sec_facts:discrepancy:{run_id}",
created_at=_now(),
))
ticker_counts = await sec_universe.apply_ticker_updates(
db, staged.resolved, staged.sic_updates
)
return {
"inserted": inserted,
"existing_unchanged": len(staged.rows) - inserted,
"discrepancies": discrepancies,
"existing_unchanged": len(staged.existing_accessions),
"discrepancies": len(staged.discrepancies),
**ticker_counts,
}
@@ -269,12 +305,15 @@ class SecFundamentalsImporter:
async def _collect_index_rows(
self, client: SecClient, last_processed: date, latest: date
) -> list[dict[str, Any]]:
# Walk EVERY unprocessed date. No cap — dropping the older part of a long
# outage while still advancing source_max_date would permanently lose
# those filings. A large gap is one-time cost, not silent data loss.
tracked = set(self._resolved.cik_to_ticker_ids) if self._resolved else set()
start = max(last_processed + timedelta(days=1), latest - timedelta(days=_MAX_INDEX_WALK_DAYS))
if start > last_processed + timedelta(days=1):
logger.warning("sec_facts: index gap > %d days; walking from %s", _MAX_INDEX_WALK_DAYS, start)
gap = (latest - last_processed).days
if gap > 60:
logger.warning("sec_facts: %d-day index gap since %s; walking all", gap, last_processed)
rows: list[dict[str, Any]] = []
day = start
day = last_processed + timedelta(days=1)
while day <= latest:
for r in await client.daily_index(day):
if r["form"] in _XBRL_FORMS and r["cik"] in tracked:
@@ -339,5 +378,6 @@ def _row_values(row: SnapshotRow, run_id: int) -> dict[str, Any]:
return values
def _differs(row: SnapshotRow, old: FundamentalSnapshot) -> bool:
return any(getattr(row, col) != getattr(old, col) for col in _DISCREPANCY_COLS)
def _diff_fields(row: SnapshotRow, old: FundamentalSnapshot) -> list[str]:
"""Source fields where a re-parsed row differs from the stored (immutable) row."""
return [col for col in _COMPARE_COLS if getattr(row, col) != getattr(old, col)]