Compare commits

..
Author SHA1 Message Date
Gerald LonlasandClaude Opus 4.7 2301e15111 fix: include both endpoint errors when chat.completions fallback also fails
When the responses endpoint fails and the chat.completions fallback also
fails (raises or returns empty), the original responses error was captured
in fallback_reason but never surfaced. Wrap the fallback call in its own
try/except and re-raise a RuntimeError that chains both errors with
context, so users see why both endpoints failed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-19 00:46:20 +08:00
10 changed files with 156 additions and 930 deletions
+16 -6
View File
@@ -1,6 +1,7 @@
from __future__ import annotations
import base64
import json
import math
import re
import secrets
@@ -34,9 +35,9 @@ def normalize_server_url(server_url: str) -> str:
if not parsed.netloc:
raise ValueError("server_url must include host and optional port")
# Ensure no trailing slash and no duplicate /v1 (case-insensitive).
# Ensure no trailing slash and no duplicate /v1.
value = value.rstrip("/")
if value.lower().endswith("/v1"):
if value.endswith("/v1"):
value = value[:-3]
return value
@@ -232,10 +233,9 @@ def extract_responses_text(response: Any) -> str:
text = getattr(entry, "text", None)
if isinstance(text, str):
parts.append(text)
else:
text = getattr(item, "text", None)
if isinstance(text, str):
parts.append(text)
text = getattr(item, "text", None)
if isinstance(text, str):
parts.append(text)
if parts:
return "\n".join(parts)
@@ -243,6 +243,16 @@ def extract_responses_text(response: Any) -> str:
return ""
def dump_openai_response(response: Any) -> str:
if hasattr(response, "model_dump"):
return json.dumps(response.model_dump(), indent=2, default=str)
if hasattr(response, "to_dict"):
return json.dumps(response.to_dict(), indent=2, default=str)
if isinstance(response, dict):
return json.dumps(response, indent=2, default=str)
return str(response)
_THINK_BLOCK_RE = re.compile(r"<think>.*?</think>", flags=re.IGNORECASE | re.DOTALL)
_THINK_START_RE = re.compile(r"<think>.*$", flags=re.IGNORECASE | re.DOTALL)
+21 -15
View File
@@ -164,21 +164,27 @@ class LMStudioImageToText(io.ComfyNode):
first_image = image[0] if is_batch else image
image_data_url = comfy_image_to_base64_png_url(first_image)
completion = client.chat.completions.create(
model=connection.model,
messages=build_chat_messages_with_image(
system_prompt=system_prompt,
user_prompt=user_prompt,
image_data_url=image_data_url,
),
seed=resolved_seed,
temperature=connection.temperature,
max_tokens=connection.max_tokens,
n=1,
)
text = extract_chat_completion_text(completion).strip()
if not text:
raise ValueError("chat.completions fallback returned no text output")
try:
completion = client.chat.completions.create(
model=connection.model,
messages=build_chat_messages_with_image(
system_prompt=system_prompt,
user_prompt=user_prompt,
image_data_url=image_data_url,
),
seed=resolved_seed,
temperature=connection.temperature,
max_tokens=connection.max_tokens,
n=1,
)
text = extract_chat_completion_text(completion).strip()
if not text:
raise ValueError("chat.completions fallback returned no text output")
except Exception as chat_error:
raise RuntimeError(
f"Both endpoints failed. responses error: {fallback_reason}. "
f"chat.completions error: {chat_error}"
) from chat_error
if batch_size > 1:
fallback_reason += (
-1
View File
@@ -13,7 +13,6 @@ dependencies = [
test = [
"pytest>=8.0.0",
"pytest-cov>=5.0.0",
"aiohttp>=3.9.0",
]
[tool.comfy]
+8
View File
@@ -14,6 +14,14 @@ def make_chat_completion(content: Any) -> SimpleNamespace:
return ns(choices=[ns(message=ns(content=content))])
def make_response_with_output_text(text: str) -> SimpleNamespace:
return ns(output_text=text)
def make_response_with_output_items(*items: Any) -> SimpleNamespace:
return ns(output=list(items))
class FakeTensor:
def __init__(self, array: np.ndarray):
self._array = np.asarray(array, dtype=np.float32)
-545
View File
@@ -1,545 +0,0 @@
"""
Tests for behavioral contracts that 100% line/branch coverage leaves unverified.
Each test is annotated with the mutation it would catch (i.e. what regression
would slip through if the test didn't exist).
"""
from __future__ import annotations
import asyncio
import json
from types import SimpleNamespace
import pytest
import client
from helpers.imports import import_repo_module
# ---------------------------------------------------------------------------
# client.py – normalize_server_url
# ---------------------------------------------------------------------------
def test_normalize_server_url_whitespace_only_raises() -> None:
"""
Whitespace-only input must raise 'must not be empty'.
A mutation that strips before the emptiness check would silently accept
whitespace and try to parse it, producing a confusing error or wrong URL.
"""
with pytest.raises(ValueError, match="must not be empty"):
client.normalize_server_url(" ")
def test_normalize_server_url_trailing_slash_no_v1() -> None:
"""
A URL with a trailing slash but no /v1 suffix must have the slash stripped.
If the strip was removed, to_openai_base_url would produce 'http://host//v1'.
"""
assert client.normalize_server_url("http://127.0.0.1:1234/") == "http://127.0.0.1:1234"
def test_normalize_server_url_https_accepted() -> None:
"""
https:// scheme must be accepted.
A mutation that only allows http:// would silently break TLS setups.
"""
assert client.normalize_server_url("https://example.com:5000") == "https://example.com:5000"
# ---------------------------------------------------------------------------
# client.py – coerce_seed: whitespace-only string
# ---------------------------------------------------------------------------
def test_coerce_seed_whitespace_only_string_returns_none() -> None:
"""
A whitespace-only seed string (e.g. " ") must return None (treated as
'no seed provided'), exactly like an empty string.
A mutation that compared `value == ""` instead of `not stripped` would
return None for "" but raise ValueError for " ", breaking nodes that
receive padding from UI widgets.
"""
assert client.coerce_seed(" ") is None
assert client.coerce_seed("\t") is None
# ---------------------------------------------------------------------------
# client.py – resolve_request_seed: seed=0 is valid (boundary)
# ---------------------------------------------------------------------------
def test_resolve_request_seed_zero_is_valid() -> None:
"""
seed=0 is explicitly >= 0 and not -1, so it must be returned as-is.
A mutation of the seed == -1 check to seed <= 0 would incorrectly
randomise seed=0 instead of honouring the deterministic value.
"""
assert client.resolve_request_seed(0) == 0
def test_resolve_request_seed_large_positive_passes_through() -> None:
"""
A large positive seed must pass through unchanged.
Ensures no accidental clipping or modulo is applied.
"""
big = 2**62
assert client.resolve_request_seed(big) == big
# ---------------------------------------------------------------------------
# client.py – build_responses_input_text: content structure
# ---------------------------------------------------------------------------
def test_build_responses_input_text_content_type_is_input_text() -> None:
"""
The 'type' field in the content entry MUST be 'input_text', not 'text'.
A mutation swapping the type string would break LMStudio's Responses API
but would not be caught by shape-only assertions.
"""
payload = client.build_responses_input_text("hello world")
entry = payload[0]["content"][0]
assert entry["type"] == "input_text"
assert entry["text"] == "hello world"
# Exactly one content entry; no extra fields.
assert len(payload[0]["content"]) == 1
# ---------------------------------------------------------------------------
# client.py – build_chat_messages: system prompt skipping edge
# ---------------------------------------------------------------------------
def test_build_chat_messages_whitespace_system_skipped() -> None:
"""
A system_prompt that is only whitespace must be omitted (same as empty),
so that only the user message appears.
A mutation removing the .strip() call would include a whitespace-only
system message, poisoning the model's context.
"""
messages = client.build_chat_messages(" \t ", "ask something")
assert len(messages) == 1
assert messages[0]["role"] == "user"
assert messages[0]["content"] == "ask something"
def test_build_chat_messages_empty_user_prompt_included() -> None:
"""
Even an empty user prompt must produce a user message (the slot must be
present). A mutation that skipped the user message when content is empty
would cause the API call to fail with a missing-message error.
"""
messages = client.build_chat_messages("System", "")
# system + user
assert len(messages) == 2
assert messages[1] == {"role": "user", "content": ""}
# ---------------------------------------------------------------------------
# client.py – list_models: ordering / dedup contract
# ---------------------------------------------------------------------------
def test_list_models_returns_sorted_unique_ids() -> None:
"""
list_models must return a sorted, deduplicated list.
A mutation removing sorted() would produce an arbitrary order, breaking
deterministic dropdown selection in the UI.
A mutation removing set() dedup would expose duplicate model entries.
"""
from types import SimpleNamespace as NS
model_data = [
NS(id="z-model"),
NS(id="a-model"),
NS(id="m-model"),
NS(id="a-model"), # duplicate
NS(id=""), # empty – filtered
]
fake_client = NS(models=NS(list=lambda: NS(data=model_data)))
result = client.list_models(fake_client)
assert result == ["a-model", "m-model", "z-model"]
# Idempotent: second call produces same order.
assert client.list_models(fake_client) == result
# ---------------------------------------------------------------------------
# client.py – strip_think_content: multiple blocks, ordering preserved
# ---------------------------------------------------------------------------
def test_strip_think_content_preserves_surrounding_text_order() -> None:
"""
Text before and after think blocks must appear in the correct order.
A mutation reversing the substitution or collapsing surrounding text
would reorder the visible output.
"""
result = client.strip_think_content("A <think>x</think> B <think>y</think> C")
assert result == "A B C"
def test_strip_think_content_only_think_block_returns_empty() -> None:
"""
A string consisting entirely of a think block must return "".
A mutation that stripped surrounding text instead of the block would
produce the block content rather than the empty string.
"""
assert client.strip_think_content("<think>only reasoning</think>") == ""
# ---------------------------------------------------------------------------
# routes.py – _query_int: negative integer rejected
# ---------------------------------------------------------------------------
def _routes_module():
return import_repo_module("routes", force_reload=True)
def test_query_int_negative_value_raises() -> None:
"""
Negative integers (e.g. -1, -5) must be rejected with '>= 1'.
The check is `value < 1`, which covers both 0 (already tested upstream)
and negative numbers. A mutation changing < 1 to == 0 would silently
accept -1 as a timeout, causing immediate connection failures.
"""
routes = _routes_module()
for raw in ["-1", "-100"]:
with pytest.raises(ValueError, match="must be >= 1"):
routes._query_int(
SimpleNamespace(query={"timeout_seconds": raw}),
"timeout_seconds",
15,
)
def test_query_int_empty_string_uses_default() -> None:
"""
An empty-string query value must fall back to the default, not raise.
A mutation that only checked `raw is None` (not `raw == ""`) would try
to parse "" and raise ValueError instead of returning the default.
"""
routes = _routes_module()
request = SimpleNamespace(query={"timeout_seconds": ""})
assert routes._query_int(request, "timeout_seconds", 15) == 15
# ---------------------------------------------------------------------------
# routes.py – _models_handler: empty models list produces correct default_model
# ---------------------------------------------------------------------------
def test_models_handler_empty_models_default_model_is_empty_string(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
When the server returns no models, default_model must be "" (empty string),
not raise an IndexError.
A mutation changing `models[0] if models else ""` to just `models[0]`
would crash on empty lists.
"""
routes = _routes_module()
monkeypatch.setattr(routes, "normalize_server_url", lambda url: "http://x:1234")
monkeypatch.setattr(routes, "get_server_models", lambda **kwargs: [])
request = SimpleNamespace(query={"server_url": "http://x:1234"})
response = asyncio.run(routes._models_handler(request))
payload = json.loads(response.text)
assert response.status == 200
assert payload["ok"] is True
assert payload["default_model"] == ""
assert payload["models"] == []
# ---------------------------------------------------------------------------
# routes.py – _test_handler: model_count matches actual list length
# ---------------------------------------------------------------------------
def test_test_handler_model_count_matches_models_list(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
model_count in the response must equal len(models).
The existing test only checks model_count==1; this one checks that the
count is derived from the list, not hardcoded.
A mutation replacing len(models) with 1 would pass the existing test
but fail here.
"""
routes = _routes_module()
monkeypatch.setattr(routes, "normalize_server_url", lambda url: "http://x:1234")
monkeypatch.setattr(routes, "get_server_models", lambda **kwargs: ["a", "b", "c"])
request = SimpleNamespace(query={"server_url": "http://x:1234"})
response = asyncio.run(routes._test_handler(request))
payload = json.loads(response.text)
assert payload["model_count"] == 3
assert payload["models"] == ["a", "b", "c"]
assert "3 model(s)" in payload["message"]
def test_test_handler_error_prefix_is_connection_test_failed(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
The 502 error message for _test_handler must use the prefix
'Connection test failed', NOT 'Failed to list models' (which is the
_models_handler prefix). A copy-paste mutation that shared the same
error string between the two handlers would confuse operator debugging.
"""
routes = _routes_module()
monkeypatch.setattr(routes, "normalize_server_url", lambda url: "http://x:1234")
monkeypatch.setattr(
routes,
"get_server_models",
lambda **kwargs: (_ for _ in ()).throw(RuntimeError("timeout")),
)
request = SimpleNamespace(query={"server_url": "http://x:1234"})
response = asyncio.run(routes._test_handler(request))
payload = json.loads(response.text)
assert response.status == 502
assert "Connection test failed" in payload["error"]
assert "Failed to list models" not in payload["error"]
def test_models_handler_error_prefix_is_failed_to_list(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
Symmetric guard: the 502 error from _models_handler must use
'Failed to list models', not 'Connection test failed'.
"""
routes = _routes_module()
monkeypatch.setattr(routes, "normalize_server_url", lambda url: "http://x:1234")
monkeypatch.setattr(
routes,
"get_server_models",
lambda **kwargs: (_ for _ in ()).throw(RuntimeError("boom")),
)
request = SimpleNamespace(query={"server_url": "http://x:1234"})
response = asyncio.run(routes._models_handler(request))
payload = json.loads(response.text)
assert response.status == 502
assert "Failed to list models" in payload["error"]
assert "Connection test failed" not in payload["error"]
# ---------------------------------------------------------------------------
# connect_node.py – auto-discover model when model="" and test_connectivity=False
# ---------------------------------------------------------------------------
def _connect_module():
return import_repo_module("connect_node", force_reload=True)
def test_execute_discovers_model_when_empty_and_connectivity_disabled(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
When model="" (blank) and test_connectivity=False, the node must still
probe the server (because no model is selected) and use the first result.
should_probe_models is `test_connectivity OR model in {"", MODEL_PLACEHOLDER}`.
A mutation changing `or` to `and` would skip the probe and raise ValueError
('No model selected') instead of auto-selecting.
"""
connect_node = _connect_module()
monkeypatch.setattr(connect_node, "get_server_models", lambda **_: ["auto-model"])
output = connect_node.LMStudioConnect.execute(
server_url="http://127.0.0.1:1234",
api_token="-",
model="",
reasoning_enabled=False,
test_connectivity=False, # disabled, but model is blank → must still probe
max_tokens=128,
temperature=0.5,
timeout_seconds=10,
use_tooling_mcp=False,
)
payload = output[0]
assert payload.model == "auto-model"
def test_execute_status_no_probe_does_not_say_found_models(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
When test_connectivity=False and a model is explicitly set, the status
message must say 'Connection prepared' and NOT say 'Found N model(s)'.
A mutation that always used the probe-status template would expose
incorrect model-count information.
"""
connect_node = _connect_module()
monkeypatch.setattr(
connect_node,
"get_server_models",
lambda **_: (_ for _ in ()).throw(AssertionError("must not probe")),
)
output = connect_node.LMStudioConnect.execute(
server_url="http://127.0.0.1:1234",
api_token="-",
model="chosen-model",
reasoning_enabled=False,
test_connectivity=False,
max_tokens=128,
temperature=0.5,
timeout_seconds=10,
use_tooling_mcp=False,
)
status = output[1]
assert "Connection prepared" in status
assert "Found" not in status
def test_execute_status_with_probe_includes_model_count(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
When the probe runs, the status must include 'Found N model(s)' with the
actual count.
A mutation hardcoding the count as 0 or 1 would pass the existing test
(which uses 2 models) but would be exposed by checking a different count.
"""
connect_node = _connect_module()
monkeypatch.setattr(
connect_node,
"get_server_models",
lambda **_: ["x", "y", "z"],
)
output = connect_node.LMStudioConnect.execute(
server_url="http://127.0.0.1:1234",
api_token="-",
model="x",
reasoning_enabled=False,
test_connectivity=True,
max_tokens=128,
temperature=0.5,
timeout_seconds=10,
use_tooling_mcp=False,
)
status = output[1]
assert "Found 3 model(s)" in status
# ---------------------------------------------------------------------------
# text_gen_node.py – _responses_kwargs: 'input' field is always present
# ---------------------------------------------------------------------------
def _text_module():
return import_repo_module("text_gen_node", force_reload=True)
def _connection_payload(**overrides):
models = import_repo_module("models", force_reload=True)
base = dict(
server_url="http://127.0.0.1:1234",
base_url="http://127.0.0.1:1234/v1",
api_key="token",
model="model-a",
reasoning_enabled=False,
max_tokens=111,
temperature=0.4,
timeout_seconds=30,
use_tooling_mcp=False,
)
base.update(overrides)
return models.LMStudioConnectionPayload(**base)
def test_responses_kwargs_input_field_is_present_and_has_user_role() -> None:
"""
The 'input' key in _responses_kwargs must be set to a list containing
a user-role message with the prompt text.
A mutation that removed or misspelled the 'input' key would send an
incomplete payload that LMStudio's Responses API would reject.
"""
text_gen_node = _text_module()
kwargs = text_gen_node.LMStudioTextGen._responses_kwargs(
connection=_connection_payload(),
system_prompt="",
user_prompt="describe the image",
seed=1,
)
assert "input" in kwargs
assert isinstance(kwargs["input"], list)
assert len(kwargs["input"]) == 1
assert kwargs["input"][0]["role"] == "user"
# The content must carry the user_prompt text.
content = kwargs["input"][0]["content"]
assert any(
(isinstance(entry, dict) and entry.get("text") == "describe the image")
for entry in content
), f"user_prompt not found in content: {content!r}"
def test_responses_kwargs_temperature_and_max_tokens_forwarded() -> None:
"""
temperature and max_output_tokens in _responses_kwargs must mirror the
connection payload values exactly.
A mutation swapping temperature and max_output_tokens, or using a
hardcoded default, would break generation quality silently.
"""
text_gen_node = _text_module()
conn = _connection_payload(temperature=1.5, max_tokens=512)
kwargs = text_gen_node.LMStudioTextGen._responses_kwargs(
connection=conn,
system_prompt="",
user_prompt="hello",
seed=7,
)
assert kwargs["temperature"] == 1.5
assert kwargs["max_output_tokens"] == 512
# ---------------------------------------------------------------------------
# text_gen_node.py – execute: seed appears in status for fallback path too
# ---------------------------------------------------------------------------
def test_execute_fallback_status_includes_seed(monkeypatch: pytest.MonkeyPatch) -> None:
"""
The status message must include the resolved seed even when falling back
to chat.completions. The existing fallback test checks for the fallback
reason, but NOT for the seed, so a mutation zeroing the seed in the
status template would go undetected.
"""
text_gen_node = _text_module()
completion = SimpleNamespace(
choices=[SimpleNamespace(message=SimpleNamespace(content="Answer"))]
)
fake_client = SimpleNamespace(
responses=SimpleNamespace(
create=lambda **_: (_ for _ in ()).throw(RuntimeError("responses off"))
),
chat=SimpleNamespace(completions=SimpleNamespace(create=lambda **_: completion)),
)
monkeypatch.setattr(text_gen_node, "resolve_request_seed", lambda _: 42)
monkeypatch.setattr(text_gen_node, "create_openai_client", lambda **_: fake_client)
output = text_gen_node.LMStudioTextGen.execute(
connection=_connection_payload(),
system_prompt="sys",
user_prompt="hello",
seed=0,
)
assert output[0] == "Answer"
assert "(seed=42)" in output.ui.text
assert "via chat.completions" in output.ui.text
def test_execute_user_prompt_none_treated_as_empty_raises() -> None:
"""
user_prompt=None must be treated as empty and raise ValueError.
A mutation that skipped the `user_prompt = user_prompt or ""` coercion
would cause a NoneType AttributeError instead of a clean ValueError.
"""
text_gen_node = _text_module()
with pytest.raises(ValueError, match="user_prompt must not be empty"):
text_gen_node.LMStudioTextGen.execute(
connection=_connection_payload(),
system_prompt="sys",
user_prompt=None,
seed=1,
)
+15 -51
View File
@@ -2,6 +2,7 @@ from __future__ import annotations
import base64
from io import BytesIO
import json
import sys
import types
@@ -29,18 +30,6 @@ def test_normalize_server_url_rejects_invalid_values() -> None:
client.normalize_server_url("http://")
def test_normalize_server_url_strips_v1_case_insensitive() -> None:
# Lowercase (existing behaviour).
assert client.normalize_server_url("http://localhost:1234/v1") == "http://localhost:1234"
# Uppercase /V1 was silently left in place before the fix, causing
# to_openai_base_url() to produce "…/V1/v1" — a double-suffix broken URL.
assert client.normalize_server_url("http://localhost:1234/V1") == "http://localhost:1234"
# Mixed case.
assert client.normalize_server_url("http://localhost:1234/V1/") == "http://localhost:1234"
# to_openai_base_url must append exactly one /v1 regardless of input casing.
assert client.to_openai_base_url("http://localhost:1234/V1") == "http://localhost:1234/v1"
def test_to_openai_base_url_appends_v1() -> None:
assert client.to_openai_base_url("http://127.0.0.1:1234") == "http://127.0.0.1:1234/v1"
@@ -194,48 +183,23 @@ def test_extract_responses_text_variants() -> None:
assert client.extract_responses_text(ns(output_text="", output=[])) == ""
def test_extract_responses_text_no_duplicate_when_item_has_content_and_text() -> None:
# An output item that has BOTH a content list and a top-level text attribute
# should only yield text from the content list; the item-level text field
# must NOT be appended a second time (regression guard for the duplicate-text bug).
item_with_both = ns(content=[ns(text="the-answer")], text="the-answer")
response = ns(output=[item_with_both])
result = client.extract_responses_text(response)
assert result == "the-answer", (
f"Expected 'the-answer' but got {result!r} — "
"item.text was appended even though content was already processed"
)
def test_dump_openai_response_branches() -> None:
class WithModelDump:
def model_dump(self):
return {"a": 1}
class WithToDict:
def to_dict(self):
return {"b": 2}
class Plain:
def __str__(self) -> str:
return "plain-response"
def test_extract_responses_text_dict_item_non_list_content_is_skipped() -> None:
# Branch 221->227: dict item where "content" is not a list.
# The function should skip it (continue) and return empty string — not crash.
response = ns(output=[{"content": "not-a-list"}, {"content": {"nested": "dict"}}])
assert client.extract_responses_text(response) == ""
def test_extract_responses_text_dict_item_content_list_with_non_dict_entries() -> None:
# Branch 223->222: content list contains non-dict entries (e.g. plain strings).
# Those entries must be silently ignored; only dict entries with a str "text" key count.
response = ns(output=[{"content": ["string-entry", 42, None, {"text": "kept"}]}])
assert client.extract_responses_text(response) == "kept"
def test_extract_responses_text_dict_item_content_entry_text_non_string() -> None:
# Branch 225->222: content entry is a dict but "text" value is not a str.
# These entries must be silently ignored.
response = ns(output=[{"content": [{"text": 42}, {"text": None}, {"text": "good"}]}])
assert client.extract_responses_text(response) == "good"
def test_extract_responses_text_object_item_content_entry_text_non_string() -> None:
# Branch 233->231: object item with content list where getattr(entry, "text") is not a str.
# The entry should be skipped; other valid entries should still be collected.
bad_entry = ns(text=99)
good_entry = ns(text="valid")
response = ns(output=[ns(content=[bad_entry, good_entry])])
assert client.extract_responses_text(response) == "valid"
assert json.loads(client.dump_openai_response(WithModelDump())) == {"a": 1}
assert json.loads(client.dump_openai_response(WithToDict())) == {"b": 2}
assert json.loads(client.dump_openai_response({"c": 3})) == {"c": 3}
assert client.dump_openai_response(Plain()) == "plain-response"
def test_strip_think_content_case_insensitive_and_multiple() -> None:
-300
View File
@@ -1,300 +0,0 @@
"""
Tests targeting the remaining uncovered branches in client.py and image_to_text_node.py.
Coverage gaps addressed (as reported by pytest-cov on the main branch):
client.py : lines 117-118, 142, 207, 222->228, 224->223, 226->223, 234->232, 291->293
image_to_text_node: lines 112->114, 157, 181, 183->189
"""
from __future__ import annotations
from types import SimpleNamespace
import numpy as np
import pytest
import client
from helpers.fakes import FakeTensor, make_chat_completion, ns
from helpers.imports import import_repo_module
# ---------------------------------------------------------------------------
# client.py – coerce_seed: .item() call raises, falls back to plain value
# ---------------------------------------------------------------------------
class _BadItem:
"""Numpy-scalar-like object whose .item() always raises."""
def item(self):
raise RuntimeError("item() failed")
def test_coerce_seed_item_raises_falls_back_to_unsupported_type() -> None:
"""
When value has .item() but the call raises, coerce_seed catches the exception
(lines 117-118) and falls through to the type-check below. Since _BadItem is
not int/float/str/bytes, it must then hit the final raise (line 142).
"""
with pytest.raises(ValueError, match="Unsupported seed type"):
client.coerce_seed(_BadItem())
def test_coerce_seed_unsupported_type_raises() -> None:
"""
A plain object with no recognised numeric/string interface must raise the
'Unsupported seed type' error (line 142).
"""
with pytest.raises(ValueError, match="Unsupported seed type"):
client.coerce_seed(object())
with pytest.raises(ValueError, match="Unsupported seed type"):
client.coerce_seed({"a": 1})
# ---------------------------------------------------------------------------
# client.py – extract_chat_completion_text: content=[] returns ""
# ---------------------------------------------------------------------------
def test_extract_chat_completion_text_empty_list_content() -> None:
"""
When the message.content is an empty list the function should return "".
This exercises the branch at line 207 where content is a list but parts
ends up empty so "\n".join returns "".
"""
completion = make_chat_completion([])
assert client.extract_chat_completion_text(completion) == ""
def test_extract_chat_completion_text_list_with_no_usable_text() -> None:
"""
List content items where getattr(part, 'text', None) is None and the item
is also not a dict – they are silently skipped, exercising the branches
at lines 222->228 / 224->223 / 226->223.
"""
content = [
ns(other="no-text"), # has no .text, not a dict – skipped
ns(text=None), # text attr is None – skipped
{"other": "ignored"}, # dict without 'text' key – skipped
ns(text="kept"), # text attr is str – included
]
completion = make_chat_completion(content)
assert client.extract_chat_completion_text(completion) == "kept"
def test_extract_chat_completion_text_non_list_non_str_content() -> None:
"""
When message.content is neither a str nor a list (e.g. an int or None),
the function should return "" via the final branch at line 207.
"""
assert client.extract_chat_completion_text(make_chat_completion(42)) == ""
assert client.extract_chat_completion_text(make_chat_completion(None)) == ""
# ---------------------------------------------------------------------------
# client.py – extract_responses_text: output list with no content at all
# ---------------------------------------------------------------------------
def test_extract_responses_text_output_list_empty_items() -> None:
"""
output is a list but its items carry neither usable 'content' lists nor
a 'text' attribute, so parts stays empty and the function returns "".
This exercises the branch at line 234->232.
"""
response = ns(output_text="", output=[ns(content=None), ns()])
assert client.extract_responses_text(response) == ""
def test_extract_responses_text_output_none() -> None:
"""output attribute is None – falls through to return "" at the end."""
assert client.extract_responses_text(ns(output_text="", output=None)) == ""
# ---------------------------------------------------------------------------
# client.py – build_chat_messages_with_image: no system prompt (line 291->293)
# ---------------------------------------------------------------------------
def test_build_chat_messages_with_image_no_system_prompt() -> None:
"""
When system_prompt is blank/empty the system message must be omitted so
only the user message is present. This covers branch 291->293.
"""
messages = client.build_chat_messages_with_image(
system_prompt=" ",
user_prompt="describe it",
image_data_url="data:image/png;base64,abc==",
)
assert len(messages) == 1
assert messages[0]["role"] == "user"
user_content = messages[0]["content"]
assert user_content[0] == {"type": "text", "text": "describe it"}
assert user_content[1]["type"] == "image_url"
# None system_prompt is treated the same way.
messages_none = client.build_chat_messages_with_image(
system_prompt=None,
user_prompt="hello",
image_data_url="data:image/png;base64,xyz==",
)
assert len(messages_none) == 1
# ---------------------------------------------------------------------------
# image_to_text_node.py – _responses_kwargs: empty system_prompt (line 112->114)
# ---------------------------------------------------------------------------
def _image_module():
return import_repo_module("image_to_text_node", force_reload=True)
def _connection_payload(*, reasoning_enabled: bool = False, use_tooling_mcp: bool = False):
models = import_repo_module("models", force_reload=True)
return models.LMStudioConnectionPayload(
server_url="http://127.0.0.1:1234",
base_url="http://127.0.0.1:1234/v1",
api_key="token",
model="vision-model",
reasoning_enabled=reasoning_enabled,
max_tokens=128,
temperature=0.3,
timeout_seconds=30,
use_tooling_mcp=use_tooling_mcp,
)
def _single_image() -> FakeTensor:
return FakeTensor(np.array([[[1.0, 0.0, 0.0]]], dtype=np.float32))
def test_responses_kwargs_no_system_prompt_omits_instructions() -> None:
"""
When system_prompt is blank the 'instructions' key must NOT appear in
the returned kwargs. This covers the branch at line 112->114.
"""
image_node = _image_module()
kwargs = image_node.LMStudioImageToText._responses_kwargs(
connection=_connection_payload(),
image=_single_image(),
system_prompt="",
user_prompt="describe",
seed=1,
)
assert "instructions" not in kwargs
kwargs_whitespace = image_node.LMStudioImageToText._responses_kwargs(
connection=_connection_payload(),
image=_single_image(),
system_prompt=" ",
user_prompt="describe",
seed=1,
)
assert "instructions" not in kwargs_whitespace
# ---------------------------------------------------------------------------
# image_to_text_node.py – execute: responses returns empty text (line 157)
# ---------------------------------------------------------------------------
def test_execute_responses_empty_text_falls_back_to_chat(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
When the responses endpoint returns an empty string the node must raise
a ValueError internally and fall back to chat.completions.
This covers line 157 ('raise ValueError("responses endpoint returned no text output")').
"""
image_node = _image_module()
completion = SimpleNamespace(
choices=[SimpleNamespace(message=SimpleNamespace(content="chat fallback text"))]
)
fake_client = SimpleNamespace(
responses=SimpleNamespace(create=lambda **_: SimpleNamespace(output_text="")),
chat=SimpleNamespace(completions=SimpleNamespace(create=lambda **_: completion)),
)
monkeypatch.setattr(image_node, "resolve_request_seed", lambda _: 5)
monkeypatch.setattr(image_node, "create_openai_client", lambda **_: fake_client)
output = image_node.LMStudioImageToText.execute(
connection=_connection_payload(),
image=_single_image(),
system_prompt="sys",
user_prompt="describe",
seed=-1,
)
assert output[0] == "chat fallback text"
assert "via chat.completions" in output.ui.text
assert "responses endpoint returned no text output" in output.ui.text
# ---------------------------------------------------------------------------
# image_to_text_node.py – execute: chat fallback also empty (line 181)
# ---------------------------------------------------------------------------
def test_execute_both_endpoints_empty_raises(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
When both the responses endpoint and the chat.completions fallback return
empty text, a ValueError must propagate to the caller.
This covers line 181 ('raise ValueError("chat.completions fallback returned no text output")').
"""
image_node = _image_module()
empty_completion = SimpleNamespace(
choices=[SimpleNamespace(message=SimpleNamespace(content=""))]
)
fake_client = SimpleNamespace(
responses=SimpleNamespace(create=lambda **_: SimpleNamespace(output_text="")),
chat=SimpleNamespace(completions=SimpleNamespace(create=lambda **_: empty_completion)),
)
monkeypatch.setattr(image_node, "resolve_request_seed", lambda _: 3)
monkeypatch.setattr(image_node, "create_openai_client", lambda **_: fake_client)
with pytest.raises(ValueError, match="chat.completions fallback returned no text output"):
image_node.LMStudioImageToText.execute(
connection=_connection_payload(),
image=_single_image(),
system_prompt="sys",
user_prompt="describe",
seed=0,
)
# ---------------------------------------------------------------------------
# image_to_text_node.py – execute: chat fallback single image, no dropped-frames note (line 183->189)
# ---------------------------------------------------------------------------
def test_execute_chat_fallback_single_image_no_dropped_frames_note(
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""
When the responses endpoint fails and the image is a *single* frame (not a
batch), the fallback status message must NOT mention dropped frames.
This covers the branch at line 183->189 where batch_size == 1.
"""
image_node = _image_module()
def raise_responses(**kwargs):
raise RuntimeError("responses unavailable")
completion = SimpleNamespace(
choices=[SimpleNamespace(message=SimpleNamespace(content="single fallback"))]
)
fake_client = SimpleNamespace(
responses=SimpleNamespace(create=raise_responses),
chat=SimpleNamespace(completions=SimpleNamespace(create=lambda **_: completion)),
)
monkeypatch.setattr(image_node, "resolve_request_seed", lambda _: 7)
monkeypatch.setattr(image_node, "create_openai_client", lambda **_: fake_client)
output = image_node.LMStudioImageToText.execute(
connection=_connection_payload(),
image=_single_image(),
system_prompt="sys",
user_prompt="describe",
seed=0,
)
assert output[0] == "single fallback"
assert "via chat.completions" in output.ui.text
assert "dropped" not in output.ui.text
+38
View File
@@ -158,3 +158,41 @@ def test_execute_chat_fallback_reports_dropped_frames(
assert "via chat.completions" in output.ui.text
assert "responses unavailable" in output.ui.text
assert "additional frame(s) were dropped" in output.ui.text
def test_execute_chains_both_errors_when_chat_completions_also_raises(
monkeypatch: pytest.MonkeyPatch,
) -> None:
image_node = _image_module()
def raise_responses(**kwargs):
raise RuntimeError("responses boom")
def raise_chat(**kwargs):
raise RuntimeError("chat boom")
fake_client = SimpleNamespace(
responses=SimpleNamespace(create=raise_responses),
chat=SimpleNamespace(completions=SimpleNamespace(create=raise_chat)),
)
monkeypatch.setattr(image_node, "resolve_request_seed", lambda _: 41)
monkeypatch.setattr(image_node, "create_openai_client", lambda **_: fake_client)
monkeypatch.setattr(
image_node, "comfy_image_to_base64_png_url", lambda _img: "data:image/png;base64,xxx"
)
with pytest.raises(RuntimeError) as exc_info:
image_node.LMStudioImageToText.execute(
connection=_connection_payload(),
image=_single_image(),
system_prompt="sys",
user_prompt="describe",
seed=0,
)
message = str(exc_info.value)
assert "Both endpoints failed" in message
assert "responses boom" in message
assert "chat boom" in message
assert isinstance(exc_info.value.__cause__, RuntimeError)
assert str(exc_info.value.__cause__) == "chat boom"
+41 -1
View File
@@ -161,10 +161,50 @@ def test_execute_raises_if_both_endpoints_return_no_text(
monkeypatch.setattr(text_gen_node, "create_openai_client", lambda **_: fake_client)
monkeypatch.setattr(text_gen_node, "resolve_request_seed", lambda _: 55)
with pytest.raises(ValueError, match="chat\\.completions fallback returned no text output"):
with pytest.raises(RuntimeError) as exc_info:
text_gen_node.LMStudioTextGen.execute(
connection=_connection_payload(),
system_prompt="sys",
user_prompt="hello",
seed=1,
)
message = str(exc_info.value)
assert "Both endpoints failed" in message
assert "responses endpoint returned no text output" in message
assert "chat.completions fallback returned no text output" in message
def test_execute_chains_both_errors_when_chat_completions_also_raises(
monkeypatch: pytest.MonkeyPatch,
) -> None:
text_gen_node = _text_module()
def raise_responses(**kwargs):
raise RuntimeError("responses boom")
def raise_chat(**kwargs):
raise RuntimeError("chat boom")
fake_client = SimpleNamespace(
responses=SimpleNamespace(create=raise_responses),
chat=SimpleNamespace(completions=SimpleNamespace(create=raise_chat)),
)
monkeypatch.setattr(text_gen_node, "create_openai_client", lambda **_: fake_client)
monkeypatch.setattr(text_gen_node, "resolve_request_seed", lambda _: 55)
with pytest.raises(RuntimeError) as exc_info:
text_gen_node.LMStudioTextGen.execute(
connection=_connection_payload(),
system_prompt="sys",
user_prompt="hello",
seed=1,
)
message = str(exc_info.value)
assert "Both endpoints failed" in message
assert "responses boom" in message
assert "chat boom" in message
assert isinstance(exc_info.value.__cause__, RuntimeError)
assert str(exc_info.value.__cause__) == "chat boom"
+17 -11
View File
@@ -136,17 +136,23 @@ class LMStudioTextGen(io.ComfyNode):
via_endpoint = "chat.completions"
fallback_reason = str(responses_error)
completion = client.chat.completions.create(
model=connection.model,
messages=build_chat_messages(system_prompt, user_prompt),
seed=resolved_seed,
temperature=connection.temperature,
max_tokens=connection.max_tokens,
n=1,
)
text = extract_chat_completion_text(completion).strip()
if not text:
raise ValueError("chat.completions fallback returned no text output")
try:
completion = client.chat.completions.create(
model=connection.model,
messages=build_chat_messages(system_prompt, user_prompt),
seed=resolved_seed,
temperature=connection.temperature,
max_tokens=connection.max_tokens,
n=1,
)
text = extract_chat_completion_text(completion).strip()
if not text:
raise ValueError("chat.completions fallback returned no text output")
except Exception as chat_error:
raise RuntimeError(
f"Both endpoints failed. responses error: {fallback_reason}. "
f"chat.completions error: {chat_error}"
) from chat_error
text = strip_think_content(text)