diff --git a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts index 5420fc92..8d4b18dc 100644 --- a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts +++ b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts @@ -51,6 +51,15 @@ type UseEmbeddableChatParams = { contextId?: string; }; +/** + * Keys that are held rather than typed. + * + * A modifier press is not editing, and ending a recall on one would break the ordinary act of holding Shift + * between two presses of Up. `Dead` is a pending accent on layouts that compose — also not a keystroke you + * have finished making. + */ +const MODIFIER_KEYS = new Set(['Shift', 'Control', 'Alt', 'Meta', 'CapsLock', 'Dead']); + export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComplete?: () => void) { const { initialMessage, defaultInput = '', promptPrefix, cwd, autoSend = false, chat: externalChat } = params; @@ -440,6 +449,18 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp } if (ev.key === 'ArrowUp') return recallPrompt(ev, -1); if (ev.key === 'ArrowDown') return recallPrompt(ev, 1); + /* + Anything else ends the recall, and Left/Right are the ones that matter. + + A recalled prompt you have started moving around in is a prompt you have adopted — so the next Up must + not walk further back over it, for the same reason Up does not overwrite a fresh draft. Arrowing left + into the middle of it is the clearest signal of intent there is, and it types nothing, so a rule based + on the text changing would miss it entirely. + + Bare modifiers are excluded: holding Shift to capitalise the next letter is not editing, and treating + it as such would end the recall between two presses of Up. + */ + if (!MODIFIER_KEYS.has(ev.key)) history.leave(); if (ev.key === 'Enter' && !ev.shiftKey) { ev.preventDefault(); // Enter on an EMPTY composer with something queued means "don't wait — say it now". The keystroke diff --git a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/usePromptHistory.test.tsx b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/usePromptHistory.test.tsx index c01f41ca..435e7c06 100644 --- a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/usePromptHistory.test.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/usePromptHistory.test.tsx @@ -103,3 +103,44 @@ describe('what gets remembered', () => { expect(current.step(-1, '')).toBe('three'); }); }); + +describe('leaving the history', () => { + // The rule this exists for: a recalled prompt you have started editing — or merely arrowed into — is + // yours now, and the next Up must not walk further back over it. + test('a keystroke after a recall ends it, so Up no longer walks', () => { + const { current } = mount(); + current.record('one'); + current.record('two'); + + expect(current.step(-1, '')).toBe('two'); + expect(current.isRecalling()).toBe(true); + + current.leave(); + expect(current.isRecalling()).toBe(false); + }); + + // The composer's guard is "empty, or recalling". Once we have left, a non-empty box is a draft — so the + // two halves together are what stop Up from eating an edited prompt. + test('after leaving, the recalled text is treated as a draft', () => { + const { current } = mount(); + current.record('one'); + current.step(-1, ''); + current.leave(); + + const mayRecall = (value: string) => value.trim() === '' || current.isRecalling(); + expect(mayRecall('one, edited')).toBe(false); + expect(mayRecall('')).toBe(true); + }); + + // The stashed draft goes with it. Handing an older draft back after you have started editing something + // else would paste the wrong text over the right one. + test('the stashed draft does not come back later', () => { + const { current } = mount(); + current.record('sent'); + expect(current.step(-1, 'a draft')).toBe('sent'); + current.leave(); + // Entering again stashes what is there NOW, and Down returns that rather than the abandoned draft. + expect(current.step(-1, 'something else')).toBe('sent'); + expect(current.step(1, 'sent')).toBe('something else'); + }); +}); diff --git a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/usePromptHistory.ts b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/usePromptHistory.ts index 2c0d1acc..c107aa09 100644 --- a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/usePromptHistory.ts +++ b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/usePromptHistory.ts @@ -34,6 +34,17 @@ export type PromptHistory = { * then refuse. This is the difference between "there is text" and "there is text of yours". */ isRecalling: () => boolean; + /** + * Stop being in the history — the recalled text is now the person's own draft. + * + * Called on any keystroke that is not Up or Down. The moment you edit a recalled prompt, or even just + * move the caret through it with Left, you have adopted it: pressing Up after that would throw away + * something you were working on, which is the same fault as recalling over a fresh draft. + * + * The stashed draft goes too. It belonged to the composing session this leaves, and handing it back after + * you have started editing something else would paste an older draft over a newer one. + */ + leave: () => void; }; export function usePromptHistory(): PromptHistory { @@ -98,5 +109,10 @@ export function usePromptHistory(): PromptHistory { const isRecalling = useCallback(() => indexRef.current !== null, []); - return { record, step, isRecalling }; + const leave = useCallback(() => { + indexRef.current = null; + draftRef.current = ''; + }, []); + + return { record, step, isRecalling, leave }; }