fix(llm): address multimodal review findings
This commit is contained in:
Yichen Jiang
parent
cd7e45ced6
commit
4a02791c9a
15 changed files with 268 additions and 83 deletions
|
|
@ -2,5 +2,5 @@
|
|||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-08-19-direct-deepseek-vision-input.md
|
||||
2026-08-19-direct-deepseek-vision-input.md: 60f55f10e108a29d465e388fa385b1955b7f2ce5
|
||||
2026-08-19-direct-deepseek-vision-input.zh.md: 7f41d89ffd13da1b1b170f114dfc481e66075ef2
|
||||
2026-08-19-direct-deepseek-vision-input.md: 5c2f2ea3ff87b58906ce76caf9f611e07f3e2d04
|
||||
2026-08-19-direct-deepseek-vision-input.zh.md: 3aea6b6a4f27f7116c1cb935b1719dfbcf7ed8fc
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ Canonical messages continue to store only `ImageAttachmentRef`. Data URLs exist
|
|||
|
||||
## Verification
|
||||
|
||||
Package tests pin model discovery and fallback capabilities, configuration validation and live settings updates, user and tool-result wire messages, all admitted MIME types, cancellation, attachment failures, 413 classification, exact image-bound behavior, and pi-ai equivalence. A keyless assembled ACP request records the native adapter's tool-result data URL and oldest-image placeholder. A key-gated real-API e2e sends a deterministic image to the official vision model.
|
||||
Package tests pin model discovery and fallback capabilities, configuration validation and live settings updates, user and tool-result wire messages, all admitted MIME types, cancellation, attachment failures, 413 classification, exact image-bound behavior, and pi-ai equivalence. A keyless assembled ACP request records the native adapter's tool-result data URL and oldest-image placeholder.
|
||||
|
||||
## Consequences
|
||||
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ DeepSeek 视觉部署使用 chat-completions 图片协议，但直接 `deepseek-
|
|||
|
||||
## Verification
|
||||
|
||||
包测试固定模型发现与回退能力、配置校验与存活 settings 更新、user 和工具结果协议消息、所有已准入 MIME 类型、取消、附件失败、413 分类、确切图片上限行为和 pi-ai 等价性。无需密钥的组装 ACP 请求会记录原生适配器的工具结果 data URL 与最旧图片占位文本。受密钥控制的真实 API e2e 会向官方视觉模型发送一张确定性图片。
|
||||
包测试固定模型发现与回退能力、配置校验与存活 settings 更新、user 和工具结果协议消息、所有已准入 MIME 类型、取消、附件失败、413 分类、确切图片上限行为和 pi-ai 等价性。无需密钥的组装 ACP 请求会记录原生适配器的工具结果 data URL 与最旧图片占位文本。
|
||||
|
||||
## Consequences
|
||||
|
||||
|
|
|
|||
|
|
@ -3,4 +3,4 @@
|
|||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write docs/config-catalog.md
|
||||
config-catalog.md: 7d42923c1bc4c73636b52713694ff8f6eeb9146a
|
||||
config-catalog.zh.md: b9327fa127ba8e341a15954685942f2d15d6b8d3
|
||||
config-catalog.zh.md: 327a4951c513e7cd2360488026e7578e95458f47
|
||||
|
|
|
|||
|
|
@ -897,7 +897,7 @@ export interface DeepSeekCatalogModel {
|
|||
|
||||
依赖：[`ModelModality`](../packages/llm/llm/src/index.ts) · [`RetryPolicyConfig`](../packages/llm/llm/src/index.ts)
|
||||
|
||||
来源：[`packages/llm/llm-deepseek/src/index.ts:72`](../packages/llm/llm-deepseek/src/index.ts)
|
||||
来源：[`packages/llm/llm-deepseek/src/index.ts:66`](../packages/llm/llm-deepseek/src/index.ts)
|
||||
|
||||
<a id="deepseek-aidsh-llm-pi-ai"></a>
|
||||
|
||||
|
|
|
|||
|
|
@ -2,5 +2,5 @@
|
|||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write docs/module-graph.md
|
||||
module-graph.md: 0bd5f80534ba65e0d483bd04228cdac321e082bc
|
||||
module-graph.zh.md: 6239520b7d819eb14c2b859afa44a0c987545200
|
||||
module-graph.md: 5398515912c98962152d2b121b1b02d7a06097b9
|
||||
module-graph.zh.md: 7a398374603d66f8a3661a1d927b6418a99235a6
|
||||
|
|
|
|||
|
|
@ -407,6 +407,7 @@ flowchart TD
|
|||
pkg_settings_file --> pkg_invariants
|
||||
pkg_settings_file --> pkg_settings
|
||||
pkg_llm_deepseek --> pkg_anonymous_user_id
|
||||
pkg_llm_deepseek --> pkg_attachment
|
||||
pkg_llm_deepseek --> pkg_credentials
|
||||
pkg_llm_deepseek --> pkg_invariants
|
||||
pkg_llm_deepseek --> pkg_launch_environment
|
||||
|
|
@ -1463,7 +1464,7 @@ flowchart TD
|
|||
| [`client-hmr`](../packages/client/hmr) | `client` | [`client-modules`](../packages/client/modules), [`host-webserver`](../packages/host/webserver), [`invariants`](../packages/runtime-diagnostics/invariants) |
|
||||
| [`credentials-local`](../packages/credentials/credentials-local) | `credentials` | [`atomic-write`](../packages/util/atomic-write), [`credentials`](../packages/credentials/credentials), [`home-paths`](../packages/util/home-paths), [`invariants`](../packages/runtime-diagnostics/invariants), [`launch-environment`](../packages/util/launch-environment) |
|
||||
| [`settings-file`](../packages/settings/settings-file) | `settings` | [`atomic-write`](../packages/util/atomic-write), [`home-paths`](../packages/util/home-paths), [`invariants`](../packages/runtime-diagnostics/invariants), [`settings`](../packages/settings/settings) |
|
||||
| [`llm-deepseek`](../packages/llm/llm-deepseek) | `llm` | [`anonymous-user-id`](../packages/identity/anonymous-user-id), [`credentials`](../packages/credentials/credentials), [`invariants`](../packages/runtime-diagnostics/invariants), [`launch-environment`](../packages/util/launch-environment), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) |
|
||||
| [`llm-deepseek`](../packages/llm/llm-deepseek) | `llm` | [`anonymous-user-id`](../packages/identity/anonymous-user-id), [`attachment`](../packages/attachment/attachment), [`credentials`](../packages/credentials/credentials), [`invariants`](../packages/runtime-diagnostics/invariants), [`launch-environment`](../packages/util/launch-environment), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) |
|
||||
| [`llm-pi-ai`](../packages/llm/llm-pi-ai) | `llm` | [`attachment`](../packages/attachment/attachment), [`credentials`](../packages/credentials/credentials), [`invariants`](../packages/runtime-diagnostics/invariants), [`launch-environment`](../packages/util/launch-environment), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) |
|
||||
| [`session`](../packages/core/session) | `core` | [`brand`](../packages/util/brand), [`invariants`](../packages/runtime-diagnostics/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope), [`typert-protocol`](../packages/typert/protocol) |
|
||||
| [`system-prompt`](../packages/core/system-prompt) | `core` | [`invariants`](../packages/runtime-diagnostics/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope) |
|
||||
|
|
|
|||
|
|
@ -409,6 +409,7 @@ flowchart TD
|
|||
pkg_settings_file --> pkg_invariants
|
||||
pkg_settings_file --> pkg_settings
|
||||
pkg_llm_deepseek --> pkg_anonymous_user_id
|
||||
pkg_llm_deepseek --> pkg_attachment
|
||||
pkg_llm_deepseek --> pkg_credentials
|
||||
pkg_llm_deepseek --> pkg_invariants
|
||||
pkg_llm_deepseek --> pkg_launch_environment
|
||||
|
|
@ -1465,7 +1466,7 @@ flowchart TD
|
|||
| [`client-hmr`](../packages/client/hmr) | `client` | [`client-modules`](../packages/client/modules), [`host-webserver`](../packages/host/webserver), [`invariants`](../packages/runtime-diagnostics/invariants) |
|
||||
| [`credentials-local`](../packages/credentials/credentials-local) | `credentials` | [`atomic-write`](../packages/util/atomic-write), [`credentials`](../packages/credentials/credentials), [`home-paths`](../packages/util/home-paths), [`invariants`](../packages/runtime-diagnostics/invariants), [`launch-environment`](../packages/util/launch-environment) |
|
||||
| [`settings-file`](../packages/settings/settings-file) | `settings` | [`atomic-write`](../packages/util/atomic-write), [`home-paths`](../packages/util/home-paths), [`invariants`](../packages/runtime-diagnostics/invariants), [`settings`](../packages/settings/settings) |
|
||||
| [`llm-deepseek`](../packages/llm/llm-deepseek) | `llm` | [`anonymous-user-id`](../packages/identity/anonymous-user-id), [`credentials`](../packages/credentials/credentials), [`invariants`](../packages/runtime-diagnostics/invariants), [`launch-environment`](../packages/util/launch-environment), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) |
|
||||
| [`llm-deepseek`](../packages/llm/llm-deepseek) | `llm` | [`anonymous-user-id`](../packages/identity/anonymous-user-id), [`attachment`](../packages/attachment/attachment), [`credentials`](../packages/credentials/credentials), [`invariants`](../packages/runtime-diagnostics/invariants), [`launch-environment`](../packages/util/launch-environment), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) |
|
||||
| [`llm-pi-ai`](../packages/llm/llm-pi-ai) | `llm` | [`attachment`](../packages/attachment/attachment), [`credentials`](../packages/credentials/credentials), [`invariants`](../packages/runtime-diagnostics/invariants), [`launch-environment`](../packages/util/launch-environment), [`llm`](../packages/llm/llm), [`settings`](../packages/settings/settings), [`timeout`](../packages/util/timeout) |
|
||||
| [`session`](../packages/core/session) | `core` | [`brand`](../packages/util/brand), [`invariants`](../packages/runtime-diagnostics/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope), [`typert-protocol`](../packages/typert/protocol) |
|
||||
| [`system-prompt`](../packages/core/system-prompt) | `core` | [`invariants`](../packages/runtime-diagnostics/invariants), [`llm`](../packages/llm/llm), [`scope`](../packages/core/scope) |
|
||||
|
|
|
|||
|
|
@ -148,9 +148,12 @@ async function contentParts(
|
|||
|
||||
/** Keep text-only user messages on the compact string wire form. */
|
||||
function userContent(parts: readonly WireUserContentPart[]): string | WireUserContentPart[] {
|
||||
return parts.some(part => part.type === 'image_url')
|
||||
? [...parts]
|
||||
: parts.map(part => part.type === 'text' ? part.text : '').join('')
|
||||
const text: string[] = []
|
||||
for (const part of parts) {
|
||||
if (part.type === 'image_url') return [...parts]
|
||||
text.push(part.text)
|
||||
}
|
||||
return text.join('')
|
||||
}
|
||||
|
||||
/** Serialize one assistant message (text + reasoning + tool calls). */
|
||||
|
|
@ -268,11 +271,12 @@ export async function serializeMessagesWithImages(
|
|||
const toolResults = message.content.filter((block): block is Extract<ContentBlock, { type: 'tool-result' }> => (
|
||||
block.type === 'tool-result'
|
||||
))
|
||||
if (regular.length > 0 || toolResults.length === 0) {
|
||||
const content = userContent(await contentParts(regular, attachments, signal))
|
||||
if (content.length > 0 || toolResults.length === 0) {
|
||||
flushToolImages()
|
||||
wire.push({
|
||||
role: 'user',
|
||||
content: userContent(await contentParts(regular, attachments, signal)),
|
||||
content,
|
||||
})
|
||||
}
|
||||
for (const result of toolResults) {
|
||||
|
|
|
|||
|
|
@ -1,18 +1,10 @@
|
|||
import { mkdtemp, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { createHash } from 'node:crypto'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { Context } from '@deepseek-ai/cordis'
|
||||
import LlmRuntime, { createUserMessage, CallId, ReasoningEffortId , createMessage } from '@deepseek-ai/dsh-llm'
|
||||
import type { Message, ToolSchema } from '@deepseek-ai/dsh-llm'
|
||||
import AttachmentStore, { AttachmentId } from '@deepseek-ai/dsh-attachment'
|
||||
import type {
|
||||
ImageAttachmentLimits,
|
||||
ImageAttachmentRef,
|
||||
SaveImageAttachment,
|
||||
StoredImageAttachment,
|
||||
} from '@deepseek-ai/dsh-attachment'
|
||||
import { LocalCredentialProvider } from '@deepseek-ai/dsh-credentials-local'
|
||||
import * as LlmDeepSeek from '@deepseek-ai/dsh-llm-deepseek'
|
||||
import type { Config } from '@deepseek-ai/dsh-llm-deepseek'
|
||||
|
|
@ -26,41 +18,6 @@ import { assemble, type AssembledResult } from './assemble.ts'
|
|||
|
||||
const FLASH = 'deepseek-v4-flash'
|
||||
const PRO = 'deepseek-v4-pro'
|
||||
const VISION = 'deepseek-v4-flash-vision-exp'
|
||||
const RED_IMAGE = Buffer.from(
|
||||
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC',
|
||||
'base64',
|
||||
)
|
||||
const RED_IMAGE_REF: ImageAttachmentRef = {
|
||||
attachmentId: AttachmentId(`sha256:${createHash('sha256').update(RED_IMAGE).digest('hex')}`),
|
||||
mediaType: 'image/png',
|
||||
bytes: RED_IMAGE.byteLength,
|
||||
width: 1,
|
||||
height: 1,
|
||||
}
|
||||
|
||||
class E2eAttachmentStore extends AttachmentStore {
|
||||
readonly imageLimits: ImageAttachmentLimits = {
|
||||
maxImageBytes: 1024,
|
||||
maxImagesPerMessage: 1,
|
||||
maxMessageImageBytes: 1024,
|
||||
maxImagePixels: 1,
|
||||
maxImageDimension: 1,
|
||||
mediaTypes: ['image/png'],
|
||||
}
|
||||
|
||||
validateImage(_input: SaveImageAttachment): Promise<void> {
|
||||
return Promise.resolve()
|
||||
}
|
||||
|
||||
saveImage(_input: SaveImageAttachment): Promise<ImageAttachmentRef> {
|
||||
return Promise.resolve(RED_IMAGE_REF)
|
||||
}
|
||||
|
||||
readImage(_ref: ImageAttachmentRef, _signal?: AbortSignal): Promise<StoredImageAttachment> {
|
||||
return Promise.resolve({ ref: RED_IMAGE_REF, data: RED_IMAGE })
|
||||
}
|
||||
}
|
||||
const contexts: Context[] = []
|
||||
let identityHome: string
|
||||
|
||||
|
|
@ -73,7 +30,6 @@ async function harness(_model: string, config: Partial<Config> = {}) {
|
|||
const ctx = new Context()
|
||||
contexts.push(ctx)
|
||||
await ctx.plugin(LlmRuntime)
|
||||
await ctx.plugin(E2eAttachmentStore)
|
||||
await ctx.plugin(LlmDeepSeek, config)
|
||||
return ctx
|
||||
}
|
||||
|
|
@ -109,23 +65,6 @@ const weatherTool: ToolSchema = {
|
|||
}
|
||||
|
||||
describe.skipIf(!process.env.DEEPSEEK_API_KEY)('llm-deepseek e2e (real API)', () => {
|
||||
it('recognizes a deterministic image with the official vision model', async () => {
|
||||
const ctx = await harness(VISION, { thinking: 'disabled' })
|
||||
const result = await assemble(ctx, {
|
||||
model: VISION,
|
||||
messages: [createUserMessage({
|
||||
content: [
|
||||
{ type: 'text', text: 'This image is one solid color. Reply with only its English color name.' },
|
||||
{ type: 'image', attachment: RED_IMAGE_REF },
|
||||
],
|
||||
source: { kind: 'plugin', plugin: 'test' },
|
||||
})],
|
||||
maxTokens: 50,
|
||||
})
|
||||
expect(result.finish.kind).toBe('stop')
|
||||
expect(textOf(result).toLowerCase()).toContain('red')
|
||||
})
|
||||
|
||||
it('serves a real request with the key held only by a credentials-local document', async () => {
|
||||
const key = process.env.DEEPSEEK_API_KEY
|
||||
if (key === undefined) throw new Error('e2e ran without DEEPSEEK_API_KEY')
|
||||
|
|
|
|||
|
|
@ -178,7 +178,10 @@ describe('DeepSeekAdapter against a mock server', () => {
|
|||
const server = await mockServer([])
|
||||
const resolveApiKey = vi.fn(() => Promise.resolve('k'))
|
||||
const adapter = new DeepSeekAdapter({
|
||||
options: () => resolveAdapterOptions({ baseURL: server.url }),
|
||||
options: () => resolveAdapterOptions({
|
||||
baseURL: server.url,
|
||||
models: [{ id: 'deepseek-v4-flash-vision-exp', inputModalities: ['text', 'image'] }],
|
||||
}),
|
||||
resolveApiKey,
|
||||
resolveUserId: () => TEST_USER_ID,
|
||||
})
|
||||
|
|
@ -865,6 +868,21 @@ describe('plugin registration and config', () => {
|
|||
])
|
||||
})
|
||||
|
||||
it('defaults an adapter-supplied catalog entry to text input', async () => {
|
||||
const connection = resolveAdapterOptions({ models: [] })
|
||||
const adapter = new DeepSeekAdapter({
|
||||
options: () => ({ ...connection, models: [{ id: 'adapter-model' }] }),
|
||||
resolveApiKey: () => Promise.resolve('k'),
|
||||
resolveUserId: () => TEST_USER_ID,
|
||||
})
|
||||
await expect(adapter.listModels('deepseek-official')).resolves.toEqual([{
|
||||
provider: 'deepseek-official',
|
||||
id: 'adapter-model',
|
||||
name: 'adapter-model',
|
||||
inputModalities: ['text'],
|
||||
}])
|
||||
})
|
||||
|
||||
it('advertises configured models without restricting arbitrary request ids', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LlmRuntime)
|
||||
|
|
@ -954,6 +972,18 @@ describe('plugin registration and config', () => {
|
|||
expect(ctx.llm.listProviders()).toEqual([])
|
||||
})
|
||||
|
||||
const invalidProgrammaticModalities: Array<[LlmDeepSeek.DeepSeekCatalogModel[], RegExp]> = [
|
||||
[[{ id: 'm', inputModalities: [] }], /inputModalities must not be empty/],
|
||||
[[{
|
||||
id: 'm',
|
||||
inputModalities: ['audio'] as unknown as NonNullable<LlmDeepSeek.DeepSeekCatalogModel['inputModalities']>,
|
||||
}], /inputModalities must contain only "text" and "image"/],
|
||||
]
|
||||
|
||||
it.each(invalidProgrammaticModalities)('rejects programmatic modality config that bypasses the schema', (models, message) => {
|
||||
expect(() => resolveAdapterOptions({ models: [...models] })).toThrow(message)
|
||||
})
|
||||
|
||||
it.each([0, 1.5])('rejects a per-model output cap of %s', (maxTokens) => {
|
||||
expect(() => resolveAdapterOptions({ models: [{ id: 'bad-cap', maxTokens }] }))
|
||||
.toThrow(/maxTokens must be a positive integer/)
|
||||
|
|
|
|||
|
|
@ -393,6 +393,95 @@ describe('image serialization', () => {
|
|||
])
|
||||
})
|
||||
|
||||
it('does not emit an empty user message for ignored content beside a tool result', async () => {
|
||||
const messages = [createUserMessage({
|
||||
content: [
|
||||
{ type: 'text', text: '' },
|
||||
{ type: 'chart', data: 'ignored' } as unknown as ContentBlock,
|
||||
{
|
||||
type: 'tool-result',
|
||||
toolCallId: CallId('result'),
|
||||
content: [{ type: 'text', text: 'ok' }],
|
||||
},
|
||||
],
|
||||
source: { kind: 'plugin', plugin: 'test' },
|
||||
})]
|
||||
|
||||
await expect(serializeMessagesWithImages(
|
||||
messages,
|
||||
attachmentStore(),
|
||||
new AbortController().signal,
|
||||
)).resolves.toEqual([
|
||||
{ role: 'tool', tool_call_id: 'result', content: 'ok' },
|
||||
])
|
||||
})
|
||||
|
||||
it('recursively converts nested tool-result content and preserves the empty fallback', async () => {
|
||||
const messages = [createUserMessage({
|
||||
content: [
|
||||
{
|
||||
type: 'tool-result',
|
||||
toolCallId: CallId('nested'),
|
||||
content: [{
|
||||
type: 'tool-result',
|
||||
toolCallId: CallId('inner'),
|
||||
content: [{ type: 'text', text: 'inside' }],
|
||||
}],
|
||||
},
|
||||
{ type: 'tool-result', toolCallId: CallId('empty'), content: [] },
|
||||
],
|
||||
source: { kind: 'plugin', plugin: 'test' },
|
||||
})]
|
||||
|
||||
await expect(serializeMessagesWithImages(
|
||||
messages,
|
||||
attachmentStore(),
|
||||
new AbortController().signal,
|
||||
)).resolves.toEqual([
|
||||
{ role: 'tool', tool_call_id: 'nested', content: 'inside' },
|
||||
{ role: 'tool', tool_call_id: 'empty', content: '(no output)' },
|
||||
])
|
||||
})
|
||||
|
||||
it('flushes tool-result images before system and assistant history', async () => {
|
||||
const imageResult = (id: string) => createUserMessage({
|
||||
content: [{
|
||||
type: 'tool-result',
|
||||
toolCallId: CallId(id),
|
||||
content: [{ type: 'image', attachment: imageRef() }],
|
||||
}],
|
||||
source: { kind: 'plugin' as const, plugin: 'test' },
|
||||
})
|
||||
const messages = [
|
||||
imageResult('before-system'),
|
||||
createMessage({
|
||||
role: 'system',
|
||||
content: [{ type: 'text', text: 'system history' }],
|
||||
source: { kind: 'plugin', plugin: 'test' },
|
||||
}),
|
||||
imageResult('before-assistant'),
|
||||
createMessage({
|
||||
role: 'assistant',
|
||||
content: [{ type: 'text', text: 'assistant history' }],
|
||||
source: { kind: 'plugin', plugin: 'test' },
|
||||
}),
|
||||
]
|
||||
|
||||
const wire = await serializeMessagesWithImages(
|
||||
messages,
|
||||
attachmentStore(),
|
||||
new AbortController().signal,
|
||||
)
|
||||
expect(wire).toEqual([
|
||||
{ role: 'tool', tool_call_id: 'before-system', content: '(see attached image)' },
|
||||
expect.objectContaining({ role: 'user' }),
|
||||
{ role: 'system', content: 'system history' },
|
||||
{ role: 'tool', tool_call_id: 'before-assistant', content: '(see attached image)' },
|
||||
expect.objectContaining({ role: 'user' }),
|
||||
{ role: 'assistant', content: 'assistant history' },
|
||||
])
|
||||
})
|
||||
|
||||
it('offloads oldest images before reads and keeps the newest image', async () => {
|
||||
const readImage = vi.fn((ref: ImageAttachmentRef) => Promise.resolve({
|
||||
ref,
|
||||
|
|
@ -435,6 +524,37 @@ describe('image serialization', () => {
|
|||
expect(readImage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects unsupported image history before request offloading can replace it', async () => {
|
||||
const readImage = vi.fn()
|
||||
await expect(serializeRequestWithImages(request({
|
||||
messages: [createMessage({
|
||||
role: 'system',
|
||||
content: [{ type: 'image', attachment: imageRef('image/png', 300) }],
|
||||
source: { kind: 'plugin', plugin: 'test' },
|
||||
})],
|
||||
}), {
|
||||
attachments: attachmentStore(readImage),
|
||||
maxRequestImageBytes: 1,
|
||||
signal: new AbortController().signal,
|
||||
})).rejects.toMatchObject({ code: 'UNSUPPORTED_CONTENT' })
|
||||
expect(readImage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('prepends the request system prompt on the image path', async () => {
|
||||
const wire = await serializeRequestWithImages(request({
|
||||
system: 'system prompt',
|
||||
messages: [createUserMessage({
|
||||
content: [{ type: 'image', attachment: imageRef() }],
|
||||
source: { kind: 'plugin', plugin: 'test' },
|
||||
})],
|
||||
}), {
|
||||
attachments: attachmentStore(),
|
||||
maxRequestImageBytes: 20 * 1024 * 1024,
|
||||
signal: new AbortController().signal,
|
||||
})
|
||||
expect(wire.messages[0]).toEqual({ role: 'system', content: 'system prompt' })
|
||||
})
|
||||
|
||||
it('preserves stable attachment failure codes', async () => {
|
||||
const readImage = vi.fn(() => Promise.reject(new AttachmentError(
|
||||
'Stored attachment bytes are corrupt.',
|
||||
|
|
@ -446,6 +566,15 @@ describe('image serialization', () => {
|
|||
})], attachmentStore(readImage), new AbortController().signal))
|
||||
.rejects.toMatchObject({ code: 'ATTACHMENT_CORRUPT' })
|
||||
})
|
||||
|
||||
it('preserves non-attachment resolver failures', async () => {
|
||||
const failure = new Error('resolver failed')
|
||||
const readImage = vi.fn(() => Promise.reject(failure))
|
||||
await expect(serializeMessagesWithImages([createUserMessage({
|
||||
content: [{ type: 'image', attachment: imageRef() }],
|
||||
source: { kind: 'plugin', plugin: 'test' },
|
||||
})], attachmentStore(readImage), new AbortController().signal)).rejects.toBe(failure)
|
||||
})
|
||||
})
|
||||
|
||||
describe('review fixes: assistant content shapes', () => {
|
||||
|
|
|
|||
|
|
@ -26,6 +26,18 @@ function toolResultText(blocks: readonly ContentBlock[]): string {
|
|||
: block.type === 'tool-result' ? toolResultText(block.content) : '').join('')
|
||||
}
|
||||
|
||||
/** Reject image roles that pi-ai cannot replay before request-size offloading can replace them. */
|
||||
function assertSupportedImageRoles(messages: readonly Message[]): void {
|
||||
for (const message of messages) {
|
||||
if (message.role !== 'user' && contentHasImage(message.content)) {
|
||||
throw new LlmError(
|
||||
`pi-ai cannot represent an image in an in-history ${message.role} message`,
|
||||
'UNSUPPORTED_CONTENT',
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async function userContent(
|
||||
blocks: readonly ContentBlock[],
|
||||
attachments: AttachmentStore,
|
||||
|
|
@ -169,15 +181,13 @@ async function toPiContextWithImages(
|
|||
onReplayDegrade?: (reason: string) => void,
|
||||
maxRequestImageBytes?: number,
|
||||
): Promise<PiContext> {
|
||||
assertSupportedImageRoles(options.messages)
|
||||
const requestMessages = offloadRequestImages(options.messages, maxRequestImageBytes)
|
||||
const toolNames = new Map<CallId, string>()
|
||||
const messages: PiMessage[] = []
|
||||
|
||||
for (const message of requestMessages) {
|
||||
if (message.role === 'system') {
|
||||
if (contentHasImage(message.content)) {
|
||||
throw new LlmError('pi-ai cannot represent an image in an in-history system message', 'UNSUPPORTED_CONTENT')
|
||||
}
|
||||
// pi-ai has a single systemPrompt slot; in-history system messages are
|
||||
// folded into user messages to preserve order (rare in practice — the
|
||||
// harness sends the system prompt via options.system).
|
||||
|
|
|
|||
|
|
@ -140,6 +140,59 @@ describe('pi-ai request context conversion', () => {
|
|||
])
|
||||
})
|
||||
|
||||
it('recursively converts nested tool-result text and images', async () => {
|
||||
const callId = CallId('nested-call')
|
||||
const context = await toPiContext(request([user([{
|
||||
type: 'tool-result',
|
||||
toolCallId: callId,
|
||||
content: [
|
||||
{
|
||||
type: 'tool-result',
|
||||
toolCallId: callId,
|
||||
content: [{ type: 'text', text: 'nested text' }],
|
||||
},
|
||||
{
|
||||
type: 'tool-result',
|
||||
toolCallId: callId,
|
||||
content: [{ type: 'image', attachment: ref }],
|
||||
},
|
||||
],
|
||||
}])]), attachments)
|
||||
|
||||
expect(context.messages).toEqual([{
|
||||
role: 'toolResult',
|
||||
toolCallId: 'nested-call',
|
||||
toolName: 'unknown',
|
||||
content: [
|
||||
{ type: 'text', text: 'nested text' },
|
||||
{ type: 'image', data: 'AQ==', mimeType: 'image/png' },
|
||||
],
|
||||
isError: false,
|
||||
timestamp: 0,
|
||||
}])
|
||||
})
|
||||
|
||||
it('flattens nested text-only tool results and ignores other block types without storage', () => {
|
||||
const callId = CallId('nested-text')
|
||||
expect(toPiContext(request([user([{
|
||||
type: 'tool-result',
|
||||
toolCallId: callId,
|
||||
content: [
|
||||
{ type: 'chart', data: 'ignored' } as unknown as ContentBlock,
|
||||
{
|
||||
type: 'tool-result',
|
||||
toolCallId: callId,
|
||||
content: [{ type: 'text', text: 'nested' }],
|
||||
},
|
||||
],
|
||||
}])]))).toMatchObject({
|
||||
messages: [{
|
||||
role: 'toolResult',
|
||||
content: [{ type: 'text', text: 'nested' }],
|
||||
}],
|
||||
})
|
||||
})
|
||||
|
||||
it('replaces the oldest images with placeholders once the request payload bound is exceeded', async () => {
|
||||
const readImage = vi.fn(() => Promise.resolve({ ref: { ...ref, bytes: 3 }, data: Uint8Array.of(1, 2, 3) }))
|
||||
const store = { readImage } as unknown as AttachmentStore
|
||||
|
|
@ -249,9 +302,14 @@ describe('pi-ai request context conversion', () => {
|
|||
})
|
||||
|
||||
it('handles in-history system and assistant messages explicitly on the image path', async () => {
|
||||
await expect(toPiContext(request([
|
||||
history('system', [{ type: 'image', attachment: ref }]),
|
||||
]), attachments)).rejects.toMatchObject({ code: 'UNSUPPORTED_CONTENT' })
|
||||
for (const role of ['system', 'assistant'] as const) {
|
||||
const readImage = vi.fn()
|
||||
const store = { readImage } as unknown as AttachmentStore
|
||||
await expect(toPiContext(request([
|
||||
history(role, [{ type: 'image', attachment: ref }]),
|
||||
]), store, undefined, 1)).rejects.toMatchObject({ code: 'UNSUPPORTED_CONTENT' })
|
||||
expect(readImage).not.toHaveBeenCalled()
|
||||
}
|
||||
|
||||
await expect(toPiContext(request([
|
||||
history('system', [{ type: 'text', text: 'history system' }]),
|
||||
|
|
|
|||
|
|
@ -73,4 +73,17 @@ describe('offloadRequestImages', () => {
|
|||
expect(offloadRequestImages(messages, 8)[0]?.content)
|
||||
.toEqual([{ type: 'text', text: OFFLOADED_IMAGE_TEXT }])
|
||||
})
|
||||
|
||||
it('keeps unchanged nested content while replacing a later image', () => {
|
||||
const nested = {
|
||||
type: 'tool-result' as const,
|
||||
toolCallId: CallId('text-only'),
|
||||
content: [{ type: 'text' as const, text: 'kept' }],
|
||||
}
|
||||
const messages = [createUserMessage({ content: [nested, image(3)], source })]
|
||||
expect(offloadRequestImages(messages, 1)[0]?.content).toEqual([
|
||||
nested,
|
||||
{ type: 'text', text: OFFLOADED_IMAGE_TEXT },
|
||||
])
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue