LCORE-354 Implement the /conversations endpoint #244
Conversation
|
|
""" WalkthroughA new REST API module for managing conversation history was introduced, providing endpoints to retrieve and delete conversations by ID. Supporting response models were added, and the router was registered in the main application. Conversation-to-agent mapping is now maintained in query and streaming endpoints. Associated tests were updated for router inclusion. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FastAPI
participant Auth
participant ConversationRouter
participant LlamaStackClient
Client->>FastAPI: GET /v1/conversations/{conversation_id}
FastAPI->>Auth: Validate authentication
Auth-->>FastAPI: Auth result
FastAPI->>ConversationRouter: Validate conversation_id, lookup agent_id
ConversationRouter->>LlamaStackClient: Retrieve session data
LlamaStackClient-->>ConversationRouter: Session data / Error
ConversationRouter-->>FastAPI: Response or HTTPException
FastAPI-->>Client: ConversationResponse or error
Client->>FastAPI: DELETE /v1/conversations/{conversation_id}
FastAPI->>Auth: Validate authentication
Auth-->>FastAPI: Auth result
FastAPI->>ConversationRouter: Validate conversation_id, lookup agent_id
ConversationRouter->>LlamaStackClient: Delete session
LlamaStackClient-->>ConversationRouter: Deletion result / Error
ConversationRouter-->>FastAPI: DeleteResponse or HTTPException
FastAPI-->>Client: ConversationDeleteResponse or error
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Tip For best results, initiate chat on the files or code changes. @eranco74, hello! I see you've mentioned me. How can I help you today? I notice you shared what appears to be conversation logs or API responses earlier. Are you looking for assistance with analyzing that data, or is there something specific you'd like me to help you with regarding this pull request? |
|
coderabbitai review |
| } | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
In order to keep the API compatibility with road-core, we need to return the same structure from GET .../conversations/<id>.
This return model doesn't seem compatible with road-core [0][1]
There was a problem hiding this comment.
Does it have to be compatible?
What llama-stack is returning seems a lot better than I see in the link above.
input_message and output_message seems better than content + type.
Not to mention the timestamps.
I agree that I should remove a few fields (steps, tool_execution, etc), but is it a requirement to align to what we have in road-core?
There was a problem hiding this comment.
What about something like this:
{
"conversation_id": "46f02ed5-29ee-41b6-a452-25938f926a52",
"chat_history": {
{
"messages": [
{
"content": "what can you do?",
"type": "user"
},
{
"content": "I can help you with various aspects of OpenShift cluster management using the assisted installer. Here's a list of things I can do:\n\n* **Cluster Information:**\n * Retrieve comprehensive information about a specific cluster...",
"type": "assistant"
}
],
"started_at": "2025-07-15T14:14:48.298571Z",
"completed_at": "2025-07-15T14:14:50.554798Z"
},
{
"messages": [
{
"content": "hello",
"type": "user"
},
{
"content": "Hello! How can I help you with your OpenShift cluster today?",
"type": "assistant"
}
],
"started_at": "2025-07-15T14:14:41.580122Z",
"completed_at": "2025-07-15T14:14:44.120312Z"
}
]
}
}
There was a problem hiding this comment.
Let's see what others say, from my understand, teams like Ansible are trying to replace the road-core/service with this new llama-stack implementation without having to change their client. @manstis please correct me if I am wrong.
If that's the case I think we will have to make it backward compatible.
That said, I do agree on cleaning up a bit the llama-stack output to remove the noise and I do like the timestamps too, they are useful.
I don't think the problem is adding new fields, we can totally do that as long as we keep the structure and old fields as before (assuming we want to keep it backward compatible).
There was a problem hiding this comment.
@umago we don't use /conversations in road-core so are unaffected if there's backwards compatibility issues.
There was a problem hiding this comment.
Hmm, perhaps I can do this:
"chat_history": {
{
"content": "what can you do?",
"type": "user"
"timestamp": "2025-07-15T14:14:48.298571Z",
},
{
"content": "I can help you with various aspects of OpenShift cluster management using the assisted installer. Here's a list of things I can do:\n\n* **Cluster Information:**\n * Retrieve comprehensive information about a specific cluster...",
"type": "assistant"
"timestamp": "2025-07-15T14:14:50.554798Z"
},
}
```
There was a problem hiding this comment.
Althouh I prefer the messages list since it provides better logical grouping of the messages. Better represent the conversation flow and makeing it easier to manipulate the output for analysis.
The original API flattens this context.
There was a problem hiding this comment.
Thanks @manstis for clarifying. Our team also doesn't rely on the backward compatibility for this endpoint. Perhaps one last thing we can do is to ask in the internal slack if any of the teams relies on it. If no one cares, yeah, I'm all about changing it to something new and better
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Has this been properly tested? I mean, either I am missing something about what we mean by a "conversation" or our whole conversation tracking is wrong. Are we referring to a conversation as a single Our current code creates a new Agent [1][2] on each call [1][2]. This means that each Last time I checked this I did it with 2 simple queries: From what I saw there are a good number of limitations currently around this:
Also, keeping the We are also not persisting the sessions in the llama-stack server, because we are not passing I don't remember if session persistence on llama-stack server requires some specific configuration (I think it didn't). |
|
Akrog I agree it's not GA, but it's a start.
you have it wrong, we currently use agent_cache to reuse the same agent for the same conversation.
That is currect, we will handle that in https://issues.redhat.com/browse/LCORE-373
We are passing enable_session_persistence=True |
- Add GET /v1/conversations/{conversation_id} to retrieve conversation history
- Add DELETE /v1/conversations/{conversation_id} to delete conversations
- Use llama-stack client.agents.session.retrieve and .delete methods
- Map conversation ID to agent ID for LlamaStack operations
- Add ConversationResponse and ConversationDeleteResponse models
- Include conversations router in main app routing
- Maintain consistent error handling and authentication patterns
|
Sorry, apparently I looked at an old code base (as shown in the links I pasted) and in there (and when I tried this) there was no caching of the agents as well as no passing of the persistence parameter. |
5 participants
Add /conversations endpoint for conversation history management
Description
Type of change
Related Tickets & Documents
LCORE-354
Checklist before requesting a review
Testing
Summary by CodeRabbit