apunkt/plano
1
0
Fork 0
mirror of https://github.com/katanemo/plano.git synced 2026-06-17 15:25:17 +02:00
Code Issues Projects Releases Packages Wiki Activity

resolving PR comments

Browse source
This commit is contained in:
Salman Paracha 2025-12-16 21:55:24 -08:00
parent 6aa37af2a6
commit edb5cb58c5
10 changed files with 159 additions and 112 deletions
Download patch file Download diff file Expand all files Collapse all files

2
arch/arch_config_schema.yaml
View file

@ -331,7 +331,7 @@ properties:
model:
type: string
additionalProperties: false
state_storage_v1_responses:
state_storage:
type: object
properties:
type:

4
arch/tools/cli/utils.py
View file

@ -161,6 +161,10 @@ def get_llm_provider_access_keys(arch_config_file):
matches = re.findall(pattern, connection_string)
for var in matches:
access_key_list.append(f"${var}")
else:
raise ValueError(
"Invalid connection string received in state_storage_v1_responses"
)
return access_key_list

2
crates/brightstaff/src/main.rs
View file

@ -108,7 +108,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
// Configurable via arch_config.yaml state_storage section
// If not configured, state management is disabled
// Environment variables are substituted by envsubst before config is read
let state_storage: Option<Arc<dyn StateStorage>> = if let Some(storage_config) = &arch_config.state_storage_v1_responses {
let state_storage: Option<Arc<dyn StateStorage>> = if let Some(storage_config) = &arch_config.state_storage {
let storage: Arc<dyn StateStorage> = match storage_config.storage_type {
common::configuration::StateStorageType::Memory => {
info!("Initialized conversation state storage: Memory");

189
crates/brightstaff/src/state/memory.rs
View file

@ -47,14 +47,14 @@ impl StateStorage for MemoryConversationalStorage {
match storage.get(response_id) {
Some(state) => {
debug!(
"[PLANO | BRIGHTSTAFF | MEMORY_STORAGE] RESP_ID:{} | Retrieved conversation state: input_items={}",
"[PLANO | MEMORY_STORAGE | RESP_ID:{} | Retrieved conversation state: input_items={}",
response_id, state.input_items.len()
);
Ok(state.clone())
}
None => {
warn!(
"[PLANO | BRIGHTSTAFF | MEMORY_STORAGE] RESP_ID:{} | Conversation state not found",
"[PLANO_RESP_ID:{} | MEMORY_STORAGE | Conversation state not found",
response_id
);
Err(StateStorageError::NotFound(response_id.to_string()))
@ -85,16 +85,16 @@ impl StateStorage for MemoryConversationalStorage {
#[cfg(test)]
mod tests {
use super::*;
use hermesllm::apis::openai_responses::{InputItem, InputMessage, MessageRole, InputContent};
use hermesllm::apis::openai_responses::{InputItem, InputMessage, MessageRole, InputContent, MessageContent};
fn create_test_state(response_id: &str, num_messages: usize) -> OpenAIConversationState {
let mut input_items = Vec::new();
for i in 0..num_messages {
input_items.push(InputItem::Message(InputMessage {
role: if i % 2 == 0 { MessageRole::User } else { MessageRole::Assistant },
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: format!("Message {}", i),
}],
}]),
}));
}
@ -222,9 +222,9 @@ mod tests {
// Create current input with 1 message
let current_input = vec![InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "New message".to_string(),
}],
}]),
})];
// Merge
@ -244,24 +244,30 @@ mod tests {
// Current input has message 2
let current_input = vec![InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Message 2".to_string(),
}],
}]),
})];
let merged = storage.merge(&prev_state, current_input);
// Verify order: prev messages first, then current
let InputItem::Message(msg) = &merged[0];
match &msg.content[0] {
InputContent::InputText { text } => assert_eq!(text, "Message 0"),
_ => panic!("Expected InputText"),
let InputItem::Message(msg) = &merged[0] else { panic!("Expected Message") };
match &msg.content {
MessageContent::Items(items) => match &items[0] {
InputContent::InputText { text } => assert_eq!(text, "Message 0"),
_ => panic!("Expected InputText"),
},
_ => panic!("Expected MessageContent::Items"),
}
let InputItem::Message(msg) = &merged[2];
match &msg.content[0] {
InputContent::InputText { text } => assert_eq!(text, "Message 2"),
_ => panic!("Expected InputText"),
let InputItem::Message(msg) = &merged[2] else { panic!("Expected Message") };
match &msg.content {
MessageContent::Items(items) => match &items[0] {
InputContent::InputText { text } => assert_eq!(text, "Message 2"),
_ => panic!("Expected InputText"),
},
_ => panic!("Expected MessageContent::Items"),
}
}
@ -290,9 +296,9 @@ mod tests {
let current_input = vec![InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Only message".to_string(),
}],
}]),
})];
let merged = storage.merge(&prev_state, current_input);
@ -377,16 +383,16 @@ mod tests {
// Original user message
InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "What's the weather in San Francisco?".to_string(),
}],
}]),
}),
// Assistant's function call (converted from OutputItem::FunctionCall)
InputItem::Message(InputMessage {
role: MessageRole::Assistant,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Called function: get_weather with arguments: {\"location\":\"San Francisco, CA\"}".to_string(),
}],
}]),
}),
],
created_at: 1234567890,
@ -397,9 +403,9 @@ mod tests {
// Step 2: Current request includes function call output
let current_input = vec![InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Function result: {\"temperature\": 72, \"condition\": \"sunny\"}".to_string(),
}],
}]),
})];
// Step 3: Merge should combine all conversation history
@ -409,32 +415,41 @@ mod tests {
assert_eq!(merged.len(), 3);
// Verify the order and content
let InputItem::Message(msg1) = &merged[0];
let InputItem::Message(msg1) = &merged[0] else { panic!("Expected Message") };
assert!(matches!(msg1.role, MessageRole::User));
match &msg1.content[0] {
InputContent::InputText { text } => {
assert!(text.contains("weather in San Francisco"));
}
_ => panic!("Expected InputText"),
match &msg1.content {
MessageContent::Items(items) => match &items[0] {
InputContent::InputText { text } => {
assert!(text.contains("weather in San Francisco"));
}
_ => panic!("Expected InputText"),
},
_ => panic!("Expected MessageContent::Items"),
}
let InputItem::Message(msg2) = &merged[1];
let InputItem::Message(msg2) = &merged[1] else { panic!("Expected Message") };
assert!(matches!(msg2.role, MessageRole::Assistant));
match &msg2.content[0] {
InputContent::InputText { text } => {
assert!(text.contains("get_weather"));
}
_ => panic!("Expected InputText"),
match &msg2.content {
MessageContent::Items(items) => match &items[0] {
InputContent::InputText { text } => {
assert!(text.contains("get_weather"));
}
_ => panic!("Expected InputText"),
},
_ => panic!("Expected MessageContent::Items"),
}
let InputItem::Message(msg3) = &merged[2];
let InputItem::Message(msg3) = &merged[2] else { panic!("Expected Message") };
assert!(matches!(msg3.role, MessageRole::User));
match &msg3.content[0] {
InputContent::InputText { text } => {
assert!(text.contains("Function result"));
assert!(text.contains("temperature"));
}
_ => panic!("Expected InputText"),
match &msg3.content {
MessageContent::Items(items) => match &items[0] {
InputContent::InputText { text } => {
assert!(text.contains("Function result"));
assert!(text.contains("temperature"));
}
_ => panic!("Expected InputText"),
},
_ => panic!("Expected MessageContent::Items"),
}
}
@ -449,21 +464,21 @@ mod tests {
input_items: vec![
InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "What's the weather and time in SF?".to_string(),
}],
}]),
}),
InputItem::Message(InputMessage {
role: MessageRole::Assistant,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Called function: get_weather with arguments: {\"location\":\"SF\"}".to_string(),
}],
}]),
}),
InputItem::Message(InputMessage {
role: MessageRole::Assistant,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Called function: get_time with arguments: {\"timezone\":\"America/Los_Angeles\"}".to_string(),
}],
}]),
}),
],
created_at: 1234567890,
@ -475,15 +490,15 @@ mod tests {
let current_input = vec![
InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Weather result: {\"temp\": 68}".to_string(),
}],
}]),
}),
InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Time result: {\"time\": \"14:30\"}".to_string(),
}],
}]),
}),
];
@ -493,22 +508,28 @@ mod tests {
assert_eq!(merged.len(), 5);
// Verify first item is original user message
let InputItem::Message(first) = &merged[0];
let InputItem::Message(first) = &merged[0] else { panic!("Expected Message") };
assert!(matches!(first.role, MessageRole::User));
// Verify last two are function outputs
let InputItem::Message(second_last) = &merged[3];
let InputItem::Message(second_last) = &merged[3] else { panic!("Expected Message") };
assert!(matches!(second_last.role, MessageRole::User));
match &second_last.content[0] {
InputContent::InputText { text } => assert!(text.contains("Weather result")),
_ => panic!("Expected InputText"),
match &second_last.content {
MessageContent::Items(items) => match &items[0] {
InputContent::InputText { text } => assert!(text.contains("Weather result")),
_ => panic!("Expected InputText"),
},
_ => panic!("Expected MessageContent::Items"),
}
let InputItem::Message(last) = &merged[4];
let InputItem::Message(last) = &merged[4] else { panic!("Expected Message") };
assert!(matches!(last.role, MessageRole::User));
match &last.content[0] {
InputContent::InputText { text } => assert!(text.contains("Time result")),
_ => panic!("Expected InputText"),
match &last.content {
MessageContent::Items(items) => match &items[0] {
InputContent::InputText { text } => assert!(text.contains("Time result")),
_ => panic!("Expected InputText"),
},
_ => panic!("Expected MessageContent::Items"),
}
}
@ -524,30 +545,30 @@ mod tests {
// Turn 1: User asks about weather
InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "What's the weather?".to_string(),
}],
}]),
}),
// Turn 1: Assistant calls get_weather
InputItem::Message(InputMessage {
role: MessageRole::Assistant,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Called function: get_weather".to_string(),
}],
}]),
}),
// Turn 2: User provides function output
InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Weather: sunny, 72°F".to_string(),
}],
}]),
}),
// Turn 2: Assistant responds with text
InputItem::Message(InputMessage {
role: MessageRole::Assistant,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "It's sunny and 72°F in San Francisco today!".to_string(),
}],
}]),
}),
],
created_at: 1234567890,
@ -558,9 +579,9 @@ mod tests {
// Turn 3: User asks follow-up question
let current_input = vec![InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Should I bring an umbrella?".to_string(),
}],
}]),
})];
let merged = storage.merge(&prev_state, current_input);
@ -569,16 +590,22 @@ mod tests {
assert_eq!(merged.len(), 5);
// Verify the entire conversation flow is preserved
let InputItem::Message(first) = &merged[0];
match &first.content[0] {
InputContent::InputText { text } => assert!(text.contains("What's the weather")),
_ => panic!("Expected InputText"),
let InputItem::Message(first) = &merged[0] else { panic!("Expected Message") };
match &first.content {
MessageContent::Items(items) => match &items[0] {
InputContent::InputText { text } => assert!(text.contains("What's the weather")),
_ => panic!("Expected InputText"),
},
_ => panic!("Expected MessageContent::Items"),
}
let InputItem::Message(last) = &merged[4];
match &last.content[0] {
InputContent::InputText { text } => assert!(text.contains("umbrella")),
_ => panic!("Expected InputText"),
let InputItem::Message(last) = &merged[4] else { panic!("Expected Message") };
match &last.content {
MessageContent::Items(items) => match &items[0] {
InputContent::InputText { text } => assert!(text.contains("umbrella")),
_ => panic!("Expected InputText"),
},
_ => panic!("Expected MessageContent::Items"),
}
}
}

6
crates/brightstaff/src/state/mod.rs
View file

@ -1,5 +1,5 @@
use async_trait::async_trait;
use hermesllm::apis::openai_responses::{InputItem, InputMessage, InputContent, MessageRole, InputParam};
use hermesllm::apis::openai_responses::{InputItem, InputMessage, InputContent, MessageContent, MessageRole, InputParam};
use serde::{Deserialize, Serialize};
use std::error::Error;
use std::fmt;
@ -123,9 +123,9 @@ pub fn extract_input_items(input: &InputParam) -> Vec<InputItem> {
InputParam::Text(text) => {
vec![InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: text.clone(),
}],
}]),
})]
}
InputParam::Items(items) => items.clone(),

14
crates/brightstaff/src/state/postgresql.rs
View file

@ -230,16 +230,16 @@ Run that SQL file against your database before using this storage backend.
#[cfg(test)]
mod tests {
use super::*;
use hermesllm::apis::openai_responses::{InputContent, InputItem, InputMessage, MessageRole};
use hermesllm::apis::openai_responses::{InputContent, InputItem, InputMessage, MessageContent, MessageRole};
fn create_test_state(response_id: &str) -> OpenAIConversationState {
OpenAIConversationState {
response_id: response_id.to_string(),
input_items: vec![InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Test message".to_string(),
}],
}]),
})],
created_at: 1234567890,
model: "gpt-4".to_string(),
@ -298,9 +298,9 @@ mod tests {
state2.model = "gpt-4-turbo".to_string();
state2.input_items.push(InputItem::Message(InputMessage {
role: MessageRole::Assistant,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "Response".to_string(),
}],
}]),
}));
storage.put(state2).await.unwrap();
@ -384,9 +384,9 @@ mod tests {
let prev_state = create_test_state("test_resp_005");
let current_input = vec![InputItem::Message(InputMessage {
role: MessageRole::User,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: "New message".to_string(),
}],
}]),
})];
let merged = storage.merge(&prev_state, current_input);

2
crates/common/src/configuration.rs
View file

