feat(sdk): per-call enableTracking and experienceKeys on the feature entry points by abbaseya · Pull Request #63 · convertcom/android-sdk

abbaseya · 2026-09-16T19:09:53Z

Summary

runFeature and runFeatures gain the two per-call controls the docs already promised them, implementing SPEC-per-call-bucketing-attributes for the Android SDK.

enableTracking: Boolean = true — suppresses that call's bucketing exposure. On Android that means both the outbound network event and the in-process SystemEvents.BUCKETING fire, which is a deliberate divergence from the JavaScript SDK's "wire event only". It does not disable persistence: the sticky decision is written under either value, because persistence is gated on preview state alone.
experienceKeys: List<String>? = null — narrows a feature read to named experiences, so the ones a caller did not ask about are neither bucketed, persisted nor reported. An empty list means every experience, not none.

Both carry @JvmOverloads, both default to today's behaviour, and parameter order mirrors the experience pair.

Why this was worth doing

The consent playbook's per-feature row could not be followed. TrackingControl tells a developer to "pass enableTracking = false on the specific calls you want silent" — advice that works on the experience pair and had no expressible form on the feature pair, where runFeature/runFeatures handed FeatureManager a hard-coded literal true. A developer following the documented consent recipe on a feature flag shipped an exposure anyway.

And running-features documented experienceKeys as available. It was not. One runFeatures() call committed a sticky decision and reported an exposure for every experience carrying any declared feature, with no way to scope it.

The change is smaller than the tests suggest

19 lines of production code. FeatureManager already declared, defaulted and threaded enableTracking, so CAP-1 is two signatures and two call sites; CAP-2 adds one continue to an existing walk. No DataManager, BucketingManager, RuleManager or runExperience change, and no bucketing-contract or parity-fixture change.

`@JvmOverloads` is load-bearing, and this repo has nothing else guarding it

Widening a public fun replaces its JVM descriptor. Without the annotation, the shipped runFeature(String) and runFeatures() descriptors would vanish and every app compiled against the current AAR would fail at run time with NoSuchMethodError. There is no binary-compatibility-validator, no .api dump, and CI runs only detekt, lint, Dokka, tests and Kover.

ConvertContextPublicArityTest is therefore the only guard. It asserts all ten descriptors through getDeclaredMethod, which matches on exact parameter-type sequence — so it pins order, not just presence. Review confirmed independently with javap on the release variant.

Test plan

Suite green at fe24f2c1: core 497/0, sdk 310/0 (from a 283 baseline), sdk-lint 8/0, plus detekt, lintDebug, koverVerify and dokkaGenerate.

A green suite is not by itself evidence that tests discriminate, so four mutations were applied and reverted — each failed the expected tests and only those:

Mutation	Result
delete the `experienceKeys` filter	5 of 10 scope tests fail — exactly the five asserting narrowing
neutralise the preview guard on the persist gate	3 of 8 zero-trace tests fail, including the pre-existing one
revert the `enableTracking` threading	2 of 5 fail, via the enqueue channel
make the fresh-bucket fire unconditional, enqueue still gated	2 of 5 fail, via the fire channel

The last two are separate deliberately. The enqueue assertion precedes the sink assertion and assertTrue throws, so the first never reaches the fire assertion — a decision audit caught that, and the second closes it. It also proves the 200 ms barrier is long enough, since a short one would have let the mutant pass.

Zero test mutations. Every test path is new or purely additive relative to origin/main — git diff origin/main...HEAD -- '*Test*' produces no deletion lines at all.

Decisions

AgDR-0187-robolectric-forces-junit-4-so-mockk-is-not-the-android-test-seam — the spec prescribed JUnit 5 + MockK spies. Robolectric 4.x is a JUnit 4 runner, so nothing needing an Android runtime can be a JUnit 5 test, and MockK has zero importers repo-wide. The repo's own seams are used instead, and the normative requirement is preserved.
AgDR-0188-a-commit-subject-is-the-android-changelog-so-tdd-markers-ship — this repo has no CHANGELOG.md; release.config.mjs makes feat: commit subjects the published release notes.
AgDR-0185-a-compile-failure-is-the-red-signal-for-a-typed-public-api — why the RED phase is split into a reflection test and a compile failure, and why order matters.
AgDR-0189-the-shared-docs-directory-rule-outranks-a-spec-companions-local-precedent and AgDR-0186-cap-4-wiki-items-are-a-spec-defect-not-this-runs-scope — both cover the documentation half.

Three older records share this repo's feature slug and are cited only because the gate resolves by slug: AgDR-0007-introduce-a-single-public-seam-function-bucketing-manager-r, AgDR-0008-anchoredbucketingparitytest-s-vector-wrapper-goldenbucketi and AgDR-0009-wire-convertcontext-allocateandrecord-to-call-the-shared-r. They are from the original Android SDK sprint and are not this run's work.

Note on the record numbers. Three of these were renumbered after this PR was opened. agdr-renumber.yml moves an id that is already taken on main, and the orchestration branch's records collided with a concurrent run's. The slug half of each stem is immutable and is what identifies the record; only the number moved.

Spec defects carried, not built

documentation-surfaces.md items 4–6 instruct edits to android-sdk.wiki pages. Out of scope under wikis-are-not-edited-by-feature-work.md — the drift routine owns that refresh, and its watermark for this wiki equals current origin/main, so merging this moves it. CAP-4's criterion naming TrackingControl.md is therefore not met by this PR. One clause was moot anyway: the page already states the corrected semantics.
SPEC.md's "Why" overstates exposure volume. It claims one runFeature call enqueues for every experience carrying the feature. Measured: evaluate returns on the first resolving one, so ordinarily it is one. Fixtures are built on the measured rule.
FeatureManager.evaluate's KDoc was factually wrong — it claimed the in-process event still fires when tracking is off. Corrected here; the spec located that falsehood only in a wiki page, which has since been fixed, and never looked at the code.

…ry points CAP-1 / CAP-3 (SPEC-per-call-bucketing-attributes). Reflection over ConvertContext's declared methods, so the file compiles today and fails at run time — the only genuine RED available for a statically-typed public-API change. 8 rows, 2 failing: runFeature(String,Boolean) and runFeatures(Boolean) do not exist yet. The six runExperience/runExperiences rows pass and are the file's own positive control. @jvmoverloads is the only thing regenerating a widened public fun's original descriptor and this repo ships no binary-compatibility validator, so without this test nothing would catch an app compiled against the current AAR breaking at run time. Beads: ai-driven-product-dev-igow Agent: sdk-test-writer Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ndary CAP-1 (SPEC-per-call-bucketing-attributes). Six tests: fixture sanity that each feature is exposed by only its own experience; enableTracking=false on each entry point suppressing enqueue and fire while still persisting sticky and returning the tracked call's value; the default arities unchanged; and the sticky-revisit case where one experience carrying two features enqueues once but fires twice. RED is the compiler, which is the honest signal for a statically-typed public-API change: four call sites, all 'No parameter with name enableTracking found', and no other error — so the fixture, the seams and every assertion already type-check. Observation uses the repo's shipped seams rather than the spec's MockK spies, which have zero importers here and cannot reach a Robolectric-built SDK: RecordingApiManager for the enqueue, a synchronized-list RecordingEventCallback for the fire, and getStoreData for persistence. Beads: ai-driven-product-dev-igow Agent: sdk-test-writer Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

CAP-1 (SPEC-per-call-bucketing-attributes). Both feature entry points gain enableTracking: Boolean = true as their first optional parameter and pass it to FeatureManager instead of a hard-coded literal. FeatureManager already declared, defaulted and threaded the parameter, so the control was plumbed to one line short of the caller and this adds no engine code. @jvmoverloads on both is load-bearing, not decorative: widening a public fun replaces its JVM descriptor, so without it the shipped runFeature(String) and runFeatures() arities vanish and every app compiled against the current AAR breaks at run time. The repo has no binary-compatibility validator, which is why ConvertContextPublicArityTest asserts all six descriptors. enableTracking comes first so the arity-2 Java form matches runExperience's, and because @jvmoverloads generates prefix arities — which is what keeps the key-only form generated. Also shortens two test names in this branch's own RED commit to clear detekt MaxLineLength. Both blocks are new relative to origin/main, so this is growth rather than a change of test intent. Suite: core 497/0, sdk 296/0 (was 283), sdk-lint 8/0, detekt clean, lintDebug clean, koverVerify green. Beads: ai-driven-product-dev-igow Agent: sdk-android Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

CAP-2 (SPEC-per-call-bucketing-attributes), with CAP-3's binary half extended. Descriptor guard gains the two arity-3 rows. Verified in isolation before the behavioural file existed, since a compile error anywhere in the source set would have hidden it: 10 rows, exactly 2 failing with NoSuchMethodException, all 8 CAP-1 rows still green. After CAP-2 each entry point must expose three descriptors, because @jvmoverloads generates prefix arities and an app on the published AAR may call any of them. Ten behavioural tests at the ConvertContext boundary: baseline, narrowing with B reported DISABLED rather than omitted, a narrowed-away runFeature returning DISABLED rather than null, scoping proven real by the absence of both a sticky write and an enqueue for the excluded experience, the four edge inputs including emptyList meaning EVERY experience (D-5, the one that fails silently in the dangerous direction), and key-not-id matching. RED here is the compiler: five call sites, all 'No parameter with name experienceKeys found', and no other error. Beads: ai-driven-product-dev-0eor Agent: sdk-test-writer Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ints CAP-2 (SPEC-per-call-bucketing-attributes). Both feature entry points gain experienceKeys: List<String>? = null as their second optional parameter, threaded into FeatureManager.evaluate and .evaluateAll. The engine change is one line — a filter on evaluate's existing walk, placed before experienceExposesFeature so an excluded experience never reaches runExperience and is therefore never bucketed, never persisted and never reported. evaluateAll forwards. Nothing in DataManager, BucketingManager, RuleManager or runExperience changes. isNullOrEmpty is what makes null and emptyList impossible to diverge: an empty list means EVERY experience, not none (D-5). Reading it the other way would turn a caller's empty list into every feature DISABLED, silently. Narrowing does not shrink the result list — a feature reachable only through an excluded experience comes back DISABLED rather than omitted, which falls out of evaluate's existing disabledFeature tail rather than any new code path. Mutation-checked: deleting the filter line fails 5 of the 10 scope tests, and exactly the five that assert narrowing. Suite: core 497/0, sdk 308/0 (was 296), sdk-lint 8/0, detekt clean, lintDebug clean, koverVerify green. Beads: ai-driven-product-dev-0eor Agent: sdk-android Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@after

CAP-3 (SPEC-per-call-bucketing-attributes). Two new blocks assert that on a context with a preview set, runFeature and runFeatures called with enableTracking = true still produce zero track-endpoint requests, zero in-memory ApiManager queue entries, zero FileEventQueue entries and zero sticky-bucketing writes — so no caller value can reach past preview. Pure growth: 135 insertions, zero deletions, every pre-existing line including the @Before/@after apparatus byte-identical. The spec calls this 'qs-08's AC6 assertion, unchanged and unweakened', but the file contained zero occurrences of runFeature before this commit — there was no existing assertion to preserve, so no test intent changes. They live here rather than beside the CAP-1 tests because this is the only file carrying the real apparatus: a MockWebServer, the real ApiManager queue and the real FileEventQueue. The lighter RecordingApiManager used elsewhere overrides enqueueBucketingEvent and bypasses both queues, so it cannot see three of the four counts. Mutation-checked twice by the author and once independently: neutralising the persist gate's preview guard fails both new blocks and the pre-existing zero-trace test. Suite: core 497/0, sdk 310/0, sdk-lint 8/0, detekt clean, lintDebug clean, koverVerify green. Beads: ai-driven-product-dev-mceb Agent: sdk-test-writer Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@PARAM

…tion CAP-4 (SPEC-per-call-bucketing-attributes), the in-repo half. Dokka publishes this into the Javadoc JAR on Maven Central and it is what a developer's IDE shows on hover, so it is a shipped surface rather than a comment. runFeature and runFeatures gain @PARAM entries for both new parameters, stating the two things a caller gets wrong: enableTracking = false suppresses the in-process SystemEvents.BUCKETING fire as well as the network event but does NOT disable persistence, and experienceKeys as an empty list means every experience rather than none. FeatureManager.evaluate's @PARAM enableTracking was factually wrong — it claimed 'the outbound queue is suppressed but sticky + internal events still fire'. That is the JavaScript SDK's semantics, not Android's: ConvertContext gates the enqueue and the in-process fire on the same conjunction, on both the sticky-recall and fresh-bucket paths. The spec located this falsehood only in a wiki page, which has since been corrected, and never looked at the KDoc of the method the parameter flows through. evaluate and evaluateAll also gain @PARAM experienceKeys. Suite: core 497/0, sdk 310/0, sdk-lint 8/0, detekt clean, lintDebug clean, koverVerify green, dokkaGenerate green. Beads: ai-driven-product-dev-lq4n Agent: sdk-android Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The build-SDK / attach-spy / subscribe-sink / create-context preamble was written out seven times. A private Arm holder and a newArm(configJson, visitorId) helper replace it — the shape this feature set for itself as CD-3 and already used in the CAP-2 scope tests, applied to the file that most needed it. The decision audit flagged the omission. Pure extraction: every assertion, expected count and test name is byte-identical. newArm takes visitorId as a required parameter with no default, and each of the seven arms passes its own — that is load-bearing rather than incidental, because an untracked call still writes the sticky decision, so two arms sharing a visitor would make the second read zero enqueues for the wrong reason. The discrimination property was re-checked after the refactor, not assumed: reverting both entry points to hand FeatureManager a hard-coded enableTracking = true still fails exactly the same two untracked tests, with the other three surviving because they never exercise the false value. Zero deletions in any test path relative to origin/main — the file is new on this branch, so this is growth. Suite: core 497/0, sdk 310/0, sdk-lint 8/0, detekt clean, lintDebug clean, koverVerify green. Beads: ai-driven-product-dev-igow Agent: sdk-test-writer Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@PARAM

The note said the per-experience loop has three continues. It has five: no key, excluded by experienceKeys, no exposed feature, not bucketed, and no matching feature change on the bucketed variation. It was already wrong by one before this feature — the findFeatureChange guard was never counted — and CAP-2's filter made it wrong by two. Corrected on the same reasoning that fixed the @PARAM above it: a false statement in Android's own source is worth fixing when the method is being edited anyway, and this one the feature itself made worse. Comment only. The @Suppress stays, because the loop genuinely still has too many jumps for the rule. Beads: ai-driven-product-dev-0eor Agent: sdk-android Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The file documents itself as the interop guard — if it compiles, the public API's Java annotations are correct, and the compiler is the test. This feature's entire risk surface is @jvmoverloads, so a change of this shape not touching it was a gap; code review raised it below the blocking threshold and it is worth two lines. Not redundant with ConvertContextPublicArityTest. That asserts the descriptors exist; this asserts they are callable from Java source, which is what the annotation is for and what a consumer actually does. Both the one-argument and zero-argument forms are exercised, since those are the arities @jvmoverloads regenerates and the ones an already-published app calls. New method, 28 insertions, zero deletions — growth on a file that predates the branch. Suite: core 497/0, sdk 310/0, sdk-lint 8/0, detekt clean, lintDebug clean, koverVerify green. Beads: ai-driven-product-dev-igow Agent: sdk-test-writer Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

JosephSamirL

Review — convertcom/android-sdk PR #63 @ `fe24f2c`

Reviewer: convert-code-reviewer (independent pass, 2026-09-17). Spot-checked by the session: filter line in FeatureManager.evaluate and the four test-result XMLs (8+5+10+10 tests, 0 failures) both verified on disk.

VERDICT: APPROVED

Summary

Reviewed PR #63 against origin/main (5112e162) in a detached worktree: 7 files, +938/-10, only two production files (ConvertContext.kt, FeatureManager.kt). The engine change is one filter line in FeatureManager.evaluate plus parameter threading; everything else is KDoc and new tests. No DataManager, BucketingManager, RuleManager or runExperience change; the bucketing path (allocateAndRecord → bucketingManager.resolveVariationId) is untouched — the parity contract is not moved.

All five shared semantics hold, verified against code and tests. Java interop and binary compatibility verified with javap. CI green on all 10 checks. The four changed test classes were run locally: 33/33 pass.

CRITICAL: none. IMPORTANT: none.

Semantics verification (the shared contract)

enableTracking suppresses reporting only — never the decision, never persistence. ConvertContext.allocateAndRecord gates updateBucketing on !isPreviewActive() alone and returns the variation unconditionally; the enqueue is gated on enableTracking && !isPreviewActive(). ConvertContextRunFeatureTrackingControlTest asserts sticky written, zero enqueues, and assertEquals(trackedFeature, untrackedFeature). Caveat: Android also suppresses the in-process SystemEvents.BUCKETING fire — a pre-existing, deliberate Android divergence (F-134), recorded in SPEC D-8 and bucketing-attributes.md §3, and already in the bundled TrackingControl.md. This PR did not introduce it.
Empty experienceKeys = no filter. FeatureManager.evaluate: if (!(experienceKeys.isNullOrEmpty() || expKey in experienceKeys)) continue. Tested for both null and emptyList() → both features ENABLED.
Unknown key skipped, not raised. Membership test only; no lookup, no throw. Tested (one-unknown-among-known → known still resolve; all-unknown → all DISABLED, zero writes, zero enqueues).
Key order ignored; evaluation follows config order. The loop iterates data.experiences (config order) and tests membership — same as the JS reference (FeatureManager.runFeatures → DataManager.getItemsByKeys). Verified by code; no test pins it (see notes).
Bucketing untouched. No file under packages/core in the diff.

Filter placement is before experienceExposesFeature and before runExperience, so an excluded experience is never bucketed, persisted, or reported — ConvertContextFeatureExperienceScopeTest proves it via store read + RecordingApiManager. Matching is by ConfigExperience.key; the id-passed-as-key test pins that.

Java interop and binary compatibility

javap -p on the compiled ConvertContext.class shows all six feature-pair descriptors, all public final with unchanged return types: runFeature(String), runFeature(String, boolean), runFeature(String, boolean, List<String>), runFeatures(), runFeatures(boolean), runFeatures(boolean, List<String>). The two pre-existing descriptors an already-published app calls are present, so no NoSuchMethodError. ConvertContextPublicArityTest asserts exactly these via getDeclaredMethod. JavaInteropSample.java exercises all six from Java source and compiled. Parameter order (enableTracking first) mirrors runExperience.

Zero-trace preview (CAP-3)

ConvertContextPreviewZeroTraceTest gains two blocks (135 insertions, 0 deletions). With a preview set, runFeature/runFeatures with enableTracking = true assert 0 ApiManager queue entries, 0 FileEventQueue entries, 0 track-endpoint requests, and empty sticky store. Both the preview-forced leg and the non-previewed leg that reaches the allocateAndRecord gates are exercised.

Non-blocking notes (below the 75 threshold)

