fix: emit prerequisite events deepest-first - #55
Merged
Conversation
Move the current flag's handleEventsForEval call to after the recursive prerequisite walk in variationDetail. This aligns the Roku client SDK with the other client SDKs (.NET, JS, Flutter, Android, iOS, C++) and every server SDK (Go, .NET, JS, Java, Python, Ruby, C++), all of which emit prerequisite feature events before the parent's event. Prior to this change, evaluating flagA with prereq flagB emitted [flagA, flagB]. It now emits [flagB, flagA], matching the cross-SDK convention. Individual events are self-describing and LD's event ingestion does not depend on intra-batch ordering, so this is a correctness-for-parity change rather than a functional bug fix. Also updates the four cycle-detection tests in Test__Client.brs to assert deepest-first ordering.
keelerm84
approved these changes
Jul 23, 2026
Reconciles the deepest-first prereq-event ordering with the LaunchDarklyClientSharedPrivateFunctions encapsulation that landed in main. # Conflicts: # rawsrc/LaunchDarklyClient.brs
tanderson-ld
added a commit
that referenced
this pull request
Jul 23, 2026
🤖 I have created a release *beep* *boop* --- ## [2.4.1](2.4.0...2.4.1) (2026-07-23) ### Bug Fixes * add defensive cycle guard to prerequisite evaluation ([ceab01d](ceab01d)) * add defensive cycle guard to prerequisite evaluation ([#54](#54)) ([d4d325f](d4d325f)) * emit prerequisite events deepest-first ([d729aa7](d729aa7)) * emit prerequisite events deepest-first ([#55](#55)) ([3fe85d1](3fe85d1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Diff is only version manifest and changelog; no runtime code changes in this PR. > > **Overview** > **Release 2.4.1** — bumps the package version in `.release-please-manifest.json` from **2.4.0** to **2.4.1** and adds the **2.4.1** section to `CHANGELOG.md` (Release Please). > > The changelog records two prerequisite-evaluation fixes already landed on main: a **cycle guard** so cyclic prerequisite graphs stop recursing instead of running without bound, and **deepest-first** ordering when emitting prerequisite feature events (aligned with client contract expectations). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b6d0e87. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
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
2 participants
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.
Summary
Aligns Roku's prerequisite event emission order with the rest of the LaunchDarkly SDK family.
variationDetailinLaunchDarklyClient.brsnow emits the current flag's feature event after the recursive prerequisite walk instead of before it. For a flagAwith prerequisiteB, events now land as[B, A](matching every other client and server SDK) rather than[A, B].Tracker: SDK-2745. Stacked on #54.
Motivation
A survey during the cycle-detection work in #54 found that Roku was the only outlier across all client SDKs (.NET, JS, Flutter, Android, iOS, C++) and every server SDK (Go, .NET, JS, Java, Python, Ruby, C++) — all of which emit prerequisite events before the parent's event. LaunchDarkly's event ingestion does not depend on intra-batch ordering, so the pre-existing behavior wasn't a functional bug, but the divergence is a cross-SDK anomaly worth correcting.
Changes
rawsrc/LaunchDarklyClient.brs: move thehandleEventsForEval(launchDarklyLocalState)call to after the prerequisite recursion block.src/test/source/tests/Test__Client.brs: update the four cycle-detection tests to expect deepest-first ordering (e.g.,[flagB, flagA]for a two-cycle evaluating A).Test plan
TestSuite__Clientcases, including the fiveCycleDetection_*tests).events/summary events/prerequisites/handles cycles/*andevents/prerequisite events handle cycles/*case.[B, A].Note
Low Risk
Single reorder of event emission in
variationDetail; flag evaluation and cycle detection are untouched, and ingestion does not rely on intra-batch event order.Overview
variationDetailinLaunchDarklyClient.brsnow callshandleEventsForEvalafter walking prerequisites instead of before. Prerequisite feature events are emitted deepest-first (e.g.BthenAwhenAdepends onB), matching other LaunchDarkly client and server SDKs; Roku had been parent-first.The four
CycleDetection_*tests inTest__Client.brswere updated to assert the new event key order for two-cycles, three-cycles, and the diamond graph (including duplicateDevents on separate paths). Evaluation values and cycle-guard behavior are unchanged.Reviewed by Cursor Bugbot for commit 3de34ff. Bugbot is set up for automated code reviews on this repo. Configure here.