mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-13 00:15:05 +00:00
Remove union type annotations from tools.
Codex can't deal with them, I suspect ChatGPT will also have problems. This leads to slightly less expressive python code, but it isn't a large problem and is worth the extension in usability for serena.
This commit is contained in:
1 parent
65544d57d4
commit
cdc0d178f8
3 files changed
+127
-16
No files matched your search
+113
-4
@@ -6,6 +6,7 @@ import sys
|
||||
from abc import abstractmethod
|
||||
from collections.abc import AsyncIterator, Iterator, Sequence
|
||||
from contextlib import asynccontextmanager
|
||||
from copy import deepcopy
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, Literal, cast
|
||||
|
||||
@@ -57,12 +58,119 @@ class SerenaMCPFactory:
|
||||
self.project = project
|
||||
|
||||
@staticmethod
|
||||
def make_mcp_tool(tool: Tool) -> MCPTool:
|
||||
def _sanitize_for_openai_tools(schema: dict) -> dict:
|
||||
"""
|
||||
This method was written by GPT-5, I have not reviewed it in detail.
|
||||
Only called when `openai_tool_compatible` is True.
|
||||
|
||||
Make a Pydantic/JSON Schema object compatible with OpenAI tool schema.
|
||||
- 'integer' -> 'number' (+ multipleOf: 1)
|
||||
- remove 'null' from union type arrays
|
||||
- coerce integer-only enums to number
|
||||
- best-effort simplify oneOf/anyOf when they only differ by integer/number
|
||||
"""
|
||||
s = deepcopy(schema)
|
||||
|
||||
def walk(node):
|
||||
if not isinstance(node, dict):
|
||||
# lists get handled by parent calls
|
||||
return node
|
||||
|
||||
# ---- handle type ----
|
||||
t = node.get("type")
|
||||
if isinstance(t, str):
|
||||
if t == "integer":
|
||||
node["type"] = "number"
|
||||
# preserve existing multipleOf but ensure it's integer-like
|
||||
if "multipleOf" not in node:
|
||||
node["multipleOf"] = 1
|
||||
elif isinstance(t, list):
|
||||
# remove 'null' (OpenAI tools don't support nullables)
|
||||
t2 = [x if x != "integer" else "number" for x in t if x != "null"]
|
||||
if not t2:
|
||||
# fall back to object if it somehow becomes empty
|
||||
t2 = ["object"]
|
||||
node["type"] = t2[0] if len(t2) == 1 else t2
|
||||
if "integer" in t or "number" in t2:
|
||||
# if integers were present, keep integer-like restriction
|
||||
node.setdefault("multipleOf", 1)
|
||||
|
||||
# ---- enums of integers -> number ----
|
||||
if "enum" in node and isinstance(node["enum"], list):
|
||||
vals = node["enum"]
|
||||
if vals and all(isinstance(v, int) for v in vals):
|
||||
node.setdefault("type", "number")
|
||||
# keep them as ints; JSON 'number' covers ints
|
||||
node.setdefault("multipleOf", 1)
|
||||
|
||||
# ---- simplify anyOf/oneOf if they only differ by integer/number ----
|
||||
for key in ("oneOf", "anyOf"):
|
||||
if key in node and isinstance(node[key], list):
|
||||
simplified = []
|
||||
changed = False
|
||||
for sub in node[key]:
|
||||
sub = walk(sub) # recurse
|
||||
simplified.append(sub)
|
||||
# If all subs are the same after integer→number, collapse
|
||||
try:
|
||||
import json
|
||||
|
||||
canon = [json.dumps(x, sort_keys=True) for x in simplified]
|
||||
if len(set(canon)) == 1:
|
||||
# copy the single schema up
|
||||
only = simplified[0]
|
||||
node.pop(key, None)
|
||||
for k, v in only.items():
|
||||
if k not in node:
|
||||
node[k] = v
|
||||
changed = True
|
||||
except Exception:
|
||||
pass
|
||||
if not changed:
|
||||
node[key] = simplified
|
||||
|
||||
# ---- recurse into known schema containers ----
|
||||
for child_key in ("properties", "patternProperties", "definitions", "$defs"):
|
||||
if child_key in node and isinstance(node[child_key], dict):
|
||||
for k, v in list(node[child_key].items()):
|
||||
node[child_key][k] = walk(v)
|
||||
|
||||
# arrays/items
|
||||
if "items" in node:
|
||||
node["items"] = walk(node["items"])
|
||||
|
||||
# allOf/if/then/else - pass through with integer→number conversions applied inside
|
||||
for key in ("allOf",):
|
||||
if key in node and isinstance(node[key], list):
|
||||
node[key] = [walk(x) for x in node[key]]
|
||||
|
||||
if "if" in node:
|
||||
node["if"] = walk(node["if"])
|
||||
if "then" in node:
|
||||
node["then"] = walk(node["then"])
|
||||
if "else" in node:
|
||||
node["else"] = walk(node["else"])
|
||||
|
||||
return node
|
||||
|
||||
return walk(s)
|
||||
|
||||
@staticmethod
|
||||
def make_mcp_tool(tool: Tool, openai_tool_compatible: bool = True) -> MCPTool:
|
||||
"""
|
||||
Create an MCP tool from a Serena Tool instance.
|
||||
|
||||
:param tool: The Serena Tool instance to convert.
|
||||
:param openai_tool_compatible: whether to process the tool schema to be compatible with OpenAI tools
|
||||
(doesn't accept integer, needs number instead, etc.). This allows using Serena MCP within codex.
|
||||
"""
|
||||
func_name = tool.get_name()
|
||||
func_doc = tool.get_apply_docstring() or ""
|
||||
func_arg_metadata = tool.get_apply_fn_metadata()
|
||||
is_async = False
|
||||
parameters = func_arg_metadata.arg_model.model_json_schema()
|
||||
if openai_tool_compatible:
|
||||
parameters = SerenaMCPFactory._sanitize_for_openai_tools(parameters)
|
||||
|
||||
docstring = docstring_parser.parse(func_doc)
|
||||
|
||||
@@ -113,12 +221,12 @@ class SerenaMCPFactory:
|
||||
pass
|
||||
|
||||
# noinspection PyProtectedMember
|
||||
def _set_mcp_tools(self, mcp: FastMCP) -> None:
|
||||
def _set_mcp_tools(self, mcp: FastMCP, openai_tool_compatible: bool = False) -> None:
|
||||
"""Update the tools in the MCP server"""
|
||||
if mcp is not None:
|
||||
mcp._tool_manager._tools = {}
|
||||
for tool in self._iter_tools():
|
||||
mcp_tool = self.make_mcp_tool(tool)
|
||||
mcp_tool = self.make_mcp_tool(tool, openai_tool_compatible=openai_tool_compatible)
|
||||
mcp._tool_manager._tools[tool.get_name()] = mcp_tool
|
||||
log.info(f"Starting MCP server with {len(mcp._tool_manager._tools)} tools: {list(mcp._tool_manager._tools.keys())}")
|
||||
|
||||
@@ -225,6 +333,7 @@ class SerenaMCPFactorySingleProcess(SerenaMCPFactory):
|
||||
|
||||
@asynccontextmanager
|
||||
async def server_lifespan(self, mcp_server: FastMCP) -> AsyncIterator[None]:
|
||||
self._set_mcp_tools(mcp_server)
|
||||
openai_tool_compatible = self.context.name in ["chatgpt", "codex"]
|
||||
self._set_mcp_tools(mcp_server, openai_tool_compatible=openai_tool_compatible)
|
||||
log.info("MCP server lifetime setup complete")
|
||||
yield
|
||||
@@ -289,8 +289,8 @@ class SearchForPatternTool(Tool):
|
||||
substring_pattern: str,
|
||||
context_lines_before: int = 0,
|
||||
context_lines_after: int = 0,
|
||||
paths_include_glob: str | None = None,
|
||||
paths_exclude_glob: str | None = None,
|
||||
paths_include_glob: str = "",
|
||||
paths_exclude_glob: str = "",
|
||||
relative_path: str = "",
|
||||
restrict_search_to_code_files: bool = False,
|
||||
max_answer_chars: int = TOOL_DEFAULT_MAX_ANSWER_LENGTH,
|
||||
@@ -327,10 +327,10 @@ class SearchForPatternTool(Tool):
|
||||
:param context_lines_after: Number of lines of context to include after each match
|
||||
:param paths_include_glob: optional glob pattern specifying files to include in the search.
|
||||
Matches against relative file paths from the project root (e.g., "*.py", "src/**/*.ts").
|
||||
Only matches files, not directories.
|
||||
Only matches files, not directories. If left empty, all non-ignored files will be included.
|
||||
:param paths_exclude_glob: optional glob pattern specifying files to exclude from the search.
|
||||
Matches against relative file paths from the project root (e.g., "*test*", "**/*_generated.py").
|
||||
Takes precedence over paths_include_glob. Only matches files, not directories.
|
||||
Takes precedence over paths_include_glob. Only matches files, not directories. If left empty, no files are excluded.
|
||||
:param relative_path: only subpaths of this path (relative to the repo root) will be analyzed. If a path to a single
|
||||
file is passed, only that will be searched. The path must exist, otherwise a `FileNotFoundError` is raised.
|
||||
:param max_answer_chars: if the output is longer than this number of characters,
|
||||
@@ -355,8 +355,8 @@ class SearchForPatternTool(Tool):
|
||||
relative_path=relative_path,
|
||||
context_lines_before=context_lines_before,
|
||||
context_lines_after=context_lines_after,
|
||||
paths_include_glob=paths_include_glob,
|
||||
paths_exclude_glob=paths_exclude_glob,
|
||||
paths_include_glob=paths_include_glob.strip(),
|
||||
paths_exclude_glob=paths_exclude_glob.strip(),
|
||||
)
|
||||
else:
|
||||
if os.path.isfile(abs_path):
|
||||
|
||||
@@ -87,10 +87,10 @@ class FindSymbolTool(Tool, ToolMarkerSymbolicRead):
|
||||
self,
|
||||
name_path: str,
|
||||
depth: int = 0,
|
||||
relative_path: str | None = None,
|
||||
relative_path: str = "",
|
||||
include_body: bool = False,
|
||||
include_kinds: list[int] | None = None,
|
||||
exclude_kinds: list[int] | None = None,
|
||||
include_kinds: list[int] = [], # noqa: B006
|
||||
exclude_kinds: list[int] = [], # noqa: B006
|
||||
substring_matching: bool = False,
|
||||
max_answer_chars: int = TOOL_DEFAULT_MAX_ANSWER_LENGTH,
|
||||
) -> str:
|
||||
@@ -136,8 +136,10 @@ class FindSymbolTool(Tool, ToolMarkerSymbolicRead):
|
||||
:param include_kinds: Optional. List of LSP symbol kind integers to include. (e.g., 5 for Class, 12 for Function).
|
||||
Valid kinds: 1=file, 2=module, 3=namespace, 4=package, 5=class, 6=method, 7=property, 8=field, 9=constructor, 10=enum,
|
||||
11=interface, 12=function, 13=variable, 14=constant, 15=string, 16=number, 17=boolean, 18=array, 19=object,
|
||||
20=key, 21=null, 22=enum member, 23=struct, 24=event, 25=operator, 26=type parameter
|
||||
20=key, 21=null, 22=enum member, 23=struct, 24=event, 25=operator, 26=type parameter.
|
||||
If not provided, all kinds are included.
|
||||
:param exclude_kinds: Optional. List of LSP symbol kind integers to exclude. Takes precedence over `include_kinds`.
|
||||
If not provided, no kinds are excluded.
|
||||
:param substring_matching: If True, use substring matching for the last segment of `name`.
|
||||
:param max_answer_chars: Max characters for the JSON result. If exceeded, no content is returned.
|
||||
:return: a list of symbols (with locations) matching the name.
|
||||
@@ -167,8 +169,8 @@ class FindReferencingSymbolsTool(Tool, ToolMarkerSymbolicRead):
|
||||
self,
|
||||
name_path: str,
|
||||
relative_path: str,
|
||||
include_kinds: list[int] | None = None,
|
||||
exclude_kinds: list[int] | None = None,
|
||||
include_kinds: list[int] = [], # noqa: B006
|
||||
exclude_kinds: list[int] = [], # noqa: B006
|
||||
max_answer_chars: int = TOOL_DEFAULT_MAX_ANSWER_LENGTH,
|
||||
) -> str:
|
||||
"""
|
||||
|
||||
Reference in new issue
Block a user