fix(editor): restore unmerge option + in-editor merge confirmation (#691)

src/providers/codexCellEditorProvider/codexCellEditorMessagehandling.ts

@@ -3121,7 +3121,7 @@ const messageHandlers: Record<string, (ctx: MessageHandlerContext) => Promise<vo
 
     confirmCellMerge: async ({ event, document, webviewPanel, provider }) => {
         const typedEvent = event as Extract<EditorPostMessages, { command: "confirmCellMerge"; }>;
-        const { currentCellId, previousCellId, currentContent, previousContent, message } = typedEvent.content;
+        const { currentCellId, previousCellId, currentContent, previousContent } = typedEvent.content;
 
         debug("confirmCellMerge message received for cells:", { currentCellId, previousCellId });
 
@@ -3149,45 +3149,36 @@ const messageHandlers: Record<string, (ctx: MessageHandlerContext) => Promise<vo
                 }
             }
 
-            // No child cells found, proceed with existing confirmation flow
-            const confirmed = await vscode.window.showWarningMessage(
-                message,
-                { modal: false },
-                "Yes",
-                "No"
-            );
-
-            if (confirmed === "Yes") {
-                // User confirmed, proceed with merge
-                const mergeEvent: EditorPostMessages = {
-                    command: "mergeCellWithPrevious" as const,
-                    content: {
-                        currentCellId,
-                        previousCellId,
-                        currentContent,
-                        previousContent
-                    }
-                };
+            // No child cells found. Confirmation already happened in the webview modal, so
+            // proceed directly with the merge.
+            const mergeEvent: EditorPostMessages = {
+                command: "mergeCellWithPrevious" as const,
+                content: {
+                    currentCellId,
+                    previousCellId,
+                    currentContent,
+                    previousContent
+                }
+            };
 
-                // Call the existing merge handler
-                await messageHandlers.mergeCellWithPrevious({
-                    event: mergeEvent,
-                    document,
-                    webviewPanel,
-                    provider,
-                    updateWebview: () => {
-                        provider.refreshWebview(webviewPanel, document);
-                    }
-                });
+            // Call the existing merge handler
+            await messageHandlers.mergeCellWithPrevious({
+                event: mergeEvent,
+                document,
+                webviewPanel,
+                provider,
+                updateWebview: () => {
+                    provider.refreshWebview(webviewPanel, document);
+                }
+            });
 
-                // Only merge in target if we're working with a source file
-                if (isSourceFile) {
-                    const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri);
-                    if (!workspaceFolder) {
-                        throw new Error("No workspace folder found");
-                    }
-                    await provider.mergeMatchingCellsInTargetFile(currentCellId, previousCellId, document.uri.toString(), workspaceFolder);
+            // Only merge in target if we're working with a source file
+            if (isSourceFile) {
+                const workspaceFolder = vscode.workspace.getWorkspaceFolder(document.uri);
+                if (!workspaceFolder) {
+                    throw new Error("No workspace folder found");
                 }
+                await provider.mergeMatchingCellsInTargetFile(currentCellId, previousCellId, document.uri.toString(), workspaceFolder);
             }
         } catch (error) {
             console.error("Error in confirmCellMerge:", error);

types/index.d.ts

@@ -603,7 +603,6 @@ export type EditorPostMessages =
             previousCellId: string;
             currentContent: string;
             previousContent: string;
-            message: string;
         };
     }
     | {

webviews/codex-webviews/src/CodexCellEditor/CellContentDisplay.tsx

@@ -137,6 +137,13 @@ const CellContentDisplay: React.FC<CellContentDisplayProps> = React.memo(
         const [showSparkleButton, setShowSparkleButton] = useState(false);
         const [showAuthModal, setShowAuthModal] = useState(false);
         const [showOfflineModal, setShowOfflineModal] = useState(false);
+        // Pending merge awaiting in-editor confirmation (replaces the native VS Code popup)
+        const [pendingMerge, setPendingMerge] = useState<{
+            currentCellId: string;
+            previousCellId: string;
+            currentContent: string;
+            previousContent: string;
+        } | null>(null);
         const [isLockButtonGlowing, setIsLockButtonGlowing] = useState(false);
         const [isLockButtonFlashing, setIsLockButtonFlashing] = useState(false);
         const [isResolvingStructure, setIsResolvingStructure] = useState(false);
@@ -390,18 +397,27 @@ const CellContentDisplay: React.FC<CellContentDisplayProps> = React.memo(
                 return;
             }
 
-            // Send confirmation request to VS Code instead of using window.confirm
+            // Open an in-editor confirmation modal instead of the native VS Code popup
+            // (which appears in the bottom-right corner of the screen, far from the cell).
+            setPendingMerge({
+                currentCellId: currentCell.cellMarkers[0],
+                previousCellId: targetCell.cellMarkers[0],
+                currentContent: currentCell.cellContent,
+                previousContent: targetCell.cellContent,
+            });
+        };
+
+        const handleConfirmMerge = () => {
+            if (!pendingMerge) return;
             vscode.postMessage({
                 command: "confirmCellMerge",
-                content: {
-                    currentCellId: currentCell.cellMarkers[0],
-                    previousCellId: targetCell.cellMarkers[0],
-                    currentContent: currentCell.cellContent,
-                    previousContent: targetCell.cellContent,
-                    message:
-                        "Are you sure you want to merge this cell with the previous non-merged cell? This action cannot be undone.",
-                },
+                content: pendingMerge,
             } as any);
+            setPendingMerge(null);
+        };
+
+        const handleCancelMergeConfirmation = () => {
+            setPendingMerge(null);
         };
 
         // Line numbers are always generated and shown at the beginning of each line
@@ -977,6 +993,34 @@ const CellContentDisplay: React.FC<CellContentDisplayProps> = React.memo(
                                             </Button>
                                         </div>
                                     )}
