Bugfix/codeql fixes

.github/workflows/codeql.yml

@@ -38,6 +38,11 @@ jobs:
         with:
           languages: python
           queries: security-and-quality
+          config: |
+            paths-ignore:
+              # grpcio-tools output — regenerated from connector.proto, not hand-edited
+              - '**/*_pb2.py'
+              - '**/*_pb2_grpc.py'
 
       - name: Perform CodeQL Analysis
         uses: github/codeql-action/analyze@8272c299f21ca24af15dfe9ac0971ba969e5e0d5 # v3.36.2

playground/scenario_post_visit.py

@@ -75,23 +75,23 @@ async def run_scenario():
 
     print("\n=== STEP 1: Patient Discovery ===")
     patient_search_params = {"family": "Smith", "given": "Jason", "birthdate": "1985-01-01"}
-    logger.info(f"Searching for patient: {patient_search_params}")
+    logger.info("Searching for patient by fields: %s", ", ".join(sorted(patient_search_params)))
 
     try:
         patient_result = await connector.internal_execute(
             FhirPatientReadInput(action="read_patient", search_params=patient_search_params),
             trace_id=trace_id,
         )
         patient_id = patient_result.resource.get("id")
-        logger.info(f"Found Patient ID: {patient_id}")
+        logger.info("Patient resolved successfully")
     except Exception as e:
         logger.error(f"Patient search failed: {e}")
         return
 
     print("\n=== STEP 2: Encounter Identification ===")
     today = datetime.now(tz=timezone.utc).strftime("%Y-%m-%d")
     encounter_params = {"patient": patient_id, "status": "finished", "date": today}
-    logger.info(f"Finding encounter for patient {patient_id} on {today}")
+    logger.info(f"Finding encounter for resolved patient on {today}")
 
     try:
         enc_result = await connector.internal_execute(
@@ -116,7 +116,7 @@ async def run_scenario():
             return
 
         encounter_id = enc_result.resources[0].get("id")
-        logger.info(f"Selected Encounter ID: {encounter_id}")
+        logger.info("Encounter selected")
     except Exception as e:
         logger.error(f"Encounter search failed: {e}")
         return
@@ -142,7 +142,7 @@ async def run_scenario():
         context={"encounter": [{"reference": f"Encounter/{encounter_id}"}]},
     )
 
-    logger.info(f"Uploading clinical note for Encounter {encounter_id}")
+    logger.info("Uploading clinical note for selected encounter")
     try:
         doc_result = await connector.internal_execute(doc_input, trace_id=trace_id)
         logger.info(f"SUCCESS! Created DocumentReference: {doc_result.resource_id}")

playground/scenarios.py

@@ -16,6 +16,7 @@
 from pydantic import BaseModel, ValidationError, model_validator
 from dotenv import load_dotenv
 import os
+import sys
 import asyncio
 from node_wire_runtime.errors import ErrorMapper
 from node_wire_runtime.models import ErrorCategory
@@ -252,7 +253,6 @@ def _safe_error_return(
 ) -> ScenarioResponse:
     from node_wire_runtime.errors import ErrorMapper
     from node_wire_runtime.models import ErrorCategory
-    import logging
 
     log = logging.getLogger("playground.scenarios")
 
