-
Notifications
You must be signed in to change notification settings - Fork 345
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
fix: max tokens #820
base: main
Are you sure you want to change the base?
fix: max tokens #820
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
super(); | ||
this.messages = init?.messages ?? []; | ||
this.messagesBefore = this.messages.length; | ||
this.summaryPrompt = init?.summaryPrompt ?? defaultSummaryPrompt; | ||
this.llm = init?.llm ?? new OpenAI(); | ||
if (!this.llm.metadata.maxTokens) { | ||
if (!this.llm.metadata.maxTokens || !init?.maxTokens) { |
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.
Should we use Infinity
if it's undefined?
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.
the code below won't work if maxTokens is not specified, so I think it's ok, to throw this error, it's basically a configuration error.
this class also needs a tokenizer to work correctly (and for ollama it could be any LLM with any tokenizer).
So I think we should set tokenizer
to globalsHelper.defaultTokenizer.encode
only if the LLM is OpenAI and throw an error if tokenizer
is not explicitly specified otherwise
e61e843
to
f1906bf
Compare
Fixes: https://discord.com/channels/1059199217496772688/1133167189860565033/1237394506274439268