Canonical fix: persist fully_diluted_supply even when incoherent with total_supply#118
Merged
Conversation
Root cause: SupplyBasisSnapshot.__post_init__ hard-rejected any total_supply > fully_diluted_supply, which real CoinGecko data for Sky violates by a relative gap of ~2.8e-12 (provider rounding, not a real economic contradiction). The only prior workaround (omitting fully_diluted_supply entirely, PR #117) was independently flagged as insufficient: ADR 0022 Scope condition 3 requires all three supply quantities to be independently observable, and an omitted component cannot satisfy that. Replace the hard raise with a tolerance-based policy, mirroring the existing conflict_state="open" pattern already used elsewhere in this record family (SupplyAndValueCaptureService._apply_conflict_policy): raw provider values are always persisted exactly as observed, never clamped or fabricated; a gap within SUPPLY_COHERENCE_RELATIVE_TOLERANCE (0.01%, entity/provider-agnostic) is accepted as ordinary data; a larger gap is not rejected either, but surfaces via conflict_state="open" instead of silently passing as "none". No exception is ever raised by this check anymore. scripts/acquire_sky_supply_basis.py now always includes fully_diluted_supply when max_supply is observed, since the coherence policy decision now lives exclusively in the canonical model. No schema/migration change; every previously-accepted SupplyBasisSnapshot already satisfied the strict form of this check, so relaxing it cannot invalidate existing persisted data. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZwAhhLUYJMGgJm9yyzs7H
fafa33
marked this pull request as ready for review
July 26, 2026 08:41
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90de0a2a57
ℹ️ 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".
…erence
Independent review (Codex) on this same PR found the first version of
this fix left a bypass: it only forced conflict_state="open" when the
caller supplied "none", so an initial record (no
supersedes_record_id) could self-declare "resolved" for a
beyond-tolerance total>fully_diluted gap and pass straight through
strict_known_supply/strict_known_evidence's {"none", "resolved"}
eligibility check, since _authorize_correction only runs authority
checks when a correction predecessor exists.
An initial record has no prior, authority-checked correction that
could legitimately have resolved anything, so conflict_state is now
forced to "open" unconditionally in that case, regardless of what the
payload claims. Only a correction (already gated by
_authorize_correction's source-tier transition check) may supply an
explicit non-"none" conflict_state that is respected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZwAhhLUYJMGgJm9yyzs7H
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
2 participants
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.
Summary
Independent review (Codex, on PR #110) flagged that Sky's persisted
SupplyBasisSnapshothad nofully_diluted_supplycomponent — an interim omission-based fix (PR #117) that satisfiedSupplyBasisSnapshot's canonicaltotal<=fully_dilutedinvariant but didn't actually satisfy ADR 0022 Scope condition 3 (all three supply quantities independently observable). This PR fixes the canonical architecture instead of the acquisition script:SupplyBasisSnapshot.__post_init__no longer hard-rejectstotal_supply > fully_diluted_supply.Full root cause analysis, architecture rationale, exact invariant change, and empirical proof are in
docs/IMPLEMENTATION_REPORTS/v3.6.1-supply-basis-coherence-tolerance.md.Scope and architecture
SupplyBasisSnapshot.__post_init__'stotal<=fully_dilutedcheck is relaxed to a tolerance-based, non-rejecting policy (raw values always persisted verbatim;conflict_state="open"surfaces gaps beyond a small entity/provider-agnostic tolerance instead of silently accepting or hard-rejecting). No other coherence check in the same model was touched.Acceptance-criteria matrix
SupplyBasisSnapshot(...)construction with Sky's exact real CoinGecko values succeeds, no exceptionquantity_componentsstores caller-supplied values verbatim in both the within-tolerance and beyond-tolerance test casesobserved_market_fact_ids/evidence_record_idsunchanged, no new required fieldstest_supply_basis_accepts_total_above_fully_diluted_within_precision_tolerancetest_supply_basis_flags_conflict_when_total_exceeds_fully_diluted_beyond_toleranceSUPPLY_COHERENCE_RELATIVE_TOLERANCEis a flat constant applied uniformly; no entity/provider name anywhere in the checkFAIL/BLOCKEDcriterion remains.Verification
ruff check .black --check .python -m mypypytestsuiteOperational validation
Not applicable in the live-acquisition sense — this is a model-layer fix, not an acquisition run. The report's "Proof" section demonstrates, with Sky's exact real observed CoinGecko values (reproduced from the actual failed workflow run's log), that the previously-rejected construction now succeeds. A fresh live acquisition run against the corrected
main(via.github/workflows/acquire-sky-supply-basis.yml) is the next step after this merges, to replace PR #110 branch's existing (fully_diluted_supply-omitted)SupplyBasisSnapshotwith a complete one.Remaining limitations and risks
SUPPLY_COHERENCE_RELATIVE_TOLERANCE = 0.0001is a considered policy constant, not derived from a formal model of any specific provider's rounding behavior — disclosed in the report's "Known limitations" section.SupplyBasisSnapshotalready persisted on PR Issue #107 Milestone 1: entity-agnostic valuation evidence acquisition (Sky pilot) #110's branch; a fresh acquisition run is required after this merges.Implementer readiness declaration
READY FOR REVIEWGenerated by Claude Code