address pr comments

Author: bcallender

docs/topics/fenic-mcp.md

@@ -132,8 +132,8 @@ List, fetch, or drop tools:
 
 ```python
 all_tools = session.catalog.list_tools()
-age_tool = session.catalog.get_tool("users_by_age_range")
-search_tool = session.catalog.get_tool("users_by_name_regex")
+age_tool = session.catalog.describe_tool("users_by_age_range")
+search_tool = session.catalog.describe_tool("users_by_name_regex")
 session.catalog.drop_tool("users_by_age_range", ignore_if_not_exists=True)
 session.catalog.drop_tool("users_by_name_regex", ignore_if_not_exists=True)
 ```

src/fenic/_backends/cloud/catalog.py

@@ -341,7 +341,7 @@ def set_view_description(self, view_name: str, description: str) -> bool:
             "Set view description not implemented for cloud catalog"
         )
 
-    def get_tool(
+    def describe_tool(
         self,
         tool_name: str,
         ignore_if_not_exists: bool = True,

src/fenic/_backends/local/catalog.py

@@ -510,13 +510,11 @@ def set_view_description(self, view_name: str, description: Optional[str]) -> bo
                 ) from e
 
 
-    def get_tool(self, tool_name: str, ignore_if_not_exists: bool = True) -> Optional[ParameterizedToolDefinition]:
+    def describe_tool(self, tool_name: str) -> Optional[ParameterizedToolDefinition]:
         """Get a tool's metadata from the system table."""
         cursor = self.db_conn.cursor()
-        existing_tool = self.system_tables.get_tool(cursor, tool_name)
-        if existing_tool:
-            if ignore_if_not_exists:
-                return None
+        existing_tool = self.system_tables.describe_tool(cursor, tool_name)
+        if not existing_tool:
             raise ToolNotFoundError(tool_name)
         return existing_tool
 
@@ -533,7 +531,7 @@ def create_tool(
         # Ensure the tool is valid by resolving it.
         tool_definition = bind_tool(tool_name, tool_description, tool_params, result_limit, tool_query)
         cursor = self.db_conn.cursor()
-        if self.system_tables.get_tool(cursor, tool_name):
+        if self._does_tool_exist(cursor, tool_name):
             if ignore_if_exists:
                 return False
             raise ToolAlreadyExistsError(tool_name)
@@ -548,7 +546,7 @@ def list_tools(self) -> List[ParameterizedToolDefinition]:
     def drop_tool(self, tool_name: str, ignore_if_not_exists: bool = True) -> bool:
         """Drop a tool from the current catalog."""
         cursor = self.db_conn.cursor()
-        if not self.system_tables.get_tool(cursor, tool_name):
+        if not self._does_tool_exist(cursor, tool_name):
             if ignore_if_not_exists:
                 return False
             raise ToolNotFoundError(tool_name)
@@ -691,6 +689,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/_backends/local/system_table_client.py

@@ -486,7 +486,7 @@ def save_tool(self, cursor: duckdb.DuckDBPyConnection, tool: ParameterizedToolDe
                 f"Failed to save tool metadata for {tool.name}"
             ) from e
 
-    def get_tool(self, cursor: duckdb.DuckDBPyConnection, tool_name: str) -> Optional[ParameterizedToolDefinition]:
+    def describe_tool(self, cursor: duckdb.DuckDBPyConnection, tool_name: str) -> Optional[ParameterizedToolDefinition]:
         """Get a tool's metadata from the system table.
         Raises:
             CatalogError: If the tool metadata cannot be retrieved.

src/fenic/api/catalog.py

@@ -654,22 +654,19 @@ def drop_view(self, view_name: str, ignore_if_not_exists: bool = True) -> bool:
         return self.catalog.drop_view(view_name, ignore_if_not_exists)
 
     @validate_call(config=ConfigDict(strict=True))
-    def get_tool(self, tool_name: str, ignore_if_not_exists: bool = True) -> ParameterizedToolDefinition:
+    def describe_tool(self, tool_name: str) -> ParameterizedToolDefinition:
         """Returns the tool with the specified name from the current catalog.
 
         Args:
             tool_name (str): The name of the tool to get.
-            ignore_if_not_exists (bool): If True, return None when the tool doesn't exist.
-                If False, raise an error when the tool doesn't exist.
-                Defaults to True.
 
         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.
         """
-        return self.catalog.get_tool(tool_name, ignore_if_not_exists)
+        return self.catalog.describe_tool(tool_name)
 
     @validate_call(config=ConfigDict(strict=True, arbitrary_types_allowed=True))
     def create_tool(
@@ -771,6 +768,6 @@ def list_tools(self) -> List[ParameterizedToolDefinition]:
     listViews = list_views
     doesViewExist = does_view_exist
     dropView = drop_view
-    getTool = get_tool
+    getTool = describe_tool
     createTool = create_tool
     dropTool = drop_tool

src/fenic/core/_interfaces/catalog.py

@@ -145,7 +145,7 @@ def describe_view(self, view_name: str) -> DatasetMetadata:
         pass
 
     @abstractmethod
-    def get_tool(
+    def describe_tool(
         self,
         tool_name: str,
         ignore_if_not_exists: bool = True

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.get_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

@@ -122,8 +122,7 @@ class CandidateProfile(BaseModel):
     job_category_match = fc.coalesce(fc.tool_param("job_category_query", StringType).is_in(fc.col("job_category")), fc.lit(True))
     merged_filter = education_match & seniority_match & skills_match & experience_match & job_category_match
     search_candidates = candidates_df.filter(merged_filter).select("candidate_id", "first_name", "last_name", "education", "seniority", "skills", "experience", "job_category")
-    if local_session.catalog.get_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.",
@@ -140,8 +139,7 @@ class CandidateProfile(BaseModel):
 
     # Tool 2: candidate_resumes_by_candidate_ids -- given a list of candidate ids, return the raw resumes for each candidate
     candidate_resumes = candidates_df.filter(fc.col("candidate_id").is_in(fc.tool_param("candidate_ids", fc.ArrayType(element_type=IntegerType)))).select("candidate_id", "candidate_resume")
-    if local_session.catalog.get_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.",
@@ -208,8 +206,7 @@ class CandidateProfile(BaseModel):
     )
     job_category_match = fc.coalesce(fc.tool_param("job_category_query", StringType).is_in(fc.col("job_category")), fc.lit(True))
     candidates_for_job = candidates_df.filter(job_category_match).filter(fit_pred).select("candidate_id", "first_name", "last_name", "education", "seniority", "skills", "experience", "job_category")
-    if local_session.catalog.get_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.",
@@ -279,8 +276,7 @@ class CandidateProfile(BaseModel):
         ).alias("email"),
     )
     # Filter to a single candidate_id at runtime
-    if local_session.catalog.get_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.",