-
Notifications
You must be signed in to change notification settings - Fork 52.6k
fix(agent): classify provider memory-ceiling 400s as overloaded, not context_overflow #52289
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
Closed
briandevans
wants to merge
12
commits into
NousResearch:main
from
briandevans:fix/agent-memory-ceiling-overloaded-52261
Closed
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
370c4af
fix(agent): classify provider memory-ceiling 400s as overloaded, not …
briandevans 0c8b8a2
test(agent): cover the code-less mid-stream oMLX prefill-memory abort
briandevans f8d7995
test(agent): cover the oMLX 0.5.7 pre-stream prefill-memory 400
briandevans 66babc7
fix(agent): match reworded oMLX prefill-cap wording in the ceiling pa…
briandevans a17465c
test(agent): pin the oMLX pre-stream 400 as captured, not as elided
briandevans a2386d6
test(agent): correct two falsified causal claims in the oMLX docstrings
briandevans dae642e
test(agent): pin the oMLX process-memory-enforcer abort shape
briandevans fde42f1
fix(agent): route memory-ceiling 507s through the memory-ceiling cont…
briandevans 3dbf5e0
fix(agent): recognise prefill_memory_aborted as a memory-ceiling code
briandevans 1efe22b
fix(agent): route memory-ceiling 409s through the memory-ceiling cont…
briandevans 1ecb654
docs(agent): correct the memory-guard wording notes against oMLX 0.5.7
briandevans 2976b5f
test(agent): pin the second captured process-memory abort wording
briandevans 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
Oops, something went wrong.
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.
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.
Current main also has explicit context-overflow branches for 500/502 and 503/529 at
agent/error_classifier.py:1019-1044(added ina04b7024fafter this PR's base). This PR only installs the guard for 400 and no-status paths, so a 5xx memory-ceiling message containingreduce context lengthstill enters compression. Please cover those status branches before claiming this applies at every classification site.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.
Addressed in 620b19bb — the guard is now installed on both 5xx routes you named, ahead of the existing overflow checks rather than after them:
_classify_by_status, the 500/502 branch:if _is_memory_ceiling(error_msg, error_code): return _memory_ceiling_result(result_fn), placed before the overflow-as-500 check for llama.cpp/llama-server.{503, 529}branch: the same guard, ahead of the model-load-OOM overflow check — which was the likeliest 5xx home for a memory abort, since the bareoverloadedfallthrough was only reachable when the body happened to omit context wording.The 400 path (
_classify_400), the structured-code path (_classify_by_error_code) and the no-status message path (_classify_by_message) route through the same_memory_ceiling_resulthelper, so all five paths share one annotation contract instead of five hand-written results.Coverage for the two branches you flagged:
test_5xx_memory_guard_is_overloaded_not_context_overflowandtest_5xx_memory_code_reworded_message_is_overloadedare parametrized across the status codes,test_5xx_genuine_context_overflow_still_compressespins that a real overflow still compresses, andtest_503_generic_overload_unaffected_by_memory_guard/test_500_generic_server_error_unaffected_by_memory_guardare negative controls so the guard cannot swallow an ordinary 5xx.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.
Correction to the SHA above, covering this thread and the
_classify_by_error_codeshould_fallbackthread alongside it:620b19bbno longer exists. The branch was 6,030 commits behind its old base and has been rebased onto currentmain; both answers now live infc34396, and the branch head is8207128.Nothing in either answer changed, and both are re-anchored to symbols rather than to a SHA this time. The 5xx coverage is
_is_memory_ceilingatagent/error_classifier.py:1346in the 500/502 branch and:1376in the{503, 529}branch, each ahead of that branch's overflow check — plus:1581in_classify_400and:1770in_classify_by_message, where currentmain's ordering also required it to sit ahead of_USAGE_LIMIT_PATTERNS,_OVERLOADED_PATTERNS,_BILLING_PATTERNSand_RATE_LIMIT_PATTERNS. Theshould_fallbackpoint is_memory_ceiling_resultat:497: the structured-code route at:1682returns that shared helper rather than building its own result, so all five routes annotate identically. Regression coverage for both is intests/agent/test_error_classifier.pyundertest_5xx_memory_guard_is_overloaded_not_context_overflowandtest_no_status_prefill_memory_code_is_overloaded, which assert on test names that survive the next rebase.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.
Since the reply above, the request in this thread grew by two more status branches. Three commits (
e347abdab78,dd5f376a89c,0330df0a050) carried the guard past the{500, 502}/{503, 529}pair you named, to the remaining routes in_classify_by_status(agent/error_classifier.py:1201) that a local-inference memory wall can actually arrive on::1452—if status_code == 507 and _is_memory_ceiling(error_msg, error_code). oMLX mapsModelTooLargeErrorandInsufficientMemoryErrorto 507 on the model-load path, a different guard from the prefill path and one that never surfaces as a 400 or as either 5xx branch you flagged. It previously fell through to the generic "other 5xx" rule — retryable, butshould_fallbackunset — so once the retries were spent the turn died on a host whose memory ceiling had not moved.:1482—if status_code == 409 and _is_memory_ceiling(error_msg, error_code).ModelLoadingError("load aborted: process memory limit exceeded") maps to 409, which reached the generic "other 4xx" bucket and was reported asformat_error,retryable=False— a transient memory abort called a malformed request.Both are gated on the same
_is_memory_ceilingpredicate rather than on the bare status, so neither branch claims a whole status code:test_507_without_memory_wording_is_generic_server_errorandtest_409_without_memory_wording_is_still_format_errorare the controls for that.This also moves the
should_fallbackthread sitting alongside this one, because both new routes return the shared_memory_ceiling_result(:537) instead of building their own result. On 507 that flag is the regression; on 409 it was already set and what changed isretryable. The structured-code route at:1775gained a second code in the same series —prefill_memory_abortedjoined_MEMORY_CEILING_ERROR_CODES(:514), the sibling that oMLX's prefill-memory body builder selects by exception type, so "admitted then killed mid-prefill" now annotates identically to "turned away at admission" instead of diverging once the message is reworded.The evidence grades behind those three are deliberately not equal, and the code says so where each one lands: the 507 shape rests on a captured response body, whereas the 409 route and the
prefill_memory_abortedpairing are read from the engine's source with no reporter body and are markedNOT CAPTUREDin the fixtures that construct them.The line anchors in my reply above have all moved — the branch was rebased again — so, corrected against the current head:
_memory_ceiling_resultis:537(not:497), the 500/502 guard:1386(not:1346), the{503, 529}guard:1416(not:1376),_classify_400:1659(not:1581), the structured-code route:1775(not:1682),_classify_by_message:1862(not:1770).fc34396and8207128cited here, andc2c430d890dcited on theshould_fallbackthread, were rewritten by that rebase and resolve to nothing; head isf17f3af7b7bas of 2026-08-15T07:29Z.The names are the part that survives all of this.
test_5xx_memory_guard_is_overloaded_not_context_overflow(tests/agent/test_error_classifier.py:939) andtest_no_status_prefill_memory_code_is_overloaded(:835) still pin the two branches you asked for, andtest_507_omlx_model_load_ceiling_is_overloaded_with_fallback,test_409_memory_abort_is_overloaded_not_format_errorandtest_prefill_memory_aborted_code_is_overloadedpin the three additions.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.
Correction to the SHA above: the current head is
2976b5fbfdd160675ef44b65d8337cf75e769315;f17f3af7b7bwas rewritten and is a non-ancestor of that head.The status branches requested in this thread remain routed through the shared
_memory_ceiling_resultrecovery contract. The durable regression anchors aretest_5xx_memory_guard_is_overloaded_not_context_overflow(tests/agent/test_error_classifier.py:939) for 500/502/503/529 andtest_507_omlx_model_load_ceiling_is_overloaded_with_fallback(tests/agent/test_error_classifier.py:1029) for the 507 model-load path.