de8bdf529d6478ae54d1677b5e59bffe639ca8e2
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Brixyy
|
dc9d677d59 |
fix(agent): classify TypeError('NoneType ... not iterable') as retryable provider shape error
Salvages the intent of #33136 (@Brixyy) onto current main. The original PR was written against the pre-refactor monolithic run_agent.py and added a top-level _is_nonretryable_local_validation_error() helper. Both target functions have since been extracted to agent/conversation_loop.py:2869, so the salvage applies the equivalent guard inline at that canonical location rather than reintroducing the helper. ## Why After #33042 made our own Codex consumer structurally immune to NoneType crashes, third-party shims, mocked clients, and any future code path that hasn't migrated could still surface TypeError: 'NoneType' object is not iterable as a wire-shape mismatch. The agent loop's classifier currently treats ALL TypeError as a local programming bug and aborts non-retryable — users on stale Telegram/gateway turns saw bare "Non-retryable error (HTTP None)" with no recovery. This is a provider/SDK shape mismatch, not a local programming bug. The retry/fallback path should run, not be short-circuited. ## What agent/conversation_loop.py: extend is_local_validation_error to exclude TypeErrors whose message matches the NoneType-not-iterable shape (case- insensitive, both "NoneType" and "not iterable" must appear). tests/run_agent/test_jsondecodeerror_retryable.py: - update the mirror predicate to match the production check - add TestNoneTypeNotIterableIsRetryable class with 3 tests (the basic shape, message variants, unrelated TypeErrors still abort) - add TestAgentLoopSourceHasNoneTypeCarveOut to enforce the source-level invariant matches the test mirror ## Validation tests/run_agent/test_jsondecodeerror_retryable.py + tests/run_agent/test_31273_402_not_retried.py → 14/14 passing Co-authored-by: Brixyy <subrtt@gmail.com> |
||
|
teknium1
|
47823790b0 |
refactor(run_agent): review fixes — keyword-forward __init__, drop dead code, tighten guards
Four fixes from PR #27248 review: 1. **__init__ forwarder is now keyword-forwarded** (daimon-nous review). Previously the run_agent.AIAgent.__init__ wrapper forwarded all 64 params positionally to agent.agent_init.init_agent, so adding a 65th param on main would require three lockstep edits (signature, init_agent signature, forwarder call) or silently shift every value. Keyword forwarding makes this trivially safe — adding a param now only needs the two signatures and one extra keyword line. 2. **Drop dead _ra() in agent/codex_runtime.py** (daimon-nous + Copilot). The lazy run_agent reference was defined but never called inside this module — the codex paths use agent.* accessors only. 3. **Drop unused imports in agent/codex_runtime.py** (Copilot): contextvars, threading, time, uuid, Optional. Carried over from run_agent.py during the original extraction. 4. **Tighten three source-introspection test guards** (Copilot): - test_memory_nudge_counter_hydration.py — was scanning the concatenated source of run_agent.py + agent/conversation_loop.py and matching self.X or agent.X form. Now asserts the hydration block lives in agent/conversation_loop.py specifically with the agent.X form — the body never moves back, so if it ever drifts a future re-introduction fails the guard. - test_run_agent.py::TestMemoryNudgeCounterPersistence — anchor on agent.iteration_budget = IterationBudget exactly (was just iteration_budget = IterationBudget) so an unrelated identifier ending in iteration_budget can't match. - test_run_agent.py::TestMemoryProviderTurnStart — assert the agent._user_turn_count form directly (the extracted body uses agent.X, not self.X — accepting either was a transitional fudge). - test_jsondecodeerror_retryable.py — scan agent/conversation_loop.py only, not the concatenation. Not addressed in this commit: * Pre-existing bugs in agent/tool_executor.py (heartbeat index mismatch when calls are blocked, _current_tool clobber in result loop, blocked-counted-as-completed in spinner summary, dead result_preview computation). These were preserved byte-for-byte from the original _execute_tool_calls_concurrent — worth a separate follow-up PR with proper tests. * _OpenAIProxy.__instancecheck__ concern — pre-existing, not flagged by any of the original test patches (nothing actually does isinstance(x, OpenAI) against the proxy instance). * agent_init.py:949 mem_config potential NameError — pre-existing; only triggers if _agent_cfg.get('memory', {}) itself raises, which it can't with a stock dict. tests/run_agent/ + tests/agent/: 4313 passed, 1 pre-existing test_auxiliary_client failure (unchanged). run_agent.py: 3821 -> 3937 lines (+116 from the keyword-forwarded init call's verbosity). Final: 16083 -> 3937 (-12146, 75% reduction). |
||
|
teknium1
|
0530252384 |
refactor(run_agent): extract run_conversation to agent/conversation_loop.py
The 3,877-line run_conversation body — the agent loop itself — moves out of run_agent.py into a dedicated module. AIAgent.run_conversation is now a thin forwarder that delegates to agent.conversation_loop.run_conversation with the AIAgent instance as the first argument. This is the largest single extraction in the run_agent.py refactor. The body keeps all 163 self.X references intact (rewritten as agent.X), all nested closures, all retry/backoff/compression machinery. Symbols that tests or callers patch on run_agent (_set_interrupt, handle_function_call, AIAgent class attrs) are resolved through _ra() inside the extracted module so the patch surface is preserved. Five tests doing inspect.getsource(AIAgent.run_conversation) updated to scan agent.conversation_loop.run_conversation. Two source-introspection tests (TestMemoryNudgeCounterPersistence, TestMemoryProviderTurnStart) updated to accept either self.X (legacy) or agent.X (extracted form) in the matched assertions. Live E2E verified on three model paths: * openai/gpt-5.4 (OpenAI chat completions via OpenRouter) * anthropic/claude-sonnet-4.6 (Anthropic Messages via OpenRouter) * moonshotai/kimi-k2-thinking (reasoning model, reasoning_content path) Plus read_file tool execution, terminal tool, web_search. tests/run_agent/ + tests/agent/: 4313 passed, 1 pre-existing failure (test_auxiliary_client::test_custom_endpoint... — same as on main). run_agent.py: 9800 -> 5944 lines (-3856). Total reduction since baseline: 16083 -> 5944 (-10139, 63%). |
||
|
Teknium
|
c2b3db48f5 |
fix(agent): retry on json.JSONDecodeError instead of treating it as a local validation error (#15107)
json.JSONDecodeError inherits from ValueError. The agent loop's
non-retryable classifier at run_agent.py ~L10782 treated any
ValueError/TypeError as a local programming bug and short-circuited
retry. Without a carve-out, a transient JSONDecodeError from a
provider that returned a malformed response body, a truncated stream,
or a router-layer corruption would fail the turn immediately.
Add JSONDecodeError to the existing UnicodeEncodeError exclusion
tuple so the classified-retry logic (which already handles 429/529/
context-overflow/etc.) gets to run on bad-JSON errors.
Tests (tests/run_agent/test_jsondecodeerror_retryable.py):
- JSONDecodeError: NOT local validation
- UnicodeEncodeError: NOT local validation (existing carve-out)
- bare ValueError: IS local validation (programming bug)
- bare TypeError: IS local validation (programming bug)
- source-level assertion that run_agent.py still carries the carve-out
(guards against accidental revert)
Closes #14782
|