fix(engineering-analytics): drop no-op gate runs from duration charts - #71043
Conversation
A workflow run that completes in under 10 seconds without a decisive failure did no real CI work (typically an eligibility gate job with every other job skipped). These runs carpet the bottom of the run-activity scatter and drag medians/percentiles toward zero, so they are now excluded from the scatter, mini bars, and the median/p95 KPI population, while staying in run counts, pass rate, and the runs table. Generated-By: PostHog Code Task-Id: cf7b4da0-1238-44ed-aab0-a97b06634f26
|
Hey @webjunkie! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
🤖 CI report✅ Bundle size — 🟢 -8.1 KiB (-0.0%)Uncompressed size of every built Total: 64.79 MiB · 🟢 -8.1 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.1 KiB | ../node_modules/.pnpm/posthog-js@1.401.0/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 |
| 222.7 KiB | ../node_modules/.pnpm/posthog-js@1.401.0/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 |
| 106.1 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 92.7 KiB | ../packages/quill/packages/quill/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 — 🟢 -25.0 KiB (-0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1308.69 MiB · 🟢 -25.0 KiB (-0.0%)
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
…tivity Generated-By: PostHog Code Task-Id: cf7b4da0-1238-44ed-aab0-a97b06634f26
There was a problem hiding this comment.
Pull request overview
This PR improves Engineering Analytics workflow duration visualizations by filtering out “no-op” gate runs (very short successful/skipped runs) so duration charts and percentile-based KPIs better reflect real CI execution time.
Changes:
- Added a shared
isNoOpRunpredicate (andNO_OP_RUN_MAX_SECONDS) to identify short, non-decisive runs. - Updated
RunActivityChartandRunActivityMiniBarsto exclude no-op runs from plotted/charted datasets. - Updated
computeHealthSummaryto exclude no-op runs from duration percentiles while keeping them in overall counts/pass rate; added Jest coverage for the new behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| products/engineering_analytics/frontend/lib/runHealth.ts | Introduces the no-op run predicate and uses it to exclude no-op runs from duration percentile calculations in health summaries. |
| products/engineering_analytics/frontend/lib/runHealth.test.ts | Adds Jest tests for isNoOpRun boundaries and for excluding no-op runs from duration percentiles while preserving counts/pass rate. |
| products/engineering_analytics/frontend/components/RunActivityMiniBars.tsx | Switches mini-bar rendering to use a “chartable” filter that excludes no-op runs. |
| products/engineering_analytics/frontend/components/RunActivityChart.tsx | Filters no-op runs out of scatter/band/median/count surfaces and exposes isChartableRun for callers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The inline MakeLogicType block for notebookNodeSQLV2Logic was missing the interruptRun/setIsInterrupting actions and the isInterrupting value, so the frontend typechecking job's generated-files check failed on this branch after merging master. Regenerated with kea-typegen (typegen:file) and formatted with oxfmt; output matches the diff the CI check reported. Generated-By: PostHog Code Task-Id: cf7b4da0-1238-44ed-aab0-a97b06634f26
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…usions Review feedback: the predicate hid any fast non-failure run, which would also hide attention-needing conclusions like action_required or startup_failure, and it didn't enforce the documented completed-run contract. It now requires a non-null benign conclusion (passing or cancelled — the set verdictTag paints as non-warning), so fast failures and warning conclusions stay visible regardless of speed. Generated-By: PostHog Code Task-Id: cf7b4da0-1238-44ed-aab0-a97b06634f26
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Note 🤖 stamphog reviewed Frontend-only analytics chart change outside risky territory, but the current-head Codex comment about filtering no-op runs after the backend's row cap (which can hide real runs from the percentiles under high no-op volume) is unresolved and has no reply, unlike the two earlier Codex comments that were explicitly fixed and replied to.
Gate mechanics and policy version
|
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
…before its cap Review feedback: filtering no-op runs client-side happens after the activity endpoint's newest-2000 cap, so a workflow dominated by gate runs could return a capped slice of pure no-ops and the chart would go empty even though real executions exist in the window. The exclusion now lives in the workflow_run_activity query's WHERE clause (NULL-safe so in-flight runs survive), defined once in _workflow_filters next to the other shared predicates and mirroring the frontend isNoOpRun set. The run-detail table still shows every run. Generated-By: PostHog Code Task-Id: cf7b4da0-1238-44ed-aab0-a97b06634f26
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…p exclusion Review feedback: a completed run with a NULL conclusion (the warehouse column is nullable and conclusions can lag the sync) and a sub-10s duration made the NOT IN clause evaluate to NULL, dropping the row from the activity chart. Added an explicit conclusion IS NULL keep case so undecided runs stay visible, matching the frontend isNoOpRun, plus a test row covering it. Generated-By: PostHog Code Task-Id: cf7b4da0-1238-44ed-aab0-a97b06634f26
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Review feedback, three related edge cases with one root cause (the filter was unconditional): - Genuinely fast workflows (a guard check finishing in seconds) had every run classified as no-op, blanking their chart. The activity query now sorts real runs ahead of no-ops so the cap fills with real executions first, and Python drops the no-ops only when at least two real runs remain — otherwise everything is kept. - Commit rollups on the repo overview were being filtered client-side, hiding legitimately fast green commits. The chart components no longer filter at all; only the workflow activity endpoint hides no-ops. - The KPI duration percentiles now fall back to all durations when fewer than two real samples remain, mirroring the endpoint, so an all-fast workflow shows its actual median instead of a dash. Generated-By: PostHog Code Task-Id: cf7b4da0-1238-44ed-aab0-a97b06634f26
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afbb36c6b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review feedback, two edge cases in the fallback logic: - The activity endpoint's fallback threshold counted in-flight rows as real runs, so one completed real run plus an in-flight one could drop the no-op history and leave the scatter below its 2-point minimum. The threshold now counts duration-bearing rows only. - The KPI percentile fallback kicked in below 2 real samples, letting gate runs drown a lone real execution's median. The all-duration fallback is now reserved for zero real samples. Generated-By: PostHog Code Task-Id: cf7b4da0-1238-44ed-aab0-a97b06634f26
…ercentiles Extends the no-op exclusion to the run duration percentiles behind workflow_health (the Workflows table p50/p95 and the time-to-green series), so a gate-heavy workflow no longer reports a seconds-long median there while the activity chart and detail KPIs hide the same runs. Same zero-real-samples fallback as the other surfaces: a legitimately all-fast workflow falls back to all successful runs. Job percentiles are deliberately unchanged — a seconds-long job (the gate job itself) is a legitimate duration sample. SPEC updated to state the new percentile population. Generated-By: PostHog Code Task-Id: cf7b4da0-1238-44ed-aab0-a97b06634f26
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b7e308434
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tell a gate no-op from an intentionally fast workflow — mirroring the activity endpoint and | ||
| the frontend ``computeHealthSummary``.""" | ||
| return ( | ||
| f"if(countIf({RUN_DURATION_PERCENTILE_CONDITION}) > 0, " |
There was a problem hiding this comment.
When a completed successful run has a missing or unparseable updated_at, the builder leaves duration_seconds as NULL, but this countIf still treats that row as a real non-no-op sample. In a bucket or workflow with that one NULL-duration success plus fast successful gate runs, the if takes the non-no-op branch and quantileIf has no duration values, so workflow_health/time-to-green return missing p50/p95 instead of falling back to the all-successful fast durations; include duration_seconds IS NOT NULL in the real-sample count/condition before suppressing the fallback.
Useful? React with 👍 / 👎.
Problem
Workflows with an eligibility gate (a first job that decides the rest of the workflow should be skipped) produce lots of runs that "succeed" in a few seconds without doing any real CI work. On the workflow run-activity scatter these runs carpet the bottom of the chart and drag the median (and the median/p95 KPI tiles) toward zero, hiding what the workflow actually costs when it runs for real. Example shape: a run whose only executed job is a 3-second "decide eligibility" step, with every other job skipped.
Why: raised while reading the run-activity scatter for a preview-deploy workflow in engineering analytics - the no-op runs make the chart unreadable.
Changes
isNoOpRunpredicate inlib/runHealth.ts: a completed run under 10 seconds that didn't decisively fail. Fast failures are kept - a workflow failing within seconds (broken config) is signal, not noise.RunActivityChartandRunActivityMiniBarsdrop no-op runs entirely (scatter, median line, count, in-flight band).computeHealthSummaryexcludes no-op runs from the median/p95 duration percentiles only - they still count toward total runs and pass rate, and still appear in the runs table.The original idea was "runs with 1 job and under 10s", but job counts aren't part of the run-level activity data (and the jobs source isn't guaranteed to be synced), so the duration threshold plus the decisive-failure carve-out approximates it at the run level.
How did you test this code?
runHealth.test.ts: the predicate's boundary (10s kept), the fast-failure carve-out, in-flight runs, and thatcomputeHealthSummarykeeps no-op runs in counts/pass rate while excluding them from percentiles - guards a workflow dominated by gate runs reading as a ~4s median. No existing test covered short-run handling.runHealth.test.tssuite (29 passed) and confirmed the frontend typecheck reports no errors in the changed files. I (Claude) didn't manually verify the rendered chart.Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by PostHog Code (Claude) from Julian's request to keep no-op runs out of the run-activity scatter. Skills invoked: /writing-tests. Considered filtering in the backend
workflow_run_activityquery instead, but the median/p95 KPI tiles are computed frontend-side from the run list, so a backend-only filter would have left the KPIs and chart disagreeing; a shared frontend predicate keeps all duration surfaces consistent while leaving the runs table and pass-rate math untouched. The backendworkflow_healthp50/p95 (Workflows tab) still includes no-op successes - left as a possible follow-up since changing it alters an MCP-exposed endpoint.Created with PostHog Code