Skip to content

Some search comment tweaks (grammar, redundancy, clarification) - #6942

Open
dubslow wants to merge 2 commits into
official-stockfish:masterfrom
dubslow:cleanups-2026-06-B-3
Open

Some search comment tweaks (grammar, redundancy, clarification)#6942
dubslow wants to merge 2 commits into
official-stockfish:masterfrom
dubslow:cleanups-2026-06-B-3

Conversation

@dubslow

@dubslow dubslow commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

(This can be squashed into the bigger cleanup commit)

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

clang-format 20 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/plucky/clang-format-20.

(execution 28502694626 / attempt 1)

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 144235d2-29a8-406d-a671-1cc8ece5ee38

📥 Commits

Reviewing files that changed from the base of the PR and between 76de928 and 6d6e48e.

📒 Files selected for processing (3)
  • src/search.cpp
  • src/search.h
  • src/thread.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/thread.cpp
  • src/search.h
  • src/search.cpp

📝 Walkthrough

Walkthrough

This pull request changes Search::RootMove score-state bookkeeping from bound flags to inexact flags, and updates root rollback, PV reporting, and decisive-thread selection to use the new helpers and fields. It also renumbers and reformats internal step comments in search and qsearch, including final assertion formatting. No exported or public entity declarations were added.

Changes

Cohort / File(s) Summary
RootMove inexact-flag migration and search comments src/search.h, src/search.cpp, src/thread.cpp
RootMove flag API Replaces bound-flag helpers and fields with inexact-flag helpers and fields; makes operator< constexpr
RootMove flag usage Updates rollback, PV output, and thread selection to use inexact flags
Search step comments Renumbers internal step comments in search and qsearch, with assertion formatting updates

Sequence Diagram(s)

sequenceDiagram
  participant SearchWorker
  participant SearchManager
  participant ThreadPool
  participant RootMove
  SearchWorker->>RootMove: set inexactLower / inexactUpper during rollback
  SearchManager->>RootMove: read is_inexact() for PV reliability and bound output
  ThreadPool->>RootMove: read is_inexact() for decisive-move filtering
Loading

Suggested labels: refactor, search

Suggested reviewers: None identified.


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

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

clang-format 20 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/plucky/clang-format-20.

(execution 28544756141 / attempt 1)

@dubslow

dubslow commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

I've added a second commit unrelated to the first. Like the first, it is completely optional and (if accepted) can (should?) be squashed into the final cleanup commit

@dubslow
dubslow force-pushed the cleanups-2026-06-B-3 branch from 76de928 to 6d6e48e Compare July 1, 2026 20:12
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

clang-format 20 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/plucky/clang-format-20.

(execution 28544839072 / attempt 1)

@MinetaS

MinetaS commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Looks mostly ok (talking about the first commit only) but

  • the last steps in search() and qsearch() look redundant
  • I'm really against adding more comments especially at singular extensions, in fact the existing comment already feels excessive
  • I personally don't prefer having explicit "step 0"

dubslow added 2 commits July 28, 2026 20:41
Namely, in search we use `enum Bound`, and every score is by defintion a bound,
it just may be a one-sided bound or two-sided bound. So the RootMove flags track
inexactness, not the kind of bounds. Rename them accordingly

Also constexpr and an assert

no functional change
@dubslow
dubslow force-pushed the cleanups-2026-06-B-3 branch from 6d6e48e to 054f619 Compare July 29, 2026 01:47
@github-actions

Copy link
Copy Markdown

clang-format 20 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/questing/clang-format-20.
An easier way to install it might be through https://apt.llvm.org/#llvmsh.

(execution 30415064420 / attempt 1)

@dubslow

dubslow commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Rebased, addressed feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants