Repository navigation
feat(personhog): add FoldPersonDocument leader RPC - #79225
Conversation
|
😎 Merged successfully - details. |
|
Reviews (1): Last reviewed commit: "feat(personhog): add FoldPersonDocument ..." | Re-trigger Greptile |
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
posthog-app/src/scenes/AuthenticatedShell.js |
210.8 KiB | 🔺 +3.8 KiB (+1.8%) |
render-query/src/render-query/render-query.js |
21.46 MiB | 🔺 +3.7 KiB (+0.0%) |
exporter/src/exporter/scenes/ExporterDashboardScene.js |
284.5 KiB | 🟢 -2.7 KiB (-0.9%) |
posthog-app/src/scenes/experiments/Experiments.js |
25.8 KiB | 🔺 +1.3 KiB (+5.1%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How 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 import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.26 MiB · 22 files | 🔺 +1.5 KiB (+0.1%) | ███░░░░░░░ 27.9% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.27 MiB · 3,104 files | 🔺 +6.6 KiB (+0.1%) | █████████░ 85.1% of 9.71 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| 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 |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 285.5 KiB | ../node_modules/.pnpm/posthog-js@1.410.1/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 |
| 238.4 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 231.5 KiB | ../node_modules/.pnpm/posthog-js@1.410.1/node_modules/posthog-js/dist/module.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 |
| 104.5 KiB | src/lib/api.ts |
| 95.2 KiB | ../packages/quill/packages/quill/dist/index.js |
| 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
✅ Toolbar bundle — eager 2.20 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.20 MiB · 17 files | 🔺 +872 B (+0.0%) | ████░░░░░░ 38.4% of 5.72 MiB |
| Deferred (lazy) | 2.08 MiB · 33 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 724.0 KiB | dist/toolbar/toolbar-app-DKENCNQJ.css |
| 552.1 KiB | dist/toolbar/chunk-chunk-5APP6ML7.js |
| 484.6 KiB | dist/toolbar/chunk-chunk-UYYUQJSY.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-PH6M6VEG.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-FHOT2G2O.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-FI2IQ23E.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-Q6C75LDA.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +111.0 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1396.06 MiB · 🔺 +111.0 KiB (+0.0%)
⚠️ Backend coverage — 15.0% of changed backend lines covered — 16 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ███░░░░░░░░░░░░░░░░░ 15.0% (3 / 19)
| File | Patch | Uncovered changed lines |
|---|---|---|
posthog/personhog_client/proto/generated/personhog/service/v1/service_pb2.py |
0.0% | 31 |
posthog/personhog_client/proto/generated/personhog/types/v1/person_pb2.py |
0.0% | 25–28, 107–112 |
posthog/personhog_client/proto/generated/personhog/service/v1/service_pb2_grpc.py |
37.5% | 264, 540–542, 1945 |
🤖 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 31329028035 -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.5% |
8,855 / 22,413 |
warehouse_sources_queue |
█████████░░░░░░░░░░░ 45.9% |
90 / 196 |
demo |
███████████░░░░░░░░░ 56.3% |
1,497 / 2,661 |
data_tools |
█████████████░░░░░░░ 66.2% |
90 / 136 |
tasks |
██████████████░░░░░░ 70.3% |
35,477 / 50,455 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
signals |
█████████████████░░░ 82.5% |
27,210 / 32,964 |
apm |
█████████████████░░░ 83.8% |
1,220 / 1,455 |
cdp |
█████████████████░░░ 84.2% |
3,914 / 4,649 |
notebooks |
█████████████████░░░ 86.4% |
8,029 / 9,296 |
data_modeling |
█████████████████░░░ 86.5% |
8,459 / 9,782 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
wizard |
█████████████████░░░ 86.7% |
1,090 / 1,257 |
cohorts |
██████████████████░░ 87.6% |
6,482 / 7,400 |
managed_warehouse |
██████████████████░░ 87.8% |
5,928 / 6,752 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
data_warehouse |
██████████████████░░ 88.4% |
11,653 / 13,185 |
business_knowledge |
██████████████████░░ 89.0% |
4,384 / 4,928 |
exports |
██████████████████░░ 89.1% |
7,490 / 8,406 |
engineering_analytics |
██████████████████░░ 89.3% |
6,759 / 7,570 |
dashboards |
██████████████████░░ 89.3% |
5,995 / 6,710 |
visual_review |
██████████████████░░ 89.5% |
5,870 / 6,558 |
alerts |
██████████████████░░ 90.3% |
4,482 / 4,966 |
mcp_analytics |
██████████████████░░ 90.4% |
3,968 / 4,388 |
conversations |
██████████████████░░ 90.5% |
18,337 / 20,256 |
canvas |
██████████████████░░ 90.6% |
2,080 / 2,296 |
links |
██████████████████░░ 90.6% |
183 / 202 |
streamlit_apps |
██████████████████░░ 90.7% |
2,630 / 2,901 |
error_tracking |
██████████████████░░ 91.1% |
11,147 / 12,237 |
stamphog |
██████████████████░░ 91.3% |
4,505 / 4,936 |
slack_app |
██████████████████░░ 91.8% |
10,717 / 11,674 |
managed_migrations |
███████████████████░ 92.6% |
1,556 / 1,681 |
notifications |
███████████████████░ 92.6% |
1,017 / 1,098 |
ai_observability |
███████████████████░ 92.9% |
17,116 / 18,426 |
early_access_features |
███████████████████░ 92.9% |
1,347 / 1,450 |
mcp_store |
███████████████████░ 93.0% |
6,983 / 7,512 |
web_analytics |
███████████████████░ 93.1% |
15,963 / 17,154 |
marketing_analytics |
███████████████████░ 93.1% |
15,445 / 16,591 |
surveys |
███████████████████░ 93.2% |
5,919 / 6,349 |
posthog_ai |
███████████████████░ 93.3% |
1,327 / 1,423 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
approvals |
███████████████████░ 93.5% |
3,491 / 3,734 |
product_analytics |
███████████████████░ 93.5% |
7,095 / 7,587 |
legal_documents |
███████████████████░ 93.8% |
1,628 / 1,736 |
workflows |
███████████████████░ 94.3% |
7,951 / 8,436 |
endpoints |
███████████████████░ 94.3% |
8,771 / 9,306 |
tracing |
███████████████████░ 94.4% |
2,678 / 2,837 |
review_hog |
███████████████████░ 94.6% |
8,246 / 8,715 |
skills |
███████████████████░ 94.8% |
3,478 / 3,669 |
messaging |
███████████████████░ 95.4% |
3,401 / 3,565 |
replay_vision |
███████████████████░ 95.7% |
18,220 / 19,043 |
logs |
███████████████████░ 95.8% |
11,623 / 12,135 |
experiments |
███████████████████░ 95.8% |
28,701 / 29,962 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
revenue_analytics |
███████████████████░ 96.3% |
1,887 / 1,960 |
data_quality |
███████████████████░ 96.4% |
747 / 775 |
growth |
███████████████████░ 96.4% |
5,184 / 5,378 |
feature_flags |
███████████████████░ 96.4% |
17,770 / 18,429 |
user_interviews |
███████████████████░ 96.5% |
2,638 / 2,734 |
customer_analytics |
███████████████████░ 96.6% |
11,420 / 11,816 |
access_control |
███████████████████░ 96.9% |
870 / 898 |
warehouse_sources |
███████████████████░ 97.5% |
380,668 / 390,523 |
data_catalog |
████████████████████ 97.9% |
2,677 / 2,734 |
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.
⚠️ Django migration SQL — 2 new migrations to review
We've detected new migrations on this PR. Review the SQL output for each migration:
posthog/migrations/1294_add_google_calendar_integration_kind.py
BEGIN;
--
-- Alter field kind on integration
--
-- (no-op)
COMMIT;products/customer_analytics/backend/migrations/0030_meeting_models.py
BEGIN;
--
-- Create model Meeting
--
CREATE TABLE "customer_analytics_meeting" ("id" uuid NOT NULL PRIMARY KEY, "ical_uid" varchar(1024) NOT NULL, "recurrence_instance_id" varchar(64) NOT NULL, "title" text NOT NULL, "description" text NOT NULL, "start_time" timestamp with time zone NOT NULL, "end_time" timestamp with time zone NULL, "organizer_email" varchar(400) NOT NULL, "meet_code" varchar(64) NOT NULL, "status" varchar(16) NOT NULL, "created_at" timestamp with time zone NOT NULL, "updated_at" timestamp with time zone NOT NULL, "account_id" uuid NULL, "team_id" integer NOT NULL);
--
-- Create model MeetingParticipant
--
CREATE TABLE "customer_analytics_meetingparticipant" ("id" uuid NOT NULL PRIMARY KEY, "email" varchar(400) NOT NULL, "display_name" varchar(400) NOT NULL, "person_id" uuid NULL, "response_status" varchar(16) NOT NULL, "is_organizer" boolean NOT NULL, "created_at" timestamp with time zone NOT NULL, "meeting_id" uuid NOT NULL, "team_id" integer NOT NULL);
--
-- Create index idx_meeting_account_time on field(s) team, account, -start_time of model meeting
--
CREATE INDEX "idx_meeting_account_time" ON "customer_analytics_meeting" ("team_id", "account_id", "start_time" DESC);
--
-- Create constraint unique_meeting_per_team on model meeting
--
ALTER TABLE "customer_analytics_meeting" ADD CONSTRAINT "unique_meeting_per_team" UNIQUE ("team_id", "ical_uid", "recurrence_instance_id");
--
-- Create constraint unique_participant_per_meeting on model meetingparticipant
--
ALTER TABLE "customer_analytics_meetingparticipant" ADD CONSTRAINT "unique_participant_per_meeting" UNIQUE ("team_id", "meeting_id", "email");
ALTER TABLE "customer_analytics_meeting" ADD CONSTRAINT "customer_analytics_m_account_id_d385734d_fk_customer_" FOREIGN KEY ("account_id") REFERENCES "customer_analytics_account" ("id") DEFERRABLE INITIALLY DEFERRED;
CREATE INDEX "customer_analytics_meeting_account_id_d385734d" ON "customer_analytics_meeting" ("account_id");
CREATE INDEX "customer_analytics_meeting_team_id_7d280807" ON "customer_analytics_meeting" ("team_id");
ALTER TABLE "customer_analytics_meetingparticipant" ADD CONSTRAINT "customer_analytics_m_meeting_id_9334506e_fk_customer_" FOREIGN KEY ("meeting_id") REFERENCES "customer_analytics_meeting" ("id") DEFERRABLE INITIALLY DEFERRED;
CREATE INDEX "customer_analytics_meetingparticipant_meeting_id_9334506e" ON "customer_analytics_meetingparticipant" ("meeting_id");
CREATE INDEX "customer_analytics_meetingparticipant_team_id_7f9831db" ON "customer_analytics_meetingparticipant" ("team_id");
COMMIT;Last updated: 2026-08-07 14:50 UTC (db42a93)
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
db42a93 to
514247b
Compare
The merge saga's document write: fold sealed source snapshots into the target person's document under the owner's per-person lock and produce the folded result to the changelog. Properties fold target-wins with snapshot order as precedence, then the merge event's $set/$set_once; created_at folds to the min, is_identified to true, and the version is a max-merge over the target's floor and every sealed version plus one, so re-applying a fold changes no content and only bumps the version (safe under at-least-once delivery). An oversized fold trims instead of rejecting: a rejection would wedge the saga's re-drive loop. Routed to the owning leader like every person write; RouterClient grows the fold wrapper the saga runner will call. No caller yet: the merge saga wires it up next.
Cover three fold interactions the main test doesn't exercise: snapshot-contributed keys vs event_set/event_set_once, empty target properties, and created_at filtering of non-positive timestamps.
The router classified every leader FAILED_PRECONDITION as a routing-race bounce, so a fail-closed verification refusal (ReleaseFence's mark check, and the fold's new one) exhausted into a retriable UNAVAILABLE the saga would loop on forever. Refusals now carry an x-semantic-refusal metadata key and the router delivers them unchanged; bare FAILED_PRECONDITION (handoff fences, ownership races, person fences) still bounces.
Review fixes for FoldPersonDocument: - The fold now fails closed like ReleaseFence: it refuses unless the op holds the person as a live 'marked' merge target (new target_mark_status; mark_status excludes targets by design), stamped as a semantic refusal so the router delivers it. - An oversized fold trims only what the fold contributed; the target's own keys join the candidates only when its stored document already exceeded the limit, matching the update path's remediation. A trim that cannot reach the hysteresis target retries against the hard ceiling, since that band is still applyable by the writer. - When no applyable document exists (protected keys alone exceed the ceiling), the fold completes without producing instead of committing a record the writer would fail-stop on: outcome=unapplyable metric, error log, warning, and a README residual note. - folds_total increments once per request; the discard arm restores the normalized-object properties, not the raw stored value. - Doc comments state the mark check's read-time residual instead of denying it. Also deflakes at_capacity_new_fences_shed_but_reseals_succeed: both seeded persons can share a partition, and re-creating it wiped the first seed.
The rebase onto the identity/last-seen Person fields left the fold's CachedPerson literal without last_seen_at, breaking the build. The fold now max-merges it across the target and every sealed snapshot — the merged person was last seen whenever any constituent was — matching the update path's only-ever-advances semantics.
Remaining review fixes on the fold: - Snapshots must carry the target's team, a different person id, and a living document — a mismatch can only be a saga bug, and folding it silently would launder it into the target. - The target's cached properties are sanitized like every other fold input: rows loaded from Postgres or warmed from legacy changelog records can carry NULs and extreme floats the wire inputs cannot. - Sanitization counts now feed the same counters the update path uses. - The version comments state the invariant as computed: the folded version lands at or above each source's death document, not strictly above it.
Property precedence rode on request order alone, so a saga re-drive that listed sources differently folded a different document, breaking the fold's convergence claim. sealed_snapshots now pairs each Person with its lifecycle_op_person ordinal; the fold sorts by ordinal itself and rejects duplicates, so precedence is deterministic regardless of how the caller lists the sources.
parse_sealed_snapshots owns validation, parsing, sanitization, and ordinal ordering of the fold's snapshots, with unit tests for the rejection matrix, the ordinal sort, and the sanitize-stat totals the integration suite could not see.
The regenerated service stubs picked up fencePerson, releaseFence, and foldPersonDocument from the service proto, so the tests' full-service fakes no longer satisfied ServiceImpl and the Node build failed.
… scan The seeded ops carried a NULL lease and never complete, so the identity sweeper treats each as an abandoned op to resume; hundreds of leftovers in a shared database drown the engine tests' own assertions. A far-future lease keeps them invisible.
1e81545 to
9958208
Compare
Problem
UpdatePersonProperties) diff-merges event properties — it cannot fold source snapshots with precedence or fold the lifecycle scalars.Changes
FoldPersonDocumentto the leader: the target wins every key, sealed source snapshots fill still-absent keys in order, then the merge event's$setoverrides and$set_oncefills.created_atfolds to the min,is_identifiedto true.RouterClientgrows the fold wrapper the saga runner calls.Note
The proto documents the accepted at-least-once residual: the target is never fenced, so a re-driven fold recomputes over writes that landed in between and can clobber them where they overlap. This is the standard re-appliable-merge residual, closed later by operation identity.
How did you test this code?
fence_integration.rs, each pinning a distinct regression: precedence order (a swap of target/snapshot/event precedence fails it), re-fold convergence (no-op suppression creeping in fails it), folding through another op's fence (the delete-vs-merge interlock), resurrection via fold on a destroyed target, and input validation.Automatic notifications
Docs update
None: internal service RPC, no user-facing behavior.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by Claude Code in a session directed by the assignee, implementing the personhog identity/merge RFC. Skills invoked: /adding-personhog-rpc, /writing-tests, /writing-code-comments, /reviewing-personhog-protocol (adversarial two-pass review over the whole stack; the fold's convergence claim was narrowed to state the interleaving residual as a result), /stacking-prs, /writing-pr-descriptions. The base is #78258's branch; GitHub retargets this PR to master when it merges.