feat(data-warehouse): add person-target custom property config - #69979
Conversation
|
Reviews (1): Last reviewed commit: "feat(data-warehouse): add person-target ..." | Re-trigger Greptile |
Migration SQL ChangesHey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:
|
🔍 Migration Risk AnalysisWe've analyzed your migrations for potential risks. Summary: 1 Safe | 1 Needs Review | 0 Blocked
|
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
4d04770 to
b2ced64
Compare
🤖 CI report✅ Bundle size — no changeUncompressed size of every built Total: 64.43 MiB · no change No file changed by more than 1000 B. Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report ✅ Eager graph — within budgetHow much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy
🟢 Largest files eagerly shipped from
|
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 668 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 278.6 KiB | ../node_modules/.pnpm/posthog-js@1.399.5/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 234.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 222.1 KiB | ../node_modules/.pnpm/posthog-js@1.399.5/node_modules/posthog-js/dist/module.js |
| 164.0 KiB | src/queries/validators.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.9 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Dist folder size — no change
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1284.38 MiB · no change
⚠️ 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
⚠️ Backend coverage — 98.0% of changed backend lines covered — 2 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ████████████████████ 98.0% (127 / 129)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/customer_analytics/backend/logic/custom_property_sync.py |
77.8% | 73, 99 |
🤖 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 29322612352 -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 |
batch_exports |
████████░░░░░░░░░░░░ 39.7% |
8,413 / 21,215 |
demo |
███████████░░░░░░░░░ 55.2% |
1,436 / 2,601 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
tasks |
█████████████░░░░░░░ 66.8% |
24,736 / 37,026 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
data_modeling |
████████████████░░░░ 78.4% |
4,696 / 5,987 |
signals |
████████████████░░░░ 78.5% |
18,103 / 23,059 |
cdp |
████████████████░░░░ 80.6% |
3,105 / 3,851 |
wizard |
████████████████░░░░ 82.5% |
772 / 936 |
cohorts |
█████████████████░░░ 82.9% |
3,048 / 3,675 |
notebooks |
█████████████████░░░ 83.8% |
6,086 / 7,259 |
agent_platform |
█████████████████░░░ 84.1% |
3,095 / 3,678 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
product_tours |
█████████████████░░░ 87.5% |
1,266 / 1,447 |
engineering_analytics |
██████████████████░░ 87.6% |
4,417 / 5,040 |
exports |
██████████████████░░ 88.3% |
6,857 / 7,763 |
visual_review |
██████████████████░░ 88.5% |
5,565 / 6,287 |
business_knowledge |
██████████████████░░ 88.5% |
4,400 / 4,969 |
conversations |
██████████████████░░ 88.9% |
15,924 / 17,921 |
mcp_analytics |
██████████████████░░ 89.1% |
2,485 / 2,790 |
dashboards |
██████████████████░░ 89.1% |
5,650 / 6,342 |
error_tracking |
██████████████████░░ 89.2% |
9,473 / 10,615 |
streamlit_apps |
██████████████████░░ 90.4% |
2,499 / 2,764 |
slack_app |
██████████████████░░ 90.6% |
9,460 / 10,444 |
links |
██████████████████░░ 90.6% |
183 / 202 |
marketing_analytics |
██████████████████░░ 90.7% |
11,476 / 12,646 |
product_analytics |
██████████████████░░ 91.2% |
5,652 / 6,195 |
managed_migrations |
██████████████████░░ 91.9% |
908 / 988 |
workflows |
██████████████████░░ 92.0% |
4,795 / 5,210 |
mcp_store |
██████████████████░░ 92.1% |
3,665 / 3,981 |
data_warehouse |
██████████████████░░ 92.1% |
17,281 / 18,764 |
alerts |
██████████████████░░ 92.1% |
3,389 / 3,678 |
web_analytics |
███████████████████░ 92.7% |
13,702 / 14,787 |
notifications |
███████████████████░ 92.7% |
1,026 / 1,107 |
ai_observability |
███████████████████░ 92.7% |
14,670 / 15,822 |
surveys |
███████████████████░ 92.9% |
5,660 / 6,094 |
posthog_ai |
███████████████████░ 93.2% |
1,311 / 1,407 |
tracing |
███████████████████░ 93.2% |
2,423 / 2,599 |
approvals |
███████████████████░ 93.3% |
3,395 / 3,640 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
early_access_features |
███████████████████░ 93.8% |
848 / 904 |
legal_documents |
███████████████████░ 94.1% |
1,568 / 1,667 |
endpoints |
███████████████████░ 94.1% |
8,606 / 9,143 |
messaging |
███████████████████░ 94.3% |
2,366 / 2,508 |
skills |
███████████████████░ 94.4% |
2,827 / 2,995 |
revenue_analytics |
███████████████████░ 94.4% |
3,586 / 3,797 |
review_hog |
███████████████████░ 94.5% |
6,429 / 6,802 |
growth |
███████████████████░ 94.9% |
2,393 / 2,522 |
logs |
███████████████████░ 95.3% |
9,429 / 9,895 |
experiments |
███████████████████░ 95.6% |
24,017 / 25,124 |
replay_vision |
███████████████████░ 95.6% |
12,690 / 13,272 |
feature_flags |
███████████████████░ 96.0% |
14,600 / 15,203 |
warehouse_sources |
███████████████████░ 96.1% |
213,035 / 221,723 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
user_interviews |
███████████████████░ 96.4% |
2,242 / 2,325 |
access_control |
███████████████████░ 96.8% |
849 / 877 |
data_catalog |
███████████████████░ 97.3% |
1,288 / 1,324 |
customer_analytics |
███████████████████░ 97.3% |
7,396 / 7,600 |
analytics_platform |
████████████████████ 98.2% |
2,098 / 2,137 |
metrics |
████████████████████ 98.3% |
2,363 / 2,403 |
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.
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 2 · PR risk: 0/10 |
|
The explicit blank |
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
Extend Customer analytics custom properties with a person target so warehouse table columns can later be upserted onto person properties. Adds: - target_type (account|person) on CustomPropertyDefinition - external_data_schema + column_property_map bindings on CustomPropertySource (person path), source_column now nullable - nullable warehouse_origin provenance on PropertyDefinition - facade validation: exactly-one-of account/person binding, column_property_map shape, team-scoped schema check - serializer + viewset wiring and tests No runtime behaviour yet; the sync pipeline and consumer follow in later PRs. Part of RFC PostHog/requests-for-comments-internal#1195.
…erties Regenerated OpenAPI/TS/MCP types for the target_type + person-binding serializer fields. Pins two enum names via ENUM_NAME_OVERRIDES so adding a customer_analytics `target_type` doesn't auto-rename the subscriptions `target_type` enum (which would churn subscriptions' generated types and break its handwritten frontend).
…urces Format two migrations to satisfy ruff, and tighten person-source validation so an explicitly-provided empty source_column is rejected rather than silently accepted (was `source_column`, now `source_column is not None`). Generated-By: PostHog Code Task-Id: 1d6b1370-8a43-4ee1-ad1d-dea073586d5c
The source_column dataclass sentinel default is "" (contracts.py), so an omitted source_column on a person source deserializes to "". A stricter `is not None` guard would reject every valid person source. The empty string is the unset sentinel and is never persisted as a binding on the person path, so the truthy guard is the correct check. Generated-By: PostHog Code Task-Id: 1d6b1370-8a43-4ee1-ad1d-dea073586d5c
Making CustomPropertySource.source_column nullable for the person path widened its type to str | None, which broke the account saved-query sync (join/index/set-update expected str). This path only handles account sources, which always set source_column, so guard on the local before use to keep it type-safe. Generated-By: PostHog Code Task-Id: 1d6b1370-8a43-4ee1-ad1d-dea073586d5c
7d5eb95 to
88e1df9
Compare
Problem
Managed warehouse customers want to use data they already have in the warehouse (a Postgres
userstable, a CRM export) as person properties, so it works everywhere person properties do: feature flags, cohorts, insights. There's no supported path today. This is the first PR toward that, tracked by RFC PostHog/requests-for-comments-internal#1195.Customer analytics already syncs warehouse columns onto accounts (a group) via
CustomPropertyDefinition/CustomPropertySource. This PR lays the config foundation to also target persons. It is schema + validation only. No sync runs yet.Changes
CustomPropertyDefinition.target_type(accountdefault, orperson).CustomPropertySourcegains a person binding:external_data_schema(FK to the raw incremental warehouse schema,db_constraint=False) andcolumn_property_map({warehouse_column: person_property_name}).source_columnis now nullable since the person path doesn't use it.PropertyDefinition.warehouse_origin(nullable JSON) to later record which source/table/column a person property came from. Provenance lives here, not in the property name.target_type. Person sources requireexternal_data_schema+ a non-empty{str: str}column_property_map; the schema must belong to the team. Account sources are unchanged.Note
Two additive migrations, both metadata-only (no table rewrite).
warehouse_originis nullable so the Rust property-defs writer (which lists its columns explicitly) is unaffected.custompropertydefinitionhas no non-Django writers, so thetarget_typedefault backfill is safe.How did you test this code?
Automated tests I (Claude) actually ran locally against Postgres, all green:
products/customer_analytics/backend/facade/custom_property_source_person_test.py(new): person source round-trips through contract/facade; parameterized validation matrix (person↔account binding mismatch, empty/blankcolumn_property_map, cross-team schema rejected). Catches regressions where the person binding silently drops fields or the guardrails stop rejecting malformed config that the later sync pipeline would choke on.products/customer_analytics/backend/test/test_views.py(extended): endpoint wiring guards that the viewset actually routes a person source through serializer + facade (201 round-trip; wrong binding → 400).test_views.py,custom_property_sync_test.py,custom_property_value_test.py,custom_property_definition_test.pyas an account-path regression check. 246 passed.Migrations validated with
sqlmigrate(confirmed no rewrite, no strayDROP DEFAULTonpropertydefinition).Warning
I could not regenerate the frontend OpenAPI types in this environment (
orvalisn't installed here, node toolchain limited). The serializer changes meanproducts/customer_analytics/frontend/generated/*will drift. Runhogli build:openapion a full frontend toolchain (node >=24) and commit the result, or CI's generated-types check will flag it. No frontend code is otherwise touched in this PR.Automatic notifications
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with Claude Code (Opus 4.8). I (Tom) directed the design across the RFC; Claude wrote the code and tests.
Skills invoked while producing this PR:
/django-migrations(migration safety, hot-table and cross-languageNOT NULLchecks),/setup-web-tests(the localenvvenv was Python 3.12 but master now pins 3.13.13, so a 3.13 venv was built via uv to run anything),/writing-tests(kept the suite to the facade level with one endpoint wiring guard rather than a matrix of API round-trips), and/improving-drf-endpointsfor the serializer shape.Design decisions worth flagging for review: the person source binds to a raw
ExternalDataSchemarather than a saved query (so a later post-sync hook can read only changed rows without a ClickHouse re-read); no property-name prefix (the user names the property in the UI, provenance is metadata); "exactly one binding" is enforced in the facade, not a DB check constraint, to keep the migration purely additive. This PR is the config layer only; the staging sink, sync workflow, Kafka producer, and throttling consumer follow in later PRs.