Fix context overrun crash with local LLM backends - #403
Merged
teknium1 merged 1 commit intoMar 5, 2026
Conversation
…#348) Local backends (LM Studio, Ollama, llama.cpp) return HTTP 400 with messages like "Context size has been exceeded" when the context window is full. The error phrase list did not include "context size" or "context window", so these errors fell through to the generic 4xx abort handler instead of triggering compression. Changes: - Move context-length check above generic 4xx handler so it runs first (same pattern as the existing 413 check) - Add "context size" and "context window" to the phrase list - Guard 4xx handler with `not is_context_length_error` to prevent context-related 400s from being treated as non-retryable
Contributor
|
Merged in commit 3220bb8 — your PR was based on an older main where the error handler ordering was different, so it had merge conflicts, but the fix was applied with your changes preserved (added |
Closed
angelburgosrosado
pushed a commit
to angelburgosrosado/hermes-agent
that referenced
this pull request
Apr 27, 2026
…ackends Authored by ch3ronsa. Fixes NousResearch#348. Adds 'context size' (LM Studio) and 'context window' (Ollama) to context-length error detection phrases so local backend 400 errors trigger compression instead of aborting. Also removes 'error code: 400' from the non-retryable error list as defense in depth.
exiao
added a commit
to exiao/hermes-agent
that referenced
this pull request
Jun 29, 2026
Bare 403/429/503/502 substrings in the routing/retry hint lists also matched an unrelated ticket reference like 'merge PR NousResearch#403?', wrongly downgrading a human-decision block to ROUTING/RETRY. The default header must stay DECISION NEEDED. Split the numeric codes into _ROUTING_STATUS_CODES / _RETRY_STATUS_CODES and match via _has_status_code(): strip issue/PR/ticket refs first, then require a standalone \bNNN\b token. 'returned 403' still classifies as routing; 'merge PR NousResearch#403?' stays DECISION NEEDED. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md New test: test_infer_issue_number_is_not_a_status_code
exiao
added a commit
to exiao/hermes-agent
that referenced
this pull request
Jun 29, 2026
…tag) (#56) * fix(kanban): self-labeling blocked-task notifications (kind + header tag) Block pushes in the Kanban Workers Signal group were a wall of identical `⏸ … blocked: <paragraph>` alerts with no way to tell "Eric must decide" from "worker hit a routing wall" from "merge-ready". Make every block typed and self-labeling end to end. - kanban_db.block_task: BLOCK_KIND_HEADERS + ensure_reason_header() normalize the stored reason / run summary / event payload to lead with ERIC DECISION: / ROUTING: / RETRY: for needs_input / capability / transient. dependency (→todo) and None (legacy/dispatcher) untouched (back-compat preserved). - tools/kanban_tools._handle_block: REQUIRE kind on the worker tool (reject an un-typed block instead of silently storing a generic one); redirect merge-ready / awaiting-merge reasons to kanban_complete (constitution 2a). Schema marks kind required + documents the headers. - gateway/kanban_watchers: _format_block_notification() leads the push with 🔴 ERIC DECISION / 🟠 ROUTING / 🟡 RETRY — <id>: <title>, full reason below; legacy untyped block keeps the ⏸ … blocked shape. Tests: new test_kanban_block_notify_labeling.py (drives the real notifier watcher); extended test_kanban_block_kinds.py (header normalization), test_kanban_tools.py (kind required + merge-ready redirect), and fixed two existing callers to pass kind. 145 focused tests green. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-self-labeling.md Worker SOUL half (the behavioral populator) is staged for review, NOT applied: ~/.hermes/plans/hermes-patches/_soul-block-staged.diff (16 lanes). * test(cli): isolate ignore-user-config project fallback from stale repo-root config test_user_config_skipped_when_flag_set failed deterministically in CI slice 8 (passed locally). load_cli_config() falls back to the project config at Path(cli.__file__).parent/cli-config.yaml when HERMES_IGNORE_USER_CONFIG=1 skips the user config. That file is gitignored, but another test in the same slice leaves a real cli-config.yaml at the repo root and run_tests.sh's per-file subprocesses share one filesystem, so the stale file leaks in and its model.default fails the "defaults only" assertion. PR #55's file deletions reshuffled slice membership and landed victim + polluter together. Harden the victim: monkeypatch cli.__file__ into tmp_path in _reload_cli so the project fallback resolves to an empty dir. The test now uses built-in defaults regardless of any stale repo-root cli-config.yaml. Verified all 11 tests pass with a leaked file present. Patch note: test-ignore-user-config-project-fallback-isolation.md * fix(kanban): address block self-labeling review (kind prompts, dependency merge, Kanban prefix) Five findings from gemini + Codex on the typed-block notification change: - P2 (Codex): required `kind` broke live worker prompts. _handle_block now rejects kind-less worker blocks, but agent/prompt_builder.py rule #4 and the two hermes_cli/goals.py kanban goal templates still told workers to call kanban_block(reason=...) with no kind, so a worker following the prompt hit "kind is required" and couldn't terminate. Updated all three to pass kind and enumerate the four kinds. - P2 (Codex): dependency blocks couldn't mention pending merges. The merge-ready reason heuristic ran before kind was considered, so a legitimate kind='dependency' block ("waiting on parent PR pending merge") was rejected instead of routing to todo for auto-resume. Scoped the redirect to kind != "dependency"; also shrinks the false-positive surface gemini flagged. Added test_block_dependency_may_mention_pending_merge. - Medium (gemini): legacy/untyped block notification was missing the "Kanban" prefix that done/gave_up/crashed/timed_out notifications use. Restored "⏸ {tag}Kanban {task_id} blocked{suffix}" and updated the assertion. Kanban + prompt_builder + goals suites green. Patch note: kanban-block-self-labeling-review-fixes.md * fix(kanban): guard merge-ready heuristic against negation (P2 thread) * refactor(kanban): generic 'DECISION NEEDED' block header (decouple from a named user) Rename the needs_input block-kind header from 'ERIC DECISION' to the name-free 'DECISION NEEDED' so the self-labeling notification doesn't hardcode a single user. Pure label rename: kinds, routing, and idempotent header-stamping logic are unchanged. Updates the constant, its doc/comment echoes, and the test assertions/fixtures that pinned the literal. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-self-labeling.md * docs(prompt): add required kind to remaining kanban_block examples (Codex P2) The required-kind guard rejects kanban_block(reason=...) with no kind, but two worker-guidance examples still omitted it: the review-required exception and the headless-clarify path. Following either literally would now return 'kind is required' and leave the task running instead of surfacing the blocker. Both are human-decision cases → kind="needs_input". * fix(kanban): phrase-anchored merge-ready redirect + review-required marker (Codex P2 x3) Three issues with the merge-ready→complete redirect heuristic: - review-required handoffs (the wording the worker prompts mandate) were not in the marker set, so a finished-code block bypassed the redirect and still parked in blocked. Add review-required / review required markers. - the global 'any negation anywhere' check mis-fired both ways: it suppressed a real affirmative redirect on an unrelated 'no blockers remain', and was the only thing stopping a negated 'not ready to merge' real blocker. Replace with a regex that fires only when a negator directly qualifies a merge-ready phrase (not <=2 words> <marker>). Adds fail-before/pass-after tests for all three cases. * fix(kanban): route review-required to kanban_complete in prompt + docs (Codex P2 x2) The merge-ready redirect now treats review-required as a complete-not-block marker, but the worker prompt still told workers to kanban_block(review-required) — a self-contradiction that gets rejected. Point the prompt at kanban_complete instead, and update the public kanban docs (required kind on kanban_block, and the kanban_block lifecycle examples) so docs-following agents don't hit the new kind-required error. * rework(kanban): infer block-notification header from reason, drop required kind Collapse PR #56 to a notification-only feature. The 'kind' machinery is upstream (optional); only this PR had made it required + added headers + a merge-redirect, which is too much agent-facing contract to maintain. Revert every agent-facing change to upstream/live-config shape: kanban_block kind is optional again, no required constraint, no merge-ready redirect, no reason header-stamping, prompt/goals/docs/SOULs untouched. Move the self-labeling header entirely into the gateway notifier: _infer_block_header() classifies the free-text reason (routing / retry / decision-needed, default decision-needed) so the Signal push triages at a glance with zero agent contract. An explicit optional kind, if passed, still overrides inference. Misclassification is cosmetic: full reason rides in the body, default is the act-on-it tag, so the worst case is glancing at one extra alert, never a dropped one. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md * test(kanban): align untruncate block test with self-labeling header The notification-only rework changed the blocked-event push from the legacy '⏸ … blocked' shape to a self-labeling header inferred from the reason text. test_blocked_reason_is_not_clipped_at_160 still asserted the literal 'blocked' substring; a reason with no routing/retry hints now leads with '🔴 DECISION NEEDED'. Assert the header instead — the untruncation intent (full reason survives, tail not clipped) is unchanged. * fix(kanban): scope HTTP-status block hints to error context (Codex P2) Bare 403/429/503/502 substrings in the routing/retry hint lists also matched an unrelated ticket reference like 'merge PR NousResearch#403?', wrongly downgrading a human-decision block to ROUTING/RETRY. The default header must stay DECISION NEEDED. Split the numeric codes into _ROUTING_STATUS_CODES / _RETRY_STATUS_CODES and match via _has_status_code(): strip issue/PR/ticket refs first, then require a standalone \bNNN\b token. 'returned 403' still classifies as routing; 'merge PR NousResearch#403?' stays DECISION NEEDED. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md New test: test_infer_issue_number_is_not_a_status_code * fix(kanban): decision wording wins over access/status hints in block classifier (Codex P2 x4) Tighten _infer_block_header so the at-a-glance header signal isn't mislabeled by incidental substrings: - Human CHOICE/question wording wins first (a '?' or decision phrasing), so 'Should I retry the failed migration or roll back?' and 'I can't reach a decision ... without input' stay 🔴 DECISION NEEDED instead of being downgraded to 🟡 RETRY / 🟠 ROUTING by a bare retry/can't-reach substring. - Status-code matching now requires explicit HTTP/error context, so a non-HTTP id (HERMES-429, ...NousResearch/issues/429) no longer triggers RETRY. - Retry/transient evidence is checked before access→routing, so a reason with both ('no access right now: 429 rate limit') is bucketed 🟡 RETRY. Notification-labeling only; misclassification stays cosmetic (full reason always rides in the body, default is the act-on-it tag). Regression tests added per case (red-before/green-after). * fix(kanban): model block-classifier precedence explicitly (Codex P2 x3) Enumerate the input space and restructure _infer_block_header as an explicit 4-rung ladder instead of an order-dependent substring race: 1. human CHOICE/question wording -> DECISION NEEDED 2. POSITIVE (non-negated) transient evid -> RETRY 3. hard access/lane wall -> ROUTING 4. default -> DECISION NEEDED Three Codex findings, one pass: - Hyphenated HTTP markers (HTTP-429, status-503) were eaten whole by the issue-ref strip before _has_status_code saw the number, so a real transient fell through to DECISION. A negative lookahead exempts http/https/status/ code/error- prefixes so the numeric code survives. - Negated retry (do not retry / don't try again) is an operator routing instruction, not a transient signal. New _NEGATED_RETRY_RE strips negated retry phrases before the retry-hint scan (_has_positive_retry_evidence), so a credential block defers to its hard access evidence -> ROUTING. - Bare "which " dropped from _DECISION_HINTS so ordinary relative clauses ("The deploy API, which returned 429, ... try again later") classify by their retry/status evidence -> RETRY; genuine "which ... ?" questions still hit the trailing-? path -> DECISION. Notification-labeling only; misclassification stays cosmetic (full reason always rides in the body, default is the act-on-it tag). Regression test per finding (red-before/green-after); existing classifier + notify/untruncate/mixin consumers (32 tests) stay green; ruff clean. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md * fix(kanban): complete negation + status-code input space (Codex P2 x2) Round-2 same-class gaps Codex flagged on the new head: - Modal/adjective negations: _NEGATED_RETRY_RE only matched direct "do not retry"/"don't try again". Broadened to modal negations (should/must/can/will/would + n't, plus contractions) and an optional qualifier span (not safe/ok/supposed to retry), plus bare standalone "not". So "should not retry until provisioned" / "not safe to retry until credentials exist" / "must not try again" now defer to access evidence -> ROUTING. - Common status codes: added 401 -> ROUTING (unauthorized = access wall) and 408/504 -> RETRY (request/gateway timeout = transient), so "API returned 401" / "HTTP 504 from gateway" classify without the worker also spelling out unauthorized/timeout. Codes still require HTTP/error context, so a bare "Finished 401 of the rows" stays DECISION. New tests: test_infer_modal_negated_retry_defers_to_access_evidence, test_infer_common_status_codes_bucket_correctly. 23 classifier tests + the notify/untruncate/mixin consumers (34 total) stay green; ruff clean. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md * fix(kanban): single-source HTTP-context words to kill prefix drift (Codex P2 x4) Codex surfaced one new substring edge case per round; root cause was two regexes disagreeing about "what reads as HTTP": _HTTP_CONTEXT_RE accepted api/gateway/server/request while the issue-ref hyphen-exemption only protected http/status/code/error, so api-429/gateway-504 got context from one regex but were stripped by the other. Collapse that: one _HTTP_CONTEXT_WORDS tuple now drives BOTH _HTTP_CONTEXT_RE and the <word>-NNN exemption in _ISSUE_REF_RE. Four round-3 findings, one structural pass: - pull-URL refs (a PR URL ending NousResearch/pull/429): added `pull` to the issue-ref keyword arm so a PR-review handoff is not read as a status code -> DECISION. - Hyphenated api-429/gateway-504 now survive the strip (exemption derives from _HTTP_CONTEXT_WORDS) -> RETRY. - Negated transient class: _NEGATED_RETRY_RE -> _NEGATED_TRANSIENT_RE scrubs negated forms of the WHOLE transient vocabulary (retry/try again/transient/ temporary/flaky/clear), so a "not transient -- missing API key" / "won't clear on its own" reason -> ROUTING. Helper renamed _has_positive_retry_evidence -> _has_positive_transient_evidence. - decide:/decide,/to decide added to _DECISION_HINTS so "Need Eric to decide: retry or revert" -> DECISION. New tests: test_infer_pull_url_number_is_not_a_status_code, test_infer_negated_transient_word_defers_to_access, test_infer_decide_colon_is_a_decision, test_infer_hyphenated_api_gateway_status_marker_is_transient. 27 classifier tests + notify/untruncate/mixin consumers (38 total) stay green; ruff clean. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md * fix(kanban): strip whole URLs + cover determiner-negated transient (Codex P2 x2) Round-4 escapes, each closed at its root: - Numeric URL path ids: a code like an actions-run URL ending /runs/504 (any URL, not just /pull or /issues) was read as a status code because the URL's https scheme satisfied the whole-message HTTP-context gate. New _URL_RE strips whole http(s):// tokens in _has_status_code BEFORE the context check and code scan, so a pure "review this <url>" handoff has no remaining context -> DECISION. Only inline status wording (API returned 504) counts. Generalizes past per-path-keyword arms. - Determiner/adjective negation: "not a retry issue", "not a transient issue", "not retryable" slipped past _NEGATED_TRANSIENT_RE (determiner between negator and token; adjective form). Added an optional (a|an|the|any) determiner span and retryable/recoverable adjective forms, so those access blocks defer to routing -> ROUTING. A POSITIVE "retryable 503" still classifies -> RETRY. New tests: test_infer_numeric_url_path_id_is_not_a_status_code, test_infer_determiner_negated_transient_defers_to_access. 29 classifier tests + notify/untruncate/mixin consumers (40 total) stay green; ruff clean. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md --------- Co-authored-by: testuser <testuser@erics-air.mynetworksettings.com>
waefrebeorn
pushed a commit
to waefrebeorn/slermes
that referenced
this pull request
Jul 2, 2026
…ackends Authored by ch3ronsa. Fixes NousResearch#348. Adds 'context size' (LM Studio) and 'context window' (Ollama) to context-length error detection phrases so local backend 400 errors trigger compression instead of aborting. Also removes 'error code: 400' from the non-retryable error list as defense in depth.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #348
Problem
Local inference backends (LM Studio, Ollama, llama.cpp) return HTTP 400 with error messages like
"Context size has been exceeded"when the context window is full. The context-length error phrase list did not include"context size"or"context window", so these errors fell through to the generic 4xx abort handler — crashing the session instead of triggering compression.Error flow before this fix:
Fix
"context size"(LM Studio),"context window"(Ollama)not is_context_length_errorso context-related 400s are never treated as non-retryableError flow after this fix:
Tested error messages
Test plan