Skip to content

chore(ci): re-enable the taxonomy drift check - #92740

Merged
trunk-io[bot] merged 2 commits into
masterfrom
posthog/re-enable-taxonomy-drift-check
Sep 1, 2026
Merged

trunk-io[bot] merged 2 commits into
masterfrom
posthog/re-enable-taxonomy-drift-check

Conversation

@haacked

@haacked haacked commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Problem

A taxonomy change that skips the generator silently does not take effect, and nothing catches it. frontend/src/taxonomy/core-filter-definitions-by-group.json is generated from posthog/taxonomy/taxonomy.py, and the frontend reads labels, descriptions, and hidden_in_query_builders from the JSON rather than from Python.

A CI check guarded this. #31950 disabled it in May 2025 and nothing restored it. master is drifted today: the generator produces 33 lines that are not committed.

The check was disabled because it flapped between two formattings of the JSON. The generator wrote the file with json.dump, then ran a second formatter over it. #31014 listed the file in a new frontend/.prettierignore but not in the root one, so whether that second formatter ran depended on which ignore file the invocation resolved. The file had two possible shapes.

Changes

  • The property filters now offer $mcp_scope_preset, with its label and description. It reached taxonomy.py without a regeneration, so the frontend never saw it.
  • Editing taxonomy.py without regenerating now fails the Python code quality job.
  • The generator writes the file once and runs no formatter, so the output is byte-stable however ignore files resolve. This is what makes the check stable rather than only currently-passing.
  • The paths filter includes the generated JSON, so a hand-edit to it reaches the check. That is the case #31014 set out to catch, and the filter missed it.
  • The check scopes git diff to the generated file. Unrelated dirt in a 30 minute job cannot fail it.
  • The check calls python3 bin/build-taxonomy-json.py. The job installs Python but no node_modules, so pnpm exec cannot work there. The generator is pure stdlib.

Nothing in the UI changes shape. The one visible difference is the new property in filter lists, which is data, not layout.

Note

.oxfmtrc.json keeps its ignorePatterns entry for this file. Without it, lint-staged reformats the JSON on commit and reintroduces the drift from the other side.

Open PRs that have not rebased

CI runs a branch merged with master, so a PR that touches no taxonomy takes master's regenerated JSON and passes. A PR that edits taxonomy.py without regenerating fails, which is the intent. No unrebased branch needs a file it lacks.

How did you test this code?

Verified locally on the merged tree:

  • The generator produces no diff on a clean tree.
  • Three consecutive runs produce one identical hash, which is the flap this PR closes.
  • Editing one description in taxonomy.py without regenerating makes the check exit 1. Regenerating makes it exit 0. Both reverted.
  • Committing the JSON runs lint-staged's format:yaml over it and leaves it byte-identical, confirming no second formatter is left in any path.
  • hogli lint:workflows passes 8 of 8 checks. hogli ci:preflight --strict reports no failures.

Not checked: actionlint is not on this machine, and repo-wide mypy did not run. The only Python change deletes a now-unused import from a stdlib script, which ruff check confirms.

No automated tests are added. The check is itself the regression guard, and its behavior is verified by the pass and fail runs above.

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Written with Claude Code in PostHog Desktop.

Skills invoked: /writing-code-comments for the note in the generator, /writing-pr-descriptions for this body. .agents/skills/authoring-ci-workflows/SKILL.md was read directly, as AGENTS.md requires for a workflow change.

The starting brief assumed the check failed because the job had no Node and because oxfmt exited non-zero. Both turned out to be wrong. The job has npm, which an adjacent schema check already uses, and oxfmt exits 0 while formatting nothing, because .oxfmtrc.json excludes this file. oxfmt does format JSON in general, which is what made the second formatter dangerous. The Slack thread linked from #31950 named the real cause, and the plan changed to match it.

Two commits: the regeneration first, then the generator and workflow change, so the check is green on its own PR. Master advanced 159 commits mid-session and landed a separate regeneration; it merged cleanly, and pristine master was re-checked afterwards to confirm the 33 line drift is still real.

@haacked haacked self-assigned this Sep 1, 2026
@trunk-io

trunk-io Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Sep 1, 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.

✅ Complexity (TypeScript) — clean

Cyclomatic complexity above 10 in changed typescript files. Warn only: worth simplifying when you next touch these functions.

⚠️ Bundle size — 🔺 +4.4 KiB (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 68.14 MiB · 🔺 +4.4 KiB (+0.0%)

File Size Δ vs base
posthog-app/_parent/products/workflows/frontend/Workflows/WorkflowScene.js 124.2 KiB 🔺 +2.2 KiB (+1.8%)
exporter/_parent/products/workflows/frontend/Workflows/Workflow.js 97.7 KiB 🔺 +1.4 KiB (+1.5%)

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.43 MiB · 22 files 🔺 +6.1 KiB (+0.4%) ███░░░░░░░ 31.6% of 4.51 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.84 MiB · 3,249 files 🔺 +191 B (+0.0%) █████████░ 91.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
307.0 KiB ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.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
263.5 KiB ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.js
256.1 KiB src/taxonomy/core-filter-definitions-by-group.json
154.2 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.26 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.26 MiB · 18 files 🟢 -113 B (-0.0%) ████░░░░░░ 39.5% of 5.72 MiB
Deferred (lazy) 2.11 MiB · 45 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
751.7 KiB dist/toolbar/toolbar-app-JNIXJPZH.css
590.0 KiB dist/toolbar/chunk-chunk-AZVTVEKI.js
484.7 KiB dist/toolbar/chunk-chunk-RNKUEDGA.js
134.1 KiB dist/toolbar/chunk-chunk-KWI2NYRZ.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
71.3 KiB dist/toolbar/toolbar-app-VQCUEUWY.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-DOQ6MEDX.js
21.0 KiB dist/toolbar/chunk-chunk-LIUCYK4Q.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — 🔺 +245.9 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1448.68 MiB · 🔺 +245.9 KiB (+0.0%)

⚠️ Playwright — 1 flaky

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

  • Delete a person property (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!

@posthog

posthog Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

🦔 PostHog Review reviewed this pull request

Nothing worth raising this time, so here's a calming picture instead:

Someone relaxing in a sunny garden

@posthog posthog Bot removed the reviewhog label Sep 1, 2026
@trunk-io

trunk-io Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

The Python CI job regenerates core-filter-definitions-by-group.json and
fails when the result differs from the committed file, so the generated
taxonomy cannot drift from posthog/taxonomy/taxonomy.py. A hand-edit to
the JSON reaches the check too, through the paths filter.

json.dump is the only writer of that file. The generator runs no
formatter, so its output does not depend on which formatter config
resolves and the check stays deterministic.

A Jest test guards the other half of that guarantee: the JSON must stay
in ignorePatterns in .oxfmtrc.json, or lint-staged reformats it through
oxfmt and the check fails for the next person to edit taxonomy.py. It is
a Jest test because .oxfmtrc.json triggers ci-frontend and not
ci-backend, so a pytest guard would never run on the commit that removes
the entry.

Claude-Session: https://claude.ai/code/session_01TTt186JaMNTx6n7nv72Meq
@haacked
haacked force-pushed the posthog/re-enable-taxonomy-drift-check branch from 21ab56f to fda3b08 Compare September 1, 2026 17:55
@haacked haacked added the stamphog Request AI approval (no full review) label Sep 1, 2026
@haacked
haacked requested a review from a team September 1, 2026 17:58
@haacked
haacked marked this pull request as ready for review September 1, 2026 18:38
Copilot AI lite review requested due to automatic review settings September 1, 2026 18:38
@github-actions

github-actions Bot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ❌ build failed

The preview didn't come up for commit 82e1246. See the build log for the failing step. It'll retry on the next push.

Previews are optional and never block merging. A failure here is often a hogland or tailnet hiccup rather than anything in your PR, so the check stays green and this comment is the status.

@greptile-apps

greptile-apps Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
### Issue 1
frontend/src/taxonomy/coreFilterDefinitions.test.ts:22
**Redundant membership assertion**

The preceding conditional already throws whenever `GENERATED_JSON` is absent, so this assertion only runs when it is guaranteed to pass and unnecessarily duplicates the test condition.

```suggestion

```

---

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

Reviews (1): Last reviewed commit: "chore(ci): re-enable the taxonomy drift ..." | Re-trigger Greptile

Copilot AI 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.

Pull request overview

Re-enables and stabilizes the taxonomy drift guard by making the taxonomy JSON generator output deterministic and adding a CI step to fail when posthog/taxonomy/taxonomy.py changes without regenerating the committed JSON.

Changes:

  • Removes the extra formatter pass from bin/build-taxonomy-json.py so the generated JSON is byte-stable.
  • Re-enables a taxonomy drift check in Python CI and ensures edits to the generated JSON trigger the workflow.
  • Adds a frontend Jest test to assert the generated JSON stays excluded from oxfmt via .oxfmtrc.json.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
frontend/src/taxonomy/coreFilterDefinitions.test.ts Adds a Jest guard to ensure the generated taxonomy JSON remains ignored by oxfmt (so generator output stays stable).
bin/build-taxonomy-json.py Makes taxonomy JSON generation deterministic by removing the second-formatting step and documenting the invariants.
.github/workflows/ci-python.yml Re-adds a taxonomy drift check step and adjusts path filters so JSON edits reach Python CI.

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

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

Not approved yet — waiting on the conditions below.

Re-add the stamphog label to request another review once you have addressed this.

Gates denied (CI workflow deny-list match, T2-never), the change re-enables a CI check and modifies workflow logic — risky infra/CI territory — and the author is not on the owning team, with no human or substantive independent review providing assurance.

  • 👍 on the PR from greptile-apps[bot].
  • Gate verdict DENIED: touches .github/workflows CI file (deny-list infra_cicd) and classified T2-never
  • Author haacked is not on @PostHog/team-devex, the owning team for the changed files
  • No top-level human or agent review provides independent assurance; only bot merge/test-analytics comments and a generic bot review with no substantive findings are present
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✗ matches: infra_cicd
size ✓ 26L, 2F substantive, 50L/3F incl. docs/generated/snapshots — within ceiling
tier ✗ classified as T2-never: T2-never (50L, 3F, cross-cutting, chore)
stamphog 2.0.0b4 .stamphog/policy.yml @ fda3b08 · reviewed head fda3b08

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Sep 1, 2026
@thmsobrmlr
thmsobrmlr self-requested a review September 1, 2026 18:48
@trunk-io
trunk-io Bot merged commit 855361c into master Sep 1, 2026
250 checks passed
@trunk-io
trunk-io Bot deleted the posthog/re-enable-taxonomy-drift-check branch September 1, 2026 20:12
@deployment-status-posthog

deployment-status-posthog Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-09-01 20:37 UTC Run
prod-us ✅ Deployed 2026-09-01 20:52 UTC Run
prod-eu ✅ Deployed 2026-09-01 20:53 UTC Run

This branch had an error being deployed

1 failed deployment
preview-pr-92740 — 82e12461 Deployed Sep 1, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants