Apply LMR after move 10

[bdmendes]

Summary by CodeRabbit

• Refactor
  • Optimized search algorithm performance through improved pruning conditions and move evaluation strategies, with no changes to public APIs.

[coderabbitai]

Walkthrough

Modified null-move pruning strategy in Searcher::pvs by adding positional-weight-based condition checks, implementing late-move reduction, and optimizing hash-move lookup through caching. The null-move recommendation logic was simplified by removing an explicit guard clause.

Changes

Cohort / File(s)

Summary

Null-Move Pruning & Move Picking src/search/mod.rs

Tightened null-move pruning condition with window.cuts_off(static_evaluation + MAX_POSITIONAL_WEIGHT) check. Cached hash move in local variable to avoid recomputation. Added late-move-based reduction (may_late_move_reduce) that applies additional depth reduction when i >= 10. Removed explicit !window.cuts_off(null_score) guard, now using window.improves(null_score) directly for null-move recommendation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Cut depth in half with NMP #412: Modifies null-move pruning and search depth conditions in Searcher::pvs with direct overlap in the same routine.
• Common NMP formula #413: Changes null-move pruning/reduction logic inside Searcher::pvs, directly affecting the same code patterns.
• Skip NMP when seen twice #419: Tightens null-move pruning conditions in Searcher::pvs by adding additional checks alongside the main PR's refinements.

Poem

🐰✨ Hops through the search tree with glee,
Pruning null moves with finesse so keen,
Late-move reductions dance at depth ten,
Hash moves cached, no work again!
Swift optimizations make victory true 🎯

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately reflects one of the key optimizations applied in the changeset: implementing late-move reduction (LMR) after move 10.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92%. Comparing base (be20971) to head (ca3178c).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files

@@          Coverage Diff          @@
##           master   #442   +/-   ##
=====================================
+ Coverage      92%    92%   +1%     
=====================================
  Files          33     33           
  Lines        3757   3760    +3     
  Branches     3757   3760    +3     
=====================================
+ Hits         3419   3422    +3     
  Misses        319    319           
  Partials       19     19           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Against master [hash=64]: 🆗 Elo: 39.43 +/- 17.59, nElo: 48.72 +/- 21.53

[github-actions]

Against v1.6.0 [hash=64]: ✅ Elo: 62.87 +/- 18.64, nElo: 74.28 +/- 21.53

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/search/mod.rs (1)

217-247: 🧹 Nitpick | 🔵 Trivial

LMR is unconditional on move kind — consider gating on mov.is_quiet().

may_late_move_reduce = !is_frontier_node && !interesting combined with i >= 10 applies the reduction uniformly to whatever MovePicker emits at that index, including captures (good or bad) and promotions. By move 10 the picker is usually past good captures into killers/quiets/bad-captures, so in practice this is mostly safe — but in high-mobility middlegames with many legal captures, a tactically significant capture can legitimately appear at index ≥ 10 and get reduced.

The conventional LMR formulation restricts the reduction to quiet, non-check, non-promotion moves, which is cheap to evaluate and removes the tactical risk entirely. Given the reduction is only 1 ply the risk here is modest, but this is worth either gating explicitly or validating with a tactical-suite run.

♻️ Suggested gating

-        let may_late_move_reduce = !is_frontier_node && !interesting;
+        let may_late_move_reduce = !is_frontier_node && !interesting;
@@
-                let reduction = if may_late_move_reduce && i >= 10 { 1 } else { 0 };
+                let reduction = if may_late_move_reduce && i >= 10 && mov.is_quiet() {
+                    1
+                } else {
+                    0
+                };

Note: the parallel change at line 243 — dropping !window.cuts_off(null_score) and re-searching whenever improves(null_score) — is correct and in fact required by LMR: a reduced-depth null-window fail-high must be verified at full depth before being trusted, so the old short-circuit would have been unsound here.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 896ce659-a1ef-4981-bdcb-f0113592a4eb

📥 Commits

Reviewing files that changed from the base of the PR and between be20971 and ca3178c.

📒 Files selected for processing (1)

• src/search/mod.rs