+                                    <Dialog
+                                        open={!!pendingMerge}
+                                        onOpenChange={(open) => {
+                                            if (!open) handleCancelMergeConfirmation();
+                                        }}
+                                    >
+                                        <DialogContent>
+                                            <DialogHeader>
+                                                <DialogTitle>Merge with previous cell</DialogTitle>
+                                                <DialogDescription>
+                                                    This cell will be merged into the previous
+                                                    non-merged cell. You can reverse this later by
+                                                    unmerging the cell.
+                                                </DialogDescription>
+                                            </DialogHeader>
+                                            <DialogFooter>
+                                                <Button
+                                                    variant="secondary"
+                                                    onClick={handleCancelMergeConfirmation}
+                                                >
+                                                    Cancel
+                                                </Button>
+                                                <Button autoFocus onClick={handleConfirmMerge}>
+                                                    Merge
+                                                </Button>
+                                            </DialogFooter>
+                                        </DialogContent>
+                                    </Dialog>
                                 </div>
                             </div>
                         )}

webviews/codex-webviews/src/CodexCellEditor/CellList.tsx

@@ -157,24 +157,21 @@ const CellList: React.FC<CellListProps> = ({
     // State to track unresolved comments count for each cell
     const [cellCommentsCount, setCellCommentsCount] = useState<Map<string, number>>(new Map());
 
-    // Filter out merged cells if we're in correction editor mode for source text
     const filteredTranslationUnits = useMemo(() => {
         let filtered = translationUnits;
 
-        // Filter out merged cells if we're in correction editor mode for source text
-        if (isSourceText && isCorrectionEditorMode) {
-            filtered = filtered.filter((unit) => {
-                // Check if cell has merged metadata in the data property
-                const cellData = unit.data as any;
-                return !cellData?.merged;
-            });
-        }
+        // NOTE: Do NOT filter out merged cells here. In source + correction editor mode the
+        // provider intentionally keeps merged cells in the translation units (see
+        // mergeRangesAndProcess) so they render greyed-out with the "unmerge" button. In every
+        // other mode the provider already strips merged cells, so they never reach this list.
+        // Filtering them here previously hid them in correction mode, which removed the only
+        // place the unmerge button could appear (issue #691).
 
         // Filter out milestone cells from the view (they remain in JSON)
         filtered = filtered.filter((unit) => unit.cellType !== CodexCellTypes.MILESTONE);
 
         return filtered;
-    }, [translationUnits, isSourceText, isCorrectionEditorMode]);
+    }, [translationUnits]);
     // Use filtered units for all operations
     const workingTranslationUnits = filteredTranslationUnits;
     // State to track completed translations (only successful ones) - REMOVED: Now handled by parent

webviews/codex-webviews/src/CodexCellEditor/__tests___/CellContentDisplay.mergeConfirm.test.tsx

@@ -0,0 +1,173 @@
+import React from "react";
+import { describe, it, expect, beforeEach, vi } from "vitest";
+import { render, screen, fireEvent } from "@testing-library/react";
+import { QuillCellContent } from "../../../../../types";
+import { CodexCellTypes } from "../../../../../types/enums";
+import CellContentDisplay from "../CellContentDisplay";
+
+// Regression test for the #691 follow-up: the merge confirmation is now an in-editor modal
+// (rendered inside the source editor) instead of a native VS Code popup in the screen corner.
+// Clicking the merge button must open the modal and NOT post `confirmCellMerge` until the user
+// confirms in the modal.
+
+const mockVscode = {
+    postMessage: vi.fn(),
+    getState: vi.fn(),
+    setState: vi.fn(),
+};
+
+Object.defineProperty(window, "vscodeApi", {
+    value: mockVscode,
+    writable: true,
+});
+
+global.acquireVsCodeApi = vi.fn().mockReturnValue(mockVscode);
+
+vi.mock("@sharedUtils", () => ({
+    shouldDisableValidation: vi.fn().mockReturnValue(false),
+}));
+
+vi.mock("../contextProviders/UnsavedChangesContext", () => ({
+    default: React.createContext({
+        setUnsavedChanges: vi.fn(),
+        showFlashingBorder: false,
+        unsavedChanges: false,
+        toggleFlashingBorder: vi.fn(),
+    }),
+}));
+
+vi.mock("../contextProviders/TooltipContext", () => ({
+    useTooltip: () => ({
+        showTooltip: vi.fn(),
+        hideTooltip: vi.fn(),
+    }),
+}));
+
+vi.mock("../hooks/useCentralizedMessageDispatcher", () => ({
+    useMessageHandler: vi.fn(() => {}),
+}));
+
+vi.mock("../lib/audioController", () => ({
+    globalAudioController: {
+        playExclusive: vi.fn().mockResolvedValue(undefined),
+        addListener: vi.fn(),
+        removeListener: vi.fn(),
+    },
+}));
+
+vi.mock("../lib/audioCache", () => ({
+    getCachedAudioDataUrl: vi.fn().mockReturnValue(null),
+    setCachedAudioDataUrl: vi.fn(),
+}));
+
+vi.mock("../ValidationButton", () => ({
+    default: () => <div className="validation-button-container" data-testid="validation-button" />,
+}));
+
+vi.mock("../AudioValidationButton", () => ({
+    default: () => (
+        <div className="audio-validation-button-container" data-testid="audio-validation-button" />
+    ),
+}));
+
+vi.mock("../CommentsBadge", () => ({
+    default: () => <div data-testid="comments-badge" />,
+}));
+
+vi.mock("react-markdown", () => ({
+    default: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+}));
+
+const createMockCell = (cellId: string, content: string): QuillCellContent => ({
+    cellMarkers: [cellId],
+    cellContent: content,
+    cellType: CodexCellTypes.TEXT,
+    editHistory: [
+        {
+            editMap: ["value"],
+            value: content,
+            author: "test-user",
+            validatedBy: [],
+            timestamp: Date.now(),
+            type: "user-edit" as any,
+        },
+    ],
+    cellLabel: cellId,
+    timestamps: { startTime: 0, endTime: 5 },
+});
+
+const renderSecondCell = () => {
+    const firstCell = createMockCell("cell-1", "<p>First</p>");
+    const secondCell = createMockCell("cell-2", "<p>Second</p>");
+    const props = {
+        cell: secondCell,
+        vscode: mockVscode as any,
+        textDirection: "ltr" as const,
+        isSourceText: true,
+        hasDuplicateId: false,
+        highlightedCellId: null,
+        scrollSyncEnabled: true,
+        lineNumber: "2",
+        label: "Test Label",
+        lineNumbersEnabled: true,
+        isInTranslationProcess: false,
+        translationState: null as any,
+        allTranslationsComplete: false,
+        handleCellClick: vi.fn(),
+        audioAttachments: { "cell-2": "available" as const },
+        currentUsername: "test-user",
+        requiredValidations: 1,
+        requiredAudioValidations: 1,
+        isCorrectionEditorMode: true,
+        translationUnits: [firstCell, secondCell],
+    };
+    return render(<CellContentDisplay {...props} />);
+};
+
+const clickMergeButton = (container: HTMLElement) => {
+    const mergeButton = container.querySelector(".codicon-merge")?.closest("button");
+    expect(mergeButton).toBeTruthy();
+    fireEvent.click(mergeButton!);
+};
+
+const confirmCellMergeCalls = () =>
+    mockVscode.postMessage.mock.calls.filter(([msg]) => msg?.command === "confirmCellMerge");
+
+describe("CellContentDisplay - merge confirmation modal (#691 follow-up)", () => {
+    beforeEach(() => {
+        vi.clearAllMocks();
+        Element.prototype.scrollIntoView = vi.fn();
+    });
+
+    it("opens the in-editor modal on merge click and does not post confirmCellMerge yet", () => {
+        const { container } = renderSecondCell();
+        clickMergeButton(container);
+
+        // Modal is shown...
+        expect(screen.getByText("Merge with previous cell")).toBeTruthy();
+        // ...but nothing is committed until the user confirms.
+        expect(confirmCellMergeCalls()).toHaveLength(0);
+    });
+
+    it("posts confirmCellMerge (without a message field) when the merge is confirmed", () => {
+        const { container } = renderSecondCell();
+        clickMergeButton(container);
+        fireEvent.click(screen.getByRole("button", { name: "Merge" }));
+
+        const calls = confirmCellMergeCalls();
+        expect(calls).toHaveLength(1);
+        const msg = calls[0][0];
+        expect(msg.content.currentCellId).toBe("cell-2");
+        expect(msg.content.previousCellId).toBe("cell-1");
+        expect(msg.content).not.toHaveProperty("message");
+    });
+
+    it("does not post confirmCellMerge when the modal is cancelled", () => {
+        const { container } = renderSecondCell();
+        clickMergeButton(container);
+        fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
+
+        expect(confirmCellMergeCalls()).toHaveLength(0);
+        expect(screen.queryByText("Merge with previous cell")).toBeNull();
+    });
+});