fix(run-engine): decrement totalWeight in #weightedShuffle after splice (#4001)#4008
fix(run-engine): decrement totalWeight in #weightedShuffle after splice (#4001)#4008gtx20060124-bot wants to merge 2 commits into
Conversation
|
|
Hi @gtx20060124-bot, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughTwo independent bug fixes are applied across separate internal packages. In ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| // Add selected item to result and remove from items | ||
| result.push(items[index].envId); | ||
| totalWeight -= items[index].weight; |
There was a problem hiding this comment.
🚩 totalWeight fix matches existing pattern in #weightedRandomQueueOrder but variable is declared const
The addition of totalWeight -= items[index].weight; at line 229 is the correct fix for the weighted shuffle algorithm — without it, this._rng() * totalWeight on line 217 uses a stale total after items are removed, biasing selection toward later items in the array. The analogous method #weightedRandomQueueOrder at lines 293-310 correctly uses let totalWeight and decrements it at line 309. However, totalWeight in #weightedShuffle is declared as const at line 212, while the new line 229 attempts to reassign it. The declaration should be let to match the pattern in the analogous method.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes #4001
#weightedShufflewas not decrementingtotalWeightafter splicing an item, causing the weighted random selection to produce non-uniform ordering when biases are enabled.The sibling methods
#weightedRandomQueueOrderand#selectTopEnvsboth decrement totalWeight correctly — this was a copy-paste omission.Changed: moved
totalWeight -= items[index].weightbeforesplice()to use the correct index.