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

refactor!: AmazonBedrockGenerator - remove truncation #1314

Merged
merged 8 commits into from
Jan 23, 2025
Merged

Conversation

vblagoje
Copy link
Member

@vblagoje vblagoje commented Jan 22, 2025

Why:

Remove truncation in AmazonBedrockGenerator.

  • Models have super long context nowadays and truncation is rarely needed
  • We don't have truncation uniformly across generators
  • We have extra dependency on transformers to calculate tokens
  • Truncation can be done in a seperate component that feeds final prompt to generator

What:

  • Removed the transformers dependency from the pyproject.toml.
  • Deprecated max_length and truncate parameters and associated warning logic from the generator.
  • Deleted the DefaultPromptHandler class along with associated prompt truncation logic.

How can it be used:

See above

How did you test it:

  • Updated tests to not test truncation
  • Relying on exiting tests to make sure no regressions were made

Notes for the reviewer:

Please focus on the removal of the prompt handler logic and verify that all necessary tests continue to pass. Special attention may be needed to confirm that max_length and truncate parameters' deprecation does not affect downstream functionality or produce any warnings at runtime.

@github-actions github-actions bot added integration:amazon-bedrock type:documentation Improvements or additions to documentation labels Jan 22, 2025
@vblagoje vblagoje marked this pull request as ready for review January 22, 2025 10:23
@vblagoje vblagoje requested a review from a team as a code owner January 22, 2025 10:23
@vblagoje vblagoje requested review from mpangrazzi and anakin87 and removed request for a team and mpangrazzi January 22, 2025 10:23
@vblagoje
Copy link
Member Author

vblagoje commented Jan 22, 2025

@mpangrazzi not to distract you with something that @anakin87 is already familiar with and has complete context. @anakin87 I know we are not following deprecation policy here but with generators being slowly removed anyways I thought it would be an overkill to follow the policy here - so I just eviscerated the truncation support altogether. And we have internal support to remove it right away 👍

Copy link
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

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

In general, I like this change.

I left a comment.

I would also make this change more evident in the changelog with a PR title like:
"refactor!: AmazonBedrockGenerator - remove truncation "

…ts/generators/amazon_bedrock/generator.py

Co-authored-by: Stefano Fiorucci <[email protected]>
@vblagoje vblagoje changed the title chore: AmazonBedrockGenerator - remove truncation refactor!: AmazonBedrockGenerator - remove truncation Jan 22, 2025
@anakin87 anakin87 self-requested a review January 22, 2025 15:32
Copy link
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

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

Feel free to merge once linting errors are fixed (restore stacklevel).

@vblagoje
Copy link
Member Author

@anakin87 Let's batch it with #1304 once it becomes ready so we can issue a new release at that time

@vblagoje vblagoje merged commit 8b9d0e9 into main Jan 23, 2025
11 checks passed
@vblagoje vblagoje deleted the remove_truncate branch January 23, 2025 16:13
Amnah199 pushed a commit that referenced this pull request Jan 28, 2025
* AmazonBedrockGenerator - remove truncation

* Update tests

* Linting

* Remove deprecation test

* Update docs config

* Update integrations/amazon_bedrock/src/haystack_integrations/components/generators/amazon_bedrock/generator.py

Co-authored-by: Stefano Fiorucci <[email protected]>

* Lint

---------

Co-authored-by: Stefano Fiorucci <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration:amazon-bedrock type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants