Skip to content

fix(pacing): anchor intervals to transport starts - #1898

Draft
Wibias wants to merge 3 commits into
devfrom
wibias/fix-request-pacing-transport-start
Draft

fix(pacing): anchor intervals to transport starts#1898
Wibias wants to merge 3 commits into
devfrom
wibias/fix-request-pacing-transport-start

Conversation

@Wibias

@Wibias Wibias commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • make provider request pacing measure the interval at the actual outbound transport start instead of before an awaited continuation;
  • replace the wall-clock-sensitive concurrency assertion with an injected-clock regression that reproduces delayed continuation starts deterministically.

Verification

  • RED phase: test-only commit intentionally reproduces the current scheduling gap in CI before the implementation change.
  • Final focused/full checks will be recorded after the production fix lands.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. No docs change is required because this restores the documented/configured pacing guarantee without changing configuration.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. This change does not touch credentials, authentication, or secret handling.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 17, 2026
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 32665ad3-a888-4837-82b3-9787a1ddf3ed

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Wibias Wibias closed this Aug 17, 2026
@Wibias
Wibias force-pushed the wibias/fix-request-pacing-transport-start branch from 3aaa9d2 to 417ce9e Compare August 17, 2026 06:36
@Wibias Wibias reopened this Aug 17, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Reviewed as part of the Wave 5B train and held, not rejected — the direction is right and
three of the five things this needed are already here.

Present and verified in the diff:

  • intervals anchored to transport start rather than queue entry
  • a cancelled waiter does not consume a slot (aborted queued requests leave immediately and never consume a start)
  • deterministic timing via an injected clock rather than wall time

Two are missing, and they are the ones that decide whether the pacing clock is actually
correct under load:

  1. Retry does not double-advance the clock. A retried request that re-enters the pacer
    must not count as a second transport start, or the effective interval silently doubles
    under exactly the conditions that trigger retries.
  2. Per-account isolation. account appears zero times in this diff. If one account's
    pacing can delay another's, the fix trades a burst problem for a starvation problem.

One smaller thing: the description still says "Final focused/full checks will be recorded
after the production fix lands," but the production fix is already in the diff — 56 lines in
request-pacing.ts and 72 in fetch-helpers.ts, including the
waitForProviderRequestSlotrunWithProviderRequestSlot signature change. Worth refreshing
so a reviewer is not told to expect something that already happened.

Happy to see this land once those two tests exist. The PR is still draft, so nothing is
blocked by this note.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants