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

remove max_input_size property from assistants #362

Merged
merged 2 commits into from
Mar 13, 2024
Merged

Conversation

pmeier
Copy link
Member

@pmeier pmeier commented Mar 13, 2024

The abstract Assistant.max_input_size property was a left over from the prototype implementation we started from. The idea behind it was for to be able to automatically fill the context window, such that it is maxed out with the sources and the user prompt.

However, this makes quite a few assumptions, that held for our prototype, but are no longer true for Ragna:

Meaning, Assistant.max_input_size is not only unused, actually using it requires a major effort in designing a proper interface. And with all that uncertainty I'd rather not force users to implement this property.

Note that this is only mildly BC breaking. Users that had defined this property on their custom assistants are untouched by this change. It just stays unused. Only if you access this property on our builtin assistants, you will have a breakage.

@pmeier pmeier requested a review from nenb March 13, 2024 15:18
@pmeier
Copy link
Member Author

pmeier commented Mar 13, 2024

Cc @smokestacklightnin. This should also make #358 easier.

@pmeier pmeier merged commit 5ac27bd into main Mar 13, 2024
10 checks passed
@pmeier pmeier deleted the remove-input-size branch March 13, 2024 16:06
smokestacklightnin added a commit to smokestacklightnin/ragna that referenced this pull request Mar 13, 2024
smokestacklightnin added a commit to smokestacklightnin/ragna that referenced this pull request Mar 19, 2024
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.

None yet

2 participants