Skip to content

feat(signals): auto-refresh stale inbox ranking labels partitions - #111262

Merged
trunk-io[bot] merged 2 commits into
masterfrom
posthog-self-driving/featsignals-auto-refresh-stale-inbox-db1189
Oct 4, 2026
Merged

trunk-io[bot] merged 2 commits into
masterfrom
posthog-self-driving/featsignals-auto-refresh-stale-inbox-db1189

Conversation

@posthog

@posthog posthog Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • After a label schema change, the inbox ranking team cannot promote new models until someone with prod Dagster launch rights runs a manual backfill of about 58 labels partitions.
  • Until then, labels partitions in the 60-day lookback lack the new column, and the training builder silently drops those snapshot pairs.
  • The affected heads train with almost no positives, become unreadable, and promotion is refused (seen for action, fixed and dismiss_lowvalue after the schema v9 head redesign).
  • The problem returns at every FEATURE_SCHEMA_VERSION bump.

Closes #111247

Origin

  • Issues: #111247
  • First signal: 2026-10-03
  • Inbox report: open
  • Likely cause: 2991bfb
  • Task started by: auto-start, after the report was rated P2 and ready to fix

Changes

  • Labels partitions now repair themselves after a schema bump. An hourly sensor rewrites stale labels partitions in the training lookback, at most 6 per tick, newest first. 58 partitions are current again in about 10 hours.
  • The labels asset stamps feature-schema-version in each object's metadata. A partition is stale when its stamp is missing or older than FEATURE_SCHEMA_VERSION.
  • The sensor skips the newest day (the daily schedule writes it) and partitions with no labels object.
  • Difference from the issue: a run_key alone stalls the backlog. A run that is in flight or failed leaves its partition stale, so the next tick asks for the same 6 partitions again and Dagster dedupes all of them. The sensor cursor records the partitions it already requested at the current version, so each tick moves on to older ones. A failed refresh alerts and needs a person, as the issue asks.
  • Only labels are refreshed. State and embeddings are not point-in-time on a rewrite.
  • New telemetry: pairs_skipped_missing_label_columns per head on inbox_ranking_examples_built and on the examples asset metadata. A head with 0 positives now shows why.
  • New setting INBOX_RANKING_LABELS_REFRESH_MAX_RUNS (default 6). The README documents the sensor and the code-location pod memory limit.

Note

On first deploy, the sensor (RUNNING in prod US only) rewrites every labels partition in the lookback once, because no object has the stamp yet. Each run sits in the inbox_ranking_etl pool and uses the default run pod size.

How did you test this code?

  • Ran products/signals/dags/inbox_ranking/tests/ locally against Postgres and ClickHouse, plus repo-wide mypy and the dagster paths check.
  • Loaded posthog.dags.locations.signals under Django. The sensor and the job resolve.
  • Ran the sensor against a fake S3 over 10 ticks: it requested 6 partitions per tick, newest first, skipped stamped partitions, did not repeat in-flight ones, and returned a skip once all were requested.
  • Not checked: a real run in a Dagster deployment, and whether the default pod size is enough for the labels-only job.

Test rationale: test_stale_label_partitions_newest_first_capped_and_skips_requested catches a wrong order, a missing cap, a current partition flagged as stale, or the stall above coming back. test_schema_version_stamp_round_trips catches a key mismatch between the writer and the reader of the stamp. No existing test covered write_parquet metadata, so it could not be extended.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

products/signals/dags/inbox_ranking/README.md (Operating it).

🤖 Agent context

Autonomy: Fully autonomous

Agent: PostHog Desktop (Claude Code), claude-opus-5-5


Created with PostHog Desktop from this inbox report.

🤖 Generated with Claude Code

Stamp FEATURE_SCHEMA_VERSION on each labels object and add an hourly sensor that rewrites labels partitions in the training lookback whose stamp is missing or older, newest first, a few per tick. A label column change then reaches the whole lookback without a manual backfill.

Also count the snapshot pairs each head drops for a missing label column on inbox_ranking_examples_built.

Refs #111247

Generated-By: PostHog Desktop
Task-Id: 715b0db5-3f60-48ca-a00f-5c276d384b74
@posthog posthog Bot added the self-driving label Oct 3, 2026
@posthog
posthog Bot marked this pull request as ready for review October 3, 2026 08:53
@trunk-io

trunk-io Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@posthog

posthog Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor Author

🦔 PostHog Review reviewed this pull request

Nothing worth raising this time. Enjoy the moment:

Salad Fingers holds a rusty spoon

@parameterai

parameterai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Risk: No findings

This change adds an hourly Dagster sensor that auto-rewrites stale inbox-ranking labels partitions after a FEATURE_SCHEMA_VERSION bump (at most 6 per tick, newest first, deduped via a cursor), stamps the schema version into each Parquet object's S3 metadata, and adds a per-head telemetry counter for snapshot pairs skipped over missing label columns. All inputs to the new code paths are internal (Dagster cursor state, pipeline-written S3 metadata, settings, dates); no untrusted data reaches any dangerous sink, the labels asset's team-scoped SQL guards are untouched, and the partition rewrite path is the already-sanctioned idempotent re-run. No security vulnerabilities found.

Sentinel reviewed af3345a · Review settings

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

🤖 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.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

🚨 Comment density — 7% of added code lines are comments (11 of 169)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
products/signals/dags/inbox_ranking/dataset/dag.py 5 91
posthog/settings/object_storage.py 2 3
products/signals/dags/inbox_ranking/common.py 2 24
products/signals/dags/inbox_ranking/training/examples.py 2 16

This check does not block merging. It updates on every push and clears when the share drops.

✅ Playwright — all passed

All tests passed.

View test results →

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 3, 2026 08:53
@andrewm4894

Copy link
Copy Markdown
Member

/trunk merge

stamphog[bot]

This comment was marked as outdated.

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: bbe8157a-e9b4-4380-a5b6-21eea02fe0c6
📥 Commits

Reviewing files that changed from the base of the PR and between af3345a and 84f7428.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The labels asset now stamps Parquet objects with the feature schema version. An hourly sensor selects and requests stale label partitions, with a per-tick limit and versioned run keys. Training metadata and events now report per-head counts of pairs skipped because label columns are missing.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🟡 Moderate · up to af334

An invalid refresh limit could leave labels stale or trigger too many refreshes in one tick. Validate the limit before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to af334

Automatic refresh remains confined to the existing labels dataset, with no new user-facing write path identified. Negative refresh-limit values can weaken the intended batch bound. Production concurrency and schema-rollback guarantees remain unverified.

Retained concerns

  • Low · reliability · observed: The refresh batch bound depends on valid deployment configuration. Negative integers are accepted and used directly as slice limits; for example, -1 selects all but the oldest eligible partition rather than enforcing a small batch. This can weaken workload containment within the labels dataset, although the default is six and the eligible lookback remains finite.
Security review details

Security Blast Radius

  • inferred — The incremental write exposure is historical labels in the configured training dataset, not arbitrary stores or environments. Training dependents can consume refreshed labels, but this job does not rewrite state or embeddings. The effective credential permissions may be broader than this code-level selection and were not verified.

Trust Boundaries and Controls

  • inferred — The identified control path is deployment configuration to privileged orchestration to existing label storage. No attacker-controlled caller was established for the new setting. Location registration, bucket availability, existing-object filtering, and labels-only job selection constrain that path, without proving production authorization or IAM ownership.

Resilience and Maintainability Implications

  • observed — Recovery deliberately combines per-version request tracking with manual intervention after failed refreshes. Operational documentation identifies a shared execution pool, two asset retries, and failure-alert ownership. Its deployment-controlled pool limit and effective exclusion of overlapping writers remain unverified.

Hardening Proposals

  • proposed — Validate the refresh limit against an explicit supported range, including whether zero intentionally pauses refreshes, so negative slicing cannot unexpectedly widen the batch.
  • proposed — Document and verify how manual, delayed daily, and different-version writers are coordinated, and what rollback compatibility is required for higher-version label objects. Add stronger coordination or compatibility enforcement only if those guarantees are not supplied by deployment policy.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description covers the problem, changes, testing, release status, documentation, and agent context. It omits the required before-and-after Mermaid diagrams for the sensor flow, plus agent-context …
✨ Finishing Touches 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
products/signals/dags/inbox_ranking/training/examples.py (1)

261-268: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the nonzero skipped-pair count.

