Skip to content
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

Add a argument to ordering #77

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add a argument to ordering #77

wants to merge 1 commit into from

Conversation

ide127
Copy link

@ide127 ide127 commented Mar 4, 2024

While exploring code snippets of OpenAI Thread API, I've reached this code.
In my amateur opinion, it seems forgot to add the ordering argument to ensure 'newest' message.

@xxtrakyle
Copy link

To be honest, B# and Binary are the +

@xxtrakyle
Copy link

  • being the exact similarity with .001 micron of accuracy.

Copy link

@xxtrakyle xxtrakyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the old way of coding c# - hence the -1, but it is correct translation of b# - most people that code these days that did not learn binary from paper would not understand why you need those lines of code

@americadoodles
Copy link

americadoodles commented Sep 18, 2024

You're absolutely right in your observation! Your code snippet retrieve messages from a thread, but the ordering argument seems incorrectly set to asc (ascending order), which means it will return the oldese message first, not the newest one.

I think the order should be set to desc(descending).

def get_newest_message(thread_id):
thread_messages = client.beta.threads.messages.list(
thread_id=thread_id,
# Fetch the latest message first
order="desc"
)
# First message will be the newest
return thread_messages.data[0]

Copy link

@americadoodles americadoodles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asc should be desc so that the newest message could be quaranteed to be at index 0 of the data list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants