Skip to content

ci(review): wait for Codecov before the review reads CI - #163

Merged
MarketDataDev02 merged 2 commits into
mainfrom
ci/review-wait-for-codecov
Oct 2, 2026
Merged

MarketDataDev02 merged 2 commits into
mainfrom
ci/review-wait-for-codecov

Conversation

@MarketDataDev02

@MarketDataDev02 MarketDataDev02 commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #162

What happened

The review starts when Tests or Lint finishes, and require_ci_green reads the head's check runs at that moment. Codecov reports codecov/patch and codecov/project 35 to 50 seconds after Tests, and nothing starts a review when they finish. On #159 most reviews read CI before Codecov had reported, so a red codecov/project could not stop them. One read landed while Codecov was running: that review skipped and came back only through a manual re-review. The measurements are in #162.

The change

.github/code-review/policy.yml lists both checks in wait_for_checks, the key MarketData-App/code-review-bot#25 added (merged 2026-10-01). The review now waits until both exist and nothing on the head is still running, for 300 s at most (wait_for_checks_seconds, left at its default), and stops at once when CI is already red. The wait runs before the shared Codex credential is borrowed, so it holds the runner, not the lease. The comment over the key says what it does and what it costs a documentation-only pull request.

The bot reads the policy from the base branch, so this applies to pull requests once it is on main, and changes nothing for the open ones before that.

Cost: a documentation-only pull request skips the suite, so Codecov never reports on it. Its review waits the 300 s cap and then skips, since ignore_paths covers every file it changes.

Version impact

None: repository configuration only. No CHANGELOG entry.

Proof

Codecov reports as check runs, which is what the wait reads. On da96b09, the head of #157, codecov/patch and codecov/project are check runs of the codecov app, and the commit has no statuses.

The bot's own loader accepts this policy. reviewbot.config.load from code-review-bot at 19247a3, the merge of MarketData-App/code-review-bot#25, on the policy of this branch, on main's, and on two broken variants:

new policy        ok wait_for_checks=['codecov/patch', 'codecov/project'] seconds=300 require_ci_green=True check_name='Code review'
main policy       ok wait_for_checks=[] seconds=300 require_ci_green=True check_name='Code review'
typo key          PolicyError: unknown policy key: wait_for_check
own check listed  PolicyError: wait_for_checks may not list 'Code review', the review's own check, which the wait never sees

The same loader at 082fe19, the bot's main before that merge, rejects this policy with PolicyError: unknown policy key: wait_for_checks. code-review.yml calls the bot's review.yml@main, which has the key.

The wait, run on a measured case. 7759248, a head of #159, is the -24 s row of #162: the review read CI under main's policy at 18:02:43Z (the reviewbot run step of run 36462367405), and Codecov completed at 18:03:06Z and 18:03:07Z. Its recorded check runs, with the start and completion times the GitHub API keeps, replayed through the bot's own wait_for_checks and ci_state at 19247a3, with the transport answering from the recording and a simulated clock that starts at 18:02:43Z:

18:02:43Z [main's policy] Tests passed=success, codecov/patch=absent, codecov/project=absent
18:02:43Z [main's policy] wait returned after 0s; reviewbot run reads ci_state=success
18:02:43Z [this branch's policy] Tests passed=success, codecov/patch=absent, codecov/project=absent
18:02:53Z [this branch's policy] Tests passed=success, codecov/patch=absent, codecov/project=absent
18:03:03Z [this branch's policy] Tests passed=success, codecov/patch=absent, codecov/project=absent
18:03:13Z [this branch's policy] Tests passed=success, codecov/patch=success, codecov/project=success
reviewbot wait: codecov/patch, codecov/project finished
18:03:13Z [this branch's policy] wait returned after 30s; reviewbot run reads ci_state=success

Under main's policy the review reads a green CI that has no coverage in it, which is what happened on that head. Under this branch's policy it waits three polls of 10 s and reads CI with both Codecov checks in it.

The same code against the live API, on this pull request's head 1f1ea6e. The bot's wait_for_checks, with the GitHub client reading the live check runs and the policy swapped for each file, started the moment Tests passed completed:

  • attempt 1, 17:59:34Z: test (3.10) failed on the FloatingPointError of A pandas DataFrame with a null date can raise FloatingPointError at random #155, so both policies returned at once, this branch's with reviewbot wait: CI is already red on the head commit; nothing to wait for;
  • attempt 2, after re-running that job, 18:01:46Z: Codecov had already reported, so both returned at once with ci_state=success.

Checks. uv run pytest -n 4 --cov=marketdata: 1959 passed, coverage 100% (3196 statements, 0 missed). ./lint.sh clean. git merge-tree is clean against main and every open pull request.

Review round 1

Measured on 2026-10-01, on 1f1ea6e.

Proof. The review asked for a dry run showing main's policy reading CI before Codecov and this one waiting for the same checks, with the test count and coverage. All three are under Proof: the replay of 7759248 runs the two policies from the same instant on the same check runs, and the live runs on 1f1ea6e show the same code reading this pull request's checks. 1f1ea6e also adds one line to the comment over the key, on what the wait costs a documentation-only pull request.

The review starts when Tests or Lint finishes, before Codecov reports
codecov/patch and codecov/project, so require_ci_green read CI without
coverage, or skipped the review while Codecov was running. The policy
now lists both in wait_for_checks, and the review waits for them for
300 s at most.
@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (2090695) to head (1f1ea6e).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #163   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           62        62           
  Lines         3196      3196           
=========================================
  Hits          3196      3196           
Flag Coverage Δ
unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marketdata-code-review

marketdata-code-review Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

✅ Ready — Code review

This configuration makes code review wait for both Codecov checks before evaluating CI, preventing coverage failures from being missed. Version impact: none; it changes repository configuration only and touches no SDK public symbols.

Rating patch 6/6 (exemplary) · proof 6/6 (comprehensive) · overall 6/6


Reviewed 1f1ea6e · revision 2 · codex (gpt-5.6-sol) · claude unavailable · rules: default, sdk

@MarketDataDev02
MarketDataDev02 merged commit c14717e into main Oct 2, 2026
16 of 18 checks passed
@MarketDataDev02
MarketDataDev02 deleted the ci/review-wait-for-codecov branch October 2, 2026 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code review can read CI before Codecov reports, so require_ci_green misses coverage

2 participants