BUG: Return -1 from best_response_2p instead of falling through to None

[mmcky]

Extracts the one standalone fix from the long-open PR #576 (see discussion there), so it can land independently of the type-annotation policy decision in #897.

best_response_2p documents an int return, but when tol < 0 no action satisfies the tolerance condition and the final loop falls through to an implicit None. Per @oyamad's suggestion in the #576 review discussion, the function now returns -1 in that case and the docstring documents the behavior (and corrects the stated default for tol, which is 1e-8, not None).

This is hardening rather than a live bug: no internal caller passes a negative tol. A regression test covering the tol < 0 case is included.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR hardens best_response_2p (a numba-jitted helper in quantecon.game_theory.normal_form_game) so it always returns an int, addressing the documented-but-previously-implicit fallthrough to None when tol < 0.

Changes:

• Update best_response_2p to return -1 if no action satisfies the tolerance condition (previously could fall through to None).
• Update the function docstring to document the sentinel return and correct the stated default for tol.
• Add a regression test covering the tol < 0 case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
quantecon/game_theory/normal_form_game.py	Ensures best_response_2p returns an int in all paths and updates docs for the new behavior.
quantecon/game_theory/tests/test_normal_form_game.py	Adds a regression test to lock in the tol < 0 sentinel return behavior.

Suppressed comments (1)

quantecon/game_theory/normal_form_game.py:938

• The inline comment says this return is "Unreachable unless tol < 0", but the code can fall through for other inputs (e.g., tol being NaN makes all >= comparisons false). The comment should describe the actual condition for reaching this line rather than asserting it is unreachable.

    return -1  # Unreachable unless tol < 0

---

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

[coveralls]

coverage: 90.571% (+0.001%) from 90.57% — fix/best-response-2p-fallthrough into main

[mmcky]

Summary of this PR's final state, for reviewers:

The fix (623e5e0): best_response_2p previously fell through to an implicit None when tol < 0, despite documenting an int return. It now returns -1, per the resolution originally agreed with @oyamad in the #576 review discussion. A regression test covers the tol < 0 case. This is hardening rather than a live bug — the function has no internal callers (it is exported public API only), and no meaningful input reaches the sentinel.

Docstring (9e9a6c7, 7556d02, 898bfb8): after iterating on Copilot's review comment, the final wording states the precondition in the parameter docs (tol must be nonnegative) and frames -1 explicitly as an error condition in the Returns section — the numba-friendly equivalent of a Raises entry, since jitted code avoids exceptions. The two statements are complementary: precondition where the parameter is defined, failure mode where the return is defined.

This extraction lets the fall-through fix land independently of the type-annotation policy decision in #897 that gates #576.

🤖 Generated with Claude Code

[oyamad]

@mmcky Thanks!