fix(tests): dedupe toolcall regex, fix mocks returning wrong type
there were two copies of the STALE_TOOL_CALL_MARKER_RE, and the reason was given that module A couldn't import B without causing some test failure. the tests that failed have been changed to mock get_hermes_home() correctly, as a Path, not a str, and the module import order has been reversed, so B now imports A to get the regex.
This commit is contained in:
parent
1f8fdc7bd8
commit
e5d9c44778
|
|
@ -58,11 +58,9 @@ from agent.message_sanitization import (
|
|||
_strip_images_from_messages,
|
||||
_strip_non_ascii,
|
||||
)
|
||||
# Must mirror _STALE_TOOL_CALL_MARKER_RE in hermes_state.py — kept local
|
||||
# to avoid importing hermes_state at module load time (its module-level
|
||||
# DEFAULT_DB_PATH = get_hermes_home() / "state.db" breaks tests that
|
||||
# monkeypatch get_hermes_home to return a str).
|
||||
_STALE_MARKER_RE = re.compile(r"^\[[A-Za-z_][A-Za-z0-9_.-]*\]$")
|
||||
# Matches a bare protocol/tool-name marker such as "[memory]" or "[skill_manage]".
|
||||
STALE_TOOL_CALL_MARKER_RE = re.compile(r"^\[[A-Za-z_][A-Za-z0-9_.-]*\]$")
|
||||
|
||||
from agent.model_metadata import (
|
||||
MINIMUM_CONTEXT_LENGTH,
|
||||
_estimate_tools_tokens_rough,
|
||||
|
|
@ -6739,7 +6737,7 @@ def run_conversation(
|
|||
# post-tool fallback replay that token forever after compaction (#78148).
|
||||
if (
|
||||
assistant_message.tool_calls
|
||||
and _STALE_MARKER_RE.fullmatch(turn_content.strip())
|
||||
and STALE_TOOL_CALL_MARKER_RE.fullmatch(turn_content.strip())
|
||||
):
|
||||
logger.warning(
|
||||
"Discarding bare tool-call marker from assistant content: %s",
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ from agent.skill_commands import (
|
|||
SKILL_SCAFFOLD_SQL_LIKE,
|
||||
describe_skill_invocation,
|
||||
)
|
||||
from agent.conversation_loop import STALE_TOOL_CALL_MARKER_RE
|
||||
from hermes_constants import get_hermes_home
|
||||
from hermes_cli.sqlite_runtime import (
|
||||
is_sqlite_wal_reset_vulnerable as _is_sqlite_wal_reset_vulnerable,
|
||||
|
|
@ -663,10 +664,6 @@ def _strip_background_review_harness(
|
|||
return out
|
||||
|
||||
|
||||
# Matches a bare protocol/tool-name marker such as "[memory]" or "[skill_manage]".
|
||||
_STALE_TOOL_CALL_MARKER_RE = re.compile(r"^\[[A-Za-z_][A-Za-z0-9_.-]*\]$")
|
||||
|
||||
|
||||
def _is_stale_tool_call_marker_message(msg: Dict[str, Any]) -> bool:
|
||||
"""True when ``msg`` is a persisted assistant turn whose content is a bare
|
||||
bracketed marker (e.g. ``[memory]``) left over from a tool-call turn.
|
||||
|
|
@ -686,7 +683,7 @@ def _is_stale_tool_call_marker_message(msg: Dict[str, Any]) -> bool:
|
|||
content = msg.get("content")
|
||||
if not isinstance(content, str):
|
||||
return False
|
||||
return bool(_STALE_TOOL_CALL_MARKER_RE.fullmatch(content.strip()))
|
||||
return bool(STALE_TOOL_CALL_MARKER_RE.fullmatch(content.strip()))
|
||||
|
||||
|
||||
def _strip_stale_tool_call_markers(
|
||||
|
|
@ -10651,7 +10648,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
|||
affected: List[int] = []
|
||||
for row in cursor.fetchall():
|
||||
content = row["content"]
|
||||
if isinstance(content, str) and _STALE_TOOL_CALL_MARKER_RE.fullmatch(content.strip()):
|
||||
if isinstance(content, str) and STALE_TOOL_CALL_MARKER_RE.fullmatch(content.strip()):
|
||||
affected.append(row["id"])
|
||||
return affected
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ silently reset mid-turn.
|
|||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
from pathlib import Path
|
||||
|
||||
import importlib
|
||||
|
||||
|
|
@ -23,7 +24,7 @@ def server():
|
|||
"sys.modules",
|
||||
{
|
||||
"hermes_constants": MagicMock(
|
||||
get_hermes_home=MagicMock(return_value="/tmp/hermes_test_compaction")
|
||||
get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test_compaction"))
|
||||
),
|
||||
"hermes_cli.env_loader": MagicMock(),
|
||||
"hermes_cli.banner": MagicMock(),
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import json
|
|||
import sys
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
|
@ -38,7 +39,7 @@ def server():
|
|||
# the whole test would poison modules first imported inside test bodies
|
||||
# (see tests/tui_gateway/test_protocol.py for the full rationale).
|
||||
with patch.dict("sys.modules", {
|
||||
"hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value="/tmp/hermes_test")),
|
||||
"hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test"))),
|
||||
"hermes_cli.env_loader": MagicMock(),
|
||||
"hermes_cli.banner": MagicMock(),
|
||||
"hermes_state": MagicMock(),
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ thinking block tagged with its source model.
|
|||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
from pathlib import Path
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
|
|
@ -22,7 +23,7 @@ def server():
|
|||
"sys.modules",
|
||||
{
|
||||
"hermes_constants": MagicMock(
|
||||
get_hermes_home=MagicMock(return_value="/tmp/hermes_test_moa_emit")
|
||||
get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test_moa_emit"))
|
||||
),
|
||||
"hermes_cli.env_loader": MagicMock(),
|
||||
"hermes_cli.banner": MagicMock(),
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import sys
|
|||
import threading
|
||||
import time
|
||||
import types
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
|
@ -28,7 +29,7 @@ def server():
|
|||
# (a fixed shared path) forever, leaking active-session registry entries
|
||||
# across every later test in the process. Scope the patch to the import.
|
||||
with patch.dict("sys.modules", {
|
||||
"hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value="/tmp/hermes_test")),
|
||||
"hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test"))),
|
||||
"hermes_cli.env_loader": MagicMock(),
|
||||
"hermes_cli.banner": MagicMock(),
|
||||
"hermes_state": MagicMock(),
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ transcript line.
|
|||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
from pathlib import Path
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
|
|
@ -24,7 +25,7 @@ def server():
|
|||
"sys.modules",
|
||||
{
|
||||
"hermes_constants": MagicMock(
|
||||
get_hermes_home=MagicMock(return_value="/tmp/hermes_test_review_summary")
|
||||
get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test_review_summary"))
|
||||
),
|
||||
"hermes_cli.env_loader": MagicMock(),
|
||||
"hermes_cli.banner": MagicMock(),
|
||||
|
|
|
|||
|
|
@ -12,9 +12,6 @@ import pytest
|
|||
def test_slash_worker_accepts_profile_home():
|
||||
"""_SlashWorker.__init__ accepts profile_home parameter."""
|
||||
with patch.dict("sys.modules", {
|
||||
# get_hermes_home() returns a Path in production; hermes_state.py's
|
||||
# module-level DEFAULT_DB_PATH = get_hermes_home() / "state.db" does path
|
||||
# division, so a str mock makes the import raise TypeError (str / str).
|
||||
"hermes_constants": MagicMock(get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test"))),
|
||||
}):
|
||||
with patch("subprocess.Popen") as mock_popen:
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ shows a real midstream turn instead of sitting silent until persistence.
|
|||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
from pathlib import Path
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
|
|
@ -23,7 +24,7 @@ def server():
|
|||
"sys.modules",
|
||||
{
|
||||
"hermes_constants": MagicMock(
|
||||
get_hermes_home=MagicMock(return_value="/tmp/hermes_test_child_mirror")
|
||||
get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test_child_mirror"))
|
||||
),
|
||||
"hermes_cli.env_loader": MagicMock(),
|
||||
"hermes_cli.banner": MagicMock(),
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ the crash class cannot silently regress.
|
|||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
from pathlib import Path
|
||||
|
||||
import subprocess
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
|
@ -42,7 +43,7 @@ def test_slash_worker_popen_uses_utf8_replace():
|
|||
"""
|
||||
with patch.dict("sys.modules", {
|
||||
"hermes_constants": MagicMock(
|
||||
get_hermes_home=MagicMock(return_value="/tmp/hermes_test")
|
||||
get_hermes_home=MagicMock(return_value=Path("/tmp/hermes_test"))
|
||||
),
|
||||
}):
|
||||
with patch("subprocess.Popen") as mock_popen:
|
||||
|
|
|
|||
Loading…
Reference in New Issue