Skip to content

fix(orchestrator): carry a failed batch's reason to the request error log - #616

Open
behinddwalls wants to merge 2 commits into
preetam/demo-burst-modefrom
preetam/batch-failure-reason
Open

fix(orchestrator): carry a failed batch's reason to the request error log#616
behinddwalls wants to merge 2 commits into
preetam/demo-burst-modefrom
preetam/batch-failure-reason

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

When a batch failed, the request that could not land ended in error with an empty message: mergesignal and speculate recorded only the terminal batch state, and conclude terminated the request with an empty reason — so a "did not land: error" carried no diagnostic at all.

Record why the batch failed on the batch itself when it is marked failed: the merge result's reason on the merge path, and "no speculation path could pass" on the speculate path. Persist it through the batch store (new failure_reason column, new Batch.FailureReason field), and have conclude pass it to the request's terminal log instead of an empty string. The dead-letter path already threads its own reason and is unchanged.

The demo client's run summary also surfaces the reason, so a failed request reads id=error: <reason> instead of a bare id=error.

Test Plan

✅ mergesignal records the merge reason on the batch; conclude carries it to the request's terminal log (new test); the batch store round-trips failure_reason; the client summary includes the note. //submitqueue/... green, CI gates clean.

Stack

  1. feat(demo): add BURST mode for a simultaneous enqueue of independent changes #615
  2. @ fix(orchestrator): carry a failed batch's reason to the request error log #616

… log

When a batch failed, the request that could not land ended in `error` with an empty message: mergesignal and speculate recorded only the terminal batch state, and conclude terminated the request with an empty reason — so a "did not land: error" carried no diagnostic at all.

Record why the batch failed on the batch itself when it is marked failed: the merge result's reason on the merge path, and "no speculation path could pass" on the speculate path. Persist it through the batch store (new failure_reason column, new Batch.FailureReason field), and have conclude pass it to the request's terminal log instead of an empty string. The dead-letter path already threads its own reason and is unchanged.
The final "N request(s) did not land" line printed only id=status, so a failure read as "demo-queue/630=error" with no reason — even once the orchestrator began recording one on the request's terminal log. Append the request's last error to each failed entry when it has one, so a watched or scripted run reports why a change did not land, not only that it did not.
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.

1 participant