-
-
Notifications
You must be signed in to change notification settings - Fork 146
fix: restore main CI contracts after merge #480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
39329fa
6cb5d7d
053a533
54ce72a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -52,7 +52,7 @@ class TestToolRegistration: | |||||||||||||||||||||||||||||||||||||||||
| def test_all_tools_registered(self, tmp_path): | ||||||||||||||||||||||||||||||||||||||||||
| provider = _provider(tmp_path) | ||||||||||||||||||||||||||||||||||||||||||
| names = _tool_names(provider) | ||||||||||||||||||||||||||||||||||||||||||
| assert len(names) == 39, f"Expected 39 tools, got {len(names)}" | ||||||||||||||||||||||||||||||||||||||||||
| assert len(names) == 40, f"Expected 40 tools, got {len(names)}" | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
52
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Add a focused regression test for Updating the aggregate count does not prove the new tool is registered; the test could still pass with a missing tool and an unrelated duplicate. Assert that Suggested coverage def test_all_tools_registered(self, tmp_path):
provider = _provider(tmp_path)
names = _tool_names(provider)
assert len(names) == 40, f"Expected 40 tools, got {len(names)}"
+
+ def test_apply_pending_tool_present(self, tmp_path):
+ provider = _provider(tmp_path)
+ schemas = provider.get_tool_schemas()
+ schema = next(s for s in schemas if s["name"] == "mnemosyne_apply_pending")
+ assert schema["parameters"]["required"] == ["pending_ids"]
+ assert schema["parameters"]["properties"]["pending_ids"] == {
+ "type": "array",
+ "items": {"type": "string"},
+ "description": schema["parameters"]["properties"]["pending_ids"]["description"],
+ }As per path instructions: “Ensure strong coverage of: ... MCP tool surface ... Flag missing or weak test scenarios.” 📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Path instructions |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def test_canonical_tools_present(self, tmp_path): | ||||||||||||||||||||||||||||||||||||||||||
| provider = _provider(tmp_path) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: mnemosyne-oss/mnemosyne
Length of output: 7211
🏁 Script executed:
Repository: mnemosyne-oss/mnemosyne
Length of output: 11947
🏁 Script executed:
Repository: mnemosyne-oss/mnemosyne
Length of output: 634
Update the Hermes docs to match the current tool registry.
integrations/hermes/README.mdstill says 23 tools, butALL_TOOL_SCHEMASnow exposes 40. Update the count, and addmnemosyne_apply_pendingto the documented tool surface if this section is meant to enumerate it.🤖 Prompt for AI Agents
Source: Path instructions