Skip to content

Fix acquire_sky_supply_basis: omit fully_diluted_supply when incoherent with total_supply#117

Merged
fafa33 merged 1 commit into
mainfrom
claude/adr-0021-audit-kan3zl
Jul 26, 2026
Merged

Fix acquire_sky_supply_basis: omit fully_diluted_supply when incoherent with total_supply#117
fafa33 merged 1 commit into
mainfrom
claude/adr-0021-audit-kan3zl

Conversation

@fafa33

@fafa33 fafa33 commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up fix for the Milestone 1 one-shot workflow (.github/workflows/acquire-sky-supply-basis.yml, scripts/acquire_sky_supply_basis.py, merged via PR #116). The first live run against a GitHub-hosted runner reached CoinGecko successfully (first-ever live acquisition in this effort) but failed with:

ValueError: total supply must not exceed fully diluted supply

Root cause: CoinGecko's real market_data for Sky reports max_supply (23462665147.3) fractionally below total_supply (23462665147.36596) for the same snapshot — a real provider data-precision quirk, not an acquisition bug. build_supply_payload() unconditionally mapped max_supply to the fully_diluted_supply quantity component, so it handed SupplyBasisSnapshot.__post_init__'s canonical invariant (total_supply <= fully_diluted_supply, src/hunter/value_capture/models.py) a payload it correctly rejects.

Scope and architecture

  • Governing Issue: Implement canonical valuation methodology (ADR 0022) #107 (Milestone 1). No ADR change.
  • Architecture Impact Check: none. The canonical SupplyBasisSnapshot coherence invariant is untouched and unweakened, per this repo's stated constraint that canonical service/model contracts must not be relaxed to accommodate acquisition scripts.
  • Evidence Impact: none — no evidence semantics changed.
  • No unapproved scope expansion.
  • Replay/persistence boundaries intact — fully_diluted_supply is (and already was) an optional quantity_components entry; this change only adds a coherence pre-check before including it, rather than fabricating, rounding, or otherwise altering either provider-observed value.

Acceptance-criteria matrix

Acceptance criterion Status Evidence
Fix does not fabricate, round, or alter real provider-observed market fact values PASS build_supply_payload still passes total_supply/max_supply values through verbatim; it only conditionally omits fully_diluted_supply as a component
Fix does not weaken SupplyBasisSnapshot's canonical coherence invariant PASS src/hunter/value_capture/models.py unchanged
New test reproduces the real observed CoinGecko data shape (max_supply < total_supply) PASS tests/test_acquire_sky_supply_basis.py::test_build_supply_payload_omits_fully_diluted_supply_when_max_supply_is_below_total_supply
Coherent case (max_supply >= total_supply) still includes fully_diluted_supply PASS tests/test_acquire_sky_supply_basis.py::test_build_supply_payload_includes_fully_diluted_supply_when_max_supply_meets_total_supply
Existing script/test suite unaffected PASS tests/test_acquire_sky_supply_basis.py — 16/16 pass
  • No criterion omitted or inferred from green CI.
  • No FAIL/BLOCKED criterion remains.

Verification

  • ruff check .
  • black --check .
  • mypy (via python -m mypy, matching the project's installed interpreter)
  • Full pytest suite
$ python -m ruff check .
All checks passed!

$ python -m black --check .
All done! ✨ 🍰 ✨
571 files would be left unchanged.

$ python -m mypy
Success: no issues found in 570 source files

$ python -m pytest -q
1136 passed in 453.69s (0:07:33)

$ python -m pytest tests/test_acquire_sky_supply_basis.py -q
16 passed in 1.15s

Operational validation

This is a pure logic fix to an acquisition script's payload-building helper; it does not itself perform live acquisition. Operational (live CoinGecko + persisted SupplyBasisSnapshot) validation happens when the Milestone 1 workflow is re-run from the Actions tab after this PR merges to main (the workflow fetches scripts/acquire_sky_supply_basis.py fresh from main at run time).

  • Prior live run (workflow run V2: Scenario Simulation Engine #3, job 89768088150, commit 2772c9c) is the operational evidence motivating this fix: it reached live CoinGecko, ingested and persisted market facts, then failed exactly at the fully_diluted_supply coherence check this PR fixes.
  • A subsequent live run confirming the persisted SupplyBasisSnapshot is deferred to the next workflow dispatch after merge (tracked as follow-up, not part of this PR's scope).
  • No fixture, fabricated, or current-state-substituted data was used to construct the fix or its tests — the new test fixtures use the exact real values observed from the live CoinGecko response in the failed run's logs.

Operational evidence:

Prior failing run (motivating this fix):
  market fact: circulating_supply=23355983976.864086 native_units
  market fact: total_supply=23462665147.36596 native_units
  market fact: max_supply=23462665147.3 native_units
  ValueError: total supply must not exceed fully diluted supply

Remaining limitations and risks

  • The actual persisted SupplyBasisSnapshot for Sky is not yet created; that requires a fresh "Run workflow" dispatch of acquire-sky-supply-basis.yml on main after this PR merges.
  • If CoinGecko's max_supply for Sky is later corrected upstream to be >= total_supply, the fully_diluted_supply component will simply be included again automatically — no further code change needed.

Implementer readiness declaration

  • READY FOR REVIEW

Generated by Claude Code

…nt with total_supply

Real CoinGecko data for Sky reports max_supply fractionally below
total_supply for the same snapshot, which violated
SupplyBasisSnapshot's canonical total<=fully_diluted invariant and
caused the live workflow run to fail after successfully reaching
CoinGecko. build_supply_payload now omits the optional
fully_diluted_supply component when max_supply < total_supply rather
than passing through data the model would correctly reject.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZwAhhLUYJMGgJm9yyzs7H
@fafa33
fafa33 marked this pull request as ready for review July 26, 2026 07:43
@fafa33
fafa33 merged commit ee557d4 into main Jul 26, 2026
1 check passed
@fafa33
fafa33 deleted the claude/adr-0021-audit-kan3zl branch July 26, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants