Compare commits

..
Author SHA1 Message Date
Gerald LonlasandClaude Opus 4.7 6f9867a3ef fix(image-to-text): log warning when chat.completions fallback drops batch frames
When the chat.completions fallback path runs on a batched image input, only
the first frame is sent. Add a logging.warning so ComfyUI server logs surface
the dropped frame count even if the UI status string is ignored.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-19 00:45:02 +08:00
9 changed files with 47 additions and 436 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)
+8 -4
View File
@@ -1,5 +1,6 @@
from __future__ import annotations
import logging
from typing import Any
from comfy_api.latest import io, ui
@@ -16,6 +17,8 @@ from .client import (
from .iotypes import ParamConnection
from .models import LMStudioConnectionPayload
_logger = logging.getLogger(__name__)
class LMStudioImageToText(io.ComfyNode):
@classmethod
@@ -181,16 +184,17 @@ class LMStudioImageToText(io.ComfyNode):
raise ValueError("chat.completions fallback returned no text output")
if batch_size > 1:
_logger.warning(
"LMStudio image_to_text chat.completions fallback only supports "
"one image; %d additional frame(s) were dropped.",
batch_size - 1,
)
fallback_reason += (
f" Note: chat.completions fallback only supports one image; "
f"{batch_size - 1} additional frame(s) were dropped."
)
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:
-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)
+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
-35
View File
@@ -158,38 +158,3 @@ 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,
)
-35
View File
@@ -168,38 +168,3 @@ 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,
)
-4
View File
@@ -149,10 +149,6 @@ 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: