fix(arena): reset OHH historian builder after Round::Complete#267
Merged
elliottneilclark merged 1 commit intomasterfrom Apr 15, 2026
Merged
fix(arena): reset OHH historian builder after Round::Complete#267elliottneilclark merged 1 commit intomasterfrom
elliottneilclark merged 1 commit intomasterfrom
Conversation
Both `OpenHandHistoryHistorian` and `OpenHandHistoryVecHistorian` consumed their inner `HandHistoryBuilder` via `take()` when a hand completed and never re-initialised it. A historian instance reused across hands (e.g., passed to `HoldemCompetition::run` as part of a cloned context) started returning `UnableToRecordAction` on every subsequent call — silently dropping every subsequent hand's history. Store the `ConverterConfig` on `self` so the builder can be re-constructed after `build()` consumes the old one, and reset `self.builder = Some(HandHistoryBuilder::new(config.clone()))` after appending/pushing the completed hand. Replace the two tests that previously pinned the buggy "errors after completion" behaviour with regression tests asserting both historians can record multiple consecutive hands.
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
1 participant
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.
Both
OpenHandHistoryHistorianandOpenHandHistoryVecHistorianconsumed their inner
HandHistoryBuilderviatake()when a handcompleted and never re-initialised it. A historian instance reused
across hands (e.g., passed to
HoldemCompetition::runas part of acloned context) started returning
UnableToRecordActionon everysubsequent call — silently dropping every subsequent hand's history.
Store the
ConverterConfigonselfso the builder can bere-constructed after
build()consumes the old one, and resetself.builder = Some(HandHistoryBuilder::new(config.clone()))afterappending/pushing the completed hand. Replace the two tests that
previously pinned the buggy "errors after completion" behaviour with
regression tests asserting both historians can record multiple
consecutive hands.