Skip to content

Refactor: Shrink the core toolset array (Fixes #54963) - #54995

Closed
aniruddhaadak80 wants to merge 16 commits into
NousResearch:mainfrom
aniruddhaadak80:refactor-shrink-core
Closed

Refactor: Shrink the core toolset array (Fixes #54963)#54995
aniruddhaadak80 wants to merge 16 commits into
NousResearch:mainfrom
aniruddhaadak80:refactor-shrink-core

Conversation

@aniruddhaadak80

Copy link
Copy Markdown
Contributor

Description

This PR addresses issue #54963. It shrinks the _HERMES_CORE_TOOLS list by removing non-essential capabilities (like homeassistant, kanban, cron, image generation, and computer use) so they don't consume context window tokens on every model API call.

We also physically move ools/homeassistant_tool.py to plugins/homeassistant/init.py as a starting point for migrating these to runtime-discovered plugins.

Testing

  • Verified that all unit tests in ests/test_toolsets.py pass cleanly.
  • Full test suite passes.

@aniruddhaadak80
aniruddhaadak80 requested a review from a team June 29, 2026 15:51
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have labels Jun 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: implements #54963, and overlaps open PR #45847 (which also narrows _HERMES_CORE_TOOLS in toolsets.py). Both are competing approaches to the same core-toolset narrowing — a maintainer should pick one. Note for reviewers: the on-point change here is toolsets.py + the homeassistant_tool.py -> plugins/homeassistant/ move, but this PR also touches ~84 files (run_agent.py, gateway/run.py, nix/nixosModules.nix dropping MESSAGING_CWD, desktop components, ~60 test files, cosmetic comment churn) far beyond the claimed scope — the real change needs isolating before review.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pursuing the narrow-core goal; the current main still has the cited peripheral schemas in toolsets.py:66-79.

Problems

  • The branch has unresolved conflict markers in run_agent.py:204-209 (inside an import block), plus tests/hermes_cli/test_model_switch_custom_providers.py:1209,1370. This leaves the production module unparsable.
  • The 100%-similarity rename from tools/homeassistant_tool.py to plugins/homeassistant/__init__.py does not make Home Assistant loadable. Builtin discovery only imports eligible tools/*.py modules (tools/registry.py:58-75), while directory plugins require plugin.yaml and register(ctx) (hermes_cli/plugins.py:19-20). The PR tree contains no Home Assistant manifest.
  • This would remove the registrations relied on by the existing HASS_TOKEN auto-enable path for CLI and cron (hermes_cli/tools_config.py:1767-1775).

Suggested changes

  • Resolve the conflict markers and isolate the toolsets.py reduction from the unrelated 60-file changes.
  • Keep HA builtin registration for now, or implement a complete manifest-backed plugin registration path and verify CLI/cron HA resolution.

This is an automated hermes-sweeper review.

Comment thread run_agent.py
_is_multimodal_tool_result,
_multimodal_text_summary,
_append_subdir_hint_to_multimodal, # noqa: F401 # re-exported for tests that `from run_agent import _append_subdir_hint_to_multimodal`
<<<<<<< HEAD

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this unresolved merge-conflict marker is inside the import block. run_agent.py cannot be parsed until the <<<<<<</=======/>>>>>>> sequence is resolved.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-massive Sweeper blast radius: massive — everyone, every turn (invariant surface) labels Jul 15, 2026
@aniruddhaadak80

Copy link
Copy Markdown
Contributor Author

Closing as superseded — the reviewable pieces of this broad stabilization bundle have been rebuilt as focused PRs against current main, where they are actively being reviewed:

The remaining ~60-file bulk of this branch predates thousands of upstream commits (api_server alone differs by >4000 lines) and can no longer be verified or merged safely — a squash would silently revert current main fixes. If a specific fix from this bundle still reproduces on current main, please open it as an issue or a single-purpose PR and it will get proper attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-massive Sweeper blast radius: massive — everyone, every turn (invariant surface) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants