Feat conversations CRUDS API #5775
Conversation
| @@ -121,7 +121,7 @@ def _should_attach(self, request) -> bool: | |||
| if request.url.path.startswith('/api/conversation'): | |||
| # FIXME: we should be able to use path_params | |||
| path_parts = request.url.path.split('/') | |||
| if len(path_parts) > 3: | |||
| if len(path_parts) > 4: | |||
There was a problem hiding this comment.
why did this change? /api/conversations/foo should have 4 parts
There was a problem hiding this comment.
Oh I see--we don't need to attach the runtime for these.
Right now I think that's our main auth mechanism, so probably better to leave this as 3? Worth refactoring in the future though
| try: | ||
| title = conversation.title | ||
| if not title: | ||
| title = f'Conversation {conversation.conversation_id}' |
There was a problem hiding this comment.
Maybe conversation.conversation_id[:5] as a shortname?
| @@ -0,0 +1,57 @@ | |||
| from datetime import datetime | |||
There was a problem hiding this comment.
this utils folder is getting out of hand 😓 probably another thing to refactor in the future
There was a problem hiding this comment.
This is a nice feature! I just have a doubt, a simple thing about architecture here:
the event stream and its management is self-contained -ish, enough that it has always been possible, I think, to load in cli the event streams created with an UI.
Give or take that they have ugly names, 😅 but otherwise, there are no major reasons not to work. They're just events.
With the caveat that I haven't looked much in the past couple of weeks, this appears to be the case after the introduction of conversations too. At least I think it works with the default convo.
Is there a reason why managing it from cli wouldn't continue to work, with sessions or with convos alike?
In this PR the server code is not really minimally separated from backend.
Can we keep it possible to add in cli? It doesn't have to be implemented in this PR, of course, but I do think it would be nice to not go against it either: for example, the inverse dependencies mean that one cannot use the methods in utils, even though they're in utils.
All we'd need here, is to keep separate the code that works with the stream and file store, from code that works with the session_manager global or sio etc. What do you think?
|
|
||
| from openhands.core.logger import openhands_logger as logger | ||
| from openhands.server.routes.settings import ConversationStoreImpl | ||
| from openhands.server.shared import config |
There was a problem hiding this comment.
Can we have the functions that import from server in a server module, like its shared.py? IMHO the new callback is only useful for server, right?
|
@enyst - At the moment, this PR leverages the existing functionality where conversation metadata is stored when a conversation is started in web . I just did a quick check, and the same metadata is not being stored for conversations started in CLI mode - I think we should create a second PR to remedy this issue (The first line below was created via CLI, the others via Web) |
|
That's okay, we don't need to create metadata, sure. I just feel like this PR is heading in the opposite direction, when it adds more dependencies to Could we just move the new callback function, for example, if it depends on |
|
Ah, too bad it comes at the price of not exposing the |
End-user friendly description of the problem this fixes or functionality that this introduces
Backend functionality for searching and paginating through conversations.
Give a summary of what the PR does, explaining any non-trivial design decisions
New Components
ConversationInfodata model to represent conversation metadataConversationResultSetfor paginated search resultsConversationStatusenum to track conversation statessearch_utils.pywith pagination utilities (offset/page_id conversion)Modified Components
Testing
Implementation Details
This PR provides the backend foundation for the conversation search feature, enabling users to efficiently browse and find their past conversations.
To run this PR locally, use the following command: