Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/tests/test_colony_runtime_overseer.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class MockStreamingLLM(LLMProvider):
consumption. Each worker gets the scenario matching its task.
"""

model: str = "mock"

def __init__(
self,
scenarios: list[list] | None = None,
Expand Down Expand Up @@ -293,6 +295,7 @@ async def test_worker_crash_emits_synthesised_failed_report(self, tmp_path, agen
"""

class CrashingLLM(LLMProvider):
model: str = "mock"
stream_calls: list[dict] = []

async def stream(self, messages, system="", tools=None, max_tokens=4096):
Expand Down
2 changes: 2 additions & 0 deletions core/tests/test_context_handoff.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ def complete(self, messages: list[dict[str, Any]], **kwargs: Any) -> LLMResponse
class FailingLLMProvider(LLMProvider):
"""LLM provider that always raises."""

model: str = "mock"

def complete(self, messages: list[dict[str, Any]], **kwargs: Any) -> LLMResponse:
raise RuntimeError("LLM unavailable")

Expand Down
26 changes: 16 additions & 10 deletions core/tests/test_default_skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@


class TestDefaultSkillFiles:
"""Verify all 6 built-in SKILL.md files parse correctly."""
"""Verify all 7 built-in SKILL.md files parse correctly."""

def test_all_six_skills_exist(self):
assert len(SKILL_REGISTRY) == 6
def test_all_seven_skills_exist(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add return type hint to the renamed test method.

Line 22 introduces a changed function signature without an explicit type hint.

Suggested fix
-    def test_all_seven_skills_exist(self):
+    def test_all_seven_skills_exist(self) -> None:

As per coding guidelines, "Use type hints on all function signatures".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_all_seven_skills_exist(self):
def test_all_seven_skills_exist(self) -> None:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/tests/test_default_skills.py` at line 22, The test function
test_all_seven_skills_exist is missing a return type annotation; update its
signature to include an explicit return type (e.g., add "-> None") so the
function follows the project's type-hinting guideline for all function
signatures.

assert len(SKILL_REGISTRY) == 7

@pytest.mark.parametrize("skill_name,dir_name", list(SKILL_REGISTRY.items()))
def test_skill_parses(self, skill_name, dir_name):
Expand All @@ -35,7 +35,7 @@ def test_skill_parses(self, skill_name, dir_name):
assert parsed.source_scope == "framework"

def test_combined_token_budget(self):
"""All default skill bodies combined should be under 2000 tokens (~8000 chars)."""
"""All default skill bodies combined should be under 3000 tokens (~12000 chars)."""
total_chars = 0
for dir_name in SKILL_REGISTRY.values():
path = _DEFAULT_SKILLS_DIR / dir_name / "SKILL.md"
Expand All @@ -44,9 +44,9 @@ def test_combined_token_budget(self):
total_chars += len(parsed.body)

approx_tokens = total_chars // 4
assert approx_tokens < 2000, (
assert approx_tokens < 3000, (
f"Combined default skill bodies are ~{approx_tokens} tokens "
f"({total_chars} chars), exceeding the 2000 token budget"
f"({total_chars} chars), exceeding the 3000 token budget"
)

def test_data_buffer_keys_all_prefixed(self):
Expand All @@ -60,7 +60,7 @@ def test_load_all_defaults(self):
manager = DefaultSkillManager()
manager.load()

assert len(manager.active_skill_names) == 6
assert len(manager.active_skill_names) == 7
for name in SKILL_REGISTRY:
assert name in manager.active_skill_names

Expand Down Expand Up @@ -97,7 +97,7 @@ def test_disable_single_skill(self):
manager.load()

assert "hive.quality-monitor" not in manager.active_skill_names
assert len(manager.active_skill_names) == 5
assert len(manager.active_skill_names) == 6

def test_disable_all_via_convention(self):
config = SkillsConfig.from_agent_vars(default_skills={"_all": {"enabled": False}})
Expand Down Expand Up @@ -231,11 +231,17 @@ def test_context_preservation_override_threshold(self):
assert "45%" not in prompt

def test_no_unreplaced_placeholders_with_defaults(self):
"""All {{...}} placeholders should be replaced when using defaults."""
"""All {{...}} placeholders should be replaced when using defaults.

The writing-hive-skills skill contains literal ``{{placeholder}}``
as documentation text, so we strip that known occurrence before checking.
"""
manager = DefaultSkillManager()
manager.load()
prompt = manager.build_protocols_prompt()
assert "{{" not in prompt
# Remove the known literal {{placeholder}} documentation example
cleaned = prompt.replace("{{placeholder}}", "")
assert "{{" not in cleaned
Comment on lines +242 to +244

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Current placeholder cleanup can mask real regressions.

replace("{{placeholder}}", "") removes all occurrences, so multiple unintended placeholders could slip through undetected.

Suggested fix
-        # Remove the known literal {{placeholder}} documentation example
-        cleaned = prompt.replace("{{placeholder}}", "")
+        # Remove exactly one known literal {{placeholder}} documentation example
+        literal_placeholder = "{{placeholder}}"
+        assert prompt.count(literal_placeholder) == 1
+        cleaned = prompt.replace(literal_placeholder, "", 1)
         assert "{{" not in cleaned
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/tests/test_default_skills.py` around lines 242 - 244, Only remove the
single example placeholder so real leftover placeholders aren't masked: assert
that prompt.count("{{placeholder}}") >= 1, then call
prompt.replace("{{placeholder}}", "", 1) to remove just one occurrence into
cleaned, and keep the existing assert "{{" not in cleaned to catch any other
unremoved placeholders; reference the prompt variable, cleaned variable, and the
replace(...) call.



class TestBatchAutoDetection:
Expand Down
14 changes: 13 additions & 1 deletion core/tests/test_event_loop_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ class MockStreamingLLM(LLMProvider):
Cycles back to the beginning if more calls are made than scenarios.
"""

model: str = "mock"

def __init__(self, scenarios: list[list] | None = None):
self.scenarios = scenarios or []
self._call_index = 0
Expand Down Expand Up @@ -1048,6 +1050,8 @@ class ErrorThenSuccessLLM(LLMProvider):
Used to test the retry-with-backoff wrapper around _run_single_turn().
"""

model: str = "mock"

def __init__(self, error: Exception, fail_count: int, success_scenario: list):
self.error = error
self.fail_count = fail_count
Expand Down Expand Up @@ -1174,6 +1178,8 @@ async def test_stream_error_event_retried_as_runtime_error(self, runtime, node_s
call_index = 0

class StreamErrorThenSuccessLLM(LLMProvider):
model: str = "mock"

async def stream(self, messages, system="", tools=None, max_tokens=4096):
nonlocal call_index
idx = call_index
Expand Down Expand Up @@ -1343,11 +1349,13 @@ def test_empty_fingerprints_no_doom(self):
class ToolRepeatLLM(LLMProvider):
"""LLM that produces identical tool calls across outer iterations.

Alternates: even calls tool call, odd calls text (exits inner loop).
Alternates: even calls -> tool call, odd calls -> text (exits inner loop).
This ensures each outer iteration = 2 LLM calls with 1 tool executed.
After tool_turns outer iterations, always returns text.
"""

model: str = "mock"

def __init__(
self,
tool_name: str,
Expand Down Expand Up @@ -1590,6 +1598,8 @@ async def judge_eval(*args, **kwargs):
call_idx = 0

class DiffArgsLLM(LLMProvider):
model: str = "mock"

async def stream(self, messages, **kwargs):
nonlocal call_idx
idx = call_idx
Expand Down Expand Up @@ -1963,6 +1973,8 @@ async def test_safe_tool_starts_before_finish_event(self, runtime, node_spec, bu
delay = 0.25

class SlowStreamLLM(LLMProvider):
model: str = "mock"

def __init__(self):
self._calls = 0

Expand Down
2 changes: 2 additions & 0 deletions core/tests/test_litellm_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,8 @@ async def test_base_provider_acomplete_offloads_to_executor(self):
call_thread_ids = []

class SlowSyncProvider(LLMProvider):
model: str = "mock"

def complete(
self,
messages,
Expand Down
2 changes: 2 additions & 0 deletions core/tests/test_llm_judge.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
class MockLLMProvider(LLMProvider):
"""Mock LLM provider for testing."""

model: str = "mock"

def __init__(self, response_content: str = '{"passes": true, "explanation": "Test passed"}'):
self.response_content = response_content
self.complete_calls = []
Expand Down
6 changes: 2 additions & 4 deletions core/tests/test_model_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ def test_minimax_catalog_tracks_current_non_legacy_text_models():
assert minimax_default == "MiniMax-M2.7"
assert [model["id"] for model in minimax_models] == [
"MiniMax-M2.7",
"MiniMax-M2.7-highspeed",
"MiniMax-M2.5",
"MiniMax-M2.5-highspeed",
]
assert all(model["max_context_tokens"] == 204800 for model in minimax_models)
assert all(model["max_tokens"] == 32768 for model in minimax_models)
Expand Down Expand Up @@ -162,10 +160,10 @@ def test_openrouter_catalog_tracks_current_frontier_set():


def test_find_model_any_provider_returns_provider_and_model():
provider_id, model = model_catalog.find_model_any_provider("google/gemini-2.5-pro")
provider_id, model = model_catalog.find_model_any_provider("google/gemini-3.1-pro-preview-customtools")

assert provider_id == "openrouter"
assert model["max_context_tokens"] == 900000
assert model["max_context_tokens"] == 1048576


def test_get_preset_returns_subscription_specific_limits():
Expand Down
4 changes: 2 additions & 2 deletions core/tests/test_phase_compaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ async def test_current_phase_protected(self):
msgs = conv.messages
old_tool = [m for m in msgs if m.role == "tool" and m.phase_id == "research"]
assert len(old_tool) == 1
assert old_tool[0].content.startswith("[Pruned tool result")
assert old_tool[0].content.startswith("Pruned tool result")
Comment thread
coderabbitai[bot] marked this conversation as resolved.

# Current phase's tool result should be intact
current_tool = [m for m in msgs if m.role == "tool" and m.phase_id == "report"]
Expand Down Expand Up @@ -264,5 +264,5 @@ async def test_pruned_message_preserves_phase_metadata(self):

await conv.prune_old_tool_results(protect_tokens=0, min_prune_tokens=100)

pruned_msg = [m for m in conv.messages if m.content.startswith("[Pruned")][0]
pruned_msg = [m for m in conv.messages if m.content.startswith("Pruned")][0]
assert pruned_msg.phase_id == "research"
10 changes: 3 additions & 7 deletions core/tests/test_queen_memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -600,13 +600,8 @@ async def test_subscribe_reflection_triggers_runs_housekeeping_for_both_scopes(
await asyncio.sleep(0.05)

assert len(sub_ids) == 2
assert unified_short.await_count == 5
unified_long.assert_awaited_once_with(
llm,
global_memory_dir=global_dir,
queen_memory_dir=queen_dir,
queen_id="queen_technology",
)
assert unified_short.await_count == 3
unified_long.assert_not_awaited()


@pytest.mark.asyncio
Expand Down Expand Up @@ -753,6 +748,7 @@ def test_queen_phase_state_appends_global_memory_block():

def test_queen_phase_state_appends_queen_memory_block():
phase = QueenPhaseState(
phase="building",
prompt_building="base prompt",
_cached_global_recall_block="--- Global Memories ---\nglobal stuff",
_cached_queen_recall_block="--- Queen Memories: queen_technology ---\nqueen stuff",
Expand Down
2 changes: 2 additions & 0 deletions core/tests/test_run_parallel_workers_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@


class _ByTaskMockLLM(LLMProvider):
model: str = "mock"

def __init__(self, by_task: dict[str, list]):
self.by_task = by_task

Expand Down
4 changes: 2 additions & 2 deletions core/tests/test_skill_cli_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def test_shows_warnings_on_valid_skill_without_license(self, tmp_path, capsys):

class TestCmdSkillDoctor:
def test_defaults_pass_against_real_framework_skills(self):
"""All 6 framework default skills should be healthy (no mocking)."""
"""All 7 framework default skills should be healthy (no mocking)."""
args = Namespace(defaults=True, name=None, project_dir=None)
result = cmd_skill_doctor(args)
assert result == 0
Expand Down Expand Up @@ -355,7 +355,7 @@ def test_doctor_defaults_json(self, capsys):
data = json.loads(out)
assert result == 0
assert "skills" in data
assert len(data["skills"]) == 6 # 6 framework default skills
assert len(data["skills"]) == 7 # 7 framework default skills
assert data["total_errors"] == 0

def test_search_json_registry_unavailable_exits_1(self, capsys):
Expand Down
5 changes: 3 additions & 2 deletions tools/tests/test_browser_tools_comprehensive.py
Original file line number Diff line number Diff line change
Expand Up @@ -778,8 +778,9 @@ async def mock_evaluate_capture(tab_id: int, script: str) -> dict:
):
result = await browser_evaluate(script="return 42;")

# Tool passes script through unchanged — wrapping is bridge's job
assert call_args == ["return 42;"]
# Tool may issue a toast call then the actual script call
assert len(call_args) >= 1
assert any("return 42;" in arg for arg in call_args)
# Tool returns bridge's raw result
assert result == {"result": {"value": 42}}

Expand Down
Loading
Loading