@@ -266,8 +266,8 @@ def _safe_error_return(
     if hasattr(e, "errors") and callable(getattr(e, "errors", None)):
         try:
             safe_msg = e.errors()[0].get("msg", "Schema validation failed")
-        except Exception:
-            pass
+        except Exception as detail_exc:
+            log.debug("Could not extract validation error detail: %s", detail_exc)
 
     steps[-1].status = "error"
     steps[-1].details = f"[{mapped_err.category.value}] {safe_msg}"
@@ -312,6 +312,12 @@ async def execute_with_retry(
             else:
                 logger.error(f"Action failed after {max_retries + 1} attempts: {e}")
                 raise last_exception
+    # Only reachable when max_retries < 0 leaves the loop with zero iterations.
+    raise (
+        last_exception
+        if last_exception
+        else RuntimeError(f"execute_with_retry made no attempts (max_retries={max_retries})")
+    )
 
 
 # Single shared factory for playground scenarios (matches REST: enabled + exposed_via includes "rest").
@@ -408,7 +414,7 @@ def add_step(
     add_step("Patient Discovery", "pending", display_name="Identify Patient")
     try:
         if payload.patient_id:
-            logger.info(f"Performing direct Patient ID lookup: {payload.patient_id}")
+            logger.info("Performing direct Patient ID lookup")
             p_res = await execute_with_retry(
                 connector, FhirPatientReadInput(resource_id=payload.patient_id), trace_id, steps[-1]
             )
@@ -423,7 +429,17 @@ def add_step(
                 }.items()
                 if v is not None
             }
-            logger.info(f"Searching for patient: {patient_search_params}")
+            # Log only literal field names (not the payload-derived dict) so no
+            # user-controlled data reaches the log record.
+            provided_fields = {
+                "family": payload.patient_family is not None,
+                "given": payload.patient_given is not None,
+                "birthdate": payload.patient_birthdate is not None,
+            }
+            logger.info(
+                "Searching for patient by fields: %s",
+                ", ".join(sorted(k for k, present in provided_fields.items() if present)),
+            )
             p_res = await execute_with_retry(
                 connector,
                 FhirPatientReadInput(search_params=patient_search_params),
@@ -455,16 +471,15 @@ def add_step(
     add_step("Encounter Identification", "pending", display_name="Locate Medical Visit")
     try:
         if payload.encounter_id:
-            logger.info(
-                f"Using manual Encounter ID: {payload.encounter_id}", extra={"trace_id": trace_id}
-            )
+            logger.info("Using manually supplied Encounter ID", extra={"trace_id": trace_id})
             encounter_id = payload.encounter_id
             enc_type = "Manual"
             enc_status = "verified"
         else:
             visit_date = payload.visit_date or datetime.now(tz=timezone.utc).strftime("%Y-%m-%d")
             logger.info(
-                f"Searching for encounter... patient={patient_id}, date={visit_date}",
+                "Searching for encounter for resolved patient on date=%s",
+                visit_date.replace("\r", "").replace("\n", ""),
                 extra={"trace_id": trace_id},
             )
             enc_res = await execute_with_retry(
@@ -505,7 +520,7 @@ def add_step(
                 raise ValueError("The found Encounter resource is missing a valid FHIR ID.")
 
         logger.info(
-            f"Selected Encounter: ID={encounter_id}, Type={enc_type}, Status={enc_status}",
+            f"Selected Encounter: Type={enc_type}, Status={enc_status}",
             extra={"trace_id": trace_id},
         )
 
@@ -717,8 +732,6 @@ def add_step(
         http_action = connector
         response = await execute_with_retry(http_action, request_input, trace_id, steps[-1])
 
-        import json
-
         resp_body = json.loads(response.body)
 
         steps[-1].status = "success"
@@ -760,8 +773,6 @@ def add_step(
     add_step("Audit", "pending", display_name="Update Audit Log")
     try:
         # Simulate background task
-        import asyncio
-
         await asyncio.sleep(0.4)
 
         steps[-1].status = "success"
@@ -800,7 +811,7 @@ def add_step(
     add_step("Patient Discovery", "pending", display_name="Identify Patient")
     try:
         if payload.patient_id:
-            logger.info(f"Cerner: direct Patient ID lookup: {payload.patient_id}")
+            logger.info("Cerner: direct Patient ID lookup")
             p_res = await execute_with_retry(
                 connector,
                 FhirCernerPatientReadInput(resource_id=payload.patient_id),
@@ -818,7 +829,17 @@ def add_step(
                 }.items()
                 if v
             }
-            logger.info(f"Cerner: searching for patient: {search_params}")
+            # Log only literal field names (not the payload-derived dict) so no
+            # user-controlled data reaches the log record.
+            provided_fields = {
+                "family": bool(payload.patient_family),
+                "given": bool(payload.patient_given),
+                "birthdate": bool(payload.patient_birthdate),
+            }
+            logger.info(
+                "Cerner: searching for patient by fields: %s",
+                ", ".join(sorted(k for k, present in provided_fields.items() if present)),
+            )
             p_res = await execute_with_retry(
                 connector,
                 FhirCernerPatientReadInput(search_params=search_params),
@@ -1009,8 +1030,8 @@ def add_step(
                     decoded_text = base64.b64decode(content[0]["attachment"]["data"]).decode(
                         "utf-8"
                     )
-            except Exception:
-                pass
+            except Exception as decode_exc:
+                logger.debug("Could not decode attachment content: %s", decode_exc)
 
             beautiful_data = {
                 "id": doc_res.resource_id,
@@ -1852,9 +1873,6 @@ async def agent_chat(payload: AgentChatInput) -> AgentChatResponse:
     Accepts a user message + conversation history, runs through the ToolHiveAgent,
     and returns the agent's reply with any tool steps executed.
     """
-    import os
-    import sys
-
     trace_id = str(uuid.uuid4())
     logger.info(
         "Agent Chat request | trace_id=%s | provider=%s",
@@ -2009,8 +2027,6 @@ async def agent_chat_stream(payload: AgentChatInput) -> Any:
 
     async def stream_events():
         try:
-            import sys
-
             from agents.llm_factory import LLMProviderFactory
             from agents.toolhive import (
                 MultiMcpClient,
@@ -2079,13 +2095,16 @@ async def stream_events():
                     yield json.dumps(event) + "\n"
 
         except Exception as exc:
-            logger.error("Agent Chat stream failed: %s", exc, exc_info=True)
             trace_id = str(uuid.uuid4())
+            logger.error("Agent Chat stream failed (trace_id=%s): %s", trace_id, exc, exc_info=True)
             yield (
                 json.dumps(
                     {
                         "type": "final_chunk",
-                        "content": f"Sorry, I encountered an error: {exc}. Please check the server configuration and try again.",
+                        "content": (
+                            "Sorry, I encountered an internal error. "
+                            f"Please check the server configuration and try again. trace_id={trace_id}"
+                        ),
                     }
                 )
                 + "\n"
@@ -2451,8 +2470,7 @@ def add_step(
     try:
         if payload.patient_id:
             logger.info(
-                "[ExtViewer] Direct Patient ID lookup: %s on %s",
-                payload.patient_id,
+                "[ExtViewer] Direct Patient ID lookup on %s",
                 system_label,
                 extra={"trace_id": trace_id},
             )
@@ -2488,9 +2506,16 @@ def add_step(
                 }.items()
                 if v
             }
+            # Log only literal field names (not the payload-derived dict) so no
+            # user-controlled data reaches the log record.
+            provided_fields = {
+                "family": bool(payload.patient_family),
+                "given": bool(payload.patient_given),
+                "birthdate": bool(payload.patient_birthdate),
+            }
             logger.info(
-                "[ExtViewer] Identity-layer search: %s on %s",
-                search_params,
+                "[ExtViewer] Identity-layer search by fields [%s] on %s",
+                ", ".join(sorted(k for k, present in provided_fields.items() if present)),
                 system_label,
                 extra={"trace_id": trace_id},
             )

src/agents/llm_base.py

@@ -0,0 +1,89 @@
+#
+# SPDX-FileCopyrightText: 2026 AOT Technologies
+# SPDX-License-Identifier: Apache-2.0
+#
+"""
+LLM Provider Base
+=================
+Provider-agnostic data models and the abstract provider interface.
+
+Kept separate from :mod:`agents.llm_factory` so provider implementations can
+depend on the interface without importing the factory (which imports the
+providers), avoiding a module-level import cycle.
+"""
+
+from __future__ import annotations
+
+from abc import ABC, abstractmethod
+from dataclasses import dataclass, field
+from typing import Any, Dict, List, Optional
+
+
+# ---------------------------------------------------------------------------
+# Data models (provider-agnostic)
+# ---------------------------------------------------------------------------
+
+
+@dataclass
+class ToolCall:
+    """A single tool-call request returned by the LLM."""
+
+    id: str
+    name: str
+    arguments: Dict[str, Any]
+
+
+@dataclass
+class LLMMessage:
+    """A single message in the conversation thread."""
+
+    role: str  # "system" | "user" | "assistant" | "tool"
+    content: Optional[str] = None
+    tool_calls: List[ToolCall] = field(default_factory=list)
+    tool_call_id: Optional[str] = None  # required for role="tool" responses
+    name: Optional[str] = None  # tool name for role="tool"
+
+
+@dataclass
+class LLMResponse:
+    """Raw response from the LLM."""
+
+    content: Optional[str]
+    tool_calls: List[ToolCall] = field(default_factory=list)
+    stop_reason: str = "stop"  # "stop" | "tool_calls"
+
+    @property
+    def wants_tool_call(self) -> bool:
+        return bool(self.tool_calls)
+
+
+# ---------------------------------------------------------------------------
+# Abstract base
+# ---------------------------------------------------------------------------
+
+
+class BaseLLMProvider(ABC):
+    """Common interface for all LLM providers."""
+
+    @abstractmethod
+    def chat_with_tools(
+        self,
+        messages: List[LLMMessage],
+        tools: List[Dict[str, Any]],
+    ) -> LLMResponse:
+        """
+        Send a conversation to the LLM, optionally with a set of tools.
+
+        Parameters
+        ----------
+        messages:
+            Full conversation history in provider-agnostic format.
+        tools:
+            List of MCP-style tool objects with ``name``, ``description``,
+            and ``input_schema`` keys.
+
+        Returns
+        -------
+        LLMResponse
+            The model's response, which may include tool_calls.
+        """