Skip to content

epic-4/story-3: Support Cross-SDK Debugging #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
usmanabbas7 wants to merge 2 commits into from
Closed

epic-4/story-3: Support Cross-SDK Debugging #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4660935
wf(wf-43xsdk): [SDK-1] cross-SDK debugging — tests (RED)
usmanabbas7 Jun 8, 2026
05162b0
wf(wf-43xsdk): [SDK-1] cross-SDK debugging — implementation (GREEN)
usmanabbas7 Jun 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 87 additions & 8 deletions src/convert_sdk/context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

import logging
from types import MappingProxyType
from typing import TYPE_CHECKING, Any, List, Mapping, Optional
from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional

@gemini-code-assist gemini-code-assist Bot Jun 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The codebase consistently uses modern Python 3.9+ built-in generic types (like dict[str, Any] and list[str]) instead of typing.Dict or typing.List. We should avoid importing Dict from typing and use the built-in dict instead to maintain consistency.

Suggested change
from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional
from typing import TYPE_CHECKING, Any, List, Mapping, Optional


from convert_sdk._internal.redaction import SafeContext, fingerprint_visitor
from convert_sdk.domain.context_state import ContextState
Expand All @@ -41,6 +41,7 @@
GoalDiagnostic,
)
from convert_sdk.evaluation import entity_lookup
from convert_sdk.evaluation.bucketing import get_bucket_value_for_visitor
from convert_sdk.evaluation.experiences import select_experience
from convert_sdk.evaluation.features import resolve_feature, resolve_features
from convert_sdk.evaluation.segments import select_custom_segments
Expand Down Expand Up @@ -89,6 +90,7 @@ def __init__(
location_attributes: Optional[Mapping[str, Any]] = None,
tracker: Optional["Tracker"] = None,
data_store: Optional["DataStore"] = None,
environment: Optional[str] = None,
) -> None:
# Visitor identity + stored attributes + default segments + snapshot
# linkage live in the typed ContextState (visitor state stays separate
Expand All @@ -113,6 +115,13 @@ def __init__(
self._location_attributes: Mapping[str, Any] = MappingProxyType(
dict(location_attributes or {})
)
# Story 4.3: the SDK config environment (or None for a directly
# constructed context). It is an allowlist-safe operational field
# (NFR6) included in the cross-SDK-comparable diagnostic field set so
# diagnostics captured in a mixed Python/JS deployment carry the same
# environment qualifier. Optional + defaulting to None keeps the
# constructor backward compatible (Critical Warning #1).
self._environment = environment
Comment on lines +118 to +124

@gemini-code-assist gemini-code-assist Bot Jun 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To avoid repeatedly recomputing the SHA-256 hash of the static visitor_id on every log emission and diagnostic call, we can precompute and cache the hashed visitor reference (self._visitor_ref) once during Context initialization.

Suggested change
# Story 4.3: the SDK config environment (or None for a directly
# constructed context). It is an allowlist-safe operational field
# (NFR6) included in the cross-SDK-comparable diagnostic field set so
# diagnostics captured in a mixed Python/JS deployment carry the same
# environment qualifier. Optional + defaulting to None keeps the
# constructor backward compatible (Critical Warning #1).
self._environment = environment
# Story 4.3: the SDK config environment (or None for a directly
# constructed context). It is an allowlist-safe operational field
# (NFR6) included in the cross-SDK-comparable diagnostic field set so
# diagnostics captured in a mixed Python/JS deployment carry the same
# environment qualifier. Optional + defaulting to None keeps the
# constructor backward compatible (Critical Warning #1).
self._environment = environment
self._visitor_ref = fingerprint_visitor(visitor_id)


@property
def visitor_id(self) -> str:
Expand Down Expand Up @@ -335,23 +344,45 @@ def _log_conversion(self, result: "ConversionResult") -> None:
outcome=result.status.value,
)

def _log_diagnostic(self, entity_key: Optional[str], reason: DiagnosticReason) -> None:
def _log_diagnostic(
self,
entity_key: Optional[str],
reason: DiagnosticReason,
*,
environment: Optional[str] = None,
bucket_value: Optional[int] = None,
variation_key: Optional[str] = None,
) -> None:
"""Emit an additive, allowlist-only diagnostic-outcome log record (FR52).

Routes through the SAME Story 4.1 :func:`log_safe` seam every other SDK
log call site uses (no separate diagnostics module) so support teams see
the identical closed ``reason`` code in logs and in the returned typed
diagnostic. Carries ONLY the entity key, the reason code, and a HASHED
visitor reference — never the raw ``visitor_id``, visitor attributes, or
any PII (NFR6/NFR51, Critical Warning #3). Observational only — it runs
after the diagnostic outcome is already determined and cannot change it.
diagnostic. Story 4.3 mirrors the partial cross-SDK-comparable field set
(``reason``, ``environment``, ``bucket_value``, ``variation_key``, and a
HASHED visitor reference) into the log record so a mixed Python/JS
deployment can correlate diagnostic output for the same scenario. It
carries ONLY allowlist-safe fields — never the raw ``visitor_id``,
visitor attributes, or any PII (NFR6/NFR51, Critical Warning #3).
Observational only — it runs after the diagnostic outcome is already
determined and cannot change it.
"""
# Only emit allowlist-safe fields that are present (omit None) so the
# record stays compact and the partial field set is honest about misses.
optional: Dict[str, Any] = {}
if environment is not None:
optional["environment"] = environment
if bucket_value is not None:
optional["bucket_value"] = bucket_value
if variation_key is not None:
optional["variation_key"] = variation_key
log_safe(
LifecycleEvent.DIAGNOSTIC,
level=logging.DEBUG,
context=SafeContext(entity_key=entity_key),
visitor=fingerprint_visitor(self._state.visitor_id),
reason=reason.value,
**optional,
)
Comment on lines +372 to 386

@gemini-code-assist gemini-code-assist Bot Jun 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Use the built-in dict generic instead of Dict from typing for consistency with the rest of the codebase. Additionally, leverage the cached self._visitor_ref to avoid recomputing the SHA-256 hash of the visitor ID.

Suggested change
optional: Dict[str, Any] = {}
if environment is not None:
optional["environment"] = environment
if bucket_value is not None:
optional["bucket_value"] = bucket_value
if variation_key is not None:
optional["variation_key"] = variation_key
log_safe(
LifecycleEvent.DIAGNOSTIC,
level=logging.DEBUG,
context=SafeContext(entity_key=entity_key),
visitor=fingerprint_visitor(self._state.visitor_id),
reason=reason.value,
**optional,
)
optional: dict[str, Any] = {}
if environment is not None:
optional["environment"] = environment
if bucket_value is not None:
optional["bucket_value"] = bucket_value
if variation_key is not None:
optional["variation_key"] = variation_key
log_safe(
LifecycleEvent.DIAGNOSTIC,
level=logging.DEBUG,
context=SafeContext(entity_key=entity_key),
visitor=self._visitor_ref,
reason=reason.value,
**optional,
)


