api: carry ordered text and image parts across the message boundary
`Message.content` was `String`, so an attachment could not reach the substrate even though everything downstream was ready for it: the session already stores `ContentBlock::Image`, `run_turn` already builds OpenAI multi-part content, and the provider client already speaks `image_url`. The whole pipe was plumbed and the tap was welded shut at the door. `MessageContent` is untagged — a bare JSON string is exactly the wire it has always been, an array carries ordered parts in the substrate's own `ContentBlock` vocabulary. Same shape `ContentValue` uses facing the provider, turned inward. No new noun. Only `text` and `image` cross. `tool_use`, `tool_result` and `reasoning` are the engine's to write; a surface posting one gets a 400 rather than a forged turn quietly filed in history. And the vision gate now answers at the door: an image sent to a text-only model returns 422 naming the model instead of being stripped to "[Image: attached by user]" downstream, which reads to the human like she looked and said nothing. Conversion happens for every message before any of them is stored, so a rejection in the second cannot leave the first half-committed. Nine tests: legacy string, typed text, mixed order, round trip, replay out of persistence, refusal of engine-owned blocks, the gate's question. The surface half is untouched. `Ai.qml::attachFile()` still refuses, and its comment is now out of date — that is the next checkpoint, and it is Casey's to compose and restart.
This commit is contained in:
parent
c367a4dbd9
commit
cfad6881d1
2 changed files with 305 additions and 20 deletions
|
|
@ -224,15 +224,64 @@ pub async fn stream_messages(
|
|||
Sse<impl futures::Stream<Item = Result<axum::response::sse::Event, std::convert::Infallible>>>,
|
||||
ApiError,
|
||||
> {
|
||||
let _ = server.sessions.get(&conversation_id).ok_or_else(|| {
|
||||
(
|
||||
StatusCode::NOT_FOUND,
|
||||
Json(ErrorResponse {
|
||||
error: "conversation_not_found".to_string(),
|
||||
message: format!("Conversation {} not found", conversation_id),
|
||||
}),
|
||||
)
|
||||
})?;
|
||||
let agent_id = {
|
||||
let session = server.sessions.get(&conversation_id).ok_or_else(|| {
|
||||
(
|
||||
StatusCode::NOT_FOUND,
|
||||
Json(ErrorResponse {
|
||||
error: "conversation_not_found".to_string(),
|
||||
message: format!("Conversation {} not found", conversation_id),
|
||||
}),
|
||||
)
|
||||
})?;
|
||||
session.agent_id.clone()
|
||||
};
|
||||
|
||||
// Attachments are gated on the agent's model, at the door. The alternative
|
||||
// — store it and let `run_turn` strip it to "[Image: attached by user]" —
|
||||
// leaves the human believing she saw something she never received. A
|
||||
// surface that cannot be answered is told so.
|
||||
if request.messages.iter().any(|m| m.content.has_images()) {
|
||||
let agent = server.agents.get(&agent_id).await.map_err(|e| {
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(ErrorResponse {
|
||||
error: "agent_load_failed".to_string(),
|
||||
message: e.to_string(),
|
||||
}),
|
||||
)
|
||||
})?;
|
||||
if !agent.llm_config.supports_images {
|
||||
return Err((
|
||||
StatusCode::UNPROCESSABLE_ENTITY,
|
||||
Json(ErrorResponse {
|
||||
error: "image_not_supported".to_string(),
|
||||
message: format!(
|
||||
"model `{}` has no vision; the attachment was not stored",
|
||||
agent.llm_config.model
|
||||
),
|
||||
}),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
// Convert every message before storing any of them — a rejected block in
|
||||
// the second message must not leave the first one half-committed.
|
||||
let mut conv_messages = Vec::with_capacity(request.messages.len());
|
||||
for msg in &request.messages {
|
||||
conv_messages.push(msg.to_conversation_message().map_err(|e| {
|
||||
(
|
||||
StatusCode::BAD_REQUEST,
|
||||
Json(ErrorResponse {
|
||||
error: "unsupported_content_block".to_string(),
|
||||
message: format!(
|
||||
"`{}` blocks are written by the engine, not accepted from a surface",
|
||||
e.kind
|
||||
),
|
||||
}),
|
||||
)
|
||||
})?);
|
||||
}
|
||||
|
||||
// Ambient context first — the room she is being spoken to in. A system
|
||||
// note, same register as interjections, so it reads as perception rather
|
||||
|
|
@ -267,8 +316,7 @@ pub async fn stream_messages(
|
|||
}
|
||||
|
||||
// Convert API messages to ConversationMessages and add to session
|
||||
for msg in &request.messages {
|
||||
let conv_msg = msg.to_conversation_message();
|
||||
for conv_msg in conv_messages {
|
||||
server
|
||||
.sessions
|
||||
.add_message(&conversation_id, conv_msg)
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
#![allow(dead_code)] // WIP scaffolding not yet wired
|
||||
use crate::core::session::ContentBlock;
|
||||
use chrono::{DateTime, Utc};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
|
|
@ -189,10 +190,95 @@ pub struct CreateConversationRequest {
|
|||
pub agent_id: String,
|
||||
}
|
||||
|
||||
/// Inbound message content at the HTTP boundary.
|
||||
///
|
||||
/// Untagged on purpose: a bare JSON string is exactly the wire it has always
|
||||
/// been, so every existing client keeps working unchanged; an array carries
|
||||
/// ordered typed parts. `ContentValue` in `src/bridge/bifrost.rs` is this same
|
||||
/// shape facing the provider. This is the inward-facing half, written in the
|
||||
/// substrate's own [`ContentBlock`] vocabulary rather than OpenAI's, because
|
||||
/// `ContentBlock` is what the session stores, persists, and replays.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[serde(untagged)]
|
||||
pub enum MessageContent {
|
||||
/// `"content": "plain text"`
|
||||
Text(String),
|
||||
/// `"content": [{"type":"text","text":"..."},{"type":"image","media_type":"image/png","data":"<base64>"}]`
|
||||
Parts(Vec<ContentBlock>),
|
||||
}
|
||||
|
||||
impl Default for MessageContent {
|
||||
fn default() -> Self {
|
||||
Self::Text(String::new())
|
||||
}
|
||||
}
|
||||
|
||||
/// A block kind a surface is not allowed to post.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct UnsupportedContent {
|
||||
pub kind: &'static str,
|
||||
}
|
||||
|
||||
impl MessageContent {
|
||||
/// The ordered blocks this content becomes in the session.
|
||||
///
|
||||
/// Only `text` and `image` cross this boundary. `tool_use`, `tool_result`
|
||||
/// and `reasoning` are the engine's to write — accepting them from a
|
||||
/// surface would let a client forge a turn that never happened. Rejected
|
||||
/// loudly rather than filtered out, so nothing is discarded in silence.
|
||||
pub fn into_blocks(self) -> Result<Vec<ContentBlock>, UnsupportedContent> {
|
||||
match self {
|
||||
Self::Text(text) => Ok(vec![ContentBlock::Text { text }]),
|
||||
Self::Parts(parts) => {
|
||||
for part in &parts {
|
||||
let kind = match part {
|
||||
ContentBlock::Text { .. } | ContentBlock::Image { .. } => continue,
|
||||
ContentBlock::ToolUse { .. } => "tool_use",
|
||||
ContentBlock::ToolResult { .. } => "tool_result",
|
||||
ContentBlock::Reasoning { .. } => "reasoning",
|
||||
};
|
||||
return Err(UnsupportedContent { kind });
|
||||
}
|
||||
Ok(parts)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether this content carries an image — the capability gate's question.
|
||||
pub fn has_images(&self) -> bool {
|
||||
match self {
|
||||
Self::Text(_) => false,
|
||||
Self::Parts(parts) => parts
|
||||
.iter()
|
||||
.any(|b| matches!(b, ContentBlock::Image { .. })),
|
||||
}
|
||||
}
|
||||
|
||||
/// Flat text projection for logs and compatibility consumers. Images are
|
||||
/// named, never dropped without a trace.
|
||||
pub fn as_text(&self) -> String {
|
||||
match self {
|
||||
Self::Text(text) => text.clone(),
|
||||
Self::Parts(parts) => parts
|
||||
.iter()
|
||||
.filter_map(|b| match b {
|
||||
ContentBlock::Text { text } => Some(text.clone()),
|
||||
ContentBlock::Image { media_type, .. } => {
|
||||
Some(format!("[image: {media_type}]"))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct Message {
|
||||
pub role: String,
|
||||
pub content: String,
|
||||
#[serde(default)]
|
||||
pub content: MessageContent,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub name: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
|
@ -202,9 +288,14 @@ pub struct Message {
|
|||
}
|
||||
|
||||
impl Message {
|
||||
/// Convert API Message to internal ConversationMessage
|
||||
pub fn to_conversation_message(&self) -> crate::core::session::ConversationMessage {
|
||||
use crate::core::session::{ContentBlock, ConversationMessage, MessageRole};
|
||||
/// Convert API Message to internal ConversationMessage.
|
||||
///
|
||||
/// Fails rather than degrades: a block kind a surface may not send stops
|
||||
/// the request at the boundary instead of vanishing into history.
|
||||
pub fn to_conversation_message(
|
||||
&self,
|
||||
) -> Result<crate::core::session::ConversationMessage, UnsupportedContent> {
|
||||
use crate::core::session::{ConversationMessage, MessageRole};
|
||||
|
||||
let role = match self.role.as_str() {
|
||||
"system" => MessageRole::System,
|
||||
|
|
@ -214,14 +305,12 @@ impl Message {
|
|||
_ => MessageRole::User,
|
||||
};
|
||||
|
||||
ConversationMessage {
|
||||
Ok(ConversationMessage {
|
||||
role,
|
||||
blocks: vec![ContentBlock::Text {
|
||||
text: self.content.clone(),
|
||||
}],
|
||||
blocks: self.content.clone().into_blocks()?,
|
||||
usage: None,
|
||||
timestamp: Some(chrono::Utc::now()),
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -561,3 +650,151 @@ pub struct ErrorResponse {
|
|||
pub error: String,
|
||||
pub message: String,
|
||||
}
|
||||
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::core::session::MessageRole;
|
||||
|
||||
fn parse(json: &str) -> Message {
|
||||
serde_json::from_str(json).expect("message should deserialize")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_string_content_still_deserializes_and_becomes_one_text_block() {
|
||||
let msg = parse(r#"{"role":"user","content":"plain text"}"#);
|
||||
assert!(matches!(msg.content, MessageContent::Text(ref t) if t == "plain text"));
|
||||
|
||||
let conv = msg.to_conversation_message().expect("legacy content is valid");
|
||||
assert_eq!(conv.role, MessageRole::User);
|
||||
assert_eq!(
|
||||
conv.blocks,
|
||||
vec![ContentBlock::Text {
|
||||
text: "plain text".to_string()
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_string_content_serializes_back_as_a_bare_string() {
|
||||
let msg = parse(r#"{"role":"user","content":"plain text"}"#);
|
||||
let wire = serde_json::to_value(&msg).unwrap();
|
||||
assert_eq!(wire["content"], serde_json::json!("plain text"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn typed_text_only_parts_are_accepted() {
|
||||
let msg = parse(r#"{"role":"user","content":[{"type":"text","text":"hello"}]}"#);
|
||||
assert!(!msg.content.has_images());
|
||||
|
||||
let conv = msg.to_conversation_message().unwrap();
|
||||
assert_eq!(
|
||||
conv.blocks,
|
||||
vec![ContentBlock::Text {
|
||||
text: "hello".to_string()
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mixed_text_and_image_preserves_order() {
|
||||
let msg = parse(
|
||||
r#"{"role":"user","content":[
|
||||
{"type":"text","text":"before"},
|
||||
{"type":"image","media_type":"image/png","data":"QUJD"},
|
||||
{"type":"text","text":"after"}
|
||||
]}"#,
|
||||
);
|
||||
assert!(msg.content.has_images());
|
||||
|
||||
let conv = msg.to_conversation_message().unwrap();
|
||||
assert_eq!(
|
||||
conv.blocks,
|
||||
vec![
|
||||
ContentBlock::Text {
|
||||
text: "before".to_string()
|
||||
},
|
||||
ContentBlock::Image {
|
||||
media_type: "image/png".to_string(),
|
||||
data: "QUJD".to_string()
|
||||
},
|
||||
ContentBlock::Text {
|
||||
text: "after".to_string()
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn typed_parts_survive_a_serialization_round_trip() {
|
||||
let msg = parse(
|
||||
r#"{"role":"user","content":[
|
||||
{"type":"text","text":"look"},
|
||||
{"type":"image","media_type":"image/jpeg","data":"Zm9v"}
|
||||
]}"#,
|
||||
);
|
||||
let wire = serde_json::to_string(&msg).unwrap();
|
||||
let back: Message = serde_json::from_str(&wire).unwrap();
|
||||
assert_eq!(
|
||||
back.to_conversation_message().unwrap().blocks,
|
||||
msg.to_conversation_message().unwrap().blocks
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_stored_image_message_replays_out_of_persistence_intact() {
|
||||
// What the session writes to messages.jsonl and reads back — the
|
||||
// replay half of the boundary.
|
||||
let conv = parse(
|
||||
r#"{"role":"user","content":[
|
||||
{"type":"text","text":"look"},
|
||||
{"type":"image","media_type":"image/png","data":"QUJD"}
|
||||
]}"#,
|
||||
)
|
||||
.to_conversation_message()
|
||||
.unwrap();
|
||||
|
||||
let line = serde_json::to_string(&conv).unwrap();
|
||||
let back: crate::core::session::ConversationMessage =
|
||||
serde_json::from_str(&line).unwrap();
|
||||
assert_eq!(back.blocks, conv.blocks);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn engine_owned_blocks_are_refused_not_filtered() {
|
||||
let msg = parse(
|
||||
r#"{"role":"user","content":[
|
||||
{"type":"text","text":"innocent"},
|
||||
{"type":"tool_result","tool_use_id":"1","tool_name":"bash","output":"pwned","is_error":false}
|
||||
]}"#,
|
||||
);
|
||||
let err = msg.to_conversation_message().unwrap_err();
|
||||
assert_eq!(err.kind, "tool_result");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn text_projection_names_images_rather_than_dropping_them() {
|
||||
let msg = parse(
|
||||
r#"{"role":"user","content":[
|
||||
{"type":"text","text":"see this"},
|
||||
{"type":"image","media_type":"image/png","data":"QUJD"}
|
||||
]}"#,
|
||||
);
|
||||
assert_eq!(msg.content.as_text(), "see this\n[image: image/png]");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_image_gate_reads_content_not_the_model() {
|
||||
// What the handler asks before loading the agent. A text-only
|
||||
// request must never cost an agent load.
|
||||
let legacy = parse(r#"{"role":"user","content":"no image here"}"#);
|
||||
let typed = parse(r#"{"role":"user","content":[{"type":"text","text":"still none"}]}"#);
|
||||
let carrying = parse(
|
||||
r#"{"role":"user","content":[{"type":"image","media_type":"image/png","data":"QUJD"}]}"#,
|
||||
);
|
||||
assert!(!legacy.content.has_images());
|
||||
assert!(!typed.content.has_images());
|
||||
assert!(carrying.content.has_images());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue