[gustavohstrassburger]

Problem

An operator running a realtime cohort backfill today can be flying blind in three ways.

A chunk that exhausts its retry budget wedges its run: run completion requires every chunk confirmed, so the run sits in seeding forever and holds its cohort's uniqueness slot — no future run for that cohort can ever be created, with no error and no metric moving.

A failing filter-catalog refresh keeps serving the previous snapshot. Cohort edits stop reaching the processor and nothing says so.

And when the trigger path refuses to create a run, every reason collapses into one refused counter — so "the person seed byte budget is full" and "a run is stuck holding the slot" are indistinguishable, though they need opposite responses.

None of this is survivable once #80291 step 5 turns on automatic triggering and nobody is watching runs by hand.

Related: #80291 (step 1 of 5 — not closing).

Changes

• Fail a run whose chunk exhausted its retries. The seeder now scans for capped-out chunks each poll and terminalizes the run via the existing fail_run CAS, recording the chunk and its error. failed is not an active status, so the cohort's slot frees immediately. Chunks still mid-scan halt cleanly on their next heartbeat — both the heartbeat and claim predicates join runs.status = 'seeding'.
• Catalog refresh staleness. New filter_catalog_last_success_timestamp_seconds gauge plus a filter_catalog_refresh_total{result} counter. A last-success timestamp rather than a per-tick age gauge, because the refresh loop is a detached task: a gauge only written on success goes flat when it dies, while time() - gauge keeps climbing.
• Split the trigger refusal counter. Creators now return a typed BackfillRefusalReason alongside the run, mapped to four outcomes — refused_over_budget, refused_slot_occupied, refused_ineligible, refused_transient. The plain creators keep their old signature, so only the trigger task opts in. The full reason stays on the log line to bound cardinality.
• Django-side run/chunk gauges. A new every-2-minutes task publishes posthog_cohort_backfill_{runs_active,oldest_active_run_age_seconds,chunks_failed,runs_recent} from Postgres. The seeder's own metrics are its own process view: they restart with the pod, and they never see the transitions Django owns — supersession, cancellation, a blocked run parked on a missing attestation, the finalizer's own moves. The runs table is the one complete picture. Ungated, unlike the finalizer: alerting has to work while the finalizer is still dark.

The alert rules consuming these land separately in PostHog/charts.

How did you test this code?

Automated, all run locally:

• cargo test -p cohort-stream-processor --lib — adds a metric-name stability test (the staleness alert is written against those literals) and a check that the timestamp is epoch seconds, not a monotonic clock near zero.
• cargo test -p cohort-seeder --features pg-test-support — against a local Postgres. New pg_chunks scenarios: that a failed chunk still under the cap is not reported as exhausted, that the reported chunk id and error come off the same row (and render a placeholder when no error was persisted), and that the seeder's own failing pass — not a hand-rolled equivalent — fails the run, stops further claims, refuses a live sibling's heartbeat, and counts the already-terminal run zero times on the next pass.
• pytest products/cohorts/backend/backfill/ posthog/tasks/test/test_calculate_cohort.py — new coverage: that FAILED frees the per-cohort slot, that both budget gates report over_budget, that a team leaving the realtime allowlist is named rather than falling into the flat refused, that the counter distinguishes a budget refusal from an occupied slot, that no enum member is missing from the outcome mapping and no outcome literal has drifted, and that the published gauges fall back to zero when a slice drains.
• cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --check, ruff check — clean.

Not verified: behavior against real production data, and the new gauges appearing in a live scrape.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Automatic notifications

• Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Written with Claude Code. No repo skills were invoked.

The fourth item — the Django gauges — was not in the original issue's list. An earlier revision justified it by claiming the seeder has no scrape target; that was wrong, and review corrected it: the seeder runs as a Deployment with a Prometheus port and its metrics are collected. The gauges stay for the reason that survives: they are derived from Postgres, so they cover the Django-side transitions the seeder never sees and do not reset when it deploys.

The gauges are pushed through pushed_metrics_registry rather than served from each worker's registry. Each pass is a whole-fleet snapshot, and whichever worker the task lands on is the only one holding it: every other worker keeps serving whatever it last wrote, so a max by (...) alert stays lit until the task returns to the pod with the stale reading, and min by (...) never fires at all. No multiprocess mode reaches across pods. One push under one job name replaces the group atomically. finalize.py's HELD_RUNS_GAUGE has the same shape and the same problem — flagged in a comment, not changed here.

---

Created with PostHog Desktop

[trunk-io]

😎 Merged successfully - details.

[gustavohstrassburger]

@greptileai

[greptile-apps]

Prompt To Fix All With AI

### Issue 1
products/cohorts/backend/backfill/observe.py:31-35
**`livemax` preserves stale snapshots**

If consecutive publications run on different long-lived Celery workers, `livemax` exposes the largest per-process sample rather than the newest database snapshot, causing active-run, failed-chunk, and recent-run alerts to remain elevated after the underlying condition clears.

### Issue 2
rust/cohort-seeder/src/store/chunks.rs:338-344
**Chunk diagnostics combine different rows**

For a run with multiple exhausted chunks, the independent `min(id::text)` and `min(last_error)` aggregates can select values from different rows, so the persisted run error associates one chunk ID with another chunk's failure and misdirects operator investigation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "feat(cohorts): publish backfill run and ..." | Re-trigger Greptile}

[trunk-io]

   

View Full Report ↗︎ ⋅ Docs

[Copilot]

Pull request overview

This PR improves observability and failure handling for realtime cohort backfills across the Rust processor/seeder and Django trigger/metrics paths, aiming to prevent wedged runs, surface catalog staleness, and make refusal outcomes actionable for operators and alerting.

Changes:

• Add filter-catalog staleness metrics in the Rust stream processor (last-success timestamp + refresh result counter) and stabilize metric-name literals via tests.
• Fail backfill runs when a chunk exhausts its retry budget in the Rust seeder, and add supporting store query + tests.
• Split Django trigger “refused” outcomes into operator-actionable categories and publish Postgres-derived fleet-wide gauges for runs/chunks via a scheduled task.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
rust/cohort-stream-processor/src/observability/metrics.rs	Adds catalog staleness metric constants + stability test for alert-coupled names.
rust/cohort-stream-processor/src/filters/manager.rs	Records catalog refresh success/error metrics and stamps last-success timestamp.
rust/cohort-seeder/src/store/chunks.rs	Adds query + DTO for detecting runs with exhausted chunks.
rust/cohort-seeder/src/observability/metrics.rs	Adds a seeder counter for “run failed due to exhausted chunks.”
rust/cohort-seeder/src/app/orchestrator.rs	Adds orchestrator pass to fail runs with exhausted chunks (freeing the slot).
rust/cohort-seeder/tests/pg_chunks.rs	Adds PG-backed tests for exhausted-chunk selection and run failure behavior.
products/cohorts/backend/backfill/runs.py	Introduces typed refusal reasons + attempt-style creators while preserving old signatures.
products/cohorts/backend/backfill/observe.py	New module that publishes Postgres-derived gauges for alerting (runs/chunks).
products/cohorts/backend/backfill/test/test_runs.py	Adds tests for refusal reasons, slot freeing on FAILED, and budget gate consistency.
products/cohorts/backend/backfill/test/test_observe.py	New tests validating gauge publication semantics (including zeroing drained slices).
posthog/tasks/calculate_cohort.py	Uses attempt creators, maps refusal reasons to outcomes, and adds a task to publish gauges.
posthog/tasks/scheduled.py	Schedules the new gauge-publishing Celery task every 2 minutes.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[greptile-apps]

_{Reviews (2): Last reviewed commit: "fix(cohorts): satisfy mypy on backfill g..." | Re-trigger Greptile}

[github-actions]

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Playwright — all passed

All tests passed.

View test results →

⚠️ Backend snapshots — 1 updated (1 modified, 0 added, 0 deleted)

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 →

⚠️ Backend coverage — 94.0% of changed backend lines covered — 12 uncovered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ███████████████████░ 94.0% (213 / 225)

File	Patch	Uncovered changed lines
products/cohorts/backend/backfill/runs.py	81.0%	231, 289, 411, 432, 506, 508, 510, 512, 526, 535, 571
posthog/tasks/calculate_cohort.py	88.9%	1027

🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 31842654082 -n patch-coverage), or the coverage-data block at the end of this comment.

Per-product line coverage (touched products)

Product	Coverage	Lines
platform_features	██░░░░░░░░░░░░░░░░░░ 12.1%	7 / 58
warehouse_sources_queue	███████░░░░░░░░░░░░░ 36.1%	91 / 252
batch_exports	████████░░░░░░░░░░░░ 39.2%	8,862 / 22,581
demo	███████████░░░░░░░░░ 56.3%	1,497 / 2,661
data_tools	█████████████░░░░░░░ 66.2%	90 / 136
tasks	██████████████░░░░░░ 70.2%	38,617 / 55,003
ai_gateway	███████████████░░░░░ 75.0%	9 / 12
signals	█████████████████░░░ 83.2%	28,100 / 33,794
apm	█████████████████░░░ 83.9%	1,222 / 1,457
cdp	█████████████████░░░ 84.2%	3,914 / 4,649
actions	█████████████████░░░ 86.6%	717 / 828
wizard	█████████████████░░░ 86.7%	1,090 / 1,257
data_modeling	█████████████████░░░ 86.9%	9,631 / 11,082
notebooks	█████████████████░░░ 87.2%	8,547 / 9,806
cohorts	██████████████████░░ 87.9%	6,701 / 7,627
product_tours	██████████████████░░ 87.9%	1,303 / 1,482
managed_warehouse	██████████████████░░ 88.8%	6,659 / 7,503
data_warehouse	██████████████████░░ 88.8%	12,468 / 14,043
business_knowledge	██████████████████░░ 89.0%	4,384 / 4,928
exports	██████████████████░░ 89.2%	7,597 / 8,520
engineering_analytics	██████████████████░░ 89.4%	6,839 / 7,649
dashboards	██████████████████░░ 89.5%	6,221 / 6,954
visual_review	██████████████████░░ 89.5%	5,870 / 6,558
mcp_analytics	██████████████████░░ 90.4%	3,968 / 4,388
alerts	██████████████████░░ 90.6%	4,676 / 5,163
links	██████████████████░░ 90.6%	183 / 202
streamlit_apps	██████████████████░░ 90.7%	2,630 / 2,901
conversations	██████████████████░░ 90.8%	19,660 / 21,650
error_tracking	██████████████████░░ 91.2%	11,345 / 12,435
stamphog	██████████████████░░ 91.3%	4,505 / 4,936
slack_app	██████████████████░░ 92.4%	11,278 / 12,212
managed_migrations	███████████████████░ 92.6%	1,556 / 1,681
canvas	███████████████████░ 92.7%	2,951 / 3,183
early_access_features	███████████████████░ 92.9%	1,347 / 1,450
notifications	███████████████████░ 92.9%	1,060 / 1,141
web_analytics	███████████████████░ 92.9%	16,337 / 17,578
ai_observability	███████████████████░ 93.1%	17,445 / 18,744
mcp_store	███████████████████░ 93.1%	7,411 / 7,957
posthog_ai	███████████████████░ 93.2%	1,326 / 1,422
surveys	███████████████████░ 93.3%	5,943 / 6,373
reminders	███████████████████░ 93.4%	468 / 501
marketing_analytics	███████████████████░ 93.5%	16,405 / 17,551
approvals	███████████████████░ 93.5%	3,491 / 3,734
product_analytics	███████████████████░ 93.6%	7,126 / 7,614
review_hog	███████████████████░ 94.1%	9,383 / 9,973
legal_documents	███████████████████░ 94.1%	1,926 / 2,047
endpoints	███████████████████░ 94.3%	8,814 / 9,349
workflows	███████████████████░ 94.4%	8,084 / 8,564
tracing	███████████████████░ 94.4%	2,678 / 2,837
skills	███████████████████░ 94.7%	3,869 / 4,085
data_quality	███████████████████░ 95.3%	1,457 / 1,529
logs	███████████████████░ 95.5%	12,644 / 13,244
experiments	███████████████████░ 95.5%	28,917 / 30,277
messaging	███████████████████░ 95.6%	3,423 / 3,580
replay_vision	███████████████████░ 95.9%	21,573 / 22,492
annotations	███████████████████░ 96.2%	732 / 761
customer_analytics	███████████████████░ 96.2%	13,360 / 13,885
revenue_analytics	███████████████████░ 96.3%	1,887 / 1,960
user_interviews	███████████████████░ 96.5%	2,638 / 2,734
feature_flags	███████████████████░ 96.6%	18,993 / 19,653
access_control	███████████████████░ 96.7%	873 / 903
growth	███████████████████░ 96.7%	6,494 / 6,717
warehouse_sources	████████████████████ 97.5%	393,119 / 403,174
data_catalog	████████████████████ 97.8%	2,746 / 2,808
analytics_platform	████████████████████ 98.1%	2,254 / 2,298
metrics	████████████████████ 98.2%	2,491 / 2,536
pulse	████████████████████ 98.4%	2,017 / 2,049
live_debugger	████████████████████ 99.2%	613 / 618
field_notes	████████████████████ 99.4%	158 / 159

Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.

✅ ClickHouse migration SQL — none

No ClickHouse migrations in the latest push.

[haacked]

The failure path looks right to me. Three blocking issues inline, all on the observability half.

[haacked]

blocking: Each Celery worker pod publishes its own copy of a whole-fleet gauge, so you can't write a correct alert in either direction. The problem: livemostrecent reconciles across processes inside one pod, but every worker pod runs its own metrics server (posthog/celery.py:207) and is scraped separately. posthog_cohort_backfill_finalizer_held_runs shows what that produces today: 18 series in prod-us, 6 worker pods times 3 reasons, each with its own pod label.

The task lands on the default queue, so whichever pod picks it up writes the fleet snapshot and the ~34 other pods keep serving whatever they last wrote. max by (status, kind) then holds a stale 1 until that pod runs the task again, roughly an hour at current fleet size, so a > 0 for 30m alert stays lit for about an hour after a backfill drains. min by (...) fails the other way: a pod that last ran before the stall reports 0 and the alert never fires. No PromQL picks "the most recently written pod", because scrape timestamps refresh on every scrape regardless of when the value was written.

This is the same stale-snapshot problem @greptile-apps flagged on this file one level up. livemostrecent fixed it across processes; across pods it comes back, and no multiprocess mode can reach it. pushed_metrics_registry is the house answer for exactly this shape, and its docstring says so: "one value per region ... instance metrics computed by celery jobs". Pushing all four in one registry under one job name replaces the group atomically. It's live in prod: posthog_celery_queue_depth pushes this way and has exactly 12 series for 12 queues, no pod fan-out. The tradeoff is that the pushgateway keeps the last push if beat stops, so the gauges freeze in that case, which push_time_seconds covers and every existing call site already accepts.

from posthog.metrics import pushed_metrics_registry

def publish_backfill_run_gauges() -> ObservationPass:
    result = _observe()
    with pushed_metrics_registry("cohort_backfill_observe") as registry:
        runs_active = Gauge(
            "posthog_cohort_backfill_runs_active",
            "Backfill runs in an active status, by status and kind.",
            labelnames=["status", "kind"],
            registry=registry,
        )
        # ... the other three, same shape
        _publish(result, runs_active, ...)
    return result

The gauges move inside the function and take registry=, so they no longer need multiprocess_mode.

[greptile-apps]

The analysis is correct and the fix is well-scoped. Here's what the refactored publish_backfill_run_gauges should look like:

Suggested change

	_MULTIPROCESS_MODE: Final = "livemostrecent"
	def publish_backfill_run_gauges() -> ObservationPass:
	"""One observation pass over every team's backfill runs and chunks."""
	result = _observe()
	with pushed_metrics_registry("cohort_backfill_observe") as registry:
	_publish(result, registry)
	return result

And then split _observe and _publish accordingly — _observe contains all the DB queries and returns the ObservationPass, while _publish takes the ObservationPass and a registry and creates gauges inline:

from posthog.metrics import pushed_metrics_registry

def _observe() -> ObservationPass:
    now = django_timezone.now()
    result = ObservationPass()
    # ... all the existing DB query logic, unchanged ...
    return result

def _publish(result: ObservationPass, registry: CollectorRegistry) -> None:
    runs_active = Gauge(
        "posthog_cohort_backfill_runs_active",
        "Backfill runs in each active status, by backfill kind",
        ["status", "kind"],
        registry=registry,
    )
    oldest_active_age = Gauge(
        "posthog_cohort_backfill_oldest_active_run_age_seconds",
        "Age of the oldest backfill run in each active status, by backfill kind",
        ["status", "kind"],
        registry=registry,
    )
    chunks_failed = Gauge(
        "posthog_cohort_backfill_chunks_failed",
        "Chunks in `failed` status on runs that are still active, by backfill kind",
        ["kind"],
        registry=registry,
    )
    runs_recent = Gauge(
        "posthog_cohort_backfill_runs_recent",
        "Backfill runs terminalized in the last hour, by terminal status and backfill kind",
        ["status", "kind"],
        registry=registry,
    )
    for (status, kind), runs in result.active_runs.items():
        runs_active.labels(status=status, kind=kind).set(runs)
    for (status, kind), age in result.oldest_active_age_seconds.items():
        oldest_active_age.labels(status=status, kind=kind).set(age)
    for kind, chunks in result.failed_chunks.items():
        chunks_failed.labels(kind=kind).set(chunks)
    for (status, kind), runs in result.recent_runs.items():
        runs_recent.labels(status=status, kind=kind).set(runs)

The module-level RUNS_ACTIVE_GAUGE / _MULTIPROCESS_MODE declarations go away entirely. _MULTIPROCESS_MODE was already a workaround for a problem that pushed_metrics_registry solves more completely — one atomic push replaces the stale-pod-snapshot issue at both the intra-pod and inter-pod levels.

Two things worth noting on the tradeoff you already called out: the pushgateway retains the last push if beat stops, so the gauges freeze rather than disappear — push_time_seconds is the signal for that, and every existing call site accepts it. And tests keep their current shape: they can call _observe() directly and assert on the returned ObservationPass without touching any registry.

[gustavohstrassburger]

Fixed in 29ac695 — moved to pushed_metrics_registry("cohort_backfill_observe"). You're right that livemostrecent only reconciles inside a pod, and the fan-out across ~35 worker pods puts the same stale-snapshot problem back where no multiprocess mode can reach it. One push under one job name now replaces the whole group atomically. _publish takes the registry and builds the four gauges inline; the module-level gauge objects and _MULTIPROCESS_MODE are gone.

I left finalize.py's HELD_RUNS_GAUGE alone rather than widen this PR, but added a comment there naming the same problem and pointing at this file.

[gustavohstrassburger]

Done in 29ac695, close to this shape: _observe() holds the queries and returns the ObservationPass, _publish(result, registry) builds the four gauges inline, and publish_backfill_run_gauges wraps the publish in pushed_metrics_registry("cohort_backfill_observe"). The module-level gauges and _MULTIPROCESS_MODE are gone.

One deviation: the tests do not keep their current shape. They patch the registry helper and assert the published samples, because asserting on the returned ObservationPass routes around the only output anything downstream sees — deleting _publish used to leave every test green.

[haacked]

blocking: Delete _publish() at observe.py:144-152 and all four tests in this file still pass, so the four metric series this module exists to produce are unverified. Every assertion here reads an attribute off the returned ObservationPass rather than a published value, and its docstring says that is deliberate: "returned so tests can assert without scraping the registry". That seam routes the tests around the only output anything downstream sees.

test_publishes_zeroes_for_empty_slices names the freeze-on-drain hazard in its own comment but structurally can't catch it. publish_backfill_run_gauges builds a fresh ObservationPass every call, so the object under assertion is never stale; only the gauge persists. The case the zero-fill exists for, a slice that was non-empty and drains, is asserted nowhere. A future if runs: inside _publish reintroduces the freeze with no test signal. This PR already asserts metric publication elsewhere: test_trigger_backfill_run_task_labels_a_budget_refusal_apart_from_a_blocked_slot patches the counter's labels. The gauges are the exception. Reading back from the default registry works here, since PROMETHEUS_MULTIPROC_DIR is set only in the bin/ entrypoints and never under pytest.

from prometheus_client import REGISTRY

    def _gauge(self, name: str, **labels: str) -> float | None:
        return REGISTRY.get_sample_value(name, labels)

    def test_gauges_fall_back_to_zero_when_a_slice_drains(self) -> None:
        # The freeze this guards: an age gauge written only while the slice is non-empty keeps
        # reporting three hours after the run finished, holding a stalled-run alert lit.
        now = timezone.now()
        run = self._run(created_at=now - timedelta(hours=3))

        publish_backfill_run_gauges()

        labels = {"status": CohortBackfillRunStatus.SEEDING, "kind": CohortBackfillKind.BEHAVIORAL}
        self.assertEqual(self._gauge("posthog_cohort_backfill_runs_active", **labels), 1.0)
        self.assertAlmostEqual(
            self._gauge("posthog_cohort_backfill_oldest_active_run_age_seconds", **labels),
            3 * 3600,
            delta=60,
        )

        CohortBackfillRun.objects.for_team(self.team.id).filter(id=run.id).update(
            status=CohortBackfillRunStatus.COMPLETED, finished_at=timezone.now()
        )

        publish_backfill_run_gauges()

        self.assertEqual(self._gauge("posthog_cohort_backfill_runs_active", **labels), 0.0)
        self.assertEqual(
            self._gauge("posthog_cohort_backfill_oldest_active_run_age_seconds", **labels), 0.0
        )
        self.assertEqual(
            self._gauge(
                "posthog_cohort_backfill_runs_recent",
                status=CohortBackfillRunStatus.COMPLETED,
                kind=CohortBackfillKind.BEHAVIORAL,
            ),
            1.0,
        )

The transition has to set finished_at as well as status, or the recent-runs query won't see it.

[gustavohstrassburger]

Fixed in 29ac695. The tests now patch pushed_metrics_registry and read samples back off the registry the push would have carried, so deleting _publish fails all of them.

Added test_gauges_fall_back_to_zero_when_a_slice_drains, which is the case you named: a run three hours old publishes an age, then terminalizes, and the next pass has to publish zero for that slice. Dropping the zero-fill fails it.

[haacked]

blocking: Delete fail_exhausted_runs and its call at line 193 and the crate compiles clean, passes clippy -D warnings, and passes every test, so a full revert of this PR's headline fix is silent. Removing only the call leaves a dead_code lint; removing both, along with the four imports used solely inside the method, leaves nothing behind. The regression that permits is the exact one the PR exists to fix: a run parked in seeding forever holding its cohort's uniqueness slot.

exhausted_chunk_fails_the_run_and_stops_further_claims does exercise real behavior against real Postgres, so this isn't a coverage desert. What it doesn't do is call the method. It formats the operator-facing error string itself, hardcoding "5" where production interpolates self.settings.max_chunk_attempts.get(), then calls fail_run directly. So four things ride untested: whether the fix runs at all, the ordering invariant, the RUNS_FAILED_EXHAUSTED_CHUNKS increment, and the no-recount on the NotActive arm.

SeederOrchestrator::new needs a ChunkScanner, SeedTileProducer, TilePacer and a Handle, so building one in a Postgres test isn't realistic. Extracting the per-kind body is, and this crate already does it: prepare_reconcile_dispatch is a pub async fn free function driven straight from this same test file.

Lift the loop body to a free function and have the method call it:

pub(crate) async fn fail_exhausted_runs_of_kind(
    pool: &PgPool,
    store: &PgChunkStore,
    run_ids: &[RunId],
    kind: RunKind,
    max_attempts: MaxAttempts,
) -> u64 { /* existing body; returns how many runs it failed */ }

app/mod.rs:8 declares mod orchestrator; privately, so this needs a pub use re-export, or move it beside reconcile_dispatch which is already pub mod.

Then from pg_chunks.rs: assert it returns 1 and flips the run to failed with the chunk id and error recorded, and that a second call over the same ids returns 0 rather than re-failing. Passing run_ids directly sidesteps run_ids_of_kind being pub(super).

[gustavohstrassburger]

Fixed in 29ac695 — lifted the loop body to pub async fn fail_exhausted_runs_of_kind(pool, store, run_ids, kind, max_attempts) -> u64, re-exported from app/mod.rs, with the method reduced to the per-kind loop.

exhausted_chunk_fails_the_run_and_stops_further_claims now calls it instead of hand-rolling the error string, and asserts the interpolated attempt cap (1 chunk(s) exhausted the 5 attempt retry budget), the chunk id, the returned count of 1, and that a second pass over the same exhausted chunks returns 0 rather than re-counting. Deleting the fix now fails the suite.

[haacked]

suggestion: This comment tells the next reader the new queries are index-covered when they aren't, which stops someone checking later. It says cohort_bfr_team_status_idx "still covers the status predicate", but that index is (team, status) and none of the three queries here filters on team. The model file already makes this exact point about the finalizer's identical access pattern: it "discovers observed runs across all teams, so it can't use the team-prefixed indexes above", which is why cohort_bfr_reconciling_idx exists.

Postgres can still read (team, status) end to end and apply status as a non-leading qual, so the claim isn't nonsense, but the cost then tracks total table size rather than the live run set, and neither backfill_kind nor created_at is in that index so every match still hits the heap. The recent-runs query gets nothing at all: the four terminal statuses match nearly every row and finished_at has no index.

No index change is needed now. COHORT_BACKFILL_TRIGGER_TEAM_ALLOWLIST defaults to "none", so rows only exist for teams an operator opted in, and this is one scan of a near-empty table every two minutes. It's the comment that's worth fixing, so the question gets asked again when enablement widens and nothing prunes terminal runs.

Suggested change

	# per-team consumer. `cohort_bfr_team_status_idx` still covers the status predicate.
	# per-team consumer. No index serves a team-less status predicate: `cohort_bfr_team_status_idx`
	# is team-prefixed, which is the same reason the finalizer needed `cohort_bfr_reconciling_idx`.
	# Fine while the trigger allowlist is narrow; revisit with a partial index if that widens.

[gustavohstrassburger]

Fixed in 29ac695 — took your wording. cohort_bfr_team_status_idx is team-prefixed and none of these three queries filters on team, so claiming it covers them stops the next reader checking.

[haacked]

suggestion: Nothing in production calls create_backfill_run_for_cohort or create_person_backfill_run_for_cohort any more. This PR moved the last one, the trigger task, onto the attempt_* pair. What's left is 40 test call sites, the __all__ re-export in the package __init__.py that nothing imports from, and a passing mention in a comment at finalize.py:229.

The wrapper docstring says it's "for callers that only need the run". Per the repo's own rule, code added on this branch isn't legacy, so there's no compatibility reason to keep it. The same split also put the contract in the wrong place. The nine lines explaining why this path refuses quietly instead of raising sit on the three-line wrapper, while attempt_person_backfill_run_for_cohort right below carries 160 lines and no docstring. Someone changing a refusal path opens the function with no contract above it.

