diff --git a/EVALUATIONS.md b/EVALUATIONS.md index 3057819..1d08d1d 100644 --- a/EVALUATIONS.md +++ b/EVALUATIONS.md @@ -414,7 +414,7 @@ scorer = ( client.monitor.judge_scorers .builder( name="Strict grading", - judge_model="claude-opus-4-8", # any model id your judge key can reach (list_models() on the hosted platform) + judge_model="claude-opus-5", # any model id your judge key can reach (list_models() on the hosted platform) judge_prompt="""You are grading a customer support response. **User Query:** {input} @@ -863,7 +863,7 @@ report.statistics.rouge_score Cases where `expected_results` is empty or the agent returned an error are skipped from the average, so a sparse dataset still produces a meaningful score. If a toggle wasn't on for the dataset, that property returns `None`. -These four metrics also appear per-model (as `average_bleu_score`/`average_rouge_score` alongside `average_vector_similarity`/`average_jaccard_similarity`) when a dataset selects multiple comparison models, in each model's row of `report.sovereignty_index.models`. +These four metrics also appear per-model (as `average_bleu_score`/`average_rouge_score` alongside `average_vector_similarity`/`average_jaccard_similarity`) in each model's row of `report.sovereignty_index.models` when a run compares multiple models. Multi-model comparison comes from the judge scorer's sovereignty models - `client.monitor.judge_scorers.builder(sovereignty_models=[...])` - because self-host ignores `sovereigntyIndex` on the dataset and grading-config routes. ### CI gate (self-host) @@ -902,7 +902,7 @@ This run + gate flow is the **self-host CI path**. The separate CI-runs API in [ report = client.evaluations.run(...).execute(my_agent).finalize().analyze( mode="auto", # "auto" | "sync" | "batch" quality_mode="quality_first", # "quality_first" | "balanced" - judges=["gpt-5.6-luna", "claude-opus-4-8"], # 1-3 model ids; omit for the platform default judge + judges=["gpt-5.6-luna", "claude-opus-5"], # 1-3 model ids; omit for the platform default judge ) report.summary # str | None, overall narrative summary diff --git a/TRACING.md b/TRACING.md index a481dda..de42152 100644 --- a/TRACING.md +++ b/TRACING.md @@ -520,6 +520,8 @@ print(pattern.id) client.monitor.patterns.get(pattern.id) # -> MonitorPattern client.monitor.patterns.list() # -> list[MonitorPattern] +client.monitor.patterns.update(pattern.id, enabled=False) # sparse update, wire camelCase keys -> MonitorPattern +client.monitor.patterns.delete(pattern.id) # historical signals remain as history ``` #### `builder()` parameters @@ -539,9 +541,12 @@ client.monitor.patterns.list() # -> list[MonitorPattern] | `enabled` | `bool` | `True` | Whether the pattern is checked at all | | `sample_rate` | `float` | `1.0` | Fraction of matching traces to actually check, `0.0`-`1.0` | | `scope_mode` / `agent_ids` | `str` / `list[str]` | `"all"` / `[]` | Restrict this pattern to specific agents instead of the whole workspace | +| `conditions` | `list[dict]` | `None` | Self-host: the engine's full N-condition model - a list of condition dicts, each with its own detector kind and match settings, passed through verbatim. When set, the engine honors it as the pattern's whole rule set and the flat fields above become display-only metadata | `publish()` returns a `MonitorPattern` with `.id`, which you pass in `pattern_ids` at trace time. +Note: `match_mode` is write-only on self-host - the engine folds it into the pattern's stored conditions, and the pattern always reads back with `match_mode="any"` regardless of what was sent. The `"all"` semantics still apply when matching. + ### `client.monitor.signals` Read back the alerts/findings a pattern match (or a built-in detector) produced, without opening the dashboard. Read-only: a signal is the system's output from checking traces against patterns, not something you create directly. diff --git a/agentx/agentx.py b/agentx/agentx.py index 18b3e6f..09eb04f 100644 --- a/agentx/agentx.py +++ b/agentx/agentx.py @@ -4,6 +4,7 @@ import logging from agentx.util import get_headers, api_base, normalize_base +from agentx.exceptions import AgentXError from agentx.resources.agent import Agent from agentx.resources.workforce import Workforce @@ -136,6 +137,11 @@ def from_env(cls) -> "AgentX": return cls(base_url=base_url) if base_url else cls() def get_agent(self, id: str) -> Agent: + """Fetch one hosted-platform agent by id. + + Hosted platform only - the self-host engine does not serve /access/agents; + use ``client.monitor.agents.list()`` for self-host agent rows instead. + """ url = f"{self.base_url or api_base()}/access/agents/{id}" # Make a GET request to the AgentX API response = requests.get(url, headers=get_headers(self.api_key)) @@ -143,9 +149,17 @@ def get_agent(self, id: str) -> Agent: if response.status_code == 200: return Agent(**response.json()) else: - raise Exception(f"Failed to retrieve agent: {response.reason}") + raise AgentXError( + f"Failed to retrieve agent: {response.reason}. This endpoint is " + "hosted-platform only - on self-host use client.monitor.agents.list()." + ) def list_agents(self) -> List[Agent]: + """List the hosted platform's agents. + + Hosted platform only - the self-host engine does not serve /access/agents; + use ``client.monitor.agents.list()`` for self-host agent rows instead. + """ url = f"{self.base_url or api_base()}/access/agents" # Make a GET request to the AgentX API response = requests.get(url, headers=get_headers(self.api_key)) @@ -153,7 +167,10 @@ def list_agents(self) -> List[Agent]: if response.status_code == 200: return [Agent(**agent) for agent in response.json()] else: - raise Exception(f"Failed to list agents: {response.reason}") + raise AgentXError( + f"Failed to list agents: {response.reason}. This endpoint is " + "hosted-platform only - on self-host use client.monitor.agents.list()." + ) @staticmethod def list_workforces() -> List["Workforce"]: @@ -214,12 +231,18 @@ def ping(self) -> dict: return {"ok": True, "base_url": base} def get_profile(self): - """Get the current user's profile information.""" + """Get the current user's profile information. + + Hosted platform only - the self-host engine does not serve /access/getProfile; + self-host agent/monitoring data lives under ``client.monitor`` (e.g. + ``client.monitor.agents.list()``). + """ url = f"{self.base_url or api_base()}/access/getProfile" response = requests.get(url, headers=get_headers(self.api_key)) if response.status_code == 200: return response.json() else: - raise Exception( - f"Failed to get profile: {response.status_code} - {response.reason}" + raise AgentXError( + f"Failed to get profile: {response.status_code} - {response.reason}. " + "This endpoint is hosted-platform only - on self-host use client.monitor." ) diff --git a/agentx/evaluations/client.py b/agentx/evaluations/client.py index e7de74c..e4c8456 100644 --- a/agentx/evaluations/client.py +++ b/agentx/evaluations/client.py @@ -35,7 +35,6 @@ SDK_NAME = "agentx-python" _RETRYABLE_STATUS = {429, 500, 502, 503, 504} -_MAX_RETRIES = 3 _RETRY_BACKOFF = [1.0, 2.0, 4.0] # The self-host analyze route judges every result before it responds, so the client has to @@ -182,8 +181,8 @@ def _request( if resp.status_code == 422: raise AgentXValidationError(resp.text) # Gate on the schedule itself so HTTP-status retries walk the SAME full backoff - # schedule connection errors do - the old `attempt < _MAX_RETRIES - 1` gate left - # the schedule's last entry unreachable for HTTP retries (ingest_client precedent). + # schedule connection errors do - an earlier fixed retry-count gate left the + # schedule's last entry unreachable for HTTP retries (ingest_client precedent). if ( resp.status_code in _RETRYABLE_STATUS and retry @@ -248,7 +247,9 @@ def delete_dataset(self, dataset_id: str) -> None: """Deletes the dataset, its grading config, and both version histories. Past runs are kept (their dataset reference degrades to a bare id). The engine refuses (409) when the dataset's config is attached to a live scorer.""" - self._request("DELETE", f"/datasets/{dataset_id}") + # retry=False: a lost response + transport retry would turn a successful + # delete into a spurious 404. + self._request("DELETE", f"/datasets/{dataset_id}", retry=False) def list_datasets(self) -> List[Dataset]: data = self._request("GET", "/datasets", params=self._workspace_params()) @@ -440,6 +441,12 @@ def analyze_run( quality_mode: Optional[str] = None, judges: Optional[List[str]] = None, ) -> Dict[str, Any]: + """Start the qualitative AI-analysis job for a run. + + ``mode`` ("auto"/"sync"/"batch") is hosted-only: self-host engines run the analysis + synchronously and ignore it - check the response's mode field for what actually ran + (mirrors EvaluationRun.analyze's docstring). + """ # Starts the durable analysis job and returns immediately (e.g. {"jobId": ..., "status": # "pending"}); poll get_analysis_status() until it reaches a terminal status, then call # get_report(). mode/quality_mode/judges mirror the dashboard's AnalyzeEvaluationRequest. @@ -506,9 +513,10 @@ def get_report(self, run_id: str) -> Report: return self._report_from_dashboard(run_id) def get_missing_results(self, run_id: str) -> List[Dict[str, Any]]: - """Deprecated: on self-host the route's response body has no top-level list, so this - always returns ``[]``. Use :meth:`get_submitted_keys` - the same route's - ``submittedKeys`` - to find out what a run still needs.""" + """Deprecated: on self-host the route returns ``missing: []`` deliberately empty - + the engine cannot know the client's case list - so this always returns ``[]``. + Use :meth:`get_submitted_keys` - the same route's ``submittedKeys`` - to find out + what a run still needs.""" import warnings warnings.warn( @@ -517,8 +525,10 @@ def get_missing_results(self, run_id: str) -> List[Dict[str, Any]]: DeprecationWarning, stacklevel=2, ) - data = self._request("GET", f"/runs/{run_id}/missing-results") - return data if isinstance(data, list) else data.get("missing", []) + # No request at all: the route returns `missing: []` deliberately empty (the engine + # cannot know the client's case list), so the round-trip only ever bought an empty + # result. + return [] def get_submitted_keys(self, run_id: str) -> List[str]: """Idempotency keys this run has already accepted - what execute() uses to resume a diff --git a/agentx/evaluations/datasets.py b/agentx/evaluations/datasets.py index f96c12b..c2063b8 100644 --- a/agentx/evaluations/datasets.py +++ b/agentx/evaluations/datasets.py @@ -2,6 +2,7 @@ import csv import logging +import warnings from pathlib import Path from typing import Any, Dict, List, Optional, TYPE_CHECKING, Union @@ -21,7 +22,8 @@ class DatasetBuilder: ``judge_prompt``/``judge_model`` are LLM-as-judge overrides for the dataset's grading config. NOTE (self-host): the engine's dataset-create route currently ignores both - set them on a judge scorer / the evaluation settings instead. ``sovereignty_models`` - is accepted on the wire but not acted on by the self-host engine. + is dropped by the self-host engine on this route - use + ``client.monitor.judge_scorers.builder(sovereignty_models=...)`` which persists it. """ def __init__( @@ -93,8 +95,9 @@ def __init__( if rouge_score: self._payload["rougeScore"] = {"enabled": True} # Sovereignty & Portability - the models to compare on this dataset (use - # client.evaluations.list_models() to discover valid ids). Self-host: accepted on - # the wire but not acted on by the engine (see class docstring). + # client.evaluations.list_models() to discover valid ids). Self-host: dropped by + # the engine on this route - use client.monitor.judge_scorers.builder( + # sovereignty_models=...) which persists it (see class docstring). if sovereignty_models: self._payload["sovereigntyIndex"] = { "enabled": True, @@ -161,9 +164,11 @@ def add_case( main["smokeTest"] = {"enabled": True, "count": smoke_test_count} if smoke_test_guidance: main["smokeTest"]["guidance"] = smoke_test_guidance - if expected_tools: + # `is not None`, not truthiness: an explicit empty list is a real assertion (an empty + # expectedTrajectory means "this case calls no tools") and must reach the wire. + if expected_tools is not None: main["expectedTrajectory"] = {"tools": expected_tools, "mode": trajectory_match_mode} - if expected_retrieval_context: + if expected_retrieval_context is not None: main["expectedRetrievalContext"] = expected_retrieval_context if splits: main["splits"] = splits @@ -178,6 +183,15 @@ def add_case( def publish(self) -> Dataset: if not self._payload["questions"]: raise ValueError("Dataset must have at least one case before publishing") + # Warn at publish time, where the request is known: the engine's dataset-create + # route drops sovereigntyIndex, so comparison models set here never persist. + sov = self._payload.get("sovereigntyIndex") + if isinstance(sov, dict) and sov.get("models"): + warnings.warn( + "Self-host ignores sovereigntyIndex on datasets/grading configs - use " + "judge_scorers.builder(sovereignty_models=...) for model comparison runs.", + stacklevel=2, + ) logger.info( "Publishing dataset '%s' with %d case(s)", self._payload["name"], diff --git a/agentx/evaluations/evaluation_settings.py b/agentx/evaluations/evaluation_settings.py index f6f1dcd..325f7b5 100644 --- a/agentx/evaluations/evaluation_settings.py +++ b/agentx/evaluations/evaluation_settings.py @@ -64,8 +64,9 @@ def __init__( if rouge_score: self._payload["rougeScore"] = {"enabled": True} # Sovereignty & Portability - the models to compare when this config runs - # (use client.evaluations.list_models() to discover valid ids). Self-host: accepted - # on the wire but not acted on by the engine (same caveat as DatasetBuilder's). + # (use client.evaluations.list_models() to discover valid ids). Self-host: dropped + # by the engine on this route - use client.monitor.judge_scorers.builder( + # sovereignty_models=...) which persists it (same caveat as DatasetBuilder's). if sovereignty_models: self._payload["sovereigntyIndex"] = { "enabled": True, @@ -91,6 +92,15 @@ def __init__( ] def publish(self) -> EvaluationSettings: + # Warn at publish time, where the request is known: the engine's settings-create + # route drops sovereigntyIndex, so comparison models set here never persist. + sov = self._payload.get("sovereigntyIndex") + if isinstance(sov, dict) and sov.get("models"): + warnings.warn( + "Self-host ignores sovereigntyIndex on datasets/grading configs - use " + "judge_scorers.builder(sovereignty_models=...) for model comparison runs.", + stacklevel=2, + ) logger.info("Publishing evaluation settings '%s'", self._payload["name"]) return self._client.create_evaluation_settings(self._payload) diff --git a/agentx/evaluations/models.py b/agentx/evaluations/models.py index 8330666..22c63f5 100644 --- a/agentx/evaluations/models.py +++ b/agentx/evaluations/models.py @@ -179,27 +179,22 @@ class Config: # Evaluation subject # --------------------------------------------------------------------------- -FrameworkKind = Literal[ - "raw_python", - "openai", - "anthropic", - "google", - "langchain", - "llamaindex", - "crewai", - "autogen", - "n8n", - "flowise", - "other", -] - RuntimeKind = Literal["local", "ci", "customer_hosted", "low_code"] class EvaluationSubject(BaseModel): + """Describes the agent under evaluation. + + ``framework`` is an open string - the engine accepts any label (it also stamps + values like ``openai-agents``, ``langgraph``, ``google-genai``, ``litellm`` from + the tracing integrations). Common values: ``raw_python``, ``openai``, + ``anthropic``, ``google``, ``langchain``, ``llamaindex``, ``crewai``, + ``autogen``, ``n8n``, ``flowise``, ``other``. + """ + kind: Literal["custom_agent", "agentx_agent", "agentx_team"] = "custom_agent" display_name: Optional[str] = Field(default=None, alias="displayName") - framework: Optional[FrameworkKind] = None + framework: Optional[str] = None framework_version: Optional[str] = Field(default=None, alias="frameworkVersion") runtime: Optional[RuntimeKind] = "local" agent_instructions: Optional[str] = Field(default=None, alias="agentInstructions") diff --git a/agentx/evaluations/reporting.py b/agentx/evaluations/reporting.py index af5b0e7..d74b8ed 100644 --- a/agentx/evaluations/reporting.py +++ b/agentx/evaluations/reporting.py @@ -172,7 +172,7 @@ def print_report(report: Report) -> None: if report.low_scoring_cases: _section("Low-scoring Cases (rating <= 5)") for case in report.low_scoring_cases[:5]: - q = (case.get("query") or case.get("questionText", ""))[:80] + q = (case.get("query") or case.get("questionText") or "")[:80] rating = case.get("rating", "?") justification = case.get("justification", "") print(f" {red(f'[{rating}]')} {q}") diff --git a/agentx/evaluations/runner.py b/agentx/evaluations/runner.py index 232bf2a..5814147 100644 --- a/agentx/evaluations/runner.py +++ b/agentx/evaluations/runner.py @@ -229,6 +229,7 @@ def produce(case: EvaluationCase) -> EvaluationResult: ) return normalized(case) + executor = None if concurrency > 1: import concurrent.futures import contextvars @@ -308,6 +309,12 @@ def bounded() -> "Iterator[EvaluationResult]": # generator happens to be garbage-collected. if results_iter is not None: results_iter.close() + # And release the pool itself here too: close() on a NEVER-STARTED generator + # (e.g. every case was already submitted, so next() was never called) does not + # run bounded()'s finally - its executor.shutdown would never fire. shutdown() + # is idempotent, so the double call on the normal path is harmless. + if executor is not None: + executor.shutdown(wait=False, cancel_futures=True) # Flush the trailing partial batch HERE, not after the try: a mid-run exception # (agent crash, Ctrl-C) used to discard up to max_batch - 1 already-paid-for # results still waiting in it. @@ -546,7 +553,7 @@ def analyze( response's mode field for what actually ran. quality_mode: "quality_first" or "balanced" - how many items get a second/third judge. judges: 1-3 model ids from ``client.evaluations.list_models()``, e.g. - ``["gpt-5.6-luna", "claude-opus-4-8"]``. Omit to let the engine score with its + ``["gpt-5.6-luna", "claude-opus-5"]``. Omit to let the engine score with its platform default model (a single judge, rather than the dashboard's 3-judge default - SDK runs are typically lighter-weight, quick-start evaluations). poll_interval: seconds between status checks while waiting. @@ -566,16 +573,27 @@ def analyze( judges=judges, ) deadline = time.monotonic() + timeout - status = self._client.get_analysis_status(self._run.run_id) - while not status.is_terminal and time.monotonic() < deadline: - level = _ANALYSIS_LEVEL_LABELS.get(status.progress.current_level, "") - spinner.update( - f"Analyzing, {level + ' ' if level else ''}{status.progress.overall_percentage}%" - ) + status = None + while True: + try: + status = self._client.get_analysis_status(self._run.run_id) + except Exception as poll_exc: + # One transient status-poll failure (network blip, engine restart) + # must not abort the whole wait - the job keeps running + # server-side, so keep polling until the deadline. + logger.debug("Analysis status poll failed: %s", poll_exc) + if status is not None and status.is_terminal: + break + if time.monotonic() >= deadline: + break + if status is not None: + level = _ANALYSIS_LEVEL_LABELS.get(status.progress.current_level, "") + spinner.update( + f"Analyzing, {level + ' ' if level else ''}{status.progress.overall_percentage}%" + ) time.sleep(poll_interval) - status = self._client.get_analysis_status(self._run.run_id) - if not status.is_terminal: + if status is None or not status.is_terminal: _say(f" {yellow('!')} Still running after {int(timeout)}s, check the dashboard for status") elif status.status == "failed": reason = status.failure_reason.message if status.failure_reason else "unknown error" diff --git a/agentx/integrations/crewai.py b/agentx/integrations/crewai.py index 7c13076..bddd845 100644 --- a/agentx/integrations/crewai.py +++ b/agentx/integrations/crewai.py @@ -84,7 +84,7 @@ def kickoff(self, crew: Any, inputs: Optional[Dict[str, Any]] = None) -> Any: if task_timings: execution_steps, _ = self._build_steps_from_timings(task_timings, task_outputs) elif task_outputs: - execution_steps, _ = self._build_steps_evenly_divided(task_outputs, latency_ms) + execution_steps, _ = self._build_steps_evenly_divided(task_outputs, latency_ms, start) # Each task becomes its own real child span. tool_calls isn't passed to # _merge_child_run here: _build_steps_from_timings/_build_steps_evenly_divided both @@ -262,25 +262,33 @@ def _build_steps_from_timings( return execution_steps, tool_calls - def _build_steps_evenly_divided(self, task_outputs: List[Any], latency_ms: float) -> tuple: + def _build_steps_evenly_divided(self, task_outputs: List[Any], latency_ms: float, start: float) -> tuple: """ Fallback for CrewAI versions predating the events module: no per-task timing is available, so attribute the total latency evenly across tasks so the timeline still sums to the measured wall-clock - duration. + duration. Each synthesized step also gets a real start_time/end_time + (consecutive per_step_s slices from the kickoff's start) so the + dashboard timeline can position it, not just size it. """ tool_calls: List[Dict[str, Any]] = [] execution_steps: List[Dict[str, Any]] = [] - for task_out in task_outputs: + per_step_ms = latency_ms / len(task_outputs) + per_step_s = per_step_ms / 1000.0 + for i, task_out in enumerate(task_outputs): description = getattr(task_out, "description", "task") name = description[:100] task_output = str(getattr(task_out, "raw", "")) tool_calls.append({"name": name, "input": description, "output": task_output}) - execution_steps.append({"name": name, "duration_ms": 0, "input": description, "output": task_output}) - - per_step_ms = latency_ms / len(execution_steps) - for step in execution_steps: - step["duration_ms"] = per_step_ms + step_start = start + i * per_step_s + execution_steps.append({ + "name": name, + "duration_ms": per_step_ms, + "start_time": step_start, + "end_time": step_start + per_step_s, + "input": description, + "output": task_output, + }) return execution_steps, tool_calls @@ -307,4 +315,6 @@ def observe( framework="crewai", session_id=session_id or self._session_id, sync=sync, + # Same statement kickoff() makes: the root of a crew run is the agent run itself. + span_kind="agent", ) diff --git a/agentx/integrations/google_adk.py b/agentx/integrations/google_adk.py index c5a1948..a2cb57e 100644 --- a/agentx/integrations/google_adk.py +++ b/agentx/integrations/google_adk.py @@ -20,12 +20,15 @@ """ from __future__ import annotations +import logging import time -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple from uuid import uuid4 from agentx.tracing.tracer import Tracer, _safe_serialize +logger = logging.getLogger(__name__) + try: from google.adk.plugins.base_plugin import BasePlugin except ImportError as exc: # pragma: no cover @@ -94,23 +97,34 @@ def __init__( name: str = "google-adk-agent", metadata: Optional[Dict[str, Any]] = None, session_id: Optional[str] = None, + max_run_age_seconds: float = 3600.0, ) -> None: super().__init__(name="agentx") self._tracer = tracer self._agent_name = name self._metadata = metadata self._session_id = session_id + # Safety net mirroring langchain.py's _prune_stale_entries: state is normally popped + # in after_run_callback, but an invocation whose end callback never fires (hard + # crash, ADK bug) would leak forever in this long-lived plugin. Entries older than + # this are swept out at the top of before_run_callback. + self._max_run_age_seconds = max_run_age_seconds # invocation_id → accumulated run state self._runs: Dict[str, Dict[str, Any]] = {} - # invocation_id → pre-buffered user input text + # invocation_id → (pre-buffered user input text, buffered-at time) # (on_user_message_callback fires *before* before_run_callback) - self._pending_inputs: Dict[str, str] = {} - # id(tool_context) → start time float - self._tool_starts: Dict[int, float] = {} - # invocation_id → stack of model call start times (FIFO) + self._pending_inputs: Dict[str, Tuple[str, float]] = {} + # (invocation_id, tool name) → FIFO list of start times, mirroring _model_starts: + # parallel same-name tool calls each push their own start, so a second start no + # longer overwrites the first (a scalar here lost the first call's timing). Keyed + # by invocation_id, not id(tool_context): ADK creates fresh context objects per + # callback (see _model_starts' comment), and a freed context's id() can be + # recycled by an unrelated object, pairing a start with the wrong end. + self._tool_starts: Dict[Tuple[str, str], List[float]] = {} + # invocation_id → FIFO list of per-call dicts ({"start", "model", "input"}) # ADK creates new CallbackContext objects for before/after model callbacks, # so we cannot use id(callback_context) as a key - use invocation_id instead. - self._model_starts: Dict[str, List[float]] = {} + self._model_starts: Dict[str, List[Dict[str, Any]]] = {} # ------------------------------------------------------------------ # Run lifecycle @@ -124,9 +138,64 @@ async def on_user_message_callback( inv_id = invocation_context.invocation_id text = _content_to_text(user_message) if text: - self._pending_inputs[inv_id] = text + self._pending_inputs[inv_id] = (text, time.time()) + + def _prune_stale_entries(self) -> None: + """Sweep out invocation entries older than max_run_age_seconds - see __init__'s comment.""" + cutoff = time.time() - self._max_run_age_seconds + swept = 0 + + stale_inv_ids = [ + inv_id + for inv_id, state in list(self._runs.items()) + if (getattr(state.get("root_span"), "_start", None) or 0) < cutoff + ] + for inv_id in stale_inv_ids: + self._runs.pop(inv_id, None) + self._model_starts.pop(inv_id, None) + self._pending_inputs.pop(inv_id, None) + for key in [k for k in self._tool_starts if k[0] == inv_id]: + self._tool_starts.pop(key, None) + swept += len(stale_inv_ids) + + # Orphaned per-call state whose invocation state is already gone (or never existed) - + # each entry carries its own timestamp. + stale_model_ids = [ + inv_id + for inv_id, starts in list(self._model_starts.items()) + if inv_id not in self._runs + and (not starts or max(call.get("start", 0) for call in starts) < cutoff) + ] + for inv_id in stale_model_ids: + self._model_starts.pop(inv_id, None) + swept += len(stale_model_ids) + + stale_tool_keys = [ + key + for key, starts in list(self._tool_starts.items()) + if not starts or max(starts) < cutoff + ] + for key in stale_tool_keys: + self._tool_starts.pop(key, None) + swept += len(stale_tool_keys) + + stale_input_ids = [ + inv_id for inv_id, pending in list(self._pending_inputs.items()) if pending[1] < cutoff + ] + for inv_id in stale_input_ids: + self._pending_inputs.pop(inv_id, None) + swept += len(stale_input_ids) + + if swept: + logger.warning( + "AgentXADKPlugin swept %d in-flight invocation record(s) older than %.0fs - " + "their end callbacks never fired, so their traces were never sent", + swept, + self._max_run_age_seconds, + ) async def before_run_callback(self, *, invocation_context: Any) -> None: + self._prune_stale_entries() inv_id = invocation_context.invocation_id agent_name = getattr(invocation_context.agent, "name", None) or self._agent_name # Held directly (not relied on via tracer.current_span) - ADK callbacks for one @@ -134,7 +203,13 @@ async def before_run_callback(self, *, invocation_context: Any) -> None: # same thread/task, so state["root_span"] (keyed by invocation_id, same as everything # else here) is the reliable way to address the right parent. root_span = self._tracer.trace( - agent_name, framework="google-adk", metadata=self._metadata, session_id=self._session_id + agent_name, + framework="google-adk", + metadata=self._metadata, + session_id=self._session_id, + # The root of a standalone runner invocation is the agent run itself + # (langchain/llamaindex/crewai/autogen parity). + span_kind="agent", ) # Deliberately NOT root_span.__enter__() - the same reasoning as openai_agents' # on_trace_start: enter pushes onto the CALLING context's active-span stack, but ADK @@ -146,10 +221,11 @@ async def before_run_callback(self, *, invocation_context: Any) -> None: root_span._start = time.time() if root_span._session_id is None: root_span._session_id = f"sdk_{uuid4().hex}" + pending_input = self._pending_inputs.pop(inv_id, None) self._runs[inv_id] = { "root_span": root_span, "llm_call_count": 0, - "input": self._pending_inputs.pop(inv_id, None), + "input": pending_input[0] if pending_input else None, "output": None, "model": None, "error": None, @@ -160,6 +236,10 @@ async def before_run_callback(self, *, invocation_context: Any) -> None: async def after_run_callback(self, *, invocation_context: Any) -> None: inv_id = invocation_context.invocation_id state = self._runs.pop(inv_id, None) + # Drop this invocation's per-call leftovers too - an unpaired before_model_callback + # (or an input buffered after the run started) would otherwise leak here forever. + self._model_starts.pop(inv_id, None) + self._pending_inputs.pop(inv_id, None) if state is None: return # This invocation's own detail already went out as child-span rows via child_span() in @@ -285,7 +365,10 @@ async def on_model_error_callback( async def before_tool_callback( self, *, tool: Any, tool_args: Dict[str, Any], tool_context: Any ) -> None: - self._tool_starts[id(tool_context)] = time.time() + inv_id = tool_context.get_invocation_context().invocation_id + # FIFO list per (invocation, tool name) - parallel same-name calls each queue + # their own start (see __init__'s comment). + self._tool_starts.setdefault((inv_id, getattr(tool, "name", "unknown")), []).append(time.time()) async def after_tool_callback( self, @@ -299,9 +382,15 @@ async def after_tool_callback( state = self._runs.get(inv_id) if state is None: return - start_t = self._tool_starts.pop(id(tool_context), None) - end_t = time.time() tool_name = getattr(tool, "name", "unknown") + # Pop the earliest queued start (FIFO, _model_starts' pairing), dropping the + # key once its list drains so entries don't accumulate. + key = (inv_id, tool_name) + starts = self._tool_starts.get(key, []) + start_t = starts.pop(0) if starts else None + if not starts: + self._tool_starts.pop(key, None) + end_t = time.time() tool_input = _safe_serialize(tool_args) tool_output = str(result) if result is not None else None state["root_span"].child_span( @@ -331,9 +420,15 @@ async def on_tool_error_callback( state = self._runs.get(inv_id) if state is None: return - start_t = self._tool_starts.pop(id(tool_context), None) - end_t = time.time() tool_name = getattr(tool, "name", "unknown") + # Pop the earliest queued start (FIFO, _model_starts' pairing), dropping the + # key once its list drains so entries don't accumulate. + key = (inv_id, tool_name) + starts = self._tool_starts.get(key, []) + start_t = starts.pop(0) if starts else None + if not starts: + self._tool_starts.pop(key, None) + end_t = time.time() tool_input = _safe_serialize(tool_args) tool_output = f"ERROR: {error}" state["root_span"].child_span( diff --git a/agentx/integrations/langchain.py b/agentx/integrations/langchain.py index 268aeaa..9cf8246 100644 --- a/agentx/integrations/langchain.py +++ b/agentx/integrations/langchain.py @@ -383,9 +383,12 @@ def on_chain_start( parent_run_id: Optional[UUID] = None, **kwargs, ) -> None: - self._parents[run_id] = parent_run_id is_top = parent_run_id is None - self._top_level[run_id] = is_top + # Written under the lock: _prune_stale_entries iterates these dicts from other + # threads. Taken and released BEFORE the prune below - the lock is not reentrant. + with self._state_lock: + self._parents[run_id] = parent_run_id + self._top_level[run_id] = is_top if is_top: self._prune_stale_entries() # Consume any retrieval steps that ran before this chain started @@ -527,7 +530,10 @@ def resolve_parent(parent_id: Any): input=step.get("query"), output=step.get("output"), error=step.get("error"), - metadata={"kind": "retrieval"}, + metadata={ + "kind": "retrieval", + **({"doc_count": step["doc_count"]} if step.get("doc_count") is not None else {}), + }, span_kind="retrieval", ) @@ -544,15 +550,19 @@ def on_chain_end( # internals) fire on_chain_start/on_chain_end too - leaving their # entries behind here would leak forever in a long-lived singleton # handler, since nothing else ever cleans up a non-top-level run_id. - is_top = self._top_level.pop(run_id, None) + # Pops run under _state_lock: the prune sweep iterates these dicts. + with self._state_lock: + is_top = self._top_level.pop(run_id, None) if not is_top: # Close the node record before dropping this run's _parents entry - the top-ancestor # walk inside _finalize_node still needs it. self._finalize_node(run_id, output=_extract_output(outputs)) - self._parents.pop(run_id, None) + with self._state_lock: + self._parents.pop(run_id, None) return - self._parents.pop(run_id, None) - state = self._runs.pop(run_id, None) + with self._state_lock: + self._parents.pop(run_id, None) + state = self._runs.pop(run_id, None) if state is None: return output = _extract_output(outputs) @@ -622,14 +632,18 @@ def on_chain_error( parent_run_id: Optional[UUID] = None, **kwargs, ) -> None: - # See on_chain_end's comment - pop for every chain run, not just top-level. - is_top = self._top_level.pop(run_id, None) + # See on_chain_end's comment - pop for every chain run, not just top-level, + # and under _state_lock (the prune sweep iterates these dicts). + with self._state_lock: + is_top = self._top_level.pop(run_id, None) if not is_top: self._finalize_node(run_id, error=str(error)) - self._parents.pop(run_id, None) + with self._state_lock: + self._parents.pop(run_id, None) return - self._parents.pop(run_id, None) - state = self._runs.pop(run_id, None) + with self._state_lock: + self._parents.pop(run_id, None) + state = self._runs.pop(run_id, None) if state is None: return @@ -688,7 +702,9 @@ def _record_llm_start( messages: Optional[List[Any]] = None, ) -> None: """Shared logic for on_llm_start and on_chat_model_start.""" - self._parents[run_id] = parent_run_id + # Written under the lock - the pruner iterates _parents from other threads. + with self._state_lock: + self._parents[run_id] = parent_run_id kw = serialized.get("kwargs", {}) model = ( kw.get("model_name") @@ -702,7 +718,10 @@ def _record_llm_start( if top_for_tools and top_for_tools in self._runs and not self._runs[top_for_tools].get("tool_definitions"): captured = capture_tool_definitions(kwargs.get("invocation_params", {}).get("tools")) if captured: - self._runs[top_for_tools]["tool_definitions"] = captured + # Write under _state_lock - the prune sweep iterates _runs. + with self._state_lock: + if top_for_tools in self._runs: + self._runs[top_for_tools]["tool_definitions"] = captured # Insert under the lock _prune_stale_entries' iteration holds - LangGraph can start # LLM runs from worker threads while another thread's on_chain_start prunes. with self._state_lock: @@ -745,8 +764,10 @@ def on_llm_end( parent_run_id: Optional[UUID] = None, **kwargs, ) -> None: - llm_state = self._runs.pop(run_id, None) - self._parents.pop(run_id, None) + # Pops under _state_lock - the prune sweep iterates these dicts. + with self._state_lock: + llm_state = self._runs.pop(run_id, None) + self._parents.pop(run_id, None) if llm_state: start_t = llm_state.get("llm_start") end_t = time.time() @@ -806,9 +827,9 @@ def on_tool_start( parent_run_id: Optional[UUID] = None, **kwargs, ) -> None: - self._parents[run_id] = parent_run_id - # Insert under the prune's lock - see _record_llm_start. + # Both writes under the prune's lock - see _record_llm_start. with self._state_lock: + self._parents[run_id] = parent_run_id self._runs[run_id] = { "tool_name": serialized.get("name", "unknown"), "tool_input": input_str, @@ -823,8 +844,10 @@ def on_tool_end( parent_run_id: Optional[UUID] = None, **kwargs, ) -> None: - state = self._runs.pop(run_id, None) - self._parents.pop(run_id, None) + # Pops under _state_lock - the prune sweep iterates these dicts. + with self._state_lock: + state = self._runs.pop(run_id, None) + self._parents.pop(run_id, None) if state is None: return end_t = time.time() @@ -856,8 +879,10 @@ def on_tool_error( parent_run_id: Optional[UUID] = None, **kwargs, ) -> None: - state = self._runs.pop(run_id, None) - self._parents.pop(run_id, None) + # Pops under _state_lock - the prune sweep iterates these dicts. + with self._state_lock: + state = self._runs.pop(run_id, None) + self._parents.pop(run_id, None) if state is None: return end_t = time.time() @@ -890,9 +915,9 @@ def on_retriever_start( parent_run_id: Optional[UUID] = None, **kwargs, ) -> None: - self._parents[run_id] = parent_run_id - # Insert under the prune's lock - see _record_llm_start. + # Both writes under the prune's lock - see _record_llm_start. with self._state_lock: + self._parents[run_id] = parent_run_id self._retrieval_starts[run_id] = {"start": time.time(), "query": query} def on_retriever_end( @@ -903,8 +928,10 @@ def on_retriever_end( parent_run_id: Optional[UUID] = None, **kwargs, ) -> None: - state = self._retrieval_starts.pop(run_id, None) - self._parents.pop(run_id, None) + # Pops under _state_lock - the prune sweep iterates these dicts. + with self._state_lock: + state = self._retrieval_starts.pop(run_id, None) + self._parents.pop(run_id, None) if state is None: return end_t = time.time() @@ -947,8 +974,10 @@ def on_retriever_error( parent_run_id: Optional[UUID] = None, **kwargs, ) -> None: - state = self._retrieval_starts.pop(run_id, None) - self._parents.pop(run_id, None) + # Pops under _state_lock - the prune sweep iterates these dicts. + with self._state_lock: + state = self._retrieval_starts.pop(run_id, None) + self._parents.pop(run_id, None) if state is None: return end_t = time.time() diff --git a/agentx/integrations/litellm.py b/agentx/integrations/litellm.py index 461fae6..2ea99ff 100644 --- a/agentx/integrations/litellm.py +++ b/agentx/integrations/litellm.py @@ -15,6 +15,7 @@ """ from __future__ import annotations +import time from typing import Any, Dict, Optional, Tuple from agentx.tracing.tracer import Tracer, _safe_serialize @@ -29,6 +30,20 @@ ) from exc +def _to_epoch_seconds(value: Any) -> float: + """LiteLLM usually hands the CustomLogger datetimes, but some code paths (and older + releases) pass raw epoch floats/ints or nothing at all - accept all three instead of + crashing the whole callback on a missing .timestamp().""" + if hasattr(value, "timestamp"): # datetime + try: + return float(value.timestamp()) + except Exception: + return time.time() + if isinstance(value, (int, float)) and not isinstance(value, bool): + return float(value) + return time.time() + + def _extract_output_text(response: Any) -> Optional[str]: """ Extract the assistant's text reply from a LiteLLM ``ModelResponse``, @@ -116,8 +131,8 @@ def _finish(self, kwargs: Dict[str, Any], response_obj: Any, start_time: Any, en framework="litellm", metadata=self._metadata, session_id=self._session_id, - start_t=start_time.timestamp(), - end_t=end_time.timestamp(), + start_t=_to_epoch_seconds(start_time), + end_t=_to_epoch_seconds(end_time), input_repr=input_repr, output=output, model=model, diff --git a/agentx/integrations/llamaindex.py b/agentx/integrations/llamaindex.py index fae41c8..1e55767 100644 --- a/agentx/integrations/llamaindex.py +++ b/agentx/integrations/llamaindex.py @@ -247,97 +247,106 @@ def on_event_end( **kwargs: Any, ) -> None: payload = payload or {} - start_info = self._starts.pop(event_id, None) - parent_id = self._parents.pop(event_id, None) - is_root = self._roots.pop(event_id, False) - root_id = event_id if is_root else self._find_root(parent_id) - state = self._runs.get(root_id) if root_id else None + # The whole read-modify-write runs under _state_lock, mirroring langchain.py: a query + # engine and a bare llm.complete() on another thread share this singleton handler, so + # the token += and step appends below race (and the "LLM Call N" name computation can + # duplicate) without it. Only _send_trace happens outside the lock. + with self._state_lock: + start_info = self._starts.pop(event_id, None) + parent_id = self._parents.pop(event_id, None) + is_root = self._roots.pop(event_id, False) + root_id = event_id if is_root else self._find_root(parent_id) + state = self._runs.get(root_id) if root_id else None + + if state is None: + return + + exception = payload.get(EventPayload.EXCEPTION) + if exception is not None: + state["error"] = str(exception) - if state is None: - return - - exception = payload.get(EventPayload.EXCEPTION) - if exception is not None: - state["error"] = str(exception) - - start_t = start_info["start"] if start_info else time.time() - end_t = time.time() - - if event_type == CBEventType.QUERY: - response = payload.get(EventPayload.RESPONSE) - if response is not None: - state["output"] = str(response) - elif event_type == CBEventType.LLM: - completion = payload.get(EventPayload.COMPLETION) or payload.get(EventPayload.RESPONSE) - output_text = _extract_llm_text(completion) - # LLM events don't actually set MODEL_NAME (it only appears on embedding events), - # so also read it off the start payload's SERIALIZED dict - the LLM's serialized - # config, same "model"/"model_name" ladder langchain.py's _record_llm_start walks. - model = payload.get(EventPayload.MODEL_NAME) - if not model and start_info: - serialized = start_info["payload"].get(EventPayload.SERIALIZED) - if isinstance(serialized, dict): - model = serialized.get("model") or serialized.get("model_name") - model = str(model) if model else None - if model and not state["model"]: - state["model"] = model - input_tokens, output_tokens = _extract_usage_tokens(completion) - if input_tokens is not None: - state["input_tokens"] += int(input_tokens) - if output_tokens is not None: - state["output_tokens"] += int(output_tokens) - if output_text: - state["output"] = output_text - input_text = start_info["payload"].get(EventPayload.PROMPT) if start_info else None - state["execution_steps"].append({ - "name": f"LLM Call {len(state['execution_steps']) + 1}", - "duration_ms": (end_t - start_t) * 1000, - "start_time": start_t, - "end_time": end_t, - "model": model, - "input": input_text, - "output": output_text or (f"ERROR: {exception}" if exception else None), - "inputTokenSize": input_tokens, - "outputTokenSize": output_tokens, - }) - elif event_type == CBEventType.RETRIEVE: - nodes = payload.get(EventPayload.NODES) - doc_count, retrieved_text = _extract_retrieval_output(nodes) - query = start_info["payload"].get(EventPayload.QUERY_STR) if start_info else None - step: Dict[str, Any] = { - "name": f"Retrieval {len(state['retrieval_steps']) + 1}", - "duration_ms": (end_t - start_t) * 1000, - "start_time": start_t, - "end_time": end_t, - } - if query: - step["query"] = query - if doc_count is not None: - step["doc_count"] = doc_count - step["output"] = f"ERROR: {exception}" if exception else retrieved_text - state["retrieval_steps"].append(step) - elif event_type == CBEventType.FUNCTION_CALL: - start_payload = start_info["payload"] if start_info else {} - tool = start_payload.get(EventPayload.TOOL) - tool_name = getattr(tool, "name", None) or str(tool) if tool is not None else "unknown" - tool_input = start_payload.get(EventPayload.FUNCTION_CALL) - tool_output = payload.get(EventPayload.FUNCTION_OUTPUT) - state["tool_call_steps"].append({ - "name": tool_name, - # tracer._merge_child_run's tool_calls loop reads "latency_ms" - # (not "duration_ms" like execution/retrieval steps). - "latency_ms": int((end_t - start_t) * 1000), - "start_time": start_t, - "end_time": end_t, - "input": _safe_serialize(tool_input) if tool_input is not None else None, - "output": f"ERROR: {exception}" if exception else (str(tool_output) if tool_output is not None else None), - # The engine's failure test is success === false; without this a - # failed tool call would read as passing. - "success": exception is None, - }) + start_t = start_info["start"] if start_info else time.time() + end_t = time.time() + if event_type == CBEventType.QUERY: + response = payload.get(EventPayload.RESPONSE) + if response is not None: + state["output"] = str(response) + elif event_type == CBEventType.LLM: + completion = payload.get(EventPayload.COMPLETION) or payload.get(EventPayload.RESPONSE) + output_text = _extract_llm_text(completion) + # LLM events don't actually set MODEL_NAME (it only appears on embedding events), + # so also read it off the start payload's SERIALIZED dict - the LLM's serialized + # config, same "model"/"model_name" ladder langchain.py's _record_llm_start walks. + model = payload.get(EventPayload.MODEL_NAME) + if not model and start_info: + serialized = start_info["payload"].get(EventPayload.SERIALIZED) + if isinstance(serialized, dict): + model = serialized.get("model") or serialized.get("model_name") + model = str(model) if model else None + if model and not state["model"]: + state["model"] = model + input_tokens, output_tokens = _extract_usage_tokens(completion) + if input_tokens is not None: + state["input_tokens"] += int(input_tokens) + if output_tokens is not None: + state["output_tokens"] += int(output_tokens) + if output_text: + state["output"] = output_text + input_text = start_info["payload"].get(EventPayload.PROMPT) if start_info else None + state["execution_steps"].append({ + "name": f"LLM Call {len(state['execution_steps']) + 1}", + "duration_ms": (end_t - start_t) * 1000, + "start_time": start_t, + "end_time": end_t, + "model": model, + "input": input_text, + "output": output_text or (f"ERROR: {exception}" if exception else None), + "inputTokenSize": input_tokens, + "outputTokenSize": output_tokens, + }) + elif event_type == CBEventType.RETRIEVE: + nodes = payload.get(EventPayload.NODES) + doc_count, retrieved_text = _extract_retrieval_output(nodes) + query = start_info["payload"].get(EventPayload.QUERY_STR) if start_info else None + step: Dict[str, Any] = { + "name": f"Retrieval {len(state['retrieval_steps']) + 1}", + "duration_ms": (end_t - start_t) * 1000, + "start_time": start_t, + "end_time": end_t, + } + if query: + step["query"] = query + if doc_count is not None: + step["doc_count"] = doc_count + step["output"] = f"ERROR: {exception}" if exception else retrieved_text + state["retrieval_steps"].append(step) + elif event_type == CBEventType.FUNCTION_CALL: + start_payload = start_info["payload"] if start_info else {} + tool = start_payload.get(EventPayload.TOOL) + tool_name = getattr(tool, "name", None) or str(tool) if tool is not None else "unknown" + tool_input = start_payload.get(EventPayload.FUNCTION_CALL) + tool_output = payload.get(EventPayload.FUNCTION_OUTPUT) + state["tool_call_steps"].append({ + "name": tool_name, + # tracer._merge_child_run's tool_calls loop reads "latency_ms" + # (not "duration_ms" like execution/retrieval steps). + "latency_ms": int((end_t - start_t) * 1000), + "start_time": start_t, + "end_time": end_t, + "input": _safe_serialize(tool_input) if tool_input is not None else None, + "output": f"ERROR: {exception}" if exception else (str(tool_output) if tool_output is not None else None), + # The engine's failure test is success === false; without this a + # failed tool call would read as passing. + "success": exception is None, + }) + + if is_root: + self._runs.pop(root_id, None) + + # Network send happens outside the lock - it must not serialize other threads' + # event handling behind an HTTP enqueue. if is_root: - self._runs.pop(root_id, None) self._send_trace(state) # ------------------------------------------------------------------ diff --git a/agentx/integrations/openai_agents.py b/agentx/integrations/openai_agents.py index 92398e2..74f8024 100644 --- a/agentx/integrations/openai_agents.py +++ b/agentx/integrations/openai_agents.py @@ -146,6 +146,9 @@ def on_trace_start(self, trace: Any) -> None: framework="openai-agents", metadata=self._metadata, session_id=self._session_id, + # The root of a standalone Agents-SDK trace is the agent run itself + # (langchain/llamaindex/crewai/autogen parity). + span_kind="agent", ) # Deliberately NOT root_span.__enter__(): enter pushes onto the CALLING thread's # active-span stack, but the Agents SDK fires on_trace_end on whatever thread it diff --git a/agentx/monitor/__init__.py b/agentx/monitor/__init__.py index c81e0bc..ccc272b 100644 --- a/agentx/monitor/__init__.py +++ b/agentx/monitor/__init__.py @@ -1,17 +1,39 @@ -from agentx.monitor.client import MonitorClient +from agentx.monitor.agents import MonitorAgentClient +from agentx.monitor.client import AgentXMonitorError, MonitorClient +from agentx.monitor.improvement_groups import AgentXImprovementGroupsError, ImprovementGroupsClient +from agentx.monitor.judge_scorers import ( + AgentXJudgeScorersError, + JudgeScorer, + JudgeScorerBuilder, + JudgeScorersClient, +) from agentx.monitor.models import MonitorPattern, MonitorProfile, MonitorSignal, SignalOccurrence from agentx.monitor.patterns import MonitorPatternBuilder, MonitorPatternClient from agentx.monitor.profile import MonitorProfileClient +from agentx.monitor.scorer_groups import AgentXScorerGroupsError, ScorerGroup, ScorerGroupsClient +from agentx.monitor.sessions import MonitorSessionClient from agentx.monitor.signals import MonitorSignalClient __all__ = [ + "AgentXImprovementGroupsError", + "AgentXJudgeScorersError", + "AgentXMonitorError", + "AgentXScorerGroupsError", + "ImprovementGroupsClient", + "JudgeScorer", + "JudgeScorerBuilder", + "JudgeScorersClient", + "MonitorAgentClient", "MonitorClient", "MonitorPattern", "MonitorPatternBuilder", "MonitorPatternClient", "MonitorProfile", "MonitorProfileClient", + "MonitorSessionClient", "MonitorSignal", - "SignalOccurrence", "MonitorSignalClient", + "ScorerGroup", + "ScorerGroupsClient", + "SignalOccurrence", ] diff --git a/agentx/monitor/agents.py b/agentx/monitor/agents.py index ae94330..2941dfe 100644 --- a/agentx/monitor/agents.py +++ b/agentx/monitor/agents.py @@ -23,6 +23,17 @@ def create(self, name: str) -> dict: return self._client.create_agent(name) def ensure(self, name: str) -> dict: - """Get-or-create by name - idempotent, safe to re-run.""" + """Best-effort get-or-create - concurrent callers converge on the oldest row.""" existing = next((a for a in self.list() if a.get("name") == name), None) - return existing if existing is not None else self.create(name) + if existing is not None: + return existing + created = self.create(name) + # The engine's POST /agents always creates a new row, so two concurrent ensure() + # calls can both create. Re-list and return the oldest row among same-name rows + # (createdAt is ISO-8601, so lexicographic min is chronological min) - the same + # row the engine's own name resolution picks - so every caller converges on the + # same agent. + matches = [a for a in self.list() if a.get("name") == name] + if not matches: + return created + return min(matches, key=lambda a: str(a.get("createdAt") or "")) diff --git a/agentx/monitor/client.py b/agentx/monitor/client.py index b0b0ae1..0850a37 100644 --- a/agentx/monitor/client.py +++ b/agentx/monitor/client.py @@ -28,7 +28,6 @@ SDK_NAME = "agentx-python" _RETRYABLE_STATUS = {429, 500, 502, 503, 504} -_MAX_RETRIES = 3 _RETRY_BACKOFF = [1.0, 2.0, 4.0] @@ -145,22 +144,34 @@ def __init__( # CRUD and dry runs - full parity with the dashboard's Scorers page (P1.3). # Handed this client's own resolved API root, never the process-global default, so a # second AgentX() with a different base_url can't re-point it (deep-dive bug #1). - self.scorers = ScorersClient(api_key=api_key, base_url=self._api_root()) + # Workspace pinning scope (the real rule): sub-clients constructed below own their own + # _request and carry workspaceId in every body for hosted parity - the self-host + # engine's .strip() schemas ignore it. Helpers that reuse MonitorClient._request on + # _api_root() (patterns update/delete, rules, review_queue, sessions) do not send it. + self.scorers = ScorersClient( + api_key=api_key, base_url=self._api_root(), workspace_id=self._workspace_id + ) from agentx.monitor.judge_scorers import JudgeScorersClient # The unified LLM Judge Scorer (rubric + offline/online profiles in one entity) - the # surface that matches the product; evaluations.settings and online_evaluators below # remain as its profile-level views. - self.judge_scorers = JudgeScorersClient(api_key=api_key, base_url=self._api_root()) + self.judge_scorers = JudgeScorersClient( + api_key=api_key, base_url=self._api_root(), workspace_id=self._workspace_id + ) from agentx.monitor.scorer_groups import ScorerGroupsClient # Scorer groups: mixed-kind scorers composed into one 0-10 score (weights + must-pass # gates) - a group grades dataset runs (scorer_group_id) and, when online, live traffic. - self.scorer_groups = ScorerGroupsClient(api_key=api_key, base_url=self._api_root()) + self.scorer_groups = ScorerGroupsClient( + api_key=api_key, base_url=self._api_root(), workspace_id=self._workspace_id + ) from agentx.monitor.improvement_groups import ImprovementGroupsClient # Auto-improve: confirmed production failures -> improvement report -> code fix (via # the AgentX-Eval-Skill auto-improve skill). Self-host only. - self.improvement_groups = ImprovementGroupsClient(api_key=api_key, base_url=self._api_root()) + self.improvement_groups = ImprovementGroupsClient( + api_key=api_key, base_url=self._api_root(), workspace_id=self._workspace_id + ) self.profile = MonitorProfileClient(self) # Legacy view of an LLM Judge Scorer's online profile - constructed lazily so its # DeprecationWarning fires on first USE, not for every client that never touches it. @@ -216,8 +227,8 @@ def _request( if resp.status_code == 422: raise AgentXValidationError(resp.text, status_code=422) # Gate on the schedule itself so HTTP-status retries walk the SAME full backoff - # schedule connection errors do - the old `attempt < _MAX_RETRIES - 1` gate left - # the schedule's last entry unreachable for HTTP retries (ingest_client precedent). + # schedule connection errors do - an earlier fixed retry-count gate left the + # schedule's last entry unreachable for HTTP retries (ingest_client precedent). if retry and resp.status_code in _RETRYABLE_STATUS and attempt < len(schedule) - 1: logger.debug( "Retryable status %d (attempt %d)", resp.status_code, attempt + 1 @@ -293,8 +304,10 @@ def update_online_evaluator(self, evaluator_id: str, payload: dict) -> MonitorOn return MonitorOnlineEvaluator(**data["evaluator"]) def delete_online_evaluator(self, evaluator_id: str) -> None: + # retry=False: a lost response + transport retry would turn a successful + # delete into a spurious 404. self._request( - "DELETE", f"/online-evaluators/{evaluator_id}", params=self._workspace_params() + "DELETE", f"/online-evaluators/{evaluator_id}", params=self._workspace_params(), retry=False ) def get_online_evaluator_ratings(self, evaluator_id: str, window: str) -> List[OnlineEvaluatorRatingPoint]: @@ -433,7 +446,10 @@ def list_agents(self) -> List[dict]: return data.get("agents", []) if isinstance(data, dict) else data def create_agent(self, name: str) -> dict: - data = self._request("POST", "/agents", base=self._api_root(), json={"name": name}) + # The engine's POST /agents ALWAYS creates a new row (never get-or-create), so a + # transport retry after a timeout multiplies agents - no retry, same posture as + # create_pattern. + data = self._request("POST", "/agents", base=self._api_root(), json={"name": name}, retry=False) return data.get("agent", data) if isinstance(data, dict) else data # ------------------------------------------------------------------ diff --git a/agentx/monitor/improvement_groups.py b/agentx/monitor/improvement_groups.py index 3e59469..2dabb2d 100644 --- a/agentx/monitor/improvement_groups.py +++ b/agentx/monitor/improvement_groups.py @@ -5,11 +5,16 @@ import requests from agentx.util import api_base, get_headers -from agentx.exceptions import AgentXError +from agentx.exceptions import AgentXError, AgentXAuthError, AgentXValidationError class AgentXImprovementGroupsError(AgentXError): - pass + """``status_code`` carries the HTTP status when the error came from a server + response; ``None`` for transport-level failures.""" + + def __init__(self, message: str, status_code: Optional[int] = None) -> None: + super().__init__(message) + self.status_code = status_code class ImprovementGroupsClient: @@ -28,24 +33,54 @@ class ImprovementGroupsClient: offline dataset runs. Self-host only. """ - def __init__(self, api_key: Optional[str] = None, base_url: Optional[str] = None): + def __init__( + self, + api_key: Optional[str] = None, + base_url: Optional[str] = None, + workspace_id: Optional[str] = None, + ): self._api_key = api_key + # Same workspace pinning MonitorClient does - without it, requests silently land + # in whatever workspace the API key's user defaults to. + self._workspace_id = workspace_id self._base_url = (base_url or api_base()).rstrip("/") def _request(self, method: str, path: str, json: Any = None, timeout: int = 120) -> Any: + params = None + if self._workspace_id: + # Mirrors MonitorClient._workspace_params/_with_workspace: GETs (and DELETEs) + # carry workspaceId as a query param, write bodies carry it as a field. + if method.upper() in ("POST", "PUT", "PATCH"): + if json is None: + json = {"workspaceId": self._workspace_id} + elif isinstance(json, dict) and not json.get("workspaceId"): + json = {**json, "workspaceId": self._workspace_id} + else: + params = {"workspaceId": self._workspace_id} resp = requests.request( method, f"{self._base_url}/agent-monitoring{path}", headers={**get_headers(self._api_key), "Content-Type": "application/json"}, json=json, + params=params, timeout=timeout, ) + # Canonical taxonomy (evaluations/monitor client precedent): auth and validation + # failures raise the top-level typed errors, so `except agentx.AgentXAuthError` + # works whichever sub-client raised. + if resp.status_code == 401: + raise AgentXAuthError("Invalid or missing API key", status_code=401) + if resp.status_code == 422: + raise AgentXValidationError(resp.text, status_code=422) if resp.status_code >= 400: try: detail = resp.json().get("error", resp.reason) except ValueError: detail = resp.reason - raise AgentXImprovementGroupsError(f"Improvement group request failed ({resp.status_code}): {detail}") + raise AgentXImprovementGroupsError( + f"Improvement group request failed ({resp.status_code}): {detail}", + status_code=resp.status_code, + ) return resp.json() if resp.text else {} def list(self) -> List[Dict[str, Any]]: diff --git a/agentx/monitor/judge_scorers.py b/agentx/monitor/judge_scorers.py index 88ba22f..33bedcc 100644 --- a/agentx/monitor/judge_scorers.py +++ b/agentx/monitor/judge_scorers.py @@ -6,7 +6,7 @@ import requests from agentx.util import api_base, get_headers -from agentx.exceptions import AgentXError +from agentx.exceptions import AgentXError, AgentXAuthError, AgentXValidationError logger = logging.getLogger(__name__) @@ -14,7 +14,12 @@ class AgentXJudgeScorersError(AgentXError): - pass + """``status_code`` carries the HTTP status when the error came from a server + response; ``None`` for transport-level failures.""" + + def __init__(self, message: str, status_code: Optional[int] = None) -> None: + super().__init__(message) + self.status_code = status_code class JudgeScorer(dict): @@ -79,25 +84,55 @@ class JudgeScorersClient: cal = client.monitor.judge_scorers.calibration(scorer.id) """ - def __init__(self, api_key: Optional[str] = None, base_url: Optional[str] = None): + def __init__( + self, + api_key: Optional[str] = None, + base_url: Optional[str] = None, + workspace_id: Optional[str] = None, + ): self._api_key = api_key + # Same workspace pinning MonitorClient does - without it, scorer CRUD silently lands + # in whatever workspace the API key's user defaults to. + self._workspace_id = workspace_id # Captured once at construction so two clients with different bases can coexist. self._base_url = (base_url or api_base()).rstrip("/") def _request(self, method: str, path: str, json: Any = None, timeout: int = 60) -> Any: + params = None + if self._workspace_id: + # Mirrors MonitorClient._workspace_params/_with_workspace: GETs (and DELETEs) + # carry workspaceId as a query param, write bodies carry it as a field. + if method.upper() in ("POST", "PUT", "PATCH"): + if json is None: + json = {"workspaceId": self._workspace_id} + elif isinstance(json, dict) and not json.get("workspaceId"): + json = {**json, "workspaceId": self._workspace_id} + else: + params = {"workspaceId": self._workspace_id} resp = requests.request( method, f"{self._base_url}/agent-monitoring{path}", headers={**get_headers(self._api_key), "Content-Type": "application/json"}, json=json, + params=params, timeout=timeout, ) + # Canonical taxonomy (evaluations/monitor client precedent): auth and validation + # failures raise the top-level typed errors, so `except agentx.AgentXAuthError` + # works whichever sub-client raised. + if resp.status_code == 401: + raise AgentXAuthError("Invalid or missing API key", status_code=401) + if resp.status_code == 422: + raise AgentXValidationError(resp.text, status_code=422) if resp.status_code >= 400: try: detail = resp.json().get("error", resp.reason) except ValueError: detail = resp.reason - raise AgentXJudgeScorersError(f"Judge scorer request failed ({resp.status_code}): {detail}") + raise AgentXJudgeScorersError( + f"Judge scorer request failed ({resp.status_code}): {detail}", + status_code=resp.status_code, + ) return resp.json() if resp.text else {} # ------------------------------------------------------------------ @@ -147,6 +182,22 @@ def builder( # idleSeconds only applies to session scope - with trace scope the engine ignores # it, so an explicit value here would be silently inert. raise ValueError("idle_seconds requires scope='session'") + if idle_seconds is not None and not live: + # The online profile is only built when live=True - an explicit idle_seconds + # would otherwise be silently dropped. + raise ValueError("idle_seconds requires live=True") + if not live: + # Same guard for the rest of the online profile: agent_ids and a non-default + # scope only reach the wire when live=True - explicit values would otherwise + # be silently discarded. + offending = [ + kwarg + for kwarg, given in (("agent_ids", bool(agent_ids)), ("scope", scope != "trace")) + if given + ] + if offending: + verb = "require" if len(offending) > 1 else "requires" + raise ValueError(f"{' and '.join(offending)} {verb} live=True") judge: Dict[str, Any] = {} for key, value in ( ("acceptanceCriteria", acceptance_criteria), diff --git a/agentx/monitor/models.py b/agentx/monitor/models.py index 1b4996b..92e20f5 100644 --- a/agentx/monitor/models.py +++ b/agentx/monitor/models.py @@ -18,6 +18,9 @@ class MonitorPattern(BaseModel): entirely (they are legacy display fields kept for wire compatibility), so the flat ``include_terms``/``exclude_terms``/``regex``/``semantic_prompt`` attributes here stay empty/None - read the match settings from ``conditions``. + + ``match_mode`` always reads back ``"any"`` (the engine's toWire hardcodes it) even + though ``"all"`` is honored on create - read the effective mode from ``conditions``. """ id: str = Field(alias="_id") @@ -145,6 +148,7 @@ class MonitorProfile(BaseModel): enabled: bool = True failure_detection_enabled: bool = Field(default=True, alias="failureDetectionEnabled") info_detection_enabled: bool = Field(default=True, alias="infoDetectionEnabled") + topics_enabled: bool = Field(default=False, alias="topicsEnabled") coverage_mode: str = Field(default="all", alias="coverageMode") sample_rate: float = Field(default=0.1, alias="sampleRate") channels: List[str] = Field(default_factory=list) diff --git a/agentx/monitor/patterns.py b/agentx/monitor/patterns.py index ee292ac..8e7a2b9 100644 --- a/agentx/monitor/patterns.py +++ b/agentx/monitor/patterns.py @@ -20,6 +20,11 @@ class MonitorPatternBuilder: - ``"regex"``: ``regex`` - a single regular expression. - ``"semantic"``: ``semantic_prompt`` - an LLM judges whether the response violates the described rubric. + + ``conditions`` (self-host) is the engine's full N-condition model - a list of condition + dicts, each with its own detector kind and match settings - passed through verbatim to + the create payload; when set, the engine honors it as the pattern's whole rule set and + the flat fields above are only legacy display metadata. """ def __init__( @@ -41,6 +46,7 @@ def __init__( sample_rate: float = 1.0, scope_mode: str = "all", agent_ids: Optional[List[str]] = None, + conditions: Optional[List[dict]] = None, ): self._client = client self._payload: Dict[str, Any] = { @@ -63,6 +69,10 @@ def __init__( "scopeMode": scope_mode, "agentIds": agent_ids or [], } + # Passed through verbatim - the engine honors body.conditions as the full + # N-condition model (see the class docstring). + if conditions is not None: + self._payload["conditions"] = conditions def publish(self) -> MonitorPattern: logger.info("Publishing monitor pattern '%s'", self._payload["name"]) @@ -93,6 +103,7 @@ def builder( sample_rate: float = 1.0, scope_mode: str = "all", agent_ids: Optional[List[str]] = None, + conditions: Optional[List[dict]] = None, ) -> MonitorPatternBuilder: return MonitorPatternBuilder( self._client, @@ -112,11 +123,33 @@ def builder( sample_rate=sample_rate, scope_mode=scope_mode, agent_ids=agent_ids, + conditions=conditions, ) def delete(self, pattern_id: str) -> None: """Delete a pattern. Its historical signals remain as history.""" - self._client._request("DELETE", f"/agent-monitoring/patterns/{pattern_id}", base=self._client._api_root()) + # retry=False: a lost response + transport retry would turn a successful + # delete into a spurious 404. + self._client._request( + "DELETE", + f"/agent-monitoring/patterns/{pattern_id}", + base=self._client._api_root(), + retry=False, + ) + + def update(self, pattern_id: str, **fields: Any) -> MonitorPattern: + """Update a pattern's fields in place (wire camelCase keys, passed through + verbatim - e.g. ``enabled=False``, ``conditions=[...]``) and return the + updated :class:`MonitorPattern`. retry=False: a non-idempotent server-side + write must not be re-fired on a lost response.""" + data = self._client._request( + "PUT", + f"/agent-monitoring/patterns/{pattern_id}", + base=self._client._api_root(), + json=fields, + retry=False, + ) + return MonitorPattern(**data["pattern"]) def get(self, pattern_id: str) -> MonitorPattern: return self._client.get_pattern(pattern_id) diff --git a/agentx/monitor/profile.py b/agentx/monitor/profile.py index fdc25e3..b03e9de 100644 --- a/agentx/monitor/profile.py +++ b/agentx/monitor/profile.py @@ -35,6 +35,7 @@ def update( enabled: Optional[bool] = None, failure_detection_enabled: Optional[bool] = None, info_detection_enabled: Optional[bool] = None, + topics_enabled: Optional[bool] = None, coverage_mode: Optional[str] = None, sample_rate: Optional[float] = None, channels: Optional[List[str]] = None, @@ -60,6 +61,7 @@ def update( "enabled": enabled, "failureDetectionEnabled": failure_detection_enabled, "infoDetectionEnabled": info_detection_enabled, + "topicsEnabled": topics_enabled, "coverageMode": coverage_mode, "sampleRate": sample_rate, "channels": channels, diff --git a/agentx/monitor/review_queue.py b/agentx/monitor/review_queue.py index dbfea00..e390d81 100644 --- a/agentx/monitor/review_queue.py +++ b/agentx/monitor/review_queue.py @@ -62,7 +62,11 @@ def queue(self, trace_id: str, note: Optional[str] = None) -> ReviewQueueItem: payload: Dict[str, Any] = {"traceId": trace_id, "source": "manual"} if note: payload["note"] = note - data = self._client._request("POST", "/agent-monitoring/review-queue", base=self._client._api_root(), json=payload) + # retry=False: a lost response + transport retry would turn a successful queue + # into a spurious 409 (the trace is already pending). + data = self._client._request( + "POST", "/agent-monitoring/review-queue", base=self._client._api_root(), json=payload, retry=False + ) return ReviewQueueItem(data.get("item", data)) def label( @@ -90,4 +94,8 @@ def label( def dismiss(self, item_id: str) -> None: """Remove an item from the queue without a verdict (does not feed calibration).""" - self._client._request("DELETE", f"/agent-monitoring/review-queue/{item_id}", base=self._client._api_root()) + # retry=False: a lost response + transport retry would turn a successful + # delete into a spurious 404. + self._client._request( + "DELETE", f"/agent-monitoring/review-queue/{item_id}", base=self._client._api_root(), retry=False + ) diff --git a/agentx/monitor/rules.py b/agentx/monitor/rules.py index e3676bf..ebe5d84 100644 --- a/agentx/monitor/rules.py +++ b/agentx/monitor/rules.py @@ -63,7 +63,9 @@ def create( payload["sampleRate"] = sample_rate if action_config is not None: payload["actionConfig"] = action_config - data = self._request("POST", "/agent-monitoring/rules", json=payload) + # Server-side write: a timeout retry would create a duplicate rule that fans out + # webhooks / dataset appends forever - no transport retry (create_pattern posture). + data = self._request("POST", "/agent-monitoring/rules", json=payload, retry=False) return MonitorRule(data.get("rule", data)) def update(self, rule_id: str, **fields: Any) -> MonitorRule: @@ -75,4 +77,6 @@ def update(self, rule_id: str, **fields: Any) -> MonitorRule: return MonitorRule(data.get("rule", data)) def delete(self, rule_id: str) -> None: - self._request("DELETE", f"/agent-monitoring/rules/{rule_id}") + # retry=False: a lost response + transport retry would turn a successful + # delete into a spurious 404. + self._request("DELETE", f"/agent-monitoring/rules/{rule_id}", retry=False) diff --git a/agentx/monitor/scorer_groups.py b/agentx/monitor/scorer_groups.py index 2ab2615..d1b3983 100644 --- a/agentx/monitor/scorer_groups.py +++ b/agentx/monitor/scorer_groups.py @@ -9,11 +9,16 @@ import requests -from agentx.exceptions import AgentXError +from agentx.exceptions import AgentXError, AgentXAuthError, AgentXValidationError class AgentXScorerGroupsError(AgentXError): - pass + """``status_code`` carries the HTTP status when the error came from a server + response; ``None`` for transport-level failures.""" + + def __init__(self, message: str, status_code: Optional[int] = None) -> None: + super().__init__(message) + self.status_code = status_code class ScorerGroup(dict): @@ -37,20 +42,44 @@ def online(self) -> Optional[Dict[str, Any]]: class ScorerGroupsClient: - def __init__(self, api_key: str, base_url: str): + def __init__(self, api_key: str, base_url: str, workspace_id: Optional[str] = None): self._api_key = api_key + # Same workspace pinning MonitorClient does - without it, group CRUD silently lands + # in whatever workspace the API key's user defaults to. + self._workspace_id = workspace_id self._base = base_url.rstrip("/") + "/agent-monitoring/scorer-groups" def _request(self, method: str, url: str, json: Optional[Dict[str, Any]] = None) -> Any: + params = None + if self._workspace_id: + # Mirrors MonitorClient._workspace_params/_with_workspace: GETs (and DELETEs) + # carry workspaceId as a query param, write bodies carry it as a field. + if method.upper() in ("POST", "PUT", "PATCH"): + if json is None: + json = {"workspaceId": self._workspace_id} + elif not json.get("workspaceId"): + json = {**json, "workspaceId": self._workspace_id} + else: + params = {"workspaceId": self._workspace_id} response = requests.request( method, url, headers={"x-api-key": self._api_key, "content-type": "application/json"}, json=json, + params=params, timeout=30, ) + # Canonical taxonomy (evaluations/monitor client precedent): auth and validation + # failures raise the top-level typed errors, so `except agentx.AgentXAuthError` + # works whichever sub-client raised. + if response.status_code == 401: + raise AgentXAuthError("Invalid or missing API key", status_code=401) + if response.status_code == 422: + raise AgentXValidationError(response.text, status_code=422) if response.status_code >= 400: - raise AgentXScorerGroupsError(f"HTTP {response.status_code}: {response.text}") + raise AgentXScorerGroupsError( + f"HTTP {response.status_code}: {response.text}", status_code=response.status_code + ) # DELETE (and any other empty 2xx) has no body - .json() on it raises. return response.json() if response.text else {} diff --git a/agentx/monitor/scorers.py b/agentx/monitor/scorers.py index bf5a80c..92bd649 100644 --- a/agentx/monitor/scorers.py +++ b/agentx/monitor/scorers.py @@ -6,13 +6,18 @@ import requests from agentx.util import api_base, get_headers -from agentx.exceptions import AgentXError +from agentx.exceptions import AgentXError, AgentXAuthError, AgentXValidationError logger = logging.getLogger(__name__) class AgentXScorersError(AgentXError): - pass + """``status_code`` carries the HTTP status when the error came from a server + response; ``None`` for transport-level failures.""" + + def __init__(self, message: str, status_code: Optional[int] = None) -> None: + super().__init__(message) + self.status_code = status_code class ScorersClient: @@ -34,12 +39,30 @@ class ScorersClient: wire. """ - def __init__(self, api_key: Optional[str] = None, base_url: Optional[str] = None): + def __init__( + self, + api_key: Optional[str] = None, + base_url: Optional[str] = None, + workspace_id: Optional[str] = None, + ): self._api_key = api_key + # Same workspace pinning MonitorClient does - without it, catalog edits silently land + # in whatever workspace the API key's user defaults to. + self._workspace_id = workspace_id # Captured once at construction (deep-dive round 3, bug #1). self._base_url = (base_url or api_base()).rstrip("/") def _request(self, method: str, path: str, json: Any = None, params: Any = None) -> Any: + if self._workspace_id: + # Mirrors MonitorClient._workspace_params/_with_workspace: GETs (and DELETEs) + # carry workspaceId as a query param, write bodies carry it as a field. + if method.upper() in ("POST", "PUT", "PATCH"): + if json is None: + json = {"workspaceId": self._workspace_id} + elif isinstance(json, dict) and not json.get("workspaceId"): + json = {**json, "workspaceId": self._workspace_id} + elif not (params or {}).get("workspaceId"): + params = {**(params or {}), "workspaceId": self._workspace_id} resp = requests.request( method, f"{self._base_url}/agent-monitoring{path}", @@ -48,12 +71,22 @@ def _request(self, method: str, path: str, json: Any = None, params: Any = None) params=params, timeout=20, ) + # Canonical taxonomy (evaluations/monitor client precedent): auth and validation + # failures raise the top-level typed errors, so `except agentx.AgentXAuthError` + # works whichever sub-client raised. + if resp.status_code == 401: + raise AgentXAuthError("Invalid or missing API key", status_code=401) + if resp.status_code == 422: + raise AgentXValidationError(resp.text, status_code=422) if resp.status_code >= 400: try: detail = resp.json().get("error", resp.reason) except ValueError: detail = resp.reason - raise AgentXScorersError(f"Scorer request failed ({resp.status_code}): {detail}") + raise AgentXScorersError( + f"Scorer request failed ({resp.status_code}): {detail}", + status_code=resp.status_code, + ) return resp.json() if resp.text else {} # ------------------------------------------------------------------ diff --git a/agentx/tracing/ingest_client.py b/agentx/tracing/ingest_client.py index ef78c8c..648a37b 100644 --- a/agentx/tracing/ingest_client.py +++ b/agentx/tracing/ingest_client.py @@ -94,7 +94,8 @@ def __init__( # ------------------------------------------------------------------ def enqueue(self, payload: Dict[str, Any]) -> None: - """Add a trace payload to the send queue. Never blocks; drops silently on overflow.""" + """Add a trace payload to the send queue. Never blocks; drops on overflow + (logged - the 1st and every 50th drop warn, see ``_record_drop``).""" if self._workspace_id: payload = {**payload, "workspaceId": self._workspace_id} try: @@ -276,7 +277,11 @@ def create_ci_run( git_context: Optional[Dict[str, Any]] = None, workspace_id: Optional[str] = None, ) -> CIRun: - """Create a CI run and return test cases from the dataset.""" + """Create a CI run and return test cases from the dataset. + + Hosted platform only - the self-host engine does not serve /ingest/ci-runs; + use ``client.evaluations.run(...).gate(...)`` instead. + """ payload: Dict[str, Any] = {"dataset_id": dataset_id} if agent_name: payload["agent_name"] = agent_name @@ -310,7 +315,11 @@ def submit_ci_result( input: Optional[Any] = None, latency_ms: Optional[int] = None, ) -> CIQuestionScore: - """Submit an agent result for one test case and receive the score.""" + """Submit an agent result for one test case and receive the score. + + Hosted platform only - the self-host engine does not serve /ingest/ci-runs; + use ``client.evaluations.run(...).gate(...)`` instead. + """ payload: Dict[str, Any] = { "question_index": question_index, "output": output, @@ -334,14 +343,22 @@ def submit_ci_result( ) def finalize_ci_run(self, run_id: str) -> CIRunResult: - """Finalize the run and return the gate result.""" + """Finalize the run and return the gate result. + + Hosted platform only - the self-host engine does not serve /ingest/ci-runs; + use ``client.evaluations.run(...).gate(...)`` instead. + """ url = f"{self._base_url}/ingest/ci-runs/{run_id}/finalize" resp = self._session.post(url, json={}, timeout=60) self._raise_for_ci_status(resp) return self._parse_ci_result(resp.json()) def get_ci_run(self, run_id: str) -> CIRunStatus: - """Poll the status of a CI run.""" + """Poll the status of a CI run. + + Hosted platform only - the self-host engine does not serve /ingest/ci-runs; + use ``client.evaluations.run(...).gate(...)`` instead. + """ url = f"{self._base_url}/ingest/ci-runs/{run_id}" resp = self._session.get(url, timeout=15) self._raise_for_ci_status(resp) diff --git a/agentx/tracing/tracer.py b/agentx/tracing/tracer.py index a488d8d..9156789 100644 --- a/agentx/tracing/tracer.py +++ b/agentx/tracing/tracer.py @@ -8,8 +8,9 @@ import logging import threading import time +import uuid from contextlib import contextmanager -from typing import Any, Callable, Dict, Iterator, List, Optional, TypeVar +from typing import Any, Callable, Dict, Iterator, List, Mapping, Optional, TypeVar from uuid import uuid4 from agentx.exceptions import CIGateFailure @@ -22,6 +23,41 @@ F = TypeVar("F", bound=Callable[..., Any]) +# Active-span stacks for every Tracer instance, keyed by id(tracer). ONE module-level +# ContextVar (CPython's contextvars docs: create ContextVars at the top module level, +# never per-instance - the context machinery keeps a reference per Context that a +# per-instance var can never reclaim, so churning Tracer instances leaked an entry each). +# The value is an immutable mapping of tracer id -> span-stack tuple, replaced +# copy-on-write on every push/pop, so the thread/async isolation semantics (each asyncio +# task sees a copy of its creator's context, bare threads start empty) are exactly what +# the old per-instance var gave - and two Tracer instances still share no span state, +# since each reads only its own id's slot. +_SPAN_STACKS: "contextvars.ContextVar[Mapping[str, tuple]]" = contextvars.ContextVar( + "agentx_span_stacks", default={} +) + + +def _stack_key(tracer: "Tracer") -> str: + # A stable per-instance token, NOT id(self): a span leaked mid-`with` (killed thread, a + # framework callback that never fires on_*_end) leaves a stale entry, and a new Tracer + # landing on the recycled id() would read the dead tracer's stack and mis-parent spans. + return tracer._stack_token + + +def _get_stack(tracer: "Tracer") -> tuple: + return _SPAN_STACKS.get().get(_stack_key(tracer), ()) + + +def _set_stack(tracer: "Tracer", stack: tuple) -> None: + stacks = dict(_SPAN_STACKS.get()) + if stack: + stacks[_stack_key(tracer)] = stack + else: + # An empty stack is dropped rather than stored, so a finished tracer leaves no + # entry behind. + stacks.pop(_stack_key(tracer), None) + _SPAN_STACKS.set(stacks) + def _safe_serialize(value: Any, depth: int = 0) -> Any: """Best-effort conversion to a JSON-safe structure, truncated to avoid huge payloads.""" @@ -600,6 +636,9 @@ def __init__(self) -> None: self.output: Any = None +_WARNED_MEMORY_NO_SPAN = False + + class _MemoryOpRecorder: """Handle yielded by ``Tracer.trace_memory()`` - set ``output`` (what was recalled or stored) inside the block.""" @@ -646,14 +685,10 @@ def __init__(self, ingest_client: IngestClient) -> None: self._client = ingest_client self._pending_tool_calls: List[Dict[str, Any]] = [] self._pending_retrievals: List[Dict[str, Any]] = [] - # Context-local, not thread-local: two coroutines interleaving on one event loop each - # get their own asyncio task Context, so concurrent `async def` agents no longer - # mis-parent each other's spans (a thread-local stack merged them into one fabricated - # tree). Bare threads keep the old behavior - each starts an empty Context. Stored - # immutably (tuple, copy-on-write) so a child task's pushes never leak into siblings. - self._span_stack_var: "contextvars.ContextVar[tuple]" = contextvars.ContextVar( - f"agentx_span_stack_{id(self)}", default=() - ) + # The active-span stack lives in the module-level _SPAN_STACKS ContextVar, keyed by + # this stable token - see _stack_key for why it must not be id(self), and the + # ContextVar's own comment for why it must not be a per-instance ContextVar. + self._stack_token = uuid.uuid4().hex # ------------------------------------------------------------------ # Active-span stack (per context) - lets auto-instrumented integrations @@ -663,22 +698,22 @@ def __init__(self, ingest_client: IngestClient) -> None: # ------------------------------------------------------------------ def _get_span_stack(self) -> tuple: - return self._span_stack_var.get() + return _get_stack(self) def _push_active_span(self, span: "_TraceSpan") -> None: - self._span_stack_var.set(self._span_stack_var.get() + (span,)) + _set_stack(self, _get_stack(self) + (span,)) def _pop_active_span(self, span: "_TraceSpan") -> None: - stack = self._span_stack_var.get() + stack = _get_stack(self) if stack and stack[-1] is span: - self._span_stack_var.set(stack[:-1]) + _set_stack(self, stack[:-1]) elif span in stack: - self._span_stack_var.set(tuple(item for item in stack if item is not span)) + _set_stack(self, tuple(item for item in stack if item is not span)) @property def current_span(self) -> Optional["_TraceSpan"]: """The innermost ``with tracer.trace(...)`` span active in this context, if any.""" - stack = self._span_stack_var.get() + stack = _get_stack(self) return stack[-1] if stack else None @contextmanager @@ -899,19 +934,28 @@ def record_memory( With no active span the record is DROPPED (with a debug log), not queued: the only pending queue rides the next trace's ``retrieval_steps``, and memory content must never reach the engine's retrieval-context extraction for RAG judges. Wrap the call - in ``tracer.trace()`` to keep it. (``record_tool_call``/``record_retrieval`` queue - instead - see their docstrings.) + in ``tracer.trace()`` to keep it - or, on a worker thread, wrap the worker body in + ``tracer.use_span(span)`` - a bare thread starts with an empty span stack. + (``record_tool_call``/``record_retrieval`` queue instead - see their docstrings.) """ active_span = self.current_span if active_span is None: # NOT the record_retrieval queue posture: _pending_retrievals rides the next # trace's performance_summary.retrieval_steps, which the engine's # retrieval-context extraction feeds to RAG judges - recalled memory must never - # classify as knowledge grounding. Drop, and say so. - logger.debug( - "record_memory(%r) called with no active span - wrap the call in tracer.trace(); dropped", - name, - ) + # classify as knowledge grounding. Drop - and WARN (once): this is a lost write, + # and at debug level the likely trigger (a worker thread without use_span) read + # as "memory spans don't work" with no log line anywhere. + global _WARNED_MEMORY_NO_SPAN + if not _WARNED_MEMORY_NO_SPAN: + _WARNED_MEMORY_NO_SPAN = True + logger.warning( + "record_memory(%r): no active span - the memory operation was dropped " + "(wrap the call in tracer.trace() or tracer.use_span()); further drops log at debug", + name, + ) + else: + logger.debug("record_memory(%r) called with no active span; dropped", name) return active_span.child_span( name, @@ -935,6 +979,11 @@ def trace_memory( with tracer.trace_memory("user prefs", operation="read", query=user_id) as m: m.output = memory.search(user_id, question) + + With no active span the record is DROPPED (with a debug log), not queued - see + :meth:`record_memory`. An exception escaping the block records the operation as + failed (error set, output ``ERROR: ...``) and then propagates unchanged - same + posture as :meth:`trace_tool_call`. """ start_t = time.time() recorder = _MemoryOpRecorder() @@ -1128,6 +1177,9 @@ def run_eval( """ Run the full CI/CD evaluation lifecycle in one call. + Hosted platform only - the self-host engine does not serve /ingest/ci-runs; + use ``client.evaluations.run(...).gate(...)`` instead. + Creates a CI run, calls ``agent_fn(query)`` for each test case, submits results to AgentX for scoring, finalizes the run, and returns the gate decision. @@ -1142,6 +1194,12 @@ def run_eval( concurrency: Max parallel question invocations (default 1). fail_on_gate: Raise CIGateFailure if gate is "fail". timeout_per_question: Seconds to wait for agent_fn per question. + A case that times out abandons the in-flight + agent call, but the non-daemon worker thread + keeps running until agent_fn returns - and + interpreter shutdown joins those workers, so + a permanently-hung agent_fn can block + process exit. Returns: CIRunResult with gate, pass_rate, scores, and violations. @@ -1160,9 +1218,15 @@ def _process_case(tc: Any) -> CIQuestionScore: output: Optional[str] = None try: if timeout_per_question: - with concurrent.futures.ThreadPoolExecutor(max_workers=1) as ex: + # No `with` block: the executor's __exit__ would join the worker + # thread, making a timed-out case block for the agent's full + # runtime. shutdown(wait=False) lets the timeout actually fire. + ex = concurrent.futures.ThreadPoolExecutor(max_workers=1) + try: future = ex.submit(agent_fn, query) output = future.result(timeout=timeout_per_question) + finally: + ex.shutdown(wait=False) else: output = agent_fn(query) except Exception as exc: @@ -1196,12 +1260,15 @@ def _process_case(tc: Any) -> CIQuestionScore: raise CIGateFailure(final) return final else: - with concurrent.futures.ThreadPoolExecutor(max_workers=concurrency) as ex: + # No `with` block: on the fail-fast path __exit__ would still join the + # in-flight workers after shutdown(wait=False, cancel_futures=True), + # defeating the fast exit. The finally guarantees the shutdown instead. + ex = concurrent.futures.ThreadPoolExecutor(max_workers=concurrency) + try: futures = {ex.submit(_process_case, tc): tc for tc in run.test_cases} for future in concurrent.futures.as_completed(futures): score = future.result() if score.gate_fired: - ex.shutdown(wait=False, cancel_futures=True) result = self.get_ci_run(run.run_id) final = CIRunResult( run_id=run.run_id, @@ -1214,6 +1281,8 @@ def _process_case(tc: Any) -> CIQuestionScore: if fail_on_gate: raise CIGateFailure(final) return final + finally: + ex.shutdown(wait=False, cancel_futures=True) result = self.finalize_ci_run(run.run_id) if fail_on_gate and result.gate == "fail": @@ -1229,7 +1298,11 @@ def create_ci_run( git_context: Optional[Dict[str, Any]] = None, workspace_id: Optional[str] = None, ) -> CIRun: - """Create a CI run and receive test cases from the dataset.""" + """Create a CI run and receive test cases from the dataset. + + Hosted platform only - the self-host engine does not serve /ingest/ci-runs; + use ``client.evaluations.run(...).gate(...)`` instead. + """ return self._client.create_ci_run( dataset_id, agent_name=agent_name, @@ -1247,7 +1320,11 @@ def submit_result( input: Optional[Any] = None, latency_ms: Optional[int] = None, ) -> CIQuestionScore: - """Submit an agent output for one CI run test case.""" + """Submit an agent output for one CI run test case. + + Hosted platform only - the self-host engine does not serve /ingest/ci-runs; + use ``client.evaluations.run(...).gate(...)`` instead. + """ return self._client.submit_ci_result( run_id, question_index, @@ -1257,11 +1334,19 @@ def submit_result( ) def finalize_ci_run(self, run_id: str) -> CIRunResult: - """Finalize a CI run and return the gate result.""" + """Finalize a CI run and return the gate result. + + Hosted platform only - the self-host engine does not serve /ingest/ci-runs; + use ``client.evaluations.run(...).gate(...)`` instead. + """ return self._client.finalize_ci_run(run_id) def get_ci_run(self, run_id: str) -> CIRunStatus: - """Poll the current status of a CI run.""" + """Poll the current status of a CI run. + + Hosted platform only - the self-host engine does not serve /ingest/ci-runs; + use ``client.evaluations.run(...).gate(...)`` instead. + """ return self._client.get_ci_run(run_id) def evaluate_trace( diff --git a/tests/test_judge_scorers.py b/tests/test_judge_scorers.py index aa4df8e..63dc64b 100644 --- a/tests/test_judge_scorers.py +++ b/tests/test_judge_scorers.py @@ -26,8 +26,8 @@ def recorded(monkeypatch): calls: List[Dict[str, Any]] = [] responses: List[FakeResponse] = [] - def fake_request(method, url, headers=None, json=None, timeout=None): - calls.append({"method": method, "url": url, "json": json}) + def fake_request(method, url, headers=None, json=None, params=None, timeout=None): + calls.append({"method": method, "url": url, "json": json, "params": params}) return responses.pop(0) if responses else FakeResponse({"judgeScorer": {"_id": "s1", "name": "n"}}) monkeypatch.setattr("agentx.monitor.judge_scorers.requests.request", fake_request) @@ -223,6 +223,19 @@ def test_builder_rejects_idle_seconds_without_session_scope(): client.builder("Support quality", live=True, idle_seconds=300) +def test_builder_rejects_online_kwargs_without_live(): + """agent_ids and a non-default scope only reach the wire when live=True - explicit + values with live=False were silently discarded, so they are now hard errors, same + posture as the idle_seconds guard.""" + client = JudgeScorersClient(api_key="agtx_local_test", base_url="http://localhost:1") + with pytest.raises(ValueError, match="agent_ids requires live=True"): + client.builder("Support quality", agent_ids=["support-agent"]) + with pytest.raises(ValueError, match="scope requires live=True"): + client.builder("Support quality", scope="session") + with pytest.raises(ValueError, match="agent_ids and scope require live=True"): + client.builder("Support quality", agent_ids=["support-agent"], scope="session") + + def test_builder_sends_idle_seconds_and_requires_expected(monkeypatch): """With scope="session", an explicit idle_seconds reaches the wire, and requires_expected lands in the judge section as requiresExpected.""" diff --git a/tests/test_span_tree.py b/tests/test_span_tree.py index f7c5b5e..f71f0f4 100644 --- a/tests/test_span_tree.py +++ b/tests/test_span_tree.py @@ -635,6 +635,79 @@ async def main(): assert len({w.get("session_id") for w in wires}) == 2 +def test_two_agentx_instances_share_no_span_state(): + """Regression for the per-instance ContextVar leak (#6): the active-span stack now lives + in ONE module-level ContextVar keyed by tracer id - two AgentX() instances in one process + must still each see only their own spans, and an exited tracer must leave no entry behind + in the shared mapping.""" + from agentx import AgentX + from agentx.tracing.tracer import _SPAN_STACKS + + a = AgentX(api_key="k", base_url="http://engine-a:1111/api/v1") + b = AgentX(api_key="k", base_url="http://engine-b:2222/api/v1") + a.tracer._client = MagicMock() + b.tracer._client = MagicMock() + + with a.tracer.trace("agent-a") as span_a: + # b's tracer must not adopt a's active span as its own... + assert b.tracer.current_span is None + with b.tracer.trace("agent-b") as span_b: + assert a.tracer.current_span is span_a + assert b.tracer.current_span is span_b + # ...and b's exit must not disturb a's stack. + assert a.tracer.current_span is span_a + assert a.tracer.current_span is None + assert b.tracer.current_span is None + + # b's root grew no parent edge (and no shared session) from a's span being active. + b_wire = b.tracer._client.enqueue.call_args_list[0].args[0] + assert "parent_span_id" not in b_wire + a_wire = a.tracer._client.enqueue.call_args_list[0].args[0] + assert a_wire["session_id"] != b_wire["session_id"] + + # Both stacks drained to empty, so neither tracer holds a slot in the module-level + # mapping any more - the exact leak the per-instance ContextVar had. + stacks = _SPAN_STACKS.get() + assert id(a.tracer) not in stacks + assert id(b.tracer) not in stacks + + +def test_with_trace_in_a_thread_still_parents_correctly(): + """The module-level span-stack ContextVar keeps the old per-thread semantics: a bare + thread starts with an EMPTY stack (no inherited active span), and nesting inside that + thread parents within the thread.""" + import threading + + tracer = make_tracer() + failures: list = [] + + def worker(): + try: + # A bare thread starts a fresh Context - the main thread's active span + # must not leak in as an implicit parent. + assert tracer.current_span is None + with tracer.trace("outer-t"): + with tracer.trace("inner-t"): + pass + except Exception as exc: # pragma: no cover - surfaced via `failures` below + failures.append(exc) + + with tracer.trace("main-root"): + t = threading.Thread(target=worker) + t.start() + t.join() + + assert failures == [] + wires = enqueued_wires(tracer) + outer = next(w for w in wires if w["name"] == "outer-t") + inner = next(w for w in wires if w["name"] == "inner-t") + main_root = next(w for w in wires if w["name"] == "main-root") + assert "parent_span_id" not in outer # not a child of the main thread's span + assert inner["parent_span_id"] == outer["span_id"] + assert inner["session_id"] == outer["session_id"] + assert outer["session_id"] != main_root["session_id"] + + def test_trace_memory_emits_a_memory_kind_child_span(): tracer = make_tracer() with tracer.trace("agent") as span: diff --git a/tests/test_wire_models.py b/tests/test_wire_models.py index 10e854f..47c2971 100644 --- a/tests/test_wire_models.py +++ b/tests/test_wire_models.py @@ -66,6 +66,49 @@ def create_dataset(self, payload): assert payload["codeScorers"] == DATASET_WIRE["codeScorers"] +def test_builders_warn_at_publish_when_sovereignty_models_requested(): + """Self-host drops sovereigntyIndex on both create routes, so the warning fires where + the request is known - builder.publish() - not in a run-time guard that can never see + the field.""" + import pytest + + from agentx.evaluations.datasets import DatasetBuilder + from agentx.evaluations.evaluation_settings import EvaluationSettingsBuilder + + class FakeEvalClient: + def create_dataset(self, payload): + return payload + + def create_evaluation_settings(self, payload): + return payload + + ds_builder = DatasetBuilder(FakeEvalClient(), name="ds", sovereignty_models=["m1", "m2"]) + ds_builder.add_case("q0") + with pytest.warns(UserWarning, match="Self-host ignores sovereigntyIndex"): + ds_builder.publish() + + with pytest.warns(UserWarning, match="Self-host ignores sovereigntyIndex"): + EvaluationSettingsBuilder( + FakeEvalClient(), name="cfg", sovereignty_models=["m1"] + ).publish() + + +def test_builders_publish_quietly_without_sovereignty_models(): + import warnings as _warnings + + from agentx.evaluations.datasets import DatasetBuilder + + class FakeEvalClient: + def create_dataset(self, payload): + return payload + + builder = DatasetBuilder(FakeEvalClient(), name="ds") + builder.add_case("q0") + with _warnings.catch_warnings(): + _warnings.simplefilter("error") + builder.publish() + + def test_run_result_row_response_populates_from_output_object(): """P2 regression: the engine sends the agent's answer as an `output` object, so row.response was permanently None."""