fix: replace Forager quality-gate hardcoded literal with named constant#147
Conversation
The activity-score boundary in ``_check_forager_health`` was a bare ``50.0`` literal, which contradicted the design's "no hardcoded numeric tunables" intent. The value was always meant to track ``CoinHealthTracker._NO_HISTORY_NEUTRAL_SCORE`` (the midpoint the tracker emits for unknown coins, also 50.0); pin the dependency explicitly so adjusting the neutral score in one place updates both. Also add two parametrised tests that pin the strict-inequality boundary semantics: * ``activity == _NO_HISTORY_NEUTRAL_SCORE`` → gate does not engage, cooldown fires. * ``activity > _NO_HISTORY_NEUTRAL_SCORE + epsilon`` → gate engages, cooldown is suppressed. These regression-pin the boundary so a future refactor of ``>`` to ``>=`` is detected by CI. Addresses the should-fix from PR #146 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
keitaj
left a comment
There was a problem hiding this comment.
Review: Replace Forager quality-gate hardcoded literal with named constant
Surgical follow-up to the should-fix from PR #146 review. The literal 50.0 is replaced with CoinHealthTracker._NO_HISTORY_NEUTRAL_SCORE, the comment now documents why those two values are linked, and two boundary tests pin the strict-inequality semantic so a future > → >= refactor is caught by CI. No runtime behaviour change.
1. Cross-module access to a leading-underscore attribute (nit)
strategies/market_making_strategy.py L1219:
CoinHealthTracker._NO_HISTORY_NEUTRAL_SCORE is referenced from outside the class. The leading underscore is the Python convention for "private", and now there are two consumers (strategy + the new boundary tests) treating it as public.
Suggestion (optional, future cleanup): promote the attribute to NO_HISTORY_NEUTRAL_SCORE (no leading underscore) when other Forager-internal constants accumulate, or expose it via a small accessor (@classmethod neutral_score()). Not needed for this PR — the underscore convention is informational and there's no real encapsulation barrier in Python — but worth noting for the next refactor pass.
Verdict: LGTM
No blockers, no should-fix. The change is minimal, deterministic, and adds regression-pinning tests for the boundary semantic. Ready to merge.
🤖 Generated with Claude Code
Summary
50.0literal in Forager's quality-gate with a reference toCoinHealthTracker._NO_HISTORY_NEUTRAL_SCORE, the named midpoint the tracker already uses for unknown coins.>to>=is caught by CI.Addresses the should-fix from PR #146 review (#146 (review)).
Changes
strategies/market_making_strategy.pyL1208-1219: swap the literal50.0forCoinHealthTracker._NO_HISTORY_NEUTRAL_SCORE. Update the surrounding comment to document the linkage.tests/test_forager.py: addtest_quality_gate_boundary_at_neutral_scoreandtest_quality_gate_just_above_neutral_score_blocks_triggerto pin the strict>boundary.Test plan
flake8 . --max-line-length=120passespytest tests/ -qpasses (1006 tests, 0 failures — 2 new undertest_forager.py)forager_enabled=false(the gate is unreachable)forager_enabled=trueeither — the value emitted by the named constant equals the prior literal exactly🤖 Generated with Claude Code