Skip to content

feat: Raise warning if the pipeline is unable to continue running due to a blocked component#9569

Merged
sjrl merged 22 commits intomainfrom
add-warning-when-pipeline-stops-early
Jul 15, 2025
Merged

feat: Raise warning if the pipeline is unable to continue running due to a blocked component#9569
sjrl merged 22 commits intomainfrom
add-warning-when-pipeline-stops-early

Conversation

@sjrl
Copy link
Copy Markdown
Contributor

@sjrl sjrl commented Jun 30, 2025 •
edited
Loading

Related Issues

Proposed Changes:

Update _get_next_runnable_component to raise a warning if the next runnable component is blocked and it looks like the pipeline is not finished based on what's available in the pipeline outputs.

How did you test it?

  • Running integration tests via CI to see if this affects existing tests.
  • Added new integration test

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 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

@github-actions github-actions Bot added the type:documentation Improvements on the docs label Jun 30, 2025
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Jun 30, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 16295205672

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 87 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.03%) to 90.374%
Files with Coverage Reduction New Missed Lines %
core/pipeline/pipeline.py 3 95.16%
components/agents/agent.py 9 90.8%
core/pipeline/async_pipeline.py 23 68.79%
core/pipeline/base.py 52 86.95%
Totals Coverage Status
Change from base Build 16265419365: -0.03%
Covered Lines: 11698
Relevant Lines: 12944
💛 - Coveralls

sjrl
sjrl commented Jun 30, 2025
View reviewed changes
Comment thread test/core/pipeline/features/test_run.py
sjrl
sjrl commented Jul 1, 2025
View reviewed changes
Comment thread haystack/core/pipeline/async_pipeline.py
@sjrl sjrl marked this pull request as ready for review July 9, 2025 10:53
@sjrl sjrl requested review from a team as code owners July 9, 2025 10:53
@sjrl sjrl requested review from dfokina and removed request for a team July 9, 2025 10:53
@sjrl sjrl requested review from Amnah199 and removed request for a team July 9, 2025 10:53
@sjrl sjrl changed the title feat: Raise PipelineComponentBlockedError if the pipeline is unable to continue running due to a blocked component feat: Raise warning if the pipeline is unable to continue running due to a blocked component Jul 9, 2025
@sjrl sjrl requested a review from julian-risch July 9, 2025 10:54
Amnah199
Amnah199 reviewed Jul 15, 2025
View reviewed changes
Comment thread haystack/components/agents/agent.py
Amnah199
Amnah199 reviewed Jul 15, 2025
View reviewed changes
Comment thread haystack/core/pipeline/async_pipeline.py
Amnah199
Amnah199 reviewed Jul 15, 2025
View reviewed changes
Comment thread test/core/pipeline/test_pipeline_base.py
result = pipeline._get_next_runnable_component(queue, component_visits={})
assert result is None

def test__get_next_runnable_component_blocked(self):
Copy link
Copy Markdown
Contributor

@Amnah199 Amnah199 Jul 15, 2025

Choose a reason for hiding this comment

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

Overall the PR looks great! Can you explain one last thing why is this test not working with the changes?

Copy link
Copy Markdown
Contributor Author

@sjrl sjrl Jul 15, 2025

Choose a reason for hiding this comment

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

This is a result of how I changed _get_next_runnable_component_blocked to no longer return none if the next runnable component is BLOCKED. That check is now done inside the Pipeline.run and AsyncPipeline.run methods.

Amnah199
Amnah199 approved these changes Jul 15, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@Amnah199 Amnah199 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 to me! We can decide if we want another approval.

julian-risch
julian-risch approved these changes Jul 15, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@julian-risch julian-risch left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! 👍
The only part that I found more difficult to understand is where we now added

# We still always exit the loop since we cannot run the next component.
break

It wasn't clear to me what still refers to here. We didn't break at that point before the PR. My understanding is that it means even though we could not infer that the pipeline is possibly blocked via our heuristics, we break out of the loop because the next component to run is blocked and that won't change.

@sjrl
Copy link
Copy Markdown
Contributor Author

sjrl commented Jul 15, 2025

LGTM! 👍 The only part that I found more difficult to understand is where we now added

# We still always exit the loop since we cannot run the next component.
break

It wasn't clear to me what still refers to here. We didn't break at that point before the PR. My understanding is that it means even though we could not infer that the pipeline is possibly blocked via our heuristics, we break out of the loop because the next component to run is blocked and that won't change.

You're right the still doesn't make sense here. I've removed it for clarity.

@sjrl sjrl enabled auto-merge (squash) July 15, 2025 13:52
@sjrl sjrl merged commit 7a63559 into main Jul 15, 2025
20 checks passed
@sjrl sjrl deleted the add-warning-when-pipeline-stops-early branch July 15, 2025 14:02
OGuggenbuehl pushed a commit to OGuggenbuehl/haystack that referenced this pull request Nov 21, 2025
@sjrl @OGuggenbuehl
feat: Raise warning if the pipeline is unable to continue running due…
74ff8e5 
… to a blocked component (deepset-ai#9569)

* First pass at alerting a user that the pipeline is blocked

* Change approach and add change to async pipeline

* Fix check in run_async

* Another fix

* Somehow also fixed the max_runs_per_component

* Align sync run and async run component

* Update output type of component outputs to Mapping to align with our protocol

* linting

* ruff

* Make it a core test

* Add reno

* Some refactoring

* Linting

* Fix mypy

* Converting to warning

* Small changes

* Update release note

* More cleanup

* PR comment

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

Reviewers

@julian-risch julian-risch julian-risch approved these changes

@Amnah199 Amnah199 Amnah199 approved these changes

@dfokina dfokina Awaiting requested review from dfokina dfokina was automatically assigned from deepset-ai/documentation

Assignees

No one assigned

Labels

topic:core topic:tests type:documentation Improvements on the docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Custom components: Adding warning when not all inputs can be obtained

4 participants

@sjrl @coveralls @julian-risch @Amnah199