feat(preview): experiment preview — debugToken + preview deep links (qs-02)#42
Conversation
…edaction — tests (RED) Beads: ai-driven-product-dev-pjnz Agent: sdk-ios AC1/AC2/AC3 test coverage; RED = missing debugToken API (compile gap) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…edaction — implementation (GREEN) Beads: ai-driven-product-dev-pjnz Agent: sdk-ios ConvertConfiguration.debugToken; ConfigFetchService transport+cache bypass; ToLoggable debug_token redaction Full suite 343/343 green; swiftlint 0 violations Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Beads: ai-driven-product-dev-poqd Agent: sdk-ios AC9 table-driven parse test; RED = missing PreviewParam API Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tation (GREEN) Beads: ai-driven-product-dev-poqd Agent: sdk-ios Core/Preview/PreviewParam.swift; ASCII-decimal numeric-id validation; full suite 344/344 green Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ts (RED) Beads: ai-driven-product-dev-lkfj Agent: sdk-ios AC4/AC5 table-driven full-bypass test; RED = missing PreviewDecision API Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lementation (GREEN) Beads: ai-driven-product-dev-lkfj Agent: sdk-ios Core/Preview/PreviewDecision.swift; full-bypass structural (no store/hash inputs); full suite 345/345 green Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Beads: ai-driven-product-dev-l827 Agent: sdk-ios AC8 exp-fetch + PreviewState memo (injectable Clock); RED = missing fetchExperienceConfig/PreviewState Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ation (GREEN) Beads: ai-driven-product-dev-l827 Agent: sdk-ios ConfigFetchService.buildExperienceConfigURL/fetchExperienceConfig (forced low-cache, no disk write); PreviewState actor memo (60s TTL, injected Clock, sweep-on-access); full suite 351/351 green Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ion + inert — tests (RED) Beads: ai-driven-product-dev-bc6h Agent: sdk-ios AC4/AC7 + precedence + inert forced-decision path; RED = missing setPreview/previewHTTPClient wiring Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ion + inert — implementation (GREEN) Beads: ai-driven-product-dev-bc6h Agent: sdk-ios ConvertContext.setPreview (async) + forced-decision short-circuit in runExperience/runExperiences; per-context ConfigFetchService+PreviewState built in createContext (AC7 isolation, previewHTTPClient inject); join-key resolve: numeric experienceId -> config experience -> match Variation.experienceKey to run key; inert warning + normal fallthrough. Test-file swiftlint --strict cleanup (large_tuple->named struct, 5 line wraps). Full suite 358/358 green; repo-wide swiftlint --strict 0 violations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Beads: ai-driven-product-dev-y1ke Agent: sdk-ios AC6 preview lifecycle zero-trace (queue/on-disk store/uploader/decision writes) + AC7 companion; RED = 5 real assertion failures Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…EEN) Beads: ai-driven-product-dev-y1ke Agent: sdk-ios Per-context previewActive gate (PreviewState.isPreviewActive = forcedVariation != nil): AND-in !previewActive to enableTracking for bucketing enqueue; early guard in trackConversion (suppresses dedup persist + both enqueues + bus fire); persistDecision:Bool=true threaded into ExperienceManager.selectVariation(s) to suppress sticky saveDecision under preview. EventQueue.flush early-returns on empty queue (uploader.callCount==0 for free). Full suite 360/360; repo lint 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stence — tests (RED) Beads: ai-driven-product-dev-h4es Agent: sdk-ios Decision-audit round-1 fix: RED = segment setters write to shared decision store ungated under preview (AC6 §2) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stence — implementation (GREEN) Beads: ai-driven-product-dev-h4es Agent: sdk-ios Decision-audit round-1 fix: setDefaultSegments/setCustomSegments early-return under preview (guard !previewState.isPreviewActive, mirrors trackConversion:605) -> zero visitor-state disk write under preview, AC7 preserved (no shared-state mutation). Full suite 362/362; repo swiftlint --strict 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…RED) Beads: ai-driven-product-dev-hvqm Agent: sdk-ios Decision-audit round-2 fix: RED = runFeature/runFeatures leak sticky write + .bucketing enqueue ungated under preview (AC6 §2); feature resolution preserved Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ntation (GREEN) Beads: ai-driven-product-dev-hvqm Agent: sdk-ios Decision-audit round-2 fix: FeatureManager.evaluateFeature/evaluateAllFeatures gain enableTracking/persistDecision (defaults true), forwarded to selectVariation; runFeature/runFeatures pass !previewActive (NOT combined with global network.tracking -> preserves F-171 scope asymmetry). Features still RESOLVE under preview (coherent rendering); zero sticky write + zero .bucketing enqueue. Full suite 364/364; repo swiftlint --strict 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d in runFeature/runFeatures Beads: ai-driven-product-dev-7oz6 Agent: sdk-ios Hoist one 'let previewActive = await previewState.isPreviewActive' per feature method (derive both enableTracking + persistDecision from it) -> closes torn-gate zero-trace race under concurrent setPreview, matches runExperience/runExperiences. Extracted stateless stringified(_:) to file scope to stay within type_body_length (no suppression; matches existing in-file precedent). Full suite 364/364; repo swiftlint --strict 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements the experiment preview feature (qs-02) for the iOS SDK. It introduces PreviewParam for parsing preview parameters, PreviewDecision for forcing variation decisions, and PreviewState for managing in-memory memoization of preview configurations with a 60-second TTL. It updates ConvertContext to support setting preview targets eagerly, bypassing normal bucketing, and enforcing zero-trace tracking/persistence when a preview is active. Additionally, ConfigFetchService is updated to handle preview-specific URL construction and cache bypasses, and ToLoggable is enhanced to redact debug tokens from logs. Feedback on the changes highlights a potential actor reentrancy issue in PreviewState.resolveConfig(experienceId:) that could lead to duplicate concurrent network requests for the same experience ID, suggesting a task-based deduplication approach.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
JosephSamirL
left a comment
There was a problem hiding this comment.
Review — experiment preview (qs-02)
Overall this is solid — cleaner than the Android sibling (#52), and it fixes both mediums I raised there: no in-flight tracking-suppression window (setPreview awaits resolution before setForcedVariation, and isPreviewActive == forcedVariation != nil, so nothing is suppressed during the ?exp= fetch or on bad input), and trackConversion is a full no-op under preview (it suppresses the .conversion bus fire too). Verified against the JS feat/experiment-preview reference and PR-head source, not just the diff. CI is green here (Build/Lint/Test + Parity Suite, with ThreadSanitizer) because ci.yml triggers on all branches — the coverage #52 lacked.
Verified clean
PreviewParam.parse— faithful port of JSparsePreviewParam(^\d+$+ single-dot split; the >1-dot, empty-segment, and whitespace cases all match).PreviewDecision.forcedVariation— shape identical toBucketingManager's normal decision: both build the same 4-fieldVariation(id:key:experienceId:experienceKey:)with the same?? ""degradation.- AC2 cache bypass — complete and structural:
loadCachedConfigreturns nil underdebugToken,fetchLiveConfig's write is gated, andfetchExperienceConfignever touchesfileStore(via thegetAndDecodeextraction). - URL params match JS (
exp/debug_token/_conv_low_cache/environment), tested order-agnostically; per-contextPreviewStateisolation (AC7); IOS-fix3 single-read hoist closes the torn-gate race.
One behavior to align with JS (inline)
The internal .bucketing observer event still fires for sibling experiences under preview — ExperienceManager.selectVariation step 6 fires it unconditionally (only the network enqueue and the sticky write are gated). JS fires none here, and this PR's own trackConversion already suppresses its .conversion fire — so this should match: none, not fires. Small fix: thread a !previewActive gate onto the step-6 fire. Details inline.
Minor (inline)
debug_tokenredaction regex is unanchored (the Android sibling anchored the same one — over-redaction only, never a leak).- The
rawExperienceskey-filter degrades oddly if a previewed experience ever has an empty key (near-unreachable, but worth a guard).
Gemini's actor-reentrancy note is a reasonable won't-fix — benign idempotent extra ?exp= GET, and the actor serializes the memo so there's no data race.
Not approving yet so the .bucketing alignment can land — everything else looks ready.
…ew nits (qs-02) Address code-review findings on the experiment-preview PR (#42): 1. Zero-trace parity gap — the internal `.bucketing` observer event fired unconditionally for sibling/feature-carrier experiences while a preview was active, diverging from the JS oracle (context.ts wraps every SystemEvents.BUCKETING emit in `if (!this._preview)`) and from this PR's own suppression of `.conversion` and `.segments` under preview. Thread a new `emitBucketing: Bool = true` gate from all four ConvertContext entry points (runExperience/runExperiences/runFeature/runFeatures, as `!previewActive`) through ExperienceManager.selectVariation(s) and FeatureManager.evaluate* to the single fire site. Default `true` keeps every other call site and existing behavior unchanged. 2. Redaction regex — anchor the `debug_token` branch with a negative lookbehind `(?<![A-Za-z0-9_])` so a lookalike like `x_debug_token=` is no longer over-redacted, while a real `debug_token=<value>` in any position is still fully stripped. 3. Inert-on-bad-input — PreviewDecision.forcedVariation now returns nil for an experience with a nil/empty key or id (instead of degrading to ""), so a degenerate preview target can no longer poison the runExperiences sibling filter; setPreview treats it as bad input (warns, leaves context normal). Tests: extend AC6 zero-trace to assert zero `.bucketing` observer events under preview across experience + feature paths; add regex boundary cases and PreviewDecision empty-key/id cases. Full suite green under ThreadSanitizer (370 passed / 1 skipped / 0 failed), SwiftLint --strict clean, parity + DocC coverage gates pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@JosephSamirL thanks for the thorough review — all three points addressed in
Gemini's reentrancy note left as won't-fix per your endorsement. Locally green with the changes: SwiftLint |
JosephSamirL
left a comment
There was a problem hiding this comment.
Re-review — all three prior findings resolved. Approving.
Re-verified every finding against actual code at the current head (0ca75fe), re-locating each by content (not by stale line numbers). JS oracle parity confirmed: context.ts gates every SystemEvents.BUCKETING emit behind if (!this._preview) and trackConversion early-returns under preview → no observer events under preview. iOS now matches.
1. MEDIUM — .bucketing observer fire for siblings under preview → Resolved
ExperienceManager.selectVariationnow gates the fire:if emitBucketing { await eventBus.fire(.bucketing, …) }(ExperienceManager.swift:204-211), with the variation still returned unconditionally at:212(coherent rendering preserved).selectVariationsforwards it (:292).- Threaded
emitBucketing: !previewActivefrom all fourConvertContextentry points —runExperience(:311),runExperiences(:414,previewActive = forced != nil),runFeature(:495),runFeatures(:548) — and throughFeatureManager.evaluateFeature/evaluateAllFeaturesdown to the sameselectVariationsite (FeatureManager.swift:130,:199). trackConversionstill suppresses its.conversionfire via the earlyguard !(await previewState.isPreviewActive) else { return }atConvertContext.swift:629, before the fire at:666.- Default
emitBucketing: trueon every signature → non-preview call sites unchanged (AC10). No over-suppression: the gate is!previewActive, so a non-preview run still fires. - Coherent rendering intact: forced target short-circuits/returns (
runExperience:282-284;runExperiencesexcludes then re-appends the forced variation:392/:416-418); siblings are still selected — only the observer fire + network enqueue + sticky write are suppressed. - Tested (not just claimed):
PreviewZeroTraceTests.swift:224asserts zero.bucketing"for the target or the sibling";:290covers the bulk path; regression guard:316asserts a non-preview run still fires (== 1). Feature path mirrors it:PreviewFeatureZeroTraceTests.swift:215/:243(zero under preview) +:270regression guard.
2. MINOR — debug_token redaction unanchored → Resolved
ToLoggable.swift:92-93 anchors the branch: (?<![A-Za-z0-9_])debug_token=[^&#\s]*. x_debug_token= no longer over-redacted; a real token still stripped in any position (including string-start — deliberately safer than Android's (?<=[?&])).
3. MINOR — PreviewDecision.forcedVariation nil key → "" → Resolved
PreviewDecision.swift:46-49 now guards nil/empty key and nil/empty id, returning nil (inert-on-bad-input), so a degenerate target can't poison the runExperiences sibling filter or the runExperience short-circuit.
CI — green on head 0ca75fe
All 4 check runs pinned to the head SHA succeeded: Build, Lint & Test ✅ and Parity Suite (100%-pass independent job) ✅. No failing or absent required check.
No regressions spotted in the new commit. LGTM.
Summary
Additive, SDK-only implementation of Experiment Preview (quick spec qs-02) for the Convert iOS SDK — one of six identical sibling specs (JS, PHP, Python, Ruby, Android, iOS). Mirrors the Android sibling (convertcom/android-sdk#52). Two capabilities:
(A)
debugTokenonConvertConfiguration(debugToken: String? = nil)— every config fetch (initial + refresh scheduler) carriesdebug_token=<v>AND a forced_conv_low_cache=1; the on-disk config cache is neither read on cold start nor written while set; the token never reaches the track endpoint and is redacted from all log output.(B) Per-context preview input
context.setPreview(experienceId:variationId:)— force-decides one variation with FULL bypass (audiences/segments/locations/environment/status/traffic/stored-decisions/hash) and ZERO trace: nothing reaches the in-memory queue, the on-diskCoordinatedFileEventQueueStore, or the backgroundURLSession, and no visitor-state/sticky writes occur for the entire preview. Fetches an absent experience via?exp=+_conv_low_cache=1(+debug_tokenif set), memoized in memory per experienceId (60s TTL, never on disk). Pure helperPreviewParam.parse(_:)for the canonical linkconvert_preview={experienceId}.{variationId}. Inert on bad input (warning log, normal behavior). Per-context, no cross-context leak.No API endpoint, no OpenAPI spec, no backend/serving change, no bucketing-contract change (preview bypasses bucketing entirely).
Stacked PR
Base is
feat/fullstack-v12(NOTmain), matching the Android sibling.mainis a strict ancestor of the base.Acceptance criteria (AC1–AC10) — all covered by tests
?exp=fetch · AC5 full bypass (draft/paused/env/non-running/zero-traffic/sticky)Tasks (beads, all closed — 6 feature + 3 remediation)
PreviewParam.parsePreviewDecision.forcedVariation(structural full bypass)?exp=fetch +PreviewState60s memosetPreview+ per-context wiring + isolation + inertrunFeature/runFeatures) under previewisPreviewActiveread in the feature methods (torn-gate race)QA
swiftlint --strict0 violations. Zero-trace tests exercise the realEventQueue+CoordinatedFileEventQueueStore+ spies, each with a non-preview AC7 companion.Test plan
feat/fullstack-v12?debug_token=/?exp=🤖 Generated with Claude Code