Skip to content

FFL-3459: Support flagSelection on Flags precompute requests - #4031

Merged
mark-caii merged 38 commits into
developfrom
mark.cai/FFL-3459
Nov 3, 2026
Merged

mark-caii merged 38 commits into
developfrom
mark.cai/FFL-3459

Conversation

@mark-caii

@mark-caii mark-caii commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds FlagsConfiguration.Builder.flagSelection(tags, keyPatterns) so an app loads only the flags that match the given tags or key patterns. Spec: FFL-3459. It ports the browser SDK reference implementation, DataDog/openfeature-js-client#426, to Android's precompute-only flow. The spec numbers its items and acceptance criteria like the browser spec and lists each intentional difference as P1–P16.

Public API

fun flagSelection(tags: List<String>?, keyPatterns: List<String>?): Builder
  • A null list (for example from Java) counts as absent, as in the browser SDK. A later call replaces an earlier one. FlagsConfiguration.copy() keeps the selection, and its public signature is unchanged.
  • The KDoc states the matching rules (exact, case-sensitive tags; whole-key globs where only * is special; any tag or any key pattern), the limits, the fallback to the full configuration, FLAG_NOT_FOUND for flags outside an applied selection, that older servers or disabled gates return the full configuration without a warning, and that the selection applies to every FlagsClient and flags cached under a different selection are not loaded at startup. The module README has a matching "Load only selected flags" section.
  • The names match iOS (Flags.FlagSelection(tags:keyPatterns:), [FFL-3458] Flags: support flagSelection (tags + key patterns) dd-sdk-ios#3344) and the browser SDK (flagSelection: { tags, keyPatterns }).

Behavior

  • Canonicalization (internal/selection/FlagSelection.kt, one canonicalizer for the request and the cache comparison): trim the Unicode White_Space set (an explicit set, not Kotlin trim()), drop empty values, dedupe, and sort by unsigned UTF-8 bytes (not UTF-16 compareTo). If both lists are empty afterwards, there is no selection.
  • Validation runs once, when the Flags feature is created, in this order: malformed_selection (a null element, a non-string element from a raw Java list, or an unpaired UTF-16 surrogate), too_many_values (more than 31 per list after dedupe), invalid_key_pattern and invalid_tag (more than 200 UTF-8 bytes after trimming).
  • Dropped selection: no selection is sent and the full configuration loads. One ERROR/USER log, flagSelection was not sent (<reason>: <limit>). Loading the full flag configuration., and one TELEMETRY debug event, Flag selection dropped with only reason, are emitted once per Flags.enable (P3). After that, a dropped selection behaves like no selection.
  • Request: the precompute POST carries data.attributes.flag_selection: { tags, key_patterns } through the generated PrecomputedAssignmentsRequest model. Both arrays are always present and in canonical order, including with useCustomFlagEndpoint. Without a selection, the body is unchanged.
  • Ignored header: DD-Flag-Selection-Ignored is read only when the request carried a selection and the response was successful, before the body is parsed. A non-empty value is reported once per accepted response, after the configuration is installed: one ERROR/USER log, flagSelection was ignored by the server (<reason>). Loaded the full flag configuration., and one Flag selection ignored telemetry debug event with reason. The configuration is installed as returned. A response is accepted when its body maps to a configuration and passes the obfuscation filter; a body that fails to parse or is rejected, or a response that a newer context supersedes before it is installed (experimentation opt-out), reports nothing (P16). The browser currently reports the reason before parsing. An empty value is treated as absent. A proxy behind useCustomFlagEndpoint must forward the header (P2).
  • Ignored reason allowlist (P15): the reason is reported only if it exactly matches (case-sensitive, at most 32 characters) one of malformed_selection, too_many_values, invalid_key_pattern, invalid_tag or tag_load_failed. Any other non-empty value is reported as unknown, so the raw header value never reaches the USER log or telemetry. iOS uses the same rule; the browser SDK currently reports the value verbatim.
  • Cache (P6): flags-state.json has an optional flagSelection: { tags, keyPatterns }, persisted through the generated FlagsState model. DefaultFlagsRepository uses persisted state only when its selection equals the configured one, and saves the configured selection with each install. Selected state is stored under a selection variant of the experimentation opt-out mode's key: flags-selection-state-<instance>, flags-selection-immediate-v1-state-<instance> or flags-selection-future-v1-state-<instance>. SDK versions without flag selection read only the unselected key of their mode (flags-state-, flags-immediate-v1-state- or flags-future-v1-state-<instance>, with any DataStore version, ignoring unknown fields), so a rollback cannot serve a selected subset as the full configuration. Unselected state keeps its existing key, so existing caches stay valid and older SDKs can still read it. Creating an instance removes the sibling key of the same mode, so one persisted state remains per instance and mode.
  • Experimentation opt-out (P7): a selection works alongside FlagsClient.Builder.experimentationOptOutMode. When the mode is enabled and the context carries the opt-out attribute, the precompute POST carries both flag_selection and evaluation_options; the request factory that opt-out clients build carries the configured selection.
  • Empty configuration (P13): hasConfiguration() and hasConfigurationForContext() count a valid configuration with zero flags as loaded. A selection that matches nothing gives Ready, evaluations return the default with FLAG_NOT_FOUND, and a later failed fetch or initialization timeout for the same context reports Stale.

Motivation

Large orgs can configure many flags. Mobile apps should be able to fetch only the flags they use.

Additional Notes

Telemetry volume. The Flag selection dropped and Flag selection ignored telemetry events use onlyOnce, so each is sent at most once per process, and only the first reason in a process is reported. The USER error log is not deduplicated and still reports every ignored response (AC8).

Out of scope (as in the spec)

  • Rules-based GET, query encoding, the URL-length limit and CORS (P1, P2).
  • Running the shared flag-selection/ vectors directly (P10). This repo has no submodules or vendored cross-repo fixtures. Every vector case that the Kotlin API can express has a hand-written equivalent (see below).
  • A reliability/single-fit end-to-end test, React Native / Flutter / KMP wrappers, changing the selection at runtime, public docs, initial flags, backend changes, and CHANGELOG.md (updated at release time, P12).

Local verification

At 8db220a97 (after merging develop with the experimentation opt-out), 2026-10-30.

Unit and static checks

  • ./gradlew :features:dd-sdk-android-flags:testDebugUnitTest: 920 tests, 0 failures. ktlintFormat (no changes), detekt and checkApiSurfaceChanges pass at the head, and apiDump produces no diff.
  • Experimentation opt-out with a selection: DefaultFlagsRepositoryTest checks the read key, the written key and the removed sibling key for every opt-out mode with and without a selection, and that selected state is never written to the unselected key. PrecomputedAssignmentsRequestFactoryTest checks that the body carries both flag_selection and evaluation_options.
  • testReleaseUnitTest (722 tests), checkCompilerMetadataChanges, checkAarMetadataInfoChanges and checkTransitiveDependenciesList pass at db50cef. CI runs the full set on the head.
  • Rollback protection: a new DefaultFlagsRepositoryTest case failed before f68612f, because selected state was written to flags-state-<instance>, the key older SDKs read. It passes now.
  • Mutation check: with FlagsFeature passing a null selection to the request factory, all 692 tests passed at b754a44. That mutant now fails. A non-string element from a raw Java list threw ClassCastException from Flags.enable before db50cef; it is now malformed_selection.

Shared vectors (DataDog/ffe-system-test-data@e8777633, flag-selection/selection-vectors.json): FlagSelectionCanonicalizerTest.M match the shared selection vector has one row for each of the 38 cases with an sdk expectation, in file order and under the same name. I checked every row's input and expected result against the file with a temporary loader test (not committed): 38 of 38 identical, and the canonicalizer returns the expected result for all 38. An expected empty selection is no selection on Android. The other 6 cases need a raw query string (undecodable values, repeated or percent-encoded parameter names, and malformed-first precedence through the query), so they are N/A (P1, P4). Malformed-first precedence is covered by the malformed before ... cases instead.

Android flag-selection e2e against staging (DD_SITE=datad0g.com, DD_ENV=staging), generated by scripts/e2e-android-local.sh in ddoghq/ffe-dogfooding

  • Date: 2026-10-30 14:03 EDT
  • SDK: DataDog/dd-sdk-android 8db220a97b39d25f797fe840014c6ef6f04fd515, published to a local Maven repository as 3.17.0-SNAPSHOT
  • Flags AAR sha256: 0f17d2340d37f059b8dc05361dc10dcf0a71730f96e202e33d0736e3eb79600b
  • Harness: ddoghq/ffe-dogfooding fa7acdc9e59f9b0564e76908d3512d99d83f1418; /health reported dd-sdk-android-flags 3.17.0-SNAPSHOT
  • Emulator: API 34 (ffe_api34), fresh app data for every case, subject ffl3459-e2e-subject
  • Expected sets: every flag with a requested tag or whose whole key matches a glob, from GET /api/v2/feature-flags during this run (8872 flags), intersected with this run's E1 baseline (828 of 828 loaded keys decodable)
# Case Expected Loaded Persisted state Selection logs Result
E1 No selection 828 828 flags-state-default, no selection none PASS
E2 Tag team:feature-flagging 23 23, same keys flags-selection-state-default, canonical none PASS
E3 Tags team:feature-flags, team:feature-flagging-backend 21 21, same keys flags-selection-state-default, canonical none PASS
E4 abin-* 11 11, same keys flags-selection-state-default, canonical none PASS
E5 *-v2 1 1, same key flags-selection-state-default, canonical none PASS
E6 *flag* 135 135, same keys flags-selection-state-default, canonical none PASS
E7 Tag team:feature-flagging + abin-* 29 29, same keys flags-selection-state-default, canonical none PASS
E8 Zero match (tag ffe-e2e-no-such-tag) 0, Ready 0, PROVIDER_READY; a flag evaluates to the default with FLAG_NOT_FOUND flags-selection-state-default, canonical none PASS
E9 32 tags 828, 1 error 828, same keys flags-state-default, no selection 1 too_many_values drop error PASS
E10 201-byte tag 828, 1 error 828, same keys flags-state-default, no selection 1 invalid_tag drop error PASS
E2c "\u3000team:feature-flagging ", a duplicate and "" 23 23, same keys flags-selection-state-default, {"tags":["team:feature-flagging"],"keyPatterns":[]} none PASS
E11 31 tags and 31 key patterns of 200 bytes each (server ignores) 828, 1 error 828, same keys as E1 flags-selection-state-default, canonical 1 ignored by the server (too_many_values) error PASS

Result: PASS (12 of 12 cases passed)

In E11, staging returned DD-Flag-Selection-Ignored: too_many_values with a valid body, so the response was accepted: the SDK installed and cached the full configuration under the selection and logged exactly one ignored error. No other case returned the header. Telemetry events do not appear in logcat, so the unit tests assert them. Staging data drifts between runs, so counts compare only within one run. The harness runs with the experimentation opt-out disabled, so the e2e keys are the DISABLED ones; unit tests cover the other modes.

Cache cases at f68612f (flags AAR sha256 09e6376c…cc4d47bb, same harness setup; A = tag team:feature-flagging)

# Steps Result Persisted keys
C1 E1, fresh data PROVIDER_READY, 831 flags flags-state-default only (no selection)
C2 E2 (A), fresh data PROVIDER_READY, 22 flags, same keys as expected flags-selection-state-default only, canonical A
C3 E2c, fresh data PROVIDER_READY, 22 flags flags-selection-state-default only, canonical A
C4 Relaunch from C3 with no selection PROVIDER_READY, 831 flags flags-state-default only (selected state removed)
C5 Relaunch from C4 with A PROVIDER_READY, 22 flags flags-selection-state-default only (unselected state removed)
C6 Relaunch offline with A PROVIDER_STALE (cache for A used) unchanged
C7 Relaunch offline with tag team:feature-flags PROVIDER_ERROR (cache for A not used) unchanged
C8 Relaunch offline with no selection PROVIDER_ERROR (no unselected state) none (selected state removed)
C9 Persist A online, then install the harness pinned to the released 3.14.1 over it and start it offline 3.14.1 reports PROVIDER_ERROR: it reads no state and does not serve the 22-flag subset flags-selection-state-default only
Logcat for E9, E10 and E11
E/Datadog: [_dd.sdk_core.default]: flagSelection was not sent (too_many_values: each list accepts at most 31 values). Loading the full flag configuration.
E/Datadog: [_dd.sdk_core.default]: flagSelection was not sent (invalid_tag: each tag is at most 200 UTF-8 bytes). Loading the full flag configuration.
E/Datadog: [_dd.sdk_core.default]: flagSelection was ignored by the server (too_many_values). Loaded the full flag configuration.
Spec acceptance criteria and evidence
AC Evidence
1 Selection sent, only matching flags E2–E7 key sets; PrecomputedAssignmentsRequestFactoryTest body tests; FlagsFeatureTest asserts the selection reaches the factory
2 No selection or empty after canonicalization: body unchanged M not add flag_selection W create(); FlagsFeatureTest no values after canonicalization (empty lists, whitespace-only values, null lists); E1
3 Canonicalization, shared vectors FlagSelectionCanonicalizerTest (White_Space set, U+0085 and U+3000 trimmed, interior whitespace kept for all 25 White_Space code points, U+FEFF, U+200B and U+001C kept, UTF-8 order); 38 of 38 vector rows; E2c
4 Over a limit 31/32 values and 200/201 bytes with 2-, 3- and 4-byte characters, measured after trimming; E9, E10
5 Type-invalid input lone high and low surrogates, reversed pair, null elements, non-string elements (malformed); null lists (absent); a well-formed pair (accepted)
6 Error order every pair: malformed before too many values / invalid key pattern / invalid tag, too many values before invalid key pattern / invalid tag, invalid key pattern before invalid tag
7 Drop reported once per Flags.enable FlagsFeatureTest drop test (one log and one telemetry event at onInitialize); the fetch path logs no selection message (EvaluationsManagerTest)
8 Ignored: full config, one log per accepted response; nothing reported when the body fails to parse or is rejected (P16) EvaluationsManagerTest ignored-reason tests, including ignored reason, invalid body, ignored reason, unsupported encoding and ignored reason, superseded request; E11 against staging
9 tag_load_failed reported; other values as unknown (P15); empty value ignored FlagSelectionIgnoredReasonTest (each allowed value kept; unknown value, case mismatch, over 32 characters, control and non-ASCII characters give unknown); downloader tests for allowed values, unknown values and empty ignored header; header name looked up without case sensitivity; E11 passes a real too_many_values through
10 No selection: header not read no selection sent, ignored header
11 Telemetry carries only the reason verifyLog with additionalProperties = mapOf("reason" to ...) and onlyOnce = true for drop and ignore; the USER log is verified with onlyOnce = false; the ignored reason is sanitized before logging
12 Zero match: Ready, FLAG_NOT_FOUND, then Stale DefaultFlagsRepositoryTest empty-configuration tests; failed fetch after an empty configuration gives Stale; E8
13 Different selections never share state; same canonical selection recovers it repository tests: different selection, legacy state without the field, state saved under a selection with none configured, and [" checkout", "checkout"] recovering ["checkout"]; FlagsClientTest wiring
14 Changed selection: previous state not used at startup the startup-path repository tests; cache cases C4–C8; rollback case C9
15 Experimentation opt-out (P7) M send flag_selection and evaluation_options W create() { selection and opt-out }; M use the opt-out mode key for the selection W init and setFlagsAndContext() (6 mode × selection cases)
16 End to end the table above
17 CI unit tests, API surface, detekt and ktlint jobs on this head
  • Device run by @aarsilv at 6766ce9 (API 36): 32 selection cases and 3 offline cache cases pass. The cache cases above repeat the offline checks on the head.

Open questions for reviewers

  • Default parameters for flagSelection(tags, keyPatterns). Both lists are required, so a Kotlin caller with tags only writes flagSelection(listOf("x"), null). The repo guidance avoids unneeded default arguments; iOS uses defaults. Adding = null defaults later is not a breaking change.
  • Not from this PR: a large persisted cache can take longer than PERSISTENCE_LOAD_TIMEOUT_MS (100 ms) to load on cold start, which reports Error before the cache is available.

Release notes

  • A valid configuration with zero flags now counts as loaded. A later failed fetch for the same context reports Stale instead of Error, for all apps, including through the OpenFeature provider (ProviderStale instead of ProviderError).
  • The first launch after adopting or changing a selection discards the existing cache, so that launch has no cached flags until the fetch completes.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

Add FlagsConfiguration.Builder.flagSelection(tags, keyPatterns). The
selection is canonicalized and validated like Compute and ffe-service,
sent as flag_selection on the precompute POST, and stored with the
persisted state so a cache fetched with another selection is ignored.
Report DD-Flag-Selection-Ignored, and treat a valid empty configuration
as loaded so a later failed fetch reports Stale.

Ref: FFL-3459
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Pipelines  Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 96.65%
• Overall Coverage: 72.06% (+0.10%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 198c684 | Docs | View more details | Give us feedback!

Persist the flag selection through the generated FlagsState model and
re-apply selection filtering, empty-config handling and the ignored
header on top of develop's obfuscation changes.

Ref: FFL-3459
The downloader reports DD-Flag-Selection-Ignored only for requests that
carried flag_selection, and passes the value through verbatim.

Ref: FFL-3459
Builder.flagSelection takes List<String>? for tags and key patterns. A
null list is treated as absent, matching the browser SDK; null elements
inside a list remain malformed_selection.

Ref: FFL-3459
State that limits count values after dedupe, that flags outside the
selection return the default with FLAG_NOT_FOUND, and when the server
returns the full configuration instead.

Ref: FFL-3459
Add hand-written canonicalizer cases for the selection vectors with sdk
expectations that the existing tests did not cover.

Ref: FFL-3459
@aarsilv

aarsilv commented Oct 27, 2026

Copy link
Copy Markdown

🤖 Comment from Claude working with Aaron Silverman (aarsilv):

Heads up: I ran the pending device checks for this branch on an emulator: the harness cases below and both instrumented suites. Every harness case behaved as described, and the instrumented suites pass except one test that failed once and passed in two reruns. No change requested.

Head 6766ce9 published to a local Maven repository and built into a small test app on an Android 16 (API 36) emulator. The app calls FlagsConfiguration.Builder.flagSelection(tags, keyPatterns), fetches from a live test org whose flags carry tags for these cases, and evaluates 8 test flags per case.

Results — 32 selection cases and 3 cache cases pass
Area Cases What was checked
Matching 14 exact and case-sensitive tags, OR of tags, OR of tags and keys, exact key, * globs, a key with spaces, literal ?, NFC vs NFD, an emoji tag, reserved characters, a 200-byte tag, a tag present only on an archived flag (all 8 test flags FLAG_NOT_FOUND)
Canonicalization 7 trimmed (including U+00A0 and U+3000), duplicated and empty values, mixed 3- and 4-byte UTF-8 values, 31 values, 32 raw values that dedupe to 31; the expected test flags resolve
No selection, zero matches 2 all 8 test flags resolve; Ready with all 8 FLAG_NOT_FOUND
Dropped locally 8 32 tags, 32 key patterns, 201-byte tag, 201-byte multibyte tag, 201-byte key pattern, key pattern checked before tag, a null element, an unpaired surrogate; all 8 test flags resolve and a not sent (<reason> error log appears
Ignored by the server 1 a valid but very large selection: all 8 test flags resolve and an ignored by the server (too_many_values) error log appears
Cache 3 written under A, offline relaunch under A gives Stale, under B or no selection gives Error

The flag requests go to the SDK's HTTPS endpoint, so this run checked evaluation results and logs, not the request body or the full response. The cache cases use airplane mode and a fixed boot subject, and judge the first Ready, Stale or Error after launch.

Instrumented suites — 81/81 and 74/75; the one failure passed in two reruns
Suite Result
:reliability:core-it:connectedDebugAndroidTest 81 / 81
:instrumented:integration:connectedDebugAndroidTest 74 / 75

The failure was CrossFeatureTest.logsMustNotLinkToTracesWhenCalledBefore, a 30-second ConditionWatcher timeout. It passed in two reruns of that test alone. It covers logs and traces, which this PR does not touch.

API names across SDKs — the description lists iOS parity as open
SDK Configuration
Android (this PR) FlagsConfiguration.Builder().flagSelection(tags, keyPatterns)
iOS (DataDog/dd-sdk-ios#3344) Flags.Configuration.flagSelection = Flags.FlagSelection(tags:keyPatterns:)
Browser (DataDog/openfeature-js-client#426) new DatadogProvider({ flagSelection: { tags, keyPatterns } })

The names match: flagSelection, tags, keyPatterns. On the 30 cases that all three can express, the iOS and browser builds resolved the same test flags as this branch.

Large offline caches — why the cache cases use one-flag selections

With the whole test environment cached, an offline cold start reported Error instead of Stale, with or without a selection. With 1 or 22 flags cached it reported Stale. DefaultFlagsRepository waits PERSISTENCE_LOAD_TIMEOUT_MS = 100L for the persisted state, and that constant predates this PR. I suspect the wait is too short for a large cache, but I did not measure the load time or run the same check on develop.

Treat an empty DD-Flag-Selection-Ignored value as absent, as the browser
SDK does, so it no longer logs a server ignore with an empty reason.
Clarify in the flagSelection KDoc that the byte limit applies after
trimming, that unpaired UTF-16 surrogates are malformed, and that
FLAG_NOT_FOUND applies when the server applies the selection.

Ref: FFL-3459
Verify that FlagsClient passes the configured selection to the
repository, so a persisted state saved under that selection is served.
Verify that a persisted state saved without a selection is discarded once
a selection is configured. Extend the trim test to every Unicode
White_Space character.

Ref: FFL-3459
Dedupe selection values before sorting, compute each UTF-8 sort key once,
and skip sorting a list that is already over the value limit. Pass the
response mapper by reference. Drop positional null arguments in tests
that only restated the default obfuscation value.

Ref: FFL-3459
@mark-caii
mark-caii marked this pull request as ready for review October 27, 2026 19:53
@mark-caii
mark-caii requested review from a team as code owners October 27, 2026 19:53
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T18:30:19.095991Z 198c684 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d05334269

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@datadog-datadog-prod-us1 datadog-datadog-prod-us1 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: PASS

More details

Selection validation, request encoding, and persisted-cache matching are consistent with the intended behavior, including treating a zero-match configuration as loaded. The completed static review identified no actionable defects.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 2d05334

Send the canonical selection through the generated request model that develop introduced.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b754a44011

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@mark-caii mark-caii left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of b754a44 (this branch with develop merged at ba1ae26). Scope: correctness, performance, tests, operations, design, and a search for missed callers. I ran mutation checks against the JVM unit tests of :features:dd-sdk-android-flags.

Three inline findings follow: two test gaps (P2) and one comment wording nit (P3).

Heads up: the description needs two updates. They do not need a code change.

  • It says that iOS parity of the API is not verified. The names match iOS (Flags.FlagSelection(tags:keyPatterns:)) and the browser SDK (flagSelection: { tags, keyPatterns }).
  • The local and staging results are for 2d05334 and 6766ce9. The develop merge moved the request body to the generated PrecomputedAssignmentsRequest model, so the end-to-end run must be repeated at the final head.
Checked with no finding
  • The merge: flag_selection is now a property of the generated request model, with both arrays required. A mutant that drops it fails 3 tests in PrecomputedAssignmentsRequestFactoryTest.
  • Canonicalization and validation run once, when FlagsFeature is created. They are not on the evaluation path.
  • The ignored header is read only when the request carried a selection and the response was successful. An empty value gives no reason.
  • The persisted state is used only when its stored selection equals the configured one. A state saved before this change (no field) is not used once a selection is configured.
  • hasFlags() and hasLoadedFlagsForContext() keep the obfuscation readability check for an empty configuration.
  • Telemetry events carry only the reason. Selection values are not logged or sent in telemetry.
  • Public API: one new builder method, and copy() keeps its JVM signature. checkApiSurfaceChanges, ktlintCheck and detekt pass.

Each existing thread gets a separate reply.

@mark-caii
mark-caii requested a review from a team as a code owner October 28, 2026 14:48

@datadog-datadog-prod-us1 datadog-datadog-prod-us1 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: PASS

More details

Selection handling across requests, cache matching, and empty configurations is consistent with the stated requirements. The completed static review produced no reportable findings, and human PR discussion did not change that conclusion.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 1cc6771

List all 38 cases that have an sdk expectation in the shared vectors, in file order, so each one maps to one test row.
hasFlags() and hasLoadedFlagsForContext() report whether a configuration
is installed, including a valid one with zero flags. Name them
hasConfiguration() and hasConfigurationForContext() so call sites read
as intended.
Log the DD-Flag-Selection-Ignored reason and send its telemetry only after
the response maps to a configuration and passes the obfuscation filter,
just before it is installed. A body that fails to parse, or an encoded
configuration this source cannot use, no longer reports a reason for a
configuration that was never loaded.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 28e82cdfd6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Resolve the flag selection and experimentation opt-out conflicts:

- Each opt-out mode gets a selection variant of its cache key
  (flags-selection-state, flags-selection-immediate-v1-state,
  flags-selection-future-v1-state). SDK versions without flag selection
  read only the unselected key of their mode, so they never load selected
  state as a full configuration, and creating an instance removes the
  sibling key so one persisted state remains per instance.
- The per-client request factory used for opt-out clients now carries
  the flag selection, so a request sends both flag_selection and
  evaluation_options when both apply.
- Keep the hasConfiguration() names, read the ignored reason from the
  accepted response, and map the response body.
The dropped and ignored selection telemetry events now use onlyOnce, so
frequent fetches cannot flood telemetry. The USER error log is unchanged
and still reports every ignored response.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 00787acfd7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

With the experimentation opt-out, a newer context can supersede an
accepted response, which is then not installed. Log the ignored reason
after installFlags confirms the install, so the message never claims a
configuration that was not loaded.
@mark-caii
mark-caii requested review from maycmlee and satween October 30, 2026 18:17

@maycmlee maycmlee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question

Comment thread features/dd-sdk-android-flags/README.md Resolved
Resolve the DefaultFlagsRepository constructor conflict: keep develop's
change that makes instanceName and dataStore plain constructor
parameters, and treat the branch's flagSelection the same way, since it
is only read while building the persistence manager.
Say that the flag selection is what every FlagsClient shares, and use a
function reference for the empty ignored-reason header check.
maycmlee
maycmlee previously approved these changes Nov 2, 2026
typotter
typotter previously approved these changes Nov 3, 2026
@mark-caii
mark-caii dismissed stale reviews from typotter and maycmlee via 198c684 November 3, 2026 18:26
@mark-caii
mark-caii merged commit 9493bf6 into develop Nov 3, 2026
26 checks passed
@mark-caii
mark-caii deleted the mark.cai/FFL-3459 branch November 3, 2026 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human-review All agentic review feedback was addressed and PR is ready for the human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants