Compare commits

...
Author SHA1 Message Date
Gerald LonlasandClaude Opus 4.8 cd3f58917f test(contracts): add behavioral-contract tests that catch value-mutant regressions
25 new tests across client.py, routes.py, connect_node.py, and text_gen_node.py.
Each test targets a specific behavioral contract that 100% line+branch coverage
did not verify and that a realistic mutation would silently break:

- normalize_server_url: whitespace-only input, trailing-slash stripping,
  https acceptance
- coerce_seed: whitespace-only string returns None (not ValueError)
- resolve_request_seed: seed=0 passes through; large values not clipped
- build_responses_input_text: 'type' must be 'input_text' (not 'text')
- build_chat_messages: whitespace system prompt omitted; empty user included
- list_models: result is sorted AND deduplicated (both invariants checked)
- strip_think_content: surrounding text order preserved; block-only → ""
- _query_int: negative integers rejected; empty string uses default
- _models_handler: empty models → default_model="" not IndexError;
  error prefix is 'Failed to list models' not 'Connection test failed'
- _test_handler: model_count matches actual list length; distinct error prefix
- connect_node execute: empty model with test_connectivity=False still probes;
  no-probe status omits 'Found N model(s)'; probe status includes exact count
- _responses_kwargs: 'input' key present with user-role and correct prompt text;
  temperature/max_tokens forwarded from payload
- text_gen_node execute: fallback path status includes resolved seed;
  user_prompt=None raises clean ValueError

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-27 01:19:40 +08:00
Gérald LONLASandClaude Opus 4.8 0d05296bc0 fix(client): strip /v1 suffix case-insensitively in normalize_server_url (#12)
`normalize_server_url` used a case-sensitive `endswith("/v1")` check, so
user-supplied URLs like `https://host/V1` passed through intact. The
downstream `to_openai_base_url` then appended another `/v1`, producing the
double-suffix broken URL `https://host/V1/v1`, causing every API call to
fail with a 404.

Fix: compare against `value.lower()` so any casing of the suffix is stripped.
Adds a targeted regression test covering lowercase, uppercase, and mixed-case
inputs as well as the end-to-end `to_openai_base_url` output.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-27 01:14:19 +08:00
Gérald LONLASandClaude Opus 4.8 25d7309fa7 test(client): cover all defensive branches in extract_responses_text (#14)
Add four targeted tests hitting the previously uncovered branches in
`client.extract_responses_text` (lines 221–233): dict items with a
non-list content field, content list entries that are not dicts, dict
entries whose text value is not a str, and object items where
`getattr(entry, "text")` is non-str.

Also declare `aiohttp` in the optional [test] extra so the full suite
can import `routes.py` without a ModuleNotFoundError.

Branch coverage: 99% (4 branches missed) → 100% (146/146 branches).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-27 01:13:45 +08:00
Gérald LONLASandClaude Opus 4.8 fa252dbac6 chore(tests): remove unused _batch_image helper from test_coverage_gaps.py (#13)
_batch_image() was defined in tests/test_coverage_gaps.py but never called
in that file — an identical helper already exists in test_image_to_text_node.py
where it is actively used. Dead code with no callers; safe to remove.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-27 01:13:41 +08:00
Gérald LONLASandClaude Opus 4.8 0e42058f20 chore: remove unused dump_openai_response and dead test helpers (#10)
- Delete client.dump_openai_response (and its import json): never called
  from any production path (connect_node, text_gen_node, image_to_text_node,
  routes, __init__); existed only as a debugging aid.
- Delete its test (test_dump_openai_response_branches in test_client_full.py)
  and the now-orphaned `import json` in that file.
- Delete make_response_with_output_text / make_response_with_output_items
  from tests/helpers/fakes.py: defined in the initial test scaffolding
  commit but never imported or referenced by any test module.

All 68 tests pass; branch coverage holds at 96.93 % (> 90 % required).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-27 01:04:33 +08:00
Gérald LONLASandClaude Opus 4.8 9d8e262304 test(coverage): close remaining branch gaps in client and image node (#9)
Add 12 focused unit tests targeting uncovered branches identified by
pytest-cov on main (97.01% → 99.30%):

client.py
- coerce_seed: .item() raises at runtime → falls through to unsupported-type error (lines 117-118, 142)
- coerce_seed: plain unsupported types (dict, object) raise ValueError (line 142)
- extract_chat_completion_text: non-str/non-list content returns "" (line 207)
- extract_chat_completion_text: list items with no usable text are skipped (lines 222-228)
- extract_responses_text: output list with unresolvable items returns "" (line 234)
- extract_responses_text: output=None returns "" (end of function)
- build_chat_messages_with_image: blank/None system_prompt omits the system message (line 291)

image_to_text_node.py
- _responses_kwargs: empty/whitespace system_prompt omits "instructions" key (line 112)
- execute: responses endpoint returns empty string → falls back to chat.completions (line 157)
- execute: both endpoints return empty text → ValueError propagates (line 181)
- execute: single-image chat fallback status does not mention dropped frames (line 183)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-27 01:00:30 +08:00
Gérald LONLASandClaude Opus 4.8 73ed6d7f38 fix(client): avoid duplicate text when output item has both content and text (#11)
In `extract_responses_text`, the loop over non-dict output items extracted
text from `item.content[].text` AND unconditionally also from `item.text`.
When an item carried both attributes (e.g. an OpenAI Responses API message
object that exposes `.content` as a list and also a convenience `.text`
shorthand), the same content was appended twice, producing a response like
`"the-answer\nthe-answer"` instead of `"the-answer"`.

The dict-item branch already used `continue` to prevent this; mirror that
guard in the object branch by making `item.text` extraction an `else` clause
that only runs when `item.content` is NOT a list.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-27 01:00:00 +08:00
6 changed files with 903 additions and 39 deletions
+6 -16
View File
@@ -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
@@ -233,9 +232,10 @@ def extract_responses_text(response: Any) -> str:
text = getattr(entry, "text", None)
if isinstance(text, str):
parts.append(text)
text = getattr(item, "text", None)
if isinstance(text, str):
parts.append(text)
else:
text = getattr(item, "text", None)
if isinstance(text, str):
parts.append(text)
if parts:
return "\n".join(parts)
@@ -243,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)
+1
View File
@@ -13,6 +13,7 @@ dependencies = [
test = [
"pytest>=8.0.0",
"pytest-cov>=5.0.0",
"aiohttp>=3.9.0",
]
[tool.comfy]
-8
View File
@@ -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)
+545
View File
@@ -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,
)
+51 -15
View File
@@ -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"
@@ -183,23 +194,48 @@ def test_extract_responses_text_variants() -> None:
assert client.extract_responses_text(ns(output_text="", output=[])) == ""
def test_dump_openai_response_branches() -> None:
class WithModelDump:
def model_dump(self):
return {"a": 1}
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"
)
class WithToDict:
def to_dict(self):
return {"b": 2}
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_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"
def test_strip_think_content_case_insensitive_and_multiple() -> None:
+300
View File
@@ -0,0 +1,300 @@
"""
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