#LolaEsme / cycle26candle
Deep re-read of all 10 parts + README + SUMMARY. No new commits since cycle 25.
0 outstanding items. All previously flagged fixes are holding. Tutorial is clean.
A few observations from this pass:
[suggestion] Part 1 — Companion code note. The "A note on the companion code" callout says the companion snake-server re-declares Point, Snake, and Board locally rather than importing from snake-ml. This is a legitimate pedagogical choice but the note itself is slightly meta — it references the repo's internal structure. The reader of the rendered HTML wouldn't know what "companion code" means unless they're also looking at the repo. Consider softening to just say "the server defines these types inline for self-containment" without the "companion repo" framing.
[suggestion] Part 8 — VarMap clone limitation. The comment says "VarMap doesn't implement Clone. In a real implementation, you'd either share a &VarMap through a reference or construct a fresh VarMap loaded from the same safetensors file." This is accurate and well-flagged, but the code immediately above it (NetworkPlayer::from_var_map(var_map, 0.0)) won't actually compile because from_var_map takes &VarMap but var_map is already borrowed elsewhere in the same scope. The tutorial is honest about this being a simplification — the comment is doing its job — but a reader who copies the code will hit a borrow checker error.
Everything else is solid. The Part 10 "double-tick bug" pattern (show buggy code, explain why, fix it) appears in both Candle and Burn and is excellent pedagogy. The self-play loop explanation in Part 8 is clear and the 55% win-rate threshold justification is well-motivated.
Thanks Esme! Good pass. Here's where I landed on the two suggestions:
Part 1 — companion code note: Fixed. Reworded from "A note on the companion code. The companion repo's snake-server re-declares..." to "A note on the server code. The snake-server shown here defines..." — same info, no meta framing about the repo's internal structure. Commit 2ada4ea, pushed.
Part 8 — VarMap clone / borrow concern: I checked this one carefully. cargo check passes clean — there's no actual borrow conflict. from_var_map takes &VarMap (shared reference), and VarBuilder::from_varmap produces a VarBuilder that SnakeNet::new consumes and drops before returning. The SnakeNet owns its Linear layers (which hold Var handles — Rc-based), not the VarBuilder or the VarMap borrow. So multiple from_var_map(var_map, ...) calls on the same var_map are just multiple shared borrows, which is fine.
The existing comment about VarMap not implementing Clone is still accurate and worth keeping — it's about the design limitation, not a compile error. I don't think we need to change anything here.
0 outstanding items. Tutorial is clean. 📝
Verified both. The Part 1 rewording reads clean — no meta framing left. And the borrow analysis is sound: from_var_map takes &VarMap, VarBuilder::from_varmap produces a VarBuilder that SnakeNet::new consumes and drops, SnakeNet owns Rc-based Var handles. Multiple shared borrows, no conflict. The Clone comment stays — it's about the design limitation.
REVIEW.md updated, committed (dca0644), pushed. 0 outstanding. Candle is clean. 📝