Skip to content

feat: Migrate Human in the Loop for Agents from experimental #10403

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.

Sign up for GitHub

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

Jump to bottom
Open
sjrl wants to merge 18 commits into
base: main
Choose a base branch
from
Open

feat: Migrate Human in the Loop for Agents from experimental #10403

sjrl wants to merge 18 commits into from
+2,531 −52

Conversation

@sjrl
Copy link
Contributor

@sjrl sjrl commented Jan 19, 2026 •
edited
Loading

Related Issues

Proposed Changes:

  • Migrates HiTL from experimental to main

How did you test it?

  • Brought over unit and integration tests

Notes for the reviewer

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have updated the related issue with new insights and changes.
  • I have 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 have documented my code.
  • I have added a release note file, following the contributors guidelines.
  • I have run pre-commit hooks and fixed any issue.

@vercel
Copy link

vercel bot commented Jan 19, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
haystack-docs Ignored Ignored Preview Jan 21, 2026 1:05pm

Request Review

@sjrl sjrl marked this pull request as ready for review January 20, 2026 11:57
@sjrl sjrl requested a review from a team as a code owner January 20, 2026 11:57
@sjrl sjrl requested review from davidsbatista and removed request for a team January 20, 2026 11:57
@sjrl
Copy link
Contributor Author

sjrl commented Jan 20, 2026

@davidsbatista feel free to loop in Stefano as well since he reviewed the original feature

@sjrl sjrl self-assigned this Jan 20, 2026
@sjrl sjrl changed the title feat: Migrate hitl feat: Migrate Human in the Loop for Agents from experimental Jan 20, 2026
@sjrl
Copy link
Contributor Author

sjrl commented Jan 20, 2026

Ahh I forgot, I need to create new doc section and files as well

@sjrl sjrl mentioned this pull request Jan 20, 2026
Closed
@davidsbatista
Copy link
Contributor

davidsbatista commented Jan 20, 2026

@anakin87 - maybe you can help me on this one, since you already reviewed the first iteration on experimental

@anakin87 anakin87 self-requested a review January 20, 2026 17:06
@anakin87
Copy link
Member

anakin87 commented Jan 21, 2026

@sjrl could you confirm that we don't want to migrate BreakpointConfirmationStrategy for the moment?

@anakin87
Copy link
Member

anakin87 commented Jan 21, 2026 •
edited
Loading

I took a look at the PR and it seems good.

I am thinking about directories... We currently have dataclasses > human_in_the_loop.py and then the human_in_the_loop directory.

In experimental, these modules were grouped together in components > agent > human_in_the_loop. I like the experimental structure more.

@sjrl WDYT?

@sjrl
Copy link
Contributor Author

sjrl commented Jan 21, 2026

@sjrl could you confirm that we don't want to migrate BreakpointConfirmationStrategy for the moment?

That is correct. It was left in experimental on purpose since it's not needed by platform and we haven't built an end-to-end demo with it using Hayhooks so I'm less confident that it doesn't need future changes to work well in a production scenario.

@sjrl
Copy link
Contributor Author

sjrl commented Jan 21, 2026

I took a look at the PR and it seems good.

I am thinking about directories... We currently have dataclasses > human_in_the_loop.py and then the human_in_the_loop directory.

In experimental, these modules were grouped together in components > agent > human_in_the_loop. I like the experimental structure more.

@sjrl WDYT?

I changed the structure since I was trying to make it easier to create new integrations related to confirmation strategies and policies in core-integrations. For example, the RedisConfirmationStrategy by @mpangrazzi would probably live in core-integrations and I thought it would be odd to require the folder structure for that to be components > agent > human_in_the_loop > redis vs now it would be human_in_the_loop > redis. Basically I took inspiration from how tools is it's own top-level directory and our MCP integration is in tools > mcp.

I'm open to changing this though, but what's your take on making it easy to create integrations for human in the loop features?

@anakin87
Copy link
Member

anakin87 commented Jan 21, 2026 •
edited
Loading

I changed the structure since I was trying to make it easier to create new integrations related to confirmation strategies and policies in core-integrations. For example, the RedisConfirmationStrategy by @mpangrazzi would probably live in core-integrations and I thought it would be odd to require the folder structure for that to be components > agent > human_in_the_loop > redis vs now it would be human_in_the_loop > redis. Basically I took inspiration from how tools is it's own top-level directory and our MCP integration is in tools > mcp.

I'm open to changing this though, but what's your take on making it easy to create integrations for human in the loop features?

I understand your point and makes sense.

It's a minor thing but what about also putting related dataclasses in the same HiTL directory? (This would also reflect what we have done for tools)

@sjrl
Copy link
Contributor Author

sjrl commented Jan 21, 2026

I understand your point and makes sense.

It's a minor thing but what about also putting related dataclasses in the same HiTL directory? (This would also reflect what we have done for tools)

Yeah happy to do that!

@sjrl
Copy link
Contributor Author

sjrl commented Jan 21, 2026

@anakin87 this should be ready for another review!

anakin87
anakin87 reviewed Jan 21, 2026
View reviewed changes
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.

Looks good in general.

We should add pydoc config for the new human_in_the_loop package to haystack/pydoc.

I imagine that documentation will be added in a future PR. Right?

@sjrl
Copy link
Contributor Author

sjrl commented Jan 21, 2026

Looks good in general.

We should add pydoc config for the new human_in_the_loop package to haystack/pydoc.

I imagine that documentation will be added in a future PR. Right?

Good point. I can also add it in this PR

@anakin87
Copy link
Member

anakin87 commented Jan 21, 2026

I'd prefer:

  • pydoc config in this PR
  • docs in a new PR

Not a strong opinion, in any case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anakin87 anakin87 anakin87 left review comments

@davidsbatista davidsbatista Awaiting requested review from davidsbatista davidsbatista is a code owner automatically assigned from deepset-ai/open-source-engineering

At least 1 approving review is required to merge this pull request.

Assignees

@sjrl sjrl

Labels

topic:build/distribution topic:tests type:documentation Improvements on the docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sjrl @davidsbatista @anakin87