fix: dedupe tool schema build in tools/base.py

[praisonai-triage-agent]

Fixes #3125

Summary

get_sorted_tool_schemas() previously built every tool's schema twice per call: once inside validate_tool_schema_consistency() (which discarded them) and again in its own loop for sorting. The tool-shape schema-extraction block was also duplicated verbatim across both functions.

Changes

• Added a private _extract_schema(tool) -> Optional[dict] helper that resolves a schema from all supported tool shapes (BaseTool, objects with get_schema, plain callables).
• validate_tool_schema_consistency() now accepts return_schemas=False; when True it returns the schemas it already built.
• get_sorted_tool_schemas() now validates once and sorts the returned schemas instead of rebuilding them.

Behaviour preserved

• Same ToolValidationError on the same inputs.
• Identical returned schemas and alphabetical sort order.
• All three tool shapes still supported; public API unchanged (return_schemas defaults to False).

Validation

• tests/unit/context/test_cache_optimization.py — 3 passed.
• Added sanity check confirming validation, return_schemas, sort order, and empty-list handling.

Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved tool schema validation across supported tool types.
  • More reliably detects tools with missing or inconsistent schemas.
  • Improved schema sorting and consistency by reusing validated schemas.

[MervinPraison]

@coderabbitai review

[MervinPraison]

/review

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

Tool schema extraction is centralized in _extract_schema. Validation can optionally return collected schemas, and get_sorted_tool_schemas reuses them instead of rebuilding schemas.

Changes

Tool schema reuse

Layer / File(s)	Summary
Centralized schema validation src/praisonai-agents/praisonaiagents/tools/base.py	Adds shared schema extraction for supported tool shapes and extends validation with optional schema-return behavior while preserving consistency checks.
Reuse schemas during sorting src/praisonai-agents/praisonaiagents/tools/base.py	Updates sorted schema generation to reuse schemas produced by validation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: mervinpraison

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: deduplicating tool schema construction in tools/base.py.
Linked Issues check	✅ Passed	The changes match #3125 by centralizing schema extraction and reusing built schemas without changing validation, ordering, or supported tool shapes.
Out of Scope Changes check	✅ Passed	No clear out-of-scope changes are present; the edits stay focused on the schema deduplication work in tools/base.py.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/issue-3125-20260716-1713

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR removes duplicate tool-schema construction in the shared tools module. The main changes are:

• Adds a private helper for supported schema extraction paths.
• Adds an optional schema-list return mode to consistency validation.
• Reuses validated schemas when producing sorted output.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
src/praisonai-agents/praisonaiagents/tools/base.py	Consolidates schema extraction and reuses schemas built during validation without changing the default public behavior.

_{Reviews (1): Last reviewed commit: "fix: dedupe tool schema build in tools/b..." | Re-trigger Greptile}

[MervinPraison]

@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

1. Protocol-driven: check heavy implementations vs core SDK
2. Backward compatible: ensure zero feature regressions
3. Performance: no hot-path regressions
4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
6. Repo routing: agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (praisonai.sandbox entry point) — request changes if wrongly added to praisonaiagents/

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback
9. Push all code fixes directly to THIS branch (do NOT create a new PR)
10. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
11. If all issues are resolved, approve the PR / close the Issue
12. If blocking issues remain, request changes / leave clear action items

[praisonai-triage-agent]

Claude finished @MervinPraison's task —— View job

---

Final Architecture Review — PR #3128

• Read guidelines / AGENTS.md scope
• Review changed file tools/base.py
• Phase 1: Architecture validation
• Phase 2: Fix valid issues (none required)
• Phase 3: Run tests — 13 passed
• Final verdict

---

Phase 1 — Architecture Review

Criterion	Result
Protocol-driven / correct routing	✅ Pure internal refactor in praisonaiagents/tools/base.py; no new deps, no wrapper/tools/plugins concern
Backward compatible	✅ Public API unchanged — validate_tool_schema_consistency() still returns True/raises identically; return_schemas defaults to False; get_sorted_tool_schemas() output identical
Performance	✅ Improvement — removes a full second pass of schema construction per get_sorted_tool_schemas() call (schemas were previously built, discarded in validation, then rebuilt for sorting)
SDK value	✅ Genuine value: eliminates duplicated schema-extraction block, cuts redundant work on the tool-prep path, no surface-area growth
Agent class bloat	✅ N/A — no Agent changes

Phase 2 — Issues

Reviewer verdicts: Greptile 5/5 safe to merge, CodeRabbit still processing, Qodo paused. No blocking issues raised.

Independent findings:

• Behavioral equivalence verified. The old get_sorted_tool_schemas had a defensive else: continue for unsupported tool shapes. This is safely dropped because validate_tool_schema_consistency already calls validate_tool() first, which raises ToolValidationError for any unsupported type before sorting is ever reached. Same shapes, same errors.
• _extract_schema correctly returns None for unsupported shapes; the if schema is None branch preserves the original ToolValidationError(f"Cannot extract schema from tool at index {i}").
• Empty-list handling preserved ([] vs True gated by return_schemas).

No code changes required — the fix is already minimal, correct, and complete.

Phase 3 — Tests

tests/unit/context/test_cache_optimization.py ...      [ 23%]
tests/test_tool_schema_parity.py ..........            [100%]
======================== 13 passed in 0.59s ========================

Verdict: ✅ Approve

Clean, lightweight de-duplication with a measurable hot-path win and zero behavior/API regression. Recommend merge.
· claude/issue-3125-20260716-1713

[coderabbitai]

🧹 Nitpick comments (1)

src/praisonai-agents/praisonaiagents/tools/base.py (1)

445-445: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Replace getattr with direct attribute access.

As indicated by static analysis tools, calling getattr with a constant attribute name is redundant and an anti-pattern. You can safely use direct property access here since hasattr already confirms the attribute exists.

♻️ Proposed refactor

-    elif hasattr(tool, 'get_schema') and callable(getattr(tool, 'get_schema')):
+    elif hasattr(tool, 'get_schema') and callable(tool.get_schema):

🤖 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/base.py` at line 445, Update the
get_schema capability check to use direct attribute access in the callable test
instead of getattr, while retaining the existing hasattr guard and conditional
behavior.

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/base.py`:
- Line 445: Update the get_schema capability check to use direct attribute
access in the callable test instead of getattr, while retaining the existing
hasattr guard and conditional behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2a5c46d7-093d-4c9b-b83b-1ea6cf35b39e

📥 Commits

Reviewing files that changed from the base of the PR and between 254f1ef and f449c75.

📒 Files selected for processing (1)

• src/praisonai-agents/praisonaiagents/tools/base.py