Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions docs/design/acp-channel-initialize-profiling.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,66 @@ telemetry failure isolation, Config event ordering, and the serve fast-path
bundle boundary. The release-built candidate is compared with the exact #6907
merge baseline on the representative 2C4G host with paired, alternating cold
runs before any optimization is selected.

## P0-B optimization decision

The 2C4G P0-A profile attributed 67.3% of child startup P50 to Gemini and ACP
module loading. CPU profiles then showed that source-module compilation was the
largest CPU cost and that the ACP static import graph loaded Ink, React, React
Reconciler, and Yoga even though the ACP child does not render a TUI.

The optional edges were existing UI-only dependencies rather than a new ACP
entry point. The ACP Session imported an API-error classifier through a React
hook; extension completion imported its data shape and result limit through a
render component; the command registry statically loaded UI support needed
only when `/init` asks for confirmation, approval mode enters auto mode, or
collapsed history expands. The optimization moves the two pure data helpers
out of render modules, makes the React type import type-only, and loads the
three interactive action dependencies only when those actions execute.

The ACP initialize response, startup ordering, Config initialization, command
registry contents, failure handling, and Session behavior remain unchanged. A
bundle-metafile check follows the ACP agent's static output closure and rejects
Ink, React, React Reconciler, or Yoga inputs while continuing to allow them
behind dynamic imports.

The causal comparison used release artifacts built from the same main commit,
`af6a9b640c5d9097c5151b8705dd73aee8e180d0`, with only this optimization
applied to the candidate. Two alternating cold runs produced 60 pairs after an
excluded warmup; a separate alternating preheated run produced 30 pairs. The
second cold run was started after the first run exposed two candidate-side
parent-listener stalls before the ACP path. No samples from either run were
discarded. The pooled cold P50 results were:

| Metric | Matched control | P0-B candidate | Change |
| ------------------------- | --------------: | -------------: | -----------------: |
| ACP import | 115.06 ms | 52.00 ms | -63.06 ms (-54.8%) |
| Child process to response | 1102.88 ms | 1041.09 ms | -61.80 ms |
| `channel.initialize` | 1098.25 ms | 1035.61 ms | -62.64 ms |
| Process to first Session | 2046.88 ms | 1980.03 ms | -66.85 ms |
| Cold Session request | 1358.95 ms | 1290.23 ms | -68.72 ms |

All 60 cold profiles in each variant and all 30 preheated profiles in each
variant were complete. Every run exited cleanly, and concurrent first Sessions,
telemetry-disabled startup, and legacy default `single` behavior succeeded in
both functional rounds. In the pooled cold data, warm-Session P95 changed from
137.53 ms to 104.98 ms, first-health P95 from 962.99 ms to 824.14 ms, and
process-tree RSS P95 from 442.27 MiB to 435.70 MiB. In the preheated data,
Session P50 changed from 73.90 ms to 73.75 ms and P95 from 88.38 ms to 76.17 ms.

Transient host-wide stalls affected both variants and were retained. In the
first 30-pair run, two candidate parent-listener stalls raised first-health P95
from 803.82 ms to 1175.67 ms even though the health requests themselves took
6-11 ms and the changed ACP path had not started. The diagnostic retry reversed
the direction, with control/candidate first-health P95 of 1522.44/727.64 ms;
pooling all 60 retained pairs produced the values above. The exact P0-A merge
was also compared with the candidate as a secondary 30-pair check and
independently showed the same ACP-import reduction and no P95 regression.

