Skip to content

Doxygen documentation#69

Merged
winapiadmin merged 21 commits into
mainfrom
doxygen_documentation
Jun 17, 2026
Merged

Doxygen documentation#69
winapiadmin merged 21 commits into
mainfrom
doxygen_documentation

Conversation

@winapiadmin

Copy link
Copy Markdown
Owner

No functional changes

Fixes #68

winapiadmin and others added 19 commits June 15, 2026 12:36
… comments across position.h, types.h, attacks.h, fwd_decl.h, and moves_io.h to document public API and reach the 80% doc coverage target. Ran Doxygen and updated tools/missing.json.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…vailable for faster bishop/rook attacks

Detect BMI2 at runtime (GCC/Clang __builtin_cpu_supports, MSVC CPUID) and use _pext_u64; fall back to multiply-and-shift. Rebuild and benchmarked perft: 17.15s (186.36 Mnps).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…erted runtime BMI2 detection changes to restore portable magic lookup used previously for universal binaries.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tures; remove unused code and improve clarity
Docstrings generation was requested by @winapiadmin.

The following files were modified:

* `attacks.cpp`
* `attacks.h`
* `fwd_decl.h`
* `movegen.cpp`
* `moves_io.cpp`
* `moves_io.h`
* `position.cpp`
* `position.h`
* `printers.cpp`
* `types.h`

These file types are not supported:
* `.github/workflows/doxygen.yml`
* `.github/workflows/test.yml`
* `.gitignore`
* `Doxyfile`
@winapiadmin winapiadmin self-assigned this Jun 16, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@winapiadmin, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 35 minutes and 15 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2d645ec8-f014-438e-94c9-9334049718e5

📥 Commits

Reviewing files that changed from the base of the PR and between e9a81ce and f4bc3fc.

📒 Files selected for processing (2)
  • .github/workflows/test.yml
  • moves_io.cpp
📝 Walkthrough

Walkthrough

This PR addresses the Doxygen CI failure (issue #68) by setting RECURSIVE = NO in the Doxyfile, enforcing LF line endings in .gitattributes, removing !doc/ from .gitignore, and normalizing the workflow YAML. It also repairs a malformed ValueList template declaration in types.h, adds HOTFUNC to six movegen.h declarations, adjusts includes and documentation in position.h, and performs a broad Doxygen comment cleanup across attacks.cpp, attacks.h, fwd_decl.h, movegen.cpp, moves_io.h, and position.cpp. Additionally, moves_io.cpp is refactored with parseSan parameter renamed from raw_san to san and internal error-tracking logic updated.

Changes

Doxygen Failure Fix and Documentation Cleanup

Layer / File(s) Summary
Repo config, .gitignore, and Doxyfile settings
.gitattributes, .gitignore, Doxyfile, .github/workflows/doxygen.yml
.gitattributes enforces eol=lf via the * pattern; .gitignore drops the !doc/ exemption and adjusts *.s placement; Doxyfile sets RECURSIVE = NO; the Doxygen workflow YAML is line-ending normalized with no behavioral change.
Code-level fixes: types.h, movegen.h, position.h
types.h, movegen.h, position.h
ValueList's malformed template declaration is corrected to a valid single-line template header, and CountOnlyList's duplicate public: block is removed. HOTFUNC is added to six gen* function declarations in movegen.h. position.h replaces bitboard.h with movegen.h and types.h includes, adds @param entries for sq and by in isAttacked documentation, and removes the @return nothing line from scan_attacks_ray.
Doxygen comment cleanups across source files
attacks.cpp, attacks.h, fwd_decl.h, movegen.cpp, moves_io.h, position.cpp
Parameter and return-value documentation updated for bishop, rook functions; multi-line doc block removed from slider function; Board alias deprecation clarified; record_moves doc block reformatted and @param entries added for genKnightMoves and genSlidingMoves; stray SAN-parsing Doxygen block removed from moves_io.h; obsolete comment block removed before setFEN in position.cpp.
moves_io.cpp refactoring and parseSan rename
moves_io.cpp
Entire file reformatted. parseSan parameter renamed from raw_san to san; implementation refactored so _san (used in error messages) is constructed from the current input_san passed to do_parse, and the remove_illegals loop progressively trims san and retries until a legal move is found. All error paths now consistently use san/_san derived from current attempts. All other functions (squareToString, moveToUci, uciToMove, moveToSan, explicit instantiations) unchanged in behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • winapiadmin/chesslib#55: Modifies parseSan in moves_io.cpp, overlapping directly with this PR's parameter rename and error-path refactoring in the same function.
  • winapiadmin/chesslib#63: Also touches the parseSan code path in moves_io.cpp, making it directly related at the function level with overlapping SAN input variable handling.

Poem

🐇 Hop, hop, the docs were broken,
Recursive YES — a cursed token!
I fixed the Doxyfile with care,
And cleaned up comments everywhere.
Now parseSan has learned to thrive,
With proper naming, fresh and alive! 🌿

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Doxygen documentation' is vague and generic, not clearly summarizing the specific changes made across multiple files. Provide a more specific title that describes the actual fixes, such as 'Fix Doxygen configuration and documentation issues' or 'Resolve Doxygen generation failures'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description adequately relates to the changeset by indicating no functional changes and referencing the linked issue #68 about Doxygen failures.
Linked Issues check ✅ Passed The PR successfully addresses the Doxygen failure in issue #68 by fixing configuration and documentation issues preventing documentation generation.
Out of Scope Changes check ✅ Passed All changes are directly related to resolving Doxygen issues: configuration fixes, documentation cleanup, and function annotation updates for performance-critical code.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch doxygen_documentation

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
position.h (1)

20-23: ⚠️ Potential issue | 🟠 Major

Add #include "zobrist.h" to make position.h self-contained.

This header uses zobrist::RandomEP, zobrist::RandomTurn, zobrist::RandomPiece, and zobrist::RandomCastle directly in inline function implementations (e.g., doNullMove(), material_key()). Without an explicit include, it depends on a transitive re-export from another header, which is fragile and violates header self-containment.

🔧 Suggested fix
 `#include` "attacks.h"
+#include "zobrist.h"
 `#include` "movegen.h"
 `#include` "types.h"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@position.h` around lines 20 - 23, Add the missing `#include "zobrist.h"`
directive to the include section of position.h to ensure the header is
self-contained. The header currently uses symbols from the zobrist namespace
(RandomEP, RandomTurn, RandomPiece, RandomCastle) in inline function
implementations such as doNullMove() and material_key(), but relies on a
transitive re-export from another header. Include the zobrist header explicitly
alongside the existing includes for attacks.h, movegen.h, types.h, and algorithm
to properly declare these dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@moves_io.cpp`:
- Around line 180-418: The function parameter in parseSan was renamed from
raw_san to san on the function signature, but the implementation still
references the undefined identifier raw_san in four locations. Replace all
occurrences of raw_san with san: in the local variable initialization where _san
is declared (line 188), in the trimmed_san initialization within the
remove_illegals block (line 407), in the error message string concatenation
(line 415), and in the do_parse function call (line 418). This will resolve the
undefined identifier compilation errors throughout the function body.

---

Outside diff comments:
In `@position.h`:
- Around line 20-23: Add the missing `#include "zobrist.h"` directive to the
include section of position.h to ensure the header is self-contained. The header
currently uses symbols from the zobrist namespace (RandomEP, RandomTurn,
RandomPiece, RandomCastle) in inline function implementations such as
doNullMove() and material_key(), but relies on a transitive re-export from
another header. Include the zobrist header explicitly alongside the existing
includes for attacks.h, movegen.h, types.h, and algorithm to properly declare
these dependencies.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 846381a3-2224-4112-8b92-d2135c2e1a0d

📥 Commits

Reviewing files that changed from the base of the PR and between 35656db and 5f8801b.

📒 Files selected for processing (14)
  • .gitattributes
  • .github/workflows/doxygen.yml
  • .gitignore
  • Doxyfile
  • attacks.cpp
  • attacks.h
  • fwd_decl.h
  • movegen.cpp
  • movegen.h
  • moves_io.cpp
  • moves_io.h
  • position.cpp
  • position.h
  • types.h
💤 Files with no reviewable changes (2)
  • position.cpp
  • moves_io.h

Comment thread moves_io.cpp Outdated
@winapiadmin winapiadmin merged commit b7e65d6 into main Jun 17, 2026
27 checks passed
@winapiadmin winapiadmin deleted the doxygen_documentation branch June 17, 2026 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Doxygen failure

1 participant