chore(retention): make first-time anchor entity-polymorphic

[thmsobrmlr]

Refactor to make "first time" feature possible in follow-up work.

Problem

RetentionBaseQueryBuilder.get_first_time_anchor_expr is hard-coded to minIf(events.timestamp, <events_entity_predicate>). That coupling blocks every downstream track that wants to resolve a first-ever anchor against a data warehouse entity from a single shared primitive — both the strict-calendar-date first-time parity work and the 24-hour-window DWH retrofit need to fan out from the same place.

Changes

Make the first-time anchor primitive polymorphic over RetentionEntity:

• get_first_time_anchor_expr(entity) now takes a RetentionEntity and routes by entity.type.
• For EntityType.EVENTS, the produced expression is identical to before: minIf(events.timestamp, <events_entity_predicate>) with property-stripped and property-included variants for the first-ever case.
• For EntityType.DATA_WAREHOUSE, the expression becomes minIf(<entity.table_name>.<entity.timestamp_field>, <predicate>), where the predicate is property_to_expr(entity.properties) if properties are configured and Constant(True) otherwise.
• The two existing call sites (retention_base_query_fixed.py, retention_base_query_rolling.py) pass self.start_event to the helper.
• start_entity_expr_no_props becomes a thin events-default wrapper around the new polymorphic entity_expr_no_props(entity) — the breakdown argMinIf caller is unchanged because DWH breakdowns are out of scope for this slice.

Pure refactor for events callers — zero behaviour change. No DWH call site uses the new branch yet; that wiring lands in subsequent slices.

How did you test this code?

I'm an agent (Claude). I only ran automated tests:

• New focused unit tests in posthog/hogql_queries/insights/retention/test/test_retention_first_time_anchor.py exercise the four interesting shapes — events first-ever, events first-time-matching, DWH with properties, DWH without properties. All four pass.
• Confirmed the existing first-time retention test suite still collects 23 tests in test_retention_query_runner.py with the new signature. CI will run them against ClickHouse.
• Confirmed zero remaining events.timestamp references inside first-time anchor logic across posthog/hogql_queries/insights/retention/. Other hits (breakdown argMinIf, rolling-t0 non-first-time path, generic events-only paths) are deliberate and out of scope per the ticket.
• Confirmed retention_base_query_variant_comparison_excluded_tests in both test classes is byte-identical — this refactor closes no parity gap on its own.

Publish to changelog?

no

🤖 Agent context

Authored by Claude Code (Opus 4.7, 1M context) via the /tdd skill against a Notion ticket titled "Foundation: entity-polymorphic first-time anchor".

Approach:

• Followed the TDD skill's vertical-slice discipline: events-regression test → signature refactor; DWH-with-properties test → DWH branch; DWH-no-properties test → truthy-constant predicate.
• Picked unit tests over a full snapshot diff because the helper's contract is structural (which AST nodes appear where) and a stub MagicMock runner is enough — no Django Team, no ClickHouse. That bypassed a broken local CH setup without sacrificing coverage of the meaningful branches.
• One AST observation that surfaced via TDD: parse_expr substitution shares nodes, so a tree walk counts the reused minIf more than twice for the first-ever shape. The DWH-no-properties test asserts on the property of every predicate (isinstance(..., Constant) and truthy) rather than an exact count.
• The work was originally proven on the main checkout but kept getting reset by an external process touching these specific files; final application happened inside an isolated git worktree.

Decisions:

• Kept start_entity_expr_no_props as a cached_property wrapper instead of deleting it. The breakdown call site still depends on it for events, DWH breakdown is downstream work, and keeping the wrapper means existing snapshot tests don't move.
• Added entity_timestamp_field, entity_expr_with_props, and entity_expr_no_props as public methods on the builder rather than _private helpers — the next two tracks will compose them.
• Did not touch RetentionBaseQueryVariantComparisonMixin exclusion lists. The ticket explicitly says no parity gap is closed by this slice.

[github-actions]

🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down.

Add the run-playwright label if you want an E2E sweep before merging — CI will pick it up automatically.

Most PRs don't need this. Real regressions still get caught on master and fix-forward.

[greptile-apps]

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
posthog/hogql_queries/insights/retention/retention_base_query_builder.py:71-74
`assert` for precondition checks in a method reached via user-configured schema data (`RetentionEntity.table_name` / `timestamp_field` are both `Optional`). When either field is `None` the assertion fires an unhelpful bare `AssertionError`, and under `python -O` the check is silently stripped, producing `ast.Field(chain=[None, None])` and a confusing downstream error instead.

```suggestion
    def entity_timestamp_field(self, entity: RetentionEntity) -> ast.Expr:
        if entity.type == EntityType.DATA_WAREHOUSE:
            if not entity.table_name or not entity.timestamp_field:
                raise ValueError(
                    f"DATA_WAREHOUSE RetentionEntity requires table_name and timestamp_field, "
                    f"got table_name={entity.table_name!r}, timestamp_field={entity.timestamp_field!r}"
                )
            return ast.Field(chain=[entity.table_name, entity.timestamp_field])
```

### Issue 2 of 3
posthog/hogql_queries/insights/retention/test/test_retention_first_time_anchor.py:13-62
**Duplicate AST traversal logic** — `_collect_field_chains`, `_collect_constants`, and `_collect_min_if_predicates` each contain identical recursive walk logic: visit a dataclass's fields, then recurse into lists. Only the leaf-node collection differs. A single generic walker (e.g. `_walk_ast(node, collect_fn)`) would express the idea once, making it easier to handle edge cases (e.g. dict-valued fields) consistently across all three helpers.

### Issue 3 of 3
posthog/hogql_queries/insights/retention/test/test_retention_first_time_anchor.py:84-167
**Non-parameterised tests** — all four methods exercise the same function (`get_first_time_anchor_expr`) with different entity fixtures and expected AST properties. The team prefers parameterised tests; a single `@pytest.mark.parametrize` case list with `(entity, is_first_ever, is_first_matching, expected_chains, unexpected_chains, expected_constants)` tuples would eliminate the repeated builder/assert boilerplate and make adding future shapes trivial.

_{Reviews (1): Last reviewed commit: "chore(retention): make first-time anchor..." | Re-trigger Greptile}

[gesh]

Looks good!

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-05-27 10:36 UTC	Run
prod-us	✅ Deployed	2026-05-27 10:47 UTC	Run
prod-eu	✅ Deployed	2026-05-27 10:49 UTC	Run