The existing refund test already creates a horizon pair with refund_count missing from the scoring snapshot and checks that the examples are skipped. Extend it to check the count as well.

Suggested test change
@@
     Snapshot,
     assemble_snapshot,
     birth_day_positives,
     build_examples,
+    build_head_examples,
@@
-    assert build_examples(snapshots, head, TABULAR_FEATURE_SET).empty
+    built = build_head_examples(snapshots, head, TABULAR_FEATURE_SET)
+    assert built.examples.empty
+    assert built.pairs_skipped_missing_label_columns == 1

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Enterprise
  • Run ID: 9ee98472-d2bd-44e5-9dae-0ff30ceab3ef
📥 Commits

Reviewing files that changed from the base of the PR and between 0120c1a and af3345a.

📒 Files selected for processing (10)
  • posthog/dags/locations/signals.py
  • posthog/settings/object_storage.py
  • products/signals/dags/inbox_ranking/README.md
  • products/signals/dags/inbox_ranking/common.py
  • products/signals/dags/inbox_ranking/dataset/dag.py
  • products/signals/dags/inbox_ranking/tests/test_dataset.py
  • products/signals/dags/inbox_ranking/tests/test_training.py
  • products/signals/dags/inbox_ranking/training/dag.py
  • products/signals/dags/inbox_ranking/training/examples.py
  • products/signals/dags/inbox_ranking/training/telemetry.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.

INBOX_RANKING_PROMOTION_MIN_DAYS = get_from_env("INBOX_RANKING_PROMOTION_MIN_DAYS", 3, type_cast=int)
# Labels refresh sensor (products/signals/dags/inbox_ranking/dataset): how many stale labels
# partitions one hourly tick rewrites after a FEATURE_SCHEMA_VERSION bump, newest first.
INBOX_RANKING_LABELS_REFRESH_MAX_RUNS = get_from_env("INBOX_RANKING_LABELS_REFRESH_MAX_RUNS", 6, type_cast=int)

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Require a positive refresh limit.

If INBOX_RANKING_LABELS_REFRESH_MAX_RUNS=0, the sensor requests no stale partitions. If the value is -1, stale_label_partitions uses [:-1] and can request nearly the entire lookback in one tick. Reject non-positive values before the sensor uses the setting.

@hosthog

hosthog Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

HostHog preview — posthog-desktop-web

The previews for this PR have been torn down and no longer serve.

@trunk-io

trunk-io Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

…esh-stale-inbox-db1189

Generated-By: PostHog Desktop
Task-Id: 97b0a0e3-3143-4257-adbe-8388f6715940
@stamphog
stamphog Bot dismissed their stale review October 4, 2026 08:33

A new stamphog review started for this PR — the fresh verdict replaces this approval.

@stamphog stamphog 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.

Approved.

This is a contained Dagster sensor and telemetry change in the inbox-ranking ML pipeline. It sits outside risky territory, rewrites only idempotent labels partitions in the team's own bucket, and has a current-head human approval. The one open CodeRabbit thread, on validating the refresh-limit setting, is a minor hardening point on an ops-controlled env var.

  • andrewm4894 reviewed the current head.
  • Optional hardening: validate INBOX_RANKING_LABELS_REFRESH_MAX_RUNS as positive (CodeRabbit inline thread); a 0 or negative value would stall or over-request refreshes.
  • On first deploy the sensor rewrites every labels partition in the lookback once, at 6 per hour. This is documented in the PR.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 170L, 7F substantive, 204L/10F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (204L, 10F, two-areas, feat)
stamphog 2.3.1 .stamphog/policy.yml @ 84f7428 · reviewed head 84f7428

@trunk-io
trunk-io Bot merged commit abd1247 into master Oct 4, 2026
280 checks passed
@trunk-io
trunk-io Bot deleted the posthog-self-driving/featsignals-auto-refresh-stale-inbox-db1189 branch October 4, 2026 09:12
@deployment-status-posthog

deployment-status-posthog Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-04 09:28 UTC Run
prod-us ✅ Deployed 2026-10-04 09:38 UTC Run
prod-eu ✅ Deployed 2026-10-04 09:39 UTC Run

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(signals): refresh stale inbox ranking labels partitions automatically after a schema bump

1 participant