Skip to content

Fix MC nested-dataclass round-trip; add CI + vesting-curve test#16

Open
Pattermesh wants to merge 2 commits into
mainfrom
pattermesh/token-simulator-ci-test
Open

Fix MC nested-dataclass round-trip; add CI + vesting-curve test#16
Pattermesh wants to merge 2 commits into
mainfrom
pattermesh/token-simulator-ci-test

Conversation

@Pattermesh

Copy link
Copy Markdown

What

  • Fix a real bug in the Monte Carlo config round-trip. On a clean clone, 4 tests in tests/test_monte_carlo.py failed with AttributeError: 'dict' object has no attribute 'volume_usd_m0': _dict_to_config / _dict_to_dataclass left revenue_streams and vest_buckets as plain dicts, so run() crashed.
    • Root cause 1: _dict_to_config inferred the list element type from a default SimConfig(), whose list fields are empty (field(default_factory=list)), so the reconstruction branch was dead code.
    • Root cause 2: _dict_to_dataclass did cls() with no args, which raises for dataclasses with required positional fields (RevenueStream, VestBucket).
    • Fix: declare the nested-list-field → dataclass mapping explicitly, and build instances directly from matching dict fields.
  • Add CI (.github/workflows/ci.yml): installs the package with dev extras and runs pytest -q on Python 3.10, 3.11, 3.12, on push to main and on PRs. No workflow existed before.
  • Add one meaningful test for the vesting-curve model path (test_vesting_curve_respects_cliff_and_unlocks_full_allocation): asserts supply stays flat through the cliff, then exactly fraction_of_supply * total_supply is released linearly over the unlock window with no overshoot. Mutation-checked: it fails if the cliff logic is removed.

Verification

Clean clone + pip install -e ".[dev]" + pytest -q:

...................                                                      [100%]
19 passed in 0.41s

(18 pre-existing tests, previously 4 failing on a clean clone, plus the 1 new vesting-curve test.)

🤖 Generated with Claude Code

…g test

The Monte Carlo config round-trip (_dict_to_config / _dict_to_dataclass)
left revenue_streams and vest_buckets as dicts, so run() crashed with
AttributeError: 'dict' object has no attribute 'volume_usd_m0'. Four
existing monte_carlo tests failed on a clean clone.

Root cause: _dict_to_config inferred list element types from a default
SimConfig(), whose list fields are empty, so the reconstruction branch
was dead. _dict_to_dataclass also called cls() with no args, which fails
for dataclasses with required positional fields.

Fix: declare the nested-list field -> dataclass mapping explicitly, and
build dataclass instances directly from matching dict fields.

Also add a GitHub Actions CI workflow (pytest on 3.10-3.12) and one
vesting-curve test asserting the cliff holds supply flat then the bucket
releases exactly fraction_of_supply * total_supply over the window.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@abhicris

Copy link
Copy Markdown

Welcome to kcolbchain, @Pattermesh — glad you're here. 🌱

Here's what happens from this PR:

  1. Our automated review looks for obvious issues (tests, secrets, size) within a couple of hours.
  2. If it's clean and CI passes, we merge without back-and-forth.
  3. If we need changes, we'll leave a specific comment — not a generic nit. Push another commit and we re-review.

While you wait:

  • Run the repo's tests locally (see the repo README.md).
  • Keep the PR scoped to one concern — bigger PRs land slower.
  • Don't commit tokens or .env contents.

What happens after your first merge

Thanks for writing the code. We're building this to last.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants