Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 8 additions & 1 deletion apps/kimi-code/src/tui/commands/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ export async function setDefaultModel(
effort,
model === undefined ? undefined : effectiveModelForHost(host, model),
);
const hadSession = host.session !== undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wait for lazy creation before choosing the event producer

In a session-less v2 TUI, the selector restores the editor and starts this function without awaiting it; if the user submits the first prompt while setConfig or the config refresh is pending, the lazy session can become live after hadSession is captured. refreshConfigAfterLogin then reaches session.setModel, causing the engine to emit model_switch, but the stale false snapshot still causes the TUI to emit a duplicate. Coordinate with waitForLazyCreation as the regular model-switch path does, or otherwise record whether activation actually reached a live session.

Useful? React with 👍 / 👎.

await host.harness.setConfig({
defaultModel: alias,
thinking,
Expand All @@ -317,7 +318,13 @@ export async function setDefaultModel(
if (thinking.effort === undefined && effort !== 'off' && effort !== 'on') {
await host.authFlow.activateModelAfterLogin(alias, effort);
}
host.track('model_switch', { model: alias });
// With a live session the engine already tracks model_switch from setModel
// (via activateModelAfterLogin); without one the engine never sees the pick
// (v2 defers creation to the first prompt, v1 binds the model on creation
// without an event), so the TUI stays the sole producer for that path.
if (!hadSession) {
host.track('model_switch', { model: alias });
}
host.showStatus(`Default model set to ${alias} with thinking ${effort}.`);
}

Expand Down
17 changes: 16 additions & 1 deletion apps/kimi-code/test/tui/commands/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { describe, expect, it, vi } from 'vitest';
import type { SlashCommandHost } from '#/tui/commands';
import { setDefaultModel } from '#/tui/commands/provider';

function makeHost() {
function makeHost(options: { withSession?: boolean } = {}) {
const appState = {
availableModels: {
// Declares no efforts; the Anthropic profile inference supplies
Expand All @@ -30,6 +30,7 @@ function makeHost() {
};
const host = {
state: { appState },
session: options.withSession === true ? ({} as SlashCommandHost['session']) : undefined,
harness: {
setConfig: vi.fn(async () => ({})),
},
Expand All @@ -45,6 +46,7 @@ function makeHost() {
refreshConfigAfterLogin: ReturnType<typeof vi.fn>;
activateModelAfterLogin: ReturnType<typeof vi.fn>;
};
track: ReturnType<typeof vi.fn>;
};
return { host };
}
Expand All @@ -65,6 +67,9 @@ describe('setDefaultModel', () => {
expect(
host.authFlow.activateModelAfterLogin.mock.invocationCallOrder[0]!,
).toBeGreaterThan(host.authFlow.refreshConfigAfterLogin.mock.invocationCallOrder[0]!);
// Without a session the engine never sees the pick, so the TUI stays the
// sole model_switch producer.
expect(host.track).toHaveBeenCalledWith('model_switch', { model: 'opus' });
});

it('does not re-apply the effort when the pick persists', async () => {
Expand All @@ -90,4 +95,14 @@ describe('setDefaultModel', () => {
});
expect(host.authFlow.activateModelAfterLogin).not.toHaveBeenCalled();
});

it('leaves model_switch to the engine when a session is live', async () => {
const { host } = makeHost({ withSession: true });

await setDefaultModel(host, 'opus', 'high');

// activateModelAfterLogin routes through session.setModel, which the
// engine already tracks — a TUI-side event would double-count the switch.
expect(host.track).not.toHaveBeenCalled();
});
});
4 changes: 2 additions & 2 deletions packages/node-sdk/src/kimi-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -650,8 +650,8 @@ export class KimiHarness {
// caller-supplied sessionStartedProperties that happen to share a key.
// `client_id` is always null here: a single-process host has no
// per-connection client id (that concept only exists for daemon clients,
// see core-impl.ts). Kept as an explicit key so both producers share the
// same session_started schema.
// see core-impl.ts). Kept as an explicit key so this row carries the
// same client-attribution shape as the daemon-client producer there.
client_id: null,
client_name: this.identity?.productName ?? null,
client_version: this.identity?.version ?? null,
Expand Down
8 changes: 8 additions & 0 deletions packages/node-sdk/src/sdk-rpc-client-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,12 +543,20 @@ export class SDKRpcClientV2 extends SDKRpcClientBase {
* section gates engine events the same way the v2 print runner gates them;
* the host keeps owning the client's lifecycle (flush / shutdown stay with
* the host, matching the v1 core's arrangement).
*
* The engine's own `session_started` is dropped here: `KimiHarness` emits
* that event for every session it opens (create / resume / reload / fork)
* with the richer client-attribution schema, so forwarding the engine's
* `{resumed, experimental_flags}` copy double-counted every open. Hosts
* without a harness (run-v2-print, kap-server) wire their own appenders and
* still get the engine's row.
*/
private installEngineTelemetry(client: TelemetryClient | undefined): void {
if (client === undefined) return;
const telemetry = this.app.accessor.get(ITelemetryService);
telemetry.addAppender({
track: (record) => {
if (record.event === 'session_started') return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve session events for direct RPC client users

When a consumer instantiates the exported SDKRpcClientV2 directly with a telemetry client and calls createSession or resumeSession, there is no KimiHarness to emit the replacement row, so this unconditional filter discards the only session_started event. Keep forwarding enabled for direct clients and suppress it only when createKimiHarnessV2 installs the second producer.

AGENTS.md reference: AGENTS.md:L23-L23

Useful? React with 👍 / 👎.

client.track(record.event, record.properties);
},
});
Expand Down
26 changes: 22 additions & 4 deletions packages/node-sdk/test/sdk-rpc-client-v2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,7 @@ describe('SDKRpcClientV2 engine telemetry', () => {
}
});

it('reports the same enabled experimental flags on every session_started row', async () => {
it('emits session_started once per open, with the harness schema and enabled experimental flags', async () => {
const homeDir = await mkdtemp(join(tmpdir(), 'kimi-sdk-v2-tel-flags-'));
tempDirs.push(homeDir);
const workDir = await mkdtemp(join(tmpdir(), 'kimi-sdk-v2-tel-flags-work-'));
Expand All @@ -1372,16 +1372,34 @@ describe('SDKRpcClientV2 engine telemetry', () => {
});
try {
const session = await harness.createSession({ workDir });
// The harness row is the sole producer: the forwarding appender drops
// the engine's own session_started, or every open would double-count.
const started = records.filter((record) => record.event === 'session_started');
expect(started.length).toBeGreaterThanOrEqual(2);
expect(started).toHaveLength(1);
expect(started[0]).toMatchObject({
sessionId: session.id,
properties: {
client_name: 'kimi-code-cli',
client_version: '0.0.0-test',
ui_mode: 'shell',
resumed: false,
},
});
for (const record of started) {
const flags = String(record.properties?.['experimental_flags'] ?? '').split(',');
expect(flags).toContain('subagent_fork');
expect(flags).toContain('wait_for');
}
const distinct = new Set(started.map((record) => record.properties?.['experimental_flags']));
expect(distinct.size).toBe(1);
await session.close();
await harness.resumeSession({ id: session.id });
const afterResume = records.filter((record) => record.event === 'session_started');
expect(afterResume).toHaveLength(2);
expect(afterResume[1]).toMatchObject({
sessionId: session.id,
properties: { resumed: true },
});
const distinct = new Set(afterResume.map((record) => record.properties?.['experimental_flags']));
expect(distinct.size).toBe(1);
} finally {
await harness.close();
}
Expand Down
Loading