Skip to content

Conversation

@dmontagu
Copy link
Contributor

@dmontagu dmontagu commented Nov 4, 2025

Closes #3296

I realized that there were two issues in the existing implementation due to not thinking deeply enough about what happens when you have multiple joins feeding into each other downstream of a single fork:

  • I believe we shouldn't be resetting the fork stack in join nodes that are intermediate nodes of other join nodes with the same parent fork. It's possible this hasn't been done quite correctly in this PR, and we are leaving the fork stack unmodified in too many cases (I believe getting this right amounts to the exact definition of what a "final" join node is), but I suspect that it would require some exotic graph structure to reproduce that I'm not sure how to produce, and I'd be happy to try to fix that if/when a new issue gets reported. (All existing tests continue to pass, obviously.)
  • When deciding whether to proceed with a join, we need to make sure that there aren't upstream joins. This is actually the main issue in Bug Report: collect(Reducer) loses synchronization in pydantic_graph join sequence #3296, and is fixed in this PR by tracking intermediate joins and looking for active reducers that correspond to intermediate joins with the same fork stack.

I think the first (fork stack) bullet above was a bug, and may still have some bugs, but I think it wasn't what was actively causing problems — the second bullet was what was causing the problem. But I think if we don't fix the fork stack tracking it will be the source of other bugs in the future. If related bugs do arise in the future, whoever works on it (probably me, but still) should probably study the changes in this PR and think more deeply about what the "correct" behavior is (assuming the behavior in this PR is not actually correct.. which I'm not totally sure of at this time sadly, other than that again, all existing tests pass).

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

Docs Preview

commit: 7d39f04
Preview URL: https://38e3f001-pydantic-ai-previews.pydantic.workers.dev

@DouweM DouweM enabled auto-merge (squash) November 4, 2025 21:54
@DouweM DouweM merged commit 63f6942 into main Nov 4, 2025
30 checks passed
@DouweM DouweM deleted the dmontagu/fix-graph-bug branch November 4, 2025 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug Report: collect(Reducer) loses synchronization in pydantic_graph join sequence

3 participants