address pr comments

Author: bcallender

src/fenic/_backends/local/catalog.py

@@ -525,15 +525,14 @@ def create_tool(
     ) -> bool:
         """Create a new tool in the current catalog."""
         # Ensure the tool is valid by resolving it.
-        with self.lock:
-            tool_definition = bind_tool(tool_name, tool_description, tool_params, result_limit, tool_query)
-            cursor = self.db_conn.cursor()
-            if self.system_tables.describe_tool(cursor, tool_name):
-                if ignore_if_exists:
-                    return False
-                raise ToolAlreadyExistsError(tool_name)
-            self.system_tables.save_tool(cursor, tool_definition)
-            return True
+        tool_definition = bind_tool(tool_name, tool_description, tool_params, result_limit, tool_query)
+        cursor = self.db_conn.cursor()
+        if self._does_tool_exist(cursor, tool_name):
+            if ignore_if_exists:
+                return False
+            raise ToolAlreadyExistsError(tool_name)
+        self.system_tables.save_tool(cursor, tool_definition)
+        return True
 
     def list_tools(self) -> List[ParameterizedToolDefinition]:
         """List all tools in the current catalog."""
@@ -687,6 +686,14 @@ def get_metrics_for_session(self, session_id: str) -> Dict[str, float]:
         """Get metrics for a specific session from the metrics system read-only table."""
         return self.system_tables.get_metrics_for_session(self.db_conn.cursor(), session_id)
 
+    def _does_tool_exist(self, cursor: duckdb.DuckDBPyConnection, tool_name: str) -> bool:
+        try:
+            return self.system_tables.describe_tool(cursor, tool_name) is not None
+        except Exception as e:
+            raise CatalogError(
+                f"Failed to check if tool: {tool_name} exists"
+            ) from e
+
     def _does_table_exist(self, cursor: duckdb.DuckDBPyConnection, table_identifier: TableIdentifier) -> bool:
         try:
             return cursor.execute(

src/fenic/api/catalog.py

@@ -661,7 +661,7 @@ def describe_tool(self, tool_name: str) -> ParameterizedToolDefinition:
             tool_name (str): The name of the tool to get.
 
         Raises:
-            ToolNotFoundError: If the tool doesn't exist and ignore_if_not_exists is False
+            ToolNotFoundError: If the tool doesn't exist.
 
         Returns:
             Tool: The tool with the specified name.

src/fenic/scripts/fenic_serve.py

@@ -42,7 +42,7 @@
 from fenic.api.mcp.tool_generation import ToolGenerationConfig
 from fenic.api.session.config import SessionConfig
 from fenic.api.session.session import Session
-from fenic.core.error import ConfigurationError
+from fenic.core.error import ConfigurationError, ToolNotFoundError
 
 
 def _parse_args() -> argparse.Namespace:
@@ -101,7 +101,10 @@ def main() -> None:
     tools = []
     if args.tools:
         for tool_name in args.tools:
-            tools.append(session.catalog.describe_tool(tool_name))
+            try:
+                tools.append(session.catalog.describe_tool(tool_name))
+            except ToolNotFoundError as e:
+                raise ConfigurationError(f"Tool {tool_name} not found in the catalog.") from e
     else:
         tools = session.catalog.list_tools()
 

tools/mcp_demo_setup.py

@@ -186,8 +186,7 @@ class CandidateProfile(BaseModel):
         "experience",
         "job_category",
     )
-    if local_session.catalog.describe_tool("search_candidates"):
-        local_session.catalog.drop_tool("search_candidates")
+    local_session.catalog.drop_tool("search_candidates")
     local_session.catalog.create_tool(
         "search_candidates",
         "Search candidates by education, seniority, skills, and experience using regex patterns.",
@@ -234,8 +233,7 @@ class CandidateProfile(BaseModel):
             fc.tool_param("candidate_ids", fc.ArrayType(element_type=IntegerType))
         )
     ).select("candidate_id", "candidate_resume")
-    if local_session.catalog.describe_tool("candidate_resumes_by_candidate_ids"):
-        local_session.catalog.drop_tool("candidate_resumes_by_candidate_ids")
+    local_session.catalog.drop_tool("candidate_resumes_by_candidate_ids")
     local_session.catalog.create_tool(
         "candidate_resumes_by_candidate_ids",
         "Return the raw resumes for a list of candidate ids.",
@@ -323,8 +321,7 @@ class CandidateProfile(BaseModel):
             "job_category",
         )
     )
-    if local_session.catalog.describe_tool("candidates_for_job_description"):
-        local_session.catalog.drop_tool("candidates_for_job_description")
+    local_session.catalog.drop_tool("candidates_for_job_description")
     local_session.catalog.create_tool(
         "candidates_for_job_description",
         "Find candidates who are a good fit for a free-form job description using structured profiles.",
@@ -404,8 +401,7 @@ class CandidateProfile(BaseModel):
         ).alias("email"),
     )
     # Filter to a single candidate_id at runtime
-    if local_session.catalog.describe_tool("create_outreach_for_candidate"):
-        local_session.catalog.drop_tool("create_outreach_for_candidate")
+    local_session.catalog.drop_tool("create_outreach_for_candidate")
     local_session.catalog.create_tool(
         "create_outreach_for_candidate",
         "Create a personalized recruiting email for a candidate using resume and cover letter context.",