Feat: Tools aop registry and tools namespace #19

EvilFreelancer · 2025-09-13T23:13:22Z

Приветствую! Предлагаю вашему вниманию рефакторинг тулов о котором я размышлял всю прошлую неделю.

Первым делом выполнил небольшой рефакторинг, призванный разделить тулы и ядро в отдельные логические блоки.

Вместо core.tools.base и core.tools.research была выполнена декомпозиция тулов, все тулы была разнесены по отдельным файлам, при этом логика тулов осталась прежней
Все тулы были перемещены из core.tools на уровень выше и хранятся теперь в tools
BaseTool был так же вынесен в отдельный файл base_tool.py, но при этом остался в core
NextStepToolStub и NextStepToolsBuilder так же были вынесены в отдельный файл
next_step_tools.py в core

Следующим важным нововведеним данного PR является механизм реестра тулов.

В core был добавлен tools_registry, который содержит в себе класс ToolsRegistry и декоратор @tool позволяющий регистрировать в указанном регистри объекты расширящие класс BaseTool, то есть тулы
Для того чтобы идентифицировать системные и исследовательские тулы был добавлен атрибут is_system_tool с типом boolean, по умолчанию он имеет статус False
В классе ToolsRegistry имеются два метода, get_system_tools - возвращает список системных тулов и get_research_tools который будет возвращать исследовательские тулы, указанные методы призваны заменить хардкод system_agent_tools и research_agent_tools

virrius · 2025-09-14T11:11:47Z

sgr_deep_research/core/tools_registry.py

+    @classmethod
+    def get_research_tools(cls) -> list[Type[BaseTool]]:
+        """Get all research tools (is_system_tool = False).
+
+        Returns:
+            List of research tool classes
+        """
+        return [tool for tool in cls._tools.values() if not tool.is_system_tool]


Вот это немного нарушает мою изначальную концепцию, где можно было выпускать разные наборы тулов. Условно system, research, file, web search, server и всё такое. Импортишь их всем пакетом и агент приобретает необходимые навыки

Предлагаю заменить на признак is_enable: bool, чтобы можно было управлять активными тулами на уровне регистри.

Сделал что пользователь можешь собрать любой набор какой хочет, если он передаёт их через toolkit в контроллере, то пользовательские тулы сливаются с системными.

Плюс во всех агентах используются все тулы, кроме случаев когда необходимо отключить ризонинг.

sgr_deep_research/core/next_step_tools.py

sgr_deep_research/core/tools_registry.py

virrius · 2025-09-14T11:25:57Z

Я чучуточку скептичен насчёт регистри. Оно мне кажется немного оверкиллом. Пока не очевидны сценарии, где тулзы понадобились бы централизованно из регистри
Могу представить, что это было бы полезно в случае динамической сборки агентов в рантайме, но этот сценарий видится слишком специфичным
Можешь подробнее описать, в чём видишь пользу и удобство регистри?

Не вижу смысла в бинарном разделении system/research. В изначальной моей идее из коммента выше - это просто один из возможных пакетов тулзов В моём понимании флоу: импортим/пишем необходимые тулзы/их наборы, подсовываем в тулкит агента, радуемся

разделение на тулзов на файлики выглядит хорошо

Кроме того, стоит и конкретных агентов вынести на уровень с тулзами

sgr_deep_research/core/tools_registry.py

sgr_deep_research/core/__init__.py

MiXaiLL76 · 2025-09-14T18:17:10Z

sgr_deep_research/core/agents/base_agent.py

 logging.basicConfig(
    level=logging.INFO,


@virrius а мы зачем так внутри агента настраиваем basicConfig логера?

Это наследие оригинальной версии и вопрос скорее к @virrius мой код на данном участке просто выполняет декомпозицию (раскладывает классы по файликам).

EvilFreelancer · 2025-09-18T10:08:25Z

Я чучуточку скептичен насчёт регистри. Оно мне кажется немного оверкиллом. Пока не очевидны сценарии, где тулзы понадобились бы централизованно из регистри Могу представить, что это было бы полезно в случае динамической сборки агентов в рантайме, но этот сценарий видится слишком специфичным Можешь подробнее описать, в чём видишь пользу и удобство регистри?

Сделал регистри опциональным, это теперь для нас история, мы можем добалять тулы в общий пул из любого места, юзерам возможно это тоже понравится, возможность использовать кастомные toolkit вдобавок к дефолтному набору - сохранена.

Не вижу смысла в бинарном разделении system/research. В изначальной моей идее из коммента выше - это просто один из возможных пакетов тулзов В моём понимании флоу: импортим/пишем необходимые тулзы/их наборы, подсовываем в тулкит агента, радуемся

Убрал эту логику, теперь список тулов запрашивается из регистир, плюс к этому набору добавляется набор переданный юзером через toolkit.

Кроме того, стоит и конкретных агентов вынести на уровень с тулзами

Предлагаю агентов перенести в свой неймспейс отдельным PR, а то в этом и так уже много правок.

MiXaiLL76

tool, tool, tool

MiXaiLL76 · 2025-09-18T11:27:01Z

sgr_deep_research/core/agents/sgr_agent.py

+from sgr_deep_research.tools import (
    AgentCompletionTool,
-    BaseTool,
    ClarificationTool,
    CreateReportTool,
    NextStepToolsBuilder,
    NextStepToolStub,
    ReasoningTool,
    WebSearchTool,
-    research_agent_tools,
-    system_agent_tools,
 )


Много тулзов. Они тут зачем если есть регистри? я вижу там удаляются некоторые тулзы по возможности, мб просто по tool.class.name будем делать? чтобы меньше импортов было?

MiXaiLL76 · 2025-09-18T11:28:42Z

sgr_deep_research/core/agents/sgr_agent.py

-            *(toolkit or []),
-        ]
+        self.toolkit = [*ToolsRegistry.get_tools(), *(toolkit or [])]
        self.toolkit.remove(ReasoningTool)  # we use our own reasoning scheme


ну типо вот тут, сделать удаление через class.name, можно тупо перебором мб?

MiXaiLL76 · 2025-09-18T11:29:19Z

sgr_deep_research/core/agents/sgr_tools_agent.py

    AgentCompletionTool,
-    BaseTool,
    ClarificationTool,
    CreateReportTool,
    ReasoningTool,
    WebSearchTool,


tool, tool, tool, tool. Если есть регистри, они не нужны в таком виде

MiXaiLL76 · 2025-09-18T11:29:54Z

sgr_deep_research/core/agents/sgr_tools_agent.py

+            tools = {
                ReasoningTool,
                CreateReportTool,
                AgentCompletionTool,
-            ]
+            }


tool, tool, tool....

registry.get("name")?

MiXaiLL76 · 2025-09-18T11:30:05Z

sgr_deep_research/core/agents/tools_agent.py

+from sgr_deep_research.tools import (
    AgentCompletionTool,
-    BaseTool,
    ClarificationTool,
    CreateReportTool,
    ReasoningTool,
    WebSearchTool,


tool, tool, tool

virrius

Моё понимание концепции сейчас на уровне:
коллекция для централизованного хранения и обработки тулзов - ОК, но само по себе не слишком полезно. Добавляет абстракцию, добавляет список стейтов (enabled) и накидывает лишней логики копирования

Применение в агенте - не ОК - оно используется буквально для инициализации toolkit, который по идее должен быть полностью заменён локальным registry агента

Добавление/удаление тулзов - не ОК, непонятно, как эффективно выбрать необходимый набор тулзов из общего количества

Регистри всё ещё кажется излишним и не слишком удобным. Я не понимаю/не оценил идею, в которой оно может быть полезно в текущей реализации

Предположим, количество тулзов в проекте возрастёт до сотни. А агент мы хотим формировать из 7-12. Каков будет механизм выбора тулзов из регистри? Писать 90 раз disable_tool? Сейчас внего копируется весь общий регистри. Согласно нашим обсуждениям об идее регистри, стоит сделать минимальную обёртку, которая могла бы фыормировать необходимый список тулов из общего количество по конфигу

Моё предложение - пивотнуться на уровне идеи или прикопать это дело до момента, когда появится более сформированная потребность

virrius · 2025-09-20T10:19:31Z

sgr_deep_research/core/agents/base_agent.py

-                if isinstance(action_tool, ClarificationTool):
+                if isinstance(action_tool, BaseTool):


Мне кажется, тут что-то пошло не так с переименованием. Важно чтобы этот кейс срабатывал именно для ClarificationTool.

Выше в тайпинге тоже бы предпочёл видеть конкретно ReasoningTool

virrius · 2025-09-20T10:23:11Z

sgr_deep_research/core/base_tool.py

+class BaseTool(BaseModel):
+    """Class to provide tool handling capabilities."""
+
+    is_enabled: ClassVar[bool] = True


Вот тут вангую проблемы будут. Это поле где-то используется? Предполагается к использованию?

virrius · 2025-09-20T10:31:28Z

sgr_deep_research/core/tools_registry.py

+        }
+
+    @classmethod
+    def get_default_registry(cls) -> "ToolsRegistry":


Выглядит лишним в текущей логике. Можно просто через конструктор инициализировать

EvilFreelancer added 4 commits September 14, 2025 01:24

Tools decomposition and refactoring from core to root

d309ba4

is_system_tool attr added

61a731c

Tools registry added

93b9909

Next step tools moved back to core

9eb37a0

EvilFreelancer requested review from MiXaiLL76 and virrius September 13, 2025 23:13

Linter fix

5f86f3e

virrius reviewed Sep 14, 2025

View reviewed changes

sgr_deep_research/core/tools_registry.py Outdated Show resolved Hide resolved

MiXaiLL76 reviewed Sep 14, 2025

View reviewed changes

sgr_deep_research/core/__init__.py Show resolved Hide resolved

MiXaiLL76 reviewed Sep 14, 2025

View reviewed changes

EvilFreelancer added 5 commits September 16, 2025 18:46

Brushup

1c0aaf3

Conflicts solved

48c8e29

is_enabled instead of is system

fc5b0e6

Brushup of tools

e8e939b

Registry is just for storingg list of embedded tools

4732819

EvilFreelancer requested review from MiXaiLL76 and virrius September 18, 2025 10:03

EvilFreelancer added 3 commits September 18, 2025 13:09

Linter fix

6c013fe

Typo fixed

e4f4be7

Typo fixed

c972062

MiXaiLL76 reviewed Sep 18, 2025

View reviewed changes

MiXaiLL76 approved these changes Sep 18, 2025

View reviewed changes

ToolsRegistry updated

02fa68c

MiXaiLL76 approved these changes Sep 19, 2025

View reviewed changes

virrius requested changes Sep 20, 2025

View reviewed changes

EvilFreelancer closed this Sep 20, 2025

		if isinstance(action_tool, ClarificationTool):
		if isinstance(action_tool, BaseTool):

Feat: Tools aop registry and tools namespace #19

Feat: Tools aop registry and tools namespace #19

Uh oh!

Conversation

EvilFreelancer commented Sep 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

virrius commented Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EvilFreelancer commented Sep 18, 2025

Uh oh!

MiXaiLL76 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

virrius left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

virrius Sep 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

virrius Sep 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

virrius Sep 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

virrius commented Sep 14, 2025 •

edited

Loading

virrius left a comment •

edited

Loading

virrius Sep 20, 2025 •

edited

Loading

virrius Sep 20, 2025 •

edited

Loading

virrius Sep 20, 2025 •

edited

Loading