mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-13 21:27:41 +02:00
[arnav] remove dead code identified by audit
Each removal verified by checking actual production callers (frontend,
electron, internal HTTP, MCP-server subprocesses) — not just test
references. Symbols whose only callers were tests are removed along
with those tests.
Production removals (~390 LOC):
- backend/main.py
- websocket_session: drop `agent:edit_message` WS branch. Frontend
only ever uses HTTP `POST /api/agents/sessions/{id}/edit_message`
(frontend/src/shared/state/agentsSlice.ts); nothing on the wire
sends a WS message of this type.
- backend/apps/agents/agent_manager.py
- AgentManager._build_connected_tools_context (~80 LOC): zero call
sites in production; the connected-tools system-prompt context is
built inline in _compose_system_prompt now.
- AgentManager._approx_tokens / _summarize_message_block: pure
helpers whose only callers were tests. The compaction path uses
LLM-driven _maybe_compact instead.
- backend/apps/agents/browser_agent.py
- clear_browser_history: only used by tests. _browser_history is
pruned via the size cap inline.
- MODEL_MAP constant: never read.
- backend/apps/agents/mcp_preflight.py
- DISCOVERY_SCAFFOLDING (~25-line system-prompt block): defined but
never appended anywhere. The header comment described an intended
use that the codebase no longer has.
- backend/apps/agents/providers/registry.py
- thinking_params_for, _is_9router_available, OPENROUTER_BASE_URL,
get_context_window: zero callers in production. Thinking-params
routing is done by the provider classes directly; 9Router presence
is detected at request time; context-window numbers are stamped
onto sessions from BUILTIN_MODELS at launch.
- backend/apps/agents/tools/{base,web}.py
- BaseTool.get_schema (abstract) + WebSearchTool/WebFetchTool
overrides: production code in backend/apps/web/web.py instantiates
these tools and only calls .execute(); the JSON-schema lives in
the HTTP wrapper, not on the tool class.
- backend/apps/outputs/outputs.py
- _resolve_model + MODEL_MAP: tests-only.
- load_output: docstring claimed it was a public helper for "other
modules" but no module imported it.
- backend/apps/service/client.py
- set_user_id, the _user_id module global, and the dead cache short-
circuit in _get_user_id: setter was tests-only. _get_user_id now
reads user_email directly from settings on every call.
- backend/apps/settings/credentials.py
- get_provider_credentials: zero callers. The sibling get_agent_sdk_env
is kept (it has the explicit "Legacy helpers" keep-comment).
Test updates:
- test_agent_manager_unit.py: drop _approx_tokens / _summarize_message_block
cases (5 tests), update module docstring index.
- test_browser_agent_unit.py: drop clear_browser_history cases (2 tests)
and the unused _Boom helper class in the repr-fallback test.
- test_outputs_unit.py: drop _resolve_model / load_output cases
(4 tests), update docstring + import list.
- test_v2_invariants.py: drop get_context_window tests + get_schema
assertions on web tools (kept name + BaseTool inheritance checks).
- test_service.py: rewrite the 4 set_user_id-driven tests to drive
user_id through settings.user_email instead, so _get_user_id's live
envelope-stamping path stays covered.
Verification:
- ruff --select F401,F811,F841 backend/ → clean.
- pytest backend/tests/ → 1167 passed, 1 deselected (pre-existing
sandbox git test, unrelated). No tests dropped silently — every
deletion is paired with the corresponding test removal/rewrite.
- Dead-code scan re-run: dead WS events 1→0, Tier-2 high-confidence
14→11 (residue is SDK-callback `context` params + Pydantic `cls`
validators — both false positives vulture can't see through),
vulture total 165→145.
Total diff: -565 / +34 LOC across 15 files.
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Arnav Naval
@@ -12,8 +12,7 @@ Test groups:
|
||||
- pure instance methods (`_resolve_mode`, `_compose_system_prompt`,
|
||||
`_resolve_context_paths`, `_build_dir_tree`, `_resolve_forced_tools`,
|
||||
`_resolve_attached_skills`, `_get_branch_messages`,
|
||||
`_build_history_prefix`, `_approx_tokens`,
|
||||
`_summarize_message_block`, `_truncate_large_tool_result`,
|
||||
`_build_history_prefix`, `_truncate_large_tool_result`,
|
||||
`_build_search_text`, `_maybe_compact`)
|
||||
- lifecycle (launch/update/edit/switch_branch/duplicate/close/
|
||||
delete/resume/stop, history, browser-agent children, approval,
|
||||
@@ -564,7 +563,7 @@ def test_get_branch_messages_walks_fork_lineage():
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _build_history_prefix / _approx_tokens / _summarize_message_block
|
||||
# _build_history_prefix
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -587,59 +586,6 @@ def test_build_history_prefix_empty_returns_empty():
|
||||
assert AgentManager._build_history_prefix([]) == ""
|
||||
|
||||
|
||||
@pytest.mark.parametrize("text,expected", [
|
||||
("", 1),
|
||||
("a" * 4, 1),
|
||||
("a" * 16, 4),
|
||||
("a" * 100, 25),
|
||||
])
|
||||
def test_approx_tokens_chars_over_four(text, expected):
|
||||
assert AgentManager._approx_tokens(text) == expected
|
||||
|
||||
|
||||
def test_approx_tokens_handles_none():
|
||||
assert AgentManager._approx_tokens(None) == 1
|
||||
|
||||
|
||||
def test_summarize_message_block_empty_returns_empty():
|
||||
assert AgentManager._summarize_message_block([]) == ""
|
||||
|
||||
|
||||
def test_summarize_message_block_extracts_initial_task_and_counts():
|
||||
msgs = [
|
||||
Message(role="user", content="please do the thing"),
|
||||
Message(role="tool_call", content={"tool": "Read", "input": {}}),
|
||||
Message(role="tool_call", content={"tool": "Bash", "input": {}}),
|
||||
Message(role="tool_call", content={"tool": "Read", "input": {}}),
|
||||
Message(role="tool_result", content="ok"),
|
||||
Message(role="assistant", content="done"),
|
||||
]
|
||||
out = AgentManager._summarize_message_block(msgs)
|
||||
assert "<compacted_history>" in out
|
||||
assert "please do the thing" in out
|
||||
# Tool counts
|
||||
assert "Read×2" in out
|
||||
assert "Bash×1" in out
|
||||
assert "Tool calls so far (3 total)" in out
|
||||
assert "Tool results received: 1" in out
|
||||
assert "Last assistant message:" in out
|
||||
assert "done" in out
|
||||
|
||||
|
||||
def test_summarize_message_block_assistant_list_content():
|
||||
"""Assistant messages with list content (Anthropic block shape)
|
||||
should still surface their text."""
|
||||
msgs = [
|
||||
Message(role="user", content="task"),
|
||||
Message(role="assistant", content=[
|
||||
{"type": "text", "text": "the answer"},
|
||||
{"type": "tool_use", "id": "t1"},
|
||||
]),
|
||||
]
|
||||
out = AgentManager._summarize_message_block(msgs)
|
||||
assert "the answer" in out
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _truncate_large_tool_result
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -11,8 +11,7 @@ in small helpers that can be exercised in isolation:
|
||||
conversations.
|
||||
- `_format_tool_result` — translate `ws_manager` result dicts into
|
||||
Anthropic content blocks.
|
||||
- `clear_browser_history` / `execute_browser_tool` — small async + state
|
||||
helpers.
|
||||
- `execute_browser_tool` — small async + state helper.
|
||||
|
||||
The integration-level tests for `run_browser_agent` /
|
||||
`run_browser_agents` / `_create_browser_card` live in the sister file
|
||||
@@ -40,7 +39,6 @@ from backend.apps.agents.browser_agent import (
|
||||
_summarize_messages,
|
||||
_trim_history_by_turns,
|
||||
_validate_message_pairing,
|
||||
clear_browser_history,
|
||||
execute_browser_tool,
|
||||
)
|
||||
|
||||
@@ -59,28 +57,6 @@ def _clear_browser_history_module_state():
|
||||
ba._browser_history.clear()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# clear_browser_history
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_clear_browser_history_drops_only_target_entry():
|
||||
ba._browser_history["b-1"] = [{"role": "user", "content": "hi"}]
|
||||
ba._browser_history["b-2"] = [{"role": "user", "content": "yo"}]
|
||||
|
||||
clear_browser_history("b-1")
|
||||
|
||||
assert "b-1" not in ba._browser_history
|
||||
assert "b-2" in ba._browser_history
|
||||
|
||||
|
||||
def test_clear_browser_history_missing_id_is_noop():
|
||||
"""Calling clear on an unknown id must not raise (it's used in
|
||||
cleanup paths where the cache may already be empty)."""
|
||||
clear_browser_history("never-existed")
|
||||
assert ba._browser_history == {}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _hash_tool_call
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -4,12 +4,12 @@ These exercise pure logic and pydantic models without booting FastAPI.
|
||||
The integration surface (routes) lives in `test_api_outputs.py`.
|
||||
|
||||
Covers:
|
||||
- outputs.py helpers: _resolve_model, _validate_against_schema,
|
||||
- outputs.py helpers: _validate_against_schema,
|
||||
_build_data_injection, _inject_data_into_html,
|
||||
_inject_token_into_relative_urls (every branch in
|
||||
_ABSOLUTE_URL_PREFIXES + token-already-present + fragment),
|
||||
_decode_data_param, _walk_directory.
|
||||
- On-disk store helpers: _save / _load / load_output / _load_all.
|
||||
- On-disk store helpers: _save / _load / _load_all.
|
||||
- Models: legacy `frontend_code` / `backend_code` / `schema_json`
|
||||
migration into `files`, plus the property accessors.
|
||||
- executor.execute_backend_code: happy path, stdout capture,
|
||||
@@ -35,12 +35,9 @@ from backend.apps.outputs.outputs import (
|
||||
_inject_token_into_relative_urls,
|
||||
_load,
|
||||
_load_all,
|
||||
_resolve_model,
|
||||
_save,
|
||||
_validate_against_schema,
|
||||
_walk_directory,
|
||||
load_output,
|
||||
MODEL_MAP,
|
||||
)
|
||||
from backend.apps.outputs.models import (
|
||||
AutoRunConfig,
|
||||
@@ -55,22 +52,6 @@ from backend.apps.outputs.executor import (
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _resolve_model
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_resolve_model_known_short_name():
|
||||
assert _resolve_model("sonnet") == MODEL_MAP["sonnet"]
|
||||
assert _resolve_model("opus") == MODEL_MAP["opus"]
|
||||
assert _resolve_model("haiku") == MODEL_MAP["haiku"]
|
||||
|
||||
|
||||
def test_resolve_model_unknown_passthrough():
|
||||
assert _resolve_model("claude-3-5-haiku") == "claude-3-5-haiku"
|
||||
assert _resolve_model("") == ""
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _validate_against_schema
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -272,7 +253,7 @@ def test_walk_directory_skips_unreadable(tmp_path):
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _load_all / _save / _load / load_output
|
||||
# _load_all / _save / _load
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -291,18 +272,6 @@ def test_load_missing_raises_404(tmp_data_dirs):
|
||||
assert exc.value.status_code == 404
|
||||
|
||||
|
||||
def test_load_output_returns_none_for_missing(tmp_data_dirs):
|
||||
assert load_output("does-not-exist") is None
|
||||
|
||||
|
||||
def test_load_output_returns_resolved(tmp_data_dirs):
|
||||
out = Output(name="x")
|
||||
_save(out)
|
||||
fetched = load_output(out.id)
|
||||
assert fetched is not None
|
||||
assert fetched.name == "x"
|
||||
|
||||
|
||||
def test_load_all_picks_up_saved(tmp_data_dirs):
|
||||
a = Output(name="a")
|
||||
b = Output(name="b")
|
||||
|
||||
@@ -45,7 +45,6 @@ def patch_settings(tmp_path):
|
||||
def fresh_client(tmp_path):
|
||||
import backend.apps.service.client as client
|
||||
client._install_id = None
|
||||
client._user_id = None
|
||||
client._test_sink = None
|
||||
spool = tmp_path / "spool.db"
|
||||
with patch.object(client, "_spool_path", lambda: str(spool)):
|
||||
@@ -78,35 +77,42 @@ def test_sync_carries_install_id(sink):
|
||||
assert body["client_state"]["install_id"] == "test-install-abc"
|
||||
|
||||
|
||||
def test_sync_carries_user_id_when_set(sink):
|
||||
from backend.apps.service.client import sync, set_user_id
|
||||
set_user_id("alice@example.com")
|
||||
sync({})
|
||||
def test_sync_carries_user_id_when_set_in_settings(tmp_path, sink):
|
||||
"""User id is read from settings.user_email at envelope-build time."""
|
||||
sf = tmp_path / "settings_with_email.json"
|
||||
sf.write_text(json.dumps({
|
||||
"installation_id": "test-install-abc",
|
||||
"analytics_opt_in": True,
|
||||
"user_email": "alice@example.com",
|
||||
}))
|
||||
import backend.apps.settings.settings as settings_mod
|
||||
with patch.object(settings_mod, "SETTINGS_FILE", str(sf)):
|
||||
from backend.apps.service.client import sync
|
||||
sync({})
|
||||
_, body = sink[0]
|
||||
assert body["client_state"]["user_id"] == "alice@example.com"
|
||||
|
||||
|
||||
def test_sync_no_user_id_when_not_set(sink):
|
||||
def test_sync_no_user_id_when_email_not_set(sink):
|
||||
"""No user_email in settings → user_id absent from envelope."""
|
||||
from backend.apps.service.client import sync
|
||||
sync({})
|
||||
_, body = sink[0]
|
||||
assert "user_id" not in body["client_state"]
|
||||
|
||||
|
||||
def test_sync_user_id_cleared_with_none(sink):
|
||||
from backend.apps.service.client import sync, set_user_id
|
||||
set_user_id("alice")
|
||||
set_user_id(None)
|
||||
sync({})
|
||||
_, body = sink[0]
|
||||
assert "user_id" not in body["client_state"]
|
||||
|
||||
|
||||
def test_sync_user_id_cleared_with_empty(sink):
|
||||
from backend.apps.service.client import sync, set_user_id
|
||||
set_user_id("alice")
|
||||
set_user_id("")
|
||||
sync({})
|
||||
def test_sync_no_user_id_when_email_empty(tmp_path, sink):
|
||||
"""Empty-string user_email is treated like missing."""
|
||||
sf = tmp_path / "settings_empty_email.json"
|
||||
sf.write_text(json.dumps({
|
||||
"installation_id": "test-install-abc",
|
||||
"analytics_opt_in": True,
|
||||
"user_email": "",
|
||||
}))
|
||||
import backend.apps.settings.settings as settings_mod
|
||||
with patch.object(settings_mod, "SETTINGS_FILE", str(sf)):
|
||||
from backend.apps.service.client import sync
|
||||
sync({})
|
||||
_, body = sink[0]
|
||||
assert "user_id" not in body["client_state"]
|
||||
|
||||
|
||||
@@ -926,23 +926,6 @@ def test_find_builtin_model_returns_dict_for_known():
|
||||
assert sonnet.get("api") == "anthropic"
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# Group R — context window
|
||||
# ===========================================================================
|
||||
|
||||
|
||||
def test_get_context_window_known_model():
|
||||
from backend.apps.agents.providers.registry import get_context_window
|
||||
cw = get_context_window("Anthropic", "sonnet")
|
||||
assert cw >= 200_000
|
||||
|
||||
|
||||
def test_get_context_window_unknown_returns_default():
|
||||
from backend.apps.agents.providers.registry import get_context_window
|
||||
cw = get_context_window("Unknown", "fake-model")
|
||||
assert cw == 128_000
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# Group S — calculate_cost regression tests
|
||||
# ===========================================================================
|
||||
@@ -1051,18 +1034,16 @@ def test_web_tools_classes_inherit_basetool():
|
||||
assert issubclass(WebFetchTool, BaseTool)
|
||||
|
||||
|
||||
def test_web_search_tool_has_name_and_schema():
|
||||
def test_web_search_tool_has_name():
|
||||
from backend.apps.agents.tools.web import WebSearchTool
|
||||
tool = WebSearchTool()
|
||||
assert tool.name
|
||||
assert isinstance(tool.get_schema(), dict)
|
||||
|
||||
|
||||
def test_web_fetch_tool_has_name_and_schema():
|
||||
def test_web_fetch_tool_has_name():
|
||||
from backend.apps.agents.tools.web import WebFetchTool
|
||||
tool = WebFetchTool()
|
||||
assert tool.name
|
||||
assert isinstance(tool.get_schema(), dict)
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
|
||||
Reference in New Issue
Block a user