Add deterministic historical backtesting engine with strict no-lookahead guarantees (BACKTEST-001) - #62
Conversation
…ead guarantees (BACKTEST-001) Issue #38 (P0): no backtesting engine existed at all -- signals must not be marketed as reliable without verified backtesting, and lookahead bias is the classic, easy-to-introduce-silently bug class this issue calls out as the highest correctness risk in the whole backlog. - backtest/engine.ts: pure, DB-free simulation core. At loop index i, only evaluates evaluateSignal() (reused unmodified from technical-analysis.ts) against closes[0..i] -- never anything beyond it. A fired signal enters at candle i+1's open (never the signal candle's own close), with stop/take-profit distances taken from evaluateSignal's ATR-based levels but re-anchored to the real fill price. Same-candle stop/target collision resolves conservatively as a stop-loss. A trade that can't be resolved because the dataset itself runs out is excluded (skippedSignalCount), never fabricated as a result. - engine.test.ts: 16 tests, including a constructed lookahead-bait regression (two datasets identical up to a shared prefix, diverging wildly afterward -- the entry decision must be identical between them, proving it depends only on the past) plus stop-loss/take-profit/same- candle-collision/time-exit/dataset-truncation/gap-detection/fees/ slippage/funding/determinism coverage. - backtest/summary.ts: aggregates trades into the full metrics contract, reusing DATA-015's exact 10/30-trade sample-adequacy tiers rather than inventing a new threshold, and analytics/metrics.ts's "don't call it Sharpe" discipline for expectancy/profit-factor naming. - backtest/dataset.ts: fetches historical candles/funding directly from Hyperliquid for the exact requested range (the live ingestion cycle only keeps a short rolling window, nowhere near enough depth for an arbitrary backtest range), enforcing a hard per-symbol candle cap so the engine isn't unbounded-by-design even before SEC-HARDEN-001 formalizes limits. - backtest/runner.ts + router.ts: orchestration and API (POST/GET /api/backtests, GET /api/backtests/:id), with the same ownership-check discipline execution/router.ts already established. - New backtest_runs/backtest_trades tables (nullable summary until complete; a run is always finalized to COMPLETED or FAILED, never left stuck, including when candle-fetching itself fails before a run row could even be created). - docs/product/backtesting-methodology.md documents every assumption above explicitly, per the issue's own "conservative, documented assumption" requirement. Verified end-to-end against live Hyperliquid mainnet, not just unit tests: ran a real 30-day BTC/1h backtest through the actual API (60 real trades, full-tier summary with realistic win-rate/drawdown/by-signal-strength figures), re-ran the identical request and confirmed a byte-identical summary and dataset hash (real determinism, not just asserted in a test), verified the insufficient-history path returns a real FAILED run with a clear reason, and verified cross-user ownership is enforced (a second guest session gets a real 403 against the first guest's run). Full suite: 237/237 passing (201 baseline + 36 new). Build clean. Client results UI is deliberately out of scope for this issue (per its own Accessibility Review section, permitting separate scoping) -- the engine, API, and persistence are complete and independently reviewable.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec42a8eb6f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (hitStop) { | ||
| // Conservative same-candle tie-break: stop resolves first even if the target was also touched this candle. | ||
| exitIndex = j; | ||
| exitPrice = stopLoss; |
There was a problem hiding this comment.
When slippageBps is nonzero—including the default 5 bps—only the entry price receives slippage; stop-loss, take-profit, and time-based exits are recorded at their raw levels or candle close. This omits an entire adverse execution leg from every trade and systematically overstates P&L, contrary to the request contract stating that slippage applies at both entry and exit. Apply slippage using the opposite side before calculating and persisting each exit fill.
Useful? React with 👍 / 👎.
| if (hitStop) { | ||
| // Conservative same-candle tie-break: stop resolves first even if the target was also touched this candle. | ||
| exitIndex = j; | ||
| exitPrice = stopLoss; | ||
| exitReason = 'stop-loss'; |
There was a problem hiding this comment.
When a later candle opens beyond the stop level, this branch still grants a fill at stopLoss; for example, a long with a stop at 100 whose next candle opens at 90 is reported as exiting at 100. This creates a price that was unavailable after the gap and materially understates losses, especially in datasets already allowed to contain candle gaps. Check the candle open against the stop before the high/low test and use that available open-side price, with exit slippage.
Useful? React with 👍 / 👎.
| export type BacktestStatus = z.infer<typeof BacktestStatusSchema>; | ||
|
|
||
| export const CreateBacktestRequestSchema = z.object({ | ||
| symbols: z.array(z.string().trim().min(1).max(10).toUpperCase()).min(1).max(MAX_BACKTEST_SYMBOLS), |
There was a problem hiding this comment.
A request such as symbols: ["BTC", "BTC"] passes this schema. The runner subsequently fetches and simulates each array element and concatenates both identical trade sets, doubling the trade count, P&L, and diagnostic counters and potentially promoting the summary into a higher sample-adequacy tier. Enforce uniqueness after normalization so each asset can contribute only once.
Useful? React with 👍 / 👎.
| const hash = createHash('sha256'); | ||
| for (const symbol of Object.keys(candlesBySymbol).sort()) { | ||
| for (const candle of candlesBySymbol[symbol]) { | ||
| hash.update(`${symbol}|${candle.openTime.toISOString()}|${candle.close}`); |
There was a problem hiding this comment.
The engine uses candle opens for entries and highs/lows for stop and target resolution, but datasetVersion hashes only openTime and close. Consequently, revising a high enough to hit a target—or revising an entry open—can produce different trades and summaries under the same dataset identifier, defeating the persisted run's reproducibility and audit purpose. Include at least all OHLC fields consumed by the engine in the hash.
Useful? React with 👍 / 👎.
Independent review: PASS WITH WARNINGSLOOKAHEAD-BIAS SIGN-OFF: CONFIRMED SAFE. The reviewer independently read Independently reproduced, not re-trusted from the PR description:
Findings (none Critical/High), tracked as fast-follow work:
Ready for merge at your convenience; the two Medium findings are good candidates for a small fast-follow. |
There was a problem hiding this comment.
Pull request overview
Adds a new server-side deterministic historical backtesting capability (BACKTEST-001) to replay existing signal logic against fixed historical Hyperliquid candle datasets with strict no-lookahead rules, persist runs/trades, and expose an authenticated API for creating and inspecting backtests.
Changes:
- Introduces a pure backtest simulation core (
backtest/engine.ts) plus aggregation (backtest/summary.ts) and orchestration/persistence (backtest/runner.ts), with extensive unit tests. - Adds backtest API endpoints (
/api/backtests) with ownership checks and new DB tables/migrations to store runs and trades. - Adds dataset fetching/caching from Hyperliquid (
backtest/dataset.ts) with hard per-run caps, plus methodology/architecture docs; exportsMIN_HISTORYfor shared validation.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/technical-analysis.ts | Exports MIN_HISTORY so backtest can validate data sufficiency consistently. |
| server/src/server.ts | Mounts the new /api/backtests router. |
| server/src/schemas/backtest.ts | Defines backtest request/config/trade/summary schemas and hard caps. |
| server/src/db/schema.ts | Adds backtest_runs / backtest_trades tables and status enum. |
| server/src/backtest/summary.ts | Pure trade-to-summary aggregation with DATA-015 tier thresholds. |
| server/src/backtest/summary.test.ts | Unit tests for summary tiering and metric behavior. |
| server/src/backtest/runner.ts | Orchestrates fetch → engine → summary → persistence; finalizes runs. |
| server/src/backtest/runner.test.ts | Unit tests around runner lifecycle and failure/completion paths. |
| server/src/backtest/router.ts | Adds authenticated create/list/get endpoints with ownership checks. |
| server/src/backtest/engine.ts | Implements no-lookahead per-symbol simulation core with exits/costs. |
| server/src/backtest/engine.test.ts | Tests no-lookahead regression and exit/cost/determinism behaviors. |
| server/src/backtest/dataset.ts | Fetches candles/funding from Hyperliquid, caches candles, hashes dataset. |
| server/src/backtest/dataset.test.ts | Unit tests for candle filtering/sorting/caps, funding fallback, hashing. |
| server/drizzle/meta/0009_snapshot.json | Drizzle snapshot update for new schema objects. |
| server/drizzle/meta/_journal.json | Drizzle journal entry for the new migration. |
| server/drizzle/0009_thick_bushwacker.sql | SQL migration creating backtest tables and enum + indexes/FKs. |
| docs/product/backtesting-methodology.md | Documents assumptions and invariants for the backtest engine. |
| docs/architecture/current-state.md | Documents new /api/backtests router and behavior in architecture notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const exitCandle = candles[exitIndex]; | ||
| const notional = riskPerTradeNotional * leverage; | ||
| const quantity = notional / entryPrice; | ||
| const grossPnl = side === 'LONG' ? (exitPrice - entryPrice) * quantity : (entryPrice - exitPrice) * quantity; | ||
| const feesPaid = notional * (feeBps / 10_000); |
| const hash = createHash('sha256'); | ||
| for (const symbol of Object.keys(candlesBySymbol).sort()) { | ||
| for (const candle of candlesBySymbol[symbol]) { | ||
| hash.update(`${symbol}|${candle.openTime.toISOString()}|${candle.close}`); | ||
| } | ||
| } |
| for (const candle of closedOnly) { | ||
| await db | ||
| .insert(candles) | ||
| .values({ | ||
| venue: candle.venue, |
Summary
Closes #38 (BACKTEST-001, P0). No backtesting engine existed at all -- the mission explicitly requires signals not be marketed as reliable without verified backtesting, and lookahead bias is the classic, easy-to-introduce-silently bug class this issue calls out as the highest correctness risk in the whole backlog.
backtest/engine.ts-- pure, DB-free simulation core. At loop indexi, only evaluatesevaluateSignal()(reused unmodified fromtechnical-analysis.ts) againstcloses[0..i]-- never anything beyond it. A fired signal enters at candlei+1's open (never the signal candle's own close), with stop/take-profit distances taken fromevaluateSignal's ATR-based levels but re-anchored to the real fill price. Same-candle stop/target collision resolves conservatively as a stop-loss. A trade that can't be resolved because the dataset itself runs out is excluded (skippedSignalCount), never fabricated as a result.engine.test.ts-- 16 tests, including a constructed lookahead-bait regression: two datasets identical up to a shared prefix, diverging wildly afterward. The entry decision (whether a trade opens, its side, its entry time/price) is asserted identical between the two runs, proving it depends only on the past. Plus stop-loss/take-profit/same-candle-collision/time-exit/dataset-truncation/gap-detection/fees/slippage/funding/determinism coverage.backtest/summary.ts-- aggregates trades into the full metrics contract, reusing DATA-015's exact 10/30-trade sample-adequacy tiers rather than inventing a new threshold, andanalytics/metrics.ts's "don't call it Sharpe" discipline for expectancy/profit-factor naming.backtest/dataset.ts-- fetches historical candles/funding directly from Hyperliquid for the exact requested range (the live ingestion cycle only keeps a short rolling window, nowhere near enough depth for an arbitrary backtest range), enforcing a hard per-symbol candle cap so the engine isn't unbounded-by-design even beforeSEC-HARDEN-001formalizes limits.backtest/runner.ts+router.ts-- orchestration and API (POST/GET /api/backtests,GET /api/backtests/:id), with the same ownership-check disciplineexecution/router.tsalready established.backtest_runs/backtest_tradestables (nullable summary until complete; a run is always finalized toCOMPLETEDorFAILED, never left stuck -- including when candle-fetching itself fails before a run row could even be created).docs/product/backtesting-methodology.mddocuments every assumption explicitly, per the issue's own "conservative, documented assumption" requirement.Verification
FAILEDrun with a clear reason (HTTP 422), and verified cross-user ownership is enforced (a second guest session gets a real 403 against the first guest's run).git diff --statvsgit diff -w --stat-- identical).Scope note
Client results UI is deliberately out of scope for this issue, per its own Accessibility Review section ("scope that separately if it grows large") -- the engine, API, and persistence are complete and independently reviewable on their own. Will file a follow-up issue for the UI once this lands.
Test plan