mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-26 00:53:48 +00:00
While a response streams, session stats / pending-item updates rebuild commandContext. useCommandSuggestions listed commandContext in its effect deps even though only the async argument-completion callback consumes it, so every context rebuild re-ran the search and replaced the suggestions array with an identical-content copy. useCommandCompletion resets the active index to 0 whenever the suggestions array identity changes, which snapped the user's menu selection back to the first item mid-navigation. Read the context through a ref (same pattern as historyRef in slashCommandProcessor) and drop it from the deps so only real inputs — query, command list, recent commands — rebuild the suggestions. Adds hook-level regression tests wiring real useCommandCompletion + useSlashCompletion: selection survives context churn (red before the fix), query/command-list changes still reset it, and argument completion still observes the latest context.
This commit is contained in:
parent
414187a828
commit
e44a16d8b5
2 changed files with 239 additions and 9 deletions
|
|
@ -0,0 +1,222 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2026 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
/** @vitest-environment jsdom */
|
||||
|
||||
// Regression tests for #9494: while a response streams, AppContainer state
|
||||
// (session stats, pending item) changes identity, which rebuilds
|
||||
// `commandContext`. That churn must NOT rebuild the slash suggestion list —
|
||||
// rebuilding replaces the suggestions array, and the reset effect in
|
||||
// useCommandCompletion then snaps the user's menu selection back to the
|
||||
// first item.
|
||||
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { renderHook, act, waitFor } from '@testing-library/react';
|
||||
import { useCommandCompletion } from './useCommandCompletion.js';
|
||||
import type { CommandContext, SlashCommand } from '../commands/types.js';
|
||||
import { CommandKind } from '../commands/types.js';
|
||||
import { useTextBuffer } from '../components/shared/text-buffer.js';
|
||||
|
||||
vi.mock('./useAtCompletion', () => ({
|
||||
useAtCompletion: vi.fn(),
|
||||
}));
|
||||
|
||||
function createTestCommand(command: Partial<SlashCommand>): SlashCommand {
|
||||
return {
|
||||
kind: CommandKind.BUILT_IN,
|
||||
name: 'test',
|
||||
description: 'test command',
|
||||
...command,
|
||||
} as SlashCommand;
|
||||
}
|
||||
|
||||
const defaultCommands: readonly SlashCommand[] = [
|
||||
createTestCommand({ name: 'memory', description: 'Manage memory' }),
|
||||
createTestCommand({ name: 'model', description: 'Switch the model' }),
|
||||
createTestCommand({ name: 'stats', description: 'Show session stats' }),
|
||||
];
|
||||
|
||||
function makeContext(marker: string): CommandContext {
|
||||
return { services: { marker } } as unknown as CommandContext;
|
||||
}
|
||||
|
||||
interface HarnessProps {
|
||||
ctx: CommandContext;
|
||||
commands?: readonly SlashCommand[];
|
||||
}
|
||||
|
||||
function useHarness({ ctx, commands }: HarnessProps) {
|
||||
const buffer = useTextBuffer({
|
||||
initialText: '/',
|
||||
initialCursorOffset: 1,
|
||||
viewport: { width: 80, height: 20 },
|
||||
isValidPath: () => false,
|
||||
onChange: () => {},
|
||||
});
|
||||
const completion = useCommandCompletion(
|
||||
buffer,
|
||||
'/',
|
||||
commands ?? defaultCommands,
|
||||
ctx,
|
||||
false,
|
||||
);
|
||||
return { completion, buffer };
|
||||
}
|
||||
|
||||
// Wait long enough for the async fuzzy-search pipeline (AsyncFzf) triggered
|
||||
// by a dep change to settle.
|
||||
async function flushCompletionPipeline() {
|
||||
await act(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 200));
|
||||
});
|
||||
}
|
||||
|
||||
type HarnessResult = ReturnType<typeof useHarness>;
|
||||
|
||||
describe('slash completion during commandContext churn (#9494)', () => {
|
||||
it('keeps the suggestions array stable when only commandContext identity changes', async () => {
|
||||
const { result, rerender } = renderHook<HarnessResult, HarnessProps>(
|
||||
useHarness,
|
||||
{
|
||||
initialProps: { ctx: makeContext('A') },
|
||||
},
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.completion.suggestions.length).toBeGreaterThan(1);
|
||||
});
|
||||
|
||||
const before = result.current.completion.suggestions;
|
||||
|
||||
// Simulate streaming churn: the query is untouched, but the context
|
||||
// object is rebuilt (session stats / pending item changed upstream).
|
||||
rerender({ ctx: makeContext('B') });
|
||||
await flushCompletionPipeline();
|
||||
|
||||
expect(result.current.completion.suggestions).toBe(before);
|
||||
});
|
||||
|
||||
it('preserves the user menu selection across commandContext rebuilds', async () => {
|
||||
const { result, rerender } = renderHook<HarnessResult, HarnessProps>(
|
||||
useHarness,
|
||||
{
|
||||
initialProps: { ctx: makeContext('A') },
|
||||
},
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.completion.suggestions.length).toBeGreaterThan(1);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
result.current.completion.navigateDown();
|
||||
});
|
||||
expect(result.current.completion.activeSuggestionIndex).toBe(1);
|
||||
|
||||
rerender({ ctx: makeContext('B') });
|
||||
await flushCompletionPipeline();
|
||||
|
||||
expect(result.current.completion.activeSuggestionIndex).toBe(1);
|
||||
});
|
||||
|
||||
it('still resets the selection when the query actually changes', async () => {
|
||||
const { result } = renderHook<HarnessResult, HarnessProps>(useHarness, {
|
||||
initialProps: { ctx: makeContext('A') },
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.completion.suggestions.length).toBeGreaterThan(1);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
result.current.completion.navigateDown();
|
||||
});
|
||||
expect(result.current.completion.activeSuggestionIndex).toBe(1);
|
||||
|
||||
act(() => {
|
||||
result.current.buffer.setText('/s');
|
||||
});
|
||||
// "/s" only matches the 'stats' command — wait for the rebuilt list.
|
||||
await waitFor(() => {
|
||||
expect(result.current.completion.suggestions.length).toBe(1);
|
||||
});
|
||||
|
||||
expect(result.current.completion.activeSuggestionIndex).toBe(0);
|
||||
});
|
||||
|
||||
it('still rebuilds suggestions when the command list changes', async () => {
|
||||
const { result, rerender } = renderHook<HarnessResult, HarnessProps>(
|
||||
useHarness,
|
||||
{
|
||||
initialProps: { ctx: makeContext('A') },
|
||||
},
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.completion.suggestions.length).toBeGreaterThan(1);
|
||||
});
|
||||
const before = result.current.completion.suggestions;
|
||||
|
||||
rerender({
|
||||
ctx: makeContext('A'),
|
||||
commands: [
|
||||
createTestCommand({ name: 'memory', description: 'Manage memory' }),
|
||||
],
|
||||
});
|
||||
await waitFor(() => {
|
||||
expect(result.current.completion.suggestions).not.toBe(before);
|
||||
});
|
||||
expect(result.current.completion.suggestions.length).toBe(1);
|
||||
});
|
||||
|
||||
it('argument completion still receives the latest commandContext', async () => {
|
||||
const completionSpy = vi.fn<
|
||||
(context: CommandContext, argString: string) => Promise<string[]>
|
||||
>().mockResolvedValue(['arg-a', 'arg-b']);
|
||||
const markerOf = (callIndex: number): string =>
|
||||
(
|
||||
completionSpy.mock.calls[callIndex]![0] as unknown as {
|
||||
services: { marker: string };
|
||||
}
|
||||
).services.marker;
|
||||
const commands = [
|
||||
createTestCommand({
|
||||
name: 'deploy',
|
||||
description: 'Deploy things',
|
||||
completion: completionSpy,
|
||||
}),
|
||||
];
|
||||
|
||||
const { result, rerender } = renderHook<HarnessResult, HarnessProps>(
|
||||
useHarness,
|
||||
{
|
||||
initialProps: { ctx: makeContext('A'), commands },
|
||||
},
|
||||
);
|
||||
|
||||
// Switch to an argument-completion query ("/deploy ").
|
||||
act(() => {
|
||||
result.current.buffer.setText('/deploy ');
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(completionSpy).toHaveBeenCalled();
|
||||
});
|
||||
expect(markerOf(0)).toBe('A');
|
||||
|
||||
completionSpy.mockClear();
|
||||
// New context identity AND a changed argument query: the async
|
||||
// completion must observe the new context, not a stale one.
|
||||
rerender({ ctx: makeContext('B'), commands });
|
||||
act(() => {
|
||||
result.current.buffer.setText('/deploy x');
|
||||
});
|
||||
await waitFor(() => {
|
||||
expect(completionSpy).toHaveBeenCalled();
|
||||
});
|
||||
expect(markerOf(0)).toBe('B');
|
||||
});
|
||||
});
|
||||
|
|
@ -4,7 +4,7 @@
|
|||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { useState, useEffect, useMemo } from 'react';
|
||||
import { useState, useEffect, useMemo, useRef, useLayoutEffect } from 'react';
|
||||
import { AsyncFzf } from 'fzf';
|
||||
import { createDebugLogger } from '@qwen-code/qwen-code-core';
|
||||
import type { Suggestion } from '../components/SuggestionsDisplay.js';
|
||||
|
|
@ -336,6 +336,17 @@ function useCommandSuggestions(
|
|||
const [suggestions, setSuggestions] = useState<Suggestion[]>([]);
|
||||
const [isLoading, setIsLoading] = useState(false);
|
||||
|
||||
// The context is only consumed by the async argument-completion callback.
|
||||
// Reading it through a ref keeps unrelated context churn (session stats /
|
||||
// pending item updates while a response streams rebuild commandContext)
|
||||
// out of the effect deps below: re-running this effect replaces the
|
||||
// suggestions array, which snaps the user's menu selection back to the
|
||||
// first item (#9494). Same historyRef pattern as slashCommandProcessor.
|
||||
const commandContextRef = useRef(commandContext);
|
||||
useLayoutEffect(() => {
|
||||
commandContextRef.current = commandContext;
|
||||
}, [commandContext]);
|
||||
|
||||
useEffect(() => {
|
||||
const abortController = new AbortController();
|
||||
const { signal } = abortController;
|
||||
|
|
@ -369,7 +380,7 @@ function useCommandSuggestions(
|
|||
const results =
|
||||
(await leafCommand.completion(
|
||||
{
|
||||
...commandContext,
|
||||
...commandContextRef.current,
|
||||
invocation: {
|
||||
raw: `/${rawParts.join(' ')}`,
|
||||
name: leafCommand.name,
|
||||
|
|
@ -508,13 +519,10 @@ function useCommandSuggestions(
|
|||
|
||||
setSuggestions([]);
|
||||
return () => abortController.abort();
|
||||
}, [
|
||||
parserResult,
|
||||
commandContext,
|
||||
getFzfForCommands,
|
||||
getPrefixSuggestions,
|
||||
recentCommands,
|
||||
]);
|
||||
// commandContext is deliberately absent: it is read through
|
||||
// commandContextRef so context-identity churn alone never re-runs the
|
||||
// search and rebuilds the suggestions array (#9494).
|
||||
}, [parserResult, getFzfForCommands, getPrefixSuggestions, recentCommands]);
|
||||
|
||||
return { suggestions, isLoading };
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue