Compare commits
7
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d9d5c77e0f | ||
|
|
1ccce73904 | ||
|
|
4fc78624c4 | ||
|
|
0d05296bc0 | ||
|
|
25d7309fa7 | ||
|
|
fa252dbac6 | ||
|
|
0e42058f20 |
@@ -1,7 +1,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import json
|
||||
import math
|
||||
import re
|
||||
import secrets
|
||||
@@ -35,9 +34,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.
|
||||
# Ensure no trailing slash and no duplicate /v1 (case-insensitive).
|
||||
value = value.rstrip("/")
|
||||
if value.endswith("/v1"):
|
||||
if value.lower().endswith("/v1"):
|
||||
value = value[:-3]
|
||||
return value
|
||||
|
||||
@@ -244,16 +243,6 @@ 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)
|
||||
|
||||
|
||||
@@ -187,6 +187,10 @@ class LMStudioImageToText(io.ComfyNode):
|
||||
)
|
||||
|
||||
text = strip_think_content(text)
|
||||
if not text:
|
||||
raise ValueError(
|
||||
"All content was inside <think> blocks; no visible text was produced."
|
||||
)
|
||||
|
||||
status = f"Model '{connection.model}' via {via_endpoint} (seed={resolved_seed})."
|
||||
if fallback_reason:
|
||||
|
||||
@@ -13,6 +13,7 @@ dependencies = [
|
||||
test = [
|
||||
"pytest>=8.0.0",
|
||||
"pytest-cov>=5.0.0",
|
||||
"aiohttp>=3.9.0",
|
||||
]
|
||||
|
||||
[tool.comfy]
|
||||
|
||||
@@ -14,14 +14,6 @@ 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)
|
||||
|
||||
@@ -0,0 +1,545 @@
|
||||
"""
|
||||
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,
|
||||
)
|
||||
+38
-15
@@ -2,7 +2,6 @@ from __future__ import annotations
|
||||
|
||||
import base64
|
||||
from io import BytesIO
|
||||
import json
|
||||
import sys
|
||||
import types
|
||||
|
||||
@@ -30,6 +29,18 @@ 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"
|
||||
|
||||
@@ -196,23 +207,35 @@ def test_extract_responses_text_no_duplicate_when_item_has_content_and_text() ->
|
||||
)
|
||||
|
||||
|
||||
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}
|
||||
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) == ""
|
||||
|
||||
class Plain:
|
||||
def __str__(self) -> str:
|
||||
return "plain-response"
|
||||
|
||||
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_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"
|
||||
|
||||
|
||||
def test_strip_think_content_case_insensitive_and_multiple() -> None:
|
||||
|
||||
@@ -164,18 +164,6 @@ def _single_image() -> FakeTensor:
|
||||
return FakeTensor(np.array([[[1.0, 0.0, 0.0]]], dtype=np.float32))
|
||||
|
||||
|
||||
def _batch_image() -> FakeTensor:
|
||||
return FakeTensor(
|
||||
np.array(
|
||||
[
|
||||
[[[1.0, 0.0, 0.0]]],
|
||||
[[[0.0, 0.0, 1.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
|
||||
|
||||
@@ -0,0 +1,376 @@
|
||||
"""
|
||||
Behavioral-contract tests for image encoding and node status messages.
|
||||
|
||||
Each test targets a specific mutation that existing coverage does not catch:
|
||||
|
||||
1. comfy_image_to_base64_png_url — image_url field key in _responses_kwargs content.
|
||||
2. comfy_image_to_base64_png_url — RGBA (4-channel) tensors round-trip correctly.
|
||||
3. comfy_image_to_base64_png_url — non-1x1 image preserves correct dimensions.
|
||||
4. image_to_text_node.execute — status message includes the model name.
|
||||
5. image_to_text_node._responses_kwargs — every image content entry has valid data URL.
|
||||
6. text_gen_node.execute — status message includes the model name.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import base64
|
||||
from io import BytesIO
|
||||
from types import SimpleNamespace
|
||||
|
||||
import numpy as np
|
||||
import pytest
|
||||
from PIL import Image
|
||||
|
||||
import client
|
||||
from helpers.fakes import FakeTensor
|
||||
from helpers.imports import import_repo_module
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _decode_data_url_png(data_url: str) -> Image.Image:
|
||||
assert data_url.startswith("data:image/png;base64,"), (
|
||||
f"Expected data:image/png;base64, prefix, got: {data_url[:40]!r}"
|
||||
)
|
||||
encoded = data_url.split(",", maxsplit=1)[1]
|
||||
raw = base64.b64decode(encoded)
|
||||
return Image.open(BytesIO(raw))
|
||||
|
||||
|
||||
def _image_module():
|
||||
return import_repo_module("image_to_text_node", force_reload=True)
|
||||
|
||||
|
||||
def _text_module():
|
||||
return import_repo_module("text_gen_node", force_reload=True)
|
||||
|
||||
|
||||
def _connection_payload(model: str = "vision-model"):
|
||||
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=model,
|
||||
reasoning_enabled=False,
|
||||
max_tokens=128,
|
||||
temperature=0.3,
|
||||
timeout_seconds=30,
|
||||
use_tooling_mcp=False,
|
||||
)
|
||||
|
||||
|
||||
def _single_image() -> FakeTensor:
|
||||
return FakeTensor(np.array([[[1.0, 0.0, 0.0]]], dtype=np.float32))
|
||||
|
||||
|
||||
def _batch_image() -> FakeTensor:
|
||||
return FakeTensor(
|
||||
np.array(
|
||||
[
|
||||
[[[1.0, 0.0, 0.0]]], # red
|
||||
[[[0.0, 0.0, 1.0]]], # blue
|
||||
],
|
||||
dtype=np.float32,
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# client.comfy_image_to_base64_png_url — RGBA (4-channel) round-trip
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_comfy_image_to_base64_png_url_rgba_round_trip() -> None:
|
||||
"""
|
||||
A 4-channel (RGBA) float32 tensor must survive the encode-decode round-trip.
|
||||
The encoded PNG must be decodable and preserve the RGB channels.
|
||||
A mutation that accidentally dropped channels (e.g. slicing to [:,:,:3]) would
|
||||
change the pixel values and break this assertion.
|
||||
"""
|
||||
# 1x1 RGBA image: R=1.0, G=0.5, B=0.0, A=0.8
|
||||
rgba = FakeTensor(np.array([[[1.0, 0.5, 0.0, 0.8]]], dtype=np.float32))
|
||||
url = client.comfy_image_to_base64_png_url(rgba)
|
||||
img = _decode_data_url_png(url)
|
||||
# PIL will create an RGBA image for 4-channel input.
|
||||
r, g, b, a = img.getpixel((0, 0))
|
||||
assert r == 255, f"Red channel: expected 255, got {r}"
|
||||
assert g == 127 or g == 128, f"Green channel: expected ~127, got {g}" # float rounding
|
||||
assert b == 0, f"Blue channel: expected 0, got {b}"
|
||||
assert a == 204 or a == 203, f"Alpha channel: expected ~204, got {a}" # 0.8*255
|
||||
|
||||
|
||||
def test_comfy_image_to_base64_png_url_correct_dimensions() -> None:
|
||||
"""
|
||||
The encoded PNG must preserve the exact width and height of the input tensor.
|
||||
A mutation that transposed H and W (e.g. using .T) would swap width/height
|
||||
and break this assertion.
|
||||
"""
|
||||
# 3 rows, 5 columns, 3 channels
|
||||
h, w = 3, 5
|
||||
arr = np.zeros((h, w, 3), dtype=np.float32)
|
||||
tensor = FakeTensor(arr)
|
||||
url = client.comfy_image_to_base64_png_url(tensor)
|
||||
img = _decode_data_url_png(url)
|
||||
assert img.size == (w, h), f"Expected ({w}, {h}), got {img.size}"
|
||||
|
||||
|
||||
def test_comfy_image_to_base64_png_url_batch_uses_first_frame_exactly() -> None:
|
||||
"""
|
||||
For a batch tensor (ndim==4), only frame 0 must be encoded.
|
||||
Frame 1 (blue) must NOT appear in the output.
|
||||
A mutation that encoded frame 1 instead of frame 0 would change the pixel
|
||||
from red to blue.
|
||||
"""
|
||||
batch = FakeTensor(
|
||||
np.array(
|
||||
[
|
||||
[[[1.0, 0.0, 0.0]]], # frame 0: red
|
||||
[[[0.0, 0.0, 1.0]]], # frame 1: blue
|
||||
],
|
||||
dtype=np.float32,
|
||||
)
|
||||
)
|
||||
url = client.comfy_image_to_base64_png_url(batch)
|
||||
img = _decode_data_url_png(url)
|
||||
r, g, b = img.getpixel((0, 0))[:3]
|
||||
assert r == 255 and g == 0 and b == 0, (
|
||||
f"Expected red (255,0,0) from frame 0, got ({r},{g},{b})"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# image_to_text_node._responses_kwargs — image_url field key correctness
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_responses_kwargs_image_content_field_key_is_image_url() -> None:
|
||||
"""
|
||||
Each image entry in the content list must use the key 'image_url', not 'url'
|
||||
or any other variant. A mutation renaming the field would break LMStudio's
|
||||
Responses API contract silently.
|
||||
This test covers both the single (ndim==3) and batch (ndim==4) paths.
|
||||
"""
|
||||
image_node = _image_module()
|
||||
|
||||
# Single image (3D tensor from __getitem__ on batch)
|
||||
single = FakeTensor(np.array([[[0.0, 1.0, 0.0]]], dtype=np.float32))
|
||||
kwargs_single = image_node.LMStudioImageToText._responses_kwargs(
|
||||
connection=_connection_payload(),
|
||||
image=single,
|
||||
system_prompt="sys",
|
||||
user_prompt="describe",
|
||||
seed=1,
|
||||
)
|
||||
single_content = kwargs_single["input"][0]["content"]
|
||||
image_entry = single_content[1]
|
||||
assert image_entry["type"] == "input_image"
|
||||
assert "image_url" in image_entry, (
|
||||
f"Expected 'image_url' key, found keys: {list(image_entry.keys())}"
|
||||
)
|
||||
assert image_entry["image_url"].startswith("data:image/png;base64,")
|
||||
|
||||
# Batch (4D tensor) — every image entry must have valid image_url
|
||||
batch = _batch_image()
|
||||
kwargs_batch = image_node.LMStudioImageToText._responses_kwargs(
|
||||
connection=_connection_payload(),
|
||||
image=batch,
|
||||
system_prompt="sys",
|
||||
user_prompt="describe",
|
||||
seed=2,
|
||||
)
|
||||
batch_content = kwargs_batch["input"][0]["content"]
|
||||
image_entries = [e for e in batch_content if e.get("type") == "input_image"]
|
||||
assert len(image_entries) == 2, f"Expected 2 image entries, got {len(image_entries)}"
|
||||
for i, entry in enumerate(image_entries):
|
||||
assert "image_url" in entry, (
|
||||
f"Image entry {i} missing 'image_url' key; keys: {list(entry.keys())}"
|
||||
)
|
||||
assert entry["image_url"].startswith("data:image/png;base64,"), (
|
||||
f"Image entry {i} 'image_url' lacks data URL prefix: {entry['image_url'][:40]!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_responses_kwargs_batch_images_are_distinct_data_urls() -> None:
|
||||
"""
|
||||
In a multi-frame batch, the data URLs for different frames must differ.
|
||||
A mutation that passed the same frame index for all images would produce
|
||||
identical URLs and break this assertion.
|
||||
"""
|
||||
image_node = _image_module()
|
||||
batch = FakeTensor(
|
||||
np.array(
|
||||
[
|
||||
[[[1.0, 0.0, 0.0]]], # red
|
||||
[[[0.0, 0.0, 1.0]]], # blue
|
||||
],
|
||||
dtype=np.float32,
|
||||
)
|
||||
)
|
||||
kwargs = image_node.LMStudioImageToText._responses_kwargs(
|
||||
connection=_connection_payload(),
|
||||
image=batch,
|
||||
system_prompt="",
|
||||
user_prompt="describe",
|
||||
seed=3,
|
||||
)
|
||||
content = kwargs["input"][0]["content"]
|
||||
image_urls = [e["image_url"] for e in content if e.get("type") == "input_image"]
|
||||
assert len(image_urls) == 2
|
||||
assert image_urls[0] != image_urls[1], (
|
||||
"Both batch frames produced the same data URL — frame indexing is broken"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# image_to_text_node.execute — status message includes the model name
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_execute_status_includes_model_name_responses_path(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""
|
||||
The status message must embed the actual model name from the connection payload.
|
||||
A mutation hardcoding a model name (e.g. always "vision-model") would pass
|
||||
tests that don't check the name, but fail here when a different model is used.
|
||||
"""
|
||||
image_node = _image_module()
|
||||
fake_client = SimpleNamespace(
|
||||
responses=SimpleNamespace(
|
||||
create=lambda **_: SimpleNamespace(output_text="some result")
|
||||
),
|
||||
chat=SimpleNamespace(
|
||||
completions=SimpleNamespace(
|
||||
create=lambda **_: (_ for _ in ()).throw(AssertionError("no fallback"))
|
||||
)
|
||||
),
|
||||
)
|
||||
monkeypatch.setattr(image_node, "resolve_request_seed", lambda _: 11)
|
||||
monkeypatch.setattr(image_node, "create_openai_client", lambda **_: fake_client)
|
||||
|
||||
output = image_node.LMStudioImageToText.execute(
|
||||
connection=_connection_payload(model="my-custom-vision-model"),
|
||||
image=_single_image(),
|
||||
system_prompt="",
|
||||
user_prompt="describe this",
|
||||
seed=5,
|
||||
)
|
||||
assert "my-custom-vision-model" in output.ui.text, (
|
||||
f"Model name not found in status: {output.ui.text!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_execute_status_includes_model_name_fallback_path(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""
|
||||
The model name must also appear in the status when chat.completions fallback is used.
|
||||
"""
|
||||
image_node = _image_module()
|
||||
completion = SimpleNamespace(
|
||||
choices=[SimpleNamespace(message=SimpleNamespace(content="fallback result"))]
|
||||
)
|
||||
fake_client = SimpleNamespace(
|
||||
responses=SimpleNamespace(
|
||||
create=lambda **_: (_ for _ in ()).throw(RuntimeError("responses off"))
|
||||
),
|
||||
chat=SimpleNamespace(completions=SimpleNamespace(create=lambda **_: completion)),
|
||||
)
|
||||
monkeypatch.setattr(image_node, "resolve_request_seed", lambda _: 22)
|
||||
monkeypatch.setattr(image_node, "create_openai_client", lambda **_: fake_client)
|
||||
|
||||
output = image_node.LMStudioImageToText.execute(
|
||||
connection=_connection_payload(model="fallback-model-xyz"),
|
||||
image=_single_image(),
|
||||
system_prompt="",
|
||||
user_prompt="what is this",
|
||||
seed=0,
|
||||
)
|
||||
assert "fallback-model-xyz" in output.ui.text, (
|
||||
f"Model name not found in fallback status: {output.ui.text!r}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# text_gen_node.execute — status message includes the model name
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _text_connection_payload(model: str = "text-model"):
|
||||
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=model,
|
||||
reasoning_enabled=False,
|
||||
max_tokens=256,
|
||||
temperature=0.7,
|
||||
timeout_seconds=30,
|
||||
use_tooling_mcp=False,
|
||||
)
|
||||
|
||||
|
||||
def test_text_gen_execute_status_includes_model_name_responses_path(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""
|
||||
The status message in LMStudioTextGen.execute must include the actual model name.
|
||||
A mutation hardcoding the model name in the status template would break this.
|
||||
"""
|
||||
text_gen_node = _text_module()
|
||||
fake_client = SimpleNamespace(
|
||||
responses=SimpleNamespace(
|
||||
create=lambda **_: SimpleNamespace(output_text="text output")
|
||||
),
|
||||
chat=SimpleNamespace(
|
||||
completions=SimpleNamespace(
|
||||
create=lambda **_: (_ for _ in ()).throw(AssertionError("no fallback"))
|
||||
)
|
||||
),
|
||||
)
|
||||
monkeypatch.setattr(text_gen_node, "resolve_request_seed", lambda _: 33)
|
||||
monkeypatch.setattr(text_gen_node, "create_openai_client", lambda **_: fake_client)
|
||||
|
||||
output = text_gen_node.LMStudioTextGen.execute(
|
||||
connection=_text_connection_payload(model="unique-text-model-abc"),
|
||||
system_prompt="",
|
||||
user_prompt="generate something",
|
||||
seed=5,
|
||||
)
|
||||
assert "unique-text-model-abc" in output.ui.text, (
|
||||
f"Model name not found in text_gen status: {output.ui.text!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_text_gen_execute_status_includes_model_name_fallback_path(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""
|
||||
The model name must appear in the status even when chat.completions fallback is used.
|
||||
"""
|
||||
text_gen_node = _text_module()
|
||||
completion = SimpleNamespace(
|
||||
choices=[SimpleNamespace(message=SimpleNamespace(content="text fallback answer"))]
|
||||
)
|
||||
fake_client = SimpleNamespace(
|
||||
responses=SimpleNamespace(
|
||||
create=lambda **_: (_ for _ in ()).throw(RuntimeError("no responses"))
|
||||
),
|
||||
chat=SimpleNamespace(completions=SimpleNamespace(create=lambda **_: completion)),
|
||||
)
|
||||
monkeypatch.setattr(text_gen_node, "resolve_request_seed", lambda _: 44)
|
||||
monkeypatch.setattr(text_gen_node, "create_openai_client", lambda **_: fake_client)
|
||||
|
||||
output = text_gen_node.LMStudioTextGen.execute(
|
||||
connection=_text_connection_payload(model="special-fallback-model"),
|
||||
system_prompt="",
|
||||
user_prompt="ask something",
|
||||
seed=0,
|
||||
)
|
||||
assert "special-fallback-model" in output.ui.text, (
|
||||
f"Model name not found in fallback status: {output.ui.text!r}"
|
||||
)
|
||||
@@ -158,3 +158,38 @@ 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_raises_when_all_text_is_think_blocks(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""
|
||||
Regression: when the model returns only <think> content (no visible text),
|
||||
strip_think_content reduces the response to "". The node must raise
|
||||
ValueError rather than silently returning an empty string.
|
||||
"""
|
||||
image_node = _image_module()
|
||||
|
||||
fake_client = SimpleNamespace(
|
||||
responses=SimpleNamespace(
|
||||
create=lambda **_: SimpleNamespace(output_text="<think>internal only</think>")
|
||||
),
|
||||
chat=SimpleNamespace(
|
||||
completions=SimpleNamespace(
|
||||
create=lambda **_: (_ for _ in ()).throw(
|
||||
AssertionError("chat fallback should not run")
|
||||
)
|
||||
)
|
||||
),
|
||||
)
|
||||
monkeypatch.setattr(image_node, "resolve_request_seed", lambda _: 1)
|
||||
monkeypatch.setattr(image_node, "create_openai_client", lambda **_: fake_client)
|
||||
|
||||
with pytest.raises(ValueError, match="All content was inside <think> blocks"):
|
||||
image_node.LMStudioImageToText.execute(
|
||||
connection=_connection_payload(),
|
||||
image=_single_image(),
|
||||
system_prompt="sys",
|
||||
user_prompt="describe",
|
||||
seed=1,
|
||||
)
|
||||
|
||||
@@ -168,3 +168,38 @@ def test_execute_raises_if_both_endpoints_return_no_text(
|
||||
user_prompt="hello",
|
||||
seed=1,
|
||||
)
|
||||
|
||||
|
||||
def test_execute_raises_when_all_text_is_think_blocks(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""
|
||||
Regression: when the model returns only <think> content (no visible text),
|
||||
strip_think_content reduces the response to "". The node must raise
|
||||
ValueError rather than silently returning an empty string.
|
||||
"""
|
||||
text_gen_node = _text_module()
|
||||
|
||||
fake_client = SimpleNamespace(
|
||||
responses=SimpleNamespace(
|
||||
create=lambda **_: SimpleNamespace(output_text="<think>internal only</think>")
|
||||
),
|
||||
chat=SimpleNamespace(
|
||||
completions=SimpleNamespace(
|
||||
create=lambda **_: (_ for _ in ()).throw(
|
||||
AssertionError("chat fallback should not run")
|
||||
)
|
||||
)
|
||||
),
|
||||
)
|
||||
|
||||
monkeypatch.setattr(text_gen_node, "resolve_request_seed", lambda _: 1)
|
||||
monkeypatch.setattr(text_gen_node, "create_openai_client", lambda **_: fake_client)
|
||||
|
||||
with pytest.raises(ValueError, match="All content was inside <think> blocks"):
|
||||
text_gen_node.LMStudioTextGen.execute(
|
||||
connection=_connection_payload(),
|
||||
system_prompt="sys",
|
||||
user_prompt="hello",
|
||||
seed=1,
|
||||
)
|
||||
|
||||
@@ -149,6 +149,10 @@ class LMStudioTextGen(io.ComfyNode):
|
||||
raise ValueError("chat.completions fallback returned no text output")
|
||||
|
||||
text = strip_think_content(text)
|
||||
if not text:
|
||||
raise ValueError(
|
||||
"All content was inside <think> blocks; no visible text was produced."
|
||||
)
|
||||
|
||||
status = f"Model '{connection.model}' via {via_endpoint} (seed={resolved_seed})."
|
||||
if fallback_reason:
|
||||
|
||||
Reference in New Issue
Block a user