fix(trading-strategies): call strategy init in BacktestExecutor to mirror TradingSession - #1189
Open
bennycode wants to merge 1 commit into
Open
fix(trading-strategies): call strategy init in BacktestExecutor to mirror TradingSession#1189bennycode wants to merge 1 commit into
bennycode wants to merge 1 commit into
Conversation
…rror TradingSession TradingSession.start() calls strategy.init() before the first candle, but BacktestExecutor never did — a strategy that warms up in init() ran warm in live trading and cold in backtests, silently diverging. The executor now inits the strategy before the candle loop. Its market view is fed exclusively from the new optional BacktestConfig.warmupCandles (data from before the backtest window), never from the exchange, so init cannot see the candles under test (lookahead-bias guard).
This was referenced Jul 12, 2026
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The backtest getRecentCandles stub currently ignores intervalInMillis, which can silently give strategies incorrect warmup data when they request non-1m history in init().
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR brings BacktestExecutor behavior in line with TradingSession by invoking strategy.init?.(market, pair) before processing the first candle, enabling strategies to warm up consistently in both live runs and backtests while avoiding lookahead bias via a dedicated warmup history input.
Changes:
- Call
strategy.init?.(...)at the start ofBacktestExecutor.execute()using a warmup-onlygetRecentCandlesimplementation. - Extend
BacktestConfigwith optionalwarmupCandlesto supply pre-window history toinit(). - Add tests asserting
initis called pre-loop and only sees warmup history.
File summaries
| File | Description |
|---|---|
| packages/trading-strategies/src/backtest/BacktestExecutor.ts | Adds a pre-loop strategy.init call using warmup-only candle history. |
| packages/trading-strategies/src/backtest/BacktestExecutor.test.ts | Adds coverage for init call timing and warmup isolation. |
| packages/trading-strategies/src/backtest/BacktestConfig.ts | Introduces warmupCandles?: Candle[] with documentation about lookahead bias. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+927
to
+938
| it('feeds init from warmupCandles only — never the candles under test (lookahead guard)', async () => { | ||
| const warmup = [createCandle({close: '95', open: '90', openTimeInISO: '2025-01-01T00:00:00.000Z'})]; | ||
| const strategy = new InitProbeStrategy(); | ||
| await new BacktestExecutor({ | ||
| broker: createMockExchange(), | ||
| candles: runCandles, | ||
| strategy, | ||
| tradingPair, | ||
| warmupCandles: warmup, | ||
| }).execute(); | ||
| expect(strategy.seenWarmupCandles).toEqual(warmup); | ||
| }); |
| */ | ||
| const warmupCandles = this.#config.warmupCandles ?? []; | ||
| await strategy.init?.( | ||
| {getRecentCandles: async (_pair, count) => (count > 0 ? warmupCandles.slice(-count) : [])}, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
TradingSession.start()callsstrategy.init?.(broker, pair)before the first candle —BacktestExecutornever did. A strategy that warms itself up ininit()(seeding indicators from history, loading datasets) runs warm live but cold in backtests, so backtest results silently misrepresent live behavior. Parity between the two executors is the backtester's core promise.Fix
BacktestExecutor.execute()now callsawait strategy.init?.(...)before the candle loop, mirroringTradingSession.BacktestConfig.warmupCandles(history from before the backtest window) — never from the exchange.initstructurally cannot see the candles it is about to be tested on.Impact
No existing strategy implements
init, so current backtest results are unchanged. Only strategies opting into the hook are affected.Test plan
initfires before the first candleinitsees exactly the configured warmup candles (and never the run candles)initsees empty history when no warmup is configured