Skip to content

FIX: let concurrent chat converter runs commit their own pieces - #3225

Merged
Roman Lutz (romanlutz) merged 4 commits into
microsoft:mainfrom
ManoharPaturi:fix/converter-concurrent-runs
Oct 15, 2026
Merged

Roman Lutz (romanlutz) merged 4 commits into
microsoft:mainfrom
ManoharPaturi:fix/converter-concurrent-runs

Conversation

@ManoharPaturi

Copy link
Copy Markdown
Contributor

Description

Fixes #3224

Two conversions started in the same tick raced through runConversion: the reentry guard read state.runId from a stale render closure, both runs started, and the commit rejected the earlier run wholesale with a run id comparison, so the piece it converted kept stale results with no error.

Runs now record which inputs they own. A run commits exactly the outcomes for inputs it still owns, the per input revision and pipeline revision checks keep protecting against genuinely stale outcomes, and ownership is cleared everywhere results are reset (clear all, restore, generation and scope changes). The converting flag only clears when no run is active anymore.

Tests and Documentation

Added a regression test that gives the text and image pieces distinct first results, starts one conversion per piece in a single act, and asserts both pieces update to new values. It times out on main and passes with the change. The full converters related suites pass (useChatConverters, usePieceConverters, ConverterPanel, ChatWindow, 253 tests) and tsc --noEmit is clean.

No documentation change needed, so I did not run JupyText.

The commit at the end of runConversion rejected the whole update when
the state run id moved, so a conversion started in the same tick as
another one lost its results even though the per input revision checks
already guard staleness. Track which run owns each input, commit the
outcomes a run still owns, and clear ownership wherever results are
reset. The converting flag now only clears when no run is active.

Fixes microsoft#3224

Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
Comment thread frontend/src/hooks/useChatConverters.ts Outdated
Comment thread frontend/src/hooks/useChatConverters.ts Outdated
…inish

Run ownership and the set of active runs move into the conversion
state, so a scope or generation change that React abandons mid render
discards the reset together with that render instead of leaking it out
of a ref. The converting flag now reflects every active run, so apply
stays disabled until the last concurrent run commits, and each run
removes only itself on completion.

Follow-up to the review on microsoft#3225.

Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 15, 2026
Merged via the queue into microsoft:main with commit a2cc444 Oct 15, 2026
93 of 94 checks passed
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.

Chat converter runs started together silently drop the first one's results

2 participants