Jérôme Benoit [Tue, 23 Dec 2025 19:13:18 +0000 (20:13 +0100)]
refactor(ReforceXY): PBRS refactoring, bug fix, and documentation harmonization
This commit includes three major improvements to the PBRS implementation:
1. Bug Fix: idle_factor calculation
- Fixed incorrect variable reference in reward_space_analysis.py:625
- Changed 'factor' to 'base_factor' in idle_factor formula
- Formula: idle_factor = base_factor * (profit_aim / risk_reward_ratio) / 4.0
- Also fixed in test_reward_components.py and ReforceXY.py
2. Refactoring: Separation of concerns in PBRS calculation
- Renamed apply_potential_shaping() → compute_pbrs_components()
- Removed base_reward parameter from PBRS functions
- PBRS functions now return only shaping components
- Caller responsible for: total = base_reward + shaping + entry + exit
- Kept deprecated wrapper for backward compatibility
- Updated ReforceXY.py with parallel changes
- Adapted tests to new function signatures
3. Documentation: Complete mathematical notation harmonization
- Achieved 100% consistent notation across both implementations
- Standardized on Greek symbols: Φ(s), γ, Δ(s,a,s')
- Eliminated mixing of word forms (Phi/gamma/Delta) with symbols
- Harmonized docstrings to 156-169 lines with identical theory sections
- Added cross-references between implementations
- Fixed all instances of Δ(s,s') → Δ(s,a,s') to include action parameter
Files modified:
- reward_space_analysis/reward_space_analysis.py: Core refactoring + docs
- user_data/freqaimodels/ReforceXY.py: Parallel refactoring + docs
- tests/components/test_additives.py: Adapted to new signature
- tests/components/test_reward_components.py: Bug fix
- tests/api/test_api_helpers.py: Adapted to new signature
All 50 tests pass. Behavior preserved except for intentional bug fix.
Jérôme Benoit [Sat, 20 Dec 2025 21:33:38 +0000 (22:33 +0100)]
refactor(tests): Standardize constants, improve documentation and add docstrings (#22)
* Refactor: Standardize constant access patterns in test suite
Replace class-based constant access (self.TEST_*, self.TOL_*, etc.) with
direct imports from tests/constants module. This improves code clarity and
follows the DRY principle by using the centralized constants module as the
single source of truth.
Changes:
- Replace self.TEST_* with PARAMS.* across all test files
- Replace self.TOL_* with TOLERANCE.* for all tolerance values
- Replace self.SEED* with SEEDS.* for test seeds
- Replace self.CONTINUITY_* with CONTINUITY.*
- Replace self.EXIT_FACTOR_* with EXIT_FACTOR.*
- Replace self.BH_FP_RATE_THRESHOLD with STATISTICAL.BH_FP_RATE_THRESHOLD
- Add/update imports in 12 test files to include required constants
- Fix PNL_BASE_STD -> PNL_STD (correct constant name)
Files modified:
- tests/test_base.py: Updated imports and all constant references
- tests/api/test_api_helpers.py: Added PARAMS, SEEDS, TOLERANCE imports
- tests/cli/test_cli_params_and_csv.py: Added SEEDS import
- tests/components/test_*.py: Updated to use direct imports
- tests/integration/test_*.py: Updated constant access patterns
- tests/pbrs/test_pbrs.py: Comprehensive constant migration
- tests/robustness/test_robustness.py: Updated all robustness tests
- tests/statistics/test_statistics.py: Added required constants
All tests verified passing after migration.
* docs(tests): Add tolerance justification guide and docstring template
Phase 1.2 & 1.3: Comprehensive documentation for test suite best practices
Added:
1. TOLERANCE_GUIDE.md - Comprehensive guide for tolerance selection
- Decision matrix for choosing appropriate tolerances
- Detailed rationale for each tolerance constant
- Usage examples and anti-patterns
- Mathematical justification for tolerance magnitudes
2. .docstring_template.md - Standardized test docstring format
- Multiple template levels (minimal, standard, complex)
- Section-by-section guidelines
- Common patterns (property-based, regression, integration)
- Checklist for new tests
3. Tolerance justification comments in test files
- test_robustness.py: Added 11 inline comments explaining tolerance choices
- test_pbrs.py: Enhanced docstring with tolerance rationale
- Focused on IDENTITY_RELAXED and NUMERIC_GUARD usages
4. Fixed test_statistics.py imports
- Moved constants import to proper location (top of file)
- Removed duplicate import that was incorrectly placed mid-function
Benefits:
- New contributors can quickly understand tolerance choices
- Reduces test flakiness by documenting error accumulation models
- Establishes clear standards for test documentation
- Makes tolerance selection systematic rather than arbitrary
See also: tests/constants.py for tolerance constant definitions
* docs(tests): Enhance README with constant management and tolerance guidelines
Completes Phase 1 documentation by integrating the tolerance guide and
docstring template into the authoritative test suite README.
* fix(tests): Correct constant reference in bootstrap reproducibility test
Fix incorrect constant migration: SCENARIOS.BOOTSTRAP_ITERATIONS does not
exist. Should use STATISTICAL.BOOTSTRAP_DEFAULT_ITERATIONS instead.
The migration script incorrectly mapped self.BOOTSTRAP_DEFAULT_ITERATIONS
to SCENARIOS.BOOTSTRAP_ITERATIONS when it should have been mapped to
STATISTICAL.BOOTSTRAP_DEFAULT_ITERATIONS (from StatisticalConfig dataclass).
Fixes test_stats_hypothesis_seed_reproducibility in test_statistics.py.
* docs(tests): Clarify TOLERANCE_GUIDE.md as authoritative source
Make it explicit that TOLERANCE_GUIDE.md is the single authoritative source
for tolerance documentation, with README.md providing only a quick reference.
This follows the 'No duplication' principle from copilot-instructions.md:
maintain single authoritative documentation source; reference other sources
rather than copying.
* refactor(tests): Complete constant migration - remove all self.* references
Run migration script to eliminate remaining 28 self.* constant references
across 4 test files:
- test_api_helpers.py: self.SEED → SEEDS.SMOKE_TEST (fixed mapping)
- test_reward_components.py: self.TEST_RR → PARAMS.RISK_REWARD_RATIO (2×)
- test_reward_calculation.py: self.TEST_RR → PARAMS.RISK_REWARD_RATIO (1×)
- test_robustness.py: self.TEST_RR → PARAMS.RISK_REWARD_RATIO (24×)
The migration script (tests/scripts/migrate_constants.py) automates this
refactoring to maintain consistency. It serves two purposes:
1. Complete initial migration (this commit)
2. Future-proof tool for adding new constants or refactoring patterns
All test classes now use direct imports from tests/constants.py.
Class-level aliases in RewardSpaceTestBase can be removed in a follow-up.
* refactor(tests): Remove migration script and unused aliases
- Delete temporary migration script (tests/scripts/migrate_constants.py)
- Remove unused class constant aliases from test_base.py
- Migrate remaining self.PBRS_* references to PBRS.* in test_pbrs.py
- Fix test_case.TOL_IDENTITY_STRICT reference in test_reward_components.py
- Keep only actively used class constants (DEFAULT_PARAMS, PBRS_TERMINAL_PROB, PBRS_SWEEP_ITER, JS_DISTANCE_UPPER_BOUND)
All 154 tests passing.
* docs(tests): Add 21 docstrings to helpers, robustness and statistics tests
Add comprehensive docstrings following .docstring_template.md format:
repositories / freqai-strategies.git / log
