Merge pull request #31 from darth-veitcher/fix/ollama-structured-output-native-format

fix(ollama): route structured output through native /api/chat, fix nullable field validation

CHANGELOG.md

@@ -15,6 +15,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Changed
 - **Breaking:** `OllamaChatCompletion` → `ChatCompletion`, `OllamaModelSelector` → `LLMModelSelector`, `OllamaLoadModel` → `LLMLoadModel`, `OllamaUnloadModel` → `LLMUnloadModel`, and the `OLLAMA_CLIENT` socket type → `LLM_CLIENT` — these nodes are now backend-generic. `OllamaClient` is unchanged by name but now outputs an `OllamaProvider` rather than a plain string; existing saved workflows using the old node/socket names need reconnecting (see `comfydv.ollama.MIGRATION_MAP` for the full old→new mapping).
+- `ChatCompletion`'s `structured_output=True` path now routes Ollama through Ollama's native `/api/chat` + `"format"` instead of the shared `pydantic-ai` OpenAI-compat path — Ollama's OpenAI-compatible endpoint was found to silently reload the model at its default context size on every call, discarding any `options` (e.g. `num_ctx`) override. `LlamaCppProvider` is unaffected and keeps the shared path, switched to `pydantic-ai`'s `NativeOutput` mode (ADR-009).
+
+### Fixed
+- `structured_output=True` requests could fail validation ("token limit exceeded before any response was generated") against "thinking"-capable models, which spent their whole token budget on chain-of-thought reasoning before ever producing the structured response (ADR-009).
+- A non-required structured-output schema field rejected an explicit `null` value from the model (only an *omitted* field was tolerated), even though models routinely emit explicit `null` for absent optional fields.
 
 ## [0.1.0] — 2026-06-01
 

project-management/ADRs/ADR-009-native-structured-output-mode.md

