Suppress selectionchange echo in completion pipeline

ts/packages/dispatcher/dispatcher/src/helpers/completion/session.ts

@@ -245,6 +245,20 @@ export class PartialCompletionSession implements CompletionController {
         input: string,
         direction: CompletionDirection = "forward",
     ): void {
+        // Deduplicate: if the input and direction haven't changed and
+        // the session is already active or pending, there is no new
+        // work to do.  The shell's previousInput guard prevents
+        // selectionchange echoes from reaching here, so direction is
+        // safe to compare.
+        if (
+            input === this.lastInput &&
+            direction === this.lastDirection &&
+            (this.completionState !== undefined ||
+                this.completionP !== undefined)
+        ) {
+            debug(`update skipped: input unchanged ('${input}')`);
+            return;
+        }
         this.lastInput = input;
         this.lastDirection = direction;
         if (this.reuseSession(input, direction)) {

ts/packages/shell/src/renderer/src/partial.ts

@@ -65,10 +65,6 @@ export class PartialCompletion {
     // items) should be called on the SearchMenu.
     private lastGeneration: number = -1;
     private lastPrefix: string = "";
-    // Set when update(contentChanged=true) runs so the selectionchange
-    // handler can skip the redundant re-update that the browser fires
-    // after every content mutation.
-    private contentUpdated: boolean = false;
 
     private readonly cleanupEventListeners: () => void;
     constructor(
@@ -96,6 +92,9 @@ export class PartialCompletion {
         // pass them directly to avoid a redundant trie query in render().
         this.controller.setOnUpdate(() => {
             const state = this.controller.getCompletionState();
+            debug(
+                `onUpdate: ${state ? `prefix='${state.prefix}' items=${state.items.length}` : "hidden"}`,
+            );
             if (state) {
                 if (
                     state.generation !== this.lastGeneration ||
@@ -119,10 +118,6 @@ export class PartialCompletion {
         });
 
         const selectionChangeHandler = () => {
-            if (this.contentUpdated) {
-                this.contentUpdated = false;
-                return;
-            }
             debug("Partial completion update on selection changed");
             this.update(false);
         };
@@ -144,20 +139,32 @@ export class PartialCompletion {
         if (this.closed) {
             throw new Error("Using a closed PartialCompletion");
         }
-        debug(`update(contentChanged=${contentChanged})`);
+        debug(`update entry: contentChanged=${contentChanged}`);
         if (contentChanged) {
             // Normalize the input text to ensure selection at end is correct.
             this.input.getTextEntry().normalize();
-            // Suppress the selectionchange event that the browser fires
-            // after every content mutation — this update already covers it.
-            this.contentUpdated = true;
         }
         if (!this.isSelectionAtEnd(contentChanged)) {
+            this.previousInput = "";
             this.controller.hide();
+            debug("update: selection not at end, hiding");
             return;
         }
         const input = this.getCurrentInputForCompletion();
-        debug(`Partial completion input: '${input}'`);
+
+        // Skip if input hasn't changed since the last call we forwarded
+        // to the controller.  This prevents selectionchange echoes from
+        // recomputing direction against the already-mutated previousInput
+        // (which would flip "backward" to "forward").
+        // Reset to "" on hide (cursor moved away) so re-focus always
+        // re-activates the controller.
+        if (input === this.previousInput) {
+            debug(`update skipped: input unchanged ('${input}')`);
+            return;
+        }
+        debug(
+            `Partial completion input: '${input}' (${contentChanged ? "content changed" : "selection changed"})`,
+        );
 
         // Only use "backward" when the user is genuinely backspacing:
         // the new input must be a strict prefix of the previous input.

ts/packages/shell/test/completionUpdate.spec.ts

@@ -40,17 +40,25 @@ function collectConsoleMessages(page: Page, substring: string) {
 }
 
 test.describe("Partial completion update suppression", () => {
-    test("typing triggers exactly one update per keystroke", async () => {
+    test("typing triggers exactly one content-change update per keystroke", async () => {
         await runTestCallback(async (mainWindow: Page) => {
             await enablePartialDebug(mainWindow);
 
             const contentTrue = collectConsoleMessages(
                 mainWindow,
-                "update(contentChanged=true)",
+                "content changed",
             );
-            const contentFalse = collectConsoleMessages(
+            const updateEntries = collectConsoleMessages(
                 mainWindow,
-                "update(contentChanged=false)",
+                "update entry:",
+            );
+            const updateSkipped = collectConsoleMessages(
+                mainWindow,
+                "update skipped:",
+            );
+            const updateHidden = collectConsoleMessages(
+                mainWindow,
+                "selection not at end",
             );
 
             const input = mainWindow.locator(inputSelector);
@@ -61,13 +69,23 @@ test.describe("Partial completion update suppression", () => {
             await expect(async () => {
                 expect(contentTrue.length).toBe(1);
             }).toPass({ timeout: 2000 });
-            expect(contentFalse.length).toBe(0);
+
+            // Wait a beat for any straggling selectionchange echoes.
+            await mainWindow.waitForTimeout(500);
+
+            // Every update() call must either be the content-change
+            // path, deduped by the previousInput guard, or hidden
+            // because selection moved away from the end.  No call
+            // may slip through to recompute direction.
+            expect(updateEntries.length).toBe(
+                contentTrue.length + updateSkipped.length + updateHidden.length,
+            );
 
             await mainWindow.keyboard.press("Escape");
         });
     });
 
-    test("second keystroke also triggers exactly one update", async () => {
+    test("second keystroke also triggers exactly one content-change update", async () => {
         await runTestCallback(async (mainWindow: Page) => {
             await enablePartialDebug(mainWindow);
 
@@ -90,25 +108,41 @@ test.describe("Partial completion update suppression", () => {
             // Start collecting after the first keystroke has settled.
             const contentTrue = collectConsoleMessages(
                 mainWindow,
-                "update(contentChanged=true)",
+                "content changed",
+            );
+            const updateEntries = collectConsoleMessages(
+                mainWindow,
+                "update entry:",
+            );
+            const updateSkipped = collectConsoleMessages(
+                mainWindow,
+                "update skipped:",
             );
-            const contentFalse = collectConsoleMessages(
+            const updateHidden = collectConsoleMessages(
                 mainWindow,
-                "update(contentChanged=false)",
+                "selection not at end",
             );
 
             await mainWindow.keyboard.type("l", { delay: 0 });
 
             await expect(async () => {
                 expect(contentTrue.length).toBe(1);
             }).toPass({ timeout: 2000 });
-            expect(contentFalse.length).toBe(0);
+
+            // Wait a beat for any straggling selectionchange echoes.
+            await mainWindow.waitForTimeout(500);
+
+            // Every update() call must either be the content-change
+            // path, deduped by the previousInput guard, or hidden.
+            expect(updateEntries.length).toBe(
+                contentTrue.length + updateSkipped.length + updateHidden.length,
+            );
 
             await mainWindow.keyboard.press("Escape");
         });
     });
 
-    test("backspace triggers exactly one update", async () => {
+    test("backspace triggers exactly one content-change update", async () => {
         await runTestCallback(async (mainWindow: Page) => {
             await enablePartialDebug(mainWindow);
 
@@ -131,19 +165,35 @@ test.describe("Partial completion update suppression", () => {
             // Start collecting after the initial typing has settled.
             const contentTrue = collectConsoleMessages(
                 mainWindow,
-                "update(contentChanged=true)",
+                "content changed",
+            );
+            const updateEntries = collectConsoleMessages(
+                mainWindow,
+                "update entry:",
             );
-            const contentFalse = collectConsoleMessages(
+            const updateSkipped = collectConsoleMessages(
                 mainWindow,
-                "update(contentChanged=false)",
+                "update skipped:",
+            );
+            const updateHidden = collectConsoleMessages(
+                mainWindow,
+                "selection not at end",
             );
 
             await mainWindow.keyboard.press("Backspace");
 
             await expect(async () => {
                 expect(contentTrue.length).toBe(1);
             }).toPass({ timeout: 2000 });
-            expect(contentFalse.length).toBe(0);
+
+            // Wait a beat for any straggling selectionchange echoes.
+            await mainWindow.waitForTimeout(500);
+
+            // Every update() call must either be the content-change
+            // path, deduped by the previousInput guard, or hidden.
+            expect(updateEntries.length).toBe(
+                contentTrue.length + updateSkipped.length + updateHidden.length,
+            );
 
             await mainWindow.keyboard.press("Escape");
         });

ts/packages/shell/test/partialCompletion/renderDispatch.spec.ts

@@ -86,7 +86,7 @@ describe("render vs updatePosition dispatch", () => {
         expect(menu.updatePositionCalls.length).toBe(0);
     });
 
-    test("same generation + same prefix calls updatePosition", async () => {
+    test("same input + direction is deduplicated (no callback)", async () => {
         const result = makeCompletionResult(["song", "shuffle"], 4, {
             separatorMode: "optionalSpace",
         });
@@ -99,10 +99,32 @@ describe("render vs updatePosition dispatch", () => {
         await Promise.resolve();
 
         const rendersBefore = menu.renderCalls.length;
+        const updatePosBefore = menu.updatePositionCalls.length;
 
-        // Same input — same generation, same prefix.
+        // Same input, same direction — controller skips entirely.
         controller.update("play", "forward");
 
+        expect(menu.renderCalls.length).toBe(rendersBefore);
+        expect(menu.updatePositionCalls.length).toBe(updatePosBefore);
+    });
+
+    test("same input + different direction calls updatePosition", async () => {
+        const result = makeCompletionResult(["song", "shuffle"], 4, {
+            separatorMode: "optionalSpace",
+        });
+        const dispatcher = makeDispatcher(result);
+        const controller = createCompletionController(dispatcher);
+        const menu = new MockSearchMenu();
+        wireController(controller, menu);
+
+        controller.update("play", "forward");
+        await Promise.resolve();
+
+        const rendersBefore = menu.renderCalls.length;
+
+        // Same input, different direction — passes dedup, reuse triggers updatePosition.
+        controller.update("play", "backward");
+
         expect(menu.renderCalls.length).toBe(rendersBefore);
         expect(menu.updatePositionCalls.length).toBe(1);
     });