feat(engineering-analytics): broken-tests triage panel - #70990
Merged
Conversation
…board The per-test CI spans already carry ci.branch, but the flaky-test leaderboard only surfaced rerun/failed/PR-spread counts — never whether a test's failures hit the default branch. A flake breaking master/main is the "matters right now" discriminator: it blocks the trunk, not just a PR branch, so it deserves to be visible next to the other signals. Add master_failed_count: failed/error spans where the branch is master or main. This is the same master/main approximation run_scope_filter_clause documents — the source doesn't record which branch is the repo default — threaded from the countIf through the FlakyTestItem contract, serializer help_text, generated types, and the UI (a sortable, danger-toned "Master failures" column). Ranking is unchanged: this is an additional surfaced signal, not a new qualifier, so the HAVING and ORDER BY are left as-is.
Add a "Currently broken tests" panel above the flaky-test leaderboard on the test-health tab, to validate a ranking UX against live prod data. Reads two warehouse views by name — engineering_analytics_ci_failures (failure fingerprints) and engineering_analytics_ci_job_history (master job status) — via two ad-hoc HogQL calls from the kea loader. They hit different ClickHouse clusters (logs vs warehouse), so a single joined query would fail; the two result sets are merged client-side in the classifier selector. The classifier is temporary and lives in kea for now: it ranks failures by breaking-master vs novel-burst vs resolving vs flaky, cutting the noise the flaky leaderboard doesn't distinguish. Thresholds sit in named constants. It moves server-side once the UX proves out. Prototype-quality on purpose: no feature flag, local only, marked as such in a top-of-component comment and a "Prototype" tag in the panel header.
…to broken-tests panel Expand a broken-test row to see the latest failing run's logs, pulled lazily from the existing run_failure_logs endpoint (rides the product's own scope, not raw /query/) and grouped by failed job with line numbers plus a GitHub run link. Adds a 24h sparkline column so an escalating failure reads at a glance, off a tight 24h hourly query. Narrow the analytical window from 7 to 2 days: much lighter logs-cluster scan, and it still spans the classifier's boundaries (flaky needs a >24h span, novel_burst first-seen <24h, both fit inside 48h). The master-status floor tracks the same window with a day of slack. Also seeds a dev-only fixture (NODE_ENV=development + ph_broken_tests_fixture localStorage flag, stripped from prod builds) so the panel renders locally without the /query/ auth path that standalone OAuth mode can't reach. Temporary — goes away once the list moves to a backend endpoint.
…broken-tests-panel
…point Ports the "Currently broken tests" panel off client-side HogQL onto a named broken_tests read endpoint, following the product's facade/logic/presentation shape. Why: the prototype read the two warehouse views via raw /query/ from the kea loader, which standalone OAuth (local frontend -> prod) rejects with empty required-scopes regardless of token — so the panel only ran off a dev fixture. Riding the product's own engineering_analytics scope (like the rest of the tab) kills that auth wall. It also lands the classifier in logic/ where the Signals emitter can reuse one definition of "what's a valuable CI failure", per SPEC. - logic/queries/broken_tests.py: fingerprint aggregation + 24h sparkline over the ci_failures builder (LOGS cluster), latest default-branch job status over the workflow_jobs builder (warehouse), merged + classified server-side. Age, span, and sparkline offsets are integer dateDiffs in SQL so the classifier does no tz-sensitive datetime math. Embeds the view builders as subqueries, not the registered views by name, keeping the product off the global catalog. - Contracts + serializer + view action + MCP tool (broken_tests), plus the investigating-ci-failures skill points at it as the triage entry point. - Frontend drops the fixture, the kea classifier, the three performQuery calls, and the client-side sparkline fold; it now maps one typed endpoint response. The drill-down keeps riding run_failure_logs. Degrades rather than misreports: without the job-level source, breaking/resolved can't be decided, so those failures fall through to flaky/pr_only.
…broken-tests-panel
…broken-tests-panel
Contributor
🦔 Hogbox preview · ❌ build failedThe preview didn't come up for commit |
Contributor
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
posthog-app/_parent/products/engineering_analytics/frontend/scenes/EngineeringAnalyticsScene.js |
66.8 KiB | 🔺 +7.2 KiB (+12.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.22 MiB · 22 files | no change | ███░░░░░░░ 28.4% of 4.29 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.15 MiB · 2,978 files | no change | █████████░ 88.1% of 9.25 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 |
|---|---|
| 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 — 🔺 +43.1 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1307.98 MiB · 🔺 +43.1 KiB (+0.0%)
ℹ️ MCP UI apps size — 31 app(s), 16561.5 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 598.2 KB | 179.2 KB |
| action | 456.5 KB | 179.2 KB |
| action-list | 563.0 KB | 179.2 KB |
| cohort | 455.4 KB | 179.2 KB |
| cohort-list | 562.0 KB | 179.2 KB |
| email-template | 455.3 KB | 179.2 KB |
| error-details | 471.1 KB | 179.2 KB |
| error-issue | 456.1 KB | 179.2 KB |
| error-issue-list | 562.9 KB | 179.2 KB |
| experiment | 560.1 KB | 179.2 KB |
| experiment-list | 563.8 KB | 179.2 KB |
| experiment-results | 561.8 KB | 179.2 KB |
| feature-flag | 565.8 KB | 179.2 KB |
| feature-flag-list | 569.5 KB | 179.2 KB |
| feature-flag-testing | 459.4 KB | 179.2 KB |
| insight-actors | 560.8 KB | 179.2 KB |
| invite-email-preview | 454.7 KB | 179.2 KB |
| llm-costs | 558.1 KB | 179.2 KB |
| session-recording | 457.2 KB | 179.2 KB |
| session-summary | 462.5 KB | 179.2 KB |
| survey | 457.0 KB | 179.2 KB |
| survey-global-stats | 560.9 KB | 179.2 KB |
| survey-list | 563.7 KB | 179.2 KB |
| survey-stats | 560.8 KB | 179.2 KB |
| trace-span | 455.8 KB | 179.2 KB |
| trace-span-list | 562.9 KB | 179.2 KB |
| workflow | 455.8 KB | 179.2 KB |
| workflow-list | 562.4 KB | 179.2 KB |
| query-results | 743.7 KB | 179.2 KB |
| render-ui | 824.0 KB | 179.2 KB |
| visual-review-snapshots | 460.3 KB | 179.2 KB |
⚠️ Playwright — 1 failed
🎭 Playwright report · View test results →
❌ 1 failed test:
- password-protected insight sharing (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
⚠️ Backend coverage — 96.0% of changed backend lines covered — 7 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ███████████████████░ 96.0% (181 / 188)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/engineering_analytics/backend/presentation/views.py |
37.5% | 955–956, 961–963 |
products/engineering_analytics/backend/facade/api.py |
50.0% | 321 |
products/engineering_analytics/backend/logic/__init__.py |
80.0% | 337 |
🤖 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 29413362109 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
engineering_analytics |
██████████████████░░ 90.0% |
5,040 / 5,600 |
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.
✅ ClickHouse migration SQL — none
No ClickHouse migrations in the latest push.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR moves the Engineering Analytics “Currently broken tests” panel off client-side HogQL and into a new server-side broken_tests read endpoint, then wires it through generated OpenAPI/MCP tooling so it works in OAuth/standalone contexts and can be reused by automation (e.g. Signals emitters).
Changes:
- Added
engineering_analytics/broken_tests/backend endpoint (facade → logic → queries → serializers/viewset) that fingerprints recent CI failures, folds a 24h sparkline, joins default-branch job status, and classifies/ranks failures. - Updated the Engineering Analytics frontend to render the new typed endpoint response (including lazy drill-down via
run_failure_logs) and removed the previous client-side query/classifier path. - Enabled/registered the new MCP tool (
engineering-analytics-broken-tests) and updated the CI-failure investigation skill to use it as the triage entry point; also addedmaster_failed_countto the flaky-test leaderboard.
Reviewed changes
Copilot reviewed 18 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| services/mcp/src/tools/generated/engineering_analytics.ts | Adds the generated MCP handler for engineering-analytics-broken-tests and registers it in GENERATED_TOOLS. |
| services/mcp/src/generated/engineering_analytics/api.ts | Adds Zod schemas for the new broken-tests operation params/query params (OpenAPI-derived). |
| services/mcp/src/api/generated.ts | Extends generated MCP API types/schemas to include broken-tests response and related enums. |
| services/mcp/schema/tool-definitions-all.json | Publishes the new tool definition (description/scopes/metadata) into the aggregated tool schema. |
| services/mcp/schema/generated-tool-definitions.json | Publishes the new generated tool definition into the generated-tool schema. |
| products/engineering_analytics/skills/investigating-ci-failures/SKILL.md | Updates the skill docs to start triage with the new broken-tests MCP tool. |
| products/engineering_analytics/mcp/tools.yaml | Enables and documents the engineering-analytics-broken-tests MCP tool mapping to the new operation. |
| products/engineering_analytics/frontend/scenes/EngineeringAnalyticsTestHealth.tsx | Replaces the old panel implementation with endpoint-driven rendering + drill-down log loading; adds master-failure column to the flaky leaderboard UI. |
| products/engineering_analytics/frontend/scenes/EngineeringAnalyticsTestHealth.stories.tsx | Updates story fixtures to include master_failed_count. |
| products/engineering_analytics/frontend/scenes/engineeringAnalyticsLogic.ts | Adds loaders/selectors/types for broken-tests endpoint data + per-run failure logs caching; maps new master_failed_count. |
| products/engineering_analytics/frontend/generated/api.ts | Adds the generated frontend client for engineeringAnalyticsBrokenTests. |
| products/engineering_analytics/frontend/generated/api.schemas.ts | Adds generated types for broken-tests rows/result + new flaky-test field. |
| products/engineering_analytics/frontend/components/BrokenTestStateTag.tsx | Introduces a state→tag mapping component for broken-test classification labels. |
| products/engineering_analytics/backend/tests/test_flaky_tests.py | Updates tests to cover master_failed_count aggregation semantics. |
| products/engineering_analytics/backend/tests/test_broken_tests.py | Adds unit tests for classification, sparkline folding, join/merge logic, truncation, and “no repo” guard. |
| products/engineering_analytics/backend/presentation/views.py | Adds a new DRF broken_tests action with OpenAPI schema metadata. |
| products/engineering_analytics/backend/presentation/serializers.py | Adds serializers for BrokenTestRow / BrokenTestsResult and documents the new master_failed_count field. |
| products/engineering_analytics/backend/logic/queries/flaky_tests.py | Computes and returns master_failed_count in the flaky-tests query. |
| products/engineering_analytics/backend/logic/queries/broken_tests.py | Implements the broken-tests query/classifier and the cross-cluster merge (failures + master job status + sparkline). |
| products/engineering_analytics/backend/logic/init.py | Wires the broken-tests builder into the logic layer with fixed windows/limits. |
| products/engineering_analytics/backend/facade/contracts.py | Adds new broken-tests dataclasses/enum and the BROKEN_TEST_SPARKLINE_HOURS contract constant; adds master_failed_count to flaky contract. |
| products/engineering_analytics/backend/facade/api.py | Exposes a new get_broken_tests facade entrypoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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.
Contributor
|
✅ Visual changes approved by @webjunkie — baseline updated in 2 changed. |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
Contributor
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 |
- Order the fingerprint scan by urgency proxy (master_hits, branches, last_seen) before the LIMIT, so a trunk-breaking fingerprint can't be evicted by newer low-signal rows before the classifier runs (Codex P1). - Key default-branch job status on (workflow_name, job_name), not job name alone, so a job name reused across workflows can't cross-contaminate the verdict. - Only mark potentially_resolved when the green run is more recent than the fingerprint's last trunk failure; a stale-green warehouse row behind fresher logs now falls through instead of reading as resolved. Ages are integer dateDiffs from now() to keep the classifier tz-agnostic. - Reword the success banner so it doesn't claim every job is green when the job-level source isn't synced (unknown, not green). - Enable the run-failure-logs MCP tool the broken-tests description points to, so an agent can expand a row's latest_run_id into the failing lines.
…broken-tests-panel
…panel' into feat/eng-analytics-broken-tests-panel
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Recovery freshness now keys on completed_at (when the run finished), not
created_at — a run that started before a failure but finished green after it
is a real recovery.
- Drop the signal-floor HAVING so single-branch failures come through and
classify as pr_only; they stay hidden by default and the urgency-ordered cap
keeps them from flooding, so the "show PR-only" toggle has real rows.
- Bound the 24h sparkline scan to the fingerprints that survived the cap
(fingerprint IN {selected}), skipped entirely when the first scan is empty, so
a busy repo doesn't group every recent failure to fill ~200 sparklines.
- Mock the broken_tests endpoint in the Test Health story so the panel renders
populated in visual regression instead of an error state.
…nalytics-broken-tests-panel' into feat/eng-analytics-broken-tests-panel
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dc49694f9
ℹ️ 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".
2 updated Run: adb7bbee-b1bb-4b87-a8d6-f7c8c00a1788 Co-authored-by: webjunkie <59713+webjunkie@users.noreply.github.com>
- branches now uses uniqExactIf(branch, branch != '') so a log record missing its branch metadata doesn't inflate the spread count that feeds the novel/flaky thresholds. - The broken-tests panel shows a "top N by urgency, more matched than fit" note when the result is truncated, so a capped list isn't silently under-reported.
…broken-tests-panel
webjunkie
enabled auto-merge (squash)
July 15, 2026 11:29
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcd38bc80a
ℹ️ 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".
|
Note 🤖 stamphog reviewed Gates denied for size (1261 substantive lines / 16 files, over the 800-line ceiling), which alone requires escalation; additionally a security reviewer's data-integrity concern about forged CI telemetry was explicitly left open pending maintainer judgment rather than resolved.
Gate mechanics and policy version
|
rnegron
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The "Currently broken tests" panel ran client-side HogQL straight from the kea loader against two warehouse views. A local frontend against prod (standalone OAuth) can't call raw
/query/— it resolves to empty required-scopes and rejects regardless of token — so the panel only ever worked off a dev fixture. It also kept the failure classifier in the browser, where the SPEC wants detection defined once in the read layer so the Signals emitter can reuse it.Changes
broken_testsread endpoint (facade/logic/presentation), same shape as its siblings. It fingerprints live CI failures over the last 2 days, folds a 24h sparkline, joins the latest default-branch job status, and classifies each failure (breaking_master/novel_burst/potentially_resolved/flaky/pr_only), most-urgent first, plusbreaking_master_jobsfor the summary banner.dateDiffs in SQL, so the Python side does no timezone math. It embeds the view builders as subqueries rather than reading the registered views by name, keeping the product off the global HogQL catalog per SPEC.engineering-analytics-broken-testsMCP tool and pointed theinvestigating-ci-failuresskill at it as the triage entry point.performQuerycalls, and the client-side sparkline fold. It now maps one typed endpoint response. The drill-down keeps ridingrun_failure_logs.master_failed_countcolumn on the flaky-test leaderboard. It shares files with the panel work, so splitting it into its own PR would have been a risky rebase for little gain.Degrades rather than misreports: without the job-level source synced there is no default-branch job status, so breaking/resolved can't be decided and those failures fall through to flaky/pr_only.
Note
This removes the auth wall. The panel rides the
engineering_analyticsscope like the rest of the tab, so there's no fixture and no raw/query/anymore.How did you test this code?
Automated, and only what I (Claude) actually ran. No manual UI testing.
test_broken_tests.py: the classifier state machine (parameterized over every state plus the no-job-status degradation), the sparkline fold, andquery_broken_testsassembly (join, severity ranking, truncation, empty-repo guard) with the cluster reads mocked. No ClickHouse needed — the pure/mocked layer catches the realistic regressions (a bad threshold, a broken join, a mis-ranked list).ruffclean, frontendtscclean for engineering_analytics, lint-staged (ty check+ formatters) on commit.The panel is visually unchanged from the prototype (same table, columns, drill-down) — the change is the data path, not the UI, so no screenshots.
Automatic notifications
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
I directed this; Claude (Claude Code) implemented it. Skills invoked:
/simplify(a 4-agent cleanup pass over the diff), plus the repo's DRF-endpoint and test-writing guidance.Decisions worth flagging for review:
/query/auth wall as a side effect.workflow_jobsbuilder directly rather than composing the heavierci_job_historyview. A sibling (master_failures) does the same, and this endpoint only needs a strict subset (name/conclusion/status/branch/created_at).(1/5)match a green(3/5)of the same job and produce a wrong "resolved" verdict, so I left that as a deliberate follow-up rather than fold it in here.Warning
Heads up for CI triage:
test_presentation.py::test_requires_rollout_feature_flagis failing on master's current state — the new SDK flag provider bypasses theposthoganalytics.feature_enabledmock the test relies on. Reproduced with this PR's changes stashed, so it isn't from this work.