Skip to content

fix: anchor drawdowns at high-water marks - #27

Merged
stefan-jansen merged 2 commits into
mainfrom
fix/drawdown-peak-date
Jul 23, 2026
Merged

fix: anchor drawdowns at high-water marks#27
stefan-jansen merged 2 commits into
mainfrom
fix/drawdown-peak-date

Conversation

@stefan-jansen

Copy link
Copy Markdown
Contributor

What changed

  • Anchor each reported drawdown period at its actual high-water mark.
  • Preserve that peak while losses remain below the reporting threshold.
  • Add a regression test covering delayed threshold crossing.
  • Bump the package version to 0.1.0b25.

Why

The prior implementation set peak_date to the observation immediately before a drawdown crossed the configured threshold. For gradual declines, this shortened the reported drawdown and assigned the wrong peak date. The corrected scan continuously tracks the latest high-water mark outside a drawdown.

User impact

Drawdown tables and downstream reports now show the true peak date and full peak-to-recovery duration. Maximum drawdown depth is unchanged.

Validation

  • Reproduced b24 behavior: Jan 2 peak, 2-day duration
  • Verified b25 behavior: Jan 1 peak, 3-day duration
  • uv run pytest tests/test_evaluation/test_portfolio_analysis.py -q - 70 passed
  • uv run pytest tests/ -q -n auto --timeout 120 - 5,287 passed, 21 skipped
  • uv run ruff check src/ tests/test_evaluation/test_portfolio_analysis.py
  • uv run ruff format --check src/ tests/test_evaluation/test_portfolio_analysis.py
  • uv run ty check
  • pre-commit run --all-files
  • Built wheel and sdist; installed-wheel smoke test passed for version and corrected drawdown output

Copilot AI review requested due to automatic review settings July 23, 2026 01:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@stefan-jansen
stefan-jansen merged commit 0388578 into main Jul 23, 2026
10 checks passed
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