Skip to content

Conversation

@jdb78
Copy link
Contributor

@jdb78 jdb78 commented Jul 16, 2025

Related Issues

Proposed Changes:

added a generation_kwargs parameter to Agent which are to be passed to LLM

How did you test it?

added generation_kwargs to unit test run

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I documented my code
  • I ran pre-commit hooks and fixed any issue

@jdb78 jdb78 requested a review from a team as a code owner July 16, 2025 20:24
@jdb78 jdb78 requested review from sjrl and removed request for a team July 16, 2025 20:24
@CLAassistant
Copy link

CLAassistant commented Jul 16, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added topic:tests type:documentation Improvements on the docs labels Jul 16, 2025
Co-authored-by: Sebastian Husch Lee <[email protected]>
@sjrl
Copy link
Contributor

sjrl commented Jul 21, 2025

Hey @jdb78 to help with the CI I'd also recommend checking out how to run our code-quality checks here and to add a release note to this PR as described here.

@sjrl sjrl added the information-needed Information needed from the user label Aug 25, 2025
@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale label Sep 25, 2025
@jdb78
Copy link
Contributor Author

jdb78 commented Oct 1, 2025

@sjrl I updated the PR, addressed your comments and added release notes. Hopefully ready to merge

@github-actions github-actions bot removed the stale label Oct 2, 2025
@jdb78
Copy link
Contributor Author

jdb78 commented Oct 17, 2025

@sjrl any chance you find some minutes to look over it?

@sjrl
Copy link
Contributor

sjrl commented Oct 20, 2025

Hey @jdb78 apologies for the slow response. We are discussing internally if we'd prefer to make this a more general chat_generator_kwargs than just generation_kwargs

@jdb78
Copy link
Contributor Author

jdb78 commented Oct 21, 2025

My personal opinion: generation_kwargs is just fine. generators always seem to refer to LLMs and so changing to chat_ generation_kwargs might as well trigger a change for all the chat generators that are in the code base which would likely cause confusion. I don't think there is a need to make it explicit if it is a generator or chat generator. For the agent, I don't see how you could have both at the same time.

@jdb78
Copy link
Contributor Author

jdb78 commented Oct 27, 2025

@sjrl : Any movement on this?

@sjrl
Copy link
Contributor

sjrl commented Oct 27, 2025

Hey @jdb78 sorry for the slow response. After more discussion we agree that adding generation_kwargs makes sense. Could you go ahead and and resolve the conflict and we will give this a final review.

@vercel
Copy link

vercel bot commented Oct 27, 2025

@jdb78 is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@jdb78
Copy link
Contributor Author

jdb78 commented Oct 27, 2025

@sjrl done

Copy link
Contributor

@sjrl sjrl left a comment

Choose a reason for hiding this comment

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

Looks good!

@sjrl sjrl enabled auto-merge (squash) October 28, 2025 08:54
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 18848249981

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 9 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.005%) to 92.214%

Files with Coverage Reduction New Missed Lines %
components/agents/agent.py 9 96.48%
Totals Coverage Status
Change from base Build 18846783538: -0.005%
Covered Lines: 13467
Relevant Lines: 14604

💛 - Coveralls

@sjrl sjrl merged commit 6e12368 into deepset-ai:main Oct 28, 2025
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

information-needed Information needed from the user topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support generation_kwargs for Agent

4 participants