Skip to content

refactor: apply best practices across core SDK#259

Merged
moonpyt merged 2 commits intoXSpoonAi:mainfrom
veithly:feat/best-practices-update
Feb 25, 2026
Merged

refactor: apply best practices across core SDK#259
moonpyt merged 2 commits intoXSpoonAi:mainfrom
veithly:feat/best-practices-update

Conversation

@veithly
Copy link
Collaborator

@veithly veithly commented Feb 12, 2026

Summary

  • Fix version inconsistency between .bumpversion.cfg (0.2.0) and pyproject.toml (0.4.0)
  • Modernize type hints to PEP 604/585 across llm/manager.py, llm/config.py, llm/errors.py, and agents/base.py (Optional[X] -> X | None, List[X] -> list[X])
  • Replace overly broad except Exception with specific except ImportError in __init__.py
  • Downgrade noisy middleware unavailability warning from WARNING to DEBUG level in agents/base.py
  • Clean up pyproject.toml placeholder metadata and inline comments
  • Add schema caching to ToolManager.to_params() to avoid re-serialization every agent step
  • Preserve custom prompts in SpoonReactAI when passed via constructor (prevents _refresh_prompts() from overwriting caller-supplied values)

Test plan

  • All module imports verified (spoon_ai, llm.errors, llm.config, llm.manager, agents.base)
  • Full test suite run: 145 passed, 34 pre-existing failures, 6 pre-existing errors (all confirmed as pre-existing by running against base commit)
  • No new test regressions introduced

- Fix version inconsistency between .bumpversion.cfg (0.2.0) and pyproject.toml (0.4.0)
- Modernize type hints to PEP 604/585 (Optional[X] -> X | None, List[X] -> list[X])
- Replace overly broad exception handling (Exception) with specific types (ImportError)
- Downgrade noisy middleware warning to debug level
- Clean up pyproject.toml placeholder metadata and inline comments
- Add schema caching to ToolManager.to_params() to avoid re-serialization
- Preserve custom prompts in SpoonReactAI when passed via constructor
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2f2ea9db38

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +55 to +56
if self._params_cache is not None and self._params_cache_count == len(self.tools):
return self._params_cache

Choose a reason for hiding this comment

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

P1 Badge Invalidate tool schema cache when tool identities change

to_params() now treats the cache as valid whenever len(self.tools) is unchanged, but existing agent code mutates ToolManager.tools/tool_map directly (for example CustomAgent.add_tool replaces duplicate names via remove+append while keeping the same count). In that path, to_params() can return stale schemas for the previous tool objects, so subsequent LLM tool-calling runs use outdated names/parameters and may fail or call the wrong signature.

Useful? React with 👍 / 👎.

@moonpyt moonpyt merged commit c236fee into XSpoonAi:main Feb 25, 2026
1 check passed
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.

2 participants