feat(cohorts): add automatic backfill for person property changes

[gustavohstrassburger]

Problem

When realtime cohorts with person property filters are updated, the person property data isn't automatically backfilled. This can lead to inconsistent data until backfill is manually triggered.

Changes

• Added automatic backfill triggering for realtime cohorts when person property conditions change

How did you test this code?

• Added 21 new test cases covering all backfill scenarios
• Tests verify proper triggering conditions (realtime cohorts, person properties, feature flag)
• Tests verify proper skipping conditions (static cohorts, non-realtime, disabled flag)
• Tests verify structural change detection with identical conditions but different logical structure
• All 44 dependency tests pass

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds automatic triggering of person-property backfill workflows when realtime cohort person-property filter conditions change, to avoid inconsistent cohort membership until a manual backfill is run.

Changes:

• Added pre-save + post-save signal flow to detect person-property filter changes and trigger backfill behind a feature flag.
• Implemented structural hashing of person-property filter trees to detect logical-structure changes.
• Added extensive unit test coverage for triggering/skipping scenarios and structural change detection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
posthog/models/cohort/dependencies.py	Adds change detection + backfill triggering signals and filter-tree hashing helpers.
posthog/models/cohort/test/test_dependencies.py	Adds tests covering person-property detection, hashing/change detection, and backfill trigger gating.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Vulnerabilities

No security concerns identified. The feature flag check (posthoganalytics.feature_enabled) gates the backfill trigger, and all operations are scoped to the cohort's own team_id. No user-supplied data is passed unsanitized to the management command.

Comments Outside Diff (1)

1. posthog/models/cohort/dependencies.py, line 322-347 (link)

   Extra DB query on every cohort save

   cohort_pre_save issues a Cohort.objects.get() for every existing cohort save — including non-realtime cohorts — because there is no early guard. The post-save handler already skips non-realtime cohorts, but the pre-save cost (one extra SELECT per save) is paid unconditionally for every cohort in the system.

   Adding an early return for cohorts that could never trigger backfill eliminates the regression:

   @receiver(pre_save, sender=Cohort)
   def cohort_pre_save(sender, instance, **kwargs):
       try:
           if not instance.pk or instance.cohort_type != CohortType.REALTIME:
               instance._previous_person_property_filters = ""  # type: ignore
               return
           previous_cohort = Cohort.objects.get(pk=instance.pk)
           instance._previous_person_property_filters = _extract_person_property_filters(previous_cohort)  # type: ignore
       except Cohort.DoesNotExist:
           instance._previous_person_property_filters = ""  # type: ignore
       except Exception as e:
           logger.warning(
               "error_capturing_previous_person_property_filters",
               cohort_id=instance.pk,
               error=str(e),
           )
           instance._previous_person_property_filters = None  # type: ignore

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: posthog/models/cohort/dependencies.py
   Line: 322-347
   
   Comment:
   **Extra DB query on every cohort save**
   
   `cohort_pre_save` issues a `Cohort.objects.get()` for every existing cohort save — including non-realtime cohorts — because there is no early guard. The post-save handler already skips non-realtime cohorts, but the pre-save cost (one extra SELECT per save) is paid unconditionally for every cohort in the system.
   
   Adding an early return for cohorts that could never trigger backfill eliminates the regression:
   
   ```python
   @receiver(pre_save, sender=Cohort)
   def cohort_pre_save(sender, instance, **kwargs):
       try:
           if not instance.pk or instance.cohort_type != CohortType.REALTIME:
               instance._previous_person_property_filters = ""  # type: ignore
               return
           previous_cohort = Cohort.objects.get(pk=instance.pk)
           instance._previous_person_property_filters = _extract_person_property_filters(previous_cohort)  # type: ignore
       except Cohort.DoesNotExist:
           instance._previous_person_property_filters = ""  # type: ignore
       except Exception as e:
           logger.warning(
               "error_capturing_previous_person_property_filters",
               cohort_id=instance.pk,
               error=str(e),
           )
           instance._previous_person_property_filters = None  # type: ignore
   ```
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: posthog/models/cohort/dependencies.py
Line: 322-347

Comment:
**Extra DB query on every cohort save**

`cohort_pre_save` issues a `Cohort.objects.get()` for every existing cohort save — including non-realtime cohorts — because there is no early guard. The post-save handler already skips non-realtime cohorts, but the pre-save cost (one extra SELECT per save) is paid unconditionally for every cohort in the system.

Adding an early return for cohorts that could never trigger backfill eliminates the regression:

```python
@receiver(pre_save, sender=Cohort)
def cohort_pre_save(sender, instance, **kwargs):
    try:
        if not instance.pk or instance.cohort_type != CohortType.REALTIME:
            instance._previous_person_property_filters = ""  # type: ignore
            return
        previous_cohort = Cohort.objects.get(pk=instance.pk)
        instance._previous_person_property_filters = _extract_person_property_filters(previous_cohort)  # type: ignore
    except Cohort.DoesNotExist:
        instance._previous_person_property_filters = ""  # type: ignore
    except Exception as e:
        logger.warning(
            "error_capturing_previous_person_property_filters",
            cohort_id=instance.pk,
            error=str(e),
        )
        instance._previous_person_property_filters = None  # type: ignore
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: posthog/models/cohort/dependencies.py
Line: 283-319

Comment:
**Synchronous network I/O inside `transaction.on_commit`**

`_trigger_cohort_backfill` calls `call_command(...)`, which eventually runs `asyncio.run(_run_workflow())`. That async function opens a Temporal connection and starts the workflow — real network I/O — synchronously, blocking the web worker thread (or Celery worker) for as long as Temporal takes to respond. If Temporal is slow or unreachable the thread hangs until the exception fires.

The pattern in the rest of the codebase for kicking off Temporal work from a signal is to enqueue it via a Celery task rather than calling `asyncio.run` inline. Using a short-lived Celery task preserves fire-and-forget semantics without tying up the web thread:

```python
def _trigger_cohort_backfill(cohort: Cohort) -> None:
    try:
        from posthog.tasks.cohorts import trigger_cohort_backfill_task  # or whichever task module

        trigger_cohort_backfill_task.delay(cohort.team_id, cohort.pk)
    except Exception as e:
        logger.exception("failed_to_trigger_cohort_backfill", cohort_id=cohort.pk, error=str(e))
```

If a dedicated Celery task doesn't exist yet, the existing management command invocation can be moved into one to avoid blocking the caller.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: posthog/models/cohort/dependencies.py
Line: 208-216

Comment:
**Debug log at `info` level left in production code**

This `logger.info` call fires on every cohort update comparison and logs the raw (hashed) filter values. It will add noise to production logs and inflate log volume. Downgrade to `logger.debug` or remove it entirely.

```suggestion
        logger.debug(
            "checking_person_property_filter_changes",
            cohort_id=cohort.pk,
            changed=current_filters != previous_filters,
        )
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: posthog/models/cohort/dependencies.py
Line: 153-187

Comment:
**`_has_person_property_filters` duplicates traversal logic and has inconsistent field requirements**

`_has_person_property_filters` traverses the filter tree requiring `conditionHash`, `bytecode`, AND `key` on each leaf. `_extract_person_property_filters` traverses the same tree but only requires `conditionHash`. The two functions can disagree on whether a cohort "has person property filters", and they duplicate the traversal skeleton.

Because `_extract_person_property_filters` already returns `""` when no qualifying leaf exists, `_has_person_property_filters` can be collapsed to:

```python
def _has_person_property_filters(cohort: Cohort) -> bool:
    return bool(_extract_person_property_filters(cohort))
```

If the stricter `bytecode`/`key` check in the current implementation is intentional (e.g. to guard against partially-formed filters), the same guard should also be added to `_extract_person_property_filters` so the two functions stay consistent.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "chore: remove debug logging from cohort ..." | Re-trigger Greptile}

[github-actions]

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

• Materialize view pane (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this comment? Help fix flakies and failures and it'll disappear!

[tests-posthog]

Query snapshots: Backend query snapshots updated

Changes: 1 snapshots (1 modified, 0 added, 0 deleted)

What this means:

• Query snapshots have been automatically updated to match current output
• These changes reflect modifications to database queries or schema

Next steps:

• Review the query changes to ensure they're intentional
• If unexpected, investigate what caused the query to change

Review snapshot changes →