Feat/template#26
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the flat lessons.md file with a SQLite-backed runs/lessons.db store, introduces equal_weight and momentum_rotation as reference strategies, and upgrades the init-strategy command to support templates and a non-interactive silent mode. Additionally, the upper bound on turnover_limit has been removed. Feedback on the changes highlights two key issues: first, the momentum_rotation strategy fails to filter for positive momentum as documented, which could lead to allocating capital to assets with negative returns; second, formatting the drawdown percentage using int(mdd * 100) in the template renderer can cause truncation errors due to floating-point precision, which should be resolved by using round() instead.
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.
|
|
||
| returns = hist.pct_change(trading_days).iloc[-1] | ||
| ranked = returns.dropna().sort_values(ascending=False) | ||
| selected = [t for t in ranked.index if t in available and t != cash_asset][:top_n] |
There was a problem hiding this comment.
The implementation does not filter for positive momentum, which contradicts the docstring stating: 'If no assets have positive momentum the full allocation goes to the cash asset.' Currently, it will allocate to the top assets even if they have negative returns. Filter the selected assets to ensure they have a positive return.
| selected = [t for t in ranked.index if t in available and t != cash_asset][:top_n] | |
| selected = [t for t in ranked.index if t in available and t != cash_asset and ranked[t] > 0][:top_n] |
| "__NAME__": strategy_name, | ||
| "__UNIVERSE__": universe_str, | ||
| "__BENCHMARK__": benchmark, | ||
| "__DRAWDOWN_PCT__": str(int(mdd * 100)), |
There was a problem hiding this comment.
Using int(mdd * 100) can lead to precision issues due to floating-point representation (for example, 0.29 * 100 evaluates to 28.999999999999996, which int() truncates to 28). Use round() to ensure correct rounding.
| "__DRAWDOWN_PCT__": str(int(mdd * 100)), | |
| "__DRAWDOWN_PCT__": str(round(mdd * 100)), |
…able and prevent infinite loops
Removes all filesystem I/O from the templates module, which was the root cause of FileNotFoundError in non-editable installs (CI Linux + Python 3.14.5). Strategy .py source and program .md are now stored as inline string constants; the _find_project_root / _PACKAGE_ROOT / _source_path machinery is fully removed.
No description provided.