Skip to content

ci: let test-polars and gfql-routes-off start without waiting on test-gfql-core (#2042) - #2126

Merged
lmeyerov merged 3 commits into
masterfrom
ci/decouple-polars-routes-off-from-gfql-core
Oct 4, 2026
Merged

ci: let test-polars and gfql-routes-off start without waiting on test-gfql-core (#2042)#2126
lmeyerov merged 3 commits into
masterfrom
ci/decouple-polars-routes-off-from-gfql-core

Conversation

@lmeyerov

@lmeyerov lmeyerov commented Oct 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

#2042 asked why test-gfql-core gates downstream jobs. Measured on #2121's green run at 11edb3c:

lane start → end
test-gfql-core (3.12) 07:12:09 → 07:22:31 (10.3 min)
test-polars (3.12) 07:22:38 → 07:35:36 (started only when gfql-core finished)
gfql-routes-off (all-off) 07:22:33 → 07:41:36 (same; the slowest lane, 19 min)
changed-line-coverage 07:35:38 → 07:36:00
run wall-clock 30.7 min

Both test-polars and gfql-routes-off listed test-gfql-core in needs, so a 10-minute lane sat in front of the two longest lanes. The lane's own comment says it "runs in parallel with downstream jobs instead of blocking them"; this PR makes that true for these two: test-polars keeps test-minimal-python as its smoke gate, gfql-routes-off keeps changes/generate-lockfiles. Neither consumes an artifact from test-gfql-core: each lane's only download-artifact step pulls lockfiles from generate-lockfiles, which stays in needs (gfql-core uploads gfql-coverage-audit-py3.12, read only by changed-line-coverage). changed-line-coverage still needs it for the coverage artifact; the ai/infra-only lanes (test-core-umap, test-full-ai, test-spark) are unchanged.

Expected effect: both lanes start ~10 min earlier, so wall-clock drops from ~31 min to ~21 min (bounded by routes-off itself). Cost: on a red test-gfql-core, these two lanes now run anyway.

The original complaint (test-gfql-core (3.14) at 9+ min) no longer holds: that lane took 5.2 min on the same run; the coverage-audited 3.12 lane is the 10-minute one.

Review (2026-10-04): dropping the edge also dropped gfql-routes-off's transitive lint gate, so that lane now needs python-lint-types directly (ends at 0.8–1.2 min, so the gain stands); the in-file comment states which lane gates on what; CHANGELOG Infrastructure entry added. The routes-off half of the saving is a projection: this PR's own run skips gfql-routes-off (infra-only diff), so only the test-polars start (11.8 → 2.9 min) is measured. #2042's other half, the coverage-audited 3.12 cells taking twice the plain ones, is not addressed here and is now the critical path.

Test plan

  • yaml.safe_load parses; needs edges read back as intended
  • This PR's run at e08dae1: test-polars (3.12) started at 2.9 min instead of 11.8; wall-clock 16.5 min (routes-off skipped on this diff)

🤖 Generated with Claude Code

https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp

@lmeyerov @claude
ci: let test-polars and gfql-routes-off start without waiting on test…
e08dae1 
…-gfql-core (#2042)

Measured on PR run 11edb3c: test-gfql-core (3.12) ran 07:12-07:22, and both lanes
started at 07:22 only because they listed it in needs; routes-off then ran to 07:41.
The lane's own comment already says it runs in parallel with downstream jobs. Keep
test-minimal-python as the smoke gate for test-polars; changed-line-coverage still
needs the lane for its coverage artifact.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
@lmeyerov lmeyerov mentioned this pull request Oct 3, 2026
Open
@lmeyerov

lmeyerov commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Measured on this PR's own run (e08dae1, 81/81 green):

lane start → end
test-gfql-core (3.12) 07:49:21 → 07:59:25
test-polars (3.12) 07:51:00 → 08:04:03 (started 1.6 min after gfql-core began, gated on test-minimal-python only — on 11edb3c it waited the full 10.3 min)
changed-line-coverage 08:04:06 → 08:04:36
run wall-clock 16.4 min (vs 30.7 min on 11edb3c)

Caveat on the wall-clock: a ci.yml-only diff does not trigger the gfql-routes-off cells, so the 16.4 min is the polars-bound path, not the routes-off-bound one; the apples-to-apples claim is the test-polars start moving from +10.3 min to +1.6 min. The routes-off cells carry the same needs edit and will show the same shift on the next GFQL-touching PR.

@lmeyerov lmeyerov mentioned this pull request Oct 4, 2026
Merged
1 task done
lmeyerov and others added 2 commits October 4, 2026 00:53
@lmeyerov @claude
ci: gfql-routes-off keeps its lint gate; comment and changelog state …
d2c0dca 
…the gating contract

Review of #2126 found that dropping the test-gfql-core edge also dropped the routes-off lane's
transitive python-lint-types gate (11 cells would run on a red lint), and that the new comment
described both lanes as gated on test-minimal-python when routes-off was gated on nothing. The
lane now needs python-lint-types directly (sub-minute, so the critical-path gain stands), the
comment says which lane gates on what, and the change has an Infrastructure changelog entry.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
@lmeyerov
Merge remote-tracking branch 'origin/master' into ci/decouple-polars-…
6a7df4b 
…routes-off-from-gfql-core

# Conflicts:
#	CHANGELOG.md
@lmeyerov

lmeyerov commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Read-only review (parallel session) — CI decoupling, head e08dae1

Nothing here touches the branch; findings only, fixes deferred.

Scope: .github/workflows/ci.yml only (+4/-2). Diff range origin/master...e08dae1.

Verified against master's ci.yml

  • changed-line-coverage still lists test-gfql-core in needs (ci.yml:1304) and is the only consumer
    of the gfql-coverage-audit-py3.12 artifact (uploaded ci.yml:1264, downloaded ci.yml:1344). Neither
    test-polars nor gfql-routes-off downloads it. The decoupling drops no data edge.
  • The PR's own run (37107546804): test-gfql-core (3.12) 07:49:21→07:59:25, test-polars (3.12) started
    07:51:00 (1.6 min after gfql-core started, previously it waited ~10 min), run wall-clock 16.5 min vs the
    30.7 min baseline quoted. The polars half of the claim is demonstrated.
  • gfql-routes-off did NOT run on that PR (the change is infra-only, so changes.outputs.gfql is false
    and the lane is skipped) — the routes-off half of the claim is unverified by the PR's own CI. It will be
    exercised by the first gfql-touching PR after merge; nothing suggests it will break (same needs
    pattern as before minus one edge).

Findings

  • IMPORTANT (operability, not blocking): the stated cost — "on a red test-gfql-core these two lanes now run
    anyway" — also means a broken GFQL core no longer short-circuits ~35 min of polars + routes-off compute
    on every push of a red branch. Acceptable for wall-clock, but worth a one-line note in the lane comment so
    nobody re-adds the edge "for safety" without seeing the trade.
  • SUGGESTION: the comment added above test-gfql-core says test-polars gates on test-minimal-python
    "only" — it also gates on changes and generate-lockfiles; tighten the wording.

Recommendation: merge. Watch the first gfql-touching PR afterwards to confirm routes-off starts alongside
gfql-core and that the 11 routes-off cells still pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_017ropeBMLJUuy6ViYwy15ud

@lmeyerov

lmeyerov commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Agreed on both IMPORTANT items; fixed at 6a7df4b.

Comment misdocuments the gating contract, and routes-off lost its lint gate. Both confirmed. gfql-routes-off now has needs: [changes, python-lint-types, generate-lockfiles], so the 11 cells cannot run on a red lint, and the critical-path gain stands because lint ends inside the first minute. The comment now says which lane gates on what instead of claiming both gate on test-minimal-python.

CHANGELOG. Added under ### Infrastructure, following the test-docs entry you pointed at.

The PR's own run skips the routes-off lane. Right, and the body now says so: only the test-polars start is measured (11.8 to 2.9 minutes), the routes-off half is a projection, and #2042's other half — the coverage-audited cells costing twice the plain ones, now the critical path — is untouched here and stays open. I will watch the first GFQL-touching run after this lands, as you suggest.

CI at this head: 79 success, 2 skipped, no failures.

@lmeyerov
lmeyerov merged commit 28d8082 into master Oct 4, 2026
82 checks passed
@lmeyerov lmeyerov mentioned this pull request Oct 4, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@lmeyerov