Fixes a real gap discovered this session: ChatCompletion/FormatString's
"ui": {"text": [...]} return value was never rendered anywhere — no JS in
this repo ever implemented an onExecuted handler to read it, so the
retry-status banner from the previous commit was invisible in practice.
RandomChoice never returned a ui payload at all (OUTPUT_NODE was False).
- src/js/preview_text.js: read-only STRING preview widget for
ChatCompletion/FormatString/RandomChoice, mirroring ComfyUI core's own
PreviewAny node/JS extension (confirmed via the actual bundled frontend
source, not guessed) rather than inventing a new convention.
- RandomChoice: OUTPUT_NODE=True + a ui.text preview of its arbitrary-typed
output (str/number passthrough, else JSON, else str() — same fallback
chain as core's PreviewAny). IS_CHANGED still returns the raw picked
value, not the new ui-wrapped dict, so change-detection is unaffected.
- Live retry feedback: chat()/chat_structured() gain an on_status
callback, invoked at each retry boundary with a human-readable status
(_llm/retry.py's format_retry_status/format_recovered_status).
ChatCompletion wires this to ComfyUI's own PromptServer.send_progress_text
(the same first-party mechanism core's gaussian-splat/image nodes use)
rather than a hand-rolled custom event — the frontend already renders a
live "progressText" widget for it, so no additional JS was needed for
the live half of this feature.
Verified live against a real Ollama backend (docker-compose.dev.yml +
Playwright): forced a genuine refusal/retry via custom_phrases, confirmed
both the final preview widget and the live progress widget render
correctly on ChatCompletion, and confirmed FormatString/RandomChoice's
preview widgets render too. 410 tests passing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Three follow-ups to the refusal-retry feature:
- Each retry's request timeout now escalates (next_timeout_secs: base *
attempt) across every chat()/chat_structured() retry loop, instead of
reusing the same budget that just ran out.
- LLMProvider.chat()/chat_structured() gain an optional attempt_info
out-param (record_attempt_info), populated with the seed/timeout
actually used and the attempt/refusal counts — backward compatible,
no return-type change.
- ChatCompletion exposes a new seed_used output and prepends a status
line to its existing text preview when a retry/refusal happened, so
both are visible without any frontend/JS changes.
seed_used is appended as the LAST output (after any dynamic
structured-output fields), not inserted after model_name — dynamic
fields must keep starting at their existing index (3) so already-wired
workflows (workflows/ltx-i2v-pipeline.json, and the still-open PR #35
canvas-format file) that link a FormatString input to a ChatCompletion
field by output index aren't silently repointed at the wrong socket.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Some models (observed with an abliterated Qwen variant) answer a request
they judge "politically sensitive" with hedging refusal language instead of
erroring or returning blank — neither of which the existing blank-response
or schema-validation retry triggers catch, so the refusal just passed
through as a normal result.
Adds a hybrid detector to _llm/retry.py: a free lexical/regex pass catches
blatant refusals ("I cannot generate...") without any network call; a
response that's short and/or hedge-y enough to be ambiguous additionally
gets an embedding-similarity check against canonical refusal exemplars, via
a new optional LLMProvider.embed() (Ollama's /api/embed, llama-server's
/v1/embeddings) — deliberately model-agnostic to the backend, since this is
a model-behavior concern, not a backend one. A detected refusal is treated
exactly like a blank response or validation failure: retried with a bumped
seed (existing next_seed()/RETRY_BACKOFF_SECS machinery), never returned to
the caller.
Wired into all four retry loops: OllamaProvider.chat()/chat_structured(),
LlamaCppProvider.chat(), and the shared _llm/chat.py chat_structured() used
by LlamaCppProvider.chat_structured(). New OllamaOptionRefusalRetry node
(same composable OLLAMA_OPTIONS chain as OllamaOptionDisableThinking)
configures it: enabled toggle, optional embedding_model (blank = lexical-
only), similarity threshold.
Claude-Session: https://claude.ai/code/session_01YArD9ZjBWKsAvazmS48amA
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Adds OllamaOptionDisableThinking, a composable node chaining into the
same OLLAMA_OPTIONS socket every other OllamaOption* node uses. Unlike
those (Ollama-native sampling params passed through verbatim), the
"think" key it emits is a comfydv-level convention: every LLMProvider
implementation pops it out of options and translates it to its own wire
shape before building a request, since neither backend recognizes a
literal "think" key nested inside a generic options object.
Confirmed live: Ollama's native /api/chat and OpenAI-compatible
/v1/chat/completions both silently ignore "think" nested in options —
it must be a top-level request field, or the model burns its whole
token budget on chain-of-thought reasoning before ever responding
(eval_count: 223 vs 2 in a direct comparison). llama.cpp's translation
(chat_template_kwargs/reasoning_effort) is sourced from llama-server's
documented request-body fields, not live-verified against a running
instance.
This also fixes two existing live integration tests that already passed
options={"think": False} under the mistaken assumption it worked — it
was a silent no-op until now.
See ADR-010 for the full design discussion, including why this ended up
as a composable option node rather than a new ChatCompletion input.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YArD9ZjBWKsAvazmS48amA
Ollama's OpenAI-compatible endpoint silently reloads the model at its
default context size on every call, discarding any options.num_ctx
override — confirmed live even when the same options are included in
that request. OllamaProvider.chat_structured() now hand-rolls a call to
Ollama's native /api/chat + "format" instead of routing through the
shared pydantic-ai path, since that endpoint correctly preserves and
applies context-size options. LlamaCppProvider keeps the shared path
(llama-server's context is fixed at process launch, not per-request),
switched to pydantic-ai's NativeOutput mode so a thinking-capable model's
reasoning no longer competes with the structured response for token
budget.
Also fixes _build_structured_model typing non-required schema fields as
bare `py_type` with a None default, which only tolerates the field being
omitted — an explicit `null` (which models routinely emit) failed
pydantic validation until the field was typed `py_type | None`.
See ADR-009 for the full investigation and rejected alternatives.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YArD9ZjBWKsAvazmS48amA
Images on the current turn ride on Agent.run user_prompt as BinaryContent;
images on a history user turn ride on its UserPromptPart; text-only calls
stay plain-string. Red: chat.py passes only .content today. Witnesses both
us3_structured_image.feature scenarios.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvS9TFMCFYNHC4MMvzJaZS
beacon-reviewer caught a real gap: when a caller pins options["seed"],
the retry loop set the new seed on ModelSettings' top-level "seed"
field but left the same request's extra_body.options.seed (the
Ollama-native passthrough) at the original pinned value. A backend
that honors the nested field over the top-level OpenAI one would keep
sending the identical effective seed on every retry, silently
defeating this fix for exactly the pinned-seed case it needs to cover.
Now both fields are kept in sync on every retry attempt, built via
fresh copies rather than mutating the caller's options dict in place
(that dict is shared across every attempt, and possibly across other
calls). Added a regression test asserting the caller's options dict is
untouched after a multi-attempt retry.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
Reported live on a fresh runpod: structured_output extraction on
ChatCompletion would fail with
RuntimeError: chat_structured: response failed validation against
schema after 3 attempt(s) ... Last error: Exceeded maximum output
retries (0)
...but succeed if a plain (non-structured) chat call with the same
prompt was run first. Root cause: pydantic-ai's Agent is built with
retries=0 (comfydv drives its own outer retry loop, not pydantic-ai's
internal one), so a single bad/empty tool-call response instantly
exhausts one attempt. comfydv's own retry loop then retried with the
*exact same request* every time — if the model is genuinely stuck
(e.g. still warming up on a freshly-started backend) or produces a
degenerate response for a given seed, every attempt fails identically.
Confirmed the seed was unset in the reporting user's workflow (Ollama
picks its own per call) and it still failed 3/3 in a row, which rules
out "stuck on one bad seed" and points at request-level determinism
plus zero backoff between attempts as the real gap.
Plain chat() had zero retry/validation logic at all before this change
— it silently returned whatever content came back, even blank. Given
the underlying failure mode (a fresh model's first response sometimes
being blank) is identical for both chat modes, this fix applies to
both, not just structured output.
Adds src/comfydv/_llm/retry.py: next_seed() (deterministic, starting
from any user-pinned options["seed"], incrementing per retry — attempt
1 is always the caller's original, untouched request) and
RETRY_BACKOFF_SECS (a flat delay between retries, giving a still-
loading model time to finish rather than hammering it with identical
requests back-to-back).
Both OllamaProvider.chat() and LlamaCppProvider.chat() now retry (per
the node's existing max_retries widget) when a response comes back
blank, injecting the incremented seed each retry — Ollama via the
native options.seed field, llama.cpp via the OpenAI spec's top-level
"seed" field (nesting it under "options" like the rest of the
Ollama-native passthrough would silently not work — llama-server's
OpenAI-compatible endpoint doesn't read seed from there, a pre-existing,
documented non-goal for general options passthrough that doesn't apply
to this internal retry mechanism). Neither raises if every retry stays
blank — matches chat()'s existing never-validates contract.
chat_structured() in _llm/chat.py now injects the same incrementing
seed via pydantic-ai's ModelSettings["seed"] (which correctly maps to
the OpenAI API's top-level seed param for both backends) and adds the
same backoff between attempts, on top of its existing validation-retry
loop.
Live-verified against real Ollama: the happy path (model responds
normally first try) still makes exactly one network call for chat()
and completes chat_structured() normally — no regression from the new
retry scaffolding. The specific cold/stuck-model failure itself is
runpod-cold-start-dependent and wasn't independently reproducible in
this session's environment (a second live attempt succeeded on the
very first try) — unit tests directly exercise the retry/seed/backoff
logic instead (test_llm_retry.py, and new cases in
test_ollama_provider.py, test_llamacpp_provider.py, and
test_llm_chat_structured.py), verified against both providers.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
chat_structured() silently dropped the options dict (temperature, seed,
num_predict, repeat_penalty, etc. — set via the OllamaOption* nodes) in
structured_output mode: OllamaProvider.chat_structured() only used it for
the cache key, and the shared comfydv._llm.chat.chat_structured() helper
had no options parameter at all. On main, the hand-rolled implementation
sent options verbatim in the request body; this migration silently lost it,
contradicting FR-008 (structured-output behavior must be unchanged).
Fixed by forwarding options as pydantic-ai's model_settings.extra_body,
matching the original payload shape exactly rather than lossily remapping
onto ModelSettings' own standardized field names (which don't cover
Ollama-native params like num_predict/repeat_penalty/top_k anyway).
Verified against the live local Ollama server: structured output with
temperature=0.0/seed=42 succeeds on first attempt.
Caught by an independent beacon-reviewer pass on the diff before opening a
PR — exactly what that review step is for.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
Provider-layer only — src/comfydv/_llm/chat.py doesn't touch ollama.py, so
it's safe to land ahead of the deferred atomic node cutover (issue #16).
Ports ADR-006's exact retry/validation contract: bounded retries (0-5,
clamped), RuntimeError naming model/attempt-count/truncated-last-response
on exhaustion. Agent's own internal retries disabled so the error contract
is comfydv's, not pydantic-ai's.
Required an API-discovery spike (Agent/OpenAIProvider/OpenAIChatModel
constructors and exception surface aren't reliably knowable from training
data for a fast-moving library) before tests could be written meaningfully
— tests and implementation were validated together rather than strictly
red-first; noted in tasks.md rather than presented as pure TDD.
Also fixes a pre-existing gap in test_packaging.py's requirements.txt/
pyproject.toml consistency check: it stripped `[extras]` on one side of the
comparison but not the other, which this change's pydantic-ai-slim[openai]
entry exposed as the first dependency with an extras marker.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
