Enable dtest -r for -s calc

[tameware]

Summary

• Extend dtest -r / --report so -s calc prints the same per-deal timing table as solve.
• Record optional per-strain-board solve times in calc_all_tables_x, then sum them to per-deal times in the harness.
• Update help text and remove the calc-only “timing not available” message.

Test plan

• bazelisk test //library/tests:report_board_timings_test //library/tests:loop_failure_test //library/tests/system:calc_all_tables_x_test
• bazelisk build //library/tests:dtest && ./bazel-bin/library/tests/dtest -f hands/list10.txt -s calc -n 1 -r shows a non-empty per-board timing report
• Confirm -s solve -r still reports as before

Made with Cursor

[Copilot]

🟡 Changes recommended

Zero-deal calc paths can leave reused timing output stale.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Extends dtest -r reporting to calc mode by collecting per-strain solve timings and aggregating them per deal.

Changes:

• Adds optional timing output to CalcAllTablesX APIs.
• Integrates calc timings into reports.
• Updates help text and adds aggregation and loop tests.

File summaries

File	Reviewed changes	Findings
library/tests/testcommon.cpp	Enables calc timing reports.	—
library/tests/report_board_timings.hpp	Declares timing aggregation helpers.	—
library/tests/report_board_timings.cpp	Implements per-deal timing aggregation.	—
library/tests/report_board_timings_test.cpp	Tests aggregation and saturation behavior.	—
library/tests/loop.hpp	Extends the loop API for timing output.	—
library/tests/loop.cpp	Collects and aggregates calc timings across batches.	—
library/tests/loop_failure_test.cpp	Tests cross-batch timing collection.	—
library/tests/args.cpp	Updates report help text.	—
library/src/calc_tables.hpp	Declares timing-aware APIs.	Moderate, 2 votes: zero-deal paths do not clear or resize strain_times_us.
library/src/calc_tables.cpp	Collects optional per-strain timings.	Moderate, 1 vote: the main zero-deal path can leave stale output. Moderate, 1 vote: the PBN zero-deal path likewise leaves stale output.

Review details

Suppressed comments (2)

library/src/calc_tables.cpp:799

• The PBN helper has the same stale-output edge case: its zero-deal success return bypasses the forwarding call, so a caller-provided strain_times_us is not cleared despite the optional-output contract. Clear it before returning for numDeals == 0.

    return calc_all_tables_x(
      numDeals, binary.data(), mode, trumpFilter, results, par, maxThreads,
      strain_times_us);

library/src/calc_tables.cpp:711

• The new timing-output contract says a non-null strain_times_us is resized to numDeals * included_strains, but the numDeals == 0 success path returns before this assignment and leaves a reused vector unchanged. Clear the optional output on the zero-deal path so callers cannot observe stale timings.

    if (strain_times_us != nullptr)
      *strain_times_us = std::move(local_strain_times);

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Lite

---

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

[Copilot]

🔵 Needs a closer look

Add a direct <utility> include in calc_tables.cpp for std::move.

Review details

Suppressed comments (1)

library/src/calc_tables.cpp:12

• This translation unit now calls std::move below, but it does not include <utility> directly and therefore relies on transitive standard-library includes. That is not portable across toolchains; add the direct header so the new code has a guaranteed declaration.

#include <chrono>

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite

[tameware]

Addressed the follow-up Copilot note: library/src/calc_tables.cpp now includes <utility> directly for std::move on the strain-timing path.

[Copilot]

🟢 Approval recommended

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

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 0 new
• Review effort level: Lite

[zzcgumn]

I like the idea that we can report fine-grained performance data and feed it to any statistical analysis toolkit we want to use.