Skip to content

🧪 Add tests for get_tool logic#77

Open
badMade wants to merge 4 commits into
mainfrom
jules-8720967847253227231-118e9c13
Open

🧪 Add tests for get_tool logic#77
badMade wants to merge 4 commits into
mainfrom
jules-8720967847253227231-118e9c13

Conversation

@badMade
Copy link
Copy Markdown
Owner

@badMade badMade commented Apr 30, 2026

🎯 What: Added tests for the get_tool function in src/tools.py as it was lacking coverage.
📊 Coverage: The test module now thoroughly tests for basic matches, mixed casing, various edge cases (empty strings, whitespace), unknown tools, and importantly, first-match priority behaviour when multiple tools share the same name in PORTED_TOOLS snapshot.
Result: Test coverage and reliability of get_tool is improved and clearly documented.


PR created automatically by Jules for task 8720967847253227231 started by @badMade

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 30, 2026 05:11
@google-labs-jules
Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds stronger unit test coverage around src.tools.get_tool to improve confidence in case-insensitive lookup behavior and edge-case handling, while also cleaning up an untracked helper script.

Changes:

  • Expanded test_get_tool to cover mixed casing, empty/whitespace inputs, unknown tools, and duplicate-name first-match behavior.
  • Standardized quoting/formatting in tests/test_tools.py.
  • Removed patch_pr.py from the repository.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/test_tools.py Adds new get_tool test cases (mixed casing, edge cases, duplicates) and minor formatting/quote normalization.
patch_pr.py Deletes a one-off patch helper script from the repo root.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_tools.py Outdated
Comment thread tests/test_tools.py
Comment on lines +87 to +89
if t.name.lower() == dupe_name.lower() and t != expected_first_module
]
self.assertTrue(len(other_modules) > 0)
@badMade
Copy link
Copy Markdown
Owner Author

badMade commented May 1, 2026

@claude code review

@codex code review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@badMade
Copy link
Copy Markdown
Owner Author

badMade commented May 1, 2026

@copilot, resolve the merge conflicts in this pull request.

@badMade badMade marked this pull request as ready for review May 1, 2026 02:53
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copilot AI and others added 2 commits May 1, 2026 03:33
…detection, assert source_hint in dupe test

Agent-Logs-Url: https://github.com/badMade/claw-code/sessions/7e6aed0f-9e29-4fe8-b6ea-7f90c4edbeab

Co-authored-by: badMade <106821302+badMade@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 1, 2026 03:36
@badMade badMade review requested due to automatic review settings May 1, 2026 03:36
Copy link
Copy Markdown

Copilot AI commented May 1, 2026

@copilot, resolve the merge conflicts in this pull request.

Merged origin/main into this branch (commit e6cb38f). The only conflict was patch_pr.py (deleted here vs. modified in main) — kept it deleted. The optimized src/tools.py from main (_get_tool_lookup with lru_cache + MappingProxyType) was brought in cleanly, and all existing tests were retained and still pass.

Also took the opportunity to address the two reviewer comments on the duplicate-name test: switched to collections.Counter for O(n) detection (commit 5113b72), and added an explicit source_hint assertion to match the comment's documented intent.

@badMade
Copy link
Copy Markdown
Owner Author

badMade commented May 6, 2026

@gemini-code-assist, what are the steps and line numbers to make this fix?

@gemini-code-assist
Copy link
Copy Markdown

To address the merge conflicts and implement the requested improvements in src/tools.py and tests/test_tools.py, the following changes were applied:

1. Optimization of get_tool in src/tools.py

  • Change: Replaced the O(N) linear scan with an O(1) dictionary lookup using lru_cache and MappingProxyType.
  • Implementation: Added _get_tool_lookup (lines 52-59) to lazily initialize the lookup dictionary, ensuring first-match priority by checking if name_lower not in lookup.
  • Update: Refactored get_tool (lines 62-63) to use this cached lookup.

2. Test Coverage in tests/test_tools.py

  • Change: Added comprehensive tests for the new lookup logic.
  • Implementation:
    • Mixed Casing: Added a test case (lines 54-58) to verify case-insensitive matching.
    • Edge Cases: Added tests for empty strings, whitespace, and newlines (lines 61-63).
    • Duplicate Handling: Used collections.Counter (lines 70-74) to identify tools with duplicate names and verified that get_tool correctly returns the first-match (lines 76-94).

These changes ensure that the tool retrieval logic is both performant and robust against the edge cases and potential name collisions identified in the PORTED_TOOLS snapshot.

@badMade
Copy link
Copy Markdown
Owner Author

badMade commented May 25, 2026

@copilot, resolve the merge conflicts in this pull request.

Copilot AI review requested due to automatic review settings May 25, 2026 23:27
@badMade badMade review requested due to automatic review settings May 25, 2026 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants