Code review checklist
Lesson 29 · about 12 min
Before a system trades a dollar, someone should read it with the specific intent of finding the ways it is wrong. Ideally that is another person; if it is you, do it a day later and with this list in hand. Each item below is a failure mode covered in the course, phrased as a question with a concrete check. Work through them in order and write the answers down; a review without a record is a feeling, not a review.
Data
- Is every timestamp timezone-aware and in UTC?
assert bars.index.tz is not None. The store refuses naive indexes; confirm nothing bypasses the store. - Is the index sorted and unique?
validate_barschecks it; confirm it is called on every load, including inside the liveget_bars. - What does the bar's timestamp mean? Open time or close time, and is it the same for every source in use? Write the answer as a comment in the adapter.
- Are prices adjusted, and how? State it. If signals use price levels (breakouts), adjustment changes them.
- Does the data report look sane? Gaps, zero-volume bars, extreme returns, NaNs. Print it in the review.
- Is the universe survivorship-biased? If the symbols were chosen because they exist today, say so in the report.
Look-ahead
- Every
shiftis positive?grep -n "shift(-" src/should return only analysis code, never a signal function. - No
center=True, nobfill, nointerpolatein signal code? Same grep. - No whole-sample statistics in features? Any
.mean()or.std()without arollingorexpandingin front, inside a signal, is a leak. - Does
check_no_lookaheadpass for every signal function? It should be a test intests/. - Is the execution lag right? Close signal with close fill is
shift(1)on close-to-close returns; close signal with open fill isshift(2)on open-to-open returns. Confirm which pair the code uses, and that it is the pair the live program implements.
Costs and execution
- Is turnover charged? Find the line. If
cost_bpsis zero inconfig.py, the review stops here. - What is the result at double the cost? It must be in the report.
- Does the vectorized result agree with the event-driven result on a strategy both can express? Within a few percent. If not, one of them is wrong.
- Are stops modelled by the event engine or only implied by sizing? Say which.
- Same-bar ambiguity: is the stop checked before the target? Confirm the priority order in
_execute.
Sizing and risk
- Is position size derived from a risk budget and a stop distance, or from conviction? It must be the former, with the cap from the risk plan.
- Is the cap on total exposure enforced in the live path, not just the backtest? The paper broker rejects unaffordable orders; does the real adapter?
- Are quantities rounded down to whole lots, with zero meaning skip? Check
to_units. - Does the daily loss limit match the written risk plan? Same number in
config.pyand in the plan document.
Search and evaluation
- How many trials? The number from the trials log goes in the report. If there is no log, the answer is "unknown", which is the worst answer.
- Were parameters chosen by neighbourhood, not by peak? Read
choose. - Were window lengths and the rolling/anchored choice fixed before results were seen? They should be constants with a date in a comment.
- Is the headline number out-of-sample? In-sample numbers may appear only beside their OOS counterparts.
- Is the OOS result driven by one window? Look at the per-window log.
- Trade count and t-stat? Fewer than 30 OOS trades or a t-stat under 2 means "insufficient evidence", which must be written as such.
- Bootstrap drawdown percentile against the risk plan? The 5th percentile must be inside what the plan says you would survive.
Live path
- Is the strategy code shared between backtest and live, or copied? Copied code diverges. The live
desired_qtymust call the same signal function the backtest used. - Does the run happen after the bar closes, in the market's timezone? Read the cron line.
- Is every order submitted with a deterministic client order id? Test: submit twice, expect one
duplicate. - Is state written atomically? Temp file and rename.
- Does the run reconcile before trading and halt on mismatch? Test it with a hand-edited position.
- Are all three kill switches present and tested? HALT file, daily loss, failure counter.
- Is there a heartbeat checked by an independent job? Two cron entries, not one.
- Are credentials in the environment and nowhere in the repository?
git log -p | grep -i secretshould return nothing. If it ever did, the key is compromised and must be rotated. - Are paper and live endpoints different constants that cannot be confused? A single
LIVE = Falseflag that switches host and keys together. - Does every network call have a timeout, and does every failure path log and alert? Read the adapter.
Process
- Can a stranger reproduce the headline number?
requirements.txt, the seed, the stored data file, and one command. Try it in a fresh venv. - Is the report complete per the Module 8 standard? Eight items. Tick them.
- What is the plan if the first week live loses more than the backtest's worst week? If the answer is "see what happens", the review fails.
Key idea: A review is a list of specific questions with recorded answers, not a read-through. Every question above maps to a failure the course has shown you how to produce; the review is where you prove you did not.
Using the list
Copy it into REVIEW.md. For each item write PASS, FAIL or N/A with one line of evidence (the grep output, the test name, the number), and commit the file. A system goes to paper trading with zero FAIL entries and to real money after a month of paper fills that matched the backtest.
Try it: Run the checklist against the project script from lesson 1 as it stands. Several items will fail (no trials log, no event-engine cross-check, no live path yet). Record them honestly, fix the two cheapest, and re-run.
Recap
- Review data, look-ahead, costs, sizing, search, live path and process as separate sections with concrete checks.
- Record
PASS/FAIL/N/Awith evidence in a committedREVIEW.md. - Shared strategy code between backtest and live; deterministic order ids; atomic state; reconcile; three kill switches; independent heartbeat.
- The headline number must be out-of-sample, reproducible by a stranger, and accompanied by the trials count.
- Zero failures before paper trading; a month of matching paper fills before real money.
See it drawn
Original diagrams for the ideas on this page. Illustrative, not real market data.