Skip to content

feat(preview): experiment preview — debug_token + set_preview (qs-03)#41

Merged
abbaseya merged 15 commits into
feat/fullstack-v12from
feat/experiment-preview
Jul 15, 2026
Merged

feat(preview): experiment preview — debug_token + set_preview (qs-03)#41
abbaseya merged 15 commits into
feat/fullstack-v12from
feat/experiment-preview

Conversation

@abbaseya

@abbaseya abbaseya commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements qs-03 Experiment Preview for the Convert Ruby SDK — an additive, SDK-only feature (no API endpoint, no OpenAPI, no backend, no bucketing-contract change). Two capabilities from the normative cross-SDK contract:

  • (A) debug_token: config option — a QA config-transport option. When set, every config-fetch URL carries debug_token=<value> and _conv_low_cache=1 (forced regardless of cache_level); config caching is disabled (always live, no store read-fallback and no cache write, so a shared store is never poisoned with the status-widened debug config); the token is redacted like sdk_key_secret and never sent to the track endpoint.
  • (B) Per-context preview inputcontext.set_preview(experience_id:, variation_id:) force-decides one variation with full bypass (audiences/segments/locations/environment/status/traffic/stored-decisions/bucketing-hash) and zero trace (no track requests, no visitor-state store writes, across the whole lifecycle including the at_exit flush). Canonical link param convert_preview={experienceId}.{variationId} parsed by the pure helper ConvertSdk.parse_preview_param.

Stacked PR — base is feat/ruby-sdk-cache-level (the open qs-02 PR #40), NOT master. This reuses the qs-02 _conv_low_cache append seam on that base. Merge qs-02 (#40) first.

Acceptance criteria (AC1–AC10) — all covered

AC Coverage
AC1 debug_token transport config_url forces _conv_low_cache=1 + appends debug_token=; composes with environment; refresh path inherits
AC2 always-live no store read-fallback and no cache write under debug_token
AC3 token hygiene redacted like sdk_key_secret; absent from to_internal (never tracked)
AC4 forced decision set_preview resolves in-config or via ?exp= fetch; run_experience short-circuits the forced variation
AC5 full bypass DataManager#get_preview_decision pure, bypasses every gate (draft/paused/mismatched-env/stopped/zero-traffic/stored-decision)
AC6 zero trace no enqueue (suppressed at source → empty queue → at_exit/flush no-op) + no store write across run_experience(s)/run_feature(s)/track_conversion/segments
AC7 isolation preview state per-context; concurrent non-preview context buckets/persists/tracks normally; shared config deep-frozen (no mutation leak)
AC8 memoization process-wide 60s fork-safe memo keyed sdk_key:experience_id, one fetch per key/window, evicts expired entries on write
AC9 parse helper parse_preview_param("123.456") → ["123","456"]; malformed → nil
AC10 regression lock full existing suite green; RBS/steep updated for all new public API

Implementation tasks (beads epic ai-driven-product-dev-gc7u)

  • RB-1 debug_token config option + config_url transport + always-live + hygiene
  • RB-2 ConvertSdk.parse_preview_param pure helper
  • RB-4 DataManager#get_preview_decision pure full-bypass
  • RB-3 ApiManager#get_config_by_experience + process-wide 60s fork-safe memo
  • RB-5 Context#set_preview + forced-decision path + isolation
  • RB-6 zero-trace hardening (suppress all tracking + all store writes on a preview context)
  • Review R1 fixes: memo eviction on write (parity + unbounded-growth guard); zero-trace test lock-in for plural/feature paths

Design note — deliberate Ruby/JS divergence (audited)

On a preview context, genuinely-decided OTHER experiences still fire the SystemEvents::BUCKETING lifecycle event (Ruby's Story 4.5 invariant: the lifecycle event is decisioning observability that fires even under consent denial; only the outbound enqueue is gated). JS wraps this in if (!this._preview). Zero-trace per the spec is defined over track requests + visitor-state persistence — not the in-process pub/sub event — so AC6 holds (0 POSTs, empty queue, 0 store writes). An independent decision-audit (analyst → auditor) verified this PASS.

Test plan

  • Full suite green: 1318 examples, 0 failures (spec/unit spec/integration spec/cross_sdk spec/docs)
  • steep check clean; rbs validate clean; rubocop lib/ clean
  • Decision audit PASS (10 decisions, independent analyst + auditor)
  • Code review clean after round 2

Human follow-up (separate repos — cannot edit here)

  • ruby-sdk wiki Configuration.md option table: add a debug_token row + a "Preview / QA" section (the canonical option table the README defers to lives in the separate wiki repo).
  • Shared "QA & Preview for Fullstack" guide + UI "Copy preview link" work are out of this repo's scope (recorded in the spec's rollout notes).

🤖 Generated with Claude Code

abbaseya and others added 14 commits July 7, 2026 03:32
…lways-live + hygiene — tests (RED)

Beads: ai-driven-product-dev-oj5t
Agent: sdk-ruby

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lways-live + hygiene — implementation (GREEN)

Beads: ai-driven-product-dev-oj5t
Agent: sdk-ruby
AC1 config_url forces _conv_low_cache=1 + appends debug_token; AC2 no store read-fallback + no cache write under debug_token; AC3 redacted like sdk_key_secret, absent from to_internal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r — tests (RED)

Beads: ai-driven-product-dev-8rrk
Agent: sdk-ruby

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r — implementation (GREEN)

Beads: ai-driven-product-dev-8rrk
Agent: sdk-ruby
AC9: dot-split numeric-only pair -> [exp_id, var_id] | nil; JS parse-preview-param.ts parity.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l-bypass — tests (RED)

Beads: ai-driven-product-dev-ei9u
Agent: sdk-ruby

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l-bypass — implementation (GREEN)

Beads: ai-driven-product-dev-ei9u
Agent: sdk-ruby
AC4/AC5: resolves variation directly from passed experience, bypasses all gates, zero side effects; nil on unknown variation. JS getPreviewDecision parity.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… memo — tests (RED)

Beads: ai-driven-product-dev-sr5k
Agent: sdk-ruby

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… fork-safe memo — implementation (GREEN)

Beads: ai-driven-product-dev-sr5k
Agent: sdk-ruby
AC8: process-wide mutex-guarded memo keyed sdk_key:experience_id, 60s wall-clock TTL, one fetch per key/window, fork-child clear. AC4-fetch: exp-scoped URL (env,exp,_conv_low_cache=1,debug_token). Byte-order test fixed to assert on builder output (WebMock URI alphabetizes) mirroring client_spec pattern; fork_safety child-callback lock updated to 2.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…solation — tests (RED)

Beads: ai-driven-product-dev-4rjz
Agent: sdk-ruby

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…solation — implementation (GREEN)

Beads: ai-driven-product-dev-4rjz
Agent: sdk-ruby
AC4: set_preview resolves in-config (new DataManager#experience_by_id) or via ?exp= fetch, validates variation via get_preview_decision; run_experience short-circuits forced preview for target experience (no bucketing event). AC7: inert-on-bad-input (warn, preview stays nil, never raises); per-context isolation; shared config deep-frozen so no mutation leak. JS setPreview parity.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Beads: ai-driven-product-dev-usvz
Agent: sdk-ruby

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…EEN)

Beads: ai-driven-product-dev-usvz
Agent: sdk-ruby
AC6/AC3: preview context leaves zero trace — enqueue suppressed at source (enable_tracking:false path) so timers + at_exit have nothing to send; enable_storage:false threaded decision_attributes->persist_bucketing suppresses sticky writes; segments writes gated (put_segments/select_custom_segments enable_storage kwarg); track_conversion full no-op under preview. AC7: concurrent non-preview context tracks/persists normally. Other experiences still DECIDE; BUCKETING lifecycle event still fires (Ruby Story 4.5 observability invariant — documented divergence from JS if(!preview) wrap).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… expired entries

Beads: ai-driven-product-dev-mxwg
Agent: sdk-ruby
Sweep expired entries on write (JS oracle parity) — bounds the process-wide class-level memo against link-param-driven experience_id growth. Also tightened experience_by_id RBS untyped->String.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…experiences/run_feature(s) under preview

Beads: ai-driven-product-dev-uoeh
Agent: sdk-ruby
Test-only (AC6 lock-in): plural decisioning + feature resolution on a preview context leave zero trace (empty queue, no store write) while a non-preview context persists; uses full_chain feature-carrier vector. No impl change — behavior already correct.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request implements the experiment preview feature (qs-03), introducing a debug token configuration, experience-scoped config fetching with process-wide memoization, and a zero-trace preview context that suppresses tracking and persistence writes. The feedback highlights several critical robustness improvements: preventing invalid CDN requests in data-only mode when the SDK key is nil, avoiding a potential memory leak by referencing the class directly in a global ForkGuard callback, and defensively converting experience and variation IDs to strings to prevent TypeErrors during URL generation.

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.

Comment thread lib/convert_sdk/api_manager.rb
Comment thread lib/convert_sdk/api_manager.rb Outdated
Comment thread lib/convert_sdk/context.rb
Comment thread lib/convert_sdk/api_manager.rb
@abbaseya abbaseya self-assigned this Jul 7, 2026
@abbaseya
abbaseya requested a review from a team July 7, 2026 03:25
…lf-capture, coerce preview ids, rubocop

Gemini review round 2 on PR #41 plus a RuboCop CI failure: guard
ApiManager#get_config_by_experience against a nil sdk_key (data-only mode)
before it can build a guaranteed-failing config-fetch URL; reference the
ApiManager constant directly (not self.class) in the process-wide
fork-child cache-clear callback so it no longer gratuitously captures the
instance; coerce Context#set_preview's experience_id/variation_id to
String once at the entry and thread the coerced locals everywhere
downstream; rewrite context_preview_spec.rb's warn_messages to avoid a
false-positive Style/HashSlice autocorrect that would raise TypeError on
the Array-of-pairs sink.entries shape.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Base automatically changed from feat/ruby-sdk-cache-level to feat/fullstack-v12 July 15, 2026 15:58

@JosephSamirL JosephSamirL 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.

Review — experiment preview (qs-03): debug_token + set_preview

Verdict: Approve with minor changes. The implementation is correct, well-tested, and faithfully mirrors the JS oracle on every substantive contract. I found one comment that misrepresents the oracle (with a small, real behavioral divergence behind it), one undocumented divergence, and a stale PR description. Nothing blocks merge on correctness.

How this was verified (against the actual oracle, not the PR's paraphrase)

  • Read the full +1983/−59 diff across all 26 files, with the branch checked out for surrounding context on every seam.
  • Compared each Ruby behavior to the actual JS source at javascript-sdk@feat/experiment-preview (head 487f8f6) — context.ts, data-manager.ts, api-manager.ts, segments-manager.ts, parse-preview-param.ts — line by line.
  • Traced the live Ruby seams: fire_bucketing/suppress_bucketing_enqueue?, get_preview_decisionretrieve_variation/build_bucketed_variation (confirmed pure + full-bypass), persist_bucketing's enable_storage gate, the feature_manager → get_bucketing → persist_bucketing chain, and to_internal (explicit allowlist — debug_token absent by construction).
  • Ran it: full suite 1320 examples / 0 failures, rubocop lib/ clean, rbs validate clean, steep check clean, 97.9% line coverage.

🟡 F1 (Medium) — update_visitor_properties under preview diverges from JS, and its comment misstates the oracle

Ruby gates only the store write behind if @preview.nil?, then runs @attributes = @attributes.merge(normalised) unconditionally — so under preview the in-memory merge still happens (locked by context_preview_zero_trace_spec.rb:2466-2474).

The JS oracle does the opposite — context.ts:622-630:

updateVisitorProperties(visitorId, visitorProperties) {
  if (this._preview) return;                 // public method returns ENTIRELY
  this._dataManager.putData(visitorId, {segments: visitorProperties});
}

The Ruby comment claims "JS's public updateVisitorProperties has no preview guard of its own, but the PRIVATE helper it calls to persist DOES skip entirely under preview … leaving the in-memory side unaffected there too." That is wrong on three counts: the guard is in the public method (line 628, not a private helper); putData has no gate (the JS comment itself says so); and JS does no in-memory merge here — it returns first.

Impact: low but real. @attributes feeds decision_attributes → audience matching, so on a preview context update_visitor_properties changes how other experiences decide in Ruby but not JS. It's in-memory only, so zero-trace (track requests + store writes) is not violated — but it's an observable parity delta in a parity-defined feature, and the comment will mislead the next maintainer.

Fix: either (a) match JS — return self before the merge under preview (and update that one test), or (b) keep the Ruby behavior but rewrite the comment to state the truth and add it to the deliberate-divergence list. The mis-describing comment should go either way.

🟢 F2 (Low) — set_preview failure paths don't reset a prior preview; JS does

JS nulls _preview when the experience or variation can't be resolved (context.ts:172,182,195); Ruby's warn_preview_inert intentionally leaves @preview untouched. Only manifests on a re-preview (a 2nd set_preview after a good one) with bad input: JS drops preview mode, Ruby keeps the prior preview. Ruby's choice is arguably safer, but it's an undocumented divergence — match JS, or add a one-liner to the divergence list.

⚪ F3 (Housekeeping) — PR description is stale

The description still says "base is feat/ruby-sdk-cache-level (#40) … Merge qs-02 (#40) first." #40 has merged and this PR auto-retargeted to feat/fullstack-v12; the diff is now clean qs-03-only. Worth updating so reviewers aren't chasing a phantom stacked dependency.


✅ On the one deliberate divergence (BUCKETING event) — concur with the PASS

Under preview, other experiences still fire SystemEvents::BUCKETING in Ruby (only the enqueue is suppressed), whereas JS wraps every fire in if (!this._preview) (context.ts:260,320,391,412,472). Verified against both the oracle and Ruby's fire_bucketing (event always fires; enqueue gated by track:). The Story 4.5 rationale (BUCKETING is decisioning observability, orthogonal to the tracking switch) is sound and consistent with how Ruby already treats consent-denied tracking. Not a defect — noting only so the reviewer consciously accepts that a Ruby preview context emits BUCKETING events to subscribers where JS emits none.

Minor notes (not blockers)

  • Memo concurrency: JS caches the in-flight Promise so concurrent same-key callers coalesce to one fetch; Ruby's synchronous read-miss → fetch → write isn't atomic, so N concurrent first-callers for the same (sdk_key, experience_id) could each fetch. Benign (correct data, last-write-wins) and preview is a low-concurrency QA path; AC8's "one fetch per key/window" holds for sequential lookups.
  • run_experiences doesn't force the target's preview variation — but this is exact JS parity (JS runExperiences also decides the target naturally under preview; only runExperience short-circuits). Shared characteristic, not a Ruby bug.
  • URL encoding: Ruby URL-encodes exp/environment/debug_token; JS interpolates raw. No practical difference for numeric ids / typical tokens; Ruby is marginally safer.

Strengths worth calling out

  • parse_preview_param is an exact parity match including the subtle newline edge — Ruby correctly chose \A\d+\z (\z, not \Z) to match JS's non-multiline ^\d+$.
  • Token hygiene is airtight: to_internal is an explicit allowlist (token absent by construction), secret-registration wired, URL-query stripping covers the logged-URL case — all tested.
  • One clean seam (enable_storage: @preview.nil? through decision_attributes) covers experiences and features for persistence suppression — the feature chain plumbs it through intact.
  • get_preview_decision is genuinely pure/full-bypass (verified internals, test-locked with store spies), and the config-cache-poisoning guard (config_cache_disabled) is a thoughtful Ruby-only hardening beyond parity.

Reviewed with Claude Code. Findings F1/F2 verified against javascript-sdk@feat/experiment-preview head 487f8f6; suite/rubocop/rbs/steep run locally on the PR head.

@abbaseya
abbaseya merged commit e5a8bb5 into feat/fullstack-v12 Jul 15, 2026
12 checks passed
@abbaseya
abbaseya deleted the feat/experiment-preview branch July 15, 2026 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants