Skip to content

Commit ab585cc

Browse files
Brian KrafftCopilot
andcommitted
fix(P4/4.1): address review findings from /8eyes + /collab
Round 1 fixes: - Fix llm_kwargs spread order so explicit timeout/max_retries take precedence (L3) - Add skills.enabled guard inside ToolRegistry.discover() (F4) - Remove dead agent-None guard in select_and_inject (M2) - Expand test suite: 15 → 23 tests covering: - skills.enabled=False path - grounding_config=None path - store quality metrics forwarding - store exception swallowing - no-LLM skip path - backend_scope forwarding - LLM returns None when all options exhausted - agent=None fallback to llm_client - llm_kwargs forwarding verification - Strengthen assertion on _tool_registry initial value Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4f5e46b commit ab585cc

3 files changed

Lines changed: 963 additions & 9 deletions

File tree

openspace/tool_registry.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ def discover(self) -> bool:
7979
else None
8080
)
8181

82+
if skill_cfg and not getattr(skill_cfg, "enabled", True):
83+
logger.debug("Skills explicitly disabled in config")
84+
return False
85+
8286
# 1. Host agent skill directories from env (standalone mode support)
8387
host_dirs_raw = os.environ.get("OPENSPACE_HOST_SKILL_DIRS", "")
8488
if host_dirs_raw:
@@ -201,7 +205,7 @@ async def select_and_inject(
201205
return False
202206

203207
# Inject active skills (full SKILL.md content, backend-aware)
204-
agent_backends = agent.backend_scope if agent else None
208+
agent_backends = agent.backend_scope
205209
context_text = self._registry.build_context_injection(
206210
selected, backends=agent_backends
207211
)
@@ -223,10 +227,10 @@ def _get_selection_llm(
223227
# 1. Dedicated skill selection model
224228
if self._config.skill_registry_model:
225229
return LLMClient(
230+
**self._config.llm_kwargs,
226231
model=self._config.skill_registry_model,
227232
timeout=30.0,
228233
max_retries=2,
229-
**self._config.llm_kwargs,
230234
)
231235

232236
# 2. Tool retrieval model from grounding agent

0 commit comments

Comments
 (0)