chore(devex): lint cyclomatic complexity of changed files - #91305
Conversation
Complexity 11-15 warns, above 15 fails — in hogli lint:complexity, ci:preflight, and CI (ci-python, ci-frontend). Scoped to changed files under posthog/, ee/, products/, and frontend/. Python via ruff C901, TypeScript via a small checker on the TS compiler API (oxlint has no complexity rule). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
😎 Merged successfully - details. |
🤖 CI report🚨 Trunk lane — universal laneThis PR is assigned to the universal lane. It cannot merge in parallel with other PRs, so it can take longer to merge. Ask dev-ex if you think this is wrong. ✅ Complexity (TypeScript) — cleanCyclomatic complexity above 10 in changed typescript files. Warn only: worth simplifying when you next touch these functions. ✅ Bundle size — no changeUncompressed size of every built Total: 68.04 MiB · no change No file changed by more than 1000 B. 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 |
|---|---|
| 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 |
| 252.9 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.7 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 | no change | ████░░░░░░ 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 |
|---|---|
| 750.7 KiB | dist/toolbar/toolbar-app-HC2BLVR5.css |
| 588.3 KiB | dist/toolbar/chunk-chunk-UZOF23P7.js |
| 484.7 KiB | dist/toolbar/chunk-chunk-OQ4CRAT2.js |
| 134.1 KiB | dist/toolbar/chunk-chunk-ESP3L22S.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 71.3 KiB | dist/toolbar/toolbar-app-PMPWTAOL.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-XNAOAA7H.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-U6ERKZIE.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 — no change
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1443.06 MiB · no change
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5ff46c8ab
ℹ️ 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".
pauldambra
left a comment
pauldambra
left a comment
There was a problem hiding this comment.
Note
🤖 Automated comment by QA Swarm — not written by a human
QA Swarm review complete. See inline comments.
|
Note 🤖 Automated comment by QA Swarm — not written by a human Multi-perspective review: router (cheap-first pass) + delegated reviewers (qa-team, paul-reviewer, xp-reviewer, security-audit as warranted) Verdict: ✅ APPROVE (round 3 @ f3c1657)Round 3 reviewed the switch of both CI complexity steps from hand-rolled git fetch-and-diff to the changes job's paths-filter file list (list-files: escape, consumed with the ci-backend.yml printf|xargs precedent). Wiring, if-conditions, and globs all check out. Danger LOW, confidence HIGH, no delegation. Key findingsNone this round. ConvergenceNone — single-reviewer round (router did not delegate). Reviewer summaries
Previous rounds (2)round 1 @ e5ff46c — ✅ APPROVE: implementation verified empirically; 2 LOW + 1 NIT findings, all since fixed and resolved. Automated by QA Swarm — not a human review |
Open PRs that haven't rebased onto this commit lack bin/lint-complexity.mjs and the lint:complexity hogli command, so the new CI steps would fail before their existing checks finish. Skip each step gracefully when its companion tooling isn't present yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- resolve class-field arrow function names (e.g. handleClick = () => {})
instead of falling through to <anonymous>
- only count files toward "checked" when their checker actually ran, and
surface a degraded run to stdout so a soft preflight check reports it as
a warning instead of a silent pass
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hecked The out-of-scope fixture path didn't exist, so it was filtered by the earlier is_file() check rather than by matches_globs against PYTHON_SCOPE, which the test comment claims to cover. manage.py exists and is genuinely out of scope, so it exercises the intended branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github.event.pull_request.base.sha can lag the merge ref GitHub rebuilds against current master, so a two-dot diff against it blamed files master changed after the event fired. HEAD^1 is the exact base snapshot the checkout contains, so the diff can never include master's own commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Gates denied on CI workflow changes (deny-listed infra_cicd) and cross-cutting size; this is risky territory (modifies GitHub Actions CI jobs) and the author is not on the owning devex team, with no independent review verified against the current head commit.
Gate mechanics and policy version
|
The hand-rolled fetch-and-diff snippets already misfired once when the event's base sha lagged the rebuilt merge ref. paths-filter reads the PR files API, which is authoritative for what the PR changed, so the steps now consume its escaped file list instead of any git plumbing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…CI report Batched PRs kept failing CI on the error tier for pre-existing complex files they only touched. Drop the error tier: every finding above the threshold is now a warning, and the check never fails a job or preflight. Findings now surface in the grouped CI report comment (one section per language) alongside the diff annotations, so reviewers see them without opening job logs. Both linters gained a --report flag that writes the findings as JSON for the new .github/scripts/post-complexity-section.mjs poster; clean runs post an ok section so stale warnings clear on the next push. Generated-By: PostHog Desktop Task-Id: 3a98bf2c-534d-40b2-bbd7-b34eb51e7afd
🦔 Hogbox preview · ❌ build failedThe preview didn't come up for commit 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. |
![parameterai[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
File paths and function names in the findings come from PR-authored files, and git allows filenames containing backticks, pipes, and newlines. Interpolated into the CI report comment raw, a filename could break out of the table or forge section markers that the report parser trusts as metadata on the next write. Strip control characters, backticks, and pipes from interpolated cells via a shared markdownCell helper in the ci-report formatter, with tests covering the forged-marker case. Generated-By: PostHog Desktop Task-Id: 3a98bf2c-534d-40b2-bbd7-b34eb51e7afd
Generated-By: PostHog Desktop Task-Id: 3a98bf2c-534d-40b2-bbd7-b34eb51e7afd
pauldambra
added
the
stamphog
PostHog
![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.
Gates denied this PR (CI/workflow files touched, classified as never-auto tier), and the change is genuine infra/CI-pipeline territory — new GitHub Actions steps and workflow wiring, not incidental. The only "approvals" are on stale commits (gantoine, Codex) or self-posted by the author's own account (QA Swarm), so there's no independent assurance on the current head, and the author isn't on the owning devex team.
- Gate verdict DENIED (deny-list: infra_cicd, tier: T2-never) — cannot auto-approve regardless of content
- Modifies .github/workflows/ci-frontend.yml and ci-python.yml plus adds a new CI report-posting script — genuine CI/deploy tooling change
- Author pauldambra is not on the owning @PostHog/team-devex team
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: infra_cicd |
| size | ✓ | 485L, 9F substantive, 611L/12F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (611L, 12F, cross-cutting, chore) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 9e5fad2 · reviewed head 9e5fad2 |
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2 participants
it really stuck with me when Tom O said (roughly) "agents can template warehouse sources cos the code is really clean, so it's easy to copy"
but in general at PostHog we've not bothered too much about "clean code" signals like file/module size or code complexity. leaning on "high talent density" to get by... but agents are not talented humans so...
Problem
Changes
hogli lint:complexityruns the same check on demand: changed files by default, explicit paths, or--against <ref>. It exits 0 even with findings.bin/lint-complexity.mjson the TypeScript compiler API, since oxlint has no complexity rule. A test pins the thresholds and the--reportcontract between the two sides.hogli ci:preflightshows the findings as a non-blocking⚠ warning.hogli.yamlcommand wiring, the running-ci-preflight skill note, and a new.github/scripts/post-complexity-section.mjswriter for the CI report.Before
flowchart LR D{{PR diff}} --> R[ruff check, repo-wide] D --> O[oxlint, repo-wide] R --> G[CI verdict] O --> G classDef phBlue fill:#1d4aff,color:#fff,stroke:#1d4aff classDef phYellow fill:#f9bd2b,color:#000,stroke:#f9bd2b class R,O phBlue class D,G phYellowAfter
flowchart LR D{{PR diff}} --> C[complexity check, changed files only] C --> W[warnings: diff annotations + CI report sections] D --> R[ruff check, repo-wide] D --> O[oxlint, repo-wide] R --> G[CI verdict] O --> G classDef phBlue fill:#1d4aff,color:#fff,stroke:#1d4aff classDef phYellow fill:#f9bd2b,color:#000,stroke:#f9bd2b class C,R,O phBlue class W,D,G phYellowHow did you test this code?
tools/hogli-commands/hogli_commands/tests/test_complexity_lint.py:--reportJSON file the CI poster reads.--reportcontract of the mjs pinned to the Python side.--strict.frontend/bin/ci-reportjest tests pass with the two new section ids registered.hogli lint:complexityonfrontend/src/scenes/data-warehouse/editor/OutputPane.tsx(the file that failed CI in batched PRs) now prints 5 warnings and exits 0;hogli lint:workflowsand mypy on the hogli package pass.Automatic notifications
Docs update
The running-ci-preflight skill documents the warning-only behavior in this PR.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
--reportflag so CI steps hand findings to the report poster as JSON.Created with PostHog Desktop