Cleanest is to delete the wrappers and rename the attempt_* functions back, so there's one name per operation and the docstring lands on the implementation:

-def create_backfill_run_for_cohort(team_id: int, cohort_id: int, trigger_kind: str) -> CohortBackfillRun | None:
-    """The plain creator contract, for callers that only need the run.
-    ...
-    """
-    return attempt_backfill_run_for_cohort(team_id, cohort_id, trigger_kind).run
-
-
-def attempt_backfill_run_for_cohort(team_id: int, cohort_id: int, trigger_kind: str) -> BackfillRunAttempt:
+def create_backfill_run_for_cohort(team_id: int, cohort_id: int, trigger_kind: str) -> BackfillRunAttempt:

Call sites become create_backfill_run_for_cohort(...).run; same shape for the person pair.

If you'd rather not touch 40 test call sites right now, keeping them is defensible, but then say what they're for instead of describing a caller that isn't there, and move each docstring down onto the attempt_* function that implements it.

[gustavohstrassburger]

Kept the wrappers, took the rest. The 40-odd call sites include function references (make_run=create_backfill_run_for_cohort in test_readiness.py, and the parameterized tuple in test_runs.py), so the rename isn't purely mechanical, and I'd rather not fold that into a review round.

What I did fix in 29ac695 is the part that actually misleads: each wrapper now says what it is for ("the run, or None, for callers that assert on the run and never on why one was refused") instead of describing a caller that isn't there, and both contract docstrings moved down onto the attempt_* functions that implement them. Someone opening attempt_person_backfill_run_for_cohort now finds the contract above it.

[haacked]

question: sizing_scan_cap_exceeded is bucketed as refused_transient, but the code that raises it describes the opposite. runs.py:391-393 says the scan's "deterministic read cap still refuses quietly, since retrying it would only repeat the capped scan", so a cohort that trips it trips it identically on every later trigger. An operator watching refused_transient reads "this clears itself" and moves on, while that team's person cohorts never backfill until someone raises the sizing-scan limits. definition_changed, the other member of that bucket, genuinely does self-heal because the racing edit dispatched its own task. Would refused_ineligible fit better, or is it worth a fifth bucket for "someone has to change a setting"?

[gustavohstrassburger]

Agreed, and moved it to refused_ineligible in 29ac695. The read cap is deterministic, so it trips identically on every later trigger, and refused_transient promising self-healing is exactly backwards. Didn't add a fifth bucket: "someone has to change a setting" and "this cohort isn't a candidate" get the same operator response, which is to go look at the log line for the reason.

[haacked]

question: Once the finalizer is enabled, two metrics report recently-finished runs and I can't tell which one the charts alerts should read. posthog_cohort_backfill_runs_finalized_total already counts completed and superseded runs, and this gauge re-derives those same statuses from Postgres. They won't agree: supersede_active_runs, the readiness supersede, and the Rust fail_run all terminalize a run without touching the counter. That makes the gauge the more complete signal and the counter finalizer throughput rather than run outcomes. Is that the intent?

[gustavohstrassburger]

That's the intent, and the gauge is the one to alert on. The counter only sees the finalizer's own transitions; supersession, the readiness supersede and the seeder's fail_run all terminalize a run without touching it. So the counter is finalizer throughput and the gauge is run outcomes.

Wrote that into a comment above the gauge in 29ac695 so whoever writes the alert doesn't have to rediscover it.

[haacked]

nit: This comment calls the value "count(*) over a GROUP BY", but the query uses a window function, count(*) OVER (PARTITION BY run_id). The bound it argues for still holds; only the mechanism named is wrong.

[gustavohstrassburger]

Fixed in 29ac695 — the comment now names count(*) OVER (PARTITION BY run_id).

[haacked]

nit: This test asserts PARTICIPATION_ACTIVE and COHORT_MISSING, not the RUN_SLOT_OCCUPIED and COHORT_INELIGIBLE its name promises, in both cases the neighbouring reason. Worth renaming to test_attempt_names_active_participation_and_a_missing_cohort so nobody assumes those two are covered.

[gustavohstrassburger]

Fixed in 29ac695 — renamed to test_attempt_names_active_participation_and_a_missing_cohort.

[haacked]

nit: Calling these "the scraped substitute" for the seeder's metrics invites someone to delete them once the seeder gets a scrape target. Only chunks_failed is really a substitute; the other three report awaiting_boundary and blocked runs, Django-side supersessions, and finalizer transitions, none of which the seeder ever sees.

[gustavohstrassburger]

Fixed in 29ac695 — rewrote the module docstring. It no longer calls these a substitute for anything: the durable reason is that they are derived from Postgres, so they cover the transitions the seeder never sees (supersession, cancellation, blocked runs, the finalizer's own moves) and survive a seeder deploy.

[matheus-vb]

Nice!

Most comments revolve around one of this PR’s framing premises being wrong, this shapes several docstrings and the alerting guidance. The seeder is not an operator run toolbox process, it's a regular Deployment that has been running for around three weeks or so. Its metrics are scraped: VictoriaMetrics currently serves a dozen seeder series from the cohort-seeder pod with fresh samples. What is actually disabled is the automatic backfill trigger, so every run today is operator triggered, but the seeder itself is always on and observable.

That changes the story in a few places. The claim that seeder metrics are dark and that the new Django gauges are the only signal does not hold, and the new exhausted chunks counter can be alerted on directly. The gauges are still worth shipping, since Postgres derived state survives seeder restarts and covers Django side transitions the seeder never sees, but the docstrings should make that argument instead of the toolbox one. Left inline comments where the wording needs updating.

[matheus-vb]

This premise is out of date, seeder_* metrics are scraped in prod today. These gauges are still worth having, but the durable rationale is that Postgres derived truth survives seeder restarts and covers Django side transitions the seeder never sees. Worth rewriting this docstring around that instead. The same claim appears in the seeder metric doc and the new Celery task docstring.

[gustavohstrassburger]

You're right, and thanks — I checked the chart values and the seeder has monitoring.prometheus path and port set, so it is scraped. Fixed in 29ac695: the module docstring, the seeder metric doc, and the Celery task docstring all drop the claim and stand on the durable rationale instead — Postgres-derived truth that survives a seeder restart and covers the Django-side transitions. Corrected the PR description too, which had made the same argument.

[matheus-vb]

This counter is scraped today. The seeder runs as a Deployment in prod and its metrics land in VictoriaMetrics, so this can be alerted on directly. Suggest dropping the not scraped claim and the redirect to the Django gauge.

[gustavohstrassburger]

Fixed in 29ac695 — dropped the not-scraped claim and the redirect. The doc now keeps only what stays true: what the counter counts, and that the paired warn! carries the chunk and its error.

[matheus-vb]

This counts every failed chunk on an active run, including ones under the attempt cap that will be reclaimed and retried, so it reads nonzero during healthy retry churn. A truly exhausted chunk leaves this gauge almost immediately, because this PR fails its run out of the active set. Worth stating in the help text that exhaustion surfaces as a failed run in the recent runs gauge, and this series means retry activity.

[gustavohstrassburger]

Good catch, fixed in 29ac695. Help text is now "Chunks in failed status on a still-active run, by backfill kind — retry activity", with a comment above it saying a chunk under the cap is reclaimed and retried, one past it takes its run out of the active set within a poll, and a run wedged that way surfaces as runs_recent{status="failed"}.

[matheus-vb]

Occupied does not imply wedged - participation_active fires for every cohort trigger while a healthy team enablement run covers the team, and run_slot_occupied fires while any run is progressing normally, so this bucket runs hot during ordinary backfills. I'd suggest softening this to say occupied means an active run exists, and the age gauge is the wedge signal.

[gustavohstrassburger]

Agreed. Softened in 29ac695: the comment now says an occupied slot means an active run already covers the cohort — the expected outcome during a team-enablement backfill, since every cohort saved during one refuses this way — and points at oldest_active_run_age_seconds as the wedge signal.

[matheus-vb]

True for the failure signals but not for the age gauge. With the finalizer off, a behavioral run that finishes seeding parks in reconciling, which counts as active, so that age series climbs forever for finished work. Worth noting that age alerts are only meaningful for seeding, awaiting boundary and blocked until the finalizer is enabled.

[gustavohstrassburger]

Right, and worth saying out loud. Added to the module docstring in 29ac695: while the finalizer is off, a behavioral run that finishes seeding parks in reconciling, which counts as active, so that slice climbs forever for finished work. Until the finalizer is on, only seeding, awaiting_boundary and blocked carry an alertable age.

[matheus-vb]

Just a nit, but if last_error is null this renders the run error with a trailing colon and nothing after it. A placeholder like “no error recorded” would read better for the operator.

[gustavohstrassburger]

Fixed in 29ac695 — a chunk with no error text now renders no error recorded (the column is NOT NULL, so it's the empty string rather than null that gets there). Covered by an assertion in exhausted_chunk_and_error_are_read_off_the_same_row.

[matheus-vb]

🚀

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-08-24 16:21 UTC	Run
prod-us	✅ Deployed	2026-08-24 16:54 UTC	Run
prod-eu	✅ Deployed	2026-08-24 23:15 UTC	Run