Skip to content

fix(cohorts): consult correct fields for time windows#66394

Merged
matheus-vb merged 1 commit into
masterfrom
matheus-vb/fix-explicit_datetime-ignored
Jun 26, 2026
Merged

fix(cohorts): consult correct fields for time windows#66394
matheus-vb merged 1 commit into
masterfrom
matheus-vb/fix-explicit_datetime-ignored

Conversation

@matheus-vb

Copy link
Copy Markdown
Member

Problem

A realtime behavioral cohort like OR(performed_event_multiple $pageview gte 3 [last 7 days], person email is_set) loaded but emitted nothing to the shadow topic.
The cohort UI stores a relative window such as "last 7 days" as explicit_datetime: "-7d" with no time_interval/time_value, but the performed_event_multiple window resolver effective_window_days read only time_interval/time_value.
With no interval it returned 0, which maps to HourlyDeferred, drops the leaf, and collapses the whole cohort to Excluded(HasDroppedLeaf).
The single performed_event path already consulted explicit_datetime, so only _multiple diverged from the production oracle.

The service in only deployed in the dev environment for now.

Changes

effective_window_days now consults explicit_datetime(_to) first, reusing the single path's explicit_eviction_window classifier via a new explicit_window_days helper.
A relative lower only bound ("-7d", "-1y") resolves to its sliding window; every other explicit shape (sub day, absolute range, two sided/relative upper, unparseable) still returns 0 and drops, exactly as before.
The signature stays u32, so both call sites are untouched, and state keys already discriminate explicit_datetime, so no key change is needed.

How did you test this code?

Automated unit tests:
pick_state.rs covers window day routing and parity with the time_value/time_interval path.
leaf_classifier.rs covers Keep/Drop classification of explicit _multiple leaves.
reverse_index.rs covers the frozen window_days/predicate_op metadata and the same parity.
cargo test -p cohort-stream-processor --lib passes 737 tests and cargo clippy is clean.

@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "consult correct fields + tests" | Re-trigger Greptile

@matheus-vb matheus-vb added the stamphog Request AI approval (no full review) label Jun 26, 2026

@github-actions github-actions Bot 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.

Bug fix corrects performed_event_multiple to consult explicit_datetime fields (mirroring the single-event path) by delegating to the existing explicit_eviction_window helper. Logic is isolated, reuses proven code, and is covered by 7 new parametrized tests that verify edge cases and parity with the time_value/time_interval path.

@github-project-automation github-project-automation Bot moved this from In Review to Approved in Feature Flags Jun 26, 2026
@matheus-vb matheus-vb merged commit 4f45a21 into master Jun 26, 2026
225 of 226 checks passed
@matheus-vb matheus-vb deleted the matheus-vb/fix-explicit_datetime-ignored branch June 26, 2026 16:50
@github-project-automation github-project-automation Bot moved this from Approved to Done in Feature Flags Jun 26, 2026
@deployment-status-posthog

deployment-status-posthog Bot commented Jun 26, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-06-26 17:33 UTC Run
prod-us ✅ Deployed 2026-06-26 17:58 UTC Run
prod-eu ✅ Deployed 2026-06-26 18:02 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant