Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces version 2 of the balatrobot API with breaking changes (!). The main focus is on restructuring how tags are represented in the game state and improving error messages across all endpoints to be more actionable and helpful.
Changes:
- Restructured tag representation from flat
tag_name/tag_effectfields to nestedtagobjects withkey,name, andeffectfields - Added
tagsarray to gamestate for tracking accumulated player-owned tags - Enhanced error messages across all endpoints with actionable guidance (e.g., suggesting
reroll,sell, etc.) - Added support for selling jokers when Buffoon packs are open (SMODS_BOOSTER_OPENED state)
- Implemented voucher effect extraction using game's localize function
- Added comprehensive Tag enum definitions and test coverage
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lua/utils/types.lua | Updated Blind type to use nested Tag object instead of flat tag_name/tag_effect fields; added Tag class definition |
| src/lua/utils/openrpc.json | Updated OpenRPC schema to reflect Tag object structure and enhanced sell endpoint description |
| src/lua/utils/gamestate.lua | Implemented voucher effect extraction, tag ownership tracking, and updated blind tag structure |
| src/lua/utils/enums.lua | Added comprehensive Tag.Key enum definitions for all Balatro tag types |
| src/lua/endpoints/sell.lua | Added support for SMODS_BOOSTER_OPENED state with Buffoon pack validation |
| src/lua/endpoints/skip.lua | Enhanced error message with actionable guidance |
| src/lua/endpoints/buy.lua | Enhanced error messages with actionable guidance |
| src/lua/endpoints/add.lua | Updated to support pack additions and refactored voucher handling to use dedicated SMODS function |
| src/lua/endpoints/use.lua | Enhanced error messages with actionable guidance |
| src/lua/endpoints/play.lua | Enhanced error message with actionable guidance |
| src/lua/endpoints/discard.lua | Enhanced error messages with actionable guidance |
| src/lua/endpoints/pack.lua | Enhanced error messages with actionable guidance |
| tests/lua/endpoints/test_skip.py | Added tests for tag accumulation after skipping blinds |
| tests/lua/endpoints/test_pack.py | Added tests for selling jokers during Buffoon pack selection |
| tests/lua/endpoints/test_gamestate.py | Added comprehensive test coverage for voucher effects and tag structure |
| tests/lua/endpoints/test_buy.py | Updated error message expectations |
| tests/lua/endpoints/test_add.py | Updated error message expectations |
| docs/api.md | Updated documentation to reflect new Tag structure and enhanced endpoint descriptions |
Comments suppressed due to low confidence (4)
src/lua/endpoints/add.lua:409
- The comment says "For jokers and consumables" but this else branch will also execute for vouchers and packs, creating unnecessary params that won't be used. Consider adding an explicit check:
elseif card_type == "joker" or card_type == "consumable" thento match the comment and avoid creating unused params for vouchers and packs.
else
-- For jokers and consumables - just pass the key
params = {
key = args.key,
skip_materialize = true,
stickers = {},
force_stickers = true,
}
-- Add edition if provided
if edition_value then
params.edition = edition_value
end
-- Add eternal if provided (jokers only - validation already done)
if args.eternal then
params.stickers[#params.stickers + 1] = "eternal"
end
-- Add perishable if provided (jokers only - validation already done)
if args.perishable then
params.stickers[#params.stickers + 1] = "perishable"
end
-- Add rental if provided (jokers only - validation already done)
if args.rental then
params.stickers[#params.stickers + 1] = "rental"
end
end
tests/lua/endpoints/test_skip.py:43
- Grammar issue in comment: "because it used immediately" should be "because it is used immediately"
assert "tag_investment" not in gamestate["tags"] # because it used immediately
tests/lua/endpoints/test_skip.py:53
- Grammar issue in comment: "because it used immediately" should be "because it is used immediately"
assert "tag_investment" not in gamestate["tags"] # because it used immediately
src/lua/utils/types.lua:58
- Typo: "bilnd" should be "blind"
---@field status Blind.Status Status of the bilnd
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert gamestate["tags"][0]["key"] == "tag_polychrome" | ||
| assert "tag_investment" not in gamestate["tags"] # because it used immediately |
There was a problem hiding this comment.
This test file has a bug that will cause test_skip_big_boss to fail. The test at line 54-58 (not shown in diff) expects the error message "Cannot skip Boss blind" but skip.lua line 39 now returns "Cannot skip Boss blind. Use select to select and play the boss blind." The expected error message in the test needs to be updated to match the new implementation.
| assert gamestate["blinds"]["big"]["status"] == "SKIPPED" | ||
| assert gamestate["blinds"]["boss"]["status"] == "SELECT" | ||
| assert gamestate["tags"][0]["key"] == "tag_polychrome" | ||
| assert "tag_investment" not in gamestate["tags"] # because it used immediately |
There was a problem hiding this comment.
This assertion is checking if the string "tag_investment" is in a list of tag objects. Since gamestate["tags"] is a list of objects (each with "key", "name", "effect" fields), the in operator will never find a string match. This should likely be checking if any tag in the list has key == "tag_investment", such as: assert not any(tag["key"] == "tag_investment" for tag in gamestate["tags"])
| assert gamestate["state"] == "BLIND_SELECT" | ||
| assert gamestate["blinds"]["boss"]["status"] == "SELECT" | ||
| assert gamestate["tags"][0]["key"] == "tag_polychrome" | ||
| assert "tag_investment" not in gamestate["tags"] # because it used immediately |
There was a problem hiding this comment.
This assertion is checking if the string "tag_investment" is in a list of tag objects. Since gamestate["tags"] is a list of objects (each with "key", "name", "effect" fields), the in operator will never find a string match. This should likely be checking if any tag in the list has key == "tag_investment", such as: assert not any(tag["key"] == "tag_investment" for tag in gamestate["tags"])
Previously, used_vouchers extracted descriptions from static voucher_data.description which was unreliable. Now uses get_voucher_effect() that fetches effect text via the game's localize() function with proper loc_vars for each voucher type. Also adds strip_color_codes() helper and comprehensive parametrized tests covering all 32 voucher types. Closes #154.
Improve error messages across 6 endpoint files by adding actionable guidance to help bots self-heal from failed tool calls. Changes: - buy.lua: Add endpoint suggestions for empty shop/slot errors - use.lua: Add card parameter guidance for consumable errors - discard.lua/play.lua: Add card limit suggestions - pack.lua: Add pack buying and target selection hints - skip.lua: Add boss blind selection suggestion - Update test_buy.py to match new error messages Closes #148.
Closes #143.
- Remove .claude/ directory (settings.json, skills/balatrobot/SKILL.md) - Remove CLAUDE.md in favor of AGENTS.md - Remove .mux/ directory (init, mcp.jsonc, tool_env, tool_post) - Remove .mdformat.toml (flags moved to Makefile) - Add AGENTS.md with project structure and rules - Add CONTEXT.md with glossary of domain terms - Add .agents/skills/balatrobot/SKILL.md for pi skill
Replace verbose boilerplate with minimal, curated entries covering macOS, Python, Lua, and project-specific ignores.
Inline --number and --exclude flags since the config file was removed.
Remove integration marker from pyproject.toml markers config.
The integration marker is no longer used. Remove auto-marking hooks from conftest files and the @pytest.mark.integration decorator.
Rename BalatroInstance module to match its primary export. Update import paths in tests.
Introduce BalatroPool with start/stop lifecycle, automatic port allocation, fail-fast cleanup, and async context-manager support. Includes InstanceInfo frozen dataclass for connection metadata.
StateFile wraps BalatroPool with a JSON state file (Jupyter pattern). Atomic write on pool start, delete on stop. Supports PID-based liveness checks, stale-file cleanup, and resolve-by-host:port or index. Add platformdirs dependency for cross-platform state directory.
Replace single BalatroInstance with pool-based serve. Adds -n / --num-instances flag for launching multiple instances. State file is written on start and cleaned up on exit.
New `balatrobot list` command reads the state file and displays running instances. Supports --json for machine-readable output. Register in CLI app.
Host/port are now optional — when omitted, the api command resolves the target from the state file. Supports --index flag to select an instance (default: 0). Falls back gracefully when no state file exists.
Add check_alive() that delegates to each instance's check_alive(). Remove __aenter__/__aexit__ — lifecycle is now managed by the caller (Server) rather than the pool itself.
Replace instance-based context manager with static write()/delete() methods. Remove __init__, __aenter__/__aexit__, _write_state, _delete_state, and all instance properties (path, instances, is_started). Lifecycle ownership moves to the new Server class.
Add Server async context manager that owns pool start/stop, state file write/delete, and a run() supervision loop (SIGTERM + check_alive). Rewrite _serve() and serve() to use Server, replacing the old StateFile context manager. Add InstanceDiedError and StateFileBusy exception handling in serve().
- test_instance: add check_alive tests (healthy, dead, not-started) - test_pool: replace context manager tests with check_alive tests - test_state: replace context manager tests with write/delete tests - test_server: new file with 5 tests for Server lifecycle and run loop
Remove BalatroPool, InstanceInfo, and StateFile from __all__. These are CLI internals, not part of the public bot-author API. Keep APIError, BalatroClient, BalatroInstance, Config, __version__.
Add sys.platform != 'win32' guard around add_signal_handler/ remove_signal_handler. Wrap the supervision loop in try/finally to ensure the signal handler is always removed, even on InstanceDiedError.
Replace 'async context-manager protocol' with check_alive(). The pool no longer has __aenter__/__aexit__.
Verify that add_signal_handler is never called on win32 and that the supervision loop still exits cleanly.
Sends SIGTERM to the server PID from the state file, polls for process death (100ms intervals, 5s timeout). Idempotent — calling twice or on an already-dead process is safe.
Verify that Server.run() registers a SIGTERM signal handler on non-Windows platforms and that the full SIGTERM flow triggers clean shutdown (state file deleted, pool stopped).
- Move InstanceInfo from pool.py to instance.py, change log_path type to Path | None (stringify only at JSON boundary) - Rename _default_state_path to default_state_path (public API) - Fix Server.__aexit__ ordering: stop pool before deleting state file - Document BalatroPool as non-restartable after stop() - Fix stale docstring in test_instance.py - Drop redundant host/port args in api.py else branch
Reusable skill for generating conventional commits with auto-staging and logical grouping.
Promote routine operational messages (endpoint actions, state transitions, mode activations) from sendDebugMessage to their appropriate levels: sendInfoMessage for normal flow, sendWarnMessage for recoverable issues, sendErrorMessage for failures. Shorten endpoint lifecycle messages to concise labels: "Init foo()" → "foo()", "Return foo()" → "foo() → ok". Rename utils/logger.lua to utils/format.lua (BB_LOGGER → BB_FORMAT) to reflect that the module provides formatting/serialization helpers, not logging — actual logging uses SMODS send*Message functions.
When BALATROBOT_LOGS_PATH is set, the Lua server appends each request and response as a JSONL line to <port>.req.jsonl and <port>.res.jsonl respectively. The Python launcher now exposes this env var automatically so sessions are recorded out of the box. These trace files feed the new CLI replay mode introduced in the next commit.
Add --requests and --responses flags that replay a JSONL trace file against a live server. With --responses, the command verifies each reply matches the expected result and reports divergences. tqdm shows progress when installed. Also move tqdm from test to main dependencies since it now ships to end users.
Update balatrobot skill to cover the automatic req.jsonl/res.jsonl recording and the new --requests/--responses flags on the api command.
When a Charm Tag (or similar) opens a free booster pack from BLIND_SELECT, skipping or selecting all cards must return the game to BLIND_SELECT — not SHOP. The polling loop now checks for both states so it doesn't hang waiting for SHOP after a tag-triggered pack. This issue was first noted in PR #190. Co-authored-by: icebear <icebear0828@users.noreply.github.com>
Add a boss parameter to the set endpoint that overrides which Boss Blind appears during the blind selection screen. This enables testing boss-specific bugs without playing through multiple antes. Implementation uses the game's own perscribed_bosses mechanism with G.FUNCS.reroll_boss, bypassing the charge via G.from_boss_tag. The response awaits the controller lock release before returning the updated gamestate. Validation ensures: - Only callable in BLIND_SELECT state - Boss blind state must be Upcoming - Key must exist in G.P_BLINDS and have .boss = true - Mutually exclusive with the shop parameter Also adds key field to the Blind type in gamestate output, Blind.Key enum alias, OpenRPC spec update, and docs with boss blind key reference table. Tests: 7 unit tests + 1 integration test (set -> skip -> select -> play -> verify boss name).
Add inline # comments describing each blind's effect, matching the style used by other enums in the file. Showdown bosses are grouped at the end and tagged with (Showdown).
…raction The API now returns a `key` field on each blind (`bl_small`, `bl_big`, `bl_manacle`). Update the expected dict to include these.
… card The Cerulean Bell boss blind (bl_final_bell) forces one random card to always be selected via card.ability.forced_selection. Both endpoints called unhighlight_all() which preserves forced cards, causing the forced card to silently leak into the played/discarded hand alongside the user's requested cards. Add validation that checks for forced_selection on any card in hand and rejects with BAD_REQUEST if the forced card is not included in the request. Replace unhighlight_all() with targeted logic that only clears non-forced highlights and only clicks cards not already highlighted, avoiding reliance on toggle no-op behavior. This issue was first notice in PR #190 Co-authored-by: icebear <icebear0828@users.noreply.github.com>
…card Add state-SELECTING_HAND--blinds.boss.key-bl_final_bell fixture under both play and discard sections in fixtures.json. Setup uses set boss bl_final_bell, skip small and big blinds, then select boss.
Add test_cerulean_bell_forced_card_not_included_in_play and test_cerulean_bell_forced_card_not_included_in_discard. Both load the bl_final_bell fixture, find the forced-highlighted card, then attempt to play/discard a different card and assert a BAD_REQUEST error containing 'forced-selected by the boss blind'.
Remove trailing whitespace in docs/api.md set_value and boss-blind tables. Split the long string-concatenation line in discard.lua error message for readability.
Uh oh!
There was an error while loading. Please reload this page.