Skip to content

Avoid duplicate selection step enqueueing#5675

Merged
rmosolgo merged 2 commits into
rmosolgo:masterfrom
ydah:fix-selections-step-duplicate-enqueue
Jul 22, 2026
Merged

Avoid duplicate selection step enqueueing#5675
rmosolgo merged 2 commits into
rmosolgo:masterfrom
ydah:fix-selections-step-duplicate-enqueue

Conversation

@ydah

@ydah ydah commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Prevent SelectionsStep from enqueueing field steps from previous selection groups more than once. continue_selections was initialized outside the group loop, so each group retained and re-enqueued every step collected from earlier groups.

For example, two groups containing first and second were enqueued as [first, first, second]. This change scopes continue_selections to each group, producing [first, second].

@rmosolgo

Copy link
Copy Markdown
Owner

Interesting ... I think the CI failures are real here. IIRC this array has something do about directives which pause execution. But I see what you mean about this array accumulating things outside of scope.

Another possible fix could be here:

continue_selections.each do |frs|
@runner.add_step(frs)
end

Maybe we could do a continue_selections.clear after the steps are all passed along. (That would also let us keep using the same Array instance.)

@ydah

ydah commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! I tried continue_selections.clear after enqueueing, but the three directive feature tests still failed.
The issue is that NullDataloader#add_step executes synchronously. Earlier field steps ran immediately, then a later group’s replace(prototype_result) reset those results to nil. The previous duplicate enqueueing happened to restore them.
I moved the enqueue loop after all selection groups have been processed instead. This avoids duplicate enqueueing while ensuring all prototype updates are complete before any field step runs. I pushed the fix in a824a8a.

@rmosolgo

Copy link
Copy Markdown
Owner

That's great. Thanks again for hunting this down and contributing it!

@rmosolgo
rmosolgo merged commit 092831c into rmosolgo:master Jul 22, 2026
13 checks passed
@rmosolgo rmosolgo added this to the 2.6.7 milestone Jul 22, 2026
@ydah
ydah deleted the fix-selections-step-duplicate-enqueue branch July 22, 2026 14:05
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.

2 participants