Skip to content

test: analyse_later_board TT counter reset/report (#379 item 4) - #407

Merged
wopdevries merged 3 commits into
developfrom
wopdevries-analyse-later-board-tt-stats
Sep 28, 2026
Merged

wopdevries merged 3 commits into
developfrom
wopdevries-analyse-later-board-tt-stats

Conversation

@wopdevries

@wopdevries wopdevries commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Adds the reset/report lifecycle for analyse_later_board(), item 4 of #379 and the last remaining item.

Change (library/src/solver_if.cpp)

analyse_later_board() reuses the caller's warm SolverContext/TT across a whole AnalysePlayBin call. It previously had no counter reset and no DDS_PRINT_TT_STATS report at all.
It now resets tt_lookup_count, tt_hit_count and the TT op-stats at entry. Only the counters are reset. The warm TT is deliberately left untouched, per the #157 warm-context contract.
It reports on both the normal exit and the cardCount<=4 last-trick early return, which previously reported nothing.
The reporting now lives in one shared print_tt_stats() helper, including the n/a TransTableS case from #393. solve_board_internal, solve_same_board and analyse_later_board all call it, replacing the two duplicated inline blocks.

Tests (library/tests/solve_board/analyse_later_board_tt_stats_test.cpp)

ResetsLookupAndHitCountersIndependentlyOfPriorSolve seeds tt_lookup_count and tt_hit_count with a large sentinel just before a direct analyse_later_board call. It confirms the printed counts can't include the sentinel.
ResetsTtOpStatsIndependentlyOfPriorSolve covers op-stats, which have no public setter to poison. It compares two identical runs, one with an extra manual reset_op_stats() before the call. If analyse_later_board really resets, both print identical adds, overwrites and harvests. This doesn't depend on how much work the later search does. It forces DDS_TT_KIND=large so the counts are real numbers, not n/a.
ReportsOnBothNormalAndLastTrickEarlyReturnExits drives a full play through the real AnalysePlayPBN on a forced one-suit-per-hand deal. It confirms every card, including the last-trick early return, gets its own report.

Verification

Each of the three resets (lookups, hits, op-stats) was disabled individually. Each break is caught only by the test meant to catch it.
The tests also fail against the pre-fix code.
#402's tt_print_stats_test still passes against the refactored solve_board_internal and solve_same_board.
Both test targets pass under bazelisk.

Closes #379.

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Address the requested helper reuse, complete counter assertions, and naming consistency.

Review effort: Lite
Findings: 3 Low severity

Open (3)
What changed in this PR

Adds TT counter reset/reporting to analyse_later_board() with coverage for normal and last-trick exits.

Changes:

  • Adds TT statistics reporting and counter resets.
  • Adds deterministic lifecycle tests.
  • Registers the new Bazel test target.
File Summary
library/​src/​solver_if.cpp Implements TT counter lifecycle and reporting.
library/​tests/​solve_board/​analyse_later_board_tt_stats_test.cpp Adds reset and reporting integration tests.
library/​tests/​solve_board/​BUILD.bazel Registers the test target.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread library/src/solver_if.cpp Outdated
Comment thread library/src/solver_if.cpp
Comment thread library/tests/solve_board/analyse_later_board_tt_stats_test.cpp Outdated

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Make the op-stat reset assertion deterministic.

Review effort: Lite
Findings: 1 Low severity

Open (1)
Resolved since last review (2)

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

All reviewed changes are covered and no unresolved blocking issues were identified.

Review effort: Lite
Findings: 1 Low severity

Open (1)

@wopdevries

wopdevries commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator Author

Addressed in 9ae71e9 and f1eff63. Hits are covered by the sentinel check, and op-stats are covered by comparing two runs that differ only in a manual reset_op_stats(), so it doesn't depend on how much work the later search does.
print_tt_stats() is now shared by solve_board_internal, solve_same_board and analyse_later_board, since Copilot asked for that dedup and it changes code beyond analyse_later_board.

@wopdevries
wopdevries merged commit 91b0322 into develop Sep 28, 2026
13 checks passed
@wopdevries
wopdevries deleted the wopdevries-analyse-later-board-tt-stats branch September 28, 2026 13:53
@wopdevries wopdevries self-assigned this Sep 29, 2026
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.

Follow-up: TT instrumentation test coverage and additional code paths

3 participants