fix!: resume a pipeline snapshot taken on a later loop visit - #12162
Conversation
…pipelines. This fixes a bug when using a pipeline snapshots with pipelines that contain loops.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| The inputs a socket received are stored keyed by position rather than as a list, because a list gets a single | ||
| schema derived from its first item. A socket with several senders can hold values of different types, for | ||
| example a value from one sender and a `_NoOutputProduced` marker from another, and those need one schema | ||
| each to survive the round trip. |
There was a problem hiding this comment.
This explains why this function is needed as a workaround since _serialize_with_field_fallback does not support serializing mixed-type lists.
| def _deserialize_internal_inputs(serialized_inputs: dict[str, Any]) -> dict[str, Any]: | ||
| """ | ||
| Restore the pipeline's internal inputs state from a snapshot written by `_serialize_internal_inputs`. |
There was a problem hiding this comment.
Related to comment https://github.com/deepset-ai/haystack/pull/12162/changes#r3656582668 that this function is only needed to work around _deserialize_value_with_schema not supporting deserializing mixed type lists.
| class _NoOutputProduced: | ||
| """ | ||
| Marker a socket input holds when its sender ran without producing a value for that socket. |
There was a problem hiding this comment.
This was upgraded to a proper standalone class b/c it must support serde since it can be included in the serialized inputs in PipelineState.inputs
| pipeline.run(data={"comp1": {"input_value": "test"}}, break_point=Breakpoint(component_name="comp2")) | ||
| snapshot = exc_info.value.pipeline_snapshot | ||
| @pytest.mark.parametrize("visit_count", [0, 1, 2, 3]) | ||
| def test_break_point_in_loop_resumes_on_any_visit(self, visit_count): |
|
added |
|
|
||
|
|
||
| class TestCreatePipelineSnapshot: | ||
| def test_create_pipeline_snapshot_all_fields(self): |
There was a problem hiding this comment.
nit: maybe worth a "live" test for this one ?
|
overall looks good, left a comment regarding the tests |
davidsbatista
left a comment
There was a problem hiding this comment.
looks good, left a suggestion for an extra test, not blocking
…aystack into minimal-fix-snapshot-resume
Related Issues
Proposed Changes:
Fixes
Fixed resuming a
Pipelinefrom apipeline_snapshotthat was taken on a component's second or later visit,which failed with
PipelineComponentsBlockedError: Cannot run pipeline - all components are blocked. A snapshot stored only the values of the pipeline's inputs and dropped the information about which component had sent each one, so on resume every restored input looked like it came from outside the pipeline, and such an input can only trigger a component on its first visit. Snapshots now record the sender of each input. Snapshots created by earlier versions of Haystack behave as before, so re-create them to resume anywhere in a looping pipeline.Fixed a resumed
Pipelinepassing malformed inputs to the component the snapshot was taken on, whenever that component ran more than once after the resume, for example inside a loop. Every visit after the first reused the handling meant only for the paused visit and skipped the regular input consumption, so a variadic component could receive a bare value where it expected a list, raising errors such asTypeError: object of type 'int' has no len()from aBranchJoiner. This affected snapshots taken at any visit count, including the first.The sentinel marking a socket whose sender produced no output for it is now its own type,
_NoOutputProduced, rather than an alias of the_emptymarker for an unsetInputSocket.default_value. The two record unrelated things, and we needed to add serde support to_NoOutputProduced.Breaking Change
PipelineSnapshot.pipeline_state.inputschanged shape. It used to store one flattened value per socket,{component: {socket: value}}. It now stores the pipeline's internal inputs, keeping the component that senteach one and keying the inputs a socket received by their position:
{component: {socket: {"0": {"sender": ..., "value": ...}}}}. Positions are used instead of a list so that each input carries its own type schema, which a list cannot do.PipelineStategained aninputs_formatfield recording which of the two shapes a snapshot uses. The same applies toBreakpointException.inputs, which returns that field.You are affected if you read
pipeline_state.inputs(orBreakpointException.inputs) directly, for exampleto display or post-process a snapshot. Resuming a pipeline with
Pipeline.run(pipeline_snapshot=...)is notaffected, and neither is code that only passes snapshots around or persists them.
To adapt, read the input through its position and take its
value:A socket that received inputs from several senders now has one entry per position,
"0","1"and so on,each recording the
senderthat produced it.Snapshots written by earlier versions of Haystack have
inputs_formatset toNoneand keep the flattenedshape, so branch on that field if you need to handle both.
How did you test it?
New tests and updated existing ones.
Notes for the reviewer
This has a breaking change associated with it in addition to being a fix. The inputs format has changed so anyone who relied on inspecting on the old format will get errors. More details in the reno.
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.