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
Wires the 6-agent prompt-compiler pipeline from
project-management/Work/planning/ltx.md onto comfydv's existing generic
LLM nodes (OllamaClient, ChatCompletion, FormatString) — no new node code,
purely a wiring exercise. Verified end-to-end against a live local Ollama
server through the actual ComfyUI node graph (all 6 agents plus the final
judge/refiner output), which is what surfaced and validated the fixes in
the preceding commit.
workflows/README.md documents the deliberate adaptations from the
reference design (single-round Judge/Refiner since ComfyUI graphs can't
express the retry loop, JSON-string list handling for FormatString's
STRING-only dynamic sockets, etc.) and how the wiring was verified against
the real node source before ever touching a live server.
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
Start the vlm-image-input bullet for this branch (BEACON BUILD tracking);
mark T009 with an honest note on pre-existing gate exceptions (repo-wide ty
diagnostics, a baseline Docker test, 007's deferred tasks) — all out of scope
for spec 009, whose own code is ruff/format/pytest green and doctor-clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvS9TFMCFYNHC4MMvzJaZS
Add the BEACON DESIGN scaffold for wiring a ComfyUI IMAGE into the existing
generic ChatCompletion node so a vision-capable model can describe/understand
images on either backend.
- specs/009-vlm-image-input/spec.md — WHAT/WHY: optional IMAGE input, same
node on both backends, structured-output-with-image, graceful degradation
on non-vision models; text-only path unchanged when no image is wired
- ADR-008 (Proposed) — carry images on an optional Message.images field; each
provider translates to its own wire shape (Ollama /api/chat images,
llama.cpp OpenAI image_url parts, shared chat_structured multimodal
content). Extends ADR-007's adapter pattern to a second input modality
- epics/vlm-image-input.md — new epic with success criteria/non-goals; spec
backlinked via .beacon.toml and listed under the epic
- Roadmap + ADR index updated
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvS9TFMCFYNHC4MMvzJaZS
Spec 008-llamacpp-integration is complete; epic-finish bookkeeping
(archive) follows in a separate chore PR once this merges, matching
the llm-provider-abstraction epic's pattern.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
Full spec/plan/tasks/BDD scaffold for the llamacpp-integration epic, now
that its dependency (llm-provider-abstraction, PR #17) is satisfied.
Researched llama-server's router-mode API shape live (postdates training
data) rather than assuming it — two details that would have been wrong by
assumption: the model identifier field is "id" not "name" (differs from
Ollama's /api/tags), and "status" is a nested object ({"value": "..."}) not
a flat string.
Unlike the prerequisite epic, this decomposition genuinely holds as 4
independent user stories — LlamaCppClient is a new node, not a changed one,
so there's no shared "output type" migration forcing an atomic cutover.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
Full line-by-line inventory of every reference in src/comfydv/ollama.py
and tests/test_ollama.py (1820 lines, read in full — not estimated)
affected by the OllamaClient/OllamaChatCompletion/OllamaModelSelector/
OllamaLoadModel/OllamaUnloadModel rename, plus the design decisions it
surfaced that "just rename it" was hiding:
- Single source of truth for HTTP/cache infra (comfydv._llm.ollama_provider)
- client == "<host string>" equality is removed, not preserved (OllamaProvider
isn't a str subclass)
- Bare-string client backward compatibility is removed (undocumented side
effect of the old str-subclass trick, never an intended feature)
- Test-layer split: node-contract/delegation tests stay in test_ollama.py
using a _FakeProvider double; Ollama-wire-protocol tests move to a new
test_ollama_provider.py — this is what actually resolves the 35 relocated
_post_json monkeypatches, rather than patching them 1:1 at a seam that no
longer exists
- Structured-output retry tests are not ported 1:1 — already covered by
tests/test_llm_chat_structured.py's existing 6 tests
Produces a 12-step sequenced task list (T-CUT-01..12) in
atomic-cutover-plan.md that supersedes tasks.md's original US1/US3
decomposition (struck through, kept for history). This is now the
authoritative plan for the next BUILD session to execute.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
Discovered mid-implementation: OllamaClient is a single shared producer for
every downstream Ollama node, so changing its output type per ADR-007
breaks all consumers simultaneously, and the class rename breaks
tests/test_ollama.py's 125 references atomically, not per-story. Confirmed
by independent product + engineering review (agent-trio deliberation,
aligned). Re-scoped the node-layer cutover as its own follow-up session,
tracked in issue #16. ADR-007's decision itself is unaffected — only
delivery sequencing changes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
Adopt a shared LLMProvider protocol (list/load/unload/chat/structured-chat)
as the adapter pattern answer to GitHub issue #15 — Ollama and llama.cpp
share the chat/structured-output mechanism via pydantic-ai while model
lifecycle stays backend-specific per provider. Supersedes ADR-006, narrows
ADR-004's scope to non-chat REST calls.
Adds epic 007 (llm-provider-abstraction, prerequisite) and the
llamacpp-integration epic it unblocks, plus the full spec/plan/tasks/BDD
scaffold for the prerequisite epic.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
Adds structured_output/output_schema/max_retries inputs to
OllamaChatCompletion, fixing three real reliability problems: models
prepending commentary, wrapping responses in code fences, and occasionally
returning blank output. When enabled, requests go through Ollama's
OpenAI-compatible tool-calling endpoint (a single forced tool call matching
output_schema) instead of native /api/chat, and each response is validated
against a dynamically-built pydantic model — invalid/blank responses retry
with fresh network calls, exhausting into a clear RuntimeError rather than
silently passing bad data downstream. One additional ComfyUI output socket
is exposed per schema property, mirroring FormatString's existing
dynamic-output pattern. Fully backward compatible: structured_output
defaults off and non-structured behavior is untouched.
Ollama's native /api/chat "format" field (JSON-Schema-constrained decoding)
was tried first but proved unreliable against a real local model during
implementation — see ADR-006 for the full investigation and why
tool-calling was chosen over both native format and pydantic-ai (the latter
would reintroduce httpx/openai-sdk, reversing ADR-004).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
All owned specs shipped, so beacon epic finish moves it to archive/ and
flips status to Done. Clears the epic-spec-lifecycle and epic-gates
warnings beacon doctor --strict was raising.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Parallel subagent audit (2026-06-28) identified four categories of defects:
- requirements.txt broken for ComfyUI Manager install
- FormatString: no debounce, clears connections, ID mismatch, class mutation
- RandomChoice/CircuitBreaker correctness bugs
- Stale metadata and dead code throughout
Epic captures 15 success criteria across installation, UX, and correctness.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: initialise BEACON framework with all bootstrap artefacts
- Problem statement, constitution, architecture doc, roadmap populated
- CHANGELOG.md created (Keep a Changelog); README expanded with
What-is-this, Install, and Quickstart sections
- pyproject.toml gains [project.urls] (repository + documentation)
- beacon doctor: 32 pass, 2 pre-commit warns, 0 failures
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: Phase 1 — add NullHandler to package root, remove hardcoded setLevel
T001: logging.getLogger("comfydv").addHandler(NullHandler()) in __init__.py
T002: remove logger.setLevel(logging.DEBUG) from format_string.py
Also fix pyproject.toml TOML structure (project.urls was inside [project] block)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: failing tests for logging modernisation (T010-T through T030-T)
RED phase — 5 tests fail for the correct reasons before implementation:
T010-T: format_string produces stdout (print block)
T011-T: update_widget emits INFO records on hot path
T012-T: random_choice produces stdout (colorama/rich prints)
T020-T: load_node_state uses print() on error instead of logger.error
T021-T: circuit_breaker uses print() instead of logger
T001/T002/T030-T already green: NullHandler registered, setLevel removed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat: standardise logging across all nodes (T010-I through T044)
GREEN phase — all 11 logging tests pass:
T010-I: Remove 8-line diagnostic print block from format_string()
T011-I: Downgrade all hot-path logger.info() calls to logger.debug();
switch all logger calls to %-style formatting; remove rich import
T012-I: Replace colorama/termcolor/rich print calls in random_choice with
logger.debug(); add logger = logging.getLogger(__name__)
T020-I: Convert print() on load_node_state error to logger.error()
T021-I: Add logger to circuit_breaker; replace print() with logger.debug();
fix logic so status=False triggers the interrupt (per BDD spec)
T040: Remove colorama, rich, termcolor from pyproject.toml dependencies
T042: ruff check + format clean
T044: beacon doctor --strict passes (34/34)
Also adds ADR-001 and ADR-002 capturing the stdlib logging and NullHandler
decisions, linked from the logging-modernisation epic.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: mark all 001-standardise-logging tasks complete in tasks.md
All [x] checkboxes flipped after 11/11 tests pass and beacon doctor --strict
reports 0 failures.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: mark logging-modernisation epic success criteria complete
All success criteria verified: NullHandler added, setLevel removed, print()
calls converted, colorama/rich/termcolor removed from deps, zero stdout in
normal operation, errors surface at ERROR level, all tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: commit spec artefacts and dependency lock for 001-standardise-logging
Includes spec.md, plan.md, research.md, BDD feature files, contracts, .beacon.toml
backlink, and uv.lock after removing colorama/rich/termcolor.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: James Veitch <darthveitcher@office-mac-mini.local>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
