perf(ci): stop label events and unreadable paths re-running CI - #74017
Conversation
Gate the three semgrep jobs that had no `changes` filter at all: - semgrep-general only scans the repo root plus the directories the other semgrep jobs exclude, so a change confined to frontend/, posthog/ or products/ leaves it nothing to scan. - semgrep-devex mirrors the directory list it passes to semgrep. - semgrep-test-rules runs `semgrep --test .semgrep/`, which only exercises the rules against their own committed fixtures. Narrow semgrep-products-frontend to the scope prefer-codegen-api declares for itself (`/products/*/frontend/`, typescript/javascript, no generated). Exclude Django pytest modules from the MCP and Playwright filters. Both suites drive a running server and never import them. Deliberately scoped to pytest modules rather than whole test/ and tests/ directories: those hold helpers that runtime code does import, notably posthog/test/base.py via setup_test_environment and posthog/test/playwright_setup_functions.py which posthog/urls.py serves at api/setup_test/. Drop the '*Dockerfile' entry from the Node.js filter. It matched only the root Django Dockerfile, which no compose service builds, and never Dockerfile.node, which ci-nodejs-container.yml already gates. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
Review caught that `*_test.py` is not only a pytest convention under posthog/. Twenty-two production files use that suffix, including two ClickHouse migrations (0097_session_replay_events_v2_test.py, 0110_log_entries_v2_test.py), the DDL module they import, and the aggregate_funnel_*_test.py UDF scripts that MCP CI copies into ClickHouse before running `manage.py migrate_clickhouse`. Excluding the suffix meant a PR touching only those files skipped MCP CI and Playwright entirely, with both aggregators counting the skip as a pass. Drop the `*_test.py` exclude from the posthog/, ee/ and common/ trees and keep it only under products/*/backend, where it is the colocated-test convention and no production file uses it. Also drop the file-extension allowlist from the products-frontend filter. prefer-codegen-api is `languages: [typescript, javascript]`, which covers more suffixes than the allowlist kept up with, and products/dashboards/frontend/bin/generate-widget-config-zod.mjs already sat outside it. Scoping to the rule's own path bounds is enough. Refresh the comments the narrowing falsified: the ci-mcp.yml claims that the filter triggers on "any Python change across the app", the "language-specific" description of the semgrep changes job, and the fail-open NOTE, which does not hold for a truncated >3000-file PR under the include-'**' `general` filter. Correct the exclude-veto rationale in the new comments too: an exclude is tested against every file the filter sees, it does not veto the filter as a whole. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
`check-skills` boots Docker and Postgres, migrates, runs setup_dev, then builds the skills ZIP with `hogli build:skills`, whose affected-path map is `products/*/skills/*`. The filter's `products/posthog_ai/**` include drags in that product's React frontend, its largest subtree at ~204 files, which cannot change the ZIP. Measured on a PR that touched one file under products/posthog_ai/frontend: 23.9 job-minutes of Docker, Postgres and migrations to rebuild a ZIP from inputs the PR did not touch. Scoped to posthog_ai rather than the general `products/*/frontend/**` so a future deliberate frontend include in this filter is not silently vetoed, since an exclude is tested against every file the filter sees. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
Reverts every change to ci-security.yaml, restoring semgrep-general, semgrep-devex and semgrep-test-rules to running on all PRs, and products-frontend to its original 'products/**' include. That file is owned by @PostHog/team-security, and keeping those three jobs unconditional is a decision the security team has taken twice: #68640 added path filtering for the language-specific shards while stating the three cross-cutting scans stay unconditional, and #70907 hit the same cost problem this PR was chasing and chose to give the general scan four workers instead, explicitly to speed it up "without weakening the check". Gating them on which paths changed is that team's call to make, not a side effect of a CI cost pass. The measurement stands on its own and belongs in a proposal to them rather than in this PR. What remains here are four scoping fixes in workflows owned by the teams that run them: the MCP and Playwright pytest excludes, the inverted '*Dockerfile' glob in the Node.js filter, and keeping the posthog_ai frontend out of the agent skills check. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
The comments added earlier in this PR ran 7 to 18 lines each, against CLAUDE.md's "default to short or 1-line comments. Explain why, not what". Compressed them while keeping every fact that encodes a trap: - posthog/test/ holds code the running app imports, so it is not excludable wholesale. - '*_test.py' under posthog/ names production code, so that suffix is only excluded under products/*/backend. - an exclude is tested against every file the filter sees, hence per-tree prefixes rather than a bare '**/'. - ci-nodejs has no Dockerfile glob on purpose. - check-skills builds its ZIP from products/*/skills/*. Two other changes worth noting. ci-mcp.yml's runtime-import example is now posthog/test/base.py, which setup_test_environment imports during the backend boot this filter gates, rather than duplicating ci-e2e-playwright.yml's playwright_setup_functions.py; each file now carries the example relevant to it. And an em-dash on a line this PR already touched is gone, per the writing-code-comments skill. Comments only. Verified by parsing each workflow, re-parsing every `filters:` literal block (its comments are part of the string the action receives), and deep-comparing the result against the previous commit: byte-identical, so every match outcome is unchanged. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
ci-backend, ci-frontend, ci-storybook, ci-ai and ci-hobby all take `labeled` and `unlabeled` in their pull_request types. Their comments say why: the run-ci-*, no-ci, evals-ready and hobby-preview escape hatches have to take effect without needing a push. But the trigger fires on *every* label, and only those are read. stamphog[bot] applies its approval stamp to nearly every PR (31 of 40 sampled) and often removes it again. Each add and each remove re-dispatches the full gating suite against a SHA that already went green. PR #74204: `labeled stamphog` at 12:36:54, three runs dispatched at 12:36:58 with identical matrices (23/15/6 jobs, same names) — 165 job-minutes for a label. PR #73987 is worse, because the removal dispatches too: 16:59:45 Storybook + Frontend + Backend -> all success 17:40:10 labeled stamphog -> 3 runs at 17:40:13 17:43:02 unlabeled stamphog -> 3 more at 17:43:05, cancelling those Measured across ten sampled PRs: 318 duplicate successful gating job-minutes, 285 of it on frontend PRs where Storybook (~110 min) and Frontend CI (~52 min) re-run. 22 of 30 sampled frontend PRs affected. Each workflow now ignores label events for labels it does not read. Every other trigger is untouched, so opened / synchronize / ready_for_review / reopened / master push all behave exactly as before and the merge gate is unchanged. The label lists were read out of each file rather than assumed: ci-backend reads four labels, not two. This follows the existing pattern in review-hog.yml, which already gates on github.event.label.name, and reuses the skip path the `no-ci` guard on the same expression already relies on: every other job needs `changes`, and the gates treat skipped as success. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
🤖 CI report✅ Bundle size — 🟢 -1.8 KiB (-0.0%)Uncompressed size of every built Total: 65.22 MiB · 🟢 -1.8 KiB (-0.0%)
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 |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 281.6 KiB | ../node_modules/.pnpm/posthog-js@1.407.7/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 |
| 235.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 226.5 KiB | ../node_modules/.pnpm/posthog-js@1.407.7/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 |
| 105.2 KiB | src/lib/api.ts |
| 94.0 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.18 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.18 MiB · 17 files | no change | ████░░░░░░ 38.2% of 5.72 MiB |
| Deferred (lazy) | 2.07 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 |
|---|---|
| 717.2 KiB | dist/toolbar/toolbar-app-JFACJRNE.css |
| 546.0 KiB | dist/toolbar/chunk-chunk-7TKDTM3F.js |
| 484.3 KiB | dist/toolbar/chunk-chunk-CQF6YMXB.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-N3HPOEXP.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-NODR77N7.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-SB466JBG.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-TKCOQG2P.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 — 🔺 +3.4 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1367.86 MiB · 🔺 +3.4 KiB (+0.0%)
✅ Django migration risk — no migrations to analyze
No Django migrations need risk analysis.
Reverts 640173e. This approach was already tried and rejected in #65900, and the version I pushed had the P1 hole that review caught. The hole, quoted from the Codex review on the same ci-backend.yml line I edited: with pull_request still firing for every labeled/unlabeled event, a job-level guard on `changes` means an unrelated label starts a run where `changes` is skipped, every downstream job cascades to skipped, and the required aggregator runs with `if: always()` treating skipped as success. So an unrelated label can replace a failed or in-progress `Django Tests Pass` with a green one without running any tests — and the workflow-level `cancel-in-progress` kills the legitimate run on the way. Verified present in what I pushed: `django_tests`, `frontend_tests` and `visual_regression_tests` all run `if: always()` and pass on `result == 'skipped'`, and the concurrency group is keyed on head_ref alone, so it does not separate label events from real ones. A false green on a required merge check is strictly worse than the runner time it saves. #65900 also patched that hole (separate `-noop` concurrency group plus the same label filter on each aggregator) and was still rejected, because the label list then has to be repeated in three places per workflow across nine files. The maintainer's verdict was "Given the downsides, I don't like any of the approaches here." The waste is real and still unaddressed — one review request costs two full gate re-dispatches. But the fix needs a design that doesn't distribute the label list, e.g. dropping labeled/unlabeled from the triggers in favour of a single dispatcher. That belongs in its own proposal, not here. Leaves this PR as the four path-filter fixes it started as. Also clears the ci-backend-shadow-drift failure, since ci-backend.yml is no longer touched and so needs no matching .depot/workflows/ci-backend.yml bump. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
Backend CI, Frontend CI and Storybook take `labeled`/`unlabeled` in their pull_request types. GitHub cannot filter a label trigger by name, so every label re-dispatches all three against a commit they have already covered. Measured: one `stamphog` label on #74204 dispatched 13 workflow runs totalling 168 job-minutes. 165 of those were these three; the other ten workflows gate on their own label name and skip for ~1.3 job-minutes combined, PR Approval Agent included, which spent 1.9 doing the job the label was actually added for. On a Python PR the Backend CI half is far worse: #73694, #73742 and #74032 each ran the full 61-shard Django matrix twice on one SHA, ~530 job-minutes a time, job fingerprints identical, one label event and no ready_for_review between them. Usage since 2026-07-01 says the trigger does not earn that: labels these workflows actually read were applied 124 times (run-ci-frontend 116, run-ci-backend 7, no-ci 1), against 1,774+ events from labels they do not read. Deleting the trigger rather than filtering inside the workflow, because filtering is unsafe here. The `changes` job gates everything else, and the required aggregators run `if: always()` and pass on a skipped dependency, so a guard that skips `changes` makes them post green without testing while `cancel-in-progress` kills the real run. #73160 uses that same skip path deliberately but bounds it to drafts, which cannot merge. Label events are not so bounded: pr-approval-agent excludes drafts, so stamphog only lands on ready PRs, which is exactly where a false green merges. #65900 tried the in-workflow guard across nine workflows, hit that P1 in review, patched it with a `-noop` concurrency group plus per-aggregator guards, and was closed as unmaintainable. `ci-mcp.yml` already runs without these triggers and documents the trade: a label takes effect from the next push. The comments here now say the same. Every label read is unchanged, so run-ci-backend, run-ci-frontend, no-ci, no-snapshot-update and test-new-events-schema all still work; they apply on the next push or on ready_for_review. Left alone deliberately: ci-hobby, whose droplet teardown fires on the `unlabeled` event, so removing its trigger would leak droplets; ci-ai, where `evals-ready` is the sole activation; and the six others that already gate correctly. Adding `workflow_dispatch` to ci-frontend and ci-storybook as a push-free fallback was considered and dropped, since a dispatch run has no pull_request context for the paths filter or the draft checks. The depot shadow moves in lockstep, as ci-backend-shadow-drift requires. Generated-By: PostHog Code Task-Id: 7bad9f9c-04b8-40b0-b44c-e967a3f8c731
|
Reviews (1): Last reviewed commit: "perf(ci): drop the label triggers from t..." | Re-trigger Greptile |
|
😎 Merged successfully - details. |
…ci-path-filters # Conflicts: # .github/workflows/ci-mcp.yml
Problem
Two sources of CI that cannot change its own result.
1. Any label re-runs the three merge gates
GitHub has no trigger-level label filter, so
labeled/unlabeledintypes:means every label re-dispatches Backend CI, Frontend CI and Storybook against a commit they already covered.flowchart LR L[Any label, e.g. stamphog] --> B[Backend CI<br/>61-shard Django matrix] L --> F[Frontend CI<br/>full jest matrix] L --> S[Storybook<br/>23 chromium jobs] B --> W[(Re-tests a SHA<br/>already covered)] F --> W S --> W classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff; classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000; classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000; class L phYellow; class B,F,S phRed; class W phGray;stamphoglabel on #74204 dispatched 13 runs 4s later. Storybook ran 23 jobs, then the same 23 againNote
The trigger is exactly as old as draft-PR gating. None of the three had a
types:list before it, so none of them saw label events at all. #60703 addedlabeled/unlabeledto Backend CI (2026-06-01) and #58053 did the same for Frontend CI and Storybook two weeks later, both so therun-ci-*escape hatch would take effect on a narrowed draft run without a push. The escape hatch is worth keeping. Paying for it on every unrelated label is what the numbers above measure.2. Four path filters match files their jobs can't read
'*Dockerfile'Dockerfile, missesDockerfile.nodecheck-skillsonly zipsproducts/*/skills/*Changes
labeled/unlabeledfrom the three gates'types:, Depot shadow in lockstep.'*Dockerfile', keep posthog_ai's frontend out of agent-skills.AGENTS.md: how to force the full matrix on a draft now, plus whylabeled/unlabeledmust not come back.What this costs
Warning
Every label still works. Label reads are untouched, exactly as
ci-mcp.ymlalready operates. What goes away is a label starting a run by itself. On a draft,run-ci-backendandrun-ci-frontendnow take effect on the next push or when the PR is marked ready for review. Push an empty commit if you want it sooner.run-ci-backendrun-ci-frontendtest-new-events-schemano-snapshot-updateno-ciMeasured on the 60 most recent PRs per label, 108
run-ci-*labelings in total.ready_for_reviewMedian gap from label to that next event: 43 minutes. So the 69% majority were already redundant, and no PR that merged depended on a label-triggered run. The cost lands on draft iteration, mostly agents: 20 of the 34 are posthog[bot], the other 14 spread across 7 people.
flowchart LR L[Any label] -.->|no dispatch| R[Label read on the next run] P[Next push or ready_for_review] --> G[Backend CI, Frontend CI, Storybook] R --> G G --> V[(Tests each SHA once)] classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000; classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000; class L,P phYellow; class G phBlue; class R,V phGray;Note
Deletion, not an in-workflow guard. Aggregators are
if: always()and pass on skipped deps, so a guard that skipschangesposts green without testing whilecancel-in-progresskills the real run. #73160 uses that path safely by bounding it to drafts. Label events aren't bounded, sincepr-approval-agentexcludes drafts.How this differs from #65900
That PR tried the guard above across nine workflows in June and was closed unmerged.
ci-mcp.ymlprecedent behind it. Neither existed in that threadWhat was rejected there was the in-workflow guard and a central label-dispatcher workflow. Nothing in the thread argues against removing the trigger.
Untouched:
ci-hobby(teardown fires onunlabeled, removing it leaks droplets),ci-ai(evals-readyis its only activation), six others already correct.How did you test this code?
Not run end to end. This only decides what dispatches.
labeled+unlabeledremoved, every label read unchanged, everything outsideon:byte-identical (YAML-parsed, 4 files), canonical ↔ depot in syncFilterclass run against the realfilters:blocks, driven by 30 audited PRs and a 136-PR window, diffed against the same filters with the negations stripped*_test.py, onposthog/test/base.pyandtools/hogli/tests/conftest.pyci:preflight --strict,lint:workflows6/6,actionlint1.7.12 cleanWF003-dorny-negationonly inspectsdorny/paths-filter@and every call site here is the vendored action, so lint passing isn't evidence. The assertions are.Automatic notifications
Docs update
Not user-facing.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Opus via PostHog Code. Skills:
/authoring-ci-workflows,/writing-code-comments,/simplify.**/tests/**,**/test*/**,*_test.py(which also names 22 production files underposthog/)The label finding only surfaced on a fourth audit pass, after excluding semgrep.
Created with PostHog Code