# --- evaluation surface ------------------------------------------------
Expand Down Expand Up @@ -689,12 +720,23 @@ def diagnose_experience(
location_attributes=location,
)
if result is not None:
# Story 4.3: recompute the deterministic bucket value for the
# resolved experience so the diagnostic carries the cross-SDK
# comparable bucketing outcome. This mirrors the value
# ``select_experience`` used internally (same visitor + experience
# id → same value against the same snapshot) without changing the
# frozen ``ExperienceResult`` public shape.
bucket_value = get_bucket_value_for_visitor(
self._state.visitor_id, experience_id=result.experience_id
)
return self._diagnose(
ExperienceDiagnostic,
experience_key,
DiagnosticReason.RESOLVED,
"experience resolved to a variation",
{"experience_key": experience_key, "variation_key": result.variation_key},
bucket_value=bucket_value,
variation_key=result.variation_key,
)
# The experience exists but produced no result: the visitor did not
# qualify for (or bucket within) it — surfaced as an audience mismatch.
Expand Down Expand Up @@ -824,15 +866,52 @@ def _diagnose(
reason: DiagnosticReason,
message: str,
details: Mapping[str, Any],
*,
bucket_value: Optional[int] = None,
variation_key: Optional[str] = None,
) -> Any:
"""Build a typed diagnostic and mirror miss-path reasons to the log seam.

Centralizes the log emission so every ``diagnose_*`` path emits the SAME
allowlist-only, hashed-visitor diagnostic record through Story 4.1's
:func:`log_safe`. A ``RESOLVED`` outcome is not a miss, so it is not
logged (parity with the observational bucketing/conversion logs).

Story 4.3: the typed diagnostic's read-only ``details`` mapping (Story
4.2's frozen surface — NOT new top-level fields) is augmented with the
partial cross-SDK-comparable field set so a mixed Python/JS deployment
can compare diagnostic output for the same visitor scenario:

* ``reason`` — the closed :class:`DiagnosticReason` code value.
* ``environment`` — the SDK config environment (``None`` when unwired).
* ``visitor_ref`` — the HASHED visitor reference via
:func:`~convert_sdk._internal.redaction.fingerprint_visitor`; the raw
``visitor_id`` NEVER appears (NFR6/NFR51, Critical Warning #1).
* ``bucket_value`` / ``variation_key`` — present only on a resolved
experience diagnostic (the only path that buckets); ``None`` otherwise.

The AC-1 fields ``config_version``, ``bucketing_inputs`` (key/traffic/
seed/salt), and ``experience_key`` completion are intentionally DEFERRED
to Story 4.5 and are NOT emitted here. The formal byte-comparable
contract document is owned by Story 4.5; the parity-comparison helper +
diagnostic-vector fixtures are owned by Story 5.1.
"""
diagnostic = cls(reason=reason, message=message, details=details)
# Merge the comparable field set onto the caller's allowlist-safe
# details WITHOUT mutating the caller's mapping. The _Diagnostic
# dataclass re-wraps this read-only in __post_init__.
comparable: Dict[str, Any] = dict(details)
comparable["reason"] = reason.value
comparable["environment"] = self._environment
comparable["visitor_ref"] = fingerprint_visitor(self._state.visitor_id)
comparable["bucket_value"] = bucket_value
comparable["variation_key"] = variation_key
diagnostic = cls(reason=reason, message=message, details=comparable)
Comment on lines +902 to +908

@gemini-code-assist gemini-code-assist Bot Jun 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Use the built-in dict generic instead of Dict from typing for consistency with the rest of the codebase. Additionally, leverage the cached self._visitor_ref to avoid recomputing the SHA-256 hash of the visitor ID.

Suggested change
comparable: Dict[str, Any] = dict(details)
comparable["reason"] = reason.value
comparable["environment"] = self._environment
comparable["visitor_ref"] = fingerprint_visitor(self._state.visitor_id)
comparable["bucket_value"] = bucket_value
comparable["variation_key"] = variation_key
diagnostic = cls(reason=reason, message=message, details=comparable)
comparable: dict[str, Any] = dict(details)
comparable["reason"] = reason.value
comparable["environment"] = self._environment
comparable["visitor_ref"] = self._visitor_ref
comparable["bucket_value"] = bucket_value
comparable["variation_key"] = variation_key
diagnostic = cls(reason=reason, message=message, details=comparable)

if reason is not DiagnosticReason.RESOLVED:
self._log_diagnostic(entity_key, reason)
self._log_diagnostic(
entity_key,
reason,
environment=self._environment,
bucket_value=bucket_value,
variation_key=variation_key,
)
return diagnostic
3 changes: 3 additions & 0 deletions src/convert_sdk/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ def create_context(
location_attributes=location_attributes,
tracker=self._tracker,
data_store=self._data_store,
# Story 4.3: forward the config environment so per-visitor
# diagnostics carry the cross-SDK-comparable environment qualifier.
environment=self._config.environment,
)

def _hydrate_visitor_state(
Expand Down
190 changes: 190 additions & 0 deletions tests/test_cross_sdk_debugging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
"""Story 4.3 — Support Cross-SDK Debugging (FR48, partial AC #1).

This story extends the EXISTING diagnostic surface (Story 4.1's ``log_safe``
seam + Story 4.2's typed ``_Diagnostic.details`` mapping) so a diagnostic
carries the partial cross-SDK-comparable field set IN SCOPE for 4.3:

* ``reason`` — the closed Story 4.2 ``DiagnosticReason`` code value
* ``environment`` — the SDK config environment (or ``None`` for a directly
constructed context with no environment wired)
* ``bucket_value`` — the deterministic bucketing value for a resolved
experience (``None`` on a miss / non-experience diagnostic)
* ``variation_key`` — the selected variation key for a resolved experience
* ``visitor_ref`` — a HASHED visitor reference via
:func:`convert_sdk._internal.redaction.fingerprint_visitor` (NEVER the raw id)

Deferred (NOT exercised here): ``config_version``, ``bucketing_inputs``
(key/traffic/seed/salt), ``experience_key`` completion (Story 4.5); the
parity-comparison helper + diagnostic-vector fixtures (Story 5.1); the formal
byte-comparable contract document ``docs/debugging.md`` (Story 4.5).

Guardrails (driver scope + story Architecture): there is NO ``diagnostics.py``
module, NO ``events.visitor_reference`` helper. The comparable fields live in
``_Diagnostic.details`` (4-2's read-only mapping) and are mirrored through
``log_safe`` (4-1). The visitor id appears ONLY as a ``fingerprint_visitor``
hash. The JS SDK has no diagnostic visitor-hash mechanism, so visitor-reference
byte-comparable parity is NOT asserted across SDKs.
"""

from __future__ import annotations

import contextlib
import logging

from convert_sdk._internal.redaction import fingerprint_visitor
from convert_sdk.context import Context
from convert_sdk.domain.config_snapshot import ConfigSnapshot
from convert_sdk.domain.results import DiagnosticReason, ExperienceDiagnostic

VISITOR = "visitor-cross-sdk-001"

# One always-qualifying experience (resolves + buckets) and one unknown key.
RESOLVING_CONFIG = {
"account_id": "100123",
"project": {"id": "200456"},
"experiences": [
{
"id": "e1",
"key": "open-exp",
"variations": [
{"id": "v1", "key": "treat", "traffic_allocation": 100.0},
],
},
],
"features": [],
"goals": [{"id": "g1", "key": "signup"}],
}

#: The partial cross-SDK-comparable field set in scope for Story 4.3.
PARTIAL_CONTRACT_FIELDS = {
"reason",
"environment",
"bucket_value",
"variation_key",
"visitor_ref",
}

#: Fields required by AC-1 but DEFERRED — must NOT have been smuggled in early.
DEFERRED_AC1_FIELDS = {"config_version", "bucketing_inputs", "seed", "salt", "traffic"}


def _context(*, environment=None, visitor=VISITOR, config=RESOLVING_CONFIG):
return Context(
visitor,
ConfigSnapshot.from_normalized(config),
environment=environment,
)


# --- Task 4.1: capture diagnostic output + assert the partial contract -------


def test_resolved_experience_diagnostic_carries_partial_cross_sdk_field_set():
"""A RESOLVED experience diagnostic exposes the full partial field set."""
diag = _context(environment="prod").diagnose_experience("open-exp")
assert isinstance(diag, ExperienceDiagnostic)
assert diag.reason is DiagnosticReason.RESOLVED

details = diag.details
assert PARTIAL_CONTRACT_FIELDS.issubset(set(details.keys()))
assert details["reason"] == DiagnosticReason.RESOLVED.value
assert details["environment"] == "prod"
assert details["variation_key"] == "treat"
# bucket_value is the deterministic bucketing value for the resolved exp.
assert isinstance(details["bucket_value"], (int, float))
assert details["visitor_ref"] == fingerprint_visitor(VISITOR)


def test_partial_contract_does_not_leak_deferred_ac1_fields():
"""The deferred AC-1 fields (Story 4.5) must NOT appear yet."""
details = _context(environment="prod").diagnose_experience("open-exp").details
assert DEFERRED_AC1_FIELDS.isdisjoint(set(details.keys()))


def test_miss_diagnostic_carries_reason_environment_and_hashed_visitor():
"""A miss still carries reason/environment/visitor_ref; bucket/variation absent-or-None."""
diag = _context(environment="staging").diagnose_experience("nope")
assert diag.reason is DiagnosticReason.EXPERIENCE_NOT_FOUND
details = diag.details
assert details["reason"] == DiagnosticReason.EXPERIENCE_NOT_FOUND.value
assert details["environment"] == "staging"
assert details["visitor_ref"] == fingerprint_visitor(VISITOR)
# No bucketing happened on a miss.
assert details.get("bucket_value") is None
assert details.get("variation_key") is None


def test_environment_defaults_to_none_when_not_wired():
"""A directly-constructed context with no environment yields environment=None."""
details = _context().diagnose_experience("open-exp").details
assert details["environment"] is None


def test_visitor_id_never_appears_raw_in_details_or_log():
"""NFR6/NFR51: the raw visitor id never leaks — only the fingerprint hash."""
pii_visitor = "raw-visitor-id-leak-check"
ctx = Context(
pii_visitor,
ConfigSnapshot.from_normalized(RESOLVING_CONFIG),
environment="prod",
visitor_attributes={"email": "user@co.com", "name": "Jane"},
)
with _caplog_at_debug() as records:
diag = ctx.diagnose_experience("open-exp")
log_text = "\n".join(r.getMessage() for r in records if r.name == "convert_sdk")
details_repr = repr(dict(diag.details))

for sink in (log_text, details_repr):
assert pii_visitor not in sink
assert "user@co.com" not in sink
assert "Jane" not in sink
# The hashed reference is what appears instead.
assert fingerprint_visitor(pii_visitor) == diag.details["visitor_ref"]


def test_diagnostic_log_mirrors_partial_field_set():
"""Miss-path diagnostic log carries reason + environment + hashed visitor."""
ctx = Context(
VISITOR,
ConfigSnapshot.from_normalized(RESOLVING_CONFIG),
environment="prod",
)
with _caplog_at_debug() as records:
ctx.diagnose_experience("nope")
text = "\n".join(r.getMessage() for r in records if r.name == "convert_sdk")
assert DiagnosticReason.EXPERIENCE_NOT_FOUND.value in text
assert "prod" in text
assert fingerprint_visitor(VISITOR) in text


def test_details_mapping_stays_read_only():
"""Comparable fields live in the frozen 4-2 read-only details mapping."""
import pytest

details = _context(environment="prod").diagnose_experience("open-exp").details
with pytest.raises(TypeError):
details["environment"] = "tamper" # type: ignore[index]


@contextlib.contextmanager
def _caplog_at_debug():
"""Capture ``convert_sdk`` records at DEBUG via a temporary handler."""

class _Capture(logging.Handler):
def __init__(self):
super().__init__()
self.records = []

def emit(self, record):
self.records.append(record)

logger = logging.getLogger("convert_sdk")
handler = _Capture()
prev_level = logger.level
logger.setLevel(logging.DEBUG)
logger.addHandler(handler)
try:
yield handler.records
finally:
logger.removeHandler(handler)
logger.setLevel(prev_level)