Repository navigation
fix(ci): make the weekly flaky test report verifiable - #111972
Conversation
|
😎 Merged successfully - details. |
🤖 CI report✅ Trunk lane — non-backend lane (
|
|
[Medium risk] Refactors the weekly test flakiness report logic and data sources. The PR is not safe to merge until the master-burst filter retains tests that have both quarantined and hard-failed runs, and the explicit null-result requirement is met. Reviews (2) · Last reviewed commit: "chore(ci): sync master" |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
HostHog preview —
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe report uses endpoint counts and recent failing-job evidence instead of rescued-run counts. It retrieves Trunk quarantine state through a shared lookup and uses recovery and quarantine facts to filter candidates. Cluster counts use member maximums for failed runs and recoveries. Slack tables show quarantine status, failed PRs, failed runs, and recovered runs. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The report’s quarantine guidance and endpoint handling do not present a remaining merge-blocking issue. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change primarily affects reporting, not CI permissions or quarantine policy. Existing credential and publishing boundaries remain in the inspected code. Repository-to-source selection and live endpoint behavior are not fully confirmed. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
.github/scripts/weekly-flaky-report.mjs-499-499 (1)
499-499: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe legend calls the fix-by date the date the quarantine ends. It is the TTL deadline.
trunkFixByaddsttl_daysto the start date. That gives the date the quarantine should end, not the date it ends. The legend also explains only two labels, fix by and overdue since. With masking on, an entry without a TTL renderssince YYYY-MM-DD, and that date is when the quarantine started. With masking off, every Trunk entry rendersflagged. The legend explains neither label, so readers can mistake asincedate for an end date.Proposed wording
- '*Quarantine* shows the date the quarantine ends (fix by) or the date it ended (overdue since). A quarantine hides the failure for a limited time, so the test needs a fix by that date.', + '*Quarantine* shows the fix deadline (fix by), the deadline that has passed (overdue since), the start date when there is no deadline (since), or flagged when Trunk lists the test but does not mask CI.',.github/scripts/weekly-flaky-report.mjs-313-339 (1)
313-339: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the file quarantine’s expiry in the status.
When an active
.test_quarantine.jsonentry has a matching candidate with same-commit recovery evidence, the report keeps that candidate.resolveFactsattaches only Trunk state, so the row shows-without a Trunk match, or a Trunk-derived status that ignores the file’sexpiresdate. Load and match the file entry, then use its expiry before the Trunk-only guards.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
173d05c9-032f-474e-872b-6b71ae1a6bb3
📒 Files selected for processing (3)
.github/scripts/weekly-flaky-report.mjs.github/scripts/weekly-flaky-report.test.mjs.github/workflows/weekly-flaky-report.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 1 remain after this review.
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
Re-add the stamphog label to request another review once you have addressed this.
stamphog can't approve this pull request automatically, and a human reviewer will need to look at it. Two gates refused it. The deny-list gate matched the infra/CI-CD category because the change touches .github/workflows/weekly-flaky-report.yml and the scripts under .github/scripts/. The tier gate then classified the change as T2-never, a tier that is never auto-reviewed.
The size gate passed, so splitting the PR wouldn't change this outcome. The author should ask a human reviewer, such as someone on the team that owns CI workflows, to review it.
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: infra_cicd |
| size | ✓ | 318L, 2F substantive, 588L/3F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (588L, 3F, single-area, fix) |
| stamphog 2.3.1 | .stamphog/policy.yml @ unknown · reviewed head 72458ea |
|
Risk: No findings This increment refines the weekly flaky-test report's filtering: the master-burst exclusion now keys off Trunk's Sentinel reviewed |
Problem
Changes
Data sources before and after
Before:
flowchart LR F[Flaky tests endpoint]:::api --> R[Weekly report]:::primary H[HogQL rescue counts, links and Trunk state]:::api --> R R --> S[Slack]:::output classDef api fill:#f54e00,stroke:#f54e00,color:#fff; classDef primary fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef output fill:#f9bd2b,stroke:#f9bd2b,color:#000;After:
flowchart LR F[Flaky tests endpoint counts]:::api --> R[Weekly report]:::primary T[Shared Trunk endpoint]:::api --> R H[HogQL log links]:::api --> R R --> S[Slack]:::output classDef api fill:#f54e00,stroke:#f54e00,color:#fff; classDef primary fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef output fill:#f9bd2b,stroke:#f9bd2b,color:#000;How did you test this code?
Release status
Docs update
🤖 Agent context
/code-review,/qa-team,/simplify,/authoring-ci-workflows,/writing-tests,/writing-code-comments,/writing-user-facing-copy,/writing-pr-descriptions,/running-ci-preflight,/reviewing-with-coderabbit,/shipping-a-pr,/merging-prs,/hogli,/chrome-devtools-mcp:chrome-devtools.