Skip to content

fix(plugins): keep a guess_number range that starts at zero - #1203

Open
lihongyuan99 wants to merge 1 commit into
TencentCloud:developfrom
lihongyuan99:fix/mini-games-zero-low-high
Open

lihongyuan99 wants to merge 1 commit into
TencentCloud:developfrom
lihongyuan99:fix/mini-games-zero-low-high

Conversation

@lihongyuan99

@lihongyuan99 lihongyuan99 commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

guess_number normalizes its range with or, so a range that legitimately starts at 0 is silently rewritten:

  • src/octop/infra/agents/plugins/bundled/mini-games/main.py:64-65 — lo = int(low or 1) / hi = int(high or 100). low=0 is falsy, so it is treated as "not passed" and the lower bound becomes 1.

Measured on develop@4667bd7b by calling the real function:

guess_number(low=0, high=50) -> {"low": 1,  "high": 50,  ...}  "我想了一个 1 到 50 之间的整数,请猜。"
guess_number(low=0, high=1)  -> {"low": 1,  "high": 100, ...}  "我想了一个 1 到 100 之间的整数,请猜。"

Two user-visible consequences:

  1. "Think of a number between 0 and 50" — the ordinary way to start such a game — makes 0 unguessable, and the reply contradicts what the user asked for. If the player then guesses 0, main.py:75-77 answers 「0 太小了」 against a range the tool itself widened.
  2. low=0, high=1 is the worst case: both bounds collapse to 1, if lo >= hi (main.py:66) then discards the requested range entirely and falls back to 1–100, so the smallest possible game becomes a hundred-number search.

The rest of the same function already uses the right form for its other optional args — int(secret) if secret is not None else ... (:68) and if guess is None (:69) — so this is just the one spot that missed the convention. The tool is registered verbatim via ctx.tool("guess_number", guess_number) (:99-106), so whatever low/high the model emits reaches these two lines; 0 is a valid int and nothing upstream filters it.

Fix: compare against None instead of truthiness, keeping both existing behaviours intact — the 1/100 defaults when the argument is omitted, and the lo >= hi fallback for an inverted or degenerate range (low=50, high=10 still resets to 1–100, as before).

Deliberately not covered here: when the model omits secret, main.py:67 re-rolls random.randint(lo, hi) on every call, so a player narrowing the range by bisection can never land on the number. That is a question of who owns game state (the model, or the tool), not a falsy-value bug, and it needs a design decision — so this PR leaves it alone.

Target branch

  • Base is develop (feature / fix — default)
  • Base is main (release/* or hotfix/* only)

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Refactor / chore
  • Release / hotfix

Test plan

Added tests/unit/test_mini_games_plugin.py — six cases, loaded offline with importlib exactly like
tests/unit/test_hot_topics_plugin.py, so no network, no model API key and no database are involved.
Three of them pin the outcome for a zero lower bound; three are guardrails proving the defaults and the
inverted-range fallback are unchanged.

  • red (new tests against develop@4667bd7b's own main.py):
    uv run pytest -q tests/unit/test_mini_games_plugin.py → 3 failed, 3 passed in 1.51s
  • green (with the two-line fix): uv run pytest -q tests/unit/test_mini_games_plugin.py → 6 passed in 1.37s
  • uv run ruff check src tests → All checks passed!
  • uv run ruff format --check src tests → 1081 files already formatted
  • uv run mypy src/octop → Success: no issues found in 530 source files
  • full suite uv run pytest -q -m "not live" -n 3 → 3853 passed, 17 skipped, 0 failed (575.66s under concurrent sibling runs), against
    3847 passed, 17 skipped, 0 failed measured on the base commit develop@4667bd7b; the +6 are exactly the new cases
  • tests/live was not run

The backend gates were run by hand as the equivalent of the CI quality job, rather than make all,
because make all also shells out to the dashboard build and this clone has no node_modules.


Rebased onto develop@ded7addd on 2026-09-30. 1.0.2b4 and 1.0.2b5 both shipped since the previous base, and each release commit folded the whole ## [Unreleased] list into a version block, which emptied that section on develop — it is this branch's only conflict. The resolution keeps develop's CHANGELOG.md verbatim and re-inserts this PR's single bullet under a fresh ### 修复; nothing else changed.

Everything below was re-measured on develop@ded7addd. Backend gates were run by hand, as the equivalents of the CI steps (make lint / make typecheck / make test); make all was not used because this clone has no dashboard/node_modules.

  • Red, with this PR's fix reverted and only its tests: uv run pytest -q tests/unit/test_mini_games_plugin.py -> 3 failed, 3 passed in 4.00s
  • Green as committed: the same command -> 6 passed in 1.51s
  • uv run ruff check src tests -> All checks passed!; uv run ruff format --check src tests -> 1112 files already formatted
  • uv run mypy src/octop -> Success: no issues found in 543 source files
  • Full suite uv run pytest -q -m "not live" -n 6 -> 3959 passed, 17 skipped, 1 failed in 213.70s, against 3953 passed, 17 skipped, 1 failed in 225.23s measured on the unmodified base commit in the same virtualenv. The single failure in both runs is tests/unit/test_plugin_manager.py::test_install_plugin_importing_legacy_harness_agent: legacy_imports.ensure_legacy_harness_agent_alias() deliberately leaves a real harness_agent distribution alone, and this local venv has an out-of-lock orcakit-harness-agent installed, so the alias assertion cannot hold here. It is pre-existing on the base, not a case this PR adds, and it is green in CI, which installs strictly from uv.lock (the Python 3.12 quality job is success on both this branch's pre- and post-rebase heads).
  • tests/live was not run; no model API key, PostgreSQL or Docker was needed.

Rebased onto develop@83b2ae2e7 (2026-10-03).

Four commits landed on develop since this branch's previous base (a02e3e18e): LDAP directory login (#1123), propagating exhausted model retries as failures (#1012), and two dashboard fixes (#1429, #1241) — each appended a bullet to ## [Unreleased], so CHANGELOG.md is this PR's only conflict. The resolution keeps develop's CHANGELOG.md verbatim and re-inserts this PR's single line into the existing ### 修复 list: git diff origin/develop 30e2f9e5a -- CHANGELOG.md is +1/-0, and git diff 755d582980 30e2f9e5a2 over every non-CHANGELOG file of the PR is empty, i.e. the code you already reviewed is byte-identical.

Re-measured on the new base with the same commands and the same virtualenv as the Test plan above:

  • ruff check src tests → All checks passed!
  • ruff format --check src tests → 1134 files already formatted
  • mypy src/octop → Success: no issues found in 551 source files
  • this PR's own tests: pytest -q tests/unit/test_mini_games_plugin.py → 6 passed in 3.52s
  • full suite pytest -q -m "not live" -n 3 → 1 failed, 4138 passed, 17 skipped, 13 warnings in 412.91s (0:06:52) (three branches measured concurrently, so the wall time is inflated), against 4132 passed, 17 skipped, 1 failed in 218.55s measured on the unmodified base 83b2ae2e7 in this same virtualenv: +6 cases against the base, which are exactly this PR's. The single failure in both runs is the pre-existing local artifact tests/unit/test_plugin_manager.py::test_install_plugin_importing_legacy_harness_agent — this venv has an out-of-lock orcakit-harness-agent distribution installed, and that test deliberately leaves a real harness_agent package alone, so its alias assertion cannot hold here. It is red on the untouched base too and green in CI, which installs strictly from uv.lock.

2026-10-06 — CHANGELOG.md re-aligned with develop. A rebase of this branch had re-introduced other PRs' release-note bullets into the released ## [1.0.2b6] block, so this PR's diff carried those foreign lines instead of only its own. CHANGELOG.md is now develop's file verbatim plus this PR's single bullet under ## [Unreleased] → ### 修复:

  • git diff --numstat 75c274aef b90b01fb4 -- CHANGELOG.md → 1\t0\tCHANGELOG.md
  • git diff --name-only 0e72a5cfe b90b01fb4 → CHANGELOG.md only, so every code and test file is byte-identical to the head that Python 3.12 and Windows / Python 3.12 both reported success on;
    the commands and counts under Test plan therefore still describe this diff.
  • CHANGELOG.md at 75c274aef is identical to today's develop tip, so this does not need a fresh rebase.

Rebased onto develop@97d6e7ff4 (2026-10-12); head b90b01fb4 -> d1836fefc.

CHANGELOG.md is again this branch's only conflict, and the resolution keeps develop's file
verbatim and re-inserts only this PR's own bullet under its matching ### subsection — checked
mechanically: the diff against the new base is 1 added / 0 deleted in CHANGELOG.md
and byte-identical (hunk headers aside) for 2 code/test file(s) before and after the
rebase. Gates re-run by hand on the new head (make all still does not apply — no
dashboard/node_modules in this clone; the venv is created per worktree so
/Users/macos/Documents/Qoder/octop-pr-mining/repos/wt-r-1203/src/octop/__init__.py is this branch's own code):

  • targeted tests for this branch: 6 passed in 19.53s
  • full suite: 4633 passed, 26 skipped, 17 warnings in 442.45s (0:07:22), against develop@97d6e7ff4 measured in the same venv: 4627 passed, 26 skipped, 0 failed
  • All checks passed!; 1180 files already formatted; Success: no issues found in 570 source files

Rebased onto develop@87a8d52c2 (2026-10-14); head d1836fefc -> c009b7be7.

CHANGELOG.md is again the only conflict. develop's file is kept verbatim and this PR's own
bullet is re-inserted into the current ## [Unreleased] list, which is now flat (no ###
subsections), so the diff against the new base is 1 added / 0 deleted there and the
2 code/test file(s) are byte-identical before and after (hunk headers aside) — checked
mechanically, not by eye. Gates re-run by hand on the new head (make all still does not apply:
no dashboard/node_modules in this clone; the venv belongs to this worktree, so
/Users/macos/Documents/Qoder/octop-pr-mining/repos/wt-r-1203/src/octop/__init__.py is this branch's own code):

  • RC ruff=0 format=0 mypy=0 targeted=0 full=0
  • targeted tests for this branch: 6 passed in 6.27s
  • full suite: 4685 passed, 26 skipped, 41 warnings in 234.09s (0:03:54), against develop@87a8d52c2 measured in the same environment today: 4679 passed, 26 skipped, 0 failed
  • All checks passed!; 1186 files already formatted; Success: no issues found in 571 source files

Rebased onto develop@99e479a6b (2026-10-16); head 0ffded5fd -> 3e9b1dc47.

CHANGELOG.md was again the only conflict: develop's file is kept verbatim and this PR's own bullet is re-inserted under its matching ### subsection of the current ## [Unreleased] block, so the diff against the new base is 1 added / 0 deleted there and the 2 code/test file(s) are byte-identical before and after the rebase (checked mechanically on the patch hash, not by eye). Gates re-run by hand on the new head (make all still does not apply — no dashboard/node_modules in this clone; the venv belongs to this worktree, so the checkout under test is this branch's own code):

  • RC ruff=0 format=0 mypy=0 targeted=0 full=0
  • targeted tests for this branch: 6 passed in 12.05s
  • full suite: 4786 passed, 32 skipped, 41 warnings in 392.88s (0:06:32), against develop@99e479a6b measured today in its own per-worktree venv on the same machine: 4780 passed, 32 skipped, 0 failed in 495.68s
  • All checks passed!; 1204 files already formatted; Success: no issues found in 581 source files

@lihongyuan99
lihongyuan99 force-pushed the fix/mini-games-zero-low-high branch 5 times, most recently from 30e2f9e to 66fb36e Compare October 3, 2026 14:39
@lihongyuan99
lihongyuan99 force-pushed the fix/mini-games-zero-low-high branch 3 times, most recently from b90b01f to d1836fe Compare October 11, 2026 23:33
@lihongyuan99
lihongyuan99 force-pushed the fix/mini-games-zero-low-high branch 2 times, most recently from c009b7b to 0ffded5 Compare October 14, 2026 23:25
@lihongyuan99
lihongyuan99 force-pushed the fix/mini-games-zero-low-high branch from 0ffded5 to 3e9b1dc Compare October 15, 2026 23:22

This branch has not been deployed

No deployments
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.

1 participant