Non-blocking residual from the fresh-context review of PR #2590 (#2215 Legacy diff-pane re-adoption), tracked here rather than fix-cycled. References are at that PR's round-1 head 2ab5bb7.
LOW, Applied with a pinned newer revision. Pane open live on A at rev-1; A leaves the queue; another reviewer saves rev-2, approves and applies; A returns with approvedRevisionId rev-2. The read-only watcher (useReviewActions.ts ~158-185, the #1397 LOW-5 conversion) converts the pane to the stored presentation, then the revision watcher fires proposalRevisionMoved('rev-1','rev-2') and resetDiffState() wipes it. At the merge base the pair was cleared during the absence, so this case converted and stayed. The four new specs pin only the null-identity terminal transitions (reject, expire). Consequence: a blink plus one extra click (reopening shows the stored preview). For a returning Approved proposal the close is arguably right because Apply would execute rev-2; only the fully-Applied variant is debatable. Decide the intended behaviour and pin it with a spec.
Note, skin divergence. Paper's watcher (PaperReviewView.vue ~566-585) resets on disappearance because it has one active proposal; Legacy now retains per-row pairs. Defensible, but the anti-drift stance (#1124 / ADR-0038) means the rationale comment in useReviewActions.ts should say the sibling skin resolves it the other way and why.
Refs #2215, #1397, PR #2590.
Non-blocking residual from the fresh-context review of PR #2590 (#2215 Legacy diff-pane re-adoption), tracked here rather than fix-cycled. References are at that PR's round-1 head 2ab5bb7.
LOW, Applied with a pinned newer revision. Pane open live on A at rev-1; A leaves the queue; another reviewer saves rev-2, approves and applies; A returns with approvedRevisionId rev-2. The read-only watcher (useReviewActions.ts ~158-185, the #1397 LOW-5 conversion) converts the pane to the stored presentation, then the revision watcher fires proposalRevisionMoved('rev-1','rev-2') and resetDiffState() wipes it. At the merge base the pair was cleared during the absence, so this case converted and stayed. The four new specs pin only the null-identity terminal transitions (reject, expire). Consequence: a blink plus one extra click (reopening shows the stored preview). For a returning Approved proposal the close is arguably right because Apply would execute rev-2; only the fully-Applied variant is debatable. Decide the intended behaviour and pin it with a spec.
Note, skin divergence. Paper's watcher (PaperReviewView.vue ~566-585) resets on disappearance because it has one active proposal; Legacy now retains per-row pairs. Defensible, but the anti-drift stance (#1124 / ADR-0038) means the rationale comment in useReviewActions.ts should say the sibling skin resolves it the other way and why.
Refs #2215, #1397, PR #2590.