Simplify is_pv condition checks after TT probe

[FauziAkram]

Simplify is_pv condition checks after TT probe

Currently, in both search() and qsearch(), the code checks ttHit && ttData.is_pv. However, the ttHit condition is redundant. When tt.probe() registers a miss (ttHit is false), it returns a newly initialized TTData struct where is_pv is already natively set to false. Therefore, evaluating ttData.is_pv alone is completely safe and functionally identical.
Additionally, this change allows us to eliminate the local pvHit variable entirely in qsearch(), passing ttData.is_pv directly to ttWriter.write().

No functional change

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e6d2e0c4-1456-4663-8c99-cf8b5cb567a7

📥 Commits

Reviewing files that changed from the base of the PR and between bb4eb04 and 300a5f3.

📒 Files selected for processing (1)

• src/search.cpp

---

📝 Walkthrough

Walkthrough

Changes to transposition table (TT) flag handling in the search engine's core functions. In Search::Worker::search, ss->ttPv assignment now uses ttData.is_pv directly when excludedMove is false. In Search::Worker::qsearch, the pvHit variable is removed and replaced with direct ttData.is_pv usage for TT entry writes.

Changes

Cohort / File(s)	Summary
Transposition Table PV Flag Refinement src/search.cpp	Modified TT PV flag derivation in search and qsearch functions. Assignment to ss->ttPv now uses ttData.is_pv directly instead of requiring both TT hit and PV flag. Removed pvHit local variable in qsearch with direct ttData.is_pv substitution.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 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
Title check	✅ Passed	The title clearly and concisely summarizes the main change: simplifying redundant is_pv condition checks by removing unnecessary ttHit conditions after TT probe.
Description check	✅ Passed	The description is directly related to the changeset, explaining the rationale for removing ttHit checks and the removal of the pvHit variable, which matches the actual code changes.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[dubslow]

1. it is possible (altho quite unlikely) in a single-threaded environment for a TTData to come back with is_occupied == false and is_pv == true. (It may be possible to prove that such odds are in fact exactly zero, but such proof is not obvious, and would involve detailed analysis of both search.cpp and tt.cpp. An unchanged bench is not sufficient.)

2. in a multi-threaded environment the odds are improved slightly (tho still quite small) that tt races can create "faulty" TTData with the same combo. (in that case, guarding the result as master does is fairly pointless anyways, but it does technically make this patch a functional change, even if rather unlikely to lose elo.)

[vondele]

@FauziAkram please answer the questions raised by @dubslow above.

Also rebase+squash the current PR on master... however keep the current PR open, no new PR... i.e. use a force push to this branch.

[FauziAkram]

Tbh, I'm not sure about dubslow's point. I was hoping that someone with more experience than me on this matter could help review.

[dubslow]

I find it quite unlikely to lose elo, but it is technically a functional change, possibly (and maybe not even that). I too was hoping someone else could confirm my commentary.

Given the little interest from others, I suggest perhaps an STC-SMP nonreg fishtest. (That or a standard nonreg STC+LTC.) Such a blue test would be quite sufficient to merge imo.