@@ -0,0 +1,82 @@
+# ADR-009: Provider-specific structured output — NativeOutput for llama.cpp, hand-rolled native `/api/chat` for Ollama
+
+## Status
+
+> Accepted
+
+_Date:_ 2026-07-25
+_Deciders:_ darth-veitcher
+
+---
+
+## Context
+
+`chat_structured()` (`src/comfydv/_llm/chat.py`, ADR-007) builds its `pydantic-ai` `Agent` with a bare `output_type=schema`. Passing a raw `pydantic.BaseModel` subclass this way makes `pydantic-ai` default to **tool-calling** (a synthetic forced function call) for structured output — inherited silently from ADR-007's move to `pydantic-ai`, never a deliberate re-decision. ADR-007 doesn't discuss output-mode choice at all.
+
+Testing a real multi-agent ComfyUI workflow (`workflows/ltx-i2v-pipeline.json`) against a live local Ollama server surfaced this as a real reliability problem: against a "thinking"-capable model (`qwen3.5:9b`), tool-calling failed consistently — the model spent its entire token budget on internal chain-of-thought reasoning and never emitted the tool call, so every attempt failed pydantic validation ("token limit exceeded before any response was generated"), each attempt taking 5-7+ minutes before giving up.
+
+### First fix attempt: `NativeOutput` + a priming call (superseded within this same ADR)
+
+`pydantic-ai` 2.9.0 exposes `NativeOutput`, which makes the `Agent` use `response_format: {"type": "json_schema", ...}` over the OpenAI-compatible endpoint instead of tool-calling. Live-tested directly against Ollama via `curl` before touching code: `/v1/chat/completions` with `response_format: json_schema` returned clean, schema-valid JSON, with the model's reasoning in a separate `message.reasoning` field — fast, and reasoning no longer competed with structured output for token budget. This part of the fix is real and is kept — see Decision §1.
+
+A second, separate problem was also found: Ollama's OpenAI-compatible endpoint doesn't honor a per-request `options` override (e.g. `num_ctx`) — sending the same request to native `/api/chat` reloaded the model at the requested context size; `/v1/chat/completions` silently kept whatever was already loaded. The first fix attempt worked around this with a priming call: hit native `/api/generate` with the desired `options` immediately before the real `/v1/chat/completions` request, on the theory that Ollama would keep the just-loaded context for the next call.
+
+**This did not work, and the failure mode looked exactly like the original bug** — confirmed while re-testing the actual workflow end-to-end (`workflows/ltx-i2v-pipeline.json`, Agent 2 "Scene Grounder": long system prompt + 9-property schema + image), which kept failing with the identical "token limit exceeded" error even after the priming fix landed, tests passed, and `max_tokens`/`num_ctx`/`timeout_secs` were all raised generously. Isolated the exact mechanism with a direct, non-ComfyUI-mediated `curl` sequence:
+
+1. `POST /api/generate` with `options: {num_ctx: 20480}`, `keep_alive: -1` → confirmed via `GET /api/ps`: `context_length: 20480`, loaded "forever".
+2. Immediately `POST /v1/chat/completions` for the same model — **even with the identical `options: {num_ctx: 20480}` included in that request's body** → `GET /api/ps` immediately after: `context_length: 4096` (back to default), `expires_at` reset to a normal ~5-minute keep-alive.
+
+So `/v1/chat/completions` doesn't merely *ignore* `options.num_ctx` — every call to it silently **reloads the model at the default context size**, discarding whatever was primed, regardless of what that same call's own `options` field says. A priming call immediately before the real request is structurally incapable of working, because the real request itself is what undoes the priming.
+
+Re-ran the same sequence against native `/api/chat` instead of `/v1/chat/completions`: the primed `context_length: 20480` was preserved through and after the call. Native `/api/chat` also accepts `"format": <json schema>` directly, giving grammar-constrained structured output in the same request that correctly honors `options` — no separate priming call needed at all.
+
+### Re-litigating ADR-006's model concern
+
+This also revisits [ADR-006](ADR-006-structured-ollama-output-tool-calling-not-pydantic-ai.md), which rejected native `format`-based output — but its rejection was scoped to one specific model, `lukey03/qwen3.5-9b-abliterated-vision`, whose degenerate chat template silently ignored the constraint. ADR-006 explicitly flagged this as revisitable: *"If well-behaved-model testing later shows native `format` is meaningfully more reliable in the common case, this decision should be revisited rather than treated as permanent."* Re-tested that exact model against native structured output: it no longer silently ignores the constraint (ADR-006's specific failure mode) — it returns schema-valid JSON, but the *content* is still garbled (`"ponáp∵49\n"` instead of the requested `"pong"`), consistent with ADR-006's "degenerate tokenizer" diagnosis. That model was also already failing under the tool-calling path (hanging without completing, observed live during this same investigation). So neither part of this decision regresses that model — it was already unusable for structured output either way. `structured_output` has no production users yet, so there is no back-compat concern in making this change.
+
+## Decision
+
+**1. `LlamaCppProvider` keeps the shared `pydantic-ai` path, switched to `NativeOutput`.** `chat.py`'s `_build_agent()` builds `Agent(chat_model, output_type=NativeOutput(schema), retries=0)` instead of a bare `output_type=schema`. llama-server's OpenAI-compatible endpoint is its genuine native structured-output surface (no equivalent context-reload bug found or expected — llama-server's context is fixed at process launch via `--ctx-size`, not a per-request concern, so there's nothing for a request to silently reset), so the shared-implementation architecture from ADR-007 stays intact for this provider.
+
+**2. `OllamaProvider.chat_structured()` no longer uses `chat.py` at all.** It hand-rolls its own call to Ollama's **native** `/api/chat` with a `"format"` JSON schema, mirroring the request-building and retry/validation contract `chat.py` established (bounded retries 0–5, `RuntimeError` naming the model/attempt-count/truncated-response on exhaustion) but without pydantic-ai in the loop for this provider — there is no bare-metal native-JSON-schema mode in pydantic-ai's OpenAI-compatible model class to point at Ollama's native (non-OpenAI-shaped) endpoint, so this is a direct `_post_json` call, parsed with `schema.model_validate_json(...)`, retried on `pydantic.ValidationError` (which pydantic v2 also raises for malformed JSON, not just schema mismatches). `options` (from `OllamaOption*` nodes) is included directly in this same request's `"options"` field and is correctly honored, since it's the native endpoint — no separate priming call, because none is needed: structured output and context sizing now apply atomically in one request.
+
+The retry/validation contract itself (bounded retries, `RuntimeError` naming the model/attempt-count/truncated-response on exhaustion) is unchanged and now implemented twice — once in `chat.py` for llama.cpp, once directly in `ollama_provider.py` for Ollama — rather than shared, which is the real cost of this decision (see Consequences).
+
+## Consequences
+
+**Easier:**
+- Structured output is now reliable against "thinking"-capable models on both providers — reasoning and structured content are separate response fields (`message.reasoning`/`message.thinking` vs `message.content`) under both `NativeOutput` and Ollama's native `format`, rather than competing for the same token stream under tool-calling.
+- `num_ctx` and other Ollama-native options now actually apply to structured-output requests — genuinely fixed this time, confirmed by re-running the actual failing workflow agent, not just by a passing test suite (the first fix attempt passed every test and still didn't work end-to-end).
+- Meaningfully faster in the success case than tool-calling against a thinking model.
+- Closes ADR-006's own explicit "revisit later" flag with concrete evidence rather than leaving it open indefinitely.
+
+**Harder / constrained:**
+- `OllamaProvider` and `LlamaCppProvider` now have two independent structured-output implementations instead of one shared one — ADR-007's "share one implementation" goal no longer holds for this piece. A future structured-output feature (e.g. plumbing reasoning content back to the caller) needs to land in both places.
+- Structured output guarantees schema-*shape* validity, not semantic correctness, on both providers now — a genuinely broken model (degenerate tokenizer, as with the abliterated test model) can still return valid-JSON garbage instead of raising a clear error. Downstream consumers should not treat "returned without error" as "returned correct content" for low-quality/unreliable models.
+- Ollama's native `/api/chat` endpoint's `"format"` field is only checked against top-level `type`/`properties`/`required` the same way the OpenAI-compat `response_format` was — no change to `_build_structured_model`'s shallow-schema behavior in `ollama.py`.
+
+**Debt introduced:**
+- Two structured-output code paths (per provider) instead of one shared one, as noted above — accepted because the two providers' actual constraints (Ollama's context-reset-per-OpenAI-compat-call bug vs. llama-server's fixed-at-launch context) are genuinely different, not incidentally different.
+- Not addressed here (flagged for a future ADR if pursued): a model's reasoning/thinking content is available (`message.reasoning` natively for Ollama, parsed into pydantic-ai's `ThinkingPart` for llama.cpp) but discarded by both `chat_structured()` implementations, which only return the validated schema instance. Plumbing this back to `ChatCompletion` as a node output would need a `LLMProvider.chat_structured()` return-type change — a `Protocol`-level change affecting both providers, out of scope here.
+- Not addressed here (pre-existing, unrelated): `LlamaCppProvider.chat()`'s own code comments already note that `OllamaOption*` nodes emit Ollama-native option names llama-server's OpenAI-compatible endpoint doesn't recognize — an accepted gap from the llama.cpp integration epic, unrelated to this decision.
+
+## Considered Alternatives
+
+### Alternative A: `NativeOutput` + priming call for both providers (the first fix attempt)
+
+**Why rejected:** This is what ADR-009 originally shipped as. It passed every test (including a new one added specifically for the priming call) and one successful live single-agent ComfyUI run, but failed to actually fix the real workflow — confirmed by re-running the full pipeline and hitting the identical original failure on a later, more complex agent. Root-caused only after that: `/v1/chat/completions` unconditionally reloads the model at default context on *every* call, so priming immediately before the real call is undone by the real call itself. No amount of retrying, raising `max_tokens`, or raising `timeout_secs` fixes a context-size problem that the request itself keeps resetting.
+
+### Alternative B: Fully switch both providers off `pydantic-ai`, hand-roll native structured output everywhere
+
+**Why rejected:** Unnecessary for `LlamaCppProvider` — no evidence llama-server's OpenAI-compatible endpoint has Ollama's context-reset behavior (its context is fixed at process launch regardless of request), so `NativeOutput` over the existing shared path is strictly simpler there and keeps ADR-007's sharing goal intact for at least one provider.
+
+### Alternative C: Do nothing, document Ollama's context-reset behavior as a known limitation
+
+**Why rejected:** The underlying failure mode (indefinite-looking hangs, or outright failures, against any Ollama model needing more than the default 4096-token context while using structured output) is common enough — any long system prompt plus a non-trivial schema hits it — that documenting around it would leave `structured_output=True` effectively broken for Ollama in exactly the cases where structured output is most useful (complex, multi-field extraction tasks).
+
+---
+
+## Links
+
+- Related ADRs: [ADR-006](ADR-006-structured-ollama-output-tool-calling-not-pydantic-ai.md) (superseded rationale, not superseded status — ADR-006's tool-calling-vs-native evidence and reasoning stand as historical record; this ADR only revisits its "revisit later" flag), [ADR-007](ADR-007-llm-provider-adapter-pattern.md) (provider abstraction this decision partially steps outside of, for Ollama only)
+- Discovered while building/testing: `workflows/ltx-i2v-pipeline.json`

src/comfydv/_llm/chat.py

@@ -1,9 +1,23 @@
 """Shared chat_structured() helper — pydantic-ai backed structured output.
 
-Used by every ``LLMProvider`` implementation's ``chat_structured()`` method
-(ADR-007) so Ollama and llama.cpp share one implementation of tool-calling/
-structured-output logic instead of each hand-rolling it, since both speak
-OpenAI-compatible ``/v1/chat/completions``.
+Used by ``LlamaCppProvider.chat_structured()`` (ADR-007) over llama-server's
+OpenAI-compatible ``/v1/chat/completions``. ``OllamaProvider`` no longer uses
+this module (ADR-009): Ollama's OpenAI-compatible endpoint was found to
+silently reload the model at its default context size on every call,
+discarding any ``options.num_ctx`` override even when included in that same
+request — a behavior specific to Ollama's compat layer, not llama-server's.
+``OllamaProvider.chat_structured()`` now hand-rolls its own structured-output
+call over Ollama's *native* ``/api/chat`` + ``"format"``, which doesn't have
+that problem.
+
+ADR-009: the Agent uses ``NativeOutput`` (``response_format``/JSON-schema
+constrained decoding), not pydantic-ai's default tool-calling. Live-tested
+against a "thinking"-capable model: tool-calling let the model spend its
+whole token budget on chain-of-thought reasoning and never emit the tool
+call; native output keeps reasoning in a separate response field and the
+constrained ``content`` always comes back as schema-valid JSON. This benefit
+still applies to llama.cpp, which is why this module (and its NativeOutput
+choice) is kept for that provider.
 
 Ports ADR-006's retry/validation contract exactly: bounded retries (0-5,
 clamped), and a ``RuntimeError`` naming the model, attempt count, and a
@@ -17,7 +31,7 @@ import asyncio
 from typing import cast
 
 from pydantic import BaseModel, ValidationError
-from pydantic_ai import Agent
+from pydantic_ai import Agent, NativeOutput
 from pydantic_ai.exceptions import ModelRetry, UnexpectedModelBehavior
 from pydantic_ai.messages import (
     BinaryContent,
@@ -58,7 +72,7 @@ def _build_agent(
         base_url=base_url, api_key="not-needed", http_client=http_client
     )
     chat_model = OpenAIChatModel(model, provider=provider)
-    return Agent(chat_model, output_type=schema, retries=0)
+    return Agent(chat_model, output_type=NativeOutput(schema), retries=0)
 
 
 def _user_prompt_content(msg: Message):

src/comfydv/_llm/ollama_provider.py

@@ -16,7 +16,7 @@ import logging
 import threading
 import time
 
-from pydantic import BaseModel
+from pydantic import BaseModel, ValidationError
 
 from .provider import Message, ModelInfo, ModelStatus
 from .retry import RETRY_BACKOFF_SECS, next_seed
@@ -404,12 +404,29 @@ class OllamaProvider:
         timeout_secs: float = 300.0,
         max_retries: int = 2,
     ) -> BaseModel:
+        """Native ``/api/chat`` + ``"format"`` (grammar-constrained JSON
+        decoding), not the shared pydantic-ai ``chat.py`` helper.
+
+        ADR-009 originally routed this through the OpenAI-compatible
+        ``/v1/chat/completions`` endpoint via pydantic-ai's ``NativeOutput``.
+        Confirmed live that endpoint silently *reloads the model at its
+        default context size on every call*, discarding any prior
+        ``options.num_ctx`` — even when the same ``options`` are included in
+        that very request. Priming with a separate native call first
+        (the original fix) didn't help: the very next OpenAI-compat call
+        undid it immediately. The native ``/api/chat`` endpoint doesn't
+        have this problem — confirmed live it preserves an already-primed
+        context, and it supports structured output directly via
+        ``"format"``, so ``options`` and structured output now apply
+        atomically in one request. ``LlamaCppProvider`` is unaffected — it
+        keeps using the shared pydantic-ai path, since llama-server's
+        context is fixed at process launch, not a per-request concern.
+        """
         if any(m.images for m in messages):
             await _require_vision_capability(self.host, model, self.headers)
 
-        from .chat import chat_structured as _chat_structured_impl
-
-        payload_messages = [m.model_dump() for m in messages]
+        payload_messages = [m.model_dump(exclude_none=True) for m in messages]
+        json_schema = schema.model_json_schema()
         cache_key = _cache_key(
             "chat_structured",
             self.host,
@@ -417,21 +434,59 @@ class OllamaProvider:
             model,
             payload_messages,
             options or {},
-            schema.model_json_schema(),
+            json_schema,
         )
         cached, hit = _CHAT_RESPONSE_CACHE.get(cache_key)
         if hit:
             return schema.model_validate(cached)
 
-        result = await _chat_structured_impl(
-            base_url=f"{self.host}/v1",
-            model=model,
-            messages=messages,
-            schema=schema,
-            headers=self.headers,
-            options=options,
-            max_retries=max_retries,
-            timeout_secs=timeout_secs,
+        total_attempts = max(0, min(int(max_retries), 5)) + 1
+        last_error: Exception | None = None
+        last_invalid_text = ""
+
+        for attempt in range(1, total_attempts + 1):
+            attempt_options = dict(options) if options else {}
+            if attempt > 1:
+                attempt_options["seed"] = next_seed(options, attempt)
+
+            payload: dict = {
+                "model": model,
+                "messages": payload_messages,
+                "format": json_schema,
+                "stream": False,
+            }
+            if attempt_options:
+                payload["options"] = attempt_options
+
+            try:
+                result = await _post_json(
+                    f"{self.host}/api/chat",
+                    payload,
+                    timeout=timeout_secs,
+                    headers=self.headers,
+                )
+            except RuntimeError as exc:
+                last_error = exc
+                last_invalid_text = str(exc)
+                if attempt < total_attempts:
+                    await asyncio.sleep(RETRY_BACKOFF_SECS)
+                continue
+
+            content = result.get("message", {}).get("content", "")
+            try:
+                parsed = schema.model_validate_json(content)
+            except ValidationError as exc:
+                last_error = exc
+                last_invalid_text = content
+                if attempt < total_attempts:
+                    await asyncio.sleep(RETRY_BACKOFF_SECS)
+                continue
+
+            _CHAT_RESPONSE_CACHE.set(cache_key, parsed.model_dump())
+            return parsed
+
+        raise RuntimeError(
+            f"chat_structured: response failed validation against schema after "
+            f"{total_attempts} attempt(s) (model={model!r}). Last error: "
+            f"{last_error}. Last response (truncated): {last_invalid_text[:300]!r}"
         )
-        _CHAT_RESPONSE_CACHE.set(cache_key, result.model_dump())
-        return result

src/comfydv/ollama.py

@@ -12,8 +12,11 @@ LLMProvider adapter-pattern boundary shared with future backends. Chat,
 model listing, and load/unload nodes are generic (ChatCompletion,
 LLMModelSelector, LLMLoadModel, LLMUnloadModel) and delegate to whichever
 provider is wired in — see MIGRATION_MAP below for the old Ollama-specific
-names these replace. Structured output is now pydantic-ai backed
-(comfydv._llm.chat), superseding ADR-006's hand-rolled tool-calling.
+names these replace. Structured output mechanism is provider-specific as of
+ADR-009: LlamaCppProvider is pydantic-ai backed (comfydv._llm.chat, native
+JSON-schema output mode); OllamaProvider hand-rolls native Ollama
+``/api/chat`` + ``"format"`` directly, since Ollama's OpenAI-compatible
+endpoint was found to silently discard per-request context-size options.
 """
 
 import json
@@ -390,9 +393,10 @@ def _history_preview(messages: list[dict]) -> str:
 # ---------------------------------------------------------------------------
 # Structured output schema helpers — stay here (pure/local, no network
 # dependency), shared by ChatCompletion.chat() and the live-preview route.
-# The actual structured-output *mechanism* (tool-calling, retry, validation)
-# moved to comfydv._llm.chat / pydantic-ai as of ADR-007, superseding
-# ADR-006's hand-rolled approach.
+# The actual structured-output *mechanism* (retry, validation, request
+# shape) lives per-provider — comfydv._llm.chat (pydantic-ai) for
+# LlamaCppProvider, native Ollama /api/chat + "format" for OllamaProvider
+# (ADR-009, superseding ADR-007's shared-pydantic-ai-for-both approach).
 
 _JSON_SCHEMA_TO_PY_TYPE: dict = {
     "string": str,
@@ -454,6 +458,13 @@ def _build_structured_model(schema: dict):
     exactly the "blank output" problem this feature exists to eliminate. An
     empty required string now fails validation and triggers a retry like any
     other malformed response, rather than silently passing through.
+
+    Non-required fields are typed ``py_type | None``, not bare ``py_type``:
+    a bare type with a ``None`` default only covers the field being *omitted*
+    entirely — pydantic still rejects an explicitly present ``null`` value
+    against a non-Optional type. Models routinely emit explicit ``null``
+    (e.g. `"duration_seconds": null`) for genuinely-absent optional fields
+    rather than omitting the key, so the type must accept that.
     """
     from pydantic import Field, create_model
 
@@ -462,7 +473,7 @@ def _build_structured_model(schema: dict):
     for name, prop in schema["properties"].items():
         py_type = _JSON_SCHEMA_TO_PY_TYPE.get(prop.get("type"), str)
         if name not in required:
-            fields[name] = (py_type, None)
+            fields[name] = (py_type | None, None)
         elif py_type is str:
             fields[name] = (py_type, Field(..., min_length=1))
         else:

tests/test_llm_chat_structured.py

@@ -87,6 +87,42 @@ def test_chat_structured_returns_validated_output(monkeypatch):
     assert fake.calls[0][0] == "describe a widget"
 
 
+def test_build_agent_uses_native_output_not_tool_calling(monkeypatch):
+    """ADR-009: the Agent must be built with NativeOutput (response_format /
+    JSON-schema constrained decoding), not pydantic-ai's tool-calling
+    default. Regression guard against reverting to a bare `output_type=schema`,
+    which let a thinking-capable model exhaust its token budget on reasoning
+    and never emit a tool call (confirmed live against a real Ollama server).
+
+    Spies on the real pydantic_ai.Agent constructor (only _build_agent's own
+    seam is mocked in every other test in this file) and asserts on its
+    output_type argument via the public NativeOutput marker class, rather
+    than pydantic-ai's private internal schema representation."""
+    from pydantic_ai import NativeOutput
+
+    captured = {}
+    real_agent_cls = chat_mod.Agent
+
+    class _SpyAgent(real_agent_cls):
+        def __init__(self, *args, **kwargs):
+            captured.update(kwargs)
+            super().__init__(*args, **kwargs)
+
+    monkeypatch.setattr(chat_mod, "Agent", _SpyAgent)
+
+    chat_mod._build_agent(
+        base_url="http://localhost:11434/v1",
+        model="llama3",
+        schema=_Widget,
+        headers=None,
+        timeout_secs=300.0,
+    )
+
+    output_type = captured["output_type"]
+    assert isinstance(output_type, NativeOutput)
+    assert output_type.outputs == _Widget
+
+
 def test_chat_structured_retries_on_validation_failure(monkeypatch):
     bad = ValidationError.from_exception_data("Widget", [])
     fake = _FakeAgent([bad, _Widget(name="b", count=2)])

tests/test_ollama.py

@@ -388,37 +388,48 @@ class TestUS4ChatCompletion:
         assert len(h2) == 4
 
     @pytest.mark.integration
-    def test_structured_output_retries_then_raises_against_live_server(
+    def test_structured_output_against_unreliable_model_stays_schema_valid(
         self, ollama_host, skip_if_no_ollama
     ):
-        """Scenario: structured_output's retry-then-raise fallback against a
-        real server, using an unreliable model that empirically cannot be
-        made to call the forced tool consistently.
+        """Scenario: structured_output against a real server, using a model
+        with a known-degenerate chat template/tokenizer (ADR-006) that
+        empirically cannot be made to call a forced tool consistently.
 
-        This intentionally does NOT assert a happy-path clean result — see
-        the original ADR-006 rationale. What IS worth proving against a live
-        server: the shared pydantic-ai mechanism (comfydv._llm.chat) makes
-        genuine repeated network calls and produces a well-formed,
-        diagnostic error rather than hanging, crashing uninformatively, or
-        silently returning bad data. The happy path is covered by
-        tests/test_llm_chat_structured.py's mocked suite.
+        ADR-006 originally hand-rolled tool-calling specifically because this
+        model silently ignored Ollama's native `format` field. ADR-009
+        switched the shared pydantic-ai mechanism to NativeOutput (the same
+        native `format`/`response_format` constrained decoding ADR-006
+        avoided) — re-verified live: this model's *output* is still
+        degenerate (garbled tokens, not the literal requested string), but
+        NativeOutput's schema-constrained decoding now forces it into valid
+        JSON shape regardless, so it no longer exhausts retries or hangs.
+        What's worth proving against a live server: the shared mechanism
+        (comfydv._llm.chat) makes a genuine network call and returns
+        well-formed, schema-valid data rather than hanging, crashing
+        uninformatively, or exhausting retries on a shape it can never
+        satisfy. The happy-path *and* retry-exhaustion mechanics are covered
+        deterministically by tests/test_llm_chat_structured.py's mocked
+        suite — this test only proves the live wiring holds up against a
+        genuinely unreliable model's *content*, not its shape.
         """
         (client,) = OllamaClient().create_client(ollama_host)
-        with pytest.raises(RuntimeError) as exc_info:
-            ChatCompletion().chat(
-                client=client,
-                model=_CHAT_MODEL,
-                prompt="Say exactly: pong",
-                options={"think": False},
-                structured_output=True,
-                output_schema=(
-                    '{"type":"object","properties":{"output":{"type":"string"}},'
-                    '"required":["output"]}'
-                ),
-                max_retries=1,
-                unique_id="smoke-test",
-            )
-        assert _CHAT_MODEL in str(exc_info.value)
+        result = ChatCompletion().chat(
+            client=client,
+            model=_CHAT_MODEL,
+            prompt="Say exactly: pong",
+            options={"think": False},
+            structured_output=True,
+            output_schema=(
+                '{"type":"object","properties":{"output":{"type":"string"}},'
+                '"required":["output"]}'
+            ),
+            max_retries=1,
+            unique_id="smoke-test",
+        )
+        response_text, _history, model_name, output = result["result"]
+        assert model_name == _CHAT_MODEL
+        assert json.loads(response_text) == {"output": output}
+        assert isinstance(output, str) and output.strip()
 
 
 # ---------------------------------------------------------------------------
@@ -442,6 +453,12 @@ _MULTI_FIELD_SCHEMA = (
     '"is_positive": {"type": "boolean"}}, '
     '"required": ["summary", "score", "is_positive"]}'
 )
+_SCHEMA_WITH_OPTIONAL_NULLABLE_FIELD = (
+    '{"type": "object", "properties": {'
+    '"output": {"type": "string"}, '
+    '"duration_seconds": {"type": "number"}}, '
+    '"required": ["output"]}'
+)
 
 
 class TestStructuredOutput:
@@ -528,6 +545,32 @@ class TestStructuredOutput:
         assert isinstance(score, int)
         assert is_positive is True
 
+    def test_optional_field_accepts_explicit_null(self):
+        """Regression guard: a non-required field must accept an *explicit*
+        null in the model's JSON, not just being omitted entirely. Models
+        routinely emit `"duration_seconds": null` rather than dropping the
+        key — `_build_structured_model` typed non-required fields as bare
+        `py_type` with a `None` default, which only covers omission; an
+        explicit `None` value failed pydantic's type check (`None` is not a
+        `float`) until the field was typed `py_type | None` instead. Found
+        live: this passed every mocked test (mocks never validate against
+        the real model) but broke the actual LTX pipeline the first time a
+        real model returned an explicit null for an optional field."""
+        fake = _FakeProvider(
+            structured_field_values={"output": "hi", "duration_seconds": None}
+        )
+        ret = ChatCompletion().chat(
+            client=fake,
+            model="m",
+            prompt="hi",
+            structured_output=True,
+            output_schema=_SCHEMA_WITH_OPTIONAL_NULLABLE_FIELD,
+            unique_id="n3b",
+        )
+        _, _, _, output, duration_seconds = ret["result"]
+        assert output == "hi"
+        assert duration_seconds is None  # FLOAT socket; only STRING coerces None to ""
+
     def test_array_object_property_json_dumped_into_string_slot(self):
         schema = (
             '{"type": "object", "properties": {'

tests/test_ollama_provider.py

@@ -481,7 +481,11 @@ def test_chat_exhausted_retries_still_returns_blank_without_raising_when_done_tr
 # ---------------------------------------------------------------------------
 
 
-def test_chat_structured_builds_v1_base_url_and_delegates(monkeypatch):
+def test_chat_structured_calls_native_api_chat_with_format(monkeypatch):
+    """ADR-009: chat_structured hand-rolls a call to Ollama's *native*
+    /api/chat with a "format" JSON schema — not the shared pydantic-ai
+    chat.py helper over the OpenAI-compat endpoint (that endpoint was found
+    to silently discard context-size options on every call)."""
     from pydantic import BaseModel
 
     class Widget(BaseModel):
@@ -489,14 +493,12 @@ def test_chat_structured_builds_v1_base_url_and_delegates(monkeypatch):
 
     captured = {}
 
-    async def fake_chat_structured(**kwargs):
-        captured.update(kwargs)
-        return Widget(name="x")
+    async def fake_post(url, payload, *, timeout=120.0, headers=None):
+        captured["url"] = url
+        captured["payload"] = payload
+        return {"message": {"content": '{"name": "x"}'}}
 
-    monkeypatch.setattr(
-        "comfydv._llm.chat.chat_structured",
-        fake_chat_structured,
-    )
+    monkeypatch.setattr(provider_mod, "_post_json", fake_post)
 
     result = _run_async(
         OllamaProvider("http://localhost:11434").chat_structured(
@@ -505,14 +507,16 @@ def test_chat_structured_builds_v1_base_url_and_delegates(monkeypatch):
     )
 
     assert result == Widget(name="x")
-    assert captured["base_url"] == "http://localhost:11434/v1"
-    assert captured["model"] == "llama3"
+    assert captured["url"] == "http://localhost:11434/api/chat"
+    assert captured["payload"]["model"] == "llama3"
+    assert captured["payload"]["format"] == Widget.model_json_schema()
+    assert captured["payload"]["messages"] == [{"role": "user", "content": "hi"}]
 
 
 def test_chat_structured_forwards_options(monkeypatch):
-    """Regression guard: options must reach the shared chat_structured()
-    helper, not just the cache key — see specs/007-llm-provider-abstraction
-    beacon-reviewer finding."""
+    """Regression guard: options must reach the native /api/chat payload,
+    not just the cache key — see specs/007-llm-provider-abstraction
+    beacon-reviewer finding (original guard, still applicable post-ADR-009)."""
     from pydantic import BaseModel
 
     class Widget(BaseModel):
@@ -520,11 +524,11 @@ def test_chat_structured_forwards_options(monkeypatch):
 
     captured = {}
 
-    async def fake_chat_structured(**kwargs):
-        captured.update(kwargs)
-        return Widget(name="x")
+    async def fake_post(url, payload, *, timeout=120.0, headers=None):
+        captured.update(payload)
+        return {"message": {"content": '{"name": "x"}'}}
 
-    monkeypatch.setattr("comfydv._llm.chat.chat_structured", fake_chat_structured)
+    monkeypatch.setattr(provider_mod, "_post_json", fake_post)
 
     _run_async(
         OllamaProvider("http://localhost:11434").chat_structured(
@@ -538,6 +542,63 @@ def test_chat_structured_forwards_options(monkeypatch):
     assert captured["options"] == {"temperature": 0.0, "seed": 42}
 
 
+def test_chat_structured_retries_on_invalid_json(monkeypatch):
+    from pydantic import BaseModel
+
+    class Widget(BaseModel):
+        name: str
+
+    responses = [
+        {"message": {"content": "not json"}},
+        {"message": {"content": '{"name": "b"}'}},
+    ]
+    calls = []
+
+    async def fake_post(url, payload, *, timeout=120.0, headers=None):
+        calls.append(payload)
+        return responses[len(calls) - 1]
+
+    monkeypatch.setattr(provider_mod, "_post_json", fake_post)
+
+    result = _run_async(
+        OllamaProvider("http://localhost:11434").chat_structured(
+            "llama3",
+            [Message(role="user", content="hi")],
+            Widget,
+            max_retries=2,
+        )
+    )
+
+    assert result == Widget(name="b")
+    assert len(calls) == 2
+
+
+def test_chat_structured_exhausted_retries_raises_runtime_error(monkeypatch):
+    from pydantic import BaseModel
+
+    class Widget(BaseModel):
+        name: str
+
+    async def fake_post(url, payload, *, timeout=120.0, headers=None):
+        return {"message": {"content": "not json"}}
+
+    monkeypatch.setattr(provider_mod, "_post_json", fake_post)
+
+    with pytest.raises(RuntimeError) as exc_info:
+        _run_async(
+            OllamaProvider("http://localhost:11434").chat_structured(
+                "llama3",
+                [Message(role="user", content="hi")],
+                Widget,
+                max_retries=2,
+            )
+        )
+
+    message = str(exc_info.value)
+    assert "llama3" in message
+    assert "3 attempt(s)" in message
+
+
 def test_chat_structured_caches_after_successful_validation(monkeypatch):
     from pydantic import BaseModel
 
@@ -546,11 +607,11 @@ def test_chat_structured_caches_after_successful_validation(monkeypatch):
 
     calls = {"n": 0}
 
-    async def fake_chat_structured(**kwargs):
+    async def fake_post(url, payload, *, timeout=120.0, headers=None):
         calls["n"] += 1
-        return Widget(name="cached")
+        return {"message": {"content": '{"name": "cached"}'}}
 
-    monkeypatch.setattr("comfydv._llm.chat.chat_structured", fake_chat_structured)
+    monkeypatch.setattr(provider_mod, "_post_json", fake_post)
 
     provider = OllamaProvider("http://localhost:11434")
     messages = [Message(role="user", content="hi")]

workflows/README.md

@@ -0,0 +1,201 @@
+# LTX-2.3 I2V Multi-Agent Pipeline
+
+`ltx-i2v-pipeline.json` wires the 6-agent prompt-compiler pipeline from
+[`project-management/Work/planning/ltx.md`](../project-management/Work/planning/ltx.md)
+onto comfydv's existing generic LLM nodes — `OllamaClient`, `ChatCompletion`
+(structured output, image input) and `FormatString` (Jinja2 templating). No
+new node code was needed; this is a wiring exercise, not a feature.
+
+## Loading it
+
+This is a ComfyUI **API-format** workflow (`{node_id: {class_type, inputs}}`),
+not the canvas/UI export format. Recent ComfyUI frontends accept this format
+directly via drag-and-drop onto the graph (it auto-lays-out the nodes), or you
+can `POST` it straight to `/prompt`. This format was chosen deliberately over
+hand-authoring the litegraph UI-export format: the latter requires exact
+per-node-type widget-value ordering and link/slot bookkeping that's easy to
+get subtly wrong by hand and impossible for me to verify without a live
+ComfyUI+Ollama instance. Every template, schema, and link index in this file
+*was* verified against the actual node source (see "How this was verified"
+below) — only the outer graph-serialization format is unverified against a
+real ComfyUI load.
+
+If you'd rather have the literal draggable canvas file, load this one once,
+arrange the nodes, and use ComfyUI's own "Save (API Format)" vs. regular
+"Save" to produce one — that guarantees a format your ComfyUI build actually
+round-trips.
+
+## Prerequisites
+
+- Ollama running locally with a model that has both `vision` and `tools`
+  capability (check `ollama show <model>`'s `capabilities` list) — every
+  agent in this pipeline uses `structured_output=True`, and some also need
+  vision. Default baked into the workflow: `qwen3.5:9b`, used for every
+  agent (text and vision alike) — edit the `model` field on each
+  `ChatCompletion` node if you have something else installed.
+  `lukey03/qwen3.5-9b-abliterated-vision` was tried and rejected: its
+  chat template is degenerate enough that it returns valid-shaped but
+  garbled content regardless of structured-output mechanism (see
+  [ADR-009](../project-management/ADRs/ADR-009-native-structured-output-mode.md)).
+- **If your model has "thinking" capability** (most current instruct
+  models do), give it real headroom: nodes `16`/`17` set
+  `max_tokens=8192`/`num_ctx=32768` for exactly this reason — a thinking
+  model's chain-of-thought reasoning consumes real tokens before it ever
+  emits the structured JSON, and Ollama's default context (4096, with
+  `--context-shift` silently evicting old context rather than stopping)
+  is nowhere near enough for these agents' long system prompts. Node `17`'s
+  `num_ctx` reaches Ollama correctly because `OllamaProvider.chat_structured()`
+  now calls Ollama's *native* `/api/chat` + `"format"` directly (ADR-009) —
+  an earlier version of this fix tried priming context via a separate call
+  before the real request and that didn't work, because Ollama's
+  OpenAI-compatible endpoint silently reloads the model at its default
+  context on every call, undoing any priming; the native endpoint doesn't
+  have that problem and applies `options` and structured output atomically
+  in one request. Every `ChatCompletion` node's `timeout_secs=600` for the
+  same headroom reason; lower it if your hardware is faster than the
+  machine this was tuned against.
+- To use `LlamaCppClient` instead of `OllamaClient`, swap node `1`'s
+  `class_type` and `host` — every `ChatCompletion` node keeps working
+  unchanged, since both emit the same `LLM_CLIENT` type (ADR-007). Note
+  llama-server's context is fixed at process launch (`--ctx-size`), not
+  a per-request setting — node `17`'s `num_ctx` only affects Ollama.
+- Replace node `2`'s `image` filename with your actual starting frame.
+
+## Status: confirmed working end-to-end against a live server
+
+Ran to completion (`status: success`) against a real local Ollama server
+(`qwen3.5:9b`), through the actual ComfyUI node graph, all 6 agents plus
+the final-output node. Getting here took two real comfydv bugs, both fixed
+and documented in [ADR-009](../project-management/ADRs/ADR-009-native-structured-output-mode.md):
+
+1. Ollama's OpenAI-compatible endpoint silently reloads the model at its
+   default (tiny) context size on every call, discarding any
+   `options.num_ctx` — fixed by switching `OllamaProvider.chat_structured()`
+   to Ollama's native `/api/chat` + `"format"`, which doesn't have that
+   problem.
+2. `_build_structured_model` (`src/comfydv/ollama.py`) typed non-required
+   schema fields as bare `py_type` with a `None` default, which only
+   covers a field being *omitted* — an explicit `null` in the model's JSON
+   (which models routinely emit) failed pydantic validation. Fixed by
+   typing those fields `py_type | None`.
+
+**One remaining quirk, not a wiring bug:** `qwen3.5:9b` sometimes
+under-attends to short/simple prompt content — in one full run it reported
+Agent 1's `user_intent` as "no content provided" despite the field being
+populated, which cascaded into an empty Director prompt, which the Judge
+correctly caught (`decision: FAIL`) and the Refiner correctly attempted to
+repair. That's the multi-agent design working as intended against a bad
+upstream extraction — the fix for *that* is prompt/model tuning on Agent 1,
+not a pipeline change. Re-run if you hit it; it isn't consistent.
+
+```bash
+# isolated single-agent test — much faster to debug than the full graph
+python3 -c "
+import json
+d = json.load(open('ltx-i2v-pipeline.json'))
+subset = {k: d[k] for k in ['1','2','16','17','3','4']}  # Agent 1 only
+json.dump({'prompt': subset}, open('/tmp/agent1_only.json','w'))
+"
+curl -X POST http://localhost:8188/prompt -H "Content-Type: application/json" \
+  --data @/tmp/agent1_only.json
+```
+
+## Pipeline shape
+
+```
+OllamaClient ─┬─────────────────────────────────────────────────────────┐
+LoadImage ────┼──────────┬──────────┬──────────┬──────────┐             │
+              │          │          │          │          │             │
+FormatString→ChatCompletion (Agent 1: Intent Compiler)     [no image]   │
+              │                                    │                     │
+              FormatString→ChatCompletion (Agent 2: Scene Grounder) ←image
+                                    │
+              FormatString→ChatCompletion (Agent 3: Manifest Verifier) ←image
+                                    │
+       intent ─┴─ audited_manifest
+              FormatString→ChatCompletion (Agent 4: Director) ←image
+                                    │
+       + intent + manifest ────────┴── candidate_prompt
+              FormatString→ChatCompletion (Agent 5: Judge) ←image
+                                    │
+       + everything above ─────────┴── judge_report
+              FormatString→ChatCompletion (Agent 6: Refiner)  [no image]
+                                    │
+              FormatString (Final Output — judge decision + both prompts)
+```
+
+## Deliberate adaptations from ltx.md
+
+1. **Single round, no retry loop.** ltx.md's reference pseudocode runs
+   `for iteration in range(2): judge → refine`, short-circuiting on PASS.
+   ComfyUI graphs are DAGs with no native conditional/loop node in this repo
+   (checked `circuit_breaker.py`, `random_choice.py` — neither fits), so a
+   real retry loop can't be expressed as a static graph. This workflow always
+   runs Judge once and Refiner once. The **Final Output** node (`15`) shows
+   the Judge's `decision` next to *both* the Director's candidate prompt and
+   the Refiner's patched prompt — read the decision and use the candidate
+   prompt on PASS, the refined prompt on FAIL. Wire a second Judge/Refiner
+   pair after node `14` yourself if you want the second round.
+
+2. **Structured output carries whole objects, not just fields.**
+   `ChatCompletion`'s `response` output is the full JSON object
+   (`parsed.model_dump_json()`), and — since `structured_output=True` also
+   adds one extra named output per top-level schema property — a specific
+   nested object can be pulled out directly by name (e.g. Agent 3's
+   `audited_manifest` output, used instead of its `response` wrapper, which
+   also contains `verification`). Templates use `{{ x }}` directly rather
+   than ltx.md's `{{ x | tojson(indent=2) }}`, since `x` arrives already
+   JSON-encoded.
+
+3. **List-valued template variables are JSON strings.** `FormatString`'s
+   dynamic inputs are always `STRING`; there's no native list socket. Fields
+   like `preservation_requirements` or `extraction_hints` are typed as JSON
+   arrays (e.g. `["keep hairstyle"]`) and unpacked in-template with the
+   `fromjson` filter your repo's `FormatString` already ships. Leave them as
+   empty string `""` to omit the section entirely — falsy-string `{% if %}`
+   checks guard every optional block, so `fromjson` is never called on an
+   empty value.
+
+4. **`output_schema` is intentionally shallow.** `ChatCompletion` only
+   enforces *top-level* property types (see `_build_structured_model` in
+   `src/comfydv/ollama.py`) — it doesn't validate nested structure. The
+   detailed nested shape each agent must produce (e.g. every field inside
+   `required_camera`) is still communicated to the model via the literal
+   JSON example embedded in that agent's system prompt (verbatim from
+   ltx.md), so nothing is lost — the `output_schema` JSON here just needs to
+   get the top-level field list and types right, which is also all
+   `ChatCompletion` uses it for.
+
+5. **Required fields exclude anything legitimately blank.** A `required`
+   *string* field is forced non-empty by `ChatCompletion`
+   (`Field(..., min_length=1)`) to catch blank-output failures. Fields that
+   are correctly empty on a non-nominal status — Director's `prompt` on
+   `UNSATISFIABLE`, Judge's `refinement_instruction` on `PASS`, Refiner's
+   `prompt`/`unresolvable_reason` — are deliberately left out of each
+   schema's `required` list so a legitimate empty string doesn't trigger a
+   retry loop against the model.
+
+6. **Optional Agent 7 (Targeted Manifest Resolver) is not wired.** It only
+   fires on a `MANIFEST_CHALLENGE`, which this static graph can't branch on.
+   Add it manually if the Director or Judge start returning that status for
+   your inputs.
+
+## How this was verified
+
+Everything except the outer graph-serialization format was checked against
+this repo's actual node code (not just read — executed), with mocked
+`comfy`/`server`/`folder_paths` modules the way `tests/conftest.py` does:
+
+- Every `[node_id, index]` link target resolves to a real node.
+- Every `FormatString` template's variables (via the same
+  `jinja_env.parse` + `meta.find_undeclared_variables` AST extraction
+  `_extract_keys` uses) exactly match the inputs supplied in the workflow.
+- Every template renders through the real `FormatString.format_string()`
+  with representative values, including the Director's `SHOT CONSTRAINTS`
+  block, which was parsed back with `json.loads` to confirm it's valid JSON
+  even when every optional field is left blank.
+- Every `output_schema` parses through the real `_parse_output_schema`/
+  `_build_structured_model`, and every link that targets a *named* structured
+  output (e.g. `audited_manifest`, `prompt`, `decision`) was checked against
+  the actual computed `(response, updated_history, model_name, *properties)`
+  output order for that schema.