ship/move ordering texel#6182
Conversation
matthewevans
commented
Jul 18, 2026
matthewevans
commented
- feat(ai): killer-move and PV move ordering for iterative deepening
- feat(ai): Texel-style self-play eval harvest pipeline
Two-slot per-ply killer moves recorded at beta cutoffs and reordered to the front of the truncated beam (reorder-only; candidate scores are never mutated), principal-variation rotation into the next rung gated to iter_depth >= 1, and RungStat/beta_cutoff/killer_ordering witnesses on PlannerServices. The iterative-deepening loop is extracted into run_iterative_deepening with a NaN-safe deterministic pv_argmax. Root siblings share one per-rung SearchBudget, so PV-first ordering spends the shared pool on the strongest known candidate before the tail starves. Includes a serve-reconstruction identity test for the eval feature extractor (rides here because it lives in planner tests).
EvalFeatures single-authority extractor in eval.rs (breakdown is reproduced as unweighted features x weights, energy offset applied post-weighting exactly as before), a drive_game_observed observer seam in the duel suite with stack-empty-gated last-snapshot-per-turn harvesting, one HarvestSink per suite run writing JSONL with a single file-scoped meta line, an ai-duel --harvest flag that forces the sequential branch, and a --source selfplay path in train_eval_weights.py (17Lands path untouched) with --allow-tiny-corpus smoke thresholds. Harvest runs are byte-deterministic for the same (matchup, seed, K); panicked games label as winner-None and are dropped. Retraining awaits a real harvest campaign - shipping weights are unchanged.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements self-play eval-feature harvesting for the Texel retrain pipeline, refactors tactical board evaluation to extract unweighted features, and introduces killer-move heuristics and iterative deepening PV threading to optimize search depth. The review feedback highlights a high-severity performance bottleneck in the search hot path due to heap allocations from sort_by_key in order_killers_first, suggesting an in-place stable insertion sort instead. Additionally, a medium-severity issue was identified in the training script where train_test_split can crash on small datasets under --allow-tiny-corpus if stratification is applied to classes with fewer than two members.
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.
| if ranked.iter().any(|r| rank(r) < 2) { | ||
| ranked.sort_by_key(rank); | ||
| self.killer_orderings += 1; | ||
| } |
There was a problem hiding this comment.
[HIGH] Avoid heap allocations on the search hot path. Evidence: crates/phase-ai/src/planner/mod.rs:819-822. Why it matters: Calling sort_by_key on the search tree's hot path allocates heap memory at every node, causing a significant performance bottleneck. Suggested fix: Use a stable in-place insertion sort for the small candidate array.
| if ranked.iter().any(|r| rank(r) < 2) { | |
| ranked.sort_by_key(rank); | |
| self.killer_orderings += 1; | |
| } | |
| if ranked.iter().any(|r| rank(r) < 2) { | |
| for i in 1..ranked.len() { | |
| let mut j = i; | |
| while j > 0 && rank(&ranked[j]) < rank(&ranked[j - 1]) { | |
| ranked.swap(j, j - 1); | |
| j -= 1; | |
| } | |
| } | |
| self.killer_orderings += 1; | |
| } |
References
- Idiomatic Rust — uses the type system, ownership model, and standard library idioms to their fullest. (link)
There was a problem hiding this comment.
Refuted. The sorted slice is the ranked beam, which is capped at max_branching: 5 in every preset (config.rs — all five presets set 5), and Rust's stable sort_by_key uses its allocation-free insertion-sort path for slices this short; the merge-buffer allocation only engages for long slices. The sort is additionally gated behind ranked.iter().any(|r| rank(r) < 2), so it runs only on the minority of nodes where a killer is actually present in the beam. No hot-path allocation occurs at these lengths.
| # train_test_split(stratify=y) requires at least two classes; a single-class | ||
| # phase (all wins or all losses) is unlabelable — skip it rather than crash. | ||
| if len(np.unique(y)) < 2: | ||
| print( | ||
| f" WARNING: {phase_name} has a single outcome class — skipping " | ||
| "(cannot stratify).", | ||
| file=sys.stderr, | ||
| ) | ||
| return None | ||
|
|
||
| X_train, X_test, y_train, y_test = train_test_split( | ||
| X, y, test_size=0.2, random_state=42, stratify=y | ||
| ) |
There was a problem hiding this comment.
[MEDIUM] Prevent crash in train_test_split on small datasets. Evidence: scripts/train_eval_weights.py:531-543. Why it matters: train_test_split with stratify=y raises a ValueError if any class has fewer than 2 members, which will crash the script when running with --allow-tiny-corpus. Suggested fix: Conditionally apply stratification only when all classes have at least 2 members.
| # train_test_split(stratify=y) requires at least two classes; a single-class | |
| # phase (all wins or all losses) is unlabelable — skip it rather than crash. | |
| if len(np.unique(y)) < 2: | |
| print( | |
| f" WARNING: {phase_name} has a single outcome class — skipping " | |
| "(cannot stratify).", | |
| file=sys.stderr, | |
| ) | |
| return None | |
| X_train, X_test, y_train, y_test = train_test_split( | |
| X, y, test_size=0.2, random_state=42, stratify=y | |
| ) | |
| unique, counts = np.unique(y, return_counts=True) | |
| if len(unique) < 2: | |
| print( | |
| f" WARNING: {phase_name} has a single outcome class — skipping.", | |
| file=sys.stderr, | |
| ) | |
| return None | |
| # stratify=y requires at least 2 members per class in train_test_split | |
| stratify = y if all(c >= 2 for c in counts) else None | |
| X_train, X_test, y_train, y_test = train_test_split( | |
| X, y, test_size=0.2, random_state=42, stratify=stratify | |
| ) |
There was a problem hiding this comment.
Confirmed — good catch. The single-class skip didn't cover a two-class phase where the least populated class has exactly 1 member, which train_test_split(stratify=y) rejects and which is reachable under --allow-tiny-corpus (per-phase floor is 2). Fixed in 91c5b93: the guard now requires two classes with >=2 members each before splitting, and skips with a warning otherwise.
train_test_split(stratify=y) raises ValueError when the least populated class has one member - reachable under --allow-tiny-corpus, whose per-phase floor is 2. Extends the existing single-class skip to require two classes with >=2 members each. Raised by Gemini review on #6182.
FeatureRow and weighted_total become pub — the JSONL row is the external trainer contract (scripts/train_eval_weights.py), so public API is the honest dead-code fix. search_value gets the workspace-precedented too_many_arguments allow for its 8-arg alpha-beta signature.
1 participant
