Skip to content

perf(ci): run the python static checks concurrently - #131571

Closed
fuziontech wants to merge 1 commit into
masterfrom
perf/ci-python-typecheck-parallel
Closed

fuziontech wants to merge 1 commit into
masterfrom
perf/ci-python-typecheck-parallel

Conversation

@fuziontech

Copy link
Copy Markdown
Member

Problem

Every pull request that touches Python waits on the Python code quality job in Python CI, and for the pull requests it ends, that job is the whole wait: it is 4.5 min at the median and 8.1 min at p90, second only to the Security lane. The job runs every static check one at a time on one runner, so its wall is the sum of six checks that read the same tree and share nothing.

Changes

  • Python code quality now runs its checks as one parallel: block, so it ends on the slowest check instead of on the total. Nothing a person sees changes: the same checks, the same commands, the same severities, and the same fail-on-finding behaviour.
  • Check ty type checking (49s), Check static typing / mypy (134s) and Check if the generated projections are up to date (18s) join the lint and pytest group that already ran concurrently. mypy is the long pole, so the job is bounded by mypy alone.
  • Save mypy cache, the schema.py freshness check and the complexity steps move below the block, because mypy now finishes inside it. This is mechanical.
  • Only run: steps are in the block. An action that writes $GITHUB_PATH or $GITHUB_ENV stays out: that is the race the runner's parallel: implementation crashed on in #76651, recorded under "Put the setup actions in a parallel: block" in docs/internal/ci-things-already-tried.md. The job already used the keyword for its lint group.
flowchart LR
  subgraph before["Before: wall = sum"]
    direction LR
    b1[checkout 33s] --> b2[ty 49s] --> b3[mypy 134s] --> b4[projections 18s] --> b5[lint + pytest 15s]
  end
Loading
flowchart LR
  subgraph after["After: wall = max"]
    direction LR
    a1[checkout 33s] --> a2
    subgraph grp["parallel block"]
      direction TB
      a2[mypy 134s]
      a3[ty 49s]
      a4[projections 18s]
      a5[lint + pytest 15s]
    end
    a2 --> a6[cache save, schema, complexity]
    a3 --> a6
    a4 --> a6
    a5 --> a6
  end
Loading

How did you test this code?

Test rationale: no new test. The change moves steps inside one job and rewrites no check, so the existing coverage is the checks themselves: the same mypy, ty, build:projections --check, ruff and pytest commands run, and the job still fails when any of them fails. The evidence is the job's own duration on this PR's run against seven pre-change runs of the same job.

Baseline, the Python code quality job on 7 pull-request runs of .github/workflows/ci-python.yml before this change:

n median p90 samples (s)
7 279s 359s 279, 279, 279, 279, 279, 279, 359

Serial steps inside those runs, medians: checkout 33s, dependencies 11s, ty 49s, mypy 134s, projections 18s, lint + pytest group 15s.

After, measured on this PR's own run: [pending: this PR's run]

Verification run locally:

  • hogli lint:workflows: 15 checks pass across 144 workflows.
  • tools/workflow-plan: 491 tests pass, including the evaluator pass that says which jobs run on a draft, ready, fork, merge-queue and schedule event.

Not run here: the job itself. This environment cannot build the full dev stack, so the timing above comes from CI rather than from a local run.

  • No feature flag controls this change

Docs update

No docs change: docs/internal/ci-things-already-tried.md already records the parallel: rule this change relies on, and no new behaviour is documented.

🤖 Agent context

Autonomy: Fully autonomous

Agent: Shelley, DeepSeek V4 Flash (deepseek-flash-api-deepseek-com)

Skills: /authoring-ci-workflows, /writing-pr-descriptions

More context for AI agents
  • Session: nightly CI-speed loop; measured 45 recently merged PRs through the GitHub API.
  • Decisions: the PR lane is a max, so only the longest job per workflow moves it. Security ends the lane for 28 of 45 PRs; #131450 already takes that one. Python is the next-longest lane, so it is this change's target.
  • Decisions: queue time is no longer a lever. After #125985 and #125810, no job in the sampled PRs queues longer than 2s.
  • Decisions: kept the runner. mypy pins its worker count to the runner's memory, and the added checks are light beside it.
  • Follow-ups: if mypy grows past the block's other work, the block stops paying and the job should move to depot-ubuntu-24.04-4 instead.
  • Read closely: the block's step list in .github/workflows/ci-python.yml. Every branch must stay a run: step.

"The `code-quality` job ran every static check one after another on one
runner, so the job wall was their sum. Measured over 7 pull-request runs
of `.github/workflows/ci-python.yml`, that job is 279s at the median and
its serial steps add up to the same 279s: ty 49s, mypy 134s, the
generated-projections check 18s, and the lint/pytest group 15s.

They read the same tree and share no state, so they now run as one
`parallel:` block and the wall becomes the longest of them. mypy is the
long pole, so the job is bounded by mypy's own duration instead of
mypy's plus every other check's. The job keeps its runner: the extra
checks are light next to mypy, which already pins its worker count to
the runner's memory.

Every command, severity and target is unchanged, and the block holds
only `run:` steps. An action that writes `$GITHUB_PATH` or `$GITHUB_ENV`
must stay out of a `parallel:` block: that is the race the runner's
implementation crashed on in `#76651`, which is why the setup actions
stay outside it. This job already used the keyword for its lint group,
so this extends a pattern the workflow relies on rather than adding one.

`Save mypy cache`, the `schema.py` freshness check and the complexity
steps move after the block, because mypy now finishes inside it.
"

Co-authored-by: Shelley <shelley@exe.dev>
@trunk-io

trunk-io Bot commented Nov 9, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@stamphog
stamphog Bot requested a review from a team November 9, 2026 06:45
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Hey @fuziontech! 👋

It looks like your git author email on this PR isn't your @posthog.com address (fuziontech@gmail.com). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

  • Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
  • Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

@parameterai

parameterai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Risk: No findings

This PR moves the ty, mypy, and generated-projections checks into the existing parallel: step block of the Python code quality job in .github/workflows/ci-python.yml, and moves the mypy cache save, schema.py freshness check, and complexity reporting below that block. I traced the ordering and failure semantics, the secret and permission surface, and the documented runner race on parallel: blocks, and found no security issue: the block contains only run: steps (the documented-safe shape), no step in it writes $GITHUB_ENV/$GITHUB_PATH, the build:projections --check path writes nothing, and all secret, token, and permission wiring is byte-for-byte unchanged. The reorder only means some post-block checks are skipped when the job is already failing, which leaves the job red and blocks merge either way.

Sentinel reviewed c56f8a2 · Review settings

@veria-ai

veria-ai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

PR overview

No security issues were found in this pull request.

Reviews

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

@fuziontech

Copy link
Copy Markdown
Member Author

Measured, and it is a regression. Closing without merging.

What I expected: the code-quality job would end on mypy instead of on the sum of its checks.

What happened: the job took 528s against a like-for-like baseline of 382s, which is 38% slower. Both sides ran with every gated pytest suite switched on, because this PR edits .github/workflows/ci-python.yml itself and that path is in every filter in the changes job. The pre-change sample is the three merged PRs that also touched this workflow: #131396 (298s), #131326 (382s), #131008 (383s).

Per step, same job, same gated suites:

step before (3 PRs) after (this PR)
Check static typing (mypy) 106s / 187s / 146s 415s
Check ty type checking 46s / 53s / 53s 137s
Check if the generated projections are up to date 36s / – / – 45s
Parallel group 47s / 49s / 41s 402s

The block itself took 402s and finished after mypy's 415s step had already ended, so the concurrency bought nothing and cost a lot.

Why it fails: code-quality runs on depot-ubuntu-24.04, which is 2 vCPU. mypy already saturates both cores with its own worker, and ty runs its own parallel checks. Overlapping them does not divide the work, it time-shares one saturated CPU between three jobs, and every one of them pays the contention. On a 2-vCPU runner the serial order is the fast order.

That also means the idea is not reachable from this job as it stands. It would need the -4 or -8 runner first, and then the extra runner cost competes with the wall time saved, which is the trade this repository has already rejected for WebKit (docs/internal/ci-things-already-tried.md, "Run the Storybook WebKit shards on 4-CPU runners").

Recording it under docs/internal/ci-things-already-tried.md on a follow-up branch so the next person does not build it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant