-
Notifications
You must be signed in to change notification settings - Fork 1
fix(gateway): recover free structured synthesis after provider 429 #1251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+498
−6
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
6042d35
fix(gateway): recover structured synthesis after 429
seonghobae 478ffcf
fix(gateway): cover structured repair and quota evidence
seonghobae aa00d63
fix(gateway): wait for quota route after size rejection
seonghobae 6801a40
Merge remote-tracking branch 'origin/main' into HEAD
seonghobae 15b9634
fix(gateway): stop structured quota replay without an active cooldown
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: ContextualWisdomLab/contextual-orchestrator
Length of output: 43214
🏁 Script executed:
Repository: ContextualWisdomLab/contextual-orchestrator
Length of output: 22307
쿨다운이 0인 429 라운드는 즉시 재시도하지 말고 종료하세요.
rate_limit_unknown_cooldown_seconds=0이면 429 처리 후 쿨다운이 즉시 만료됩니다. 이때cooling이 비어_await_rate_limit_recovery를 호출하지 않고,send_synthesis_once를 다시 호출합니다. 429 후보는request_exclusions에 추가되지 않으므로wait_deadline까지 공급자 요청이 반복될 수 있습니다.wait_deadline이 이미 만료된 경우의 storm 오류 처리는 유지하세요. 활성 쿨다운이 없는 경우에는 원래 예외를 다시 발생시키세요.🐛 Suggested fix
except ProviderUpstreamError as exc: round_attempts = synthesis_route_attempts[round_start:] if not exc.retryable or not round_attempts or any( row.get("provider_status") != 429 and row.get("outcome") != "request_too_large" for row in round_attempts ): raise + if not any( + self._rate_limit_remaining(candidate.id) is not None + for candidate in synthesis_candidates + if candidate.id not in request_exclusions + ): + raise if wait_deadline is None: wait_deadline = time.monotonic() + self._rate_limit_wait_budget(preferred) final_agent = preferred🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified against aa00d63 and reproduced the finding. Zero-cooldown all-429 and mixed 429/413 cases generated 210/257 extra calls within a 20ms wait budget. Owner-preserving successor #1284 at ced8859 adds the shared active-cooldown guard, preserves already-expired storm handling, and strengthens cooldown expiry assertion at transport entry. Structured regression file: 45 passed; adjacent admission file: 35 passed; both process exit 0. No parent head mutation. Hosted current-head gates are queued. Please independently verify the successor; this reply does not resolve the thread or assert approval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Length of output: 7299
🏁 Script executed:
Length of output: 8869
@seonghobae, verified. Commit ced8859 (PR#1284) is fetchable and contains the fix.The
send_synthesisexcept ProviderUpstreamError as excblock adds two guards after the retryable/all-429-or-413 check:if wait_deadline is not None and time.monotonic() >= wait_deadline: continue— preserves the already-expired storm path by looping back to the top, where the existingwait_deadlinecheck raisesrate_limited_storm_error.if not any(self._rate_limit_remaining(candidate.id) is not None for candidate in synthesis_candidates if candidate.id not in request_exclusions): raise— the active-cooldown guard. Whenrate_limit_unknown_cooldown_seconds=0(or any cooldown already expired), no non-excluded candidate has a positive remaining cooldown, so this re-raises the original exception instead of looping into anothersend_synthesis_oncecall.This matches the original finding: the zero-cooldown all-429 case no longer produces unbounded retries within the wait budget.
I have not run the successor's test suite myself, so I cannot independently confirm the "45 passed / 35 passed, exit 0" claim beyond inspecting the diff. The code change itself is present and logically closes the gap this thread raised.
This thread stays open per your instruction ("this reply does not resolve the thread"). Since the successor PR (
#1284) is the actual location of the fix, resolution belongs there, not here.You are interacting with an AI system.