Skip to content

Commit ec055b4

Browse files
committed
address pr comments
1 parent 98c0eb1 commit ec055b4

File tree

4 files changed

+26
-20
lines changed

4 files changed

+26
-20
lines changed

src/fenic/_backends/local/catalog.py

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -525,15 +525,14 @@ def create_tool(
525525
) -> bool:
526526
"""Create a new tool in the current catalog."""
527527
# Ensure the tool is valid by resolving it.
528-
with self.lock:
529-
tool_definition = bind_tool(tool_name, tool_description, tool_params, result_limit, tool_query)
530-
cursor = self.db_conn.cursor()
531-
if self.system_tables.describe_tool(cursor, tool_name):
532-
if ignore_if_exists:
533-
return False
534-
raise ToolAlreadyExistsError(tool_name)
535-
self.system_tables.save_tool(cursor, tool_definition)
536-
return True
528+
tool_definition = bind_tool(tool_name, tool_description, tool_params, result_limit, tool_query)
529+
cursor = self.db_conn.cursor()
530+
if self._does_tool_exist(cursor, tool_name):
531+
if ignore_if_exists:
532+
return False
533+
raise ToolAlreadyExistsError(tool_name)
534+
self.system_tables.save_tool(cursor, tool_definition)
535+
return True
537536

538537
def list_tools(self) -> List[ParameterizedToolDefinition]:
539538
"""List all tools in the current catalog."""
@@ -687,6 +686,14 @@ def get_metrics_for_session(self, session_id: str) -> Dict[str, float]:
687686
"""Get metrics for a specific session from the metrics system read-only table."""
688687
return self.system_tables.get_metrics_for_session(self.db_conn.cursor(), session_id)
689688

689+
def _does_tool_exist(self, cursor: duckdb.DuckDBPyConnection, tool_name: str) -> bool:
690+
try:
691+
return self.system_tables.describe_tool(cursor, tool_name) is not None
692+
except Exception as e:
693+
raise CatalogError(
694+
f"Failed to check if tool: {tool_name} exists"
695+
) from e
696+
690697
def _does_table_exist(self, cursor: duckdb.DuckDBPyConnection, table_identifier: TableIdentifier) -> bool:
691698
try:
692699
return cursor.execute(

src/fenic/api/catalog.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ def describe_tool(self, tool_name: str) -> ParameterizedToolDefinition:
661661
tool_name (str): The name of the tool to get.
662662
663663
Raises:
664-
ToolNotFoundError: If the tool doesn't exist and ignore_if_not_exists is False
664+
ToolNotFoundError: If the tool doesn't exist.
665665
666666
Returns:
667667
Tool: The tool with the specified name.

src/fenic/scripts/fenic_serve.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
from fenic.api.mcp.tool_generation import ToolGenerationConfig
4343
from fenic.api.session.config import SessionConfig
4444
from fenic.api.session.session import Session
45-
from fenic.core.error import ConfigurationError
45+
from fenic.core.error import ConfigurationError, ToolNotFoundError
4646

4747

4848
def _parse_args() -> argparse.Namespace:
@@ -101,7 +101,10 @@ def main() -> None:
101101
tools = []
102102
if args.tools:
103103
for tool_name in args.tools:
104-
tools.append(session.catalog.describe_tool(tool_name))
104+
try:
105+
tools.append(session.catalog.describe_tool(tool_name))
106+
except ToolNotFoundError as e:
107+
raise ConfigurationError(f"Tool {tool_name} not found in the catalog.") from e
105108
else:
106109
tools = session.catalog.list_tools()
107110

tools/mcp_demo_setup.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,7 @@ class CandidateProfile(BaseModel):
122122
job_category_match = fc.coalesce(fc.tool_param("job_category_query", StringType).is_in(fc.col("job_category")), fc.lit(True))
123123
merged_filter = education_match & seniority_match & skills_match & experience_match & job_category_match
124124
search_candidates = candidates_df.filter(merged_filter).select("candidate_id", "first_name", "last_name", "education", "seniority", "skills", "experience", "job_category")
125-
if local_session.catalog.describe_tool("search_candidates"):
126-
local_session.catalog.drop_tool("search_candidates")
125+
local_session.catalog.drop_tool("search_candidates")
127126
local_session.catalog.create_tool(
128127
"search_candidates",
129128
"Search candidates by education, seniority, skills, and experience using regex patterns.",
@@ -140,8 +139,7 @@ class CandidateProfile(BaseModel):
140139

141140
# Tool 2: candidate_resumes_by_candidate_ids -- given a list of candidate ids, return the raw resumes for each candidate
142141
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")
143-
if local_session.catalog.describe_tool("candidate_resumes_by_candidate_ids"):
144-
local_session.catalog.drop_tool("candidate_resumes_by_candidate_ids")
142+
local_session.catalog.drop_tool("candidate_resumes_by_candidate_ids")
145143
local_session.catalog.create_tool(
146144
"candidate_resumes_by_candidate_ids",
147145
"Return the raw resumes for a list of candidate ids.",
@@ -208,8 +206,7 @@ class CandidateProfile(BaseModel):
208206
)
209207
job_category_match = fc.coalesce(fc.tool_param("job_category_query", StringType).is_in(fc.col("job_category")), fc.lit(True))
210208
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")
211-
if local_session.catalog.describe_tool("candidates_for_job_description"):
212-
local_session.catalog.drop_tool("candidates_for_job_description")
209+
local_session.catalog.drop_tool("candidates_for_job_description")
213210
local_session.catalog.create_tool(
214211
"candidates_for_job_description",
215212
"Find candidates who are a good fit for a free-form job description using structured profiles.",
@@ -279,8 +276,7 @@ class CandidateProfile(BaseModel):
279276
).alias("email"),
280277
)
281278
# Filter to a single candidate_id at runtime
282-
if local_session.catalog.describe_tool("create_outreach_for_candidate"):
283-
local_session.catalog.drop_tool("create_outreach_for_candidate")
279+
local_session.catalog.drop_tool("create_outreach_for_candidate")
284280
local_session.catalog.create_tool(
285281
"create_outreach_for_candidate",
286282
"Create a personalized recruiting email for a candidate using resume and cover letter context.",

0 commit comments

Comments
 (0)