Apply reviewed SEC revenue and filing dates - #118
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb5c852e75
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| QQQ,Nasdaq Growth,ETF,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, | ||
| XLF,Financials,ETF,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, | ||
| AMD,,,,2.2434570906877664,0.8134734471758304,,5329000000.0,2.65,6735000000.0,,1.2638393694877088,0.6931882154250328,,,5585000000.0,4098000000.0,,1630600639.0,,,,,,sec_companyfacts,2017-12-30,2488,10-K,2018-02-27,0000002488-18-000042,EBITDA was not staged because no direct SEC EBITDA fact was available.,ADVANCED MICRO DEVICES INC,,, | ||
| AMD,,,,2.2434570906877664,0.8134734471758304,,34639000000.0,2.65,6735000000.0,,1.2638393694877088,0.6931882154250328,,,5585000000.0,4098000000.0,,1630600639.0,,,,,,sec_companyfacts,2017-12-30,2488,10-K,2026-02-04,0000002488-18-000042,EBITDA was not staged because no direct SEC EBITDA fact was available.,ADVANCED MICRO DEVICES INC,,, |
There was a problem hiding this comment.
Recompute FCF margin when replacing revenue
This replaces AMD's revenue denominator while retaining fcf_margin=1.263839..., which is still 6.735B / 5.329B; with the new 34.639B revenue it should be about 19.4%. The same stale-denominator problem exists for AAPL. scenario_lab._like_for_like_baseline prefers the explicit margin over recomputing it and default_scenario_parameters consequently clamps AMD's stale 126% margin to 45%, materially overstating DCF projections. Recompute the dependent margins or apply a period-coherent set of fundamentals.
Useful? React with 👍 / 👎.
| QQQ,Nasdaq Growth,ETF,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, | ||
| XLF,Financials,ETF,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, | ||
| AMD,,,,2.2434570906877664,0.8134734471758304,,5329000000.0,2.65,6735000000.0,,1.2638393694877088,0.6931882154250328,,,5585000000.0,4098000000.0,,1630600639.0,,,,,,sec_companyfacts,2017-12-30,2488,10-K,2018-02-27,0000002488-18-000042,EBITDA was not staged because no direct SEC EBITDA fact was available.,ADVANCED MICRO DEVICES INC,,, | ||
| AMD,,,,2.2434570906877664,0.8134734471758304,,34639000000.0,2.65,6735000000.0,,1.2638393694877088,0.6931882154250328,,,5585000000.0,4098000000.0,,1630600639.0,,,,,,sec_companyfacts,2017-12-30,2488,10-K,2026-02-04,0000002488-18-000042,EBITDA was not staged because no direct SEC EBITDA fact was available.,ADVANCED MICRO DEVICES INC,,, |
There was a problem hiding this comment.
Keep SEC filing metadata from the same filing
The AMD row now pairs a 2026-02-04 filing date with accession 0000002488-18-000042 and a 2017-12-30 period; AAPL similarly pairs its new 2025 date with a 2018 accession and period. build_research_change_snapshot treats sec_filed_date and sec_accession as one latest-filing identity, while _filing_events detects arrivals only when the accession changes, so these new filings are missed and downstream snapshots expose impossible date/accession pairs. Update the companion accession and period metadata or leave the filing date unchanged.
Useful? React with 👍 / 👎.
| os.chmod(temporary_path, canonical_path.stat().st_mode & 0o777) | ||
| if _live_repository_head(result.repository_root) != result.authorized_repository_head: | ||
| raise ValueError("repository HEAD changed before materialization") | ||
| os.replace(temporary_path, canonical_path) |
There was a problem hiding this comment.
Lock the canonical file across validation and replacement
If another process edits the canonical CSV after the hash check at line 398 but before this replacement—for example while the temporary file is flushed or the second Git command runs—os.replace silently overwrites those edits without rechecking the canonical hash. This defeats the stale-canonical guard and can lose concurrent work; hold an exclusive lock through validation and replacement or otherwise perform a conditional update that fails when the source changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c20d87a591
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| or not _is_sha256(binding.get("patch_preview_sha256")) | ||
| or not _is_sha256(binding.get("apply_receipt_sha256")) |
There was a problem hiding this comment.
Verify proof artifacts before exempting canonical data
In the checked commercial-research-beta.yml PR workflow, a contributor can bypass the generated-data hygiene gate by editing data/fundamentals.csv and appending a ledger row containing the readily computed before/after hashes plus arbitrary 64-character values for these two proof hashes. Because neither referenced preview nor receipt is loaded, hashed, or authenticated, the same PR that changes the canonical data can manufacture all evidence needed for _range_fundamentals_proof_matches to return true, allowing arbitrary fundamentals changes through the gate. Bind these hashes to immutable artifacts available to CI or another independently verified authorization source before granting the exemption.
Useful? React with 👍 / 👎.
Summary
revenueandsec_filed_dateEvidence
8d0da6bdd2dd52a938adcd1e882031fd3f278d3fc245151943f6f0d2c0e91553af7d95169fdf568bfee93cc2f90dc94ce15b28326dbb3ab1c372f69715aaa9441b27ac9b...->6cd354f2...RB-20260820-SEC-DIRECT-001data/fundamentals.csvadmitted as reviewed canonical data32434689197passed: full suite, dashboard startup, Research routes, public wording, generated-artifact hygiene, and whitespaceBoundaries
currencyexpansion