Repository navigation
fix(plugins): keep a guess_number range that starts at zero - #1203
Open
lihongyuan99 wants to merge 1 commit into
Open
lihongyuan99 wants to merge 1 commit into
lihongyuan99 wants to merge 1 commit into
Conversation
lihongyuan99
force-pushed
the
fix/mini-games-zero-low-high
branch
5 times, most recently
from
October 3, 2026 14:39
30e2f9e to
66fb36e
Compare
lihongyuan99
force-pushed
the
fix/mini-games-zero-low-high
branch
3 times, most recently
from
October 11, 2026 23:33
b90b01f to
d1836fe
Compare
lihongyuan99
force-pushed
the
fix/mini-games-zero-low-high
branch
2 times, most recently
from
October 14, 2026 23:25
c009b7b to
0ffded5
Compare
lihongyuan99
force-pushed
the
fix/mini-games-zero-low-high
branch
from
October 15, 2026 23:22
0ffded5 to
3e9b1dc
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
guess_numbernormalizes its range withor, 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=0is falsy, so it is treated as "not passed" and the lower bound becomes 1.Measured on
develop@4667bd7bby calling the real function:Two user-visible consequences:
main.py:75-77answers 「0 太小了」 against a range the tool itself widened.low=0, high=1is 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) andif guess is None(:69) — so this is just the one spot that missed the convention. The tool is registered verbatim viactx.tool("guess_number", guess_number)(:99-106), so whateverlow/highthe model emits reaches these two lines;0is a validintand nothing upstream filters it.Fix: compare against
Noneinstead of truthiness, keeping both existing behaviours intact — the 1/100 defaults when the argument is omitted, and thelo >= hifallback for an inverted or degenerate range (low=50, high=10still resets to 1–100, as before).Deliberately not covered here: when the model omits
secret,main.py:67re-rollsrandom.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
develop(feature / fix — default)main(release/*orhotfix/*only)Type of change
Test plan
Added
tests/unit/test_mini_games_plugin.py— six cases, loaded offline withimportlibexactly liketests/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.
develop@4667bd7b's ownmain.py):uv run pytest -q tests/unit/test_mini_games_plugin.py→3 failed, 3 passed in 1.51suv run pytest -q tests/unit/test_mini_games_plugin.py→6 passed in 1.37suv run ruff check src tests→All checks passed!uv run ruff format --check src tests→1081 files already formatteduv run mypy src/octop→Success: no issues found in 530 source filesuv run pytest -q -m "not live" -n 3→ 3853 passed, 17 skipped, 0 failed (575.66s under concurrent sibling runs), against3847 passed, 17 skipped, 0 failed measured on the base commit
develop@4667bd7b; the +6 are exactly the new casestests/livewas not runThe backend gates were run by hand as the equivalent of the CI
qualityjob, rather thanmake all,because
make allalso shells out to the dashboard build and this clone has nonode_modules.Rebased onto
develop@ded7adddon 2026-09-30.1.0.2b4and1.0.2b5both shipped since the previous base, and each release commit folded the whole## [Unreleased]list into a version block, which emptied that section ondevelop— it is this branch's only conflict. The resolution keepsdevelop'sCHANGELOG.mdverbatim 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 allwas not used because this clone has nodashboard/node_modules.uv run pytest -q tests/unit/test_mini_games_plugin.py->3 failed, 3 passed in 4.00s6 passed in 1.51suv run ruff check src tests->All checks passed!;uv run ruff format --check src tests->1112 files already formatteduv run mypy src/octop->Success: no issues found in 543 source filesuv 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 istests/unit/test_plugin_manager.py::test_install_plugin_importing_legacy_harness_agent:legacy_imports.ensure_legacy_harness_agent_alias()deliberately leaves a realharness_agentdistribution alone, and this local venv has an out-of-lockorcakit-harness-agentinstalled, 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 fromuv.lock(thePython 3.12quality job issuccesson both this branch's pre- and post-rebase heads).tests/livewas not run; no model API key, PostgreSQL or Docker was needed.Rebased onto
develop@83b2ae2e7(2026-10-03).Four commits landed on
developsince 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], soCHANGELOG.mdis this PR's only conflict. The resolution keepsdevelop'sCHANGELOG.mdverbatim and re-inserts this PR's single line into the existing### 修复list:git diff origin/develop 30e2f9e5a -- CHANGELOG.mdis+1/-0, andgit diff 755d582980 30e2f9e5a2over 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 formattedmypy src/octop→Success: no issues found in 551 source filespytest -q tests/unit/test_mini_games_plugin.py→6 passed in 3.52spytest -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 base83b2ae2e7in 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 artifacttests/unit/test_plugin_manager.py::test_install_plugin_importing_legacy_harness_agent— this venv has an out-of-lockorcakit-harness-agentdistribution installed, and that test deliberately leaves a realharness_agentpackage alone, so its alias assertion cannot hold here. It is red on the untouched base too and green in CI, which installs strictly fromuv.lock.2026-10-06 —
CHANGELOG.mdre-aligned withdevelop. 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.mdis nowdevelop's file verbatim plus this PR's single bullet under## [Unreleased]→### 修复:git diff --numstat 75c274aef b90b01fb4 -- CHANGELOG.md→1\t0\tCHANGELOG.mdgit diff --name-only 0e72a5cfe b90b01fb4→CHANGELOG.mdonly, so every code and test file is byte-identical to the head thatPython 3.12andWindows / Python 3.12both reported success on;the commands and counts under Test plan therefore still describe this diff.
CHANGELOG.mdat75c274aefis identical to today'sdeveloptip, so this does not need a fresh rebase.Rebased onto
develop@97d6e7ff4(2026-10-12); headb90b01fb4->d1836fefc.CHANGELOG.mdis again this branch's only conflict, and the resolution keepsdevelop's fileverbatim and re-inserts only this PR's own bullet under its matching
###subsection — checkedmechanically: the diff against the new base is
1 added / 0 deletedinCHANGELOG.mdand 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 allstill does not apply — nodashboard/node_modulesin this clone; the venv is created per worktree so/Users/macos/Documents/Qoder/octop-pr-mining/repos/wt-r-1203/src/octop/__init__.pyis this branch's own code):6 passed in 19.53s4633 passed, 26 skipped, 17 warnings in 442.45s (0:07:22), againstdevelop@97d6e7ff4measured in the same venv: 4627 passed, 26 skipped, 0 failedAll checks passed!;1180 files already formatted;Success: no issues found in 570 source filesRebased onto
develop@87a8d52c2(2026-10-14);headd1836fefc->c009b7be7.CHANGELOG.mdis again the only conflict.develop's file is kept verbatim and this PR's ownbullet 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 deletedthere and the2 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 allstill does not apply:no
dashboard/node_modulesin this clone; the venv belongs to this worktree, so/Users/macos/Documents/Qoder/octop-pr-mining/repos/wt-r-1203/src/octop/__init__.pyis this branch's own code):RC ruff=0 format=0 mypy=0 targeted=0 full=06 passed in 6.27s4685 passed, 26 skipped, 41 warnings in 234.09s (0:03:54), againstdevelop@87a8d52c2measured in the same environment today: 4679 passed, 26 skipped, 0 failedAll checks passed!;1186 files already formatted;Success: no issues found in 571 source filesRebased onto
develop@99e479a6b(2026-10-16); head0ffded5fd->3e9b1dc47.CHANGELOG.mdwas 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 is1 added / 0 deletedthere and the2code/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 allstill does not apply — nodashboard/node_modulesin 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=06 passed in 12.05s4786 passed, 32 skipped, 41 warnings in 392.88s (0:06:32), againstdevelop@99e479a6bmeasured today in its own per-worktree venv on the same machine: 4780 passed, 32 skipped, 0 failed in 495.68sAll checks passed!;1204 files already formatted;Success: no issues found in 581 source files