Skip to content

feat!: balatrobot v2#155

Open
S1M0N38 wants to merge 49 commits into
mainfrom
dev
Open

feat!: balatrobot v2#155
S1M0N38 wants to merge 49 commits into
mainfrom
dev

Conversation

@S1M0N38 S1M0N38 changed the title BalatroBot v2 feat!: balatrobot v2 Feb 24, 2026
@S1M0N38 S1M0N38 marked this pull request as ready for review February 25, 2026 12:09
Copilot AI review requested due to automatic review settings February 25, 2026 12:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_effect fields to nested tag objects with key, name, and effect fields
  • Added tags array 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" then to 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.

Comment thread tests/lua/endpoints/test_skip.py Outdated
Comment on lines +52 to +53
assert gamestate["tags"][0]["key"] == "tag_polychrome"
assert "tag_investment" not in gamestate["tags"] # because it used immediately
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment thread tests/lua/endpoints/test_skip.py Outdated
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
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"])

Copilot uses AI. Check for mistakes.
Comment thread tests/lua/endpoints/test_skip.py Outdated
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
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"])

Copilot uses AI. Check for mistakes.
S1M0N38 added 10 commits May 27, 2026 17:23
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.
S1M0N38 added 14 commits May 27, 2026 20:41
- 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.
S1M0N38 added 25 commits May 28, 2026 19:32
Replace separate host/port fixtures with unified InstanceInfo.
Update Lua conftest, server tests, and CLI conftest imports.
Update __init__.py to re-export the new modules so users can
import directly from the balatrobot package.
Drop uuid-based session IDs in favor of human-readable timestamps
for log directory names. Rename session_id to session_name throughout
BalatroPool and BalatroInstance.
Remove --host and --port from serve command. Ports are now always
ephemeral and discovered via state file. Add validation that
--num-instances is >= 1. Print session name and logs directory
instead of misleading PID label.
Error if only one of --host or --port is provided. Both must be
given for direct connection, or neither for state file discovery.
Wrap pool.start() and _write_state() in try/except in StateFile.__aenter__.
If writing the state file fails after the pool starts, stop the pool
before re-raising to prevent orphaned processes.
Replace misleading PID output with Started timestamp in list command.
Update tests for session_id -> session_name rename, remove unused
imports flagged by ruff, and add regression tests for:
- serve -n 0 and negative values
- api --host without --port (and vice versa)
- StateFile cleanup on write failure
- serve --help no longer shows --host/--port
Add optional log_path field to InstanceInfo, propagate it through
BalatroPool and StateFile, and display it in No running instances. output.
Restructure runbook around serve/list/api commands with examples,
auto-discovery notes, and a logs section.
Add InstanceDiedError exception for subprocess death detection and
check_alive() sync method that polls the subprocess and raises when
the process has exited. Purely additive — no existing behavior changed.
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
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.

2 participants