@ -72,7 +72,7 @@ pub struct Configuration {
pub routing: Option<Routing>,
pub agents: Option<Vec<Agent>>,
pub listeners: Vec<Listener>,
pub state_storage_v1_responses: Option<StateStorageConfig>,
pub state_storage: Option<StateStorageConfig>,
}
#[derive(Debug, Clone, Serialize, Deserialize, Default)]

44
crates/hermesllm/src/transforms/response/output_to_input.rs
View file

@ -5,12 +5,12 @@
//! for maintaining conversation history in the v1/responses API.
use crate::apis::openai_responses::{
InputContent, InputItem, InputMessage, MessageRole, OutputContent, OutputItem,
InputContent, InputItem, InputMessage, MessageContent, MessageRole, OutputContent, OutputItem,
};
/// Converts an OutputItem from a response into an InputItem for the next request
/// This is used to build conversation history from previous responses
pub fn output_item_to_input_item(output: &OutputItem) -> Option<InputItem> {
pub fn convert_responses_output_to_input_items(output: &OutputItem) -> Option<InputItem> {
match output {
// Convert output messages to input messages
OutputItem::Message {
@ -47,7 +47,7 @@ pub fn output_item_to_input_item(output: &OutputItem) -> Option<InputItem> {
Some(InputItem::Message(InputMessage {
role: message_role,
content: input_content,
content: MessageContent::Items(input_content),
}))
}
// For function calls, we'll create an assistant message with the tool call info
@ -63,9 +63,9 @@ pub fn output_item_to_input_item(output: &OutputItem) -> Option<InputItem> {
Some(InputItem::Message(InputMessage {
role: MessageRole::Assistant,
content: vec![InputContent::InputText {
content: MessageContent::Items(vec![InputContent::InputText {
text: tool_call_text,
}],
}]),
}))
}
// Skip other output types (tool outputs, etc.) as they don't convert to input
@ -77,7 +77,7 @@ pub fn output_item_to_input_item(output: &OutputItem) -> Option<InputItem> {
pub fn outputs_to_inputs(outputs: &[OutputItem]) -> Vec<InputItem> {
outputs
.iter()
.filter_map(output_item_to_input_item)
.filter_map(convert_responses_output_to_input_items)
.collect()
}
@ -99,17 +99,23 @@ mod tests {
}],
};
let input = output_item_to_input_item(&output).unwrap();
let input = convert_responses_output_to_input_items(&output).unwrap();
match input {
InputItem::Message(msg) => {
assert!(matches!(msg.role, MessageRole::Assistant));
assert_eq!(msg.content.len(), 1);
match &msg.content[0] {
InputContent::InputText { text } => assert_eq!(text, "Hello!"),
_ => panic!("Expected InputText"),
match &msg.content {
MessageContent::Items(items) => {
assert_eq!(items.len(), 1);
match &items[0] {
InputContent::InputText { text } => assert_eq!(text, "Hello!"),
_ => panic!("Expected InputText"),
}
}
_ => panic!("Expected MessageContent::Items"),
}
}
_ => panic!("Expected Message variant"),
}
}
@ -123,18 +129,24 @@ mod tests {
arguments: Some(r#"{"location":"SF"}"#.to_string()),
};
let input = output_item_to_input_item(&output).unwrap();
let input = convert_responses_output_to_input_items(&output).unwrap();
match input {
InputItem::Message(msg) => {
assert!(matches!(msg.role, MessageRole::Assistant));
match &msg.content[0] {
InputContent::InputText { text } => {
assert!(text.contains("get_weather"));
match &msg.content {
MessageContent::Items(items) => {
match &items[0] {
InputContent::InputText { text } => {
assert!(text.contains("get_weather"));
}
_ => panic!("Expected InputText"),
}
}
_ => panic!("Expected InputText"),
_ => panic!("Expected MessageContent::Items"),
}
}
_ => panic!("Expected Message variant"),
}
}

6
crates/hermesllm/src/transforms/response/to_openai.rs
View file

@ -80,9 +80,13 @@ impl TryFrom<ChatCompletionsResponse> for ResponsesAPIResponse {
// Only add the message item if there's actual content (text, audio, or refusal)
// Don't add empty message items when there are only tool calls
if !content.is_empty() {
// Avoid double-prefixing: if ID already starts with "msg_", use as-is
// Generate message ID: strip common prefixes to avoid double-prefixing
let message_id = if resp.id.starts_with("msg_") {
resp.id.clone()
} else if resp.id.starts_with("resp_") {
format!("msg_{}", &resp.id[5..]) // Strip "resp_" prefix
} else if resp.id.starts_with("chatcmpl-") {
format!("msg_{}", &resp.id[9..]) // Strip "chatcmpl-" prefix
} else {
format!("msg_{}", resp.id)
};

2
tests/e2e/arch_config_memory_state_v1_responses.yaml
View file

@ -20,6 +20,6 @@ llm_providers:
# State storage configuration for v1/responses API
# Manages conversation state for multi-turn conversations
state_storage_v1_responses:
state_storage:
# Type: memory | postgres
type: memory