fix(review): pair proposal headlines to operations by sequence, not by array index

[Chris0Jeky]

Summary

presentation.operationHeadlines is built server-side in Sequence order, but the proposal DTO's operations array was not, and ReviewProposalCard.vue paired them by index. When the two orders disagreed, the reviewer read headline n against a different operation than the one it describes.

The visible harm is on the enrichment path added by #2541. A "Move card." headline paired with a non-move operation fails the actionType / targetType gate and silently loses its destination, and two card moves paired in the wrong order name each other's columns. Both are shown in the new component spec.

This fixes it on both sides: the backend now emits one order contract, and the Legacy review card sorts defensively so it stays correct against an older backend.

Closes #2563

Changes

Backend contract (AutomationProposalService.cs)

Two reachable orderings produced the mismatch, and both now sort by Sequence:

• MapToDto mapped proposal.Operations directly. On the create path that is the in-memory AddOperation order, which is the request's array order with no database read involved, so a client that posts operations out of sequence order got a mispaired response deterministically. On the read path it is an EF Include with no ORDER BY.
• BuildEffectiveProposalDto replaced Operations with the revision's parsed set, which keeps the payload's array order. A reviewer editing the revision JSON can leave that array disagreeing with the operations' own sequence fields.

Frontend defence (ReviewProposalCard.vue)

getOperationHeadlines sorts a copy of the operations by sequence before pairing. This mirrors the sort that storedOperationsFallback in the same component and the Paper review surface already apply; it does not import from PaperReviewView.vue. The #2541 enrichment gate itself is untouched.

Consumer audit before changing the DTO order

Nothing depended on the previous order. AutomationExecutorService sorts by Sequence before dispatch, GetProposalDiffAsync, ProposalOperationContractValidator, CaptureTriageService and SimilarDecisionService all sort themselves, and CardHistoryService uses the array index only to number its own rows (its serials now follow sequence order, which is an improvement). On the client, PaperReviewView.vue, ReviewAppliedDecisionRecord.vue and editablePayload already sort by sequence; the remaining readers use .length or iterate order-independently. useBatchApproveProposals's reviewFingerprint stringifies the array, so a stable order makes that fingerprint less prone to spurious mismatches, not more.

Test plan

Verified, all from the worktree root unless noted.

• dotnet test backend/tests/Taskdeck.Application.Tests/Taskdeck.Application.Tests.csproj -c Release -m:1 --filter "FullyQualifiedName~AutomationProposalServiceTests" — 151 passed, 0 failed.
• dotnet test backend/tests/Taskdeck.Api.Tests/Taskdeck.Api.Tests.csproj -c Release -m:1 --filter "FullyQualifiedName~AutomationProposalsApiTests|FullyQualifiedName~ProposalRevisionApiTests" — 94 passed, 0 failed.
• dotnet test backend/tests/Taskdeck.Architecture.Tests/Taskdeck.Architecture.Tests.csproj -c Release -m:1 — 28 passed, 1 skipped, 0 failed.
• dotnet test backend/Taskdeck.sln -c Release -m:1 — 8850 passed, 34 skipped, 0 failed across all six assemblies (Domain 1603, Application 4171, Api 2835 with 4 skipped, Cli 206, Architecture 28 with 1 skipped, Integration 7 with 29 skipped). No pre-existing failures appeared.
• cd frontend/taskdeck-web; npx vitest --run --maxWorkers=2 src/tests/components/review src/tests/views/ReviewView.spec.ts src/tests/composables/useProposalDisplayNames.spec.ts — 12 files, 184 passed, 0 failed.
• npm run typecheck, npx eslint src/components/review/ReviewProposalCard.vue src/tests/components/review/ReviewProposalCard.diff.spec.ts, npm run build, git diff --check — all clean.

Red before green. Each test was written and run against the unfixed code first:

• GetProposalByIdAsync_ShouldOrderOperationsBySequence_WhenStoredOutOfOrder failed with {2, 1, 0} differs at index 0.
• GetProposalByIdAsync_ShouldOrderRevisedOperationsBySequence_WhenPayloadIsOutOfOrder failed with {1, 0} differs at index 0.
• GetProposal_ShouldReturnOperationsInSequenceOrder_WhenStoredOutOfSequenceOrder failed on the create response with {2, 1, 0} differs at index 0.
• Both new component cases failed, the second rendering "Move card." where "Move card to “Done”." was expected, which is the enrichment loss described above.

Mutation checks, each restored afterwards:

• Removed the MapToDto sort: the Application test failed with {2, 1, 0} and the API test failed with {2, 1, 0}.
• Removed the BuildEffectiveProposalDto sort: the revision test failed with {1, 0}.
• Removed the ReviewProposalCard sort: both new component cases failed.

NOT verified:

• No browser or E2E run. The reviewer-facing proof is the component spec, not a rendered page.
• The GET half of the API test did not go red on its own. SQLite's planner currently serves that Include from the (ProposalId, Sequence) index, so the persisted read already returned sequence order. It is kept as a contract lock and the test's doc comment says so; the deterministic wire reproduction is the create response.
• The Paper review surface was not re-tested beyond its existing specs passing, since it already sorted and is unchanged.

Boundaries and risks

• Preview == Apply is unchanged. Apply does not consume this DTO ordering. AutomationExecutorService re-parses the pinned revision itself and sorts by Sequence before dispatch, so sorting the response array cannot change what Apply executes.
• BuildPresentation is untouched, and so is the operation vocabulary and Apply dispatch.
• PaperReviewView.vue is not edited. It already sorts before indexing and is held by fix(review): bound the post-revision truth refresh and keep it retryable #2576.
• Tie behaviour. Sequence is only validated as non-negative, so duplicates are possible. OrderBy in LINQ and Array.prototype.sort in modern V8 are both stable, so operations sharing a Sequence keep their relative source order rather than being shuffled. Ordering within a tie is therefore unchanged, not newly defined.
• CardHistoryService numbers its pending rows by array index, so those serials now follow sequence order. No test asserted the previous order.
• Branch is based on 2803355 and is behind origin/main by seven documentation-only commits with no overlap in the changed files. fix(api): bring batch approve to batch execute's input and commit-boundary parity #2597, which edits the batch-approve region of the same service file, had not landed at the time of this run, so no merge of origin/main was performed.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Review record (alpha product-trust lane, review-and-ship round 1).

Reviewer: one fresh-context independent reviewer subagent (read-only), input = merge-base..head diff at 0800e31 plus the worktree at that head. Verdict: SHIP, no CRITICAL or HIGH.

What the reviewer refuted and could not break: the DTO sort cannot move what Apply executes (the executor re-parses the pinned revision payload and re-sorts by Sequence before dispatch); the order contract is total (one ProposalDto constructor site, one Operations replacement site, both sorted); pairing holds under duplicate sequences because OrderBy is stable and every consumer re-sorts the same list; saved revisions cannot carry null, negative or duplicate sequences (ProposalRevisionService and ProposalOperationStructureValidator), so only array order can disagree, which is what this PR normalizes; an independent consumer audit found no order-dependent consumer left unsorted; the frontend sort copies before sorting and the #2541 enrichment gate is untouched.

Triage of the three LOWs (none blocks; no fix commit):

• LOW-1 (ReviewProposalCard.vue:138, comparator not NaN-guarded unlike ReviewAppliedDecisionRecord.vue:61-68): declined for this PR. Sequence is an int on the wire and the plain comparator is the codebase norm at five other sites; hardening all of them together is a separate change. Noted on [Frontend][UX] Documented bare-letter shortcuts (H/T/B/I/R, G T, board C/R) have no handler, and Settings has no Keyboard page #1968's sibling list is not appropriate; recorded here only.
• LOW-2 (AutomationProposalService.cs:1668, tie stability asserted in a comment without a test): tracked as a follow-up test in the [Frontend][Review] Headlines are paired to operations by index while the two arrays use different orderings #2563 closing comment; not blocking because both arrays derive from the same stable in-memory order today.
• LOW-3 (no doc records the ordered wire contract): coordinator carry-forward. The invariant line goes into autodoc/interfaces/proposal-operation-vocabulary.md with the STATUS twelfth block.
• INFO (overlap with fix(api): bring batch approve to batch execute's input and commit-boundary parity #2597 in AutomationProposalService.cs, hunks more than 350 lines apart): whichever of fix(api): bring batch approve to batch execute's input and commit-boundary parity #2597 and fix(review): pair proposal headlines to operations by sequence, not by array index #2609 lands second takes a base change and is re-proved (ci-required at the updated head plus the Application filter locally) before merge.

Merge gate: ci-required green at the exact head, aged three minutes, then merge commit.