-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Fix: Filtering conversations with no created at #6414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -40,6 +40,8 @@ async def get_metadata(self, conversation_id: str) -> ConversationMetadata: | |||
|
|||
# Temp: force int to str to stop pydandic being, well... pedantic | |||
json_obj = json.loads(json_str) | |||
if 'created_at' not in json_obj: | |||
raise FileNotFoundError(path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why wouldn't it have a created_at
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's older than this feature 😄
We currently see a lot of "55 years ago" timestamps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah... I wonder if we could just add a created_at
if it's missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conversations are not implemented anywhere else than server
currently, but I don't think it's difficult to do, at least in some basic way. This code is outside server
. So I wonder if it makes sense to be lenient: once implemented elsewhere, e.g. cli, old streams could be loaded using conversations (replacing/using the session name).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Older entries from the start of January didn't have this field yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they don't have it, they fail to load, right? An alternative is to allow them to load and provide something (initialize it with now
)
Maybe it doesn't matter a lot, admittedly, because there aren't many that both have metadata and don't have this field.
Filter out older conversations that don't have all required details
Filtering out conversations that don't have a 'created_at' field. (The top 2 in the screenshot below):
To run this PR locally, use the following command: