mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-05-29 06:31:32 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 14

refactor(run_agent): review fixes — keyword-forward __init__, drop dead code, tighten guards

Browse source 
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).
This commit is contained in:
teknium1 2026-05-16 22:55:49 -07:00
parent 94c3e0ab8e
commit 47823790b0
No known key found for this signature in database
5 changed files with 98 additions and 40 deletions
Download patch file Download diff file Expand all files Collapse all files

13
agent/codex_runtime.py
View file

@ -16,26 +16,15 @@ compatibility.
from __future__ import annotations
import contextvars
import json
import logging
import os
import threading
import time
import uuid
from types import SimpleNamespace
from typing import Any, Dict, List, Optional
from typing import Any, Dict, List
logger = logging.getLogger(__name__)
def _ra():
"""Lazy ``run_agent`` reference for test-patch routing."""
import run_agent
return run_agent
def run_codex_app_server_turn(
agent,
*,

68
run_agent.py
View file

@ -376,7 +376,73 @@ class AIAgent:
):
"""Forwarder — see ``agent.agent_init.init_agent``."""
from agent.agent_init import init_agent
init_agent(self, base_url, api_key, provider, api_mode, acp_command, acp_args, command, args, model, max_iterations, tool_delay, enabled_toolsets, disabled_toolsets, save_trajectories, verbose_logging, quiet_mode, ephemeral_system_prompt, log_prefix_chars, log_prefix, providers_allowed, providers_ignored, providers_order, provider_sort, provider_require_parameters, provider_data_collection, openrouter_min_coding_score, session_id, tool_progress_callback, tool_start_callback, tool_complete_callback, thinking_callback, reasoning_callback, clarify_callback, step_callback, stream_delta_callback, interim_assistant_callback, tool_gen_callback, status_callback, max_tokens, reasoning_config, service_tier, request_overrides, prefill_messages, platform, user_id, user_name, chat_id, chat_name, chat_type, thread_id, gateway_session_key, skip_context_files, load_soul_identity, skip_memory, session_db, parent_session_id, iteration_budget, fallback_model, credential_pool, checkpoints_enabled, checkpoint_max_snapshots, checkpoint_max_total_size_mb, checkpoint_max_file_size_mb, pass_session_id)
init_agent(
self,
base_url=base_url,
api_key=api_key,
provider=provider,
api_mode=api_mode,
acp_command=acp_command,
acp_args=acp_args,
command=command,
args=args,
model=model,
max_iterations=max_iterations,
tool_delay=tool_delay,
enabled_toolsets=enabled_toolsets,
disabled_toolsets=disabled_toolsets,
save_trajectories=save_trajectories,
verbose_logging=verbose_logging,
quiet_mode=quiet_mode,
ephemeral_system_prompt=ephemeral_system_prompt,
log_prefix_chars=log_prefix_chars,
log_prefix=log_prefix,
providers_allowed=providers_allowed,
providers_ignored=providers_ignored,
providers_order=providers_order,
provider_sort=provider_sort,
provider_require_parameters=provider_require_parameters,
provider_data_collection=provider_data_collection,
openrouter_min_coding_score=openrouter_min_coding_score,
session_id=session_id,
tool_progress_callback=tool_progress_callback,
tool_start_callback=tool_start_callback,
tool_complete_callback=tool_complete_callback,
thinking_callback=thinking_callback,
reasoning_callback=reasoning_callback,
clarify_callback=clarify_callback,
step_callback=step_callback,
stream_delta_callback=stream_delta_callback,
interim_assistant_callback=interim_assistant_callback,
tool_gen_callback=tool_gen_callback,
status_callback=status_callback,
max_tokens=max_tokens,
reasoning_config=reasoning_config,
service_tier=service_tier,
request_overrides=request_overrides,
prefill_messages=prefill_messages,
platform=platform,
user_id=user_id,
user_name=user_name,
chat_id=chat_id,
chat_name=chat_name,
chat_type=chat_type,
thread_id=thread_id,
gateway_session_key=gateway_session_key,
skip_context_files=skip_context_files,
load_soul_identity=load_soul_identity,
skip_memory=skip_memory,
session_db=session_db,
parent_session_id=parent_session_id,
iteration_budget=iteration_budget,
fallback_model=fallback_model,
credential_pool=credential_pool,
checkpoints_enabled=checkpoints_enabled,
checkpoint_max_snapshots=checkpoint_max_snapshots,
checkpoint_max_total_size_mb=checkpoint_max_total_size_mb,
checkpoint_max_file_size_mb=checkpoint_max_file_size_mb,
pass_session_id=pass_session_id,
)
def _get_session_db_for_recall(self):
"""Return a SessionDB for recall, lazily creating it if an entrypoint forgot.

13
tests/run_agent/test_jsondecodeerror_retryable.py
View file

@ -73,17 +73,20 @@ class TestAgentLoopSourceStillHasCarveOut:
revert that happens to leave the test file intact."""
def test_run_agent_excludes_jsondecodeerror_from_local_validation(self):
import run_agent
import inspect
from agent import conversation_loop
# The body moved into agent/conversation_loop.py; scan both for safety.
src = inspect.getsource(run_agent) + inspect.getsource(conversation_loop)
# The agent loop body lives in agent/conversation_loop.py after
# the run_agent.py refactor. Assert the carve-out is present in
# the extracted module specifically — if it ever moves back or
# disappears, this fails loudly rather than silently passing
# against a non-existent inline replica.
src = inspect.getsource(conversation_loop)
# The predicate we care about must reference json.JSONDecodeError
# in its exclusion tuple. We check for the specific co-occurrence
# rather than the literal string so harmless reformatting doesn't
# break us.
assert "is_local_validation_error" in src
assert "JSONDecodeError" in src, (
"run_agent.py must carve out json.JSONDecodeError from the "
"is_local_validation_error classification — see #14782."
"agent/conversation_loop.py must carve out json.JSONDecodeError "
"from the is_local_validation_error classification — see #14782."
)

24
tests/run_agent/test_memory_nudge_counter_hydration.py
View file

@ -121,19 +121,21 @@ def test_production_code_contains_hydration_block():
run_conversation(). If someone deletes it, tests above still pass
against the inline replica — this fails them awake.
The body now lives in agent/conversation_loop.py after the
run_agent.py refactor; check both files for safety.
After the run_agent.py refactor the agent-loop body lives in
``agent/conversation_loop.py`` and uses ``agent.X`` rather than
``self.X``. Assert the block is present in the extracted module
specifically — if it ever drifts back into run_agent.py or
disappears entirely, this guard fails loudly.
"""
from pathlib import Path
repo = Path(__file__).resolve().parents[2]
src_ra = (repo / "run_agent.py").read_text(encoding="utf-8")
src_cl = (repo / "agent" / "conversation_loop.py").read_text(encoding="utf-8")
content = src_ra + src_cl
cl_path = repo / "agent" / "conversation_loop.py"
src_cl = cl_path.read_text(encoding="utf-8")
# Anchor on the unique comment + the modulo line.
assert "Hydrate per-session nudge counters from persisted history" in content
# The line uses ``self.`` in run_agent.py form and ``agent.`` in the
# extracted module, accept either.
assert (
"self._turns_since_memory = prior_user_turns % self._memory_nudge_interval" in content
or "agent._turns_since_memory = prior_user_turns % agent._memory_nudge_interval" in content
assert "Hydrate per-session nudge counters from persisted history" in src_cl, (
f"Hydration comment missing from {cl_path}"
)
assert (
"agent._turns_since_memory = prior_user_turns % agent._memory_nudge_interval"
in src_cl
), f"Hydration modulo assignment missing from {cl_path}"

20
tests/run_agent/test_run_agent.py
View file

@ -5210,12 +5210,13 @@ class TestMemoryNudgeCounterPersistence:
# The preamble resets many fields (retry counts, budget, etc.)
# before the main loop. Find that reset block and verify our
# counters aren't in it. The reset block ends at iteration_budget.
# After the run_agent.py refactor the body uses ``agent.X`` instead
# of ``self.X``, so accept either form.
preamble_end = src.index("iteration_budget = IterationBudget")
# The extracted body uses ``agent.X`` (not ``self.X``). Anchor
# exactly on ``agent.iteration_budget = IterationBudget`` so an
# unrelated identifier ending in ``iteration_budget`` (e.g.
# ``_iteration_budget`` or ``shared_iteration_budget``) can't
# match the boundary.
preamble_end = src.index("agent.iteration_budget = IterationBudget")
preamble = src[:preamble_end]
assert "self._turns_since_memory = 0" not in preamble
assert "self._iters_since_skill = 0" not in preamble
assert "agent._turns_since_memory = 0" not in preamble
assert "agent._iters_since_skill = 0" not in preamble
@ -5316,9 +5317,6 @@ class TestMemoryProviderTurnStart:
import inspect
from agent.conversation_loop import run_conversation as _rc
src = inspect.getsource(_rc)
# After the run_agent.py refactor the body uses ``agent.X`` instead
# of ``self.X``. Accept either spelling.
assert (
"on_turn_start(self._user_turn_count" in src
or "on_turn_start(agent._user_turn_count" in src
)
# The extracted body uses ``agent.X`` rather than ``self.X``;
# assert the extracted-form spelling directly.
assert "on_turn_start(agent._user_turn_count" in src