Fix/strategy optimization#24
Conversation
…deoff and floor constraints
There was a problem hiding this comment.
Code Review
This pull request introduces a hybrid gating mechanism that allows a trade-off between target metric degradation and annualized return improvement, configured via metric_return_tradeoff and metric_floor. The changes span configuration schemas, gate evaluation logic, orchestrator warnings, and unit tests. The review feedback highlights two important issues: first, the absolute metric_floor constraint is skipped when no baseline is present because it is nested inside the baseline-dependent check; second, there is a naming and description mismatch in the API reference documentation compared to the actual implementation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if baseline is not None: | ||
| result = _check_metric_improvement(report, baseline, target_metric, min_improvement) | ||
| result = _check_metric_improvement( | ||
| report, baseline, target_metric, min_improvement, tradeoff_coeff, metric_floor | ||
| ) | ||
| if not result.accepted: | ||
| return result |
There was a problem hiding this comment.
The absolute metric_floor constraint is currently only checked inside _check_metric_improvement, which is skipped entirely if baseline is None. This means candidates will not be checked against the absolute target-metric floor when evaluated without a baseline. The absolute floor check should be performed independently of the baseline presence.
| if baseline is not None: | |
| result = _check_metric_improvement(report, baseline, target_metric, min_improvement) | |
| result = _check_metric_improvement( | |
| report, baseline, target_metric, min_improvement, tradeoff_coeff, metric_floor | |
| ) | |
| if not result.accepted: | |
| return result | |
| if metric_floor is not None: | |
| candidate_val = _get_in_sample_metric_val(report, target_metric) | |
| if math.isnan(candidate_val) or candidate_val <= metric_floor: | |
| msg = ( | |
| f"Candidate {target_metric.value} ({candidate_val:.4f}) is below the" | |
| f" required floor of {metric_floor:.4f}." | |
| ) | |
| report.is_accepted = False | |
| report.rejection_reason = msg | |
| return GateResult(accepted=False, reason=msg, failed_gate="metric_floor") | |
| if baseline is not None: | |
| result = _check_metric_improvement( | |
| report, baseline, target_metric, min_improvement, tradeoff_coeff, metric_floor | |
| ) | |
| if not result.accepted: | |
| return result |
| | `sharpe_return_tradeoff` | `float` | `0.0` | Acceptable target metric reduction per 100% (1.0) increase in annualized return; `0.0` disables trade-off | | ||
| | `min_metric_floor` | `float\|None` | `None` | Absolute target metric floor below which candidates are always rejected regardless of trade-off; `None` disables | |
There was a problem hiding this comment.
The parameter names and descriptions in the API reference do not match the actual implementation in StrategyConfig (defined in src/autobacktest/strategy/config_schema.py). The fields are implemented as metric_return_tradeoff and metric_floor (not sharpe_return_tradeoff and min_metric_floor), and the tradeoff scaling is per 1pp (0.01) increase in annualized return rather than per 100% (1.0) increase.
| | `sharpe_return_tradeoff` | `float` | `0.0` | Acceptable target metric reduction per 100% (1.0) increase in annualized return; `0.0` disables trade-off | | |
| | `min_metric_floor` | `float\|None` | `None` | Absolute target metric floor below which candidates are always rejected regardless of trade-off; `None` disables | | |
| | `metric_return_tradeoff` | `float` | `0.0` | Metric reduction tolerated per 1pp (0.01) increase in annualized return; `0.0` disables | | |
| | `metric_floor` | `float\|None` | `None` | Absolute target-metric floor below which candidates are always rejected regardless of trade-off; `None` disables | |
… consistency in strategy evaluation
No description provided.