mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-23 00:41:40 +00:00
claude_code compat: skip skipped reports; drop unreachable stream-events check
- conftest: pytest_runtest_makereport now early-returns on report.skipped so pytest.skip(...) inside a compat test body doesn't get recorded as a phantom 'fail' row via the not-failed/empty-collected branch. - _basic_messaging: drop require_stream_events. The check (not outcome.events) cannot catch a buffering regression because cli_driver uses subprocess.run(capture_output=True), which only exposes the post-exit stdout blob — buffered-then-flushed and truly streamed responses are indistinguishable. The check was also unreachable as an independent failure path (empty events -> empty text -> the text check fires first). Update all five streaming callers and docstrings accordingly. Co-authored-by: Yassin Kortam <yassin@berri.ai>
This commit is contained in:
parent
f41d3f91a3
commit
b24059a92f
7 changed files with 38 additions and 33 deletions
|
|
@ -5,17 +5,16 @@ Every basic_messaging cell follows the same skeleton:
|
|||
1. Read the proxy base URL + API key from env, fail-early if missing.
|
||||
2. Fan the three Claude tiers out via `run_claude_models_parallel`.
|
||||
3. Inspect each model's outcome and report one `compat_result` row per
|
||||
model — `ClaudeCLIError`, non-zero exit, missing stream events
|
||||
(streaming variant only), and empty assistant text are all per-model
|
||||
fails; everything else is a per-model pass.
|
||||
model — `ClaudeCLIError`, non-zero exit, and empty assistant text
|
||||
are all per-model fails; everything else is a per-model pass.
|
||||
4. Surface a joined failure message via `pytest.fail(...)` so the
|
||||
pytest run also goes red.
|
||||
|
||||
The conftest infers `(feature_id, provider)` purely from the test file
|
||||
path, so each per-provider file just declares its model list and calls
|
||||
`run_basic_messaging_cell(...)`. This keeps all cell logic in one place
|
||||
— a future tweak to the env-missing guard, the failure-loop shape, or
|
||||
the stream-events check now propagates to every cell automatically.
|
||||
— a future tweak to the env-missing guard or the failure-loop shape
|
||||
now propagates to every cell automatically.
|
||||
|
||||
The leading underscore in the filename is what keeps pytest from
|
||||
collecting this module as a test file.
|
||||
|
|
@ -43,14 +42,17 @@ def run_basic_messaging_cell(
|
|||
compat_result,
|
||||
models: Sequence[str],
|
||||
prompt: str,
|
||||
require_stream_events: bool = False,
|
||||
) -> None:
|
||||
"""Run the shared `basic_messaging_*` × <provider> cell body.
|
||||
|
||||
`require_stream_events=True` adds the streaming-variant assertion
|
||||
that at least one stream-json event is observed for each model —
|
||||
the regression check that catches a proxy buffering the full
|
||||
response before flushing.
|
||||
The streaming and non-streaming variants share this body because
|
||||
the CLI driver consumes stdout via `subprocess.run(capture_output=True)`
|
||||
after the process exits — we can only observe that events arrived,
|
||||
not *when* they arrived. A wire-level "did the proxy buffer the
|
||||
full response before flushing?" check therefore can't live here;
|
||||
it belongs in a driver that streams stdout incrementally. Until
|
||||
that exists, the streaming cells exercise the same shape and
|
||||
check the same per-model outcomes as the non-streaming cells.
|
||||
"""
|
||||
base_url = os.environ.get(PROXY_BASE_URL_ENV)
|
||||
api_key = os.environ.get(PROXY_API_KEY_ENV)
|
||||
|
|
@ -91,12 +93,6 @@ def run_basic_messaging_cell(
|
|||
failures.append(error)
|
||||
continue
|
||||
|
||||
if require_stream_events and not outcome.events:
|
||||
error = f"[{model}] no stream-json events emitted; streaming wire silent"
|
||||
compat_result.add({"status": "fail", "error": error})
|
||||
failures.append(error)
|
||||
continue
|
||||
|
||||
if not outcome.text.strip():
|
||||
error = f"[{model}] claude returned empty assistant text"
|
||||
compat_result.add({"status": "fail", "error": error})
|
||||
|
|
|
|||
|
|
@ -6,10 +6,16 @@ report the outcome via `compat_result`.
|
|||
|
||||
The CLI is run with `--print --output-format stream-json`, which streams
|
||||
incremental events as the upstream produces tokens. The cell goes green
|
||||
only when every Claude tier returns a non-empty reply over a streamed
|
||||
wire (i.e. at least one stream-json event is observed). This catches
|
||||
regressions where the proxy buffers the full response before flushing,
|
||||
silently degrading the streaming experience customers rely on.
|
||||
only when every Claude tier returns a non-empty reply.
|
||||
|
||||
Note: a true "did the proxy buffer the full response before flushing?"
|
||||
check would require observing event arrival times on the wire, which
|
||||
the `cli_driver` cannot do today — it consumes stdout via
|
||||
`subprocess.run(capture_output=True)` after the process exits, so a
|
||||
buffered-then-flushed response is indistinguishable from a truly
|
||||
streamed one. That regression check belongs in a streaming-aware
|
||||
driver; until then this cell verifies the same shape as the
|
||||
non-streaming variant.
|
||||
|
||||
The (feature, provider) for this cell is inferred from the file path by
|
||||
`tests/claude_code/conftest.py`:
|
||||
|
|
@ -21,9 +27,7 @@ The (feature, provider) for this cell is inferred from the file path by
|
|||
The shared `run_basic_messaging_cell` helper fans the three Claude tiers
|
||||
out in parallel inside this single test, with one
|
||||
`compat_result.add(...)` entry per model so the matrix builder still
|
||||
sees three rows for this (feature, provider). The `require_stream_events`
|
||||
flag adds the streaming-only assertion that at least one stream-json
|
||||
event was observed per model.
|
||||
sees three rows for this (feature, provider).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -39,11 +43,10 @@ ANTHROPIC_MODELS = [
|
|||
|
||||
def test_basic_messaging_streaming_anthropic(compat_result):
|
||||
"""Drive the `claude` CLI against the LiteLLM proxy and assert a
|
||||
non-empty streamed reply (at least one stream-json event observed).
|
||||
non-empty streamed reply (one row per Claude tier).
|
||||
"""
|
||||
run_basic_messaging_cell(
|
||||
compat_result=compat_result,
|
||||
models=ANTHROPIC_MODELS,
|
||||
prompt="Count from 1 to 5, one number per line.",
|
||||
require_stream_events=True,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -30,11 +30,10 @@ AZURE_MODELS = [
|
|||
|
||||
def test_basic_messaging_streaming_azure(compat_result):
|
||||
"""Drive the `claude` CLI against the LiteLLM proxy and assert a
|
||||
non-empty streamed reply (at least one stream-json event observed).
|
||||
non-empty streamed reply (one row per Claude tier).
|
||||
"""
|
||||
run_basic_messaging_cell(
|
||||
compat_result=compat_result,
|
||||
models=AZURE_MODELS,
|
||||
prompt="Count from 1 to 5, one number per line.",
|
||||
require_stream_events=True,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -26,11 +26,10 @@ BEDROCK_CONVERSE_MODELS = [
|
|||
|
||||
def test_basic_messaging_streaming_bedrock_converse(compat_result):
|
||||
"""Drive the `claude` CLI against the LiteLLM proxy and assert a
|
||||
non-empty streamed reply (at least one stream-json event observed).
|
||||
non-empty streamed reply (one row per Claude tier).
|
||||
"""
|
||||
run_basic_messaging_cell(
|
||||
compat_result=compat_result,
|
||||
models=BEDROCK_CONVERSE_MODELS,
|
||||
prompt="Count from 1 to 5, one number per line.",
|
||||
require_stream_events=True,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -26,11 +26,10 @@ BEDROCK_INVOKE_MODELS = [
|
|||
|
||||
def test_basic_messaging_streaming_bedrock_invoke(compat_result):
|
||||
"""Drive the `claude` CLI against the LiteLLM proxy and assert a
|
||||
non-empty streamed reply (at least one stream-json event observed).
|
||||
non-empty streamed reply (one row per Claude tier).
|
||||
"""
|
||||
run_basic_messaging_cell(
|
||||
compat_result=compat_result,
|
||||
models=BEDROCK_INVOKE_MODELS,
|
||||
prompt="Count from 1 to 5, one number per line.",
|
||||
require_stream_events=True,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -26,11 +26,10 @@ VERTEX_AI_MODELS = [
|
|||
|
||||
def test_basic_messaging_streaming_vertex_ai(compat_result):
|
||||
"""Drive the `claude` CLI against the LiteLLM proxy and assert a
|
||||
non-empty streamed reply (at least one stream-json event observed).
|
||||
non-empty streamed reply (one row per Claude tier).
|
||||
"""
|
||||
run_basic_messaging_cell(
|
||||
compat_result=compat_result,
|
||||
models=VERTEX_AI_MODELS,
|
||||
prompt="Count from 1 to 5, one number per line.",
|
||||
require_stream_events=True,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -241,6 +241,16 @@ def pytest_runtest_makereport(item, call):
|
|||
elif report.when != "call":
|
||||
return
|
||||
|
||||
# A skipped test (e.g. `pytest.skip(...)` called inside the body or
|
||||
# by a `pytest.mark.skipif` evaluated at call time) is neither a
|
||||
# pass nor a fail — it just didn't run. Recording it as anything
|
||||
# here would produce a spurious row (the not-failed/empty-collected
|
||||
# branch below would mark it as a fail with "test passed without
|
||||
# reporting via compat_result"), so bail out and let the cell stay
|
||||
# "not_tested" in the published matrix.
|
||||
if report.skipped:
|
||||
return
|
||||
|
||||
inferred = _infer_feature_and_provider(Path(str(item.path)))
|
||||
if inferred is None:
|
||||
return
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue