#167

ci-speedup: stop counting an upstream job's run time as queue (OPT43)

Openyonasbwants to merge intofrom

TL;DR

The queue-time check reported a job's wait for an earlier job to finish as "queue". A verdict job that started seconds after a 4.5-minute test job got flagged with "P90 queue 334s", and its "savable wait" counted the test job's speedup a second time. A job that waits on other jobs is now measured from when those jobs finished, so this false finding goes away. A job that really sits waiting for a runner is still flagged.

before:  run trigger ──[upstream test runs 4m32s]──[3s]── verdict starts   => "queue 334s"
after:                          upstream finishes ──[3s]── verdict starts   => queue ~3s, no finding

What changed

  • collect_runs._detect_opt43_queue_time takes the workflow's scanned job graph (workflow_job_graph[wf_path]). An entry job is still measured from the run trigger. A needs:-gated job is measured from the latest completed_at of its needed upstream jobs in the same run.
  • When a job's start point can't be known (the job matches no graph node, or a needed upstream has no completion in the sampled run), that sample is excluded instead of credited. Skipped jobs are not measured.
  • Reusable workflow calls: every <caller> / <child> job maps to the caller's graph node, which does not carry the children's own needs:. Only the first child of each call to start is measured. Later children may be waiting on a sibling, so they are excluded.
  • A matrix job whose name: is only a placeholder (${{ matrix.os }}) still maps to its node, so its real queue time is still reported, and so is the queue time of jobs that wait on it. Reusable children are matched before name templates, so a template that starts with a placeholder cannot capture them.
  • If the findings input has no scanned job graph for a workflow, collect() skips OPT43 for that workflow and records the skip in detectors_skipped, rather than timing every job from the run trigger.
  • Updated the evidence note, size note, report section copy, agent-prompt copy, catalog entry (both copies) and ARCHITECTURE.md to match the new definition. verify_report does not re-derive OPT43 queue values, so no checker changes were needed.
  • CHANGELOG: one Fixed bullet dated 2026-10-29.

Tests

New detector tests in test_collect_runs_measurement.py:

  • the reported verdict shape produces no finding
  • positive control: a gated job that queued 150s after its upstream finished is flagged at 150s
  • an entry job is still measured from the run trigger (control)
  • a gated job whose upstream completion is missing is excluded
  • a job that matches nothing in the graph is excluded
  • a gated child of a reusable call is not charged a sibling child's run time, and the call's first child is still flagged on real queue
  • a placeholder-named matrix job (${{ matrix.os }}) is still flagged, and so is a job that needs it
  • a reusable child is not captured by a leading-placeholder name template
  • guards: a gated job is timed from the last matrix leg to finish, and a gated placeholder-named matrix job is timed from its upstream

Every behavior test is red on the code before its fix, and the commit messages quote each failure. The controls and guards pass on both versions and pin behavior that must not change.

One wiring test in test_offline_pipeline_e2e.py runs the real data pass over the offline replay corpus. In that corpus, verify (needs: prep) starts 2s after the 120s prep job finishes, and the test asserts it draws no OPT43 finding. Removing the job-graph argument at the call site makes this test fail (job verify P90 queue 122s over 6 runs), so a regression in the wiring turns CI red. A second wiring test removes workflow_job_graph from the scan output and asserts that OPT43 is skipped and disclosed rather than run.

🤖 Generated with Claude Code

Activity

9
greptile-apps[bot]1d ago

Confidence Score: 3/5

[Medium impact] The PR changes how OPT43 findings are measured and explained.

Fix the reusable-child and missing-graph paths before merging; both can still report running work as queue.

Findings

  1.  Reusable jobs show false queue ▶
  2.  Missing graphs inflate queue ▶

Reviews (1) · Last reviewed commit: "fix(ci-speedup): measure OPT43 queue for..." · Reviewed by Greptile

greptile-apps[bot]reviewed1d ago