Skip to content

epic-3/story-3: Add Default Segments and Custom Segment Evaluation - #35

Closed
usmanabbas7 wants to merge 10 commits into
epic-3/story-2-support-mutable-visitor-state-on-contextsfrom
epic-3/story-3-add-default-segments-and-custom-segment-evaluation
Closed

epic-3/story-3: Add Default Segments and Custom Segment Evaluation#35
usmanabbas7 wants to merge 10 commits into
epic-3/story-2-support-mutable-visitor-state-on-contextsfrom
epic-3/story-3-add-default-segments-and-custom-segment-evaluation

Conversation

@usmanabbas7

@usmanabbas7 usmanabbas7 commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Story 3.3: Add Default Segments and Custom Segment Evaluation

Part of sprint sprint/2026-04-06-convert-python-sdk. Stacked on epic-3/story-2 (#34 → #33 → … → #24). Segments-only (FR14 + FR15); entity-lookup is the separate Story 3.4.

What was built

  • domain/context_state.py — DISTINCT immutable default_segments field on the frozen ContextState + with_segments(...) immutable update (mirrors 3.2's with_attributes); kept separate from visitor_attributes.
  • context.py — public Context.set_segments(segments: dict[str, str]) -> None (default-segment association) and Context.run_custom_segments(segment_keys, rule_data=None) -> CustomSegmentsResult | None (local custom-segment evaluation). Both reuse the 3.2 rebind + persist-through-DataStore pattern and the 3.1/3.2 visitor_state_key() derivation.
  • evaluation/segments.py (NEW, L2) — resolves ConfigSegment entities from the immutable ConfigSnapshot and delegates each rule match to the existing evaluation/rules.py engine (Story 1.4). Fully local, no network.
  • core.py — persisted ContextState evolved to a structured envelope {"attributes": {...}, "segments": {...}} with backward-compatible hydration of the 3.2 plain-attributes dict; create_context rehydrates segments through the SAME store/key path.
  • tracking/conversions.py + tracker.py — wires the visitor's active default_segments into the conversion event's segments so a tracked conversion's payload reflects them (the seam tracking/payloads.py:_build_visitor_segments was waiting on per Story 2.2's TODO). Serializer shape unchanged.
  • domain/results.py — new typed CustomSegmentsResult (exported from convert_sdk/__init__.py).

Naming resolution

Public methods are the PRD-frozen Pythonic names set_segments / run_custom_segments (NOT JS setDefaultSegments/runCustomSegments, NOT the superseded bundled file's set_default_segments). Per the story's "Concrete Story 3.3 decisions" + Critical Warning #3.

Beads

Epic ai-driven-product-dev-52q2; tasks -x45k, -xsac, -l3jz, -sqzb, -gze9 — all closed.

Tests

415 → 462 (+47), zero regressions. New tests/test_segments.py; extended tests/test_context_creation.py, tests/test_context_state.py, tests/test_conversion_tracking.py, tests/test_layering.py, tests/integration/test_in_memory_store.py (qs-06 round-trip). Two Story 3.2 internal-shape assertions updated for the envelope serialization (behavior unchanged).

New public export

CustomSegmentsResult added to __all__; all previously frozen exports unchanged.

Sprint-mode notes

  • Readiness gate: PASS 9.0/10, 4 questions auto-delegated ("your call": result-type shape, reporting-carry wiring point, default_segments value type, persisted serialization shape). See conductor's work/2026-06-08-add-default-segments-and-custom-segment-evaluation/readiness-assessment.md.
  • Code review: clean, 1 round. One sub-75-confidence note (legacy-vs-envelope hydrate detection edge case on an unreleased legacy format) — no fix filed.

🤖 Generated with Claude Code

@usmanabbas7 usmanabbas7 self-assigned this Jun 8, 2026
@usmanabbas7
usmanabbas7 requested a review from clllaur June 8, 2026 08:35
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Jun 8, 2026
View reviewed changes

@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 custom segment evaluation and default segment persistence (Story 3.3). It introduces a new CustomSegmentsResult class, adds local custom segment evaluation logic in segments.py, and updates Context, ContextState, and Core to handle default_segments separately from visitor attributes. Additionally, conversion tracking has been updated to carry these segments, and comprehensive tests have been added. Feedback on the changes suggests defensively guarding the iteration over snapshot.segments in _resolve_segments to prevent potential TypeError or AttributeError if the segments list or its elements are None.

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 src/convert_sdk/evaluation/segments.py
Comment on lines +49 to +53
by_key = {
str(segment.get("key")): segment
for segment in snapshot.segments
if segment.get("key") is not None
}

@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 prevent a potential TypeError if snapshot.segments is None, or an AttributeError if any element within snapshot.segments is None, we should defensively guard the iteration and dictionary comprehension.

Suggested change
by_key = {
str(segment.get("key")): segment
for segment in snapshot.segments
if segment.get("key") is not None
}
by_key = {
str(segment.get("key")): segment
for segment in (snapshot.segments or [])
if segment is not None and segment.get("key") is not None
}

This was referenced Jun 8, 2026
Closed
Closed
Closed
Closed
usmanabbas7 and others added 9 commits June 14, 2026 21:59
@usmanabbas7 @claude
wf(wf-3p3seg): [SDK-1] ContextState default_segments — tests (RED)
69cc319 
Beads: ai-driven-product-dev-x45k

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@usmanabbas7 @claude
wf(wf-3p3seg): [SDK-1] ContextState default_segments + with_segments …
cb5d5a6 
…— implementation (GREEN)

Beads: ai-driven-product-dev-x45k

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@usmanabbas7 @claude
wf(wf-3p3seg): [SDK-2] set_segments persist + hydrate — tests (RED)
338afcd 
Beads: ai-driven-product-dev-xsac

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@usmanabbas7 @claude
wf(wf-3p3seg): [SDK-2] Context.set_segments + persist envelope + hydr…
463cf5b 
…ate — implementation (GREEN)

Beads: ai-driven-product-dev-xsac

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@usmanabbas7 @claude
wf(wf-3p3seg): [SDK-3] custom-segment evaluation — tests (RED)
a14d166 
Beads: ai-driven-product-dev-l3jz

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@usmanabbas7 @claude
wf(wf-3p3seg): [SDK-3] evaluation/segments.py + run_custom_segments —…
9d10fae 
… implementation (GREEN)

Beads: ai-driven-product-dev-l3jz

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@usmanabbas7 @claude
wf(wf-3p3seg): [SDK-4] reporting carry — tests (RED)
30b00a3 
Beads: ai-driven-product-dev-sqzb

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@usmanabbas7 @claude
wf(wf-3p3seg): [SDK-4] reporting carry default_segments into conversi…
8795f6b 
…on segments — implementation (GREEN)

Beads: ai-driven-product-dev-sqzb

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@usmanabbas7 @claude
wf(wf-3p3seg): [SDK-5] FR25 determinism + layering + public export + …
231dc35 
…integration round-trip — tests (GREEN)

Beads: ai-driven-product-dev-gze9

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@usmanabbas7
usmanabbas7 force-pushed the epic-3/story-2-support-mutable-visitor-state-on-contexts branch from c9d9d13 to 2298a37 Compare June 14, 2026 16:59
@usmanabbas7
usmanabbas7 force-pushed the epic-3/story-3-add-default-segments-and-custom-segment-evaluation branch from e29bd5f to 231dc35 Compare June 14, 2026 16:59
@usmanabbas7 @claude
patch(convert-python-sdk-3-3): implement JS/PHP segments_matched latc…
f2d8916 
…h in custom-segment evaluation — F-066

select_custom_segments evaluated each segment's rule independently, diverging
from the JS (segments-manager.ts:100-121) and PHP (SegmentsManager.php:104-128)
`segmentsMatched` latch: once an earlier segment's rule matches, subsequent
segments are recorded without re-evaluating their own rules. Ordered list
[us-visitors, de-visitors] for a US visitor now records [s_us, s_de] (was [s_us]).
Rule-less-unconditional, duplicate-skip, and typed-no-match semantics preserved.
Added two ordered-multi-segment parity tests. Full suite green (472 passed);
segments.py passes ruff + mypy --strict cleanly (pre-existing repo lint/type
debt in other files is untouched).

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

usmanabbas7 commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

F-066 — custom-segment matching now applies the JS/PHP segments_matched latch

Defect (verified parity divergence). evaluation/segments.py select_custom_segments evaluated each segment's rule independently, while JS (segments-manager.ts:100-121) and PHP (SegmentsManager.php:104-128) latch a segmentsMatched flag: once an earlier segment's rule matches, subsequent segments are recorded without re-evaluating their own rules. Ordered list [us-visitors, de-visitors] for a US visitor returned [s_us] here vs [s_us, s_de] in JS/PHP.

Fix. Implemented the latch (set once, re-evaluated only until the first match), preserving rule-less-unconditional, duplicate-skip, and typed-no-match semantics.

Sweep. evaluation/segments.py is the only site of this control flow; context.py only delegates. No other occurrence.

Tests. Added two ordered-multi-segment parity tests. Local gates:

  • uv run pytest → 472 passed
  • uv run pytest tests/test_segments.py → 23 passed · tests/parity → 82 passed
  • uv run ruff check + uv run mypy --strict on the changed files → clean (pre-existing repo lint/type debt in unrelated files is untouched; cleaned up downstream in story 5-1)

CI gate. No CI workflow on this branch → no-ci (verified: no .github/workflows/).

Spec patched separately (AC #6 + corrected parity testing note + F-066 post-mortem). Commit f2d8916.

@abbaseya

abbaseya commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Superseded — all commits already in main (bc76b64). Closing without merge as part of post-sprint cleanup.

@abbaseya abbaseya closed this Jun 18, 2026
@abbaseya
abbaseya deleted the epic-3/story-3-add-default-segments-and-custom-segment-evaluation branch June 18, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clllaur clllaur Awaiting requested review from clllaur
1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@usmanabbas7 usmanabbas7

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@usmanabbas7 @abbaseya