DAO Proposals & Community
View active proposals, submit new ideas, and connect with the SWARMS community.
Addresses item 5 in #1853. ## What `GraphWorkflow.visualize` sanitized the workflow name in a branch that can never run, and used the raw name in the branch that always does. `output_path` is assigned unconditionally at the top of the method: ```python output_path = f"{self.name}_visualization_{str(uuid.uuid4())}" ``` so the `if output_path is None:` guard ~190 lines later — the only place `safe_name` was ever built — was unreachable. graphviz treats its argument as a filesystem path, so a workflow named `team/alpha` rendered to `team/alpha_visualization_<uuid>`, a directory that does not exist, and the call failed. The sanitization written to prevent exactly that was sitting in dead code. ``` before: team/alpha_visualization_<uuid> -> renders into a missing directory after: team_alpha_visualization_<uuid> -> renders ``` ## Fix Moved the sanitization into the live assignment and deleted the unreachable branch (6 lines). Net effect is the behaviour the dead code intended, in the path that runs. The `uuid4()` suffix is kept — the dead branch omitted it, so had it ever run it would also have collided between two visualizations of the same workflow. ## Test Appended to `tests/structs/test_graph_workflow.py` — no new file. It asserts the dead branch is gone, that `safe_name` is built in the live path, and that a name containing `/` maps to `team_alpha`. ``` master source + this test: 1 failed this branch: 1 passed tests/structs/test_graph_workflow.py: 48 passed, 11 skipped black --check --line-length 70, ruff: clean ``` ## Not included The other items in #1853 are deliberately out of scope here, and two of them are already covered: - item 2 (redundant exception tuple in `agent.py`) is part of #1773 - item 4 (`_reinitialize_after_load` storing a shut-down executor) is part of #1801 - item 1 is #1857 - item 6 touches `aop.py`, which #1861 is already changing — better done after that lands than as a conflicting parallel edit That leaves item 3 (the bogus `await` on a sync `_handle_run_error`) genuinely unclaimed; happy to take it separately. The red checks are the repo-wide ones (`build` never installs the package, `test-main-features` dies on Poetry 2.x `--no-dev`), both addressed in #1812.
Proposed by ayaangazaliFixes #1788. ## What `run_agents_concurrently` collected its list-mode results with `as_completed`, which yields futures in **finish** order. All three callers pair that list positionally with the input agent list, so whenever agents did not finish in submission order — the normal case, since LLM latency tracks output length — every answer was filed under a different agent's name. Reproduced with three agents whose only difference is how long they take, first slowest: ``` master: ['third', 'second', 'first'] fixed: ['first', 'second', 'third'] ``` Exactly reversed, and silent: no exception, no warning, and the output is still well-formed. `MajorityVoting`'s consensus agent and `ma_blocks.aggregate`'s aggregator then reason over a transcript whose attributions are shuffled, and `AgentRearrange._run_concurrent_workflow` hands the scrambled mapping back to the caller as `response_dict`. Failures were misattributed the same way — an agent that raises had its `Exception` appended in completion order too, so a crash landed on a healthy agent's slot. ## Fix One place, not three. The `return_agent_output_dict` branch immediately above already iterated `zip(agents, futures)` in input order; the list branch now does the same: ```python results = [] for future in futures: try: results.append(future.result()) except Exception as e: results.append(e) ``` `future.result()` blocks, but every task was submitted before this loop starts, so the wall clock is unchanged — this waits exactly as long as `as_completed` did, for the slowest agent. Fixing the shared function rather than the three call sites keeps the diff smaller and, more importantly, means any *future* caller is correct by default. The batched-grid executor further down the same file already got this right by writing into `results[idx]`, so the two paths now agree. Also removed `agent_id_map`: two lines that built a `future -> agent` mapping which was never read. It is exactly the mapping that would have prevented this bug, left dead. The docstring promised completion order in three places; those now describe input order. ## Tests Appended to `tests/structs/test_majority_voting.py`, which owns the most affected caller — no new file. - `test_run_agents_concurrently_preserves_input_order` — agents finish in reverse of input order - `test_run_agents_concurrently_attributes_failures_to_the_right_agent` — the raising agent is also the slowest, so in completion order its error sorted last Both fail on `master` and pass here: ``` master source + these tests: 2 failed this branch: 2 passed black --check --line-length 70 clean ruff check clean ``` The red checks are the repo-wide ones (`build` never installs the package, `test-main-features` dies on Poetry 2.x `--no-dev`), both addressed in #1812.
Proposed by ayaangazali## What Two bugs in `aop.py` that make AOP block or spin forever. Both are reachable from normal use, and together they are why `tests/structs/test_aop.py` never finishes — the suite currently hangs until CI kills the runner. ## 1. Idle queue workers hold the lock while they wait `TaskQueue._worker_loop` waited on its stop event **inside** `with self._lock`: ```python with self._lock: if self._status != QueueStatus.RUNNING or not self._queue: self._stop_event.wait(0.1) # <- 100ms of sleep, holding the lock continue ``` With `max_workers` idle workers, the lock is held essentially all the time by whichever worker is sleeping in it, and they hand it off among themselves. Any outside caller that needs the same lock — `get_stats()`, and `AOP.get_server_info()` through it — is starved. Python locks are not fair, so there is no bound on the wait. Measured on an otherwise idle machine, 4 workers, 20 `get_stats()` calls: ``` before: worst get_stats() latency 20665.0 ms after: worst get_stats() latency 0.0 ms ``` Fix: compute the idle condition under the lock, wait outside it. ## 2. A server that exits immediately restarts forever `AOP.run()` reset the restart counter on every clean return from `start_server()`: ```python self.start_server() self._restart_count = 0 ``` `start_server()` returning means the server stopped. Resetting unconditionally pinned the counter at 0, so `max_restart_attempts` could never fire, and because the backoff is guarded by `if self._restart_count > 0`, no delay was applied either. A server that dies on startup respawns in a hot loop. With `max_restart_attempts=3` and a `start_server` that returns at once: ``` before: 1,665,075 calls in 5s, run() never returns, _restart_count == 0 after: 4 calls (1 start + 3 restarts), run() returns, failsafe logs "Server failed permanently due to exceeding maximum restart attempts" ``` Fix: only treat a clean return as success if the server actually stayed up for `restart_delay`; otherwise count it as a restart. This preserves the behaviour #1806 added — a healthy run clears the failure streak — which `test_persistence_restart_count_bounds_a_failing_server` covers and which still passes. ## Effect on the test suite `tests/structs/test_aop.py` hung at `test_get_server_info` (bug 1) and, once past it, at `test_run_with_persistence_success` (bug 2). Because it never terminated, the 26 failures behind it were invisible. | | master (`f894187d`) | this branch | |---|---|---| | outcome | **hangs, SIGKILL** | completes in 39s | | passed | 47 | 65 | | failed | 26 | 12 | Baseline measured by deselecting the two hanging tests, since master cannot finish otherwise. **No test that passed on master fails here** — verified by diffing the failure sets. ## Two test changes that are not mine to hide **`test_run_with_persistence_success`** asserted `run()` returns after one successful start. That is not what persistence does — its whole job is to restart a stopped server. The mock returned instantly and left the loop condition true, which is an infinite restart, not a success. It now ends the loop the way a served-then-shut-down server does. **The `real_agent` fixture was named `Test-Agent` while 15 tests refer to `test_agent`.** This is pre-existing drift, unrelated to the bugs above, but bug 1 was hiding it: `test_get_server_info` is the first test to reach the mismatch, so unhanging the suite exposes it. Renaming the fixture (one line) clears 14 of the 26 pre-existing failures. `test_add_agent_duplicate_tool_name` needed the paired change, because it registers a *second* tool called `test_agent` and that name is now the fixture's own. Happy to split the fixture rename into its own PR if you would rather keep this to the two `aop.py` fixes — say the word and I will. ## Verification ``` tests/structs/test_aop.py 12 failed, 65 passed in 39.08s black --check --line-length 70 clean (aop.py, test_aop.py) ruff check clean ``` The 12 remaining failures are all pre-existing on `master` and untouched here. The red checks on this PR are the repo-wide ones — `build` never installs the package, `test-main-features` dies on Poetry 2.x `--no-dev`, both addressed in #1812.
Proposed by ayaangazali## Summary - remove 23 legacy prompt modules that have no imports or call sites - keep the exported `swarms.prompts` API unchanged - remove 1,677 lines of unreachable prompt content from the shipped package Fixes #1835 ## Verification - `python -c "import swarms; import swarms.prompts"` - `python -m compileall -q swarms` - `python tests/test_prompt_caching.py --offline` (18 checks passed) - `python -m pytest tests/utils/test_add_prompt_to_marketplace.py -q` (13 passed) - repository-wide scan for all 23 module names (0 references)
Proposed by cakeniFixes #1831 Delete the remaining dead duplicate module `swarms/structs/various_alt_swarms.py`. The live/exported swarm-architecture implementations are in `swarms/structs/swarming_architectures.py` and are already covered by tests.
Proposed by ShlokShar## What Consolidates the four overlapping format helpers as described in #1851. ### Changes - **Delete `format_dict_to_string`** (31 lines, 0 callers) and its re-export from `swarms/utils/__init__.py` - **Add `style` parameter** to `format_data_structure`: `"indented"` (default, existing behavior) and `"compact"` (matches `any_to_str` output) - **Make `any_to_str` a thin alias** that calls `format_data_structure(data, style="compact")` — all 5+ call sites continue to work with no changes - **`func_to_str.py`** is left for the `base_tool` cleanup since no live code reaches it - **Add 16 parametrized table tests** pinning exact output for nested dict/list/tuple fixtures in both styles - **All 10 existing `any_to_str` tests pass unchanged** ### Why Two functions (`any_to_str` and `format_data_structure`) were used by different parts of the codebase to render the same kinds of objects, producing inconsistent output. Now there is one function with a predictable output format per style. ### How to test ```bash python -m pytest tests/utils/test_any_to_str.py tests/utils/test_format_data_structure.py -v ``` 26 tests pass. If this helped, RawNuke welcomes sponsorship: https://github.com/sponsors/RawNuke
Proposed by RawNukeDescription: Correct both `Conversation.return_all_except_first` variants to skip exactly one message instead of two. The existing focused tests now use direct assertions, so this regression can no longer be swallowed. Issue: Addresses item 1 in #1853. Dependencies: None. Tag maintainer: General / structures maintainers. Twitter handle: N/A. Testing: - `pytest -q tests/structs/test_conversation.py` — 51 passed - `black swarms/structs/conversation.py tests/structs/test_conversation.py --check --diff` — passed - `ruff check swarms/structs/conversation.py tests/structs/test_conversation.py` — passed with the CI-pinned Ruff 0.2.1
Proposed by nightcitybladeCloses #1838. ## 1. Dead print methods (142 lines) Removed `print_progress`, `print_panel_token_by_token`, and `print_plan_tree` from `swarms/utils/formatter.py`. Verified 0 callers across `swarms/`, `tests/`, and `examples/` before removing. Also dropped the now-unused `import time` and the `Progress`/`SpinnerColumn`/`TextColumn` imports, which only `print_progress` used. ## 2. `print_markdown` redundant alias `print_markdown`'s `markdown_handler` branch was byte-identical to `print_panel`'s markdown branch (:434-437 in the original). Its only caller outside this file was `tests/utils/test_formatter.py`. Rather than delete it and touch the test, I kept it as a one-line documented alias forwarding to `print_panel`, so existing/external callers keep working unchanged. ## 3. `_clean_output` — 8 regexes → 2 The two blocks of four near-identical `re.sub` calls (differing only by level name: INFO/DEBUG/WARNING/ERROR) collapse to two regexes using an alternation group, per the issue's suggestion. While doing this I noticed the hardcoded level list only ever covered 4 of loguru's 7 levels — `swarms` uses `logger.success(...)` throughout (e.g. `graph_workflow.py`), so SUCCESS-level lines were passing through `_clean_output` unstripped, and same for TRACE/CRITICAL. The alternation now includes all 7: `INFO|DEBUG|WARNING|ERROR|SUCCESS|TRACE|CRITICAL`. This is a small behavior improvement bundled with the consolidation, not just a refactor. I verified byte-identical output vs. the original 8-regex version for every case the original code path already covered (empty string, plain text with no matching pattern, INFO/DEBUG/WARNING/ERROR lines, `"Generated content:"` stripping, whitespace collapsing) — the consolidation changes nothing except adding coverage for the 3 previously-missed levels. ## Testing Couldn't fully install the project's heavier dependencies (litellm, mcp, opentelemetry) in my sandbox, so I verified two ways: 1. Loaded `swarms/utils/formatter.py` directly via `importlib` (bypassing `swarms/__init__.py`, which only formatter.py's own top-level imports are needed for) and ran the new assertions directly against both the original and modified file to confirm identical behavior on shared cases and correct new behavior on the 3 added levels. 2. Confirmed via git blob SHA-1 that the pushed file content is byte-identical to what was locally verified (no transcription drift). Added to `tests/utils/test_formatter.py`: - `test_clean_output_strips_every_log_level` (parametrized over all 7 loguru levels) - `test_clean_output_handles_empty_string` - `test_dead_print_methods_are_removed` - `test_print_markdown_still_works_as_alias` Also ran `ruff check` and `black --check` against the project's actual configured rules (`pyproject.toml`: `line-length = 70`, CI pins `ruff==0.2.1` with no explicit `[tool.ruff.lint] select`, so only the default `E4,E7,E9,F` rules apply) — both clean on the two changed files. `black --line-length 70` was applied to the new test additions.
Proposed by agu2347Thank you for contributing to Swarms! - Description: Prevent non-interactive agents from reading stdin when `run()` receives `None`, an empty string, or whitespace. Such calls now raise a clear `ValueError`; interactive agents retain their prompt behavior. Adds focused regression coverage that fails if stdin is accessed. - Issue: Fixes #1852 - Dependencies: None - Tag maintainer: @kyegomez - Twitter handle: N/A Please make sure your PR is passing linting and testing before submitting. Run `make format`, `make lint` and `make test` to check this locally. See contribution guidelines for more information on how to write/run tests, lint, etc: https://github.com/kyegomez/swarms/blob/master/CONTRIBUTING.md If you're adding a new integration, please include: 1. a test for the integration, preferably unit tests that do not rely on network access, 2. an example notebook showing its use. Maintainer responsibilities: - General / Misc / if you don't know who to tag: kye@swarms.world - DataLoaders / VectorStores / Retrievers: kye@swarms.world - swarms.models: kye@swarms.world - swarms.memory: kye@swarms.world - swarms.structures: kye@swarms.world If no one reviews your PR within a few days, feel free to email Kye at kye@swarms.world See contribution guidelines for more information on how to write/run tests, lint, etc: https://github.com/kyegomez/swarms Validation performed: - `.venv/bin/python -m pytest tests/structs/test_agent.py::TestBasicAgent::test_noninteractive_empty_task_does_not_read_stdin -q` — 3 passed - `.venv/bin/black . --check --diff` — 955 files unchanged - `.venv/bin/ruff check .` — passed
Proposed by nightcitybladeDescription: Removes the unused `Agent.handle_artifacts` method, its class docstring entry, and the now-unused `Artifact` import. Issue: Fixes #1823 Dependencies: None Tag maintainer: @kyegomez Checks: - `python3.12 -m py_compile swarms/structs/agent.py` - `ruff check --select F401 swarms/structs/agent.py`
Proposed by ShlokShar## What Six secondary correctness issues surfaced during the code-waste audit. Each is small and independent; grouped here to keep them from being lost. Split into separate issues if any needs its own discussion. --- ### 1. `conversation.py:1300-1319` — `return_all_except_first` skips two messages, not one Both `return_all_except_first` (:1300-1306) and `return_all_except_first_string` (:1308-1319) slice `[2:]`, not `[1:]`. The name, the docstring, and the obvious reading all say "except the first" — callers silently lose the second message too. Needs a decision: fix the slice (behavior change for anyone depending on the current output) or rename the methods to match reality. Recommend fixing the slice and adding a test. --- ### 2. `agent.py:4109-4115` — an exception tuple that collapses to `except Exception` ```python except (AgentRunError, AgentLLMError, BadRequestError, InternalServerError, AuthenticationError, Exception): ``` All five named classes are `Exception` subclasses, so the tuple is exactly equivalent to `except Exception`. The enumeration reads as if specific errors are being handled distinctly, but nothing distinguishes them. Either give each its own handler, or collapse to `except Exception` (6 lines → 1). --- ### 3. `agent.py:2901-2903` — awaiting a sync method that always raises `arun` does `await self._handle_run_error(error)`, but `_handle_run_error` (:1788) is a **sync** method whose last statement is `raise error`. The `await` therefore never actually runs. It is harmless only by accident: if `_handle_run_error` is ever changed not to raise, this becomes `TypeError: object NoneType can't be used in 'await' expression`. Drop the `await`, or make the method genuinely async. --- ### 4. `agent.py:3413-3415` — `_reinitialize_after_load` stores an already-shut-down executor ```python # if not hasattr(self, "executor") or self.executor is None: with ContextThreadPoolExecutor(...) as executor: self.executor = executor ``` The `with` block shuts the executor down on exit, so `self.executor` is assigned a dead executor. There is also a commented-out guard left above it. Related to #1793 (executor never assigned in `__init__`) but a distinct site — worth fixing together. --- ### 5. `graph_workflow.py:2557` / `:2739` — `visualize` has a dead fallback and skips filename sanitization `output_path` is assigned unconditionally at :2557, so the `if output_path is None:` branch at :2739-2744 is unreachable (6 dead lines). Worse, **the dead branch is the one that builds a sanitized `safe_name`** — the live path at :2557 uses the raw `self.name`, so a workflow whose name contains `/` produces a broken path and fails to render. Move the sanitization into the live path and delete the dead branch. --- ### 6. `aop.py:2582-2597` — over-broad network-error classification `network_keywords` lists `"timeout"` **twice**, and the list (`"connection"`, `"network"`, `"socket"`, `"reset"`, `"aborted"`, …) is broad enough that `_is_network_error` returns `True` for most exception messages — routing genuinely non-network failures into `_handle_network_error`'s retry loop, where they are retried pointlessly and reported misleadingly. The comment at :2566-2572 shows the `isinstance` check was already narrowed for exactly this reason, but the keyword list was never revisited. See also #1818. --- ## Scope - [ ] 1. Fix the `[2:]` slice (or rename); add a test - [ ] 2. Collapse the redundant exception tuple - [ ] 3. Remove the bogus `await` (or make `_handle_run_error` async) - [ ] 4. Fix the executor assignment in `_reinitialize_after_load`; remove the commented-out guard - [ ] 5. Move filename sanitization into the live `visualize` path; delete the dead branch - [ ] 6. Tighten `network_keywords` and de-duplicate `"timeout"` ## Note A seventh item from the audit — file tools in `autonomous_loop_utils.py` not confining resolved paths to the workspace root — is already tracked as #1791. ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, "Secondary correctness notes"). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `swarms/structs/agent.py:4032-4036` contains a half-commented conditional. The guard was commented out but its body was kept: ```python # # If interactive mode is enabled and no task is provided, prompt the user # if self.interactive and ( # task is None # or (isinstance(task, str) and task.strip() == "") if ( task is None or isinstance(task, str) and task.strip() == "" ): # Always show prompt when asking for initial task, even if print_on is False self.pretty_print( "Interactive mode enabled. Please enter your initial task:", ... ) ``` The `self.interactive and` clause is gone from the live condition, but the interactive prompt body remains. ## Impact `agent.run()` called with `task=None` or an empty/whitespace string now **blocks on `formatter.console.input()` regardless of the `interactive` setting**. In an interactive terminal that is merely surprising. In a server, a batch job, a CI run, or any containerized deployment with no TTY, it is a **hang** — the process waits forever on stdin for input that will never arrive, and the log line even claims "Interactive mode enabled" when the user explicitly set `interactive=False`. ## Reproduction ```python from swarms import Agent agent = Agent(agent_name="X", model_name="gpt-4", interactive=False) agent.run("") # hangs on stdin instead of erroring or no-opping ``` ## Expected behavior With `interactive=False`, an empty task should either raise a clear `ValueError` or return without prompting — it must never read stdin. ## Fix Restore the guard: ```python if self.interactive and ( task is None or (isinstance(task, str) and task.strip() == "") ): ``` and delete the commented-out lines. Leaving both a commented-out version and a live divergent version is what made this invisible in review. Consider also deciding what non-interactive callers should get for an empty task — most likely a `ValueError` with a clear message, since silently running an empty prompt is rarely intended. ## Scope - [ ] Restore the `self.interactive` guard; remove the commented-out block - [ ] Decide and implement the non-interactive empty-task behavior (recommend `ValueError`) - [ ] Add a regression test: `interactive=False` + empty task must not read stdin (monkeypatch `console.input` to raise) ## Context Found during the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, Bug 1). Flagged as the most urgent correctness item in the audit, independent of line count. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What Four separate helpers recursively walk dict/list/tuple structures and produce an indented `key: value` string. They differ only in bracket and quote cosmetics. | Function | Location | Lines | Callers | |---|---|---:|---| | `format_data_structure` | `swarms/utils/index.py:46-155` | 110 | `agent.py` ×7 | | `format_dict_to_string` | `swarms/utils/index.py:13-43` | 31 | **0** (only the `__init__` re-export) | | `any_to_str` | `swarms/utils/any_to_str.py:4-63` | 60 | `consistency_agent`, `swarm_rearrange`, `conversation`, `agent_rearrange`, `model_router` | | `function_to_str` / `functions_to_str` | `swarms/tools/func_to_str.py` | 42 | reachable only via the dead `base_tool` methods | ## Analysis - **`format_dict_to_string` is strictly redundant** — `format_data_structure` already handles every case it does, and it has zero callers. Pure deletion. - **`any_to_str` and `format_data_structure` genuinely overlap** — same traversal, different output cosmetics. They can be one function with a `style=` parameter. - **`func_to_str.py` dies automatically** with the `base_tool.py` dead-method cleanup (see that issue) — nothing else reaches it. ## Why it matters Two of these are used by different parts of the same codebase to render the same kinds of objects, so the same dict can be printed two different ways depending on which call path reached it. Consolidating gives one predictable output format. ## Scope - [ ] Delete `format_dict_to_string` (31 lines, 0 callers) and its `__init__` re-export - [ ] Fold `any_to_str` into `format_data_structure` with a `style=` flag; update the 5 call sites, keeping `any_to_str` as a thin alias if churn is a concern - [ ] Confirm `func_to_str.py` is removed alongside the `base_tool` cleanup - [ ] Add a table test pinning the exact output for a nested dict/list/tuple fixture in each style, so the consolidation is provably behavior-preserving ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 3.5). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What This is the **structural root cause** behind most of the boilerplate in the audit. `BaseSwarm` exists, but **no production orchestrator inherits from it** (see #1824), so ~25 swarm classes each independently reinvent the same four methods — with gratuitous naming and behavior differences. ## 1. `batch_run` / `batched_run` / `run_batched` — 22 implementations (~330 lines) Three different names for `return [self.run(t) for t in tasks]`, each wrapped in 8-12 lines of docstring. **Pure one-liner form:** `advisor_swarm.py:255`, `auction_swarm.py:471`, `debate_with_judge.py:545`, `llm_council.py:528`, `mixture_of_agents.py:276`, `majority_voting.py:257`, `planner_generator_evaluator.py:966`, `skill_orchestra.py:891`, `tree_swarm.py:519`, `auto_swarm_builder.py:724`, `base_swarm.py:346` **Expanded-but-identical loop form:** `cron_job.py:222`, `model_router.py:286`, `multi_agent_router.py:461`, `concurrent_workflow.py:677`, `hiearchical_swarm.py:1779`, `swarm_router.py:977`, `round_robin.py:301`, `sequential_workflow.py:460`, `groupchat.py:557` Only `agent_rearrange.py:921` genuinely differs (clone-per-task isolation via `_clone_for_task`). Note `base_swarm.run_batch` (:435) is a wrapper around a wrapper. ## 2. `__call__` and async wrappers — 18 sites (~190 lines) `__call__` is always `return self.run(task, *args, **kwargs)` under a 6-12 line docstring, in 13 classes (`sequential_workflow.py:446`, `swarm_router.py:950`, `model_router.py:308`, `agent_rearrange.py:865`, `advisor_swarm.py:268`, `auction_swarm.py:465`, `cron_job.py:235`, `multi_agent_router.py:447`, `planner_generator_evaluator.py:990`, `skill_orchestra.py:887`, `auto_agent_builder.py:420`, `base_swarm.py:199`, `image_batch_processor.py:247`). Async wrappers (`asyncio.to_thread(self.run, ...)`) at `agent_rearrange.py:1047`, `base_swarm.py:368`/`411`/`423`, `sequential_workflow.py:489`, `hiearchical_swarm.py:1834`, `social_algorithms.py:588` — with **inconsistent naming**: `arun` vs `run_async`. ## 3. `reliability_check` — 13 copies (~180 lines) The same "agents list non-empty, `max_loops != 0`" assertions rewritten with different error strings: `sequential_workflow.py:180`, `agent_rearrange.py:193`, `concurrent_workflow.py:219`, `hiearchical_swarm.py:701`, `mixture_of_agents.py:128`, `majority_voting.py:154`, `spreadsheet_swarm.py:93`, `advisor_swarm.py:124`, `council_as_judge.py:294`, `heavy_swarm.py:219`, `swarm_router.py:438`, `auto_swarm_builder.py:313`, `cron_job.py:86`. Naming is inconsistent four ways: `reliability_check` / `reliability_checks` / `_validate_inputs` / `_validate_swarm_type`. ## 4. Hand-rolled thread pools — 8 sites (~150 lines) `multi_agent_exec.run_agents_concurrently` (:95) and `batched_grid_agent_execution` (:242) **already exist**, yet 8 classes hand-roll `ThreadPoolExecutor` + `as_completed`: `majority_voting.py:273`, `mixture_of_agents.py:287` (byte-identical to the previous), `model_router.py:338`, `swarm_router.py:1024`, `agent_rearrange.py:998`, `skill_orchestra.py:895`, `base_swarm.py:469`, `hybrid_hiearchical_peer_swarm.py:221`. **Two real defects hiding in here:** - `swarm_router.concurrent_run` (:1024) submits a **single task** to a pool — it provides no concurrency at all - Each site invents its own worker count (`os.cpu_count()`, `cpu*2`, `min(len, cpu or 4)`, `max_workers_95_percent()`), so parallelism differs arbitrarily between swarm types for no stated reason ## Proposed fix One small `SwarmMixin` (or a stripped-down `BaseSwarm` — see #1824) providing: - `batch_run(tasks, *, sequential=True, max_workers=None, **run_kwargs)`, with `batched_run`/`run_batched` as thin deprecated aliases — **standardize on one name** - `__call__` → `run` - `arun` (standardize on `arun`; deprecate `run_async`) - `validate_config(agents=None, max_loops=None, output_type=None, min_agents=1, name="")` — each class keeps only its genuinely specific checks (e.g. `flow` for `AgentRearrange`, `worker_model_name` for `HeavySwarm`) - Delegate concurrency to the existing `multi_agent_exec` helpers with one consistent default worker count **~850 lines removed across ~25 files**, and every swarm behaves consistently. ## Suggested sequencing 1. Land #1824 first (strip `base_swarm.py`) so there is a clean home 2. Add the mixin with full test coverage 3. Migrate orchestrators **one file per PR** — mechanical and individually reviewable 4. Keep deprecated aliases for at least one minor release ## Related #1824 (base_swarm stub graveyard), and the autosave mixin issue — the fifth duplication with the same root cause. ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, Part 3, sections 3.1-3.4). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What Ten independently-confirmed duplications, each too small for its own issue but ~605 lines in aggregate. Each is a self-contained, low-risk fix. | # | What | Where | Lines | |---|---|---|---:| | 1 | `_serialize_callable` ×3 and `_serialize_attr` ×2 are **100% identical** (verified by AST hash); `to_dict` ×2 at 83% | `agent.py:3695`/`3714`, `base_structure.py:428`/`447`, `serialization.py:61`/`107` | ~75 | | 2 | `autonomous_loop_utils.py` five file tools repeat the same 17-line path-resolution preamble and error postamble | `create_file_tool` :643, `update_file_tool` :697, `read_file_tool` :759, `list_directory_tool` :807, `delete_file_tool` :878 | ~85 | | 3 | Five agent factories identical except name/prompt — same 22-line `agent_kwargs` + ollama block | `hierarchical_structured_communication_framework.py:1252-1360` | ~85 | | 4 | Four councilor prompt builders (same paragraph skeleton, different adjectives) + four near-identical `Agent(...)` blocks | `llm_council.py:30-125`, `:333-390` | ~105 | | 5 | `tool_parse_exec.process_json_block` (:45-113) is a copy of the outer main path (:133-220); :50-62 ≡ :150-166 and :86-105 ≡ :193-211 verbatim | `tools/tool_parse_exec.py` | ~60 | | 6 | `deep_discussion.one_on_one_debate` (:10-63) duplicates `multi_agent_debates.OneOnOneDebate.run` (:36-82) — same validation, same `speaker, other = other, speaker` swap. **Both carry the same bug**: `other` is assigned and never read | `deep_discussion.py`, `multi_agent_debates.py` | ~55 | | 7 | The identical 3-line error f-string (with the GitHub issues URL) pasted **11 times**, plus a 12th hand-formatted variant at :1755-1770 | `hiearchical_swarm.py` :656, :697, :734, :789, :874, :966, :1014, :1084, :1281, :1672, :1831 | ~35 | | 8 | Four bespoke panel printers sharing a ~12-line skeleton | `agent.py` `_visualize_function_call` :1813, `_visualize_handoff_call` :1850, `pretty_print` :4526, `print_dashboard` :1228 | ~40 | | 9 | Parallel branch (:252-289) ≡ sequential branch (:298-335) except `results.append(result)` vs `current_task = result` | `swarm_rearrange.py` | ~35 | | 10 | 8 log-strip regexes differing only by level name | `formatter.py:44-79` | ~30 | ## Suggested fixes 1. One `_serialize_callable` / `_serialize_attr` in `serialization.py`; import from the other two 2. `_resolve(agent, path)` helper + a `@_file_tool("creating")` decorator owning the except block and memory write. **Note:** these five also don't confine resolved paths to the workspace root — that is tracked in #1791 3. One `_make_agent(name: str, prompt: str) -> Agent` 4. A `COUNCILORS: list[CouncilorSpec]` data table + one prompt template + a comprehension 5. One `_execute_function_list(function_list, function_dict, max_retries, verbose) -> dict` 6. Make `OneOnOneDebate.run` delegate to `one_on_one_debate` (or vice versa), and drop the unused `other` variable 7. One module-level `_report_error(e, context) -> str` 8. One shared panel helper taking (title, content, style) 9. Extract `_run_one_swarm(swarm_name, current_task, img, *a, **kw) -> str` 10. Two regexes with an alternation group over the level names ## Scope These can land as one PR or ten small ones — they touch independent files. Recommend grouping 1/5/10 (mechanical), 2/3/4/7/8/9 (per-file), and 6 (needs a decision on which of the two implementations survives). ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 2.11 plus the formatter item from 1.10). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What There are three independent implementations of "convert a pydantic model / function into an OpenAI function schema," plus a wasteful envelope round-trip between two of them — and one of them emits the **wrong function name**. ## The bug (fix this part regardless) `swarms/tools/pydantic_to_json.py:58`: ```python name = type(pydantic_type).__name__ ``` `pydantic_type` here is a **class**, not an instance. `type(SomeModel).__name__` returns `"ModelMetaclass"`, not `"SomeModel"`. Any schema emitted through this path can carry the wrong function name, which means the LLM is told a tool is called `ModelMetaclass`. The same mistake is repeated in `check_pydantic_name` (:20-37) — which has **0 callers** and should just be deleted. ## The three implementations 1. **`swarms/tools/pydantic_to_json.py:40-109`** — `base_model_to_openai_function`: `model_json_schema()` → docstring merge via `swarms/utils/docstring_parser.parse` → `_remove_a_key(parameters, "title")` / `"additionalProperties"` → emits a legacy `{"function_call": …, "functions": […]}` envelope 2. **`swarms/tools/py_func_to_openai_func_str.py`** — `get_openai_function_schema_from_func`: the full autogen-derived pipeline (`type2schema`, `get_parameter_json_schema`, `get_parameters`), 597 lines 3. **`swarms/utils/class_to_pydantic.py`** — a third path (class → pydantic → schema) using the same `docstring_parser.parse`; referenced only by two examples ## The pointless round trip `base_tool.base_model_to_dict` (:231-320) takes #1's output, **unpacks `result["functions"][0]`, and re-wraps it** as `{"type": "function", "function": …}`. So #1 builds the legacy envelope purely for its only caller to immediately discard it. And `multi_base_model_to_openai_function` (:112-144) list-comprehends over #1 and re-wraps the envelope a third time. ## Dead pydantic-v1 branch (~60 lines) `py_func_to_openai_func_str.py:82-141` is gated on `PYDANTIC_VERSION.startswith("1.")` and re-defines `type2schema`, `model_dump`, and `model_dump_json` against pydantic v1's `schema_of`/`.dict()`/`.json()`. `pyproject.toml:64` pins `pydantic = "*"` and the codebase uses v2-only APIs throughout (`model_json_schema`, `TypeAdapter`, `model_validate_json`), so v1 cannot install and work — the branch is unreachable. Same for the inner `if PYDANTIC_V1:` at :100-113. ## Proposed fix - [ ] **Fix the name bug**: use `pydantic_type.__name__` (and add a test asserting the emitted schema name) - [ ] Delete `check_pydantic_name` (0 callers, same bug) - [ ] Have `base_model_to_openai_function` emit the modern `{"type": "function", "function": {…}}` directly; delete the envelope build + unwrap round trip in `base_tool.base_model_to_dict` and `multi_base_model_to_openai_function` - [ ] Delete the pydantic-v1 branch - [ ] Decide whether `utils/class_to_pydantic.py` earns its place or should be folded into one of the other two **~150 lines removed**, plus a correctness fix. ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, sections 2.10, 2.11, and Bug 5). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What The block that unpacks an OpenAI-style tool call into `{name, arguments, id}` is implemented **at least eight times** across the codebase, in two clusters. ## Cluster 1 — five copies inside `swarms/tools/base_tool.py` Identical bodies: read `function.name`, read `function.arguments`, `json.loads` if it is a string, append `{"name", "arguments", "id", "type": "openai"}`, catch `JSONDecodeError`, and log `f"Failed to parse arguments for {name}: {e}"` — identical down to the log string. | Lines | Context | |---|---| | 2399-2424 | response is itself a function call | | 2429-2463 | `choices[].message.tool_calls[]` | | 2465-2500 | root-level `tool_calls[]` | | 2897-2928 | OpenAI SDK objects (`getattr` variant) | | 2930-2961 | dict-shaped tool calls | Separately, `_extract_generic_function_calls` (:2614-2666) duplicates `_is_direct_function_call` (:3024-3043) + `_extract_direct_function_call` (:3044-3077) — the same "has name, and one of arguments/parameters/input" test producing the same output dict. ## Cluster 2 — three copies in `swarms/structs/` | Implementation | Lines | |---|---| | `hiearchical_swarm.parse_orders` | 1574-1700 (~125) | | `planner_worker_swarm._parse_structured_output` | 610-690 (~80) | | `heavy_swarm._parse_tool_calls` | 1249-1309 (~60) | `hiearchical_swarm.py:1584-1622` and `planner_worker_swarm.py:618-658` are structurally identical — the same nested `isinstance(output, list)` → `"content" in item` → `"function" in content_item` → `json.loads(function_data["arguments"])` walk, differing only in what they construct at the leaf. `planner_worker_swarm._parse_structured_output`'s own docstring admits it: *"Follows the same pattern as HierarchicalSwarm.parse_orders()."* Minor: `hiearchical_swarm.py:1576` does a function-local `import json` although the module already imports it. ## Why it matters Provider response shapes change. When they do, eight separate parsers must each be found and updated — and the three in `structs/` are the ones most likely to be missed, since they don't live near the tool code. Any provider that emits a slightly different envelope will work in some swarms and silently fail in others. ## Proposed fix 1. One `_iter_tool_calls(obj)` normalizer + one `_std_call(name, raw_args, call_id, kind) -> Optional[dict]` helper in `base_tool.py`, replacing the five copies there 2. One shared `parse_tool_call_output(output, model_class)` in a utils module that returns `model_class(**args)` — the three struct parsers each pass their own pydantic model **~265 lines → ~145.** ## Scope - [ ] Extract the two `base_tool.py` helpers; collapse the five copies - [ ] Merge `_extract_generic_function_calls` with the direct-call pair - [ ] Add `parse_tool_call_output` and migrate `hiearchical_swarm`, `planner_worker_swarm`, and `heavy_swarm` - [ ] Add a parser test table covering every response shape currently handled by the eight copies ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 2.9). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What Beyond the broken serialization stack (tracked separately), `swarms/structs/graph_workflow.py` carries ~490 lines of internal duplication. *(Line numbers are as of the audit; the file shifted ~11 lines earlier after `to_json`'s inline in #1826.)* ## 1. Four edge-adding methods with a byte-identical core (275 → ~110) | Method | Lines | |---|---| | `add_edge` | 1343-1397 (55) | | `add_edges_from_source` | 1399-1471 (73) | | `add_edges_to_target` | 1473-1544 (72) | | `add_parallel_chain` | 1546-1620 (75) | All four contain this identical body for the inner work (at :1440-1477, :1454-1470, :1505-1521, :1578-1597): ```python edge = Edge.from_nodes(source, target, **kwargs) if edge.source not in self.nodes: error_msg = f"Source node '{edge.source}' does not exist in GraphWorkflow" logger.error(error_msg); raise ValueError(error_msg) if edge.target not in self.nodes: ... same for target ... self.edges.append(edge) self.graph_backend.add_edge(edge.source, edge.target, **(edge.metadata or {})) ``` **Fix:** one private `_add_one_edge(source, target, **kwargs) -> Edge`; the three bulk methods become `itertools.product` / list comprehensions over it plus a single `_invalidate_compilation()`. ## 2. Mirror-image method pairs (116 → ~50) - `set_entry_points` (:1622-1651) and `set_end_points` (:1653-1682) are identical modulo the words "Entry"/"End" and the target attribute → `_set_endpoints(ids, attr, label)` - `auto_set_entry_points` (:1817-1845) and `auto_set_end_points` (:1847-1873) differ only in `in_degree` vs `out_degree` ## 3. `validate` re-implements `_fast_validate` (279 → ~120) `validate` (:3714-3914, 201 lines) and `_fast_validate` (:1179-1256, 78 lines) run the same five checks against the same adjacency maps: | Check | `_fast_validate` | `validate` | |---|---|---| | nodes with `agent is None` | 1204-1214 | 3768-3779 | | no-edges warning | 1216-1217 | 3760-3763 | | isolated nodes | 1219-1226 | 3786-3796 | | cycles | 1228-1234 | 3798-3808 | | unreachable from entry / to exit | 1236-1254 | 3821-3860 | The warning strings are identical down to the wording (`f"Found {len(isolated)} isolated nodes: {isolated}"`), and `_fast_validate`'s own docstring says it is *"Equivalent in coverage to the checks `validate` performs at compile time."* **Fix:** `validate` should call `_fast_validate` and keep only its genuinely unique logic — the auto-fix branches (:3811-3818, :3833-3843, :3853-3860) and the raise block (:3893-3899), roughly 40 of its 201 lines. ## 4. 21 try/except wrappers that do nothing (~105 lines) `try: ... except Exception as e: logger.exception(f"Error in GraphWorkflow.<name>: {e}"); raise e` appears at :1147, :1302, :1338, :1396, :1468, :1541, :1617, :1648, :1679, :1814, :1842, :1870, :1981, :2019, :2525, :2828, :2914, :3355, :3565, :3660, :3709. Each swallows nothing, re-raises the same object, and costs an extra indentation level across the whole method body. `logger.exception` logs the traceback the propagating exception would produce anyway. **Fix:** a module-level `@_logged("GraphWorkflow.add_edge")` decorator, or simply delete them — `run()` already logs at the boundary. ## 5. Repeated metric computation `visualize_simple` (:2891-2900) recomputes the fan-out/fan-in dicts with code identical to `visualize` (:2606-2618) → extract `_fan_patterns()`. And `export_summary` (:3916-4008), `get_compilation_status` (:3568-3596), and `to_json`'s metrics dict (:3269-3285) each compute node/edge/entry/end/layer counts separately. ## Scope - [ ] Extract `_add_one_edge`; rewrite the three bulk edge methods over it - [ ] Merge the entry/end and auto-entry/auto-end pairs - [ ] Make `validate` delegate its checks to `_fast_validate` - [ ] Remove or decorator-ize the 21 log-and-reraise wrappers - [ ] Extract `_fan_patterns()` and a single metrics helper - [ ] Verify validation output (warnings and errors) is unchanged on a fixture graph ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 2.8). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `Agent._run_autonomous_loop` (`swarms/structs/agent.py:1901-2762`) is a single 862-line method whose maximum indentation is **60 spaces — 15 nesting levels**. It contains the same LLM-call-and-dispatch sequence twice. ## The duplicated block This sequence appears in full, twice: > call LLM → `parse_llm_output` → add to memory → iterate the `response` list → `json.loads(tool_call["function"]["arguments"])` → visualize → dispatch → add result to memory - **Planning phase:** :2140-2240 - **Execution phase:** :2380-2540 Both hand-roll the same tool-call unpacking. Both special-case `handoff_task` with an identical `_visualize_handoff_call` + `_handoff_task_tool(handoffs=…)` + `short_memory.add(role="Tool Executor", …)` triple (:2189-2214 vs :2417-2464) — ~25 duplicated lines each. ## Also written three times The "collect existing tool names to avoid duplicates" block appears at **:989**, **:2020-2027**, and **:2266-2275**. ## Why this is worth fixing beyond line count At 15 levels of nesting in a single 862-line method: - The two phases have no enforced consistency — a fix to tool dispatch in the planning phase does not reach the execution phase - It is effectively untestable in isolation; there is no seam to inject a fake LLM response at either phase - Reviewing a change to it requires holding the whole method in your head ## Proposed fix 1. Extract `_dispatch_tool_calls(response, handlers, *, phase) -> list[dict]` returning the unhandled calls — used by both phases, with `handoff_task` registered as one handler rather than special-cased twice 2. Extract `_existing_tool_names() -> set[str]` for the three copies 3. Split the method into `_autonomous_plan()`, `_autonomous_execute()`, and the already-existing `_generate_final_summary()` (:2764-2869) **862 → ~600 lines**, with each phase independently testable. ## Scope - [ ] Extract `_dispatch_tool_calls` and `_existing_tool_names` - [ ] Register `handoff_task` as a normal handler instead of a duplicated special case - [ ] Split into plan / execute / summarize - [ ] Add tests that drive each phase with a canned LLM response — currently impossible ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 2.7). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `swarms/structs/aop.py` spends 538 lines registering MCP tools that are, for the most part, pass-throughs to public methods defined immediately above them. It also re-registers those tools on every agent add, and carries a branch that can never execute. ## 1. Pass-through tool closures (538 → ~140 lines) `_register_agent_discovery_tool` (:1744-2096, 353 lines) and `_register_queue_management_tools` (:2098-2282, 185 lines) define 14 nested closures. **Eleven are pure pass-throughs:** | MCP closure | Delegates to | |---|---| | `get_queue_stats` (:2107) | `self.get_queue_stats` (:1419) | | `pause_agent_queue` (:2123) | `self.pause_agent_queue` (:1495) | | `resume_agent_queue` (:2143) | `self.resume_agent_queue` (:1525) | | `clear_agent_queue` (:2163) | `self.clear_agent_queue` (:1555) | | `get_task_status` (:2188) | `self.get_task_status` (:1587) | | `cancel_task` (:2207) | `self.cancel_task` (:1644) | | `pause_all_queues` / `resume_all_queues` / `clear_all_queues` (:2230/:2249/:2268) | :1680 / :1700 / :1720 | | `list_agents` (:1917) | `self.list_agents` (:1363) | | `get_server_info_tool` (:2055) | `self.get_server_info` (:2818) | Each repeats an identical `try: ... except Exception as e: logger.error(...); return {"success": False, "error": ..., "<key>": <empty>}` envelope. **Inconsistency worth noting as its own defect:** the discovery block has 6 copies of that envelope; the **queue block has none**, so those tools propagate raw exceptions to MCP clients instead of the structured error the others return. **Fix:** a table-driven registrar plus one shared `_tool_envelope(fn, empty_key)` decorator: ```python _SIMPLE_TOOLS = [("pause_agent_queue", self.pause_agent_queue, "Pause the task queue…", _bool_envelope), ...] for name, fn, desc, envelope in _SIMPLE_TOOLS: self.mcp_server.tool(name=name, description=desc)(_wrap(fn, envelope)) ``` ## 2. Redundant re-registration — an O(N) startup cost `_register_agent_discovery_tool()` is called at `:712` (init), **again at :861 inside `add_agent`**, and **again at :966 inside `add_agents_batch`**. The closures read `self.agents` dynamically, so re-registration accomplishes nothing. `add_agents_batch` with 50 agents re-registers 6 MCP tools **51 times**. **Fix:** delete lines 859-861 and 964-966. ## 3. Unreachable non-persistence branch (~40 lines) `run()` returns early at :2429-2432: ```python if not self._persistence_enabled: self.start_server() return ``` Everything after sits inside `while not self._shutdown_requested and ...`. Yet the `except Exception` handler at :2506 branches on `else: # persistence disabled` (:2506-2545) and re-implements the whole network-retry-then-`start_server()` flow. `self._persistence_enabled` is `True` for that loop's entire lifetime, so those 40 lines never run. Same for the `else` at :2465-2469 in the `KeyboardInterrupt` handler. **Fix:** delete both `else` branches; the guards at :2457 and :2470 reduce to `if not self._shutdown_requested`. ## 4. Related: over-broad network-error detection `:2582-2597` — `network_keywords` lists `"timeout"` **twice**, and the list (`"connection"`, `"network"`, `"socket"`, `"reset"`, `"aborted"`) is broad enough that `_is_network_error` returns `True` for most exception messages, routing non-network failures into `_handle_network_error`'s retry loop. The comment at :2566-2572 shows the `isinstance` check was already narrowed for exactly this reason, but the keyword list was left untouched. (See also #1818.) ## Scope - [ ] Table-drive the 11 pass-through tools behind one envelope decorator - [ ] Give the queue tools the same error envelope as the discovery tools - [ ] Remove the two redundant `_register_agent_discovery_tool()` calls - [ ] Delete the unreachable `else` branches in `run()` - [ ] Tighten `network_keywords` / de-duplicate `"timeout"` - [ ] Test: register 50 agents via `add_agents_batch` and assert each tool registers exactly once ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 2.5). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `_setup_autosave()` and `_save_conversation_history()` are copy-pasted across five orchestrators, byte-identical apart from a default name string and the logger alias. ## The copies | File | Lines | Size | |---|---|---:| | `swarms/structs/concurrent_workflow.py` | 718-805 | 88 | | `swarms/structs/hiearchical_swarm.py` | 495-585 | 91 | | `swarms/structs/sequential_workflow.py` | 553-648 | 96 | | `swarms/structs/skill_orchestra.py` | 730-770 | 41 (trimmed variant) | | `swarms/structs/swarm_router.py` | 407-435 | 29 (`autosave_swarm` variant) | An automated 8-line-window clone scan flags **35 identical windows** between `concurrent_workflow.py` and `hiearchical_swarm.py`, essentially all inside this pair. `concurrent_workflow.py:726-751` and `hiearchical_swarm.py:502-527` are byte-identical apart from `"concurrent-workflow"` vs `"hierarchical-swarm"`. ## Plus the call sites (~80 lines) The same 12-line try/except save-on-success + try/except save-on-error wrapper appears at: - `sequential_workflow.py:344-360` - `concurrent_workflow.py:653-670` - `hiearchical_swarm.py:986-1008` ## Plus the helper module itself (~50 lines) `swarms/utils/swarm_autosave.py:96-303` contains **three copies of one save routine** — `save_swarm_config` (:96-155), `save_swarm_state` (:158-228), `save_swarm_metadata` (:231-303) — sharing an identical skeleton: ```python if not swarm_workspace_dir: return None try: <build dict> x_json = json.dumps(x_dict, indent=2, default=str) x_path = create_file_in_folder(swarm_workspace_dir, "<name>.json", x_json) if x_path: logger.debug(...) return x_path except Exception as e: logger.warning(...); return None ``` Only the `<build dict>` middle differs. ## Proposed fix 1. `SwarmAutosaveMixin` in `swarms/utils/swarm_autosave.py` exposing `_setup_autosave(default_name)`, `_save_conversation_history(conversation=None)`, and an `_autosave_guard()` context manager for the call sites 2. One `_save_json(workspace_dir, filename, payload, label)` helper backing the three save routines `swarms/utils/swarm_autosave.py` already exists (374 lines) and is the natural home. **~425 lines → ~90.** ## Scope - [ ] Add `SwarmAutosaveMixin` + `_save_json` to `utils/swarm_autosave.py` - [ ] Migrate the five orchestrators one file at a time, preserving each one's default workspace name - [ ] Replace the three call-site wrappers with the context manager - [ ] Verify autosave output paths and file contents are unchanged for each swarm type ## Related This is one of four duplications rooted in the fact that no production orchestrator inherits shared behavior — see the `SwarmMixin` issue and #1824 (`base_swarm.py` stub graveyard). ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 2.4). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `swarms/structs/heavy_swarm.py` (1,562 lines) duplicates its core execution logic and its configuration table several times over. ## 1. Two executors, one algorithm (532 lines → ~200) - `_execute_agents_basic` (:559-725, 167 lines) - `_execute_agents_with_dashboard` (:727-1091, 365 lines) Both implement the identical algorithm: an inner `execute_agent` closure, a variant→agent_tasks table, `ContextThreadPoolExecutor` + `as_completed`, and the same timeout/exception handling. The only difference is that the second interleaves `tracker.*` calls. Compare `:600-616` vs `:928-950`, and `:686-724` vs `:1034-1067`. **Fix:** one `_execute_agents(questions, agents, img, tracker=None)` where `tracker` defaults to a no-op object when the dashboard is off. This is the standard null-object pattern and removes ~330 lines. ## 2. The `heavy_keys` list is pasted verbatim 4 times (~80 lines) The same 15-name list appears at `:620-636`, `:954-970`, `:1443-1459`, and `:1529-1545`. **Fix:** one module-level constant. ## 3. The variant→agent-task table is built twice (~120 lines) The `if variant == "heavy" / elif "medium" / else` construction appears at `:619-685` and `:953-1032`, ~60 lines each. **Fix:** a module-level `VARIANT_SPECS: dict[str, list[tuple[label, key]]]`. ## 4. The agent-count expression recurs `15 if heavy else 3 if medium else 4` appears at `:398-404` and `:1074-1076`. Derive it from `len(VARIANT_SPECS[variant])` instead. ## Why it matters beyond line count The two executors have already drifted — they are "the same algorithm" only approximately, and any bug fixed in one silently persists in the other. Likewise, adding a 16th heavy key today means editing four separate lists and hoping none is missed. ## Scope - [ ] Extract `VARIANT_SPECS` and the `heavy_keys` constant - [ ] Merge the two executors behind a no-op tracker - [ ] Derive the agent count from the spec table - [ ] Diff the dashboard and non-dashboard output before/after to confirm identical behavior ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 2.3). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `swarms/structs/multi_agent_debates.py` contains seven classes that are the **same 90-120 line method with the variables renamed**. ## The seven | Class | Lines | leader / participants / rounds | |---|---|---| | `ExpertPanelDiscussion` | 85-178 (94) | moderator / agents / max_rounds | | `RoundTableDiscussion` | 181-280 (100) | facilitator / agents / max_cycles | | `PeerReviewProcess` | 382-474 (93) | author / reviewers / review_rounds | | `MediationSession` | 477-576 (100) | mediator / parties / max_sessions | | `BrainstormingSession` | 579-684 (106) | facilitator / participants / idea_rounds | | `CouncilMeeting` | 846-965 (120) | chairperson / council_members / voting_rounds | | `NegotiationSession` | 1073-1192 (120) | mediator / parties / negotiation_rounds | 733 lines total. ## The shared template Every one implements exactly this: 1. Validate ≥2 participants, validate the leader 2. Build `[a.agent_name for a in ...]` and an f-string participant list 3. `leader.run(intro)` 4. Loop participants, injecting `other_x = [name for j, name in enumerate(names) if j != i]` 5. Round loop: leader opens → each participant responds → leader synthesizes via `conversation_history[-len(participants):]` 6. `history_output_formatter(...)` Compare `:138-146` against `:236-246` against `:530-538` against `:904-912` against `:1129-1137` — literally the same five statements with the noun swapped. ## Proposed fix One `_RoundBasedDiscussion` base class (~120 lines) parameterized by: ```python (leader_attr, participant_attr, leader_intro_tmpl, participant_intro_tmpl, round_label, synthesis_prompt) ``` Each of the seven becomes a ~20-line subclass declaring only its strings. `CouncilMeeting` (extra vote phase) and `NegotiationSession` (extra concession phase) each override one hook. **733 → ~260 lines.** ## Why this one is worth doing Beyond the line count: today a fix to the round loop has to be applied seven times, and any divergence between the copies is invisible. There is already an existing test file (`tests/structs/test_multi_agent_debate.py`) plus ~10 examples to validate the refactor against, which makes this the lowest-risk of the large refactors in the audit. ## Scope - [ ] Add `_RoundBasedDiscussion` with the six parameterization points and two override hooks - [ ] Port all seven classes, preserving their public constructor signatures exactly - [ ] Verify against `tests/structs/test_multi_agent_debate.py` and the examples - [ ] Confirm output formatting is byte-identical before/after for a fixed seed ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 2.2). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `swarms/structs/conversation.py` (1,556 lines) carries dead methods, several pairs of accessors with identical bodies, and three partially-broken persistence paths. **Note:** the save/load round-trip failure below is a genuine runtime bug (Bug 3 in the audit) — fixing it and deduplicating the loaders is the same edit. ## 1. Methods with zero call sites (~74 lines) | Method | Lines | Span | |---|---:|---| | `get_visible_messages` | 26 | 1197-1222 | | `export_conversation` | 16 | 678-693 | | `search_keyword_in_conversation` | 14 | 1030-1043 | | `to_list` | 7 | 1189-1195 | | `import_conversation` | 7 | 695-701 | | `clear_memory` | 4 | 1475-1478 | ## 2. Duplicate / incorrect accessors - `get_last_message_as_string` (:1224-1232) and `get_final_message` (:1279-1287) have **identical bodies**. Delete one. - `to_dict` (:801-811) is `return self.conversation_history` — identical to `to_list`, **and its docstring lies**, claiming it returns `{metadata, conversation_history}`. - `get_str` (:774-780) is a documented alias of `return_history_as_string` (:728-750). - `return_dict_final` (:1376-1381) returns `(content, content)` — the same value twice. - `return_all_except_first` (:1300-1306) and `return_all_except_first_string` (:1308-1319) slice `[2:]`, not `[1:]`, contradicting both name and docstring. ## 3. `add_multiple` is broken (:597-628) - No `return` statement, so `add_multiple_messages`'s `added` is always `None` - Spins up `ThreadPoolExecutor(max_workers=int(os.cpu_count() * 0.25))` **to append to a list** — that is `max_workers=0` on any 1-3 core machine (raises `ValueError`), and it randomizes message order when it does work Replace both with a 4-line loop. ## 4. Save/load: three paths, one broken (~120 lines) `save_as_json` (:813-855), `save_as_yaml` (:857-894), and `export` (:896-935) are three entry points; `export` just dispatches to the other two. `load_from_json` (:937-968) and `load_from_yaml` (:970-1001) are **identical except `json.load` vs `yaml.safe_load`** — 64 lines that should be one `_load(filename, parser)`. **The round trip crashes:** `save_as_json` writes `json.dump(self.conversation_history, ...)` — a bare **list** (:845) — but `load_from_json` calls `data.get("metadata", {})` and `data.get("conversation_history", [])` (:949, :956). Loading any file this class wrote raises `AttributeError: 'list' object has no attribute 'get'`. Same defect in the YAML pair. ## 5. Leftovers - `export_and_count_categories` (:536-537) has two bare debug `print()` calls dumping the whole conversation to stdout - `:251-252` — a commented-out call is the **only** internal caller of `truncate_memory_with_tokenizer` (:1045-1109) + `_binary_search_truncate` (:1111-1166); 121 lines now reachable only from one example - `:1539-1556` — 18 lines of commented-out `# Example usage` scratch containing syntactically broken fragments ## Scope - [ ] Delete the 6 unused methods - [ ] Collapse duplicate accessors; fix the `to_dict` docstring - [ ] Fix the `[2:]` vs `[1:]` slices - [ ] Rewrite `add_multiple` as a simple loop that returns - [ ] **Fix the save/load round trip**; merge the loaders into `_load(filename, parser)` - [ ] Remove debug prints and commented-out blocks; decide the fate of `truncate_memory_with_tokenizer` - [ ] Add a `save_as_json` → `load_from_json` round-trip test — its absence is why this shipped ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, sections 1.7 and Bug 3). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `swarms/structs/graph_workflow.py` contains **two complete save/load systems**. One works; the other is 495 lines and is fundamentally broken. | Path | Lines | Status | |---|---|---| | `to_spec` / `save_spec` / `from_topology_spec` | 2968-3181 (214) | works | | `to_json` / `from_json` / `save_to_file` / `load_from_file` | 3183-3712 (495) | **broken** | *(Line numbers are as of the audit; `graph_workflow.py` shifted ~11 lines earlier after `to_json`'s `str_to_json` inline in #1826.)* ## The bug `to_json` serializes agents via `node.agent.to_dict()`, and `from_json` reconstructs them as **plain dicts**: ```python # graph_workflow.py:3409-3415 elif "agent_name" in agent_data and "agent_type" in agent_data: logger.info(f"Node {n['id']} using simplified agent representation: …") agent = agent_data # Store the dict representation for now ``` It then calls `cls.from_spec([n.agent for n in nodes], edges, …)`, producing a workflow whose nodes hold **dicts where `Agent` objects belong**. The resulting workflow cannot be `.run()` — it fails the moment anything calls an agent method. The working path exists precisely because of this. From `to_spec`'s own docstring (:2977-2981): *"Unlike `to_json()`, this method does not attempt to serialize the Agent objects themselves."* ## Impact Anyone following the examples to persist and reload a workflow gets an object that looks fine and then breaks at run time, with a misleading `logger.info` line ("using simplified agent representation") as the only hint. ## Who calls the broken path **No library code.** Only examples and demos: - `examples/multi_agent/graphworkflow_examples/test_enhanced_json_export.py` - `examples/guides/graphworkflow_guide/quick_start_guide.py` - `examples/guides/graphworkflow_guide/setup_and_test.py` - `examples/guides/graphworkflow_guide/comprehensive_demo.py` Internally, only `save_to_file → self.to_json` (:3636) and `load_from_file → cls.from_json` (:3697). ## Proposed fix 1. Delete `to_json`, `from_json`, `save_to_file`, `load_from_file` (495 lines) 2. If backwards compatibility matters, keep `save_to_file`/`load_from_file` as ~6-line aliases over `save_spec`/`from_topology_spec` 3. Point the four example files at the spec path 4. Add a round-trip test that **actually runs** the reloaded workflow — the missing assertion that let this ship ## Scope - [ ] Delete the four methods (or alias two onto the spec path) - [ ] Update the 4 example files - [ ] Add a save → load → **run** round-trip test ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 2.1 and Bug 4). Largest single-edit win in the audit: 495 lines removed *and* a footgun eliminated. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `swarms/utils/formatter.py` (987 lines) carries three public methods with zero callers and a hardcoded regex block that collapses to two patterns. ## 1. Dead print methods (142 lines) Verified 0 hits across `swarms/`, `tests/`, and `examples/`: | Method | Lines | Span | |---|---:|---| | `print_progress` | 28 | 487-514 | | `print_panel_token_by_token` | 35 | 515-549 | | `print_plan_tree` | 79 | 750-828 | ## 2. `print_markdown` is a redundant alias (21 lines) `print_markdown` (:446-466) is used only by `tests/utils/test_formatter.py:120` — no production caller. Its body is `self.markdown_handler.render_markdown_output(content, title, border_style)`, which is **identical to the `print_panel` markdown branch at :434-437**. Either delete it and update the test, or keep it as a documented one-line alias. ## 3. `_clean_output` — 8 near-identical regexes (~30 lines) `MarkdownOutputHandler._clean_output` (:39-113) hardcodes eight `re.sub` log-stripping patterns at :44-79 that differ **only by the log level name** (INFO, DEBUG, WARNING, ERROR, …). These collapse to two regexes using an alternation group, e.g. `(?:INFO|DEBUG|WARNING|ERROR|SUCCESS|TRACE|CRITICAL)`. Besides being ~30 lines shorter, it removes the failure mode where a new log level is added and silently isn't stripped. ## Broader note Between this file and `agent.py`, the codebase has **four bespoke panel printers** sharing a ~12-line skeleton (`_visualize_function_call`, `_visualize_handoff_call`, `pretty_print`, `print_dashboard` in `agent.py`). That consolidation is tracked in the "smaller clones" issue. ## Scope - [ ] Delete `print_progress`, `print_panel_token_by_token`, `print_plan_tree` - [ ] Resolve `print_markdown` (delete + update test, or document as an alias) - [ ] Collapse the 8 log-strip regexes into 2 with an alternation group - [ ] Add a `_clean_output` test covering every log level, so the alternation is verifiably complete ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 1.10). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What Twelve functions across `swarms/structs/` have **zero references anywhere** in the repo (verified by whole-repo name scan). Together ~90 lines. ## The functions | Function | Location | |---|---| | `get_agent_response_schema` | `multi_agent_router.py:55` | | `query_ragent` | `multi_agent_router.py:219` | | `find_multiple_agents_by_name` | `ma_blocks.py:218` | | `track_history` | `swarm_rearrange.py:128` | | `get_communication_history` | `social_algorithms.py:366` | | `get_algorithm_info` | `social_algorithms.py:614` | | `get_pending_count` | `planner_worker_swarm.py:240` | | `get_failed_count` | `planner_worker_swarm.py:263` | | `get_handbook` | `skill_orchestra.py:913` | | `update_handbook` | `skill_orchestra.py:917` | | `get_harness_result` | `planner_generator_evaluator.py:977` | | `coordinate_workflow` | `hierarchical_structured_communication_framework.py:952` | ## Why this needs a decision, not just a deletion Unlike the other dead-code issues, several of these look like **intentional public API that was never wired up or documented** — `get_handbook`/`update_handbook`, `get_harness_result`, and the `get_pending_count`/`get_failed_count` pair read as accessors a user might reasonably call on a swarm instance. The problem is that in their current state they are indistinguishable from rot: no caller, no test, no docs. Whichever way each one goes, the ambiguity should end. ## Proposed resolution per function 1. **Keep** → add a test and a mention in the class docstring / docs so it is demonstrably supported 2. **Drop** → delete it Suggested split (for discussion): keep the accessor-style ones (`get_handbook`, `update_handbook`, `get_harness_result`, `get_pending_count`, `get_failed_count`) with tests; drop the rest (`get_agent_response_schema`, `query_ragent`, `find_multiple_agents_by_name`, `track_history`, `get_communication_history`, `get_algorithm_info`, `coordinate_workflow`). ## Scope - [ ] Make an explicit keep/drop call on each of the 12 - [ ] Delete the drops - [ ] Add a test + docstring reference for each keep - [ ] Run the test suite ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 1.9). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What Beyond `handle_artifacts` (tracked separately in #1823), `swarms/structs/agent.py` carries 15 more methods with **zero call sites anywhere** in the repo — verified by grepping `swarms/`, `tests/`, `examples/`, `scripts/`, and `*.md`. ## The methods | Method | Span | Note | |---|---|---| | `enable_autosave` | 3297-3324 | 28 lines; nothing ever calls it | | `disable_autosave` | 3326-3333 | only caller is `cleanup`, which itself has no callers | | `cleanup` | 3335-3345 | no callers; guards on the autosave thread `enable_autosave` would have started | | `showcase_config` | 4449-4470 | 22 lines | | `model_dump_yaml` | 3794-3814 | third YAML-dump path beside `to_yaml` and `save_to_yaml`; only `model_dump_json` is used externally | | `undo_last` | 3491-3511 | 21 lines | | `update_tool_usage` | 3676-3693 | 18 lines | | `save_to_yaml` | 3513-3526 | | | `output_cleaner_op` | 5011-5023 | | | `tokens_checks` | 3663-3674 | sole caller of `check_available_tokens` (:3651), which only a test touches | | `get_saveable_state` | 3471-3479 | one-line wrapper: `return SafeLoaderUtils.create_state_dict(self)` | | `get_preserved_instances` | 3481-3489 | one-line wrapper | | `receieve_message` | 3555-3562 | **misspelled duplicate** of the live `receive_message` (:2930) | | `get_agent_role` | 4520-4524 | `return self.role` | | `set_system_prompt` | 1206-1208 | identical to `update_system_prompt` (:3531), which *is* used | | `update_retry_attempts` / `update_retry_interval` | 3543-3549 | trivial setters | | `to_toml` | 3769-3770 | the `to_toml` hits in tests belong to `base_structure.py`, not Agent | ## Bonus: two byte-identical private helpers `_log_saved_state_info` (:3425-3446) and `_log_loaded_state_info` (:3448-3469) are **character-for-character identical except for two log strings**. Collapse into one `_log_state_info(path, verb)` (~20 lines saved). ## Notes for the implementer - `receieve_message` is the interesting one: it is a typo'd twin of a real method. Deleting it is safe, but check whether any downstream user code calls the misspelling before a major release. - `set_system_prompt` vs. `update_system_prompt`: keep `update_system_prompt` (used), drop the other. - Anything intended as public API should get at least one test rather than being kept on faith. ## Scope - [ ] Delete the 15 methods above - [ ] Merge `_log_saved_state_info` / `_log_loaded_state_info` into one helper - [ ] Remove the corresponding mentions from the `Agent` class docstring - [ ] Run the test suite ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 1.6). `handle_artifacts` (76 lines) is tracked in #1823. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What 23 files in `swarms/prompts/` are never imported by anything — not by `swarms/`, not by `tests/`, not by `examples/`. Together they are ~1,677 lines of legacy persona prompts that no code path can reach. ## The files | Lines | File | |---:|---| | 276 | `autobloggen.py` | | 104 | `xray_swarm_prompt.py` | | 103 | `agent_self_builder_prompt.py` | | 102 | `self_operating_prompt.py` | | 101 | `multi_modal_prompts.py` | | 97 | `support_agent_prompt.py` | | 91 | `ai_research_team.py` | | 90 | `accountant_swarm_prompts.py` | | 88 | `sop_generator_agent_prompt.py` | | 88 | `sales_prompts.py` | | 78 | `project_manager.py` | | 75 | `agent_orchestration_prompt.py` | | 62 | `code_spawner.py` | | 54 | `swarm_manager_agent.py` | | 52 | `meta_system_prompt.py` | | 48 | `multi_modal_visual_prompts.py` | | 41 | `visual_cot.py` | | 39 | `urban_planning.py` | | 38 | `personal_stylist.py` | | 23 | `aot_prompt.py` | | 14 | `idea2img.py` | | 13 | `task_assignment_prompt.py` | | 0 | `refiner_agent_prompt.py` (empty file) | ## Verification method For each file, both the module name and every exported symbol (prompt constants, functions) were grepped across the entire repo. Zero hits outside the files themselves. Note: `agent_self_builder_prompt.py` mentions `AgentConfiguration` in prose, but that schema was itself deleted as dead code in #1830 — another signal this prompt is orphaned. ## Consideration before deleting These are prompt *content*, not logic, so the cost of keeping them is low and someone may consider them a library of starting points. Two reasonable outcomes: 1. **Delete them** — they are unreachable, unversioned, and untested; anything valuable lives better in docs or examples 2. **Move them to `examples/prompts/`** — preserves the content, removes it from the shipped package Recommend option 1 unless someone identifies specific prompts worth keeping, in which case they should get an actual import path and a test. ## Scope - [ ] Decide delete vs. relocate - [ ] Remove the files (and any `prompts/__init__.py` entries — currently none reference them) - [ ] Verify the package still imports ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 1.5). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What The dead-module sweep from the code-waste audit is mostly complete (PRs #1825, #1826, #1830 removed 10 modules). Two whole modules and several in-file blocks remain. ## Remaining dead modules (~356 lines) | File | Lines | Only reference | Cleanup needed | |---|---:|---|---| | `swarms/tools/tool_registry.py` | 290 | its own re-export in `tools/__init__.py` | remove the `ToolStorage`/`tool_registry` import + `__all__` entries | | `swarms/structs/collaborative_utils.py` | 66 | none — `talk_to_agent` has 0 callers repo-wide | pure deletion | Neither `ToolStorage` nor `tool_registry` is used anywhere outside the `__init__.py` line that imports them. ## In-file dead code in live modules (~160 lines) - **`swarms/schemas/base_schemas.py:8-107`** — only `AgentChatCompletionResponse` (:110-126) is imported (by `agent.py:67`). `ModelCard`, `ModelList`, `ImageUrl`, `TextContent`, `ImageUrlContent`, `ChatMessageInput`, `ChatMessageResponse`, `DeltaMessage`, `ChatCompletionRequest`, `ChatCompletionResponse*`, and `UsageInfo` are all unused (~100 lines). - **`swarms/tools/tool_utils.py:58-90`** — `is_str_valid_func_output` (33 lines), 0 callers, and byte-for-byte the same logic as `BaseTool.check_str_for_functions_valid`. - **`swarms/utils/file_processing.py`** — `check_if_folder_exists` (:13-29) and `create_file` (:85-102), 0 external callers. - **`swarms/utils/litellm_tokenizer.py`** — `get_supported_models` (:73-80), 0 callers, plus 24 lines of commented-out `__main__` block (:83-106). - **`swarms/utils/image_file_b64.py:221-252`** — `get_image_data_uri`, 32 lines of docstring around `return get_image_base64(image_path)`; only an example uses it. - **`swarms/utils/any_to_str.py:66-102`** — 37 lines of commented-out `main()`. ## Scope - [ ] Delete `swarms/tools/tool_registry.py` and its `tools/__init__.py` re-export - [ ] Delete `swarms/structs/collaborative_utils.py` - [ ] Trim the unused classes from `schemas/base_schemas.py`, keeping `AgentChatCompletionResponse` - [ ] Delete the in-file dead functions and commented-out blocks listed above - [ ] Verify all swarms modules still import (`python -c "import swarms"` plus a full submodule walk) ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 1.4). Already-deleted in prior PRs: `func_calling_utils`, `create_agent_tool`, `agent_step_schemas`, `agent_class_schema`, `openai_tool_creator_decorator`, `json_utils`, `handoffs_schema`, `openai_func_calling_schema_pydantic`, `tool_schema_base_model`, `conversation_schema`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What `swarms/tools/base_tool.py` is 3,077 lines. **Only 4 of `BaseTool`'s 30 public methods are called by the framework.** The other ~1,060 lines of public API have zero call sites in `swarms/` and are kept nominally "alive" only by a 1,675-line example harness that is itself two near-duplicate files. ## The 4 methods actually used | Method | Callers | |---|---| | `base_model_to_dict` | `agent.py`, `hiearchical_swarm.py`, `multi_agent_router.py`, `planner_worker_swarm.py` | | `multi_base_models_to_dict` | structs | | `convert_tool_into_openai_schema` | `prompts/prompt.py:225` | | `execute_function_calls_from_api_response` | `agent.py:2553, 2654, 5306, 5311` | ## The 17 with zero hits in `swarms/` | Method | Lines | Note | |---|---|---| | `func_to_dict` | 142-174 | body is `return self.function_to_dict(function)` — a pure alias | | `load_params_from_func_for_pybasemodel` | 175-230 | 55 lines wrapping `load_basemodels_if_needed(func)` | | `dict_to_openai_schema_str` | 371-427 | 57 lines wrapping `function_to_str(dict)` | | `multi_dict_to_openai_schema_str` | 428-490 | | | `get_docs_from_callable` | 491-538 | | | `execute_tool` | 539-604 | | | `detect_tool_input_type` | 605-631 | | | `dynamic_run` | 632-727 | | | `execute_tool_by_name` | 728-809 | | | `execute_tool_from_text` | 810-894 | | | `check_str_for_functions_valid` | 895-972 | duplicated again as `tools/tool_utils.is_str_valid_func_output` | | `convert_funcs_into_tools` | 973-1038 | | | `function_to_dict` | 1281-1323 | wraps `get_openai_function_schema_from_func` | | `multiple_functions_to_dict` | 1324-1381 | | | `execute_function_with_dict` | 1382-1462 | | | `execute_multiple_functions_with_dict` | 1463-1566 | body is a for-loop over the previous method | | `detect_api_response_format` | 2776-2855 | | ## The shape of the waste Every one of these follows the same pattern: a 20-40 line docstring, 5-15 lines of `raise ToolValidationError` guards, a `try:` block with three `self._log_if_verbose("info", ...)` calls sandwiching **one line** of real work, and an `except Exception as e: ... raise FunctionSchemaError(...) from e` tail. `dict_to_openai_schema_str` is 57 lines wrapping a single function call. ## Proposed fix 1. Delete the 17 methods above 2. Keep the 4 live methods plus their private helpers (`find_function_name`, `_execute_single_function_call`, the extraction helpers) 3. Collapse the repeated validate/log/re-raise boilerplate into a single decorator Realistic target: **3,077 → ~800 lines** when combined with the schema-island issue. ## Decision needed `examples/tools/base_tool_examples/test_base_tool_comprehensive.py` and `_fixed.py` (1,675 lines, two near-duplicate files, not in the test suite) are the only things referencing most of these. They should be deleted alongside, or trimmed to the surviving API. ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 1.3). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What Delete `swarms/tools/base_tool.py:1567-2193` — a 627-line block of fourteen methods that form a closed call graph nothing outside ever enters. ## The methods | Method | Lines | |---|---| | `validate_function_schema` | 1567-1642 | | `_validate_single_schema` | 1643-1682 | | `_detect_schema_provider` | 1683-1708 | | `_validate_openai_schema` | 1709-1796 | | `_validate_anthropic_schema` | 1797-1862 | | `_validate_generic_schema` | 1863-1928 | | `_validate_json_schema` (recursive) | 1929-2028 | | `get_schema_provider_format` | 2029-2050 | | `convert_schema_between_providers` | 2051-2119 | | `_extract_function_{name,description,parameters}` | 2120-2148 | | `_build_{openai,anthropic,generic}_schema` | 2149-2193 | ## Evidence it is dead - `validate_function_schema`: **0 hits in `swarms/`**; the only 2 hits anywhere are in `examples/tools/base_tool_examples/schema_validation_example.py` - `get_schema_provider_format` and `convert_schema_between_providers`: **0 hits anywhere in the repo**, including examples and tests - Every other method in the block is private and reachable only from the two above ## It is also internally duplicated - The three `_validate_*_schema` methods are near-copies of one another: check dict → check required keys → recurse into `_validate_json_schema` on the params key - The three `_build_*_schema` methods are three ~15-line dict literals over the same extracted (name, description, parameters) triple So even if it were used, it should be ~150 lines rather than 627. ## Scope - [ ] Delete lines 1567-2193 of `swarms/tools/base_tool.py` - [ ] Delete `examples/tools/base_tool_examples/schema_validation_example.py` - [ ] Optionally keep `_detect_schema_provider` (26 lines) if provider detection is wanted later — it is the only genuinely reusable piece - [ ] Run the test suite (`tests/tools/test_base_tool.py` does not touch this block) ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 1.2). Self-contained deletion; see the companion issue on `base_tool.py`'s 17 never-called public methods for the larger trim. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What Delete the rest of `swarms/structs/various_alt_swarms.py` (527 lines remaining). ## Background The file originally held 1,103 lines. PR #1825 removed 576 of them (the 10 math-sequence classes: `FibonacciSwarm`, `PrimeSwarm`, `PowerSwarm`, `LogSwarm`, `ExponentialSwarm`, `GeometricSwarm`, `HarmonicSwarm`, `StaircaseSwarm`, `SigmoidSwarm`, `SinusoidalSwarm`). This issue tracks deleting the remainder. ## What still remains (all dead) - A **second, colliding `BaseSwarm`** at `various_alt_swarms.py:15-67`, entirely unrelated to `swarms/structs/base_swarm.py` - `CircularSwarm`, `StarSwarm`, `MeshSwarm`, `PyramidSwarm` - `OneToOne`, `Broadcast`, `OneToThree` ## Why it should go 1. **Zero references repo-wide.** Verified across `.py` and `.md` in `swarms/`, `tests/`, and `examples/` — module name and every exported class name. The module is not imported in `swarms/structs/__init__.py`, so it is unreachable through the public API. 2. **It is a worse duplicate of live code.** `swarms/structs/swarming_architectures.py` (360 lines) implements `circular_swarm`, `star_swarm`, `mesh_swarm`, `pyramid_swarm`, `one_to_one`, and `broadcast` — and *that* module **is** exported from `swarms/structs/__init__.py` and covered by tests. Every remaining class here re-implements one of those functions, worse. 3. **The `run()` methods are broken.** Each builds a `responses = []` accumulator, appends to it throughout, and then never returns it — the caller receives whatever `_format_return()` produces and the accumulated list is discarded. 4. **The colliding `BaseSwarm` name is a footgun.** Two unrelated classes with the same name in the same package invite a wrong import. ## Scope - [ ] Delete `swarms/structs/various_alt_swarms.py` - [ ] Confirm no `__init__.py` references need removing (there are none today) - [ ] Run the test suite ## Context From the code-waste audit (`experimental/CODE_WASTE_AUDIT.md`, section 1.1). Pure deletion, zero call-site changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomezFixes #1753 ### Measurement Reproducing the issue's numbers on current master, and after this change: ``` full: 16 tools ~2319 tok | lazy: 5 tools ~712 tok | saved ~1607 tok (69%) ``` The resident block also stops growing with the catalogue — adding a 17th built-in tool no longer costs anything on every call. ### What `selected_tools="lazy"` ships a five-tool core and lets the agent fetch the rest by intent: | resident | `create_plan`, `subtask_done`, `complete_task`, `respond_to_user`, `search_tools` | | --- | --- | | on demand | the other 11, loaded when `search_tools` matches them | ```python agent = Agent(model_name="gpt-5.4", max_loops="auto", selected_tools="lazy") ``` The core is exactly what the loop structurally depends on: planning, subtask bookkeeping, termination, the user-facing reply, and the meta-tool to reach everything else. ### Why not the existing `selected_tools` filter Filtering already existed, but it requires the caller to name the tools up front — which, as the issue puts it, defeats the purpose, since the point is that the *agent* decides what it needs. `search_tools` scores query words against each tool's name and description, so intent works without knowing names: ``` "run a shell command" -> run_bash, grep "read a file" -> read_file, create_file, delete_file "search text in files" -> grep, list_directory "delegate to a sub agent" -> create_sub_agent, assign_task, check_sub_agent_status ``` On a match the handler appends the schemas to `tools_list_dictionary` **and rebuilds the LLM client**. Without that rebuild the client keeps the tool list it was constructed with, and the model would be told about tools it cannot actually call — the same class of bug as #1820. ### The design decision worth reviewing A query matching nothing returns the remaining tool **names**, not the remaining schemas: ``` No tool matched 'zzzz'. Still available, by name: think, create_file, update_file, … Search again using one of these names. ``` My first version returned everything on a miss, as a safety valve so the model was never stuck. That's wrong: a second identical search loaded the whole catalogue and silently undid the entire saving (there's a test pinning this now). Names cost a handful of tokens and still give the model a way forward, since a name matches strongly on the next search. ### Compatibility `selected_tools` still defaults to `"all"`, so nothing changes unless you opt in. The `"lazy"` value is handled before the existing list-membership filter — worth noting, because `"lazy"` is a string and `tool_name in "lazy"` would have done substring matching against it. Handlers are never filtered under `"lazy"`, since the model can load any schema at run time and the handler has to be present when it does. ### Tests Six cases appended to `tests/structs/test_async_subagent.py`. Not a new file — that's the only existing test module that imports `autonomous_loop_utils`, so it's the de-facto owner. They use a stub agent, so no API key and no network: lazy core is a strict subset and under half the size · search matches by intent for three different phrasings · search never returns core tools · loading appends schemas and rebuilds the client exactly once · a repeated search neither reloads nor dumps the catalogue · an unmatched query reports remaining names. ``` 13 failed, 40 passed (master baseline: 13 failed, 34 passed) ``` The 13 failures are pre-existing on an unmodified master — that module has live tests needing credentials. All 6 new tests pass; no test that passed before fails now. Three files touched, `black --line-length 70` clean.
Proposed by ayaangazaliFixes #1758 > **Stacked on #1827** (conditional edges) — that PR's commit is the parent here, so review/merge it first. The retry work reuses the skip machinery #1827 introduces. The diff above the parent is 2 files. ### What was wrong When an agent raised, `run()` turned the exception into a string: ```python output = f"[ERROR] Agent {agent_name} failed: {e}" ``` and recorded it in `prev_outputs` and `execution_results` like any other result. Downstream agents then received it through `_build_prompt` under `Output from {pred}:`, alongside instructions to "verify their findings and build upon their work". So one transient rate limit poisons the entire downstream subgraph, `run()` returns normally with no signal short of substring-matching every value for `[ERROR]`, and every downstream agent still makes its LLM call — billed in full — on garbage input. ### Three parts **1. `RetryPolicy`** ```python wf.add_node(agent, retry=RetryPolicy(max_attempts=3, backoff="exponential")) wf = GraphWorkflow(retry_policy=RetryPolicy(max_attempts=2)) # default for all nodes ``` `backoff` is `none` / `linear` / `exponential`, with `base_delay`, a `max_delay` cap, and `retry_on` to scope which exceptions qualify. `retry_on` defaults to `(Exception,)` rather than a curated list of provider error classes. Every SDK raises its own types, and a hardcoded list silently fails to retry whatever we forgot — which is exactly the failure this PR exists to stop. Callers who want it narrow can pass `retry_on=(TimeoutError, ...)`. Applied by wrapping the node-invocation callable, as the issue suggests. That means the inline single-node path and the thread-pool path get retries from one place, and each retry runs on the worker thread that owns the node instead of blocking the whole layer. **2. `on_node_failure`, once attempts are exhausted** | mode | behaviour | | --- | --- | | `skip_downstream` | **new default** — node produced nothing, dependents are pruned, independent branches continue | | `fail_fast` | raise, chaining the original exception | | `propagate_error` | the old `[ERROR] ...` string, retained for compatibility | ⚠️ **This changes the default.** The issue asks for it explicitly ("retained for compatibility but no longer the default"), and I think it's right — silently feeding an error string to the next agent is worse than not running it. But it *is* a behaviour change for anyone relying on the old string, so it's worth a deliberate call rather than slipping through. `propagate_error` restores the previous behaviour exactly, and `test_propagate_error_keeps_the_legacy_behaviour` pins it. **3. `failed_nodes`** `wf.failed_nodes` maps node id → `"ExceptionType: message"` after a run, reset per loop. Failures become inspectable without string matching. I put it on the workflow rather than in the returned dict because that dict is keyed by node id and any added key could collide with a real node. ### Tests Nine cases appended to `tests/structs/test_graph_workflow.py` — no new file, and they use `backoff="none"` so nothing sleeps: backoff schedule incl. the `max_delay` cap · constructor validation · transient failure retried then succeeds · exhausted retries skip downstream and populate `failed_nodes` · `fail_fast` raises · `propagate_error` legacy path · `retry_on` filtering · per-node policy beats workflow default · unknown failure mode rejected. ``` 65 passed, 11 skipped (47 before #1827, 56 after it) ``` Two files touched, `black --line-length 70` clean. ### Not done Structured per-node status on the result object (issue's point 3, richer form). `failed_nodes` covers the "inspectable without string matching" requirement; a full result object changes `run()`'s return type, which is a breaking change I didn't want to bundle into this. Happy to follow up if you want it.
Proposed by ayaangazaliFixes #1756 ### What `GraphWorkflow` edges were unconditional — every edge fired on every run, so the execution path could not depend on what an agent actually produced. `Edge` already carried `metadata` and it was already threaded through the backend and the serializers, but nothing ever read it for routing. This adds an optional predicate, evaluated after the source node completes: ```python Edge(source="classifier", target="escalate", condition=lambda out: "urgent" in out.lower()) Edge(source="classifier", target="routine", condition=lambda out, ctx: ctx["triage"] != "p0") ``` ### Semantics **A target runs when at least one inbound edge fires.** The "any" rule is what makes a diamond with one conditional branch behave the way people expect — the join still runs when the branch that fired reaches it. An edge with no condition always fires. **A node whose inbound edges all decline is skipped**, per the acceptance criteria: absent from results, not executed with empty input. Skipping propagates — a declined branch prunes its descendants rather than letting the next layer restart them with no input, which is the case that would otherwise produce confident output from an agent that received nothing. **Predicates take `(output)` or `(output, outputs)`.** Arity is resolved once at construction with `inspect.signature`, so the hot path never probes the callable. I deliberately avoided the "call with two args, catch `TypeError`, retry with one" shape — a predicate that raises `TypeError` internally would get called a second time, which for a predicate with side effects is a real hazard. **A predicate that raises is logged and treated as not firing.** A broken condition should not take the whole run down, and declining to route is the safe reading of "we could not establish that this edge should fire". ### Cost to existing graphs None. `compile()` sets `_has_conditions`, and the gating pass is skipped entirely when it's false, so an unconditional graph takes exactly the path it always did. `test_unconditional_graph_is_untouched` asserts the flag stays false and the run is unaffected. ### Also fixed `_build_prompt` filtered predecessor outputs but zipped them against the *unfiltered* predecessor tuple: ```python pred_outputs = [prev_outputs.get(p) for p in preds if p in prev_outputs] ... for pred, out in zip(preds, pred_outputs) ``` With predecessors `(a, b)` and only `b` having produced output, that renders `Output from a:\n<b's output>` — the wrong agent's name on the content. It's a pre-existing bug, but conditional routing makes partial predecessors normal rather than rare, so fixing it is part of making this correct. `test_prompt_labels_stay_aligned_when_a_predecessor_is_missing` fails on master and passes here. ### Acceptance criteria - [x] `Edge(..., condition=callable)` gates whether the target runs - [x] nodes whose inbound edges all fail are marked skipped, not failed, and absent from results - [x] `_build_prompt` handles predecessors that were skipped — and now labels them correctly - [x] conditional edges survive serialization *or the limitation is documented explicitly* — took the second option: a Python callable cannot round-trip through JSON, so `to_json` emits `has_condition: true` and logs a warning naming the edge. A reloaded graph is never silently unconditional. A named-predicate registry is the natural follow-up if you want true round-tripping; I didn't want to introduce a global registry as a side effect of this PR. - [ ] `validate()` warns when a condition makes an end point unreachable — **not done.** Reachability under conditions is undecidable statically (predicates read runtime output), so a truthful check would either be trivial or produce false warnings on correct graphs. Happy to add a narrower version — e.g. warn when *every* inbound edge of an end point is conditional — if that's the intent. - [x] existing unconditional graphs behave identically, no overhead added ### Tests Nine cases appended to `tests/structs/test_graph_workflow.py` — no new file. They use a local stub agent, so no API key and no network: matching branch runs / other branch skipped · skip propagates to descendants · any-inbound rule on a diamond · two-arg predicate receives all outputs · raising predicate doesn't fire and doesn't crash the run · unconditional graph untouched · prompt label alignment · conditions flagged on serialize. ``` 56 passed, 11 skipped (47 passed before this PR) ``` Two files touched, `black --line-length 70` clean.
Proposed by ayaangazali## What Strip `swarms/structs/base_swarm.py` (774 lines) down to the methods that are actually used. A repo-wide audit found ~300 removable lines: ### 31 of 76 methods have empty bodies (docstring + `...` or nothing) `communicate` (:191), `run` (:195), `step` (:219), `broadcast` (:248), `reset` (:253), `plan` (:256), `direct_message` (:285), `autoscaler` (:293), `get_agent_by_id` (:296), `assign_task` (:299), `get_all_tasks` (:302), `get_finished_tasks` (:305), `get_pending_tasks` (:308), `pause_agent` (:311), `resume_agent` (:314), `stop_agent` (:317), `restart_agent` (:320), `scale_up` (:323), `scale_down` (:326), `scale_to` (:329), `get_all_agents` (:332), `get_swarm_size` (:335), `get_swarm_status` (:339), `save_swarm_state` (:343), `add_swarm_entry` (:527), `add_agent_entry` (:538), `retrieve_swarm_information` (:549), `retrieve_joined_agents` (:560), `join_swarm` (:568), `list_agents` (:761), `agents_to_dataframe` (:770) ### 19 methods never referenced anywhere in the repo (verified by grep across swarms/, tests/, examples/) `add_agent_by_id`, `reset_all_agents`, `self_find_agent_by_name`, `self_find_agent_by_id`, `aloop`, `task_assignment_by_id`, `task_assignment_by_name`, `add_llm`, `remove_llm`, `run_on_all_agents`, `add_swarm_entry`, `add_agent_entry`, `retrieve_swarm_information`, `retrieve_joined_agents`, `join_swarm`, `agent_error_handling_check`, `export_output_schema_dict`, `export_and_autosave`, `agents_to_dataframe` ### Actively broken `add_agent_by_id` (:230) calls `self.get_agent_by_id`, which is a no-op returning `None` — it can never work. ## Migration `BaseSwarm` has exactly **one** real subclass in the repo: `HierarchicalStructuredCommunicationFramework` (`swarms/structs/hierarchical_structured_communication_framework.py:1077`). Before deleting anything, verify which inherited methods that class actually uses and keep those (or move them into the subclass). Note: the `BaseSwarm` in `various_alt_swarms.py` is an unrelated local class and is not affected. ## Why it matters beyond the 300 lines `BaseSwarm` is where shared orchestrator behavior *should* live. Because no production swarm inherits from it, ~25 orchestrator classes each reinvent `batch_run` (22 copies under 3 different names), `__call__` (13 copies), `reliability_check` (13 copies), and hand-rolled thread pools (8 copies). Cleaning this file up is the prerequisite for consolidating ~900 lines of duplicated boilerplate across `swarms/structs/` (tracked in `CODE_WASTE_AUDIT.md`, Parts 1.8 and 3). ## Scope - [ ] Audit which `BaseSwarm` methods `HierarchicalStructuredCommunicationFramework` actually calls/inherits; migrate anything it needs - [ ] Delete the 31 empty-body stubs - [ ] Delete the 19 unreferenced methods (deliberate keep/drop decision for any intended as public API) - [ ] Delete or fix the broken `add_agent_by_id` - [ ] Run the test suite 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez## What Delete `Agent.handle_artifacts` in `swarms/structs/agent.py:4372-4447` (~76 lines). ## Why A repo-wide audit found the method has **zero call sites** anywhere — verified by grepping across `swarms/`, `examples/`, `tests/`, `scripts/`, and `*.md` files. The only references are the method definition itself and a mention in the `Agent` class docstring (`agent.py:292`). It is pure dead weight in an already 5,570-line file. ## Scope - [ ] Delete the `handle_artifacts` method (`swarms/structs/agent.py:4372-4447`) - [ ] Remove the `handle_artifacts` mention from the `Agent` class docstring (`agent.py:292`) - [ ] Check whether the `swarms.artifacts` imports used only by this method become unused in `agent.py`, and remove them if so - [ ] Run the test suite to confirm nothing breaks (no test references this method) ## Context Found during the code-waste audit (`CODE_WASTE_AUDIT.md`, Part 1.6 — dead methods in `agent.py`). This is one of 16 dead methods identified in `agent.py`; this issue tracks only `handle_artifacts`, the largest single one. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by kyegomez### What `ConcurrentWorkflow` documents its failure policy as: > `on_error` (str): Failure policy for an agent that raises. `"store"` records the error string as that agent's output and lets the other agents finish; `"raise"` propagates the exception and aborts the run `_run` honours that: ```python except Exception as e: if self.on_error == "raise": raise capture_error(e, self, name="ConcurrentWorkflow.agent_error", ...) ``` `run_with_dashboard` does not: ```python except Exception as e: logger.error(f"Agent {agent.agent_name} failed: {str(e)}") results.append((agent.agent_name, f"Error: {str(e)}")) ``` ### Why it matters `run()` dispatches on `show_dashboard`: ```python if self.show_dashboard: result = self.run_with_dashboard(task, img, imgs, streaming_callback) else: result = self._run(task, img, imgs, streaming_callback) ``` So the failure policy silently depends on whether the dashboard is switched on. A caller who sets `on_error="raise"` specifically to make agent failures loud gets them swallowed the moment they enable the dashboard, and `run()` returns a normal-looking conversation with `"Error: ..."` sitting where an agent result should be. Turning on a display option is not something a user expects to change error semantics. The dashboard path also skipped `capture_error`, so those swallowed failures never reached telemetry either. ### Change Mirror the `_run` branch in `run_with_dashboard`: re-raise when `on_error == "raise"`, otherwise `capture_error` then store as before. Default behaviour (`on_error="store"`) is unchanged. ### Tests Two tests in `tests/structs/test_concurrent_workflow.py`, both using a stub agent so they need no API key: - `test_dashboard_path_honors_on_error_raise` — fails on master (the run completes instead of raising), passes with this change. - `test_dashboard_path_stores_errors_by_default` — pins the `"store"` default so the fix can't over-correct into raising for everyone. ``` # before: 1 failed, 1 passed # after: 2 passed ``` Not a duplicate of #1673 — that one makes the non-dashboard path tolerant of failures; this one makes the dashboard path stop being tolerant when the caller explicitly asked it not to be.
Proposed by ayaangazali### What `HierarchicalSwarm.run_director` and the async streaming loop both clear the director's tool schema before the planning sub-step: ```python if self.planning_enabled is True: self.director.tools_list_dictionary = None # hiearchical_swarm.py:765 out = self.setup_director_with_planning(...) ``` The same line appears again in the async path at `hiearchical_swarm.py:2003`. ### Why it's wrong That assignment cannot achieve what it looks like it's for. `setup_director_with_planning` doesn't use `self.director` at all — it constructs its own throwaway `Agent`, and it already filters `base_model` and `tools_list_dictionary` out of the settings it forwards: ```python settings.update({ key: value for key, value in self.director_settings.items() if key not in {"base_model", "tools_list_dictionary", "planning_system_prompt"} }) ``` So planning has always run schema-free on its own. Nulling the field reaches only the *real* director — the object the swarm needs structured `SwarmSpec` output from on the very next call at line 775, and the object the caller owns when a director is passed in as `HierarchicalSwarm(director=Agent(...))`. The mutation is permanent and unconditional. `Agent` gates its structured-output handling on that same attribute (`if exists(self.tools_list_dictionary)` in `agent.py:1503`, which is what converts a `SwarmSpec` `BaseModel` response into the dict `parse_orders` walks), and any later `llm_handling()` rebuild would drop the schema from the client outright. With `max_loops > 1` every loop after the first runs against a director that has been quietly disarmed. ### Change Delete both assignments. No replacement needed — the behaviour they appear to set up is already provided by the separate planning agent. ### Test `test_planning_keeps_the_director_tool_schema` in `tests/structs/test_hierarchical_swarm.py` runs `run_director` with `planning_enabled=True` and asserts the director's schema survives. It fails on master and passes with this change: ``` $ pytest tests/structs/test_hierarchical_swarm.py -k planning_keeps # before: 1 failed — where None = StubAgent.tools_list_dictionary # after: 1 passed ``` Existing hierarchical tests still pass. `black --line-length 70` and the changed-file `ruff` findings are clean.
Proposed by ayaangazaliFixes #1791 ## Problem The built-in file tools in `max_loops="auto"` have no path confinement. The model picks `file_path` itself, so any prompt injection reaching the loop, a fetched web page, a file the agent was asked to summarise, a sub-agent's task string, becomes arbitrary local file read, overwrite or deletion under the host process's privileges. Reproduced on master `06413ebc` with a tmp workspace and a secret one level above it: ``` escape path: ../../../outside_secret.txt read_file via ../ -> LEAKED read_file /etc/hosts -> LEAKED ``` Six tools shared the same two-hole resolver, `read_file_tool`, `create_file_tool`, `update_file_tool`, `list_directory_tool`, `delete_file_tool` and `grep_tool`: ```python if not os.path.isabs(file_path): workspace_dir = agent._get_agent_workspace_dir() full_path = os.path.join(workspace_dir, file_path) # ".." never normalised else: full_path = file_path # absolute taken verbatim ``` ## Fix One resolver, six call sites, no per-tool guards: ```python def _resolve_in_workspace(agent: Any, path: str) -> str: workspace = os.path.realpath(agent._get_agent_workspace_dir()) full_path = ( path if path and os.path.isabs(path) else os.path.join(workspace, path or "") ) resolved = os.path.realpath(full_path) if resolved != workspace and not resolved.startswith(workspace + os.sep): raise ValueError( f"Path is outside the agent workspace and was refused: {path}" ) return resolved ``` `realpath` is what makes it hold: it collapses `..` and follows symlinks *before* the containment test, so neither can step outside. Each tool's six-line resolve block becomes one line, and the `ValueError` lands in the `except Exception` each tool already has, so the model gets the usual error string rather than a traceback. Net **+43/-45 in one source file**. Absolute paths are still accepted when they point inside the workspace, so the check is containment rather than "relative only" and nothing legitimate is lost: ``` read inside.txt OK read <abs path in ws> OK list workspace OK grep workspace OK create in workspace OK read ../../../secret blocked read /etc/hosts blocked delete ../../../secret blocked (file still present afterwards) ``` `swarms/tools/computer_use.py` already has the right primitive in `_check_realpath`, but `create_computer_use_tools` has no call sites in `swarms/`, so the loop never saw it. I kept this fix local to `autonomous_loop_utils.py` rather than rewiring the loop onto that module, which is a much larger change. This is also the piece that makes #1751's proposed default policy safe: that issue auto-approves `read_file`, `grep` and `list_directory`, which under master means auto-approving arbitrary filesystem reads. ## Test New `tests/structs/test_autonomous_loop_file_confinement.py`. There is no existing test module for `autonomous_loop_utils.py`, so this is a new file rather than an addition to one. No agent and no network: the workspace is a `tmp_path` and the agent is a mock that only answers `_get_agent_workspace_dir`. Two tests. The first drives all six tools at an escape path, by `..` and by absolute path, and also asserts the secret's contents are unchanged after `create_file` and `update_file` are pointed at it. The second pins that ordinary workspace access still works, including absolute paths inside it. On master: ``` AssertionError: read_file relative did not refuse the escape: SECRET-OUTSIDE ``` Both pass on this branch. `tests/structs/test_async_subagent.py`, the only other suite touching these tools, is unchanged. `tests/structs/test_agent.py` is 21 failed / 41 passed / 8 errors both before and after, all pre-existing. `black --check` and `ruff check` clean at line-length 70. I use Claude Code to help me work through these and I reproduce every claim before opening anything. Since this one is a hardening change rather than a crash fix, if you would rather it warn instead of refuse while you assess the blast radius, that is a one-line change and I will send it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by ayaangazaliFixes #1798 Four one-line-ish fixes, **4 files, +5/-4 total**. Taking them in one PR as the issue suggests, since four separate PRs for four YAML lines would be noise. ## 1. `test-main-features.yml` cds into a laptop-only path, and dies before it ```diff - poetry install --with test --no-dev + poetry install --only main,test ... - name: Run Main Features Tests run: | - cd /Users/swarms_wd/Desktop/research/swarms poetry run python tests/test_main_features.py ``` `--no-dev` was removed in Poetry 2.x, so the job died at install with `The option "--no-dev" does not exist` before ever reaching the `cd`. `--only main,test` is the modern equivalent of "main plus the test group, no dev". The `cd` targeted a path that exists on one developer's machine, and the step already runs in the checkout, so deleting the line is the whole fix. The `test-coverage` job at line 138 uses plain `poetry install --with test`, which is still valid in Poetry 2.x, so I left it alone. ## 2. Bare `pytest` collects `examples/` and `scripts/` Fixed in `pyproject.toml` rather than in the workflow: ```diff [tool.pytest.ini_options] +testpaths = ["tests"] ``` One line, and it makes bare `pytest` correct everywhere, in CI and on a contributor's laptop, instead of only in `python-package.yml`. So that workflow needs no edit at all. There are currently **51** `test_*.py` / `*_test.py` files under `examples/` and `scripts/`, which is what produced the collection errors in the run log. Proof of the mechanism, in a scratch tree so nothing in this repo is imported: a file outside `tests/` that raises at import. ``` with testpaths = ["tests"] -> 1 passed without testpaths -> ERROR examples/test_b.py - RuntimeError: this file must never be imported Interrupted: 1 error during collection ``` Note this is complementary to #1809, not a replacement: `testpaths` stops collection outside `tests/`, but `tests/structs/test_agent_stream_token.py` lives *inside* `tests/` and still issues a billed live LLM call at import until that one is moved. ## 3. `tests.yml` runs unbounded ```diff test: runs-on: ubuntu-latest + timeout-minutes: 20 ``` The most recent run hit 17m35s and ended with `The runner has received a shutdown signal`. This bounds it. I did **not** do the other half of that item. Wiring provider secrets into `tests.yml`, or marking the roughly 40 test files that reach a live provider so they skip without credentials, is a real design decision about whether this workflow is meant to hit live providers at all, and it is much larger than a timeout. Tell me which way you want it and I will send that separately. ## 4. `RELEASE.yml` builds on Python 3.9 ```diff - - name: Set up Python 3.9 + - name: Set up Python 3.10 uses: actions/setup-python@v6 with: - python-version: "3.9" + python-version: "3.10" ``` `pyproject.toml` declares `python = ">=3.10,<4.0"`, so the release build was running on an interpreter the package excludes. ## Checks All three touched workflow files parse under `yaml.safe_load`, and `tests.yml`'s job now reports `timeout-minutes: 20`. No test for this one: these are workflow and config changes, and the honest verification is the next CI run on this PR. I use Claude Code to help me work through these and I check every claim against the files before opening anything. If you would rather have these as four separate PRs, say so and I will split it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by ayaangazaliFixes #1794 ## Problem `swarms/structs/agent.py:5535-5551`. The method named `tool_execution_retry` contains no retry: ```python except AgentToolExecutionError as e: logger.error( f"... Attempting to retry tool execution with 3 attempts" ) ``` The `except` block logs and falls off the end. `execute_tools` is called exactly once, `self.tool_retry_attempts` (default 3) is never read, and the exception is swallowed, so the caller cannot tell a tool failed. The log line even claims a retry is about to happen. Measured on master `829a6671`, patching `execute_tools` to raise: ``` tool_retry_attempts = 3 execute_tools calls = 1 (expected 3) propagated exception = None (expected the error to surface) ``` Both halves contradict the docstring directly above it, which promises "Maximum retry attempts are controlled by self.tool_retry_attempts (default: 3)" and "After all retries are exhausted, the exception is re-raised". ## Fix Same logic, restructured into the loop the docstring describes. Net **+18/-12 in one file**, and the early return for `None` removes a nesting level: ```python if response is None: logger.warning(...) return attempts = max(1, self.tool_retry_attempts or 1) for attempt in range(1, attempts + 1): try: self.execute_tools(response=response, loop_count=loop_count) return except AgentToolExecutionError as e: logger.error(f"... Attempt {attempt} of {attempts}") if attempt == attempts: raise ``` `max(1, ... or 1)` keeps a caller-supplied `0` or `None` from turning tool execution off entirely, which is what a bare `range()` would do. The log now reports the real attempt number instead of a hardcoded 3. **Behaviour change worth your call.** The re-raise is the "silently swallows" half of the issue and it is what the docstring specifies, but it does surface errors that master currently drops. Two of the three call sites, `agent.py:2600` and `:2693`, invoke this from inside their own `except Exception` handler as a fallback, so a tool that fails every attempt will now propagate out of the autonomous loop instead of being logged and stepped over. If you would rather keep it non-fatal, delete the two `if attempt == attempts: raise` lines and the retries still work. I can push that variant instead, just say which you prefer. ## Test One test appended to the existing `tests/structs/test_agent.py`, no new file: ```python with patch.object( agent, "execute_tools", side_effect=AgentToolExecutionError("tool blew up"), ) as execute: with pytest.raises(AgentToolExecutionError): agent.tool_execution_retry([{"function": {"name": "x"}}], loop_count=1) assert execute.call_count == 3 ``` It pins both halves at once, so either regression fails it: no retry gives `call_count == 1`, and swallowing gives `DID NOT RAISE AgentToolExecutionError`. With only `agent.py` reverted to master: ``` E Failed: DID NOT RAISE AgentToolExecutionError ``` Full-file check, master `agent.py` vs this branch, the only difference is this test: | | failed | passed | errors | |---|---|---|---| | master `829a6671` | 22 | 40 | 8 | | this branch | 21 | 41 | 8 | `black --check` and `ruff check` clean at line-length 70. I use Claude Code to help me work through these and I verify every claim against the source first. If I have read the intent wrong here, tell me and I will close it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by ayaangazaliFixes #1793 ## Problem `Agent.__init__` never assigns `self.executor`, but four methods submit to it: | Method | Behaviour on a fresh agent | |---|---| | `run_multiple_images` (`:5393`) | `AttributeError` — reached from the public `run(imgs=[...])` | | `talk_to_multiple_agents` (`:4442`) | `AttributeError` | | `run_concurrent` (`:3005`) | swallows it, returns `None` | | `run_concurrent_tasks` (`:3026`) | swallows it, returns `None` | The two `run_concurrent*` paths are the worse case — the caller gets a silent `None` instead of an error. The only assignment was inside a `with` block in `_reinitialize_after_load`: ```python # if not hasattr(self, "executor") or self.executor is None: with ContextThreadPoolExecutor(max_workers=os.cpu_count()) as executor: self.executor = executor ``` `__exit__` calls `shutdown(wait=True)`, so even after `load()` the stored pool is already dead and the next submit raises `RuntimeError: cannot schedule new futures after shutdown`. The `hasattr` guard that would have made this conditional is commented out. ## Fix Back it with a private field and build the pool on first access: ```python @property def executor(self) -> ContextThreadPoolExecutor: if self._executor is None: self._executor = ContextThreadPoolExecutor(max_workers=os.cpu_count()) return self._executor ``` `__init__` sets `self._executor = None`, and `_reinitialize_after_load` now just clears the field instead of building-and-shutting-down a pool, so a reloaded agent gets a fresh live one. Lazy rather than eager in `__init__` so agents that never run concurrently don't allocate `os.cpu_count()` threads — that would be a thread pool per Agent, and swarms build a lot of Agents. Verified: ``` lazy before use: _executor=None after access: ContextThreadPoolExecutor shutdown=False stable across access: True submit works: 42 after reload: _executor=None, new pool shutdown=False ``` I checked for external writers before converting the attribute to a read-only property — `grep -rn "\.executor" swarms/` outside `agent.py` only matches `advisor_swarm.py`'s unrelated `executor_agent` / `executor_model_name`. ## Tests `tests/structs/test_agent.py::TestExecutorLifecycle` — 3 tests, all fail on master with the errors from the issue: ``` AttributeError: 'Agent' object has no attribute 'executor' AttributeError: 'Agent' object has no attribute '_executor' RuntimeError: cannot schedule new futures after shutdown ``` They cover a usable pool on a fresh agent, laziness plus reuse of the same object, and a live pool after `_reinitialize_after_load()`. ## Full-file results | | failed | passed | errors | |---|---|---|---| | master `297ae6b7` | 20 | 38 | 8 | | this branch | 19 | 42 | 8 | Three of the four extra passes are the new tests. **The fourth I can't account for and am not claiming**: the pre-existing `TestAgentFeatures::test_agent_concurrent_execution` goes fail → pass. It's deterministic under pytest (master 6/6 fail, branch 6/6 pass, same test file both times, so the difference is the source change), but it makes live OpenAI calls and this machine has no `OPENAI_API_KEY`, and it passes standalone on master *and* this branch. I couldn't reproduce the mechanism outside pytest, so I'd treat that one as unexplained rather than as evidence this PR fixes it. Flagging it in case it means something to you. No test goes pass → fail. `black --check` and `ruff check` clean. Note: the red CI reproduces on `master` — `build` fails at `ModuleNotFoundError: No module named 'swarms'`, `test-main-features` at `poetry install --no-dev` (removed in Poetry 2.x). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by ayaangazaliFour structural defects across the workflow files, all present at `297ae6b7` (v14.0.0). Every non-trivial workflow is currently failing on master (`Run Tests`, `Python package`, `Test Main Features`, `Lint`, `Pyre`), so no job is gating merges today. Each fix below is one or two lines; grouping them into one issue since opening four separate ones for one-line YAML changes seems like noise. The `Python package` job additionally fails for a dependency reason tracked separately in #1792. ## 1. `test-main-features.yml` cds into a local macOS path on an ubuntu runner `.github/workflows/test-main-features.yml:22` declares `runs-on: ubuntu-latest`, and line 68 changes into a path that exists only on one developer's laptop: ```yaml 66 - name: Run Main Features Tests 67 run: | 68 cd /Users/swarms_wd/Desktop/research/swarms 69 poetry run python tests/test_main_features.py ``` The step can never succeed on a hosted runner. It is currently unreachable, because the job dies two steps earlier at line 54 — `poetry install --with test --no-dev` — since Poetry 2.x removed that flag: ``` ##[group]Run poetry install --with test --no-dev The option "--no-dev" does not exist ##[error]Process completed with exit code 1. ``` Fix: delete line 68, and replace `--no-dev` with `--only main,test` at line 54 (line 138 in the `test-coverage` job needs the same review). ## 2. `python-package.yml` runs bare `pytest`, collecting examples/ and scripts/ `.github/workflows/python-package.yml:42` runs `pytest` with no path, and `pyproject.toml`'s `[tool.pytest.ini_options]` (line 100) sets only `markers` — no `testpaths`. There is no `conftest.py` anywhere in the repo. pytest therefore collects from the repo root: | directory | collectible files | |---|---| | `tests/` | 65 | | `examples/` | 48 (29 `test_*.py` + 19 `*_test.py`) | | `scripts/` | 3 | | **total** | **116** | Confirmed in the run log: ``` ____ ERROR collecting examples/guides/850_workshop/test_agent_concurrent.py ____ _ ERROR collecting examples/guides/aop_examples/discovery/test_aop_discovery.py _ ``` Fix: scope the command to `pytest tests/`, or set `testpaths = ["tests"]` under `[tool.pytest.ini_options]` so bare `pytest` is correct everywhere. ## 3. `tests.yml` has no secrets and no timeout ``` $ grep -c "secrets\." .github/workflows/tests.yml 0 $ grep -n "timeout-minutes" .github/workflows/tests.yml (no match) ``` `tests.yml:31` runs the full `pytest tests/ -v` with zero provider credentials, so every test that reaches a live provider fails or hangs, and there is no `timeout-minutes` to bound it. The most recent run lasted 17m35s and ended with: ``` ##[error]The runner has received a shutdown signal. ##[error]The operation was canceled. ``` Fix: add `timeout-minutes: 20` to the job, and either wire in the same secrets `test-main-features.yml` uses (lines 58-64) or mark the provider-dependent tests so they skip without credentials. Roughly 40 test files reach a live provider and only 5 currently gate on an environment variable, so the skip-marking is the larger half of this. ## 4. `RELEASE.yml` builds on Python 3.9 while the package requires >=3.10 `.github/workflows/RELEASE.yml:23-26`: ```yaml - name: Set up Python 3.9 uses: actions/setup-python@v6 with: python-version: "3.9" cache: "poetry" ``` `pyproject.toml:59` declares `python = ">=3.10,<4.0"`. The interpreter that builds and publishes the wheel is below the package's own floor. Fix: bump to `"3.10"` to match the declared floor and the rest of the matrix. Related cleanup: `pyproject.toml:109` still sets `[tool.black] target-version = ["py38"]`, and the PyPI classifiers at line 54 list only Python 3.10 while the constraint allows up to 3.x. ## Environment swarms `main` @ `297ae6b7` (v14.0.0).
Proposed by Steve-Dusty`tool_execution_retry` is the tool-execution path of the main agent loop (`swarms/structs/agent.py:1573`, plus the autonomous-loop fallbacks at `:2601` and `:2694`). Its docstring promises `tool_retry_attempts` retries and a re-raise once they are exhausted. The body calls `execute_tools` exactly once and, on `AgentToolExecutionError`, logs and returns. Consequences: - `tool_retry_attempts` (constructor argument, default 3) has no effect on tool execution. It is read only for `MCPManager` at `:542`; it is never referenced inside `tool_execution_retry`. - When the handler fires, the failure is swallowed. `_run` continues to the next loop with no `Tool Executor` message in `short_memory`, so the model sees the tool as having produced nothing and proceeds as though it had succeeded. - The log line hardcodes "3 attempts" regardless of the configured value, and no attempt follows it. ## Root cause `swarms/structs/agent.py:5494-5510` — the entire body, under a 59-line docstring, and the end of the file: ```python try: if response is not None: self.execute_tools( response=response, loop_count=loop_count, ) else: logger.warning( f"Agent '{self.agent_name}' received None response from LLM in loop {loop_count}. " f"This may indicate an issue with the model or prompt. Skipping tool execution." ) except AgentToolExecutionError as e: logger.error( f"Agent '{self.agent_name}' encountered error during tool execution in loop {loop_count}: {str(e)}. " f"Full traceback: {traceback.format_exc()}. " f"Attempting to retry tool execution with 3 attempts" ) ``` No loop, no re-raise, no reference to `self.tool_retry_attempts`. The docstring's other claims are also inaccurate: - `:5446` — "After all retries are exhausted, the exception is re-raised." It is not. - `:5451` — "Other exceptions: Logs error and retries." Only `AgentToolExecutionError` is caught; everything else propagates. `AgentToolExecutionError` is additionally never raised anywhere in the framework — `grep -rn "raise AgentToolExecutionError" swarms/` returns nothing. `execute_tools` re-raises the underlying exception verbatim at `:5257`, so in practice the handler does not fire at all and real tool failures escape into `_run`'s broad `except Exception` retry loop, which re-runs the whole LLM call instead. ## Reproducer ```python from swarms import Agent from swarms.schemas.agent_errors import AgentToolExecutionError calls = [] a = Agent(agent_name="A", model_name="gpt-4.1", max_loops=1, tool_retry_attempts=5) def failing_execute_tools(response, loop_count): calls.append(loop_count) raise AgentToolExecutionError("simulated tool failure") a.execute_tools = failing_execute_tools print("tool_retry_attempts configured :", a.tool_retry_attempts) raised = False try: ret = a.tool_execution_retry(response=[{"function": {"name": "t"}}], loop_count=1) except Exception as e: raised = True ret = f"<raised {type(e).__name__}>" print("execute_tools invocations :", len(calls)) print("exception propagated to caller :", raised) print("return value :", ret) ``` Output on `main` @ `297ae6b7` (v14.0.0): ``` tool_retry_attempts configured : 5 execute_tools invocations : 1 exception propagated to caller : False return value : None ``` `tool_retry_attempts=5`, one call, no exception, and a log line announcing three attempts that never happen. ## Suggested change Implement the loop the docstring describes, catch the exception types that are actually raised, and re-raise once attempts are exhausted. `swarms/structs/agent.py:5494-5510`: ```python if response is None: logger.warning( f"Agent '{self.agent_name}' received None response from LLM in loop {loop_count}. " f"This may indicate an issue with the model or prompt. Skipping tool execution." ) return attempts = max(1, int(self.tool_retry_attempts or 1)) for attempt in range(1, attempts + 1): try: self.execute_tools( response=response, loop_count=loop_count, ) return except Exception as e: logger.error( f"Agent '{self.agent_name}' tool execution failed in loop {loop_count} " f"(attempt {attempt}/{attempts}): {e}\n{traceback.format_exc()}" ) if attempt == attempts: raise AgentToolExecutionError( f"Tool execution failed after {attempts} attempts in loop {loop_count}" ) from e ``` This makes `tool_retry_attempts` effective, stops the silent swallowing, and makes `AgentToolExecutionError` an exception the framework actually raises so callers can catch it. If swallowing is the intended behaviour, then the docstring at `:5442-5451` and `:5474-5477` should be corrected instead and `tool_retry_attempts` documented as MCP-only — but the current combination of a promise in the docstring, a log line claiming a retry, and no retry is the worst of the three. ## Environment swarms `main` @ `297ae6b7` (v14.0.0), Python 3.12, Linux.
Proposed by Steve-Dusty`Agent.__init__` never assigns `self.executor`, but four methods submit to it. The documented public path `Agent.run(task=..., imgs=[...])` raises `AttributeError: 'Agent' object has no attribute 'executor'` on a freshly constructed agent, so multi-image runs are unusable. | Method | Line | Behaviour | |---|---|---| | `run_multiple_images` | `swarms/structs/agent.py:5393` | raises `AttributeError`; reached from public `run(imgs=[...])` at `:4033` | | `talk_to_multiple_agents` | `:4442` | raises `AttributeError` | | `run_concurrent` | `:3005` | swallows it, returns `None` | | `run_concurrent_tasks` | `:3026` | swallows it, returns `None` | The two `run_concurrent*` methods are the worse case: they log and return `None`, so a caller receives a silent empty result rather than an error. ## Root cause The only assignment is inside a `with` block, so even after `load()` the executor is already shut down. `swarms/structs/agent.py:3354-3359`, in `_reinitialize_after_load`: ```python # Reinitialize executor if needed # if not hasattr(self, "executor") or self.executor is None: with ContextThreadPoolExecutor( max_workers=os.cpu_count() ) as executor: self.executor = executor ``` `__exit__` calls `shutdown(wait=True)` on block exit, so `self.executor` is a dead pool and any subsequent submit raises `RuntimeError: cannot schedule new futures after shutdown`. ## Reproducer ```python import traceback from swarms import Agent a = Agent(agent_name="A", model_name="gpt-4.1", max_loops=1) print("hasattr(agent, 'executor'):", hasattr(a, "executor")) # public path: Agent.run(imgs=[...]) -> run_multiple_images -> self.executor try: a.run_multiple_images(task="describe", imgs=["a.png", "b.png"]) except AttributeError: print(traceback.format_exc().strip().splitlines()[-1]) # _reinitialize_after_load() "fixes" it with an already-shut-down executor a._reinitialize_after_load() print("executor after load():", type(a.executor).__name__, "_shutdown =", a.executor._shutdown) try: a.run_multiple_images(task="describe", imgs=["a.png"]) except RuntimeError as e: print("RuntimeError:", e) ``` Output on `main` @ `297ae6b7` (v14.0.0): ``` hasattr(agent, 'executor'): False AttributeError: 'Agent' object has no attribute 'executor' executor after load(): ContextThreadPoolExecutor _shutdown = True RuntimeError: cannot schedule new futures after shutdown ``` The error fires before any network I/O. `run_concurrent` and `run_concurrent_tasks` on the same agent return `None` rather than raising. ## Suggested change Create the pool lazily so no threads are allocated for agents that never use it, and stop the `with` block from tearing it down. Add to the `Agent` class: ```python @property def executor(self) -> ContextThreadPoolExecutor: """Thread pool for the concurrent run paths, created on first use.""" if getattr(self, "_executor", None) is None: self._executor = ContextThreadPoolExecutor( max_workers=os.cpu_count() ) return self._executor ``` `swarms/structs/agent.py:3354-3359`: ```diff - # Reinitialize executor if needed - # if not hasattr(self, "executor") or self.executor is None: - with ContextThreadPoolExecutor( - max_workers=os.cpu_count() - ) as executor: - self.executor = executor + # Drop any executor restored from state; the property rebuilds it. + self._executor = None ``` If the property is unwanted, assigning `self.executor = ContextThreadPoolExecutor(max_workers=os.cpu_count())` directly in `__init__` also works — `ThreadPoolExecutor` does not spawn threads until the first `submit()`. Separately, `run_concurrent` (`:3011-3014`) and `run_concurrent_tasks` (`:3034-3035`) should re-raise after logging rather than returning `None`; a `None` return on failure is indistinguishable from a successful empty result. Note that `os.cpu_count()` is the wrong sizing for these network-bound paths — PR #1766 already made that change for `ConcurrentWorkflow`, and the same reasoning applies here. ## Environment swarms `main` @ `297ae6b7` (v14.0.0), Python 3.12, Linux.
Proposed by Steve-DustyThe built-in file tools exposed to the model in `max_loops="auto"` have no path confinement. A tool call with `file_path="../../../../etc/passwd"` or `file_path="/home/user/.ssh/id_rsa"` resolves and executes outside the agent workspace. This applies to `read_file`, `grep`, `list_directory`, `create_file`, `update_file` and `delete_file` — the model chooses `file_path` directly, so any prompt injection reaching the loop (a fetched web page, a file the agent was asked to summarize, a sub-agent's task string) becomes arbitrary local file read, overwrite or deletion under the host process's privileges. Related to but distinct from #1751. That issue asks for an approval/permission layer and lists a working-directory sandbox as an optional extra; its proposed default policy explicitly auto-approves `read_file`, `grep` and `list_directory`. Under the current implementation that default would auto-approve arbitrary filesystem *reads*. Path confinement is a separate, small fix that does not depend on any approval hook, and it is what makes #1751's default policy safe. ## Root cause Every file tool in `swarms/structs/autonomous_loop_utils.py` uses the same resolver. `read_file_tool`, lines 771-777: ```python try: # Resolve path - if relative, use agent workspace if not os.path.isabs(file_path): workspace_dir = agent._get_agent_workspace_dir() full_path = os.path.join(workspace_dir, file_path) else: full_path = file_path ``` Two holes: 1. `os.path.join(workspace, "../../../x")` is never normalized, so `..` segments escape. 2. The `else` branch takes an absolute path verbatim, with no check at all. Identical blocks appear in `create_file_tool` (lines 660-664), `update_file_tool` (719-723), `list_directory_tool` (823-827), `delete_file_tool` (892-896) and `grep_tool` (1133-1141). There is no `realpath`, no prefix/containment check and no symlink check anywhere in the file. All six are wired to model-controlled arguments at `swarms/structs/agent.py:2061-2079`. `swarms/tools/computer_use.py` already contains the correct primitive — `_check_realpath` (lines 312-331) does deny-list plus workspace containment on the resolved real path, and `_check_write_path` (334-365) additionally rejects symlink traversal — but `create_computer_use_tools` has zero call sites in `swarms/` outside the re-export in `swarms/tools/__init__.py`, so the autonomous loop gets none of it. ## Reproducer ```python import os, tempfile from swarms.structs.autonomous_loop_utils import read_file_tool BASE = tempfile.mkdtemp(prefix="swarms-traversal-") WORKSPACE = os.path.join(BASE, "agent_workspace", "agents", "worker-1") os.makedirs(WORKSPACE, exist_ok=True) OUTSIDE = os.path.join(BASE, "outside_workspace_secret.txt") open(OUTSIDE, "w").write("SECRET-OUTSIDE-WORKSPACE\n") class _Mem: def add(self, role, content): pass class StubAgent: verbose = False short_memory = _Mem() def _get_agent_workspace_dir(self): return WORKSPACE agent = StubAgent() print("workspace :", WORKSPACE) print("[1] relative ../../.:", repr(read_file_tool(agent, "../../../outside_workspace_secret.txt"))) print("[2] absolute path :", repr(read_file_tool(agent, "/etc/hostname"))) os.symlink(OUTSIDE, os.path.join(WORKSPACE, "innocent.txt")) print("[3] symlink followed:", repr(read_file_tool(agent, "innocent.txt"))) ``` Output on `main` @ `297ae6b7` (v14.0.0): ``` workspace : /tmp/swarms-traversal-o0bwykqc/agent_workspace/agents/worker-1 [1] relative ../../.: 'SECRET-OUTSIDE-WORKSPACE\n' [2] absolute path : 'DESKTOP-69LUQRR\n' [3] symlink followed: 'SECRET-OUTSIDE-WORKSPACE\n' ``` The write-side tools behave the same way; `create_file` / `update_file` / `delete_file` with a `../../../` path report success and the unnormalized path appears verbatim in the tool's own success message, e.g.: ``` Successfully created file: /tmp/.../agent_workspace/agents/worker-1/../../../created_outside_workspace.txt ``` That unresolved `..` in the returned string is the bug visible in the output: the path is never resolved, so nothing downstream can check it either. ## Suggested change Add one shared resolver in `swarms/structs/autonomous_loop_utils.py` and route all six tools through it, replacing each copy of the 5-line block: ```python def _resolve_in_workspace(agent: Any, path: str) -> str: """Resolve *path* against the agent workspace and refuse to leave it.""" workspace = os.path.realpath(agent._get_agent_workspace_dir()) candidate = ( path if os.path.isabs(path) else os.path.join(workspace, path) ) real = os.path.realpath(candidate) # normalizes '..' and resolves symlinks if real != workspace and not real.startswith(workspace + os.sep): raise ValueError( f"Path {path!r} resolves to {real}, which is outside the agent " f"workspace {workspace}." ) return real ``` Call sites become `full_path = _resolve_in_workspace(agent, file_path)`. The existing `except Exception` in each tool already converts the `ValueError` into a `tool_result` string, so the model sees the denial and can adapt rather than crash. `os.path.realpath` resolves symlinks, which closes case [3]. For the write tools, rejecting symlinked parents outright — as `_check_write_path` in `computer_use.py` already does — is stricter and preferable. An `allow_outside_workspace: bool = False` escape hatch on `Agent.__init__` would preserve today's behaviour for callers who deliberately want it, and would compose with the policy hook proposed in #1751. ## Environment swarms `main` @ `297ae6b7` (v14.0.0), Python 3.12, Linux.
Proposed by Steve-Dusty`run_agents_concurrently` returns list-mode results in **completion** order, but three callers pair that list positionally with the input agent list. Whenever agents do not finish in submission order — the normal case, since LLM latency varies with output length — each agent's answer is recorded under a **different agent's name**. This is silent. No exception, no warning, and the output is well-formed. The consensus agent in `MajorityVoting` and the aggregator in `aggregate` then reason over a transcript whose attributions are shuffled, so any per-agent judgement ("Agent-A's analysis was strongest") is meaningless. `AgentRearrange._run_concurrent_workflow` returns the scrambled mapping straight to the caller as `response_dict`. The same path misattributes failures: an agent that raises has its `Exception` appended in completion order too, so a crash is recorded against a healthy agent. Affected: `MajorityVoting.run`, `AgentRearrange._run_concurrent_workflow` (reached from `run` via the concurrent branch), `ma_blocks.aggregate`. ## Root cause `swarms/structs/multi_agent_exec.py:176-186` collects list-mode results with `as_completed`, which yields futures in finish order: ```python else: results = [] for future in concurrent.futures.as_completed( futures ): try: result = future.result() results.append(result) except Exception as e: results.append(e) return results ``` The docstring says so plainly at `multi_agent_exec.py:128`: ``` - Otherwise, the results list is in order of completion (not input order). ``` All three callers nonetheless pair it positionally. `swarms/structs/majority_voting.py:206-216` ```python output = run_agents_concurrently( agents=self.agents, task=self.conversation.get_str(), max_workers=os.cpu_count(), ) for agent, output in zip(self.agents, output): self.conversation.add( role=agent.agent_name, content=output, ) ``` `swarms/structs/ma_blocks.py:69-73` ```python results = run_agents_concurrently(agents=workers, task=task) # Zip the results with the agents for result, agent in zip(results, workers): conversation.add(content=result, role=agent.agent_name) ``` `swarms/structs/agent_rearrange.py:569-582` ```python results = run_agents_concurrently( agents=agents_to_run, task=self.conversation.get_str(), ) # Process results and update conversation response_dict = {} for i, agent_name in enumerate(agent_names): result = results[i] ``` `agents_to_run` is built by iterating `agent_names` in order, so `results[i]` is assumed to correspond to `agent_names[i]`. The function already has a correct mode, and the other four call sites use it — `mixture_of_agents.py:189`, `llm_council.py:427`, `auction_swarm.py:353` and `auction_swarm.py:429` all pass `return_agent_output_dict=True`, which keys by `agent_name`. Only the three sites above take the positional list. ## Reproducer No LLM calls. Stub agents with fixed latencies so completion order is deterministically inverted. ```python import time from swarms.structs.majority_voting import MajorityVoting class StubAgent: def __init__(s, n, a, d): s.agent_name, s.answer, s.delay = n, a, d def run(s, task=None, **kw): time.sleep(s.delay) return s.answer class StubConsensus: agent_name = "Consensus-Agent" streaming_on = False def run(self, task=None, **kw): return "(consensus stub)" agents = [ StubAgent("Agent-A", "A_ANSWER", 0.50), # slow StubAgent("Agent-B", "B_ANSWER", 0.05), # fast ] mv = MajorityVoting(agents=agents, max_loops=1, output_type="list") mv.consensus_agent = StubConsensus() # stubbed only to avoid an API call mv.run(task="ping") print("GROUND TRUTH: Agent-A -> 'A_ANSWER' Agent-B -> 'B_ANSWER'") print("AS RECORDED BY MajorityVoting:") for m in mv.conversation.conversation_history: if m["role"].startswith("Agent-"): print(f" role={m['role']!r:12} content={m['content']!r}") ``` Output on `main` @ `297ae6b7` (v14.0.0): ``` GROUND TRUTH: Agent-A -> 'A_ANSWER' Agent-B -> 'B_ANSWER' AS RECORDED BY MajorityVoting: role='Agent-A' content='B_ANSWER' role='Agent-B' content='A_ANSWER' ``` Both answers are attributed to the wrong agent. The same inversion drives the `ma_blocks` and `AgentRearrange` zips. ## Suggested change Make list mode preserve input order. `as_completed` buys nothing here: the function iterates every future and returns only once all of them have resolved, so there is no early-return or streaming benefit — iterating `futures` in submission order costs the same wall-clock time and makes the positional pairing the callers already assume actually correct. `swarms/structs/multi_agent_exec.py:176-186` ```python else: results = [] for future in futures: try: results.append(future.result()) except Exception as e: results.append(e) return results ``` and update the docstring at `multi_agent_exec.py:128`: ``` - Otherwise, the results list is in input order, aligned index-for-index with `agents`. ``` That fixes all three call sites at once. If the completion-ordered behaviour must be preserved for compatibility, the alternative is switching the three callers to `return_agent_output_dict=True`, matching `mixture_of_agents`, `llm_council` and `auction_swarm`. Note that is not a drop-in for `AgentRearrange`, which can run the same agent name twice in one concurrent group — keying by name would collapse those entries — so the order-preserving fix is preferable. Worth adding either way: a regression test that fans out stub agents with descending sleep durations and asserts `results == [a.answer for a in agents]`. ## Environment swarms `main` @ `297ae6b7` (v14.0.0), Python 3.12, Linux.
Proposed by Steve-DustyFixes #1746 ## Problem `dynamic_context_window` defaults to `True`, so every `get_str()` / `return_history_as_string()` on a transcript over `context_length` went through a **character-offset binary search**, calling `count_tokens` on a near-full copy of the transcript at each step: ```python left, right = 0, len(all_tokens) while left < right: mid = (left + right) // 2 test_tokens = count_tokens(all_tokens[mid:], self.tokenizer_model_name) ``` That is ~16–19 full tokenizations per read, and the cost grows with the transcript. Orchestrators call this inside their agent loops, so it multiplies by agents × loops. It also cuts at an arbitrary character offset, so the oldest surviving message is usually sliced mid-word. ## Fix Walk back from the newest message, keeping whole messages while they fit: ```python budget = self.context_length for position, message in enumerate(reversed(messages)): chunk = message if position == 0 else MESSAGE_SEPARATOR + message cost = count_tokens(chunk, self.tokenizer_model_name) if cost > budget: break budget -= cost kept += 1 ``` The loop stops as soon as the window is full, so the work is bounded by `context_length` instead of by transcript length — the cliff at ~16 messages disappears and the cost goes flat. Each message is measured together with the separator that will precede it. Splitting a string at any boundary can only keep or increase its token count, and the measured units concatenate to exactly the joined output, so the running total is a genuine **upper bound** — the result can never exceed `context_length`. It comes in slightly under (7814 vs 8192 in the benchmark below) because whole messages don't tile the window exactly. The character search is kept as `_truncate_to_context_length`, used only for the one case with no message boundary to cut on: a single message larger than the entire window. Without that fallback the newest message would be dropped and the read would return empty. I did **not** do the other two things the issue suggests. Tracking a running token total at `add()` time buys nothing once the walk is bounded by the window, and it adds state that has to stay correct across `add`/`delete`/`update`/`clear`/`load`. Making `_str_cache` append-only can't work while the head of the history is being trimmed. The `majority_voting.py` `max_workers=os.cpu_count()` note is a real point but is a concurrency change in an unrelated file — worth its own PR. ## Measurements Issue's reproduction — 2000-char messages, default `context_length=8192`: | messages | master | this branch | count_tokens calls (master → new) | |---|---|---|---| | 5 | 0.90 ms | 0.72 ms | 1 → 5 | | 10 | 0.95 ms | 1.07 ms | 1 → 10 | | **20** | **24.59 ms** | **1.71 ms** | 16 → 16 | | 40 | 28.87 ms | 1.76 ms | 17 → 16 | | 80 | 36.95 ms | 1.61 ms | 18 → 16 | | 120 | **47.01 ms** | **1.61 ms** | 19 → 16 | Flat past the window, ~29x at 120 messages. (Absolute numbers are lower than the issue's — different machine — but the shape and the call counts match exactly.) Call *count* is a poor proxy here; the work is characters tokenized: | messages | master | this branch | |---|---|---| | 30 | 56,902 chars | 2,738 chars | | 240 | 287,386 chars | 2,743 chars | Master scales with the transcript; this stays flat. Many-small-messages case, which is the one a per-message walk could plausibly regress: | shape | master | this branch | |---|---|---| | 2000 × 10 chars | 36.00 ms | 3.44 ms | | 5000 × 10 chars | 62.07 ms | 2.06 ms | | 500 × 10 chars | 22.44 ms | 1.72 ms | | 2000 × 100 chars | 35.11 ms | 0.97 ms | No regression — the walk stops at the window there too. The only case that gets marginally slower is a short under-budget history (10 messages: 0.95 → 1.07 ms), where master needed one tokenization and the walk does one per message. ## Correctness A randomized check over 40 conversations (context lengths 50/200/1000/8192, 1–60 messages, timestamps on and off) confirms the output never exceeds `context_length` and the newest message survives intact whenever it fits on its own. ## Tests `tests/structs/test_conversation.py` — 6 tests. Two fail on master: - **`test_dynamic_chunking_keeps_whole_messages`** — with `time_enabled`, every complete message starts with `[timestamp]`. On master the first line is `' word word word …'`, a mid-message fragment. - **`test_dynamic_chunking_work_is_bounded_by_the_window`** — an 8x longer transcript must not tokenize proportionally more. On master: `assert 287386 < (56902 * 2)`. The other four pin the behaviour that must *not* change: stays within `context_length`, short history returned verbatim, a single oversized message trimmed rather than dropped, empty history returns `""` instead of hitting the newest-message fallback. `tests/structs/test_conversation.py`: 57 passed. `tests/structs/test_agent.py`: 20 failed / 38 passed / 8 errors — identical to master, all pre-existing. `black --check` and `ruff check` clean. Note: the unrelated CI failures on this PR reproduce on `master` — the `Python package` workflow never installs `swarms` itself, so every job fails at `ModuleNotFoundError: swarms`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by ayaangazaliFixes #1718 Rebased onto master `06413ebc`, and the body below is corrected: an earlier revision of this branch called the new method `truncate_to()`, the branch now uses `checkpoint()` / `rollback()` because the rollback has to rewind MEMORY.md too. Details at the bottom. ## Problem `Agent._run` adds the LLM response to `short_memory` **before** the attempt is marked successful: ```python while attempt < self.retry_attempts and not success: try: response = self.call_llm(...) self.short_memory.add(role=self.agent_name, content=response) # <-- write ... arguments = json.loads(tool_call["function"]["arguments"]) # can raise self.mcp_tool_handling(...) # can raise success = True except (...): attempt += 1 # re-runs the whole block ``` The steps between the write and `success = True` are not individually guarded. If one raises, the broad `except` retries the entire block, calling the model again and appending a **second** assistant message for a single reasoning step. The failed attempt's message is never rolled back. Result: duplicate, often contradictory assistant turns feeding every later loop, plus the wasted tokens. Only `attempt-i: model call ok -> memory.add -> post-add step raises -> retry succeeds` triggers it, which is why happy-path tests miss it. ## Fix Snapshot the conversation before the retry loop and restore it in the `except`, so every attempt starts from the same state. **9 lines in `agent.py`**, 6 of them the comment: ```python memory_checkpoint = self.short_memory.checkpoint() while attempt < self.retry_attempts and not success: try: ... except (...) as e: self.short_memory.rollback(memory_checkpoint) ``` `Conversation.checkpoint()` and `.rollback()` are added for this. They live on `Conversation` rather than reaching into `agent.py` because two pieces of state have to move together: ```python def checkpoint(self) -> Tuple[int, int]: return (len(self.conversation_history), memory_md_size) def rollback(self, checkpoint: Tuple[int, int]): del self.conversation_history[length:] self._str_cache = None ... os.truncate(self.memory_md_path, memory_md_size) ``` Three things that would each be a bug if left out: - **`_str_cache = None`.** A stale cached string keeps feeding the dropped turn into the next prompt even though the list is correct. - **MEMORY.md.** The log is append-only and is replayed into the next process by `_preload_memory_md`, so a rolled-back message would otherwise come back after a restart. Rewinding to the recorded byte size removes exactly the blocks written since the snapshot, under the existing `_memory_md_lock`. - **Rolling back tool-result messages too**, not just the assistant turn, since a failed attempt may have appended several. I kept the retry semantics as they are. Moving the `add` after `success = True` (suggestion 1 in the issue) would reorder the history, since handoff and tool-result messages are appended in between. Wrapping the post-add steps in their own `try` (suggestion 3) would silently swallow those errors. Rollback changes neither. ## Tests Three tests appended to the existing `tests/structs/test_agent.py`, no new file. All three fail with both source files reverted to master: ``` test_retry_does_not_duplicate_the_response assert [[{'function'...final answer'] == ['final answer'] test_exhausted_retries_leave_no_partial_turns assert [[{'function'...alid json'}}]] == [] test_rollback_also_rewinds_memory_md AttributeError: 'Conversation' object has no attribute 'checkpoint' ``` They cover the three distinct paths: retry-then-succeed, every-attempt-fails, and the on-disk rewind. The cache-invalidation check is folded into the first as one extra assert rather than its own test. Full-file check, both source files at master vs this branch, the only difference is these three: | `tests/structs/test_agent.py` | failed | passed | errors | |---|---|---|---| | master `06413ebc` | 24 | 41 | 8 | | this branch | 21 | 44 | 8 | `tests/structs/test_conversation.py` is untouched by the change: **51 passed**. `black --check` and `ruff check` clean at line-length 70. ## What changed in this update - **Rebased** onto `06413ebc`. The conflict was the usual end-of-file collision, #1799's `TestToolsListIsolation` landing where my class sits. Master's class is kept intact. - **Tests trimmed from four to three**, 15 fewer lines. The dropped one was a direct unit test of `rollback()` on a bare `Conversation`; its cache assertion moved into the first test, where it also proves the agent path. - **Body corrected** to describe `checkpoint()` / `rollback()` and the MEMORY.md rewind, which the old `truncate_to()` text predated. ## Overlap you should know about #1720 by @TashfikS targets the same issue with the same rollback approach, opened before this one. It is currently CONFLICTING. I am not asking you to prefer mine: if you would rather take theirs, close this and I will happily send the MEMORY.md and `_str_cache` pieces as a follow-up on top, since those are the two parts a length-only rollback misses. I use Claude Code to help me work through these and I verify each claim against a real run first. If the duplicate turn is intentional, tell me and I will close it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Proposed by ayaangazali`mcp` 2.0.0 (2026-07-28) is a breaking rewrite of the SDK. `swarms` cannot import under it at all today. The immediate fix is to bound the dependency to `<2` so deploys stop crashlooping — tracked separately — but that bound is a holding action: it freezes the framework on the 1.x line and leaves 2.x users unable to install `swarms` at all. This issue covers the actual migration. ## What breaks Only two call sites, out of the full `mcp` surface `swarms` touches: **1. `mcp.server.fastmcp` was deleted** — `swarms/structs/aop.py:16` The module is gone in 2.0.0, not merely relocated. The apparent successor is `MCPServer`, exported from the new `mcp.server.mcpserver` module alongside `ToolBinding`, `ResourceBinding`, `Context`, `Extension`, and friends. I have not verified how far its API matches `FastMCP` — the surrounding names suggest a redesigned registration model rather than a rename, so this needs a real read of the 2.0 docs before anyone estimates it. This is the larger half of the work. `AOP` is built on `FastMCP` throughout: - `self.mcp_server = FastMCP(...)` — `aop.py:682` - 19 references to `mcp_server` in the class - 12+ tool registrations via the `@self.mcp_server.tool(...)` decorator — `aop.py:985`, `:1749`, `:1809`, `:1853`, `:1913`, `:1943`, `:2051`, `:2103`, `:2119`, `:2139`, `:2159`, `:2184` - a lifespan type parameter `[FastMCP[LifespanResultT]]` — `aop.py:608` **2. `streamablehttp_client` was renamed** — `swarms/tools/mcp_manager.py:1456` `mcp.client.streamable_http` now exports `streamable_http_client` (snake_case). The old name is not aliased; the module raises `AttributeError`. Signature compatibility still needs checking — `swarms` passes `url`, `headers`, `timeout`, `sse_read_timeout`, `auth`, with the two timeouts as `timedelta`. Unlike the AOP break this import is lazy, inside `_http_transport()`, so it only fires on the first streamable-HTTP MCP connection rather than at startup. ## What does *not* break Worth stating explicitly, because it bounds the work — everything else `swarms` imports from `mcp` is unchanged in 2.0.0, verified against a clean venv: `ClientSession` · `mcp.types.Tool` · `sse_client` · `stdio_client` · `StdioServerParameters` · `OAuthClientProvider` · `OAuthClientMetadata` · `OAuthToken` · `OAuthClientInformationFull` · `AuthSettings` · `LifespanResultT` · `TransportSecuritySettings` So the stdio and SSE transports, the whole OAuth path, and the auth/transport-security settings all carry over untouched. The migration is genuinely confined to AOP's server construction plus one client function rename. ## Open question: support both, or move the floor? Two viable shapes, and this should be decided before implementation starts: 1. **Support 1.x and 2.x together** — keep the `<2` bound off, resolve `FastMCP` vs `MCPServer` and the client-function name at import time behind a small compat shim. Keeps existing users working, at the cost of a shim in `aop.py` that has to survive both APIs. Only viable if `MCPServer` is close enough to `FastMCP` that one code path can drive both. 2. **Move the floor to 2.x** — a clean cut, `mcp>=2`, no shim. Simpler code, but a breaking change for `swarms` itself and it needs a minor-version bump and a note in the release. If `MCPServer` turns out to be a redesign rather than a rename, option 1 gets expensive fast and option 2 is probably right. ## Verification Neither break has CI coverage — nothing in the test suite installs a clean resolver and imports the package, and `tests/tools/mcp_test_server.py:50` calls `mcp.streamable_http_app()`, which is itself 1.x-era API and will need review as part of this. Whichever shape is chosen, the migration should land with a test matrix that actually installs both major versions and imports `swarms`, otherwise the next SDK release repeats this. --- Blocked by #1780, which stops the bleeding by bounding the dependency to `<2`. This issue is what removes that bound.
Proposed by Steve-Dusty`swarms/structs/social_algorithms.py` imports nothing from `swarms.telemetry.otel`. `SocialAlgorithms` is public API (`from swarms import SocialAlgorithms`). ## Missing | Element | Instrumented files using it | `social_algorithms.py` | |---|---|---| | `capture_init(self)` | 18 | missing | | `@trace_run` on `run()` | 18 | missing | - `SocialAlgorithms.__init__` — `social_algorithms.py:116` - `SocialAlgorithms.run()` — `social_algorithms.py:379` - `SocialAlgorithms.run_async()` — `social_algorithms.py:588` ## Two entry points, not one `run_async()` at `:588` is a second public entry point that wraps `run()` via `asyncio.to_thread`. Whichever approach is taken, both paths should end up traced — instrumenting only `run()` leaves async callers invisible. The class already tracks its own communication steps (`CommunicationStep`, `social_algorithms.py:15`) and returns a `SocialAlgorithmResult` (`:27`). Those are a natural source of span attributes, so this harness can produce a more informative trace than a bare `@trace_run` would give. No `ThreadPoolExecutor` in this file, so there is no executor swap. ## Known pre-existing bug in the async path Flagging so it is not mistaken for a regression introduced by this work: `run_async()` is currently unusable for an unrelated reason. `social_algorithms.py:298` installs a `SIGALRM` handler inside `run()`, which raises `signal only works in main thread of the main interpreter` whenever `run()` executes on a worker thread — which is exactly what `run_async()` does. This is true on `master` today and is independent of telemetry, but it means the `run_async()` path cannot be end-to-end verified until it is fixed. Worth a separate issue. ## Suggested change ```python from swarms.telemetry.otel import capture_init, trace_run ``` - call `capture_init(self)` as the last line of `__init__` - decorate `run()` with `@trace_run` - ensure `run_async()` at `:588` is covered too Follow the pattern in `swarms/structs/swarm_router.py` (`capture_init` at `:405`, `@trace_run` at `:896`). ## Context 19 of 61 files in `swarms/structs/` are instrumented; 25 uninstrumented multi-agent harnesses remain. Full audit in `HARNESSES.md`. ## Verification No CI coverage asserts instrumentation in `swarms/structs/`, so this will not be validated automatically. Verify manually against a collector; note the `run_async()` caveat above.
Proposed by kyegomez`swarms/structs/auto_swarm_builder.py` imports nothing from `swarms.telemetry.otel`. `AutoSwarmBuilder` is public API (`from swarms import AutoSwarmBuilder`). ## Missing | Element | Instrumented files using it | `auto_swarm_builder.py` | |---|---|---| | `capture_init(self)` | 18 | missing | | `@trace_run` on `run()` | 18 | missing | - `AutoSwarmBuilder.__init__` — `auto_swarm_builder.py:283` - `AutoSwarmBuilder.run()` — `auto_swarm_builder.py:805` - `AutoSwarmBuilder.batch_run()` — `auto_swarm_builder.py:724` ## The specific gap: a split trace `AutoSwarmBuilder` delegates execution to `SwarmRouter` (`auto_swarm_builder.py:12`), which **is** instrumented (`capture_init` at `swarm_router.py:405`, `@trace_run` at `:896`). So the delegated run already produces spans. What is missing is the parent. The LLM calls that generate the agent specifications — the build phase, via `LiteLLM` at `auto_swarm_builder.py:13` — are untraced, and the `SwarmRouter` spans have no `AutoSwarmBuilder` span to attach to. One logical operation currently shows up as an untraced build followed by a detached, separately-rooted swarm run. This makes it hard to answer the question users actually have about this class: how much of the wall-clock went to *deciding* the swarm versus *running* it. No `ThreadPoolExecutor` in this file, so there is no executor swap. ## Suggested change ```python from swarms.telemetry.otel import capture_init, trace_run ``` - call `capture_init(self)` as the last line of `__init__` - decorate `run()` with `@trace_run` - consider `batch_run()` at `:724` as well, since it is a separate public entry point ## Context 19 of 61 files in `swarms/structs/` are instrumented; 25 uninstrumented multi-agent harnesses remain. Full audit in `HARNESSES.md`. ## Verification No CI coverage asserts instrumentation in `swarms/structs/`, so this will not be validated automatically. Verify manually that the existing `SwarmRouter` spans nest under the new `AutoSwarmBuilder.run` span rather than rooting separately.
Proposed by kyegomez`swarms/structs/self_moa_seq.py` imports nothing from `swarms.telemetry.otel`. `SelfMoASeq` is public API (`from swarms import SelfMoASeq`). ## Missing | Element | Instrumented files using it | `self_moa_seq.py` | |---|---|---| | `capture_init(self)` | 18 | missing | | `@trace_run` on `run()` | 18 | missing | - `SelfMoASeq.__init__` — `self_moa_seq.py:22` - `SelfMoASeq.run()` — `self_moa_seq.py:258` ## Why this one is worth tracing `SelfMoASeq` runs a sequential sample-and-aggregate loop, so a single `run()` fans out into many LLM calls over multiple rounds. Without a span for the run there is nothing tying those calls together, and no per-run latency or token attribution — the case where a trace is most useful. No `ThreadPoolExecutor` in this file, so there is no executor swap and no context-propagation problem. `capture_init` + `@trace_run` is the whole change. Note the class extends `SerializableMixin` (`self_moa_seq.py:7`) rather than a swarm base class, so it inherits no instrumentation from a parent. ## Suggested change ```python from swarms.telemetry.otel import capture_init, trace_run ``` - call `capture_init(self)` as the last line of `__init__` - decorate `run()` with `@trace_run` Follow the pattern in `swarms/structs/swarm_router.py` (`capture_init` at `:405`, `@trace_run` at `:896`). ## Context 19 of 61 files in `swarms/structs/` are instrumented; 25 uninstrumented multi-agent harnesses remain. Full audit in `HARNESSES.md`. ## Verification No CI coverage asserts instrumentation in `swarms/structs/`, so this will not be validated automatically. Verify manually against a collector.
Proposed by kyegomez`swarms/structs/spreadsheet_swarm.py` imports nothing from `swarms.telemetry.otel`. `SpreadSheetSwarm` is public API (`from swarms import SpreadSheetSwarm`). ## Missing | Element | Instrumented files using it | `spreadsheet_swarm.py` | |---|---|---| | `capture_init(self)` | 18 | missing | | `@trace_run` on `run()` | 18 | missing | - `SpreadSheetSwarm.__init__` — `spreadsheet_swarm.py:49` - `SpreadSheetSwarm.run()` — `spreadsheet_swarm.py:274` - `SpreadSheetSwarm.run_from_config()` — `spreadsheet_swarm.py:185` ## Partial coverage already exists — no executor change needed Worth stating so this is not over-scoped: agent execution is delegated to `run_agents_with_different_tasks` from `swarms/structs/multi_agent_exec.py` (`spreadsheet_swarm.py:8-10`), which **already uses `ContextThreadPoolExecutor`**. Context propagation across threads is therefore not broken here. The gap is narrower than in harnesses holding a raw pool: agent-level spans exist, but there is no swarm-level span for them to attach to, so a `SpreadSheetSwarm` run appears as a flat set of agent spans with no parent describing the run. `run_from_config()` is a second public entry point and should be considered alongside `run()`. ## Suggested change ```python from swarms.telemetry.otel import capture_init, trace_run ``` - call `capture_init(self)` as the last line of `__init__` - decorate `run()` with `@trace_run` Follow the pattern in `swarms/structs/swarm_router.py` (`capture_init` at `:405`, `@trace_run` at `:896`). ## Context 19 of 61 files in `swarms/structs/` are instrumented; 25 uninstrumented multi-agent harnesses remain. Full audit in `HARNESSES.md`. ## Verification No CI coverage asserts instrumentation in `swarms/structs/`, so this will not be validated automatically. Verify manually that agent spans from `multi_agent_exec` nest under the new `SpreadSheetSwarm.run` span.
Proposed by kyegomez`swarms/structs/model_router.py` imports nothing from `swarms.telemetry.otel`. `ModelRouter` is public API (`from swarms import ModelRouter`) and is the highest-priority gap of this set, because it has **two problems rather than one**. ## 1. No spans | Element | Instrumented files using it | `model_router.py` | |---|---|---| | `capture_init(self)` | 18 | missing | | `@trace_run` on `run()` | 18 | missing | - `ModelRouter.__init__` — `model_router.py:179` - `ModelRouter.run()` — `model_router.py:273` - `ModelRouter.batch_run()` — `model_router.py:286` ## 2. Raw `ThreadPoolExecutor` breaks trace continuity `model_router.py:3` imports `from concurrent.futures import ThreadPoolExecutor`, used at `:352`. A plain pool does **not** propagate OpenTelemetry context across the thread boundary; `ContextThreadPoolExecutor` (`swarms/telemetry/otel.py:406`) exists for exactly this. This is strictly worse than being untraced. Spans created inside those worker threads detach from the parent and surface as orphan traces **even when the caller is fully instrumented** — so this corrupts other components' traces, not just its own. Worth fixing independently of, and ahead of, the span work. It is a one-line import swap. ## Suggested change ```python from swarms.telemetry.otel import ( ContextThreadPoolExecutor, capture_init, trace_run, ) ``` - call `capture_init(self)` as the last line of `__init__` - decorate `run()` with `@trace_run` - replace `ThreadPoolExecutor` with `ContextThreadPoolExecutor` at `:352` Follow the pattern in `swarms/structs/swarm_router.py` (`capture_init` at `:405`, `@trace_run` at `:896`). ## Context 19 of 61 files in `swarms/structs/` are instrumented; 25 uninstrumented multi-agent harnesses remain. Full audit in `HARNESSES.md`. ## Verification There is no CI coverage asserting instrumentation anywhere in `swarms/structs/`, so this will not be validated automatically. Spans should be checked manually against a collector — in particular that agent spans emitted from the pool at `:352` attach to the `ModelRouter.run` span rather than appearing as orphans.
Proposed by kyegomezCloses #1754. Also removes the import-time network fetch traced in #1739 (finding c of my comment there) and the bulk of the 1.6s measured in #1738. ## Measured | | master | this branch | |---|---|---| | `import swarms` (cold) | ~2.2s | **~0.3–0.6s** | | `litellm` in `sys.modules` after import | True | **False** | | `mcp` in `sys.modules` | True | **False** | | `openai` in `sys.modules` (pulled by litellm) | True | **False** | | network egress at import | fetches litellm's model-cost map from `raw.githubusercontent.com` | **none** — verified with `socket.socket.connect` monkeypatched to raise | First use is unchanged: constructing a `LiteLLM` (or running any agent) binds litellm then; building an `AOP` or connecting an MCP server imports mcp then. ## The import chain, for the record ``` swarms/__init__ → telemetry.bootup → utils.disable_logging → swarms.utils/__init__ → dynamic_context_window → litellm_tokenizer → litellm (1.14s, + cost-map fetch) ``` and independently `structs.agent` / `tools.mcp_manager` / `structs.aop` re-imported both. litellm loads once, so **every** eagerly-loaded module's top-level import had to move — deferring only the tokenizer would have changed nothing. ## What was done, per module **`litellm_wrapper` and `context_compressor` — patch-compatible lazy binding.** These two are patched by existing tests as *module attributes* (`patch.object(litellm_wrapper, "completion", ...)` in `tests/structs/test_agent.py`, `patch("swarms.agents.context_compressor.completion")` five times in `tests/agents/test_context_compressor.py`). A function-local `from litellm import completion` would silently bypass those patches, so instead the names live as module globals initialised to `None` and are bound on first use: ```python global litellm, completion, supports_vision if completion is None: from litellm import completion as _completion completion = _completion ``` The `is None` guard is the load-bearing part: a mock installed by `patch` is non-`None`, so a later construction can never overwrite an active patch — regardless of whether the patch was applied before or after the first bind. **`agent.py` — the hidden one.** `reasoning_effort: Literal[get_reasoning_efforts()] = "medium"` in `Agent.__init__`'s signature runs at **class-definition time**, and that function introspects `litellm.completion`'s signature — forcing the full import the moment `swarms.structs.agent` loads. A module-level AST scan doesn't catch it, which is presumably why it survived. Since `Agent` is a plain class, the `Literal` is never enforced at runtime — it is documentation that cost the entire litellm import. It now uses the static `REASONING_EFFORTS` tuple that `get_reasoning_efforts.py` already maintains as its fallback for exactly this set (litellm 1.76.x's levels). `get_reasoning_efforts()` itself is untouched for any caller that wants the introspected set after litellm loads. **Six modules — plain function-local imports.** `litellm_tokenizer` (`encode` in `count_tokens`, `model_list` in `get_supported_models`), `agent_loader` (model validation), `agent_router` / `tree_swarm` (`embedding` calls), `llm_manager` (the three `supports_*` capability checks), `agent.py` (`model_list` / `get_max_tokens` / `supports_function_calling`, all used only in `reliability_check`). None of these names are patched anywhere in the test suite — checked before choosing the simpler form. **Exceptions — deletion, not deferral.** `agent.py` and `llm_manager.py` imported `AuthenticationError` / `BadRequestError` / `InternalServerError` for their `except` tuples. Every one of those tuples also contained `Exception`, which subsumes them — so the names were dead weight. Removed from the tuples; behaviour is byte-identical and the litellm exceptions still propagate to callers through the `raise`. **mcp — annotations vs runtime.** `mcp_manager` and `aop` used mcp types in function signatures (evaluated at `def` time) and constructed `ClientSession` / `FastMCP` at runtime. Both files gain `from __future__ import annotations` so the signatures stop needing real classes, `TYPE_CHECKING` imports keep the annotations meaningful to tooling, and the three runtime sites (`ClientSession` construction, the `isinstance(tool, MCPTool)` check, `FastMCP` construction) import locally. ## Tests Two subprocess-isolated regression tests in `tests/test___init__.py` (the file existed and was empty — no new file). Subprocess because the test runner's own `sys.modules` is already polluted and would mask a regression; the probe env sets `LITELLM_LOCAL_MODEL_COST_MAP=True` so the test never touches the network: - `test_import_swarms_defers_litellm_and_mcp` — litellm, mcp, openai all absent after `import swarms` - `test_litellm_binds_on_first_llm_construction` — deferral doesn't break first use Cross-applied to master: **both fail**, as expected. On this branch: both pass. Regression sweep, all offline: `tests/test___init__.py`, `test_context_compressor.py` (the 5 completion-patching tests), `test_llm_manager.py`, `test_ssrf_url_guard.py`, `TestLLMArgsAndHandling` (the completion-patching tests merged in #1769) — **143 passed**. `test_graph_workflow.py` 51 passed / 11 skipped. `test_mcp_manager.py` has 7 pre-existing errors ("MCP test server never started" — a live-server fixture); **identical 7 on master**, 81 others pass on both. `black --check` (line-length 70) and `ruff check` clean on all 11 files. ## Not covered, deliberately - `swarms/cli/main.py` also imports litellm at module level, but it is the console-script entry point and not loaded by `import swarms`; deferring there is cosmetic and left out. - Full lazy loading of swarms' own submodules (#1755) is a different, larger change — this PR only removes the third-party heavyweights, per #1754's scope. - `bootup()` still creates `./agent_workspace/` in the importing process's cwd. That is the remaining import side effect from #1739's thread; separable, happy to do it next.
Proposed by ayaangazaliCloses #1750. ## The asymmetry, verified on master User tools in the autonomous loop go through `execute_function_calls_from_api_response`, which runs them in a thread pool (`base_tool.py`, `max_workers=4`). Every **built-in** planning tool runs one at a time in a Python `for` loop (`agent.py`, the `planning_tool_handlers` dispatch). The built-ins hit hardest are exactly the ones the issue names — `read_file`, `grep`, `list_directory`: I/O-bound, mutation-free, and the calls a model fires several of in one response. Five `read_file` calls cost 5× the latency. ## Change The issue's proposed split — read-only concurrent, mutating ordered, control-flow last — implemented with one conservative tightening: only **consecutive** read-only calls are batched. - A new `READONLY_PLANNING_TOOLS = frozenset({"read_file", "grep", "list_directory"})` lives in `autonomous_loop_utils.py` next to the other loop constants. - In the execution loop, read-only built-ins are buffered instead of executed inline. The buffer flushes through a `ContextThreadPoolExecutor` (`max_workers` capped at 4, matching the user-tool path) at three points: when any mutating or control-flow tool appears, at the end of the response, and results are always recorded into memory **in the original call order**. Why "consecutive" rather than partitioning the whole response: for `[read A, create_file B, read C]`, running both reads before the write would let C observe pre-B state when the model asked for post-B. Batching only unbroken runs of reads preserves the exact read/write ordering the response declared, and still captures the dominant pattern (a burst of reads in one response). A full partition can come later if someone shows a response shape that needs it. Ordering guarantees, each load-bearing: - **read vs write** — any mutating tool flushes the buffer *before* it executes, so a read never crosses a write in either direction. - **control flow** — `subtask_done` and `complete_task` also flush first, so the mid-response `return self._generate_final_summary(...)` on `complete_task` and the `break` on `subtask_done` can never strand buffered, unexecuted reads. - **result order** — memory entries are written from the buffer order, not completion order, so the transcript the model sees next iteration is deterministic. - **user tools** — unchanged; they already executed after the whole built-in pass, and still do. `selected_tools` filtering happens before the dispatch branch, so a read tool the caller disabled never enters the batch. One thread-safety note since the read handlers append log lines to `short_memory` internally: `Conversation` list appends are GIL-atomic and the MEMORY.md file write is behind `_memory_md_lock`, so concurrent reads interleave only those log lines — same content, order among concurrent peers unspecified. The actual `{name} result: ...` entries are written afterwards in call order. This matches the user-tool path, where handlers already run concurrently. ## Exceptions Unchanged semantics: the serial loop let a handler exception propagate to the subtask-level `except`; `future.result()` re-raises the same way. In practice all three handlers catch internally and return error strings. ## Tests Two tests added to the existing `TestAgentToolUsage` class in `tests/structs/test_agent.py` (no new file), fully offline: - `test_readonly_planning_tools_run_concurrently_in_order` — three handlers sleeping 0.2s each complete in < 0.44s (serial would be 0.6s), results recorded in call order, buffer cleared. - `test_readonly_planning_tools_single_call_and_empty_batch` — the single-call path skips the pool, the empty flush writes nothing. File still collects all 68 tests; `black --check` (line-length 70) and `ruff check` clean. Diff: +171 across 3 files, no behaviour change for mutating/control-flow/user tools.
Proposed by ayaangazaliPart 1 of #1757 — stop failing silently. Part 2 (executing cycles as cycles) needs conditional edges per the issue and is not attempted here. ## Reproduced on master ```python wf.add_edge("a", "b"); wf.add_edge("b", "c"); wf.add_edge("c", "a") wf.compile() # execution plan: [['a', 'b', 'c']] <- one parallel layer # run(): a, b, c each ran once, concurrently; b never sees a's output ``` `topological_generations()` hits the cyclic case and falls back to a Kahn layering that tolerates cycles, so the cycle collapses into one layer. The only signals were: - `validate()`: `"Found cycles in workflow"` filed under **warnings** - `compile()`: the same text logged **only when `verbose=True`** (`_fast_validate` warnings are gated on verbose; default runs print `compiled successfully`) Nothing stated the flattening or that the declared edge ordering was discarded. ## Change **`_fast_validate` (compile-time)** — cycles become an **error** naming the affected nodes and the consequence. `compile()` already logs errors unconditionally, so a default (non-verbose) run now prints: ``` GraphWorkflow compile: validation found 1 error(s) — Found cycle(s) involving nodes ['a', 'b', 'c']. Cycles are not executed as loops: these nodes are flattened into a single parallel layer, run once concurrently, and the edge ordering between them is ignored. Break the cycle, or re-run the whole graph iteratively with max_loops. ``` The nodes are found with a new `_nodes_on_cycles` helper — an O(V+E) Kahn peel over the adjacency maps `compile()` already builds (peel in-degree-0 nodes; whatever survives is on a cycle). It replaces the previous `is_dag()` backend call in `_fast_validate`, so this is the same asymptotic cost and one fewer backend call, and it names the nodes rather than just answering yes/no. Nodes off the cycle are not named: for `a→b→c→a` plus `d→e`, the error lists exactly `['a','b','c']` (covered by a test). **`validate()`** — cycles move from `warnings` to `errors` with the same explicit message, `is_valid=False`, and `result["cycles"]` still carries the enumerated cycles. `validate(raise_on_error=True)` now refuses a cyclic graph. **`has_serious_warnings`** — drops its `"cycle"` substring match. Cycles are errors now, and the old match also caught `"Could not check for cycles: <e>"`, which wrongly invalidated a workflow when cycle *detection* failed rather than when a cycle existed. `compile()` still never raises (its documented contract); strict enforcement remains `validate(raise_on_error=True)`. ## Behaviour change, stated plainly `validate()` on a cyclic graph previously returned `is_valid=False` with the cycle in `warnings`; it now returns `is_valid=False` with the cycle in `errors`, and `raise_on_error=True` raises where it already raised before (via the serious-warnings path). The flag flips for one narrow case: a graph where cycle **detection throws** (backend error) was previously marked invalid by the substring match and is now a warning only. Acyclic graphs are unaffected — the full existing suite passes unchanged. ## Acceptance criteria from the issue - [x] a cyclic graph either executes as a loop or raises/warns explicitly — never silently flattens: `compile()` logs the error unconditionally; `validate()` errors; `raise_on_error=True` raises - [ ] cycles execute as loops — part 2, needs conditional edges ## Tests Four added to the existing `tests/structs/test_graph_workflow.py` validation section (no new file): - `test_validate_reports_cycle_as_error_not_warning` — error + `is_valid=False` + flattening named + old generic warning gone - `test_validate_raise_on_error_raises_for_cycle` — `ValueError` on a cyclic graph - `test_compile_time_validation_names_cyclic_nodes` — exactly the cycle's nodes named, DAG-part nodes absent - `test_nodes_on_cycles_empty_for_dag` — peel returns `[]` and clean graphs stay `is_valid=True` Suite: **51 passed, 11 skipped** on this branch (was 47/11). Cross-applied to master, the two fix-targeting tests fail as expected: ``` FAILED test_validate_reports_cycle_as_error_not_warning FAILED test_nodes_on_cycles_empty_for_dag ``` `black --check` (line-length 70) and `ruff check` clean.
Proposed by ayaangazali## Problem `checkpoint_dir` gives crash recovery but not durable state, and its key has a correctness problem. Checkpoints are written one JSON file per layer, named: ```python task_key = hashlib.sha256(task.encode("utf-8")).hexdigest()[:16] checkpoint_path = cp_dir / f"{task_key}_layer_{layer_idx}.json" ``` The key is derived from the **task string only**. Consequences: - **Two concurrent runs of the same task collide.** They read and write the same files, and each will happily resume from the other's partial state. - **A re-run of the same task silently resumes** instead of starting fresh. There is no run identity, so "run this again" and "resume the run that crashed" are indistinguishable. - **No history.** Each layer file is overwritten. There is no way to inspect what a layer produced on a previous attempt. - **Only node outputs are captured** — not entry/end points, loop index, or conversation state — so a resume after a multi-loop run cannot restore its position. ## Proposed approach **1. Run identity (small, high value)** Key checkpoints by an explicit `run_id`/`thread_id` supplied by the caller, defaulting to a fresh id per `run()`. Resuming becomes explicit — you pass the id you want to resume — rather than an accident of task-string equality. This alone fixes the concurrent-collision bug. **2. Pluggable backend** Extract the write/read/list operations behind a small interface so checkpoints can go to sqlite, redis, or object storage rather than only the local filesystem. The current JSON-file behavior becomes the default implementation. **3. Richer state** Persist enough to resume mid-workflow rather than only at layer boundaries: loop index, conversation, and pending-node state. This is also the prerequisite for interrupt/resume — see the human-in-the-loop issue. **4. History** Keep prior checkpoints rather than overwriting, so a run can be inspected or replayed from an earlier layer. ## Acceptance criteria - [ ] concurrent runs of the same task do not share checkpoint state - [ ] resuming is explicit — a fresh `run()` of a previously-run task starts fresh by default - [ ] checkpoint storage is pluggable, with the filesystem as default - [ ] `max_loops > 1` resumes at the correct loop index - [ ] `clear_checkpoints` continues to work, scoped by run id - [ ] existing single-run usage keeps working, with the behavior change on re-run documented ## Related See `GRAPHWORKFLOW_VS_LANGGRAPH.md` in the repo root, gaps #6 and #7.
Proposed by kyegomez