The module-loading candidate therefore clears the P0-B gate: the selected
phase improves by more than 30% and 10 ms, while both `channel.initialize` and
process-to-first-Session P50 improve by more than 10 ms. Lazy top-level yargs
command builders were rejected because their selected-phase improvement did
not clear the 30% gate. Tool registry and warmup remain a separate descriptor
decoupling design; extension refresh, hierarchical memory, and transport were
too small to justify a P0 behavior change.
2 changes: 1 addition & 1 deletion packages/cli/src/acp-integration/session/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ import {
parseAcpModelOption,
resolveAcpModelOption,
} from '../../utils/acpModelUtils.js';
import { classifyApiError } from '../../ui/hooks/useGeminiStream.js';
import { classifyApiError } from '../../utils/classify-api-error.js';
import { getPersistScopeForModelSelection } from '../../config/modelProvidersScope.js';
import { writeStderrLine } from '../../utils/stdioHelpers.js';
import {
Expand Down
14 changes: 14 additions & 0 deletions packages/cli/src/ui/commands/approvalModeCommand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,20 @@ describe('approvalModeCommand', () => {
expect(mockSetApprovalMode).toHaveBeenCalledWith('yolo');
});

it('should emit the entry notice when switching to auto mode', async () => {
const result = (await approvalModeCommand.action?.(
mockContext,
'auto',
)) as MessageActionReturn;

expect(result.type).toBe('message');
expect(mockSetApprovalMode).toHaveBeenCalledWith('auto');
expect(mockContext.ui.addItem).toHaveBeenCalledWith(
expect.objectContaining({ type: 'info' }),
expect.any(Number),
);
});

it('should set approval mode to "auto-edit" when argument is "auto-edit"', async () => {
const result = (await approvalModeCommand.action?.(
mockContext,
Expand Down
27 changes: 20 additions & 7 deletions packages/cli/src/ui/commands/approvalModeCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
APPROVAL_MODES,
ApprovalMode as ApprovalModeEnum,
} from '@qwen-code/qwen-code-core';
import { emitAutoModeEntryNotices } from '../hooks/useAutoAcceptIndicator.js';
import { formatApprovalModeName } from '../utils/approvalModeDisplay.js';

/**
Expand Down Expand Up @@ -74,6 +73,24 @@ export const approvalModeCommand: SlashCommand = {
if (config) {
try {
priorMode = config.getApprovalMode();
} catch (e) {
return {
type: 'message',
messageType: 'error',
content: (e as Error).message,
};
}
}

const autoModeNotices =
mode === ApprovalModeEnum.AUTO &&
priorMode !== ApprovalModeEnum.AUTO &&
config
? await import('../hooks/useAutoAcceptIndicator.js')
: undefined;

if (config) {
try {
config.setApprovalMode(mode);
} catch (e) {
return {
Expand All @@ -87,12 +104,8 @@ export const approvalModeCommand: SlashCommand = {
// When the user switches INTO AUTO via this command (not just via
// Shift+Tab), emit the same first-time-acknowledgement + stripped-rules
// notices as the keyboard handler.
if (
mode === ApprovalModeEnum.AUTO &&
priorMode !== ApprovalModeEnum.AUTO &&
config
) {
emitAutoModeEntryNotices({
if (autoModeNotices && config) {
autoModeNotices.emitAutoModeEntryNotices({
config,
settings,
addItem: context.ui.addItem,
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/ui/commands/historyCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type { SlashCommand, MessageActionReturn } from './types.js';
import { CommandKind } from './types.js';
import { t } from '../../i18n/index.js';
import { SettingScope } from '../../config/settings.js';
import { expandCollapsedHistory } from '../utils/resumeHistoryUtils.js';

const collapseOnResumeCommand: SlashCommand = {
name: 'collapse-on-resume',
Expand Down Expand Up @@ -61,7 +60,7 @@ const expandNowCommand: SlashCommand = {
},
kind: CommandKind.BUILT_IN,
supportedModes: ['interactive'] as const,
action: (context): MessageActionReturn | void => {
action: async (context): Promise<MessageActionReturn | void> => {
const { history, loadHistory, refreshStatic } = context.ui;

const hasSuppressed = history.some(
Expand All @@ -77,6 +76,9 @@ const expandNowCommand: SlashCommand = {
}

// Remove suppressOnRestore from all items and drop collapse summary items.
const { expandCollapsedHistory } = await import(
'../utils/resumeHistoryUtils.js'
);
const updated = expandCollapsedHistory(history);
loadHistory(updated);
refreshStatic();
Expand Down
18 changes: 18 additions & 0 deletions packages/cli/src/ui/commands/initCommand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
import * as fs from 'node:fs';
import * as path from 'node:path';
import React from 'react';
import { initCommand } from './initCommand.js';
import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
import { type CommandContext } from './types.js';
Expand Down Expand Up @@ -73,6 +74,23 @@ describe('initCommand', () => {
expect(fs.writeFileSync).not.toHaveBeenCalled();
});

it(`should preserve ${DEFAULT_CONTEXT_FILENAME} if the confirmation prompt cannot be built`, async () => {
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.spyOn(fs, 'readFileSync').mockReturnValue('# Existing content');
vi.spyOn(React, 'createElement').mockImplementationOnce(() => {
throw new Error('prompt unavailable');
});

const result = await initCommand.action!(mockContext, '');

expect(result).toEqual({
type: 'message',
messageType: 'error',
content: `Unexpected error preparing ${DEFAULT_CONTEXT_FILENAME}: prompt unavailable`,
});
expect(fs.writeFileSync).not.toHaveBeenCalled();
});

it(`should create ${DEFAULT_CONTEXT_FILENAME} and submit a prompt if it does not exist`, async () => {
// Arrange: Simulate that the file does not exist
vi.mocked(fs.existsSync).mockReturnValue(false);
Expand Down
45 changes: 23 additions & 22 deletions packages/cli/src/ui/commands/initCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import type {
} from './types.js';
import { getCurrentGeminiMdFilename } from '@qwen-code/qwen-code-core';
import { CommandKind } from './types.js';
import { Text } from 'ink';
import React from 'react';
import { t } from '../../i18n/index.js';

export const initCommand: SlashCommand = {
Expand Down Expand Up @@ -42,30 +40,33 @@ export const initCommand: SlashCommand = {
try {
if (fs.existsSync(contextFilePath)) {
// If file exists but is empty (or whitespace), continue to initialize
let existing = '';
try {
const existing = fs.readFileSync(contextFilePath, 'utf8');
if (existing && existing.trim().length > 0) {
// File exists and has content - ask for confirmation to overwrite
if (!context.overwriteConfirmed) {
return {
type: 'confirm_action',
// TODO: Move to .tsx file to use JSX syntax instead of React.createElement
// For now, using React.createElement to maintain .ts compatibility for PR review
prompt: React.createElement(
Text,
null,
`A ${contextFileName} file already exists in this directory. Do you want to regenerate it?`,
),
originalInvocation: {
raw: context.invocation?.raw || '/init',
},
};
}
// User confirmed overwrite, continue with regeneration
}
existing = fs.readFileSync(contextFilePath, 'utf8');
} catch {
// If we fail to read, conservatively proceed to (re)create the file
}
if (existing && existing.trim().length > 0) {
// File exists and has content - ask for confirmation to overwrite
if (!context.overwriteConfirmed) {
const [{ Text }, { default: React }] = await Promise.all([
import('ink'),
import('react'),
]);
return {
type: 'confirm_action',
prompt: React.createElement(
Text,
null,
`A ${contextFileName} file already exists in this directory. Do you want to regenerate it?`,
),
originalInvocation: {
raw: context.invocation?.raw || '/init',
},
};
}
// User confirmed overwrite, continue with regeneration
}
}

// Ensure an empty context file exists before prompting the model to populate it
Expand Down
41 changes: 8 additions & 33 deletions packages/cli/src/ui/components/SuggestionsDisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,40 +9,16 @@ import { Box, Text, type DOMElement } from 'ink';
import { theme } from '../semantic-colors.js';
import { RowMouseController } from './shared/RowMouseController.js';
import { PrepareLabel, MAX_WIDTH } from './PrepareLabel.js';
import type {
CommandKind,
CommandSource,
ExecutionMode,
} from '../commands/types.js';
import { Colors } from '../colors.js';
import { t } from '../../i18n/index.js';
export interface Suggestion {
label: string;
value: string;
description?: string;
matchedIndex?: number;
/** @deprecated Use source/sourceBadge instead. */
commandKind?: CommandKind;
source?: CommandSource;
sourceLabel?: string;
sourceBadge?: string;
argumentHint?: string;
matchedAlias?: string;
supportedModes?: ExecutionMode[];
modelInvocable?: boolean;
/** Whether the suggestion represents a directory path. When true, handleAutocomplete should NOT append a trailing space so the user can continue tab-completing deeper into the directory tree. */
isDirectory?: boolean;
/**
* When true, the input layer should submit `/<value>` immediately on
* Enter-accept rather than just inserting the suggestion text and
* waiting for a second Enter. Mirrors the `submitOnAccept` flag on the
* underlying SlashCommand (see `commands/types.ts`). Used for parent
* commands like `/skills` whose bare action just opens a dialog and
* takes no further argument — typing `/skil<Enter>` should land in the
* dialog in one keystroke.
*/
submitOnAccept?: boolean;
}
import {
MAX_SUGGESTIONS_TO_SHOW,
type Suggestion,
} from '../utils/suggestions.js';

export { MAX_SUGGESTIONS_TO_SHOW } from '../utils/suggestions.js';
export type { Suggestion } from '../utils/suggestions.js';

interface SuggestionsDisplayProps {
suggestions: Suggestion[];
activeIndex: number;
Expand All @@ -60,7 +36,6 @@ interface SuggestionsDisplayProps {
mouseEnabled?: boolean;
}

export const MAX_SUGGESTIONS_TO_SHOW = 8;
export { MAX_WIDTH };

/**
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/ui/hooks/extension-mention-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
*/

import type { Config } from '@qwen-code/qwen-code-core';
import type { Suggestion } from '../components/SuggestionsDisplay.js';
import { MAX_SUGGESTIONS_TO_SHOW } from '../components/SuggestionsDisplay.js';
import {
MAX_SUGGESTIONS_TO_SHOW,
type Suggestion,
} from '../utils/suggestions.js';
import { t } from '../../i18n/index.js';
export {
EXTENSION_REF_PREFIX,
Expand Down
Loading
Loading