Skip to content

fix: record events in mjai_log for apply_event/observe_event (#184) - #187

Merged
smly merged 6 commits into
mainfrom
fix/record-events-mjai-log
Mar 17, 2026
Merged

fix: record events in mjai_log for apply_event/observe_event (#184)#187
smly merged 6 commits into
mainfrom
fix/record-events-mjai-log

Conversation

@smly

@smly smly commented Mar 17, 2026

Copy link
Copy Markdown
Owner

Fixes #184

apply_event() and observe_event() updated game state correctly but did not push events to mjai_log, causing get_viewer() to show only the constructor's initial events (typically just start_game, start_kyoku, tsumo).

@smly smly self-assigned this Mar 17, 2026
@smly smly added the bug Something isn't working label Mar 17, 2026
@smly
smly marked this pull request as ready for review March 17, 2026 10:55
@smly
smly requested a review from Copilot March 17, 2026 10:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a viewer/debugging regression where MJAI events applied via the Python API (apply_event() / observe_event()) updated state but were not recorded into mjai_log, causing get_viewer() to show only constructor-emitted events.

Changes:

  • Push applied/observed MJAI events into the underlying game state’s mjai_log (and per-player logs) so the viewer can render full replays.
  • Clear constructor-emitted logs when a user replays from a new start_game event.
  • Add regression tests covering mjai_log accumulation for both 4P and 3P, and for both apply_event() and observe_event().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
riichienv-python/src/env.rs Records applied/observed MJAI events into mjai_log and clears stale constructor logs on start_game.
tests/env/test_apply_event_mjai_log.py Adds regression tests to ensure mjai_log is populated correctly for replay + viewer use.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread riichienv-python/src/env.rs Outdated
Comment thread riichienv-python/src/env.rs Outdated
Comment thread riichienv-python/src/env.rs
Comment thread tests/env/test_apply_event_mjai_log.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes MJAI event logging for the Python RiichiEnv wrapper so that events applied via apply_event() / observe_event() are recorded into mjai_log, enabling get_viewer() to display replayed/custom logs correctly (regression for #184).

Changes:

  • Add shared Rust-side helper logic to “parse → (optional start_game reset) → apply → push into mjai_log” for both apply_event() and observe_event().
  • Add parse_mjai_event() helper to avoid redundant JSON parsing.
  • Add Python regression tests covering mjai_log recording for both 4P and 3P, for both apply_event() and observe_event().

Reviewed changes

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

File Description
riichienv-python/src/env.rs Refactors event parsing and ensures apply_event()/observe_event() push MJAI events into the core state’s mjai log (with start_game log reset behavior).
tests/env/test_apply_event_mjai_log.py Adds regression tests asserting that events applied/observed are recorded into env.mjai_log in correct order and with expected fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread riichienv-python/src/env.rs
Comment thread riichienv-python/src/env.rs
@smly
smly merged commit f45c7cb into main Mar 17, 2026
11 checks passed
@smly
smly deleted the fix/record-events-mjai-log branch March 17, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] IPYNB viewer doesn't work for custom logs

2 participants