fix(conversations): initialize LastRound so fresh conversations survive cleanup - #640
Open
MorquinDevlar wants to merge 1 commit into
Open
Conversation
…ve cleanup getConversation has a 2% chance per call of deleting any conversation more than 10 rounds stale, measured as rounds since LastRound. New conversations left LastRound at zero while the round counter starts at 1314000, so a brand-new conversation was always eligible: any lookup that won the maintenance roll deleted it before its first action fired. This is also why TestAttemptConversation_UsesPluginFile fails on roughly 2% of CI runs - the test creates a conversation and immediately looks it up. LastRound now starts at the current round, matching StartRound. Adds a deterministic regression test.
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.
The bug
getConversationhas a 2% chance per call of running maintenance that deletes any conversation more than 10 rounds stale, measured asrNow - LastRound. ButAttemptConversationnever setLastRound, and the round counter starts at 1,314,000 - so every freshly created conversation looked ancient until its firstNextActionscall. Any lookup that won the maintenance roll deleted a conversation that was just created.Two consequences:
TestAttemptConversation_UsesPluginFilecreates a conversation and immediately looks it up, so it fails on ~2% of runs regardless of what a PR changes. Example: the initial Go Tests failure on perf(users): sync the user index incrementally, carry character names in index records #639 (this run), which passed untouched on rerun. Reproducible on master withgo test -run TestAttemptConversation_UsesPluginFile -count 300 ./internal/conversations/.The fix
LastRoundnow starts at the current round, matchingStartRound. Dialogue timing is unchanged: conversations are created by the mobconversecommand during a round, and actions are pulled by theNewRound_IdleMobshook on the following round, so the same-round guard inNextActionsnever bit in the real flow.Includes a deterministic regression test. Verified with 300 consecutive
-countruns, which reliably failed on master before the fix.