docs(plan): plan VLM image input (009) — research, data-model, contracts
Phase 0/1 design artifacts for spec 009, resolving the wire shapes ADR-008 deferred for live verification. - research.md — verified pydantic-ai BinaryContent against the installed pydantic-ai-slim 2.9.0 source (structured path is shared via OpenAIChatModel, one change covers both backends); Ollama flat images passthrough; llama.cpp OpenAI image_url content-parts (requires --mmproj); tensor→PNG via Pillow (dev dep) with _llm/ kept torch/numpy/Pillow-free per Constitution IV; byte-identical text path by omitting an empty images key - data-model.md — Message gains optional images: list[str] | None; one carrier, three wire shapes; optional IMAGE node input (outputs untouched) - contracts/image-input-contract.md — behavioural + test contracts T1–T6 - quickstart.md — describe-an-image workflow, structured variant, backend swap - plan.md — Constitution Check passes (no new class, no new runtime dep, outputs unchanged); no Complexity Tracking needed - CLAUDE.md SpecKit marker repointed to 009 plan Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UvS9TFMCFYNHC4MMvzJaZS
This commit is contained in:
Claude
@@ -1,5 +1,5 @@
|
||||
<!-- SPECKIT START -->
|
||||
For additional context about technologies to be used, project structure,
|
||||
shell commands, and other important information, read the current plan
|
||||
at specs/008-llamacpp-integration/plan.md
|
||||
at specs/009-vlm-image-input/plan.md
|
||||
<!-- SPECKIT END -->
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
# Contract: Image Input across the LLMProvider boundary
|
||||
|
||||
**Spec**: [spec.md](../spec.md) · **Data model**: [data-model.md](../data-model.md) · **ADR**: [ADR-008](../../../project-management/ADRs/ADR-008-multimodal-image-input-across-llmprovider-boundary.md)
|
||||
|
||||
This feature adds no new protocol methods and no new socket types. The contract
|
||||
below is the **behavioural conformance** every `LLMProvider` must satisfy for
|
||||
the new `Message.images` field, plus the node's input contract.
|
||||
|
||||
---
|
||||
|
||||
## C1 — `Message.images` carrier
|
||||
|
||||
- `Message.images: list[str] | None = None`, base64 strings (no `data:` prefix).
|
||||
- `images=None` or `[]` ⇒ the turn is text-only and MUST produce a request
|
||||
**byte-for-byte identical** to the pre-feature behaviour.
|
||||
|
||||
## C2 — `LLMProvider.chat()` conformance (both providers)
|
||||
|
||||
Given `messages` where the last user turn carries `images`:
|
||||
1. The provider MUST transmit those images with that turn to its backend using
|
||||
its native shape (Ollama flat `images`; llama.cpp OpenAI `image_url` parts).
|
||||
2. The provider MUST NOT transmit an `images` field for turns that have none
|
||||
(empty key omitted).
|
||||
3. All existing behaviour is preserved: blank-retry-with-new-seed loop, response
|
||||
caching, timeout, and error surfacing are unchanged by the presence of images.
|
||||
4. A backend that cannot process images (non-vision model / no `--mmproj`) MUST
|
||||
have its error surfaced to the caller, not swallowed (FR-006).
|
||||
|
||||
## C3 — `chat_structured()` conformance (shared helper, both providers)
|
||||
|
||||
1. Images on the last user turn MUST be attached as `BinaryContent` on the
|
||||
`Agent.run` `user_prompt`; images on history user turns MUST be attached to
|
||||
their `UserPromptPart`.
|
||||
2. All existing structured guarantees hold unchanged: bounded retries (0–5),
|
||||
`RuntimeError` on exhaustion naming model/attempts/snippet, never returns a
|
||||
value that failed schema validation.
|
||||
3. A text-only structured call MUST be indistinguishable from today's.
|
||||
|
||||
## C4 — `ChatCompletion` node input contract
|
||||
|
||||
1. Adds exactly one **optional** `image: ("IMAGE",)` input. No required input
|
||||
added; `RETURN_TYPES`/`RETURN_NAMES` positions unchanged (Constitution VI).
|
||||
2. Un-wired ⇒ behaviour, request, and outputs identical to today.
|
||||
3. Wired ⇒ image(s) attached to the current user turn only; `history` turns
|
||||
unchanged (FR-007).
|
||||
4. Works with `structured_output=True` (C3) and free-text (C2) alike, on either
|
||||
backend, with no per-backend wiring difference (FR-004).
|
||||
|
||||
---
|
||||
|
||||
## Test contracts (test-first — Constitution III)
|
||||
|
||||
| ID | Level | Asserts |
|
||||
|---|---|---|
|
||||
| T1 | `Message` unit | `images` defaults `None`; round-trips base64 list; text-only dump omits the key |
|
||||
| T2 | `OllamaProvider.chat` | image turn → payload message has flat `images:[...]`; text-only payload byte-identical to today (regression) |
|
||||
| T3 | `LlamaCppProvider.chat` | image turn → `content` becomes text+`image_url` parts; text-only `content` stays a plain string (regression) |
|
||||
| T4 | `chat_structured` | image turn builds `BinaryContent` on the prompt; text-only path unchanged; retry/validation contract intact |
|
||||
| T5 | node encode helper | synthetic `[1,H,W,3]` tensor → decodable base64 PNG; `None`/empty → `[]` |
|
||||
| T6 | node contract | optional `image` in `INPUT_TYPES`; un-wired run == today; wired run attaches to last turn only |
|
||||
|
||||
All tests run without a live ComfyUI or a live backend (mock at each provider's
|
||||
own `_post_json`/`Agent.run` seam, per the `test_ollama_provider.py`
|
||||
convention). T5 uses a synthetic tensor + Pillow (dev dep), no ComfyUI.
|
||||
@@ -0,0 +1,83 @@
|
||||
# Phase 1 Data Model: VLM Image Input for ChatCompletion
|
||||
|
||||
**Spec**: [spec.md](./spec.md) · **Research**: [research.md](./research.md)
|
||||
|
||||
The feature adds **one optional field** to an existing model and defines how it
|
||||
maps into each backend's wire shape. No new entities, no new socket types.
|
||||
|
||||
---
|
||||
|
||||
## Modified entity — `Message` (`src/comfydv/_llm/provider.py`)
|
||||
|
||||
```python
|
||||
class Message(BaseModel):
|
||||
role: Literal["system", "user", "assistant"]
|
||||
content: str
|
||||
images: list[str] | None = None # NEW — base64-encoded images (no data: prefix)
|
||||
```
|
||||
|
||||
**Field: `images`**
|
||||
- **Type**: `list[str] | None`, default `None`.
|
||||
- **Meaning**: base64-encoded image payloads associated with this turn. `None`
|
||||
(or empty) means a text-only turn — **byte-for-byte identical to today**.
|
||||
- **Carrier form**: raw base64 string, no `data:` URI prefix. Chosen because
|
||||
every target adapts *from* it (Ollama `images` array, OpenAI data-URI,
|
||||
pydantic-ai `BinaryContent`) — ADR-008.
|
||||
- **Validation**: no format validation at the model layer (the model stays a
|
||||
dumb carrier); malformed data surfaces as a backend error (FR-006). A turn
|
||||
may carry ≥1 image; MVP exercises exactly one.
|
||||
- **Serialization invariant**: provider payload construction MUST omit the
|
||||
`images` key when `None`/empty so existing text-only requests are unchanged
|
||||
(research.md Decision 2; FR-003, SC-004).
|
||||
|
||||
---
|
||||
|
||||
## Mapping table — one carrier, three wire shapes
|
||||
|
||||
| Path | Code site | Transform |
|
||||
|---|---|---|
|
||||
| Ollama free-text | `ollama_provider.py::chat` | none — `model_dump()`'s flat `images` array already matches `/api/chat`; only drop the key when empty |
|
||||
| llama.cpp free-text | `llamacpp_provider.py::chat` | rebuild `content` as OpenAI parts: `[{"type":"text",...},{"type":"image_url","image_url":{"url":"data:image/png;base64,<b64>"}}]` |
|
||||
| Structured (both) | `chat.py::chat_structured` | build `BinaryContent(data=b64decode(img), media_type="image/png")`; attach to `user_prompt` (last turn) / `UserPromptPart` (history turns) as `[text, *images]` |
|
||||
|
||||
---
|
||||
|
||||
## Node input — `ChatCompletion` (`src/comfydv/ollama.py`)
|
||||
|
||||
Add to `INPUT_TYPES["optional"]`:
|
||||
|
||||
```python
|
||||
"image": ("IMAGE",),
|
||||
```
|
||||
|
||||
- **Optional** — un-wired ⇒ `image=None` ⇒ the node builds exactly today's
|
||||
text-only user message. No new required input; no output/socket change
|
||||
(Constitution VI untouched — `RETURN_TYPES` positions 0/1 unchanged).
|
||||
- When wired: encode the tensor to base64 PNG(s) (research.md Decision 4) and
|
||||
set them on the appended `Message(role="user", ...)`. History turns are not
|
||||
modified (FR-007).
|
||||
|
||||
### Encode helper (node-local, `comfy`/Pillow lazy)
|
||||
|
||||
```
|
||||
_encode_image_tensor(image) -> list[str]:
|
||||
# image: ComfyUI IMAGE, torch float tensor [B, H, W, C] in 0..1
|
||||
# → for each frame: *255 → uint8 → PIL.Image.fromarray → PNG bytes → base64
|
||||
# returns [] for None/empty so callers treat it as "no image"
|
||||
```
|
||||
|
||||
Lives in `ollama.py` (node module, already `comfy`-guarded). `src/comfydv/_llm/`
|
||||
never imports torch/numpy/Pillow — it deals only in the base64 strings this
|
||||
helper produces.
|
||||
|
||||
---
|
||||
|
||||
## State & relationships
|
||||
|
||||
- No persistent state; no new caching entity. Existing `_CHAT_RESPONSE_CACHE`
|
||||
keys already include the dumped messages, so an added `images` value
|
||||
participates in the cache key automatically (same image + prompt ⇒ cache
|
||||
hit), and a text-only turn's key is unchanged since the empty key is omitted.
|
||||
- Relationship: `images` belongs to exactly one `Message` (one turn) — this is
|
||||
why the carrier is a message field, not a side-channel parameter (ADR-008
|
||||
Alternative A rejected).
|
||||
@@ -0,0 +1,125 @@
|
||||
# Implementation Plan: VLM Image Input for ChatCompletion
|
||||
|
||||
**Branch**: `009-vlm-image-input` | **Date**: 2026-07-22 | **Spec**: [spec.md](./spec.md)
|
||||
|
||||
**Input**: Feature specification from `/specs/009-vlm-image-input/spec.md`
|
||||
|
||||
## Summary
|
||||
|
||||
Let a workflow author wire a ComfyUI `IMAGE` into the existing generic
|
||||
`ChatCompletion` node so a vision-capable model can describe or reason about it,
|
||||
on **either** backend. Per ADR-008 (extending ADR-007's adapter pattern to a
|
||||
second input modality), images ride on an optional `Message.images` carrier and
|
||||
each provider translates that carrier into its own wire shape: Ollama's flat
|
||||
`/api/chat` `images` array (passes through untouched), llama.cpp's OpenAI
|
||||
`image_url` content-parts, and — for structured output — pydantic-ai
|
||||
`BinaryContent`, shared by both backends through `OpenAIChatModel`. The node
|
||||
converts its `IMAGE` tensor to base64 PNG; everything below the node deals only
|
||||
in base64 strings. Text-only behaviour is byte-for-byte unchanged when no image
|
||||
is wired.
|
||||
|
||||
## Technical Context
|
||||
|
||||
**Language/Version**: Python ≥3.11 (unchanged, per `pyproject.toml`)
|
||||
|
||||
**Primary Dependencies**: existing only for runtime — `aiohttp` (Ollama/llama.cpp
|
||||
REST), `pydantic-ai-slim[openai]>=2.9.0` (structured path; its `BinaryContent`
|
||||
multimodal type was verified against the installed 2.9.0 source, see
|
||||
`research.md`). **No new core runtime dependency.** `pillow` is added to the
|
||||
**dev** group so the node's tensor→PNG encoder is unit-testable without a live
|
||||
ComfyUI; at runtime Pillow/numpy are ComfyUI-provided (same stance the repo
|
||||
already takes for torch).
|
||||
|
||||
**Storage**: N/A — no persistent storage; reuses the existing
|
||||
`_CHAT_RESPONSE_CACHE` (an `images` value participates in the cache key
|
||||
automatically).
|
||||
|
||||
**Testing**: `pytest` via `uv run pytest`, following the
|
||||
`tests/test_ollama_provider.py` convention (mock at each provider's own
|
||||
`_post_json` / `Agent.run` seam, no live server or ComfyUI required). Test-first
|
||||
per Constitution III; the tensor-encode test uses a synthetic tensor + Pillow.
|
||||
|
||||
**Target Platform**: ComfyUI custom-node runtime, same as the existing LLM nodes.
|
||||
|
||||
**Project Type**: Library / ComfyUI custom-node pack (single project).
|
||||
|
||||
**Performance Goals**: No new numeric target; image encoding is a one-shot
|
||||
per-execution PNG encode, negligible against inference latency.
|
||||
|
||||
**Constraints**: Text-only requests MUST stay byte-identical (FR-003/SC-004) —
|
||||
providers omit an empty `images` key. `src/comfydv/_llm/` must not import
|
||||
torch/numpy/Pillow (Constitution IV) — tensor handling stays in the node.
|
||||
llama.cpp image support requires a server launched with `--mmproj` (deployment
|
||||
prerequisite, surfaced as a clear error when absent, not configured by comfydv).
|
||||
|
||||
**Scale/Scope**: One new `Message` field; a per-provider mapping in each
|
||||
`chat()` plus the shared `chat_structured()`; one optional node input + a
|
||||
node-local encode helper. No new node classes, no new socket types, no protocol
|
||||
method changes.
|
||||
|
||||
## Constitution Check
|
||||
|
||||
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
|
||||
|
||||
| Principle | Verdict | Notes |
|
||||
|---|---|---|
|
||||
| I. ComfyUI Contract First | PASS | `ChatCompletion` keeps its `INPUT_TYPES`/`RETURN_TYPES`/`FUNCTION`/`CATEGORY`; only an optional input is added. No new registration, no ComfyUI changes. |
|
||||
| II. Sandbox All User-Supplied Code | N/A | No template/expression evaluation in this feature. |
|
||||
| III. Test-First | PASS (binding) | Test contracts T1–T6 (`contracts/image-input-contract.md`) written test-first, mirroring `test_ollama_provider.py`. Each runs without a live ComfyUI/backend. |
|
||||
| IV. Graceful Degradation Outside ComfyUI | PASS (binding) | `_llm/` stays torch/numpy/Pillow-free — pure base64 carrier + mapping, unit-testable. Tensor→PNG lives in `ollama.py` (already `comfy`-guarded) with lazy Pillow/numpy import, so module import outside ComfyUI is unaffected. |
|
||||
| V. Simplicity — Function Before Class | PASS | No new class. New logic is a `Message` field, two small per-provider transforms, one shared helper edit, and one module-level encode function. |
|
||||
| VI. Fixed Output Positions | PASS | Outputs are untouched — only an optional **input** is added; `RETURN_TYPES`/`RETURN_NAMES` positions 0/1 and the structured extra-outputs contract are unchanged. |
|
||||
|
||||
Re-checked post-Phase 1 design (data-model.md, contracts/): unchanged — no new
|
||||
gate violations. No Complexity Tracking entries needed.
|
||||
|
||||
## Project Structure
|
||||
|
||||
### Documentation (this feature)
|
||||
|
||||
```text
|
||||
specs/009-vlm-image-input/
|
||||
├── plan.md # This file
|
||||
├── research.md # Phase 0 — wire shapes verified against installed deps
|
||||
├── data-model.md # Phase 1 — Message.images + per-provider mapping
|
||||
├── quickstart.md # Phase 1 — minimal describe-an-image workflow
|
||||
├── contracts/
|
||||
│ └── image-input-contract.md # Phase 1 — behavioural + test contracts (T1–T6)
|
||||
├── checklists/requirements.md # Spec quality checklist (from /speckit-specify)
|
||||
└── tasks.md # Phase 2 (/speckit-tasks) — not created here
|
||||
```
|
||||
|
||||
### Source Code (repository root)
|
||||
|
||||
```text
|
||||
src/comfydv/
|
||||
├── ollama.py # MODIFIED — ChatCompletion: optional `image` input +
|
||||
│ # node-local _encode_image_tensor() (lazy Pillow/numpy)
|
||||
├── _llm/
|
||||
│ ├── provider.py # MODIFIED — Message gains `images: list[str] | None = None`
|
||||
│ ├── ollama_provider.py # MODIFIED — chat(): pass flat images through; omit empty key
|
||||
│ ├── llamacpp_provider.py # MODIFIED — chat(): map images → OpenAI image_url parts
|
||||
│ └── chat.py # MODIFIED — chat_structured(): images → BinaryContent on prompt
|
||||
└── __init__.py # unchanged — no new node class or mapping
|
||||
|
||||
tests/
|
||||
├── test_provider.py (or test_ollama_provider.py) # T1 Message carrier + regression
|
||||
├── test_ollama_provider.py # MODIFIED — T2 Ollama image mapping + text regression
|
||||
├── test_llamacpp_provider.py # MODIFIED — T3 llama.cpp content-parts + text regression
|
||||
├── test_llm_chat.py / chat tests # T4 chat_structured multimodal + regression
|
||||
└── test_ollama.py # MODIFIED — T5 encode helper, T6 node input contract
|
||||
|
||||
pyproject.toml # MODIFIED — add `pillow` to [dependency-groups].dev only
|
||||
```
|
||||
|
||||
**Structure Decision**: Purely additive edits to the four existing `_llm`/node
|
||||
files that ADR-007 established — no new module, because there is no new class or
|
||||
node (contrast 008, which added a provider + node). The image path threads
|
||||
through the exact seams the text path already uses, which is the whole point of
|
||||
ADR-008: a second modality on the same adapter, not a parallel structure.
|
||||
|
||||
## Complexity Tracking
|
||||
|
||||
> **Fill ONLY if Constitution Check has violations that must be justified**
|
||||
|
||||
None — see Constitution Check above.
|
||||
@@ -0,0 +1,48 @@
|
||||
# Quickstart: Describe an image with ChatCompletion
|
||||
|
||||
**Spec**: [spec.md](./spec.md)
|
||||
|
||||
Minimal end-to-end walkthrough of the feature once shipped.
|
||||
|
||||
## Prerequisites
|
||||
|
||||
- A running backend with a **vision-capable** model:
|
||||
- **Ollama** — a multimodal model pulled and available (e.g. a llava-class model), or
|
||||
- **llama.cpp** — `llama-server` launched in router mode **with a multimodal projector**: `--mmproj <projector.gguf>` alongside the model.
|
||||
- comfydv installed in ComfyUI.
|
||||
|
||||
## Steps
|
||||
|
||||
1. Add an image source to the canvas (e.g. **Load Image**) → gives an `IMAGE`.
|
||||
2. Add a client node (**OllamaClient** or **LlamaCppClient**) → gives `LLM_CLIENT`.
|
||||
3. Add **ChatCompletion**. Wire:
|
||||
- `client` ← the client node
|
||||
- `model` ← a vision-capable model name (typed or wired)
|
||||
- `prompt` ← `"Describe this image in one sentence."`
|
||||
- `image` ← the `IMAGE` from step 1 ← **the only new wire**
|
||||
4. Queue the prompt. The `response` output is a text description of the image.
|
||||
|
||||
## Structured variant (optional)
|
||||
|
||||
- On **ChatCompletion**, set `structured_output = True` and provide a schema, e.g.:
|
||||
```json
|
||||
{"type":"object","properties":{"caption":{"type":"string"},"has_text":{"type":"boolean"}},"required":["caption","has_text"]}
|
||||
```
|
||||
- Run: each field (`caption`, `has_text`) appears as its own typed output,
|
||||
populated from the image, with no required field blank.
|
||||
|
||||
## Swap backends (proves FR-004 / SC-002)
|
||||
|
||||
- Replace **OllamaClient** with **LlamaCppClient** (pointed at an `--mmproj`
|
||||
server) — **change nothing else**. Re-queue: same image description path.
|
||||
|
||||
## What stays the same
|
||||
|
||||
- Leave `image` un-wired and ChatCompletion behaves exactly as before — text-only,
|
||||
identical results. No existing workflow changes.
|
||||
|
||||
## Expected failure (proves FR-006 / SC-005)
|
||||
|
||||
- Wire an image but select a **non-vision** model (or a `llama-server` started
|
||||
without `--mmproj`): the node reports a clear error that the model/server
|
||||
can't process images — it does not silently answer as if no image was sent.
|
||||
@@ -0,0 +1,155 @@
|
||||
# Phase 0 Research: VLM Image Input for ChatCompletion
|
||||
|
||||
**Spec**: [spec.md](./spec.md) · **Plan**: [plan.md](./plan.md) · **ADR**: [ADR-008](../../project-management/ADRs/ADR-008-multimodal-image-input-across-llmprovider-boundary.md)
|
||||
|
||||
ADR-008 recorded the boundary decision (images on `Message.images`, translated
|
||||
per-provider) but deferred the exact wire shapes for live verification. This
|
||||
file resolves them against the **installed** dependency versions and the
|
||||
current provider code, not from memory.
|
||||
|
||||
---
|
||||
|
||||
## Decision 1 — pydantic-ai multimodal vehicle (structured-output path)
|
||||
|
||||
**Decision**: In the shared `chat_structured()` helper (`src/comfydv/_llm/chat.py`),
|
||||
attach images as `pydantic_ai.messages.BinaryContent(data=<png bytes>,
|
||||
media_type="image/png")` inside a `Sequence[UserContent]`. The current-turn
|
||||
image rides on `Agent.run(user_prompt=[text, BinaryContent(...)])`; a
|
||||
history turn's image rides on `UserPromptPart(content=[text, BinaryContent(...)])`.
|
||||
|
||||
**Rationale / verified**: Read directly from the pinned
|
||||
`pydantic_ai_slim==2.9.0` source in this environment:
|
||||
|
||||
- `BinaryContent` (`messages.py:521`) — `__init__(self, data: bytes, *,
|
||||
media_type: ..., identifier=None, ...)`; exposes a `.base64` helper. `data`
|
||||
is **bytes**, so `chat.py` must `base64.b64decode()` the `Message.images`
|
||||
string into bytes when building it.
|
||||
- `UserPromptPart.content: str | Sequence[UserContent]` (`messages.py:1022`)
|
||||
and `user_prompt` on `Agent.run` accept the same. `UserContent = str |
|
||||
TextContent | MultiModalContent | CachePoint` (`messages.py:899`), and
|
||||
`MultiModalContent` includes `BinaryContent`/`ImageUrl` — so a `[text,
|
||||
image]` list is the supported shape.
|
||||
- `OpenAIChatModel` renders `BinaryContent` for images as an OpenAI
|
||||
`image_url` data-URI, and reads `BinaryContent.vendor_metadata['detail']`
|
||||
for the `detail` setting (documented in the field's own docstring).
|
||||
|
||||
**Consequence**: the structured path is provider-agnostic *for free* — both
|
||||
Ollama and llama.cpp reach `/v1/chat/completions` through the same
|
||||
`OpenAIChatModel`, so one change in `chat.py` covers structured output on both
|
||||
backends. No per-provider structured code.
|
||||
|
||||
**Alternatives considered**: `ImageUrl(url="data:image/png;base64,...")` — also
|
||||
supported, but requires assembling a data URI string; `BinaryContent` from raw
|
||||
bytes + media type is the more direct representation of what we hold and lets
|
||||
pydantic-ai own the data-URI formatting.
|
||||
|
||||
---
|
||||
|
||||
## Decision 2 — Ollama free-text path (`/api/chat`)
|
||||
|
||||
**Decision**: `OllamaProvider.chat()` sends each message's images as a flat
|
||||
`images` array of **base64 strings** (no `data:` prefix) alongside `content`,
|
||||
which is exactly Ollama's native `/api/chat` message schema. Because
|
||||
`Message.images` already holds base64 strings, `Message.model_dump()` produces
|
||||
the correct shape with **no transform** — the field flows straight through.
|
||||
|
||||
**Rationale / verified**: `OllamaProvider.chat()`
|
||||
(`src/comfydv/_llm/ollama_provider.py:280`) already builds
|
||||
`payload_messages = [m.model_dump() for m in messages]` and POSTs to
|
||||
`/api/chat`. Ollama's documented `/api/chat` message object is
|
||||
`{"role", "content", "images": [<base64>, ...]}` — the flat sibling field this
|
||||
carrier maps onto directly. This is the reason base64 is the neutral carrier
|
||||
form (ADR-008).
|
||||
|
||||
**Constraint discovered — byte-identical text path (FR-003/SC-004)**: adding
|
||||
`images: list[str] | None = None` to `Message` means a text-only message would
|
||||
dump as `{"role","content","images":null}`, changing today's request body.
|
||||
Providers MUST drop a `None`/empty `images` before sending. Resolution:
|
||||
serialize provider payload messages with the images key omitted when empty
|
||||
(e.g. `model_dump(exclude_none=True)`, or drop the key explicitly). Guarded by
|
||||
the existing Ollama provider tests, which assert the exact payload.
|
||||
|
||||
---
|
||||
|
||||
## Decision 3 — llama.cpp free-text path (`/v1/chat/completions`)
|
||||
|
||||
**Decision**: `LlamaCppProvider.chat()` maps a message carrying images into
|
||||
OpenAI-style multimodal `content` **parts** before POSTing:
|
||||
`content: [{"type":"text","text":<content>},
|
||||
{"type":"image_url","image_url":{"url":"data:image/png;base64,<b64>"}}]`.
|
||||
Messages with no images keep the plain-string `content` unchanged.
|
||||
|
||||
**Rationale / verified**: `LlamaCppProvider.chat()`
|
||||
(`src/comfydv/_llm/llamacpp_provider.py:163`) builds
|
||||
`payload_messages = [m.model_dump() for m in messages]` and POSTs to
|
||||
`/v1/chat/completions`. Unlike Ollama, a flat `images` sibling is **not**
|
||||
understood there — OpenAI's vision schema requires images inside `content` as
|
||||
typed parts. `llama-server` implements this OpenAI-compatible multimodal
|
||||
format **only when launched with a multimodal projector (`--mmproj`)**; without
|
||||
it, image parts yield a server error (surfaced per FR-006, not crashed on).
|
||||
This is the single point where the two providers genuinely diverge — exactly
|
||||
the leakage ADR-008 localizes inside each provider.
|
||||
|
||||
**Alternatives considered**: normalizing Ollama *up* to content-parts too (one
|
||||
shared mapper) — rejected in ADR-008 Alternative C: it forces the
|
||||
currently-simpler Ollama path to do extra work and inverts "each provider owns
|
||||
its wire format."
|
||||
|
||||
---
|
||||
|
||||
## Decision 4 — ComfyUI IMAGE tensor → base64 PNG (node layer)
|
||||
|
||||
**Decision**: The `ChatCompletion` node converts its optional `IMAGE` input to
|
||||
base64 PNG(s) via Pillow: ComfyUI IMAGE is a float tensor `[B, H, W, C]` in
|
||||
`0..1`; scale to `uint8`, `PIL.Image.fromarray(...)`, save PNG to an in-memory
|
||||
buffer, base64-encode. A batch of `B` frames becomes `B` base64 strings in the
|
||||
turn's `images` list (natural multi-image; MVP exercises `B=1`). The import of
|
||||
Pillow/numpy is **lazy** (inside the encode function), so the module still
|
||||
imports cleanly outside ComfyUI (Constitution IV).
|
||||
|
||||
**Rationale**: Pillow is the ComfyUI-ecosystem standard for IMAGE tensor ↔
|
||||
file and is present in every ComfyUI install; numpy comes with torch. Neither
|
||||
is added to comfydv's **core** runtime deps — they are ComfyUI-provided, the
|
||||
same stance the repo already takes for torch (dev-only in `pyproject.toml`).
|
||||
To keep the encoder **test-first** (Constitution III) without a live ComfyUI,
|
||||
add `pillow` to the **dev** dependency group so a unit test can feed a
|
||||
synthetic `numpy`/`torch` tensor through the pure encode function and assert a
|
||||
decodable PNG.
|
||||
|
||||
**Boundary kept clean**: only the node (`src/comfydv/ollama.py`, already
|
||||
`comfy`-guarded) touches tensors/Pillow. Everything in `src/comfydv/_llm/`
|
||||
deals purely in base64 strings and stays unit-testable with hand-crafted
|
||||
strings — no torch, numpy, or Pillow import there.
|
||||
|
||||
**Edge cases (FR-006, Edge Cases)**: an un-wired optional input arrives as
|
||||
`None` → node builds today's exact text-only message. A zero-size / empty batch
|
||||
tensor → treated as "no image". A non-vision model or non-`mmproj` server
|
||||
returns a backend error → surfaced with a clear message, never a silent
|
||||
image-less answer.
|
||||
|
||||
---
|
||||
|
||||
## Decision 5 — where the image attaches on the turn (FR-007)
|
||||
|
||||
**Decision**: The node attaches images to the **current user turn only** — the
|
||||
`Message(role="user", content=prompt, images=[...])` it already appends. Prior
|
||||
`history` turns are untouched. The structured helper likewise only lifts images
|
||||
onto the final user turn (and any history turn that already carried them),
|
||||
matching its existing "last message is the prompt" contract
|
||||
(`chat.py:106`, which requires `messages[-1].role == "user"`).
|
||||
|
||||
---
|
||||
|
||||
## Summary of resolved unknowns
|
||||
|
||||
| Unknown (from ADR-008) | Resolved to |
|
||||
|---|---|
|
||||
| pydantic-ai multimodal type | `BinaryContent(data=bytes, media_type="image/png")` — verified in installed 2.9.0 |
|
||||
| Structured path per-provider? | No — shared via `OpenAIChatModel`; one change in `chat.py` |
|
||||
| Ollama wire shape | flat `images: [base64]` on the message; passes through `model_dump()` |
|
||||
| llama.cpp wire shape | OpenAI `image_url` content-parts; requires `--mmproj` |
|
||||
| Text-path byte-identity | drop empty `images` key in provider payloads (guarded by existing tests) |
|
||||
| Tensor → base64 | Pillow, lazy import in node; `pillow` added to dev deps for testability |
|
||||
| No new runtime deps | Confirmed — Pillow/numpy are ComfyUI-provided, dev-only here |
|
||||
|
||||
No `NEEDS CLARIFICATION` remain.
|
||||
Reference in New Issue
Block a user