Skip to content

Commit 1951eb9

Browse files
feat: add metrics CLI filter, skip field, retry improvements, and infer endpoint (#222)
* Add --metrics CLI flag to filter which metrics run Allows running a subset of configured metrics without editing YAML configs. Example: --metrics custom:answer_correctness to skip RAGAS metrics. * Add skip field to disable conversations with a reason Adds skip and skip_reason fields to EvaluationData. Conversations with skip: true are silently excluded during loading. skip_reason is documentation-only — it stays in the YAML for humans to read. * feat: add retry for all server errors and /infer endpoint support Broaden retry logic from HTTP 429 only to include 5xx server errors, enabling automatic retry with exponential backoff for transient server failures. Add RLSAPI /v1/infer endpoint support for tool call and RAG chunk metadata retrieval, used by RHEL Lightspeed backend testing. * config: increase default retry attempts from 3 to 5 Provides more resilience against transient server failures, especially during long evaluation runs. * style: apply black formatting to client and tests * fix: address PR review feedback (asamal4 + CodeRabbit) - Revert DEFAULT_LLM_RETRIES from 5 to 3 - Narrow retry codes to (429, 502, 503, 504), exclude 500 - Use RLSAPI native fields (name/args) in _rlsapi_infer_query - Fix RAG chunk accumulation across multiple mcp_call results - Redact prompt from debug log, log only metadata - Add comment about extra_request_params not forwarded to /infer - Fix tool result capture: use content with status fallback - Update endpoint_type description to include infer - Move skip tests from TestFilterByScope to TestDataValidator - Fix MockerFixture import in test_validator.py - Fix --metrics filter: handle turn_metrics=None by materializing system defaults before filtering; add conversation_metrics filter - Add metrics=None to runner test fixture for --metrics support - Add tests for metrics filter materialization Signed-off-by: Ellis Low <elow@redhat.com> * refactor: reduce complexity in client.py and validator.py to fix pylint * fix: validate tool_results content before splitting in infer response * fix: split test_client.py to satisfy pylint line limit * fix: address CodeRabbit review comments - Mock time.sleep in 5 retry tests to prevent real exponential backoff delays - Update test_is_retryable_server_error docstring to mention transient 5xx errors Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> --------- Signed-off-by: Ellis Low <elow@redhat.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
1 parent 811e1ff commit 1951eb9

11 files changed

Lines changed: 780 additions & 62 deletions

File tree

src/lightspeed_evaluation/core/api/client.py

Lines changed: 185 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,23 @@
2727
logger = logging.getLogger(__name__)
2828

2929

30-
def _is_too_many_requests_error(exception: BaseException) -> bool:
31-
"""Check if exception is a 429 error."""
32-
return (
33-
isinstance(exception, httpx.HTTPStatusError)
34-
and exception.response.status_code == 429
35-
)
30+
def _is_retryable_server_error(exception: BaseException) -> bool:
31+
"""Check if exception is a retryable HTTP error (429 or transient 5xx).
32+
33+
Only 502 Bad Gateway, 503 Service Unavailable, and 504 Gateway Timeout
34+
are retried. 500 Internal Server Error is excluded as it may indicate
35+
permanent server bugs.
36+
37+
Args:
38+
exception: The exception to check.
39+
40+
Returns:
41+
True if the exception is a retryable HTTP status error.
42+
"""
43+
if not isinstance(exception, httpx.HTTPStatusError):
44+
return False
45+
status = exception.response.status_code
46+
return status in (429, 502, 503, 504)
3647

3748

3849
class APIClient:
@@ -59,10 +70,11 @@ def __init__(
5970
retry_decorator = self._create_retry_decorator()
6071
self._standard_query_with_retry = retry_decorator(self._standard_query)
6172
self._streaming_query_with_retry = retry_decorator(self._streaming_query)
73+
self._rlsapi_infer_query_with_retry = retry_decorator(self._rlsapi_infer_query)
6274

6375
def _create_retry_decorator(self) -> Any:
6476
return retry(
65-
retry=retry_if_exception(_is_too_many_requests_error),
77+
retry=retry_if_exception(_is_retryable_server_error),
6678
stop=stop_after_attempt(
6779
self.config.num_retries + 1
6880
), # +1 to account for the initial attempt
@@ -186,6 +198,8 @@ def query(
186198

187199
if self.config.endpoint_type == "streaming":
188200
response = self._streaming_query_with_retry(api_request)
201+
elif self.config.endpoint_type == "infer":
202+
response = self._rlsapi_infer_query_with_retry(api_request)
189203
else:
190204
response = self._standard_query_with_retry(api_request)
191205

@@ -196,7 +210,7 @@ def query(
196210
except RetryError as e:
197211
raise APIError(
198212
f"Maximum retry attempts ({self.config.num_retries}) reached "
199-
"due to persistent rate limiting (HTTP 429)."
213+
"due to retryable server errors (HTTP 429/5xx)."
200214
) from e
201215

202216
def _prepare_request(
@@ -285,8 +299,7 @@ def _standard_query(self, api_request: APIRequest) -> APIResponse:
285299
except httpx.TimeoutException as e:
286300
raise self._handle_timeout_error("standard", self.config.timeout) from e
287301
except httpx.HTTPStatusError as e:
288-
# Re-raise 429 errors without conversion to allow retry decorator to handle them
289-
if e.response.status_code == 429:
302+
if _is_retryable_server_error(e):
290303
raise
291304
raise self._handle_http_error(e) from e
292305
except ValueError as e:
@@ -313,8 +326,7 @@ def _streaming_query(self, api_request: APIRequest) -> APIResponse:
313326
except httpx.TimeoutException as e:
314327
raise self._handle_timeout_error("streaming", self.config.timeout) from e
315328
except httpx.HTTPStatusError as e:
316-
# Re-raise 429 errors without conversion to allow retry decorator to handle them
317-
if e.response.status_code == 429:
329+
if _is_retryable_server_error(e):
318330
raise
319331
raise self._handle_http_error(e) from e
320332
except ValueError as e:
@@ -324,6 +336,167 @@ def _streaming_query(self, api_request: APIRequest) -> APIResponse:
324336
except Exception as e:
325337
raise self._handle_unexpected_error(e, "streaming query") from e
326338

339+
def _build_infer_request(self, api_request: APIRequest) -> dict[str, object]:
340+
"""Build request payload for the RLSAPI /infer endpoint.
341+
342+
Converts the standard APIRequest into the infer-specific format,
343+
renaming 'query' to 'question' and adding 'include_metadata'.
344+
345+
Args:
346+
api_request: The prepared API request.
347+
348+
Returns:
349+
Dictionary suitable for posting to the /infer endpoint.
350+
"""
351+
request_data = api_request.model_dump(exclude_none=True)
352+
# `extra_request_params` are not forwarded to `/infer` — the
353+
# endpoint only accepts `question` and `include_metadata`.
354+
infer_request: dict[str, object] = {
355+
"question": request_data.pop("query"),
356+
"include_metadata": True,
357+
}
358+
logger.debug(
359+
"RLSAPI infer request URL: /api/lightspeed/%s/infer",
360+
self.config.version,
361+
)
362+
logger.debug(
363+
"RLSAPI infer request: version=%s, include_metadata=%s, "
364+
"question_length=%d",
365+
self.config.version,
366+
True,
367+
len(str(infer_request.get("question", ""))),
368+
)
369+
return infer_request
370+
371+
def _extract_infer_data(self, response_data: dict[str, Any]) -> None:
372+
"""Extract and promote fields from the infer response 'data' envelope.
373+
374+
Mutates response_data in place, mapping nested data fields
375+
(text, request_id, tokens, tool_calls, tool_results) to the
376+
top-level keys expected by APIResponse.from_raw_response().
377+
378+
Args:
379+
response_data: The raw JSON response dict to update.
380+
"""
381+
if "data" not in response_data:
382+
return
383+
data = response_data["data"]
384+
if "text" in data:
385+
response_data["response"] = data["text"]
386+
if "request_id" in data:
387+
response_data["conversation_id"] = data["request_id"]
388+
if "input_tokens" in data:
389+
response_data["input_tokens"] = data["input_tokens"]
390+
if "output_tokens" in data:
391+
response_data["output_tokens"] = data["output_tokens"]
392+
if "tool_calls" in data:
393+
response_data["tool_calls"] = data["tool_calls"]
394+
if "tool_results" in data:
395+
tool_results = data["tool_results"]
396+
rag_chunks: list[dict[str, str]] = []
397+
for result in tool_results:
398+
if result.get("type") == "mcp_call":
399+
content = result.get("content")
400+
if not isinstance(content, str) or not content:
401+
logger.warning(
402+
"Skipping mcp_call tool result with missing or "
403+
"non-string content: %r",
404+
content,
405+
)
406+
continue
407+
rag_chunks.extend(
408+
[{"content": chunk} for chunk in content.split("---")]
409+
)
410+
response_data["rag_chunks"] = rag_chunks
411+
412+
def _format_infer_tool_calls(self, response_data: dict[str, Any]) -> None:
413+
"""Format infer endpoint tool calls to standard list[list[dict]] format.
414+
415+
Converts raw tool_call dicts (with 'name'/'args' keys) to the
416+
normalised format used by APIResponse, and matches each call to
417+
its corresponding tool_result by ID when available.
418+
419+
Args:
420+
response_data: The raw JSON response dict to update in place.
421+
"""
422+
if "tool_calls" not in response_data or not response_data["tool_calls"]:
423+
return
424+
raw_tool_calls = response_data["tool_calls"]
425+
formatted_tool_calls = []
426+
for tool_call in raw_tool_calls:
427+
if isinstance(tool_call, dict):
428+
formatted_tool: dict[str, object] = {
429+
"tool_name": tool_call.get("name", ""),
430+
"arguments": tool_call.get("args", {}),
431+
}
432+
if "tool_results" in response_data.get("data", {}):
433+
tool_call_id = tool_call.get("id")
434+
matching_result = next(
435+
(
436+
r
437+
for r in response_data["data"]["tool_results"]
438+
if r.get("id") == tool_call_id
439+
),
440+
None,
441+
)
442+
if matching_result:
443+
formatted_tool["result"] = matching_result.get(
444+
"content", matching_result.get("status", "")
445+
)
446+
formatted_tool["status"] = matching_result.get("status", "")
447+
formatted_tool_calls.append([formatted_tool])
448+
response_data["tool_calls"] = formatted_tool_calls
449+
450+
def _rlsapi_infer_query(self, api_request: APIRequest) -> APIResponse:
451+
"""Query the RLSAPI /infer endpoint for tool call and RAG metadata.
452+
453+
The infer endpoint uses a different request/response format than
454+
the standard query/streaming endpoints, converting "query" to
455+
"question" and parsing tool_calls and rag_chunks from tool_results.
456+
457+
Args:
458+
api_request: The prepared API request.
459+
460+
Returns:
461+
APIResponse with response text, tool calls, and RAG contexts.
462+
463+
Raises:
464+
APIError: If the request fails or response is invalid.
465+
"""
466+
if not self.client:
467+
raise APIError("HTTP client not initialized")
468+
try:
469+
infer_request = self._build_infer_request(api_request)
470+
471+
response = self.client.post(
472+
f"/api/lightspeed/{self.config.version}/infer",
473+
json=infer_request,
474+
)
475+
response.raise_for_status()
476+
477+
response_data = response.json()
478+
self._extract_infer_data(response_data)
479+
480+
if "response" not in response_data:
481+
raise APIError("API response missing 'response' field")
482+
483+
self._format_infer_tool_calls(response_data)
484+
485+
return APIResponse.from_raw_response(response_data)
486+
487+
except httpx.TimeoutException as e:
488+
raise self._handle_timeout_error("infer", self.config.timeout) from e
489+
except httpx.HTTPStatusError as e:
490+
if _is_retryable_server_error(e):
491+
raise
492+
raise self._handle_http_error(e) from e
493+
except ValueError as e:
494+
raise self._handle_validation_error(e) from e
495+
except APIError:
496+
raise
497+
except Exception as e:
498+
raise self._handle_unexpected_error(e, "infer query") from e
499+
327500
def _handle_response_errors(self, response: httpx.Response) -> None:
328501
"""Handle HTTP response errors for streaming endpoint."""
329502
if response.status_code != 200:

src/lightspeed_evaluation/core/constants.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
DEFAULT_API_VERSION = "v1"
5757
DEFAULT_API_TIMEOUT = 300
5858
DEFAULT_ENDPOINT_TYPE = "streaming"
59-
SUPPORTED_ENDPOINT_TYPES = ["streaming", "query"]
59+
SUPPORTED_ENDPOINT_TYPES = ["streaming", "query", "infer"]
6060
DEFAULT_API_CACHE_DIR = ".caches/api_cache"
6161

6262
DEFAULT_API_NUM_RETRIES = 3

src/lightspeed_evaluation/core/models/data.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,14 @@ class EvaluationData(BaseModel):
370370
min_length=1,
371371
description="Tag for grouping and filtering conversations",
372372
)
373+
skip: bool = Field(
374+
default=False,
375+
description="Skip this conversation during evaluation",
376+
)
377+
skip_reason: Optional[str] = Field(
378+
default=None,
379+
description="Why this conversation is skipped (documentation only)",
380+
)
373381

374382
# Conversation-level metrics
375383
conversation_metrics: Optional[list[str]] = Field(

src/lightspeed_evaluation/core/models/system.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ class APIConfig(BaseModel):
272272
)
273273
endpoint_type: str = Field(
274274
default=DEFAULT_ENDPOINT_TYPE,
275-
description="API endpoint type (streaming or query)",
275+
description="API endpoint type (streaming, query, or infer)",
276276
)
277277
timeout: int = Field(
278278
default=DEFAULT_API_TIMEOUT, ge=1, description="Request timeout in seconds"
@@ -302,7 +302,7 @@ class APIConfig(BaseModel):
302302
ge=0,
303303
description=(
304304
"Maximum number of retry attempts for API calls on "
305-
"429 Too Many Requests errors"
305+
"retryable server errors (HTTP 429/5xx)"
306306
),
307307
)
308308

0 commit comments

Comments
 (0)