fix: self-correcting tool resolution with suggestions and strict mode#3156
fix: self-correcting tool resolution with suggestions and strict mode#3156praisonai-triage-agent[bot] wants to merge 2 commits into
Conversation
…fixes #3153) Unknown/mistyped tool names now surface near-name suggestions via difflib over the available-tool catalogue (registry + TOOL_MAPPINGS + praisonai-tools). Adds opt-in strict mode (strict= arg / PRAISONAI_STRICT_TOOLS env) that raises ToolResolutionError carrying unknown names and suggestions. Default stays backward-compatible (warn + suggest, no raise). Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
✅ Action performedReview finished.
|
MervinPraison
added
pipeline/blocked:ci
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe tool resolver now detects unresolved names across multiple inputs, builds typo suggestions from available tools, supports callback or warning diagnostics, and raises ChangesTool Resolution Diagnostics
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Caller
participant resolve_tool_names
participant resolve_tool_name
participant _closest_names
participant on_unknown
participant ToolResolutionError
Caller->>resolve_tool_names: provide tool names and resolution options
resolve_tool_names->>resolve_tool_name: resolve each name
resolve_tool_name-->>resolve_tool_names: resolved tool or unknown result
resolve_tool_names->>_closest_names: compute suggestions for unknown names
alt strict mode
resolve_tool_names->>ToolResolutionError: raise unknown names and suggestions
else callback provided
resolve_tool_names->>on_unknown: report unknown names and suggestions
end
resolve_tool_names-->>Caller: return resolved tools
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR makes unknown tool resolution visible and optionally strict across shared agent surfaces. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix: preserve typed ToolResolutionError ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| _praisonai_tools_available: Optional[bool] = None | ||
|
|
||
|
|
||
| class ToolResolutionError(ValueError): |
There was a problem hiding this comment.
When strict mode rejects a name supplied through an Agent toolset, the existing toolset handler catches this ValueError subclass and replaces it with a plain ValueError. Callers cannot catch ToolResolutionError or read its unknown and suggestions fields on that path, so the new self-correction contract works for direct tool lists but is lost for toolsets.
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
MervinPraison
added
pipeline/awaiting-merge-gate
|
Claude finished @MervinPraison's task —— View job Final Architecture Review — PR #3156 ✅ Approve (with fix applied)
Phase 1 — Architecture review per AGENTS.md
Phase 2 — Valid issue found & fixedGreptile P1 (VALID): Toolset path erased the typed error. Fix applied (pushed to this branch):
Skipped (intentionally): no new Agent params, no new modules, no docs/wrapper changes — kept minimal per AGENTS.md. Phase 3 — Verdict✅ Approve. The one blocking P1 is resolved; the change is lightweight, backward-compatible, well-scoped to the core resolver, and now fully consistent across Tests:
Job run · branch |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/praisonai-agents/praisonaiagents/tools/resolver.py (1)
77-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider logging exceptions instead of suppressing them.
The static analysis tools flagged multiple instances of catching a blind
Exceptionand passing without logging. While the intention here is to make tool-name discovery best-effort, silently swallowing all exceptions can mask underlying issues (e.g., syntax or attribute errors during module initialization). Consider logging these exceptions at the debug level to aid troubleshooting.💡 Proposed refactor to log exceptions
def _available_tool_names() -> List[str]: """Best-effort catalogue of known tool names for suggestions.""" names: set = set() try: from .registry import get_registry names.update(get_registry().list_tools()) - except Exception: - pass + except Exception as e: + logger.debug("Failed to list tools from registry: %s", e) try: from . import TOOL_MAPPINGS names.update(TOOL_MAPPINGS.keys()) - except Exception: - pass + except Exception as e: + logger.debug("Failed to read TOOL_MAPPINGS: %s", e) global _praisonai_tools_available if _praisonai_tools_available is None: _praisonai_tools_available = importlib.util.find_spec("praisonai_tools") is not None if _praisonai_tools_available: try: import praisonai_tools names.update(getattr(praisonai_tools, "__all__", []) or []) - except Exception: - pass + except Exception as e: + logger.debug("Failed to inspect praisonai_tools: %s", e) return sorted(names)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/praisonai-agents/praisonaiagents/tools/resolver.py` around lines 77 - 107, Update the exception handlers in _available_tool_names to log caught exceptions at debug level while preserving best-effort discovery and continued execution. Use the module’s existing logger if available, and include contextual messages for registry, TOOL_MAPPINGS, and praisonai_tools discovery failures.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/praisonai-agents/praisonaiagents/tools/resolver.py`:
- Around line 77-107: Update the exception handlers in _available_tool_names to
log caught exceptions at debug level while preserving best-effort discovery and
continued execution. Use the module’s existing logger if available, and include
contextual messages for registry, TOOL_MAPPINGS, and praisonai_tools discovery
failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5beb9b31-a1f9-4118-9386-eb53c4986b8d
📒 Files selected for processing (2)
src/praisonai-agents/praisonaiagents/tools/resolver.pysrc/praisonai-agents/tests/unit/tools/test_tool_resolver.py
Strict-mode toolset resolution flattened ToolResolutionError (a ValueError subclass) into a plain ValueError, dropping .unknown/.suggestions. Re-raise the typed error before the generic handler and export the resolver API from praisonaiagents.tools so strict callers can catch it. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
MervinPraison
added
pipeline/blocked:stale-final
praisonai-triage-agent
Bot
removed
the
pipeline/blocked:cooldown
1 participant
Fixes #3153
Summary
Mistyped/unknown tool names previously produced a silently tool-less agent — a log-only warning with no suggestion and no way to fail fast. This makes the canonical core resolver (
praisonaiagents/tools/resolver.py) loud and self-correcting, benefiting all three surfaces (Python, YAML, CLI) that share it.Changes (
src/praisonai-agents/praisonaiagents/tools/resolver.py)_available_tool_names()builds a catalogue from the registry,TOOL_MAPPINGS, and (optional)praisonai-tools__all__;_closest_names()usesdifflibto suggest near matches.resolve_tool_names(names, *, strict=None, on_unknown=None).strict=True(orPRAISONAI_STRICT_TOOLS=true) raises a typedToolResolutionErrorcarrying{unknown, suggestions}.Unknown tool 'internet_serch'. Did you mean 'internet_search'? Run 'praisonai tools list'.on_unknowncallback lets callers (e.g. the wrapper CLI) render suggestions their own way.Scope
Kept minimal and lightweight per AGENTS.md — the single canonical resolver is the correct and only place needed to fix Python + YAML silent-drops. No Agent params added, no new modules/exports beyond the typed error.
Tests
Added strict/env/callback/suggestion tests to
tests/unit/tools/test_tool_resolver.py. All 8 pass.Generated with Claude Code
Summary by CodeRabbit
New Features
PRAISONAI_STRICT_TOOLSenvironment setting.Bug Fixes