refactor(llm): drop the image content block until a path can honor it
ImageBlock had no production producer and every consumer dropped it: the deepseek serializer skipped it, the pi-ai converter skipped it as unrepresentable, the ACP bridge neither advertises image prompt capability nor forwards image blocks, and compact-basic charged a flat 85-token estimate and rendered an [image] placeholder. A block constructed today would silently vanish from the wire — the vocabulary advertised a capability no path honors, the silent-data-loss shape the defensive patterns warn against. The only constructors were tests pinning the skip/estimate branches. Remove ImageBlock and its ContentBlockMap entry (its cache?: CacheHint field leaves with it; CacheHint itself and the other two cache? fields are out of scope). compact-basic loses its explicit image estimate and placeholder arms (the merge-extensible default arms absorb the case); the deepseek serializer, pi-ai converter, and ACP codec already handled image in their default arms, so only their image-naming comments change. The codec's inbound rejection of ACP-protocol image prompt content stays — that guards wire content a client can send regardless of our vocabulary. Tests that constructed harness image blocks to pin the removed branches are dropped (the 85-token estimate pin) or retargeted onto plugin-added block types / other non-text blocks, which the surviving default arms own. Docs, the type-equiv pastes, and the content-block vocabulary RFC's block list and multimodal-home consequence are updated in the same change; the RFC moves to implemented/ and the index is regenerated. A real multimodal feature reintroduces image via declaration merging together with the adapter mapping, ACP advertisement, and compaction pricing that honor it.
This commit is contained in:
@@ -34,7 +34,6 @@ A second, independent implementation of the same seam exists in `@deepseek-ai/ds
|
||||
## Limitations (MVP, documented deliberately)
|
||||
|
||||
- `prefill` throws `LlmError('UNSUPPORTED')` — DeepSeek's chat-prefix completion is a Beta feature on the `/beta` base URL; future work.
|
||||
- `image` blocks are skipped (no vision support on these models).
|
||||
- `tool_choice` is not mapped (not part of the core vocabulary).
|
||||
|
||||
## Errors
|
||||
|
||||
@@ -11,7 +11,6 @@
|
||||
* rule for thinking mode — required there, ignored elsewhere, so we save
|
||||
* the tokens elsewhere); `tool-call` → `tool_calls[]`
|
||||
* - `tool-result` → its own `{role: 'tool'}` message (text flattened)
|
||||
* - `image` → skipped (MVP limitation, documented in the README)
|
||||
*
|
||||
* @module dsh-llm-deepseek/serialize
|
||||
*/
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { CallId, LlmError } from '@deepseek-ai/dsh-llm'
|
||||
import type { GenerateOptions, Message } from '@deepseek-ai/dsh-llm'
|
||||
import type { ContentBlock, GenerateOptions, Message } from '@deepseek-ai/dsh-llm'
|
||||
import { serializeMessages, serializeRequest } from '@deepseek-ai/dsh-llm-deepseek'
|
||||
|
||||
function request(overrides: Partial<GenerateOptions> = {}): GenerateOptions {
|
||||
@@ -110,11 +110,17 @@ describe('serializeMessages', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('skips image blocks (documented MVP limitation)', () => {
|
||||
it('skips plugin-added block types (merge-extensible ContentBlockMap)', () => {
|
||||
const wire = serializeMessages([
|
||||
{ role: 'user', content: [{ type: 'image', url: 'data:image/png;base64,x' }, { type: 'text', text: 'see image' }] },
|
||||
{
|
||||
role: 'user',
|
||||
content: [
|
||||
{ type: 'chart', data: 'x' } as unknown as ContentBlock,
|
||||
{ type: 'text', text: 'see chart' },
|
||||
],
|
||||
},
|
||||
])
|
||||
expect(wire).toEqual([{ role: 'user', content: 'see image' }])
|
||||
expect(wire).toEqual([{ role: 'user', content: 'see chart' }])
|
||||
})
|
||||
|
||||
it('emits an empty user message rather than dropping block-less messages', () => {
|
||||
|
||||
@@ -31,7 +31,7 @@ pi-ai declares the openai/anthropic/google/mistral/AWS SDKs as install-time depe
|
||||
|
||||
## Limitations
|
||||
|
||||
Same MVP contract as llm-deepseek: `prefill` throws `UNSUPPORTED`, images are not representable, `tool_choice` is not mapped.
|
||||
Same MVP contract as llm-deepseek: `prefill` throws `UNSUPPORTED`, `tool_choice` is not mapped.
|
||||
|
||||
## Testing
|
||||
|
||||
|
||||
@@ -93,7 +93,7 @@ export function toPiContext(options: GenerateOptions): PiContext {
|
||||
})
|
||||
break
|
||||
default:
|
||||
// image / plugin-added block types: not representable here.
|
||||
// plugin-added block types: not representable here.
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { CallId } from '@deepseek-ai/dsh-llm'
|
||||
import type { StreamChunk } from '@deepseek-ai/dsh-llm'
|
||||
import type { ContentBlock, StreamChunk } from '@deepseek-ai/dsh-llm'
|
||||
import type { AssistantMessage, AssistantMessageEvent, Usage } from '@earendil-works/pi-ai'
|
||||
import { mapStopReason, mapUsage, toPiContext, toStreamChunks } from '@deepseek-ai/dsh-llm-pi-ai'
|
||||
|
||||
@@ -171,13 +171,13 @@ describe('toPiContext', () => {
|
||||
expect(context.messages.map(message => message.role)).toEqual(['user', 'user', 'toolResult'])
|
||||
})
|
||||
|
||||
it('skips image and unknown blocks in assistant content', () => {
|
||||
it('skips plugin-added (unknown) blocks in assistant content', () => {
|
||||
const context = toPiContext({
|
||||
model: 'm',
|
||||
messages: [{
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{ type: 'image', url: 'data:,x' },
|
||||
{ type: 'chart', data: 'x' } as unknown as ContentBlock,
|
||||
{ type: 'text', text: 'visible' },
|
||||
],
|
||||
}],
|
||||
|
||||
@@ -25,7 +25,7 @@ An adapter registry plus a single streaming call surface, interceptable via a wa
|
||||
|
||||
### Content-block vocabulary (`types.ts`)
|
||||
|
||||
Messages are arrays of typed content blocks: `text`, `reasoning`, `tool-call`, `tool-result`, `image`. The union is derived from the merge-extensible `ContentBlockMap`, so plugins can add block types via declaration merging.
|
||||
Messages are arrays of typed content blocks: `text`, `reasoning`, `tool-call`, `tool-result`. The union is derived from the merge-extensible `ContentBlockMap`, so plugins can add block types via declaration merging. The core set is limited to blocks every shipping path honors — multimodal content (images, audio, …) has no core block type; a feature that needs one adds it via the map together with the adapter/UI/compaction support that honors it.
|
||||
|
||||
Streaming is a raw chunk protocol (`block-start`, `text-delta`, `reasoning-delta`, `tool-call-delta`, `block-end`, `usage`, `finish`). `BlockAssembler` is the single shared implementation that assembles chunks into blocks/messages.
|
||||
|
||||
|
||||
@@ -57,24 +57,22 @@ export interface ToolResultBlock {
|
||||
cache?: CacheHint
|
||||
}
|
||||
|
||||
/** An image, by URL or data URL. */
|
||||
export interface ImageBlock {
|
||||
type: 'image'
|
||||
url: string
|
||||
mimeType?: string
|
||||
cache?: CacheHint
|
||||
}
|
||||
|
||||
/**
|
||||
* All known content block shapes, keyed by their `type` tag.
|
||||
* Merge-extensible: plugins add new block types via declaration merging.
|
||||
*
|
||||
* The core set is deliberately limited to blocks every shipping path honors.
|
||||
* Multimodal content (images, audio, …) has no core block type: a feature
|
||||
* that needs one adds it via declaration merging in the same coordinated
|
||||
* change that maps it in the adapters, surfaces it in the UI bridges, and
|
||||
* prices it in compaction — a producer never lands without its consumers
|
||||
* (see docs/rfc/implemented/simplification/2026-07-04-drop-image-content-block.md).
|
||||
*/
|
||||
export interface ContentBlockMap {
|
||||
'text': TextBlock
|
||||
'reasoning': ReasoningBlock
|
||||
'tool-call': ToolCallBlock
|
||||
'tool-result': ToolResultBlock
|
||||
'image': ImageBlock
|
||||
}
|
||||
|
||||
export type ContentBlockType = keyof ContentBlockMap
|
||||
|
||||
@@ -63,12 +63,12 @@ describe('BlockAssembler', () => {
|
||||
|
||||
it('throws from assemble() when a partial has an unhandled blockType', () => {
|
||||
const assembler = new BlockAssembler()
|
||||
// Directly push a block-end for an image block whose block-start never
|
||||
// called ensure — but the image block-type flows through normally.
|
||||
// What we really need is a partial whose blockType is not text/reasoning/tool-call.
|
||||
// We can achieve this via a block-start for 'image' followed by blocks().
|
||||
assembler.push({ type: 'block-start', index: 0, blockType: 'image' } as unknown as StreamChunk)
|
||||
expect(() => assembler.blocks()).toThrow('cannot assemble incomplete block of type "image"')
|
||||
// A partial whose blockType is not text/reasoning/tool-call cannot be
|
||||
// assembled without its block-end. A plugin-added block type (here
|
||||
// 'video', via the merge-extensible ContentBlockMap) opened by a
|
||||
// block-start with no closing block-end exercises that throw.
|
||||
assembler.push({ type: 'block-start', index: 0, blockType: 'video' } as unknown as StreamChunk)
|
||||
expect(() => assembler.blocks()).toThrow('cannot assemble incomplete block of type "video"')
|
||||
})
|
||||
|
||||
it('mustGet throws when an index is missing from the partials map (invariant violation)', () => {
|
||||
|
||||
@@ -81,7 +81,7 @@ describe('BlockAssembler properties', () => {
|
||||
fc.assert(fc.property(streamArb, (chunks) => {
|
||||
const blocks = feed(chunks).blocks()
|
||||
for (const block of blocks) {
|
||||
expect(['text', 'reasoning', 'tool-call', 'tool-result', 'image']).toContain(block.type)
|
||||
expect(['text', 'reasoning', 'tool-call', 'tool-result']).toContain(block.type)
|
||||
}
|
||||
}))
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user