feat: add bandits support to precomputed client#98
Conversation
0c65905 to
5b4f090
Compare
Add getBanditAction() method to EppoPrecomputedClient for contextual multi-armed bandit support. Key changes: - Add PrecomputedBandit DTO with wire format and decoded versions - Add BanditResult return type with variation and optional action - Add BanditEvent for logging bandit assignments - Update Precompute to accept banditActions parameter - Update PrecomputedRequestor to send bandit_actions in payload - Add banditLogger parameter to client initialization - Implement bandit logging with deduplication via banditAssignmentCache - Add comprehensive tests for bandit functionality
66efb74 to
17bdc1e
Compare
There was a problem hiding this comment.
Pull request overview
Adds contextual multi-armed bandit support to the iOS SDK’s EppoPrecomputedClient so precomputed assignments can return both a variation (model) and a selected action, with optional logging and deduplication.
Changes:
- Introduces bandit DTOs (
PrecomputedBandit+ decoded variant), public API return type (BanditResult), and logging model (BanditEvent/BanditLogger). - Extends the precomputed fetch payload/response to send
bandit_actionsand parsebandits, including Base64 decoding inPrecomputedConfiguration. - Adds
getBanditAction()toEppoPrecomputedClientwith bandit-event logging + deduplication, plus comprehensive unit/wire tests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Sources/eppo/precomputed/EppoPrecomputedClient.swift | Adds getBanditAction() and bandit logging/deduplication infrastructure |
| Sources/eppo/precomputed/Precompute.swift | Extends Precompute to accept optional banditActions |
| Sources/eppo/precomputed/PrecomputedRequestor.swift | Adds bandit_actions to request payload and bandits to response decoding |
| Sources/eppo/precomputed/PrecomputedConfiguration.swift | Adds bandits to configuration model and Base64 decoding into decoded config |
| Sources/eppo/precomputed/PrecomputedConfigurationStore.swift | Adds decoded bandit lookup accessor |
| Sources/eppo/dto/PrecomputedBandit.swift | New wire-format and decoded bandit DTOs |
| Sources/eppo/dto/BanditResult.swift | New public return type for bandit action lookups |
| Sources/eppo/dto/BanditEvent.swift | New public logging event model and logger typealias |
| Sources/eppo/Constants.swift | Bumps SDK version constant |
| Tests/eppo/precomputed/EppoPrecomputedClientBanditTests.swift | New unit tests for bandit retrieval, logging, dedupe, concurrency |
| Tests/eppo/precomputed/PrecomputedConfigurationWireTests.swift | Adds wire-format tests for bandits using shared test data |
| Tests/eppo/precomputed/PrecomputedRequestorTests.swift | Updates request payload test to include new initializer arg |
| Tests/eppo/precomputed/PrecomputedTestHelpers.swift | Adds helpers to construct hashed/encoded bandit test fixtures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| subjectNumericAttributes[key] = doubleValue | ||
| } | ||
| } else { | ||
| if let stringValue = try? value.getStringValue() { |
There was a problem hiding this comment.
In logBanditAction, categorical subject attributes are extracted using getStringValue(), which throws for boolean values and arrays (and will silently skip them due to try?). Since Precompute.subjectAttributes can include booleans and string arrays (see EppoValue), this will drop valid attributes from bandit logging. Consider using EppoValue.toEppoString() for non-numeric values (and skipping only nulls) so booleans become "true"/"false" and arrays can be joined consistently.
| if let stringValue = try? value.getStringValue() { | |
| if let stringValue = try? value.toEppoString() { |
aarsilv
left a comment
There was a problem hiding this comment.
Nice job bringing precomputed bandits to iOS 💪 I appreciate the clean data models, easy to understand code, and automated tests. I have mostly minor comments but there is one big one for which I'm requesting changes: we still need to log the "regular" string assignment when getting a bandit action. This is is needed for when A/B testing bandit against status quo.
| actionProbability: Double, | ||
| optimalityGap: Double |
There was a problem hiding this comment.
Since action is optional, these should be too. If no action, these should be nil
| actionNumericAttributes = try container.decodeIfPresent([String: String].self, forKey: .actionNumericAttributes) | ||
| actionCategoricalAttributes = try container.decodeIfPresent([String: String].self, forKey: .actionCategoricalAttributes) | ||
| actionProbability = try container.decodeIfPresent(Double.self, forKey: .actionProbability) ?? 0.0 | ||
| optimalityGap = try container.decodeIfPresent(Double.self, forKey: .optimalityGap) ?? 0.0 |
There was a problem hiding this comment.
Defaulting to 0.0 is a bit misleading for actionProbability and incorrect for optimalityGap, can we make these optional and default to nil?
| let actionProbability: Double | ||
| let optimalityGap: Double |
|
|
||
| // First check if there's a bandit assignment for this flag | ||
| guard let bandit = configurationStore.getDecodedBandit(forKey: hashedFlagKey) else { | ||
| // No bandit, fall back to regular string assignment |
| // No action to log | ||
| return |
| XCTAssertEqual(result.variation, "hello world") | ||
| XCTAssertNil(result.action) |
| XCTAssertEqual(result.variation, "default") | ||
| XCTAssertNil(result.action) |
|
|
||
| // MARK: - Concurrent Access Tests | ||
|
|
||
| func testConcurrentBanditAccess() throws { |
| let hashedFlagKey = getMD5Hex(flagKey, salt: decodedConfig.decodedSalt) | ||
|
|
||
| // First check if there's a bandit assignment for this flag | ||
| guard let bandit = configurationStore.getDecodedBandit(forKey: hashedFlagKey) else { |
There was a problem hiding this comment.
prior to this, we'll want to log the "regular" assignment as well
| return BanditResult(variation: variation, action: nil) | ||
| } | ||
|
|
||
| // Get the variation from the bandit |
There was a problem hiding this comment.
thinking we expand this comment to be even more clear, something like "if the bandit was invoked, the variation value is the same as the bandit key"
Then again if we have the variation (which we'll need to log for "regular" assignment log) maybe just use that to set to not confuse people.
🎟️ Fixes N/A - New feature
📜 Design Doc: N/A
Motivation and Context
Add contextual multi-armed bandit support to the iOS SDK's
EppoPrecomputedClient, achieving feature parity with the Android and JS SDKs.Description
This PR adds the
getBanditAction()method and all supporting infrastructure for bandits in precomputed flags:New Files:
Sources/eppo/dto/PrecomputedBandit.swift- Wire format and decoded bandit DTOsSources/eppo/dto/BanditResult.swift- Public return type with variation and optional actionSources/eppo/dto/BanditEvent.swift- Logging model andBanditLoggertype aliasModified Files:
Precompute.swift- AddedbanditActionsparameter for specifying available actions per flagPrecomputedConfiguration.swift- Added bandits parsing and Base64 decodingPrecomputedRequestor.swift- Addedbandit_actionsto request payload andbanditsto response handlingPrecomputedConfigurationStore.swift- AddedgetDecodedBandit()methodEppoPrecomputedClient.swift- AddedgetBanditAction(),banditLogger, and deduplication viabanditAssignmentCacheKey Features:
getBanditAction(flagKey:defaultValue:)returnsBanditResultwith variation and optional actionBanditEventlogging with deduplicationHow has this been documented?
No external documentation changes required. The public API is straightforward:
getBanditAction(flagKey:defaultValue:)- similar to existing assignment methodsbanditLoggerparameter in initialization - similar to existingassignmentLoggerHow has this been tested?
Unit tests:
EppoPrecomputedClientBanditTests.swift- 11 tests covering:Wire format tests:
PrecomputedConfigurationWireTests.swift- 3 new tests using sharedprecomputed-v1.jsontest data:testBanditActionWithSharedTestDatatestBanditActionWithExtraLoggingSharedTestDatatestBanditActionForNonBanditFlagSharedTestDataEnd-to-end: Verified with EppoButtonClicker test app against live Eppo backend
banditdemo.mov
All tests passing:
swift test✅