No test pins key-order independence. The loop structure makes order-dependence structurally impossible today; a one-line listOf(KEY_B, KEY_A) case would close it against a future refactor. (~60)
Test setup duplication between ConvertContextRunFeatureTrackingControlTest and ConvertContextFeatureExperienceScopeTest (identical json, setUp, buildSdk, recordingSink, awaitCondition). android-sdk has no Sonar duplication gate (spec D-10: no sonar-project.properties). (~55)
Spec worktree, not this PR: in ai-driven-product-dev #98 @ 2563bfae, AgDR-0182, AgDR-0183 and AgDR-0184 are committed with unresolved merge-conflict markers (<<<<<<<< HEAD:docs/agdr/AgDR-0189-…), and the AgDR-0184 file is a python-sdk record. Belongs to #98; no bearing on the Android code.
The bundled shared page running-features.md still lists experienceKeys inside a JS-shaped attributes table. Backend-owned full-stack-docs sync target (PR #7391); the drift routine owns the refresh.

What I checked

gh pr view 63 --repo convertcom/android-sdk — description and all 10 commits.
gh pr checks 63 --repo convertcom/android-sdk — 10/10 pass (CodeQL ×3, Lint, core tests, sdk tests, coverage thresholds). CI triggers on pull_request → main, which this PR targets, so the run is genuine.
git diff origin/main...HEAD --stat — 7 files, +938/-10.
git diff origin/main...HEAD -- '*Test*' | grep '^-[^-]' | wc -l → 0 deletion lines in test paths (B-G12 satisfied by construction).
Read in full: FeatureManager.kt, ConvertContext.runExperience/resolveSticky/allocateAndRecord/runFeature/runFeatures, all four test files, JavaInteropSample.java, SPEC.md, bucketing-attributes.md, AgDR-0184–0189.
./gradlew :packages:sdk:testDebugUnitTest --tests '…PublicArityTest' --tests '…RunFeatureTrackingControlTest' --tests '…FeatureExperienceScopeTest' --tests '…PreviewZeroTraceTest' --no-daemon — BUILD SUCCESSFUL in 51s; PublicArity 10/0, RunFeatureTrackingControl 5/0, FeatureExperienceScope 10/0, PreviewZeroTrace 8/0 (tests/failures). Temurin 17.0.19. Full suite not run locally; CI did.
javap -p on the compiled ConvertContext.class — six feature-pair descriptors confirmed.
JS reference (read-only): packages/js-sdk/src/feature-manager.ts runFeatures filter and packages/data/src/data-manager.ts getItemsByKeys — confirms empty=all, config-order, unknown-skipped.
Bundled wiki: android-sdk.wiki/TrackingControl.md already states both reporting paths suppressed; JavaInterop.md documents the @JvmOverloads idiom.
No other featureManager.evaluate callers; demo app uses only default arities.
No lingering gradle/kotlin daemons after the run.

JosephSamirL

Approved via /convert:approve. An independent code review ran through /convert:review, and this issues the B-G4 human marker at fe24f2c.

abbaseya and others added 10 commits September 16, 2026 17:45

abbaseya requested a review from JosephSamirL September 16, 2026 19:09

abbaseya self-assigned this Sep 16, 2026

JosephSamirL reviewed Sep 17, 2026

View reviewed changes

JosephSamirL approved these changes Sep 17, 2026

View reviewed changes

abbaseya merged commit dd99a42 into main Sep 17, 2026
10 checks passed

abbaseya deleted the feat/per-call-bucketing-attributes branch September 17, 2026 14:16

Navigation Menu

feat(sdk): per-call enableTracking and experienceKeys on the feature entry points - #63

feat(sdk): per-call enableTracking and experienceKeys on the feature entry points#63
abbaseya merged 10 commits into
mainfrom
feat/per-call-bucketing-attributes

abbaseya commented Sep 16, 2026 •

edited

Loading

Uh oh!

JosephSamirL left a comment

Uh oh!

JosephSamirL left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

abbaseya commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this was worth doing

The change is smaller than the tests suggest

@JvmOverloads is load-bearing, and this repo has nothing else guarding it

Test plan

Decisions

Spec defects carried, not built

Related

Uh oh!

JosephSamirL left a comment

Choose a reason for hiding this comment

Review — convertcom/android-sdk PR #63 @ fe24f2c

Summary

Semantics verification (the shared contract)

Java interop and binary compatibility

Zero-trace preview (CAP-3)

Non-blocking notes (below the 75 threshold)

What I checked

Uh oh!

JosephSamirL left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abbaseya commented Sep 16, 2026 •

edited

Loading

`@JvmOverloads` is load-bearing, and this repo has nothing else guarding it

Review — convertcom/android-sdk PR #63 @ `fe24f2c`