Skip to content

Commit 24ff97e

Browse files
meganroggemeganrogge
authored
Remove redundant LLM error assessment from terminal output monitor (#307181)
1 parent b1b8217 commit 24ff97e

5 files changed

Lines changed: 33 additions & 67 deletions

File tree

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/getTerminalOutputTool.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ export interface IGetTerminalOutputInputParams {
4141
export class GetTerminalOutputTool extends Disposable implements IToolImpl {
4242
async prepareToolInvocation(context: IToolInvocationPreparationContext, token: CancellationToken): Promise<IPreparedToolInvocation | undefined> {
4343
return {
44-
invocationMessage: localize('bg.progressive', "Checking background terminal output"),
45-
pastTenseMessage: localize('bg.past', "Checked background terminal output"),
44+
invocationMessage: localize('getTerminalOutput.progressive', "Checking terminal output"),
45+
pastTenseMessage: localize('getTerminalOutput.past', "Checked terminal output"),
4646
};
4747
}
4848

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts‎

Lines changed: 20 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,6 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
142142
): Promise<void> {
143143
const pollStartTime = Date.now();
144144

145-
let modelOutputEvalResponse;
146145
let resources;
147146
let output;
148147

@@ -174,14 +173,13 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
174173
case OutputMonitorState.Idle: {
175174
this._logService.trace('OutputMonitor: Entering Idle handler');
176175
const idleResult = await this._handleIdleState(token);
177-
if (idleResult.shouldContinuePollling) {
176+
if (idleResult.shouldContinuePolling) {
178177
this._logService.trace('OutputMonitor: Idle handler -> continue polling');
179178
this._state = OutputMonitorState.PollingForIdle;
180179
continue;
181180
} else {
182-
this._logService.trace(`OutputMonitor: Idle handler -> stop polling (hasResources=${!!idleResult.resources}, hasModelEval=${!!idleResult.modelOutputEvalResponse}, outputLen=${idleResult.output?.length ?? 0})`);
181+
this._logService.trace(`OutputMonitor: Idle handler -> stop polling (hasResources=${!!idleResult.resources}, outputLen=${idleResult.output?.length ?? 0})`);
183182
resources = idleResult.resources;
184-
modelOutputEvalResponse = idleResult.modelOutputEvalResponse;
185183
output = idleResult.output;
186184
}
187185
break;
@@ -200,7 +198,6 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
200198
this._pollingResult = {
201199
state: this._state,
202200
output: output ?? this._execution.getOutput(),
203-
modelOutputEvalResponse: token.isCancellationRequested ? 'Cancelled' : modelOutputEvalResponse,
204201
pollDurationMs: Date.now() - pollStartTime,
205202
resources
206203
};
@@ -220,13 +217,13 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
220217
}
221218

222219

223-
private async _handleIdleState(token: CancellationToken): Promise<{ resources?: ILinkLocation[]; modelOutputEvalResponse?: string; shouldContinuePollling: boolean; output?: string }> {
220+
private async _handleIdleState(token: CancellationToken): Promise<{ resources?: ILinkLocation[]; shouldContinuePolling: boolean; output?: string }> {
224221
const output = this._execution.getOutput(this._lastPromptMarker);
225222
this._logService.trace(`OutputMonitor: Idle output summary: len=${output.length}, lastLine=${this._formatLastLineForLog(output)}`);
226223

227224
if (detectsNonInteractiveHelpPattern(output)) {
228225
this._logService.trace('OutputMonitor: Idle -> non-interactive help pattern detected, stopping');
229-
return { shouldContinuePollling: false, output };
226+
return { shouldContinuePolling: false, output };
230227
}
231228

232229
// Check for VS Code's task finish messages (like "press any key to close the terminal").
@@ -236,7 +233,7 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
236233
if (isTask && detectsVSCodeTaskFinishMessage(output)) {
237234
this._logService.trace('OutputMonitor: Idle -> VS Code task finish message detected, stopping');
238235
// Task is finished, ignore the "press any key to close" message
239-
return { shouldContinuePollling: false, output };
236+
return { shouldContinuePolling: false, output };
240237
}
241238

242239
// Check for generic "press any key" prompts from scripts.
@@ -247,7 +244,7 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
247244
if (autoReply) {
248245
this._logService.trace('OutputMonitor: Auto-reply enabled -> not showing free-form prompt for "press any key", stopping');
249246
this._cleanupIdleInputListener();
250-
return { shouldContinuePollling: false, output };
247+
return { shouldContinuePolling: false, output };
251248
}
252249
this._logService.trace('OutputMonitor: Requesting free-form input for "press any key"');
253250
// Register a marker to track this prompt position so we don't re-detect it
@@ -266,18 +263,18 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
266263
if (receivedTerminalInput) {
267264
this._logService.trace('OutputMonitor: Free-form input received for "press any key", continue polling');
268265
await timeout(200);
269-
return { shouldContinuePollling: true };
266+
return { shouldContinuePolling: true };
270267
} else {
271268
this._logService.trace('OutputMonitor: Free-form input declined for "press any key", stopping');
272-
return { shouldContinuePollling: false };
269+
return { shouldContinuePolling: false };
273270
}
274271
}
275272

276273
// Check if user already inputted since idle was detected (before we even got here)
277274
if (this._userInputtedSinceIdleDetected) {
278275
this._logService.trace('OutputMonitor: User input detected since idle; skipping prompt and continuing polling');
279276
this._cleanupIdleInputListener();
280-
return { shouldContinuePollling: true };
277+
return { shouldContinuePolling: true };
281278
}
282279

283280
this._logService.trace('OutputMonitor: Determining user input options via language model');
@@ -288,21 +285,21 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
288285
if (this._userInputtedSinceIdleDetected) {
289286
this._logService.trace('OutputMonitor: User input arrived during input-option analysis; continuing polling');
290287
this._cleanupIdleInputListener();
291-
return { shouldContinuePollling: true };
288+
return { shouldContinuePolling: true };
292289
}
293290

294291
if (confirmationPrompt?.detectedRequestForFreeFormInput) {
295292
// Check again right before showing prompt
296293
if (this._userInputtedSinceIdleDetected) {
297294
this._logService.trace('OutputMonitor: User input arrived before showing free-form prompt; continuing polling');
298295
this._cleanupIdleInputListener();
299-
return { shouldContinuePollling: true };
296+
return { shouldContinuePolling: true };
300297
}
301298
const autoReply = this._configurationService.getValue(TerminalChatAgentToolsSettingId.AutoReplyToPrompts) || this._isAutopilotMode();
302299
if (autoReply) {
303300
this._logService.trace('OutputMonitor: Auto-reply enabled -> not propagating free-form prompt, stopping');
304301
this._cleanupIdleInputListener();
305-
return { shouldContinuePollling: false, output };
302+
return { shouldContinuePolling: false, output };
306303
}
307304
// Clean up the input listener now - the prompt will set up its own
308305
this._cleanupIdleInputListener();
@@ -314,11 +311,11 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
314311
this._logService.trace('OutputMonitor: Free-form input received; continuing polling');
315312
await timeout(200);
316313
// Continue polling as we sent the input
317-
return { shouldContinuePollling: true };
314+
return { shouldContinuePolling: true };
318315
} else {
319316
// User declined
320317
this._logService.trace('OutputMonitor: Free-form input declined; stopping');
321-
return { shouldContinuePollling: false };
318+
return { shouldContinuePolling: false };
322319
}
323320
}
324321

@@ -329,13 +326,13 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
329326
if (suggestedOptionResult?.sentToTerminal) {
330327
// Continue polling as we sent the input
331328
this._cleanupIdleInputListener();
332-
return { shouldContinuePollling: true };
329+
return { shouldContinuePolling: true };
333330
}
334331
// Check again after LLM call - user may have inputted while we were selecting option
335332
if (this._userInputtedSinceIdleDetected) {
336333
this._logService.trace('OutputMonitor: User input arrived during option selection; continuing polling');
337334
this._cleanupIdleInputListener();
338-
return { shouldContinuePollling: true };
335+
return { shouldContinuePolling: true };
339336
}
340337
// Clean up the input listener now - the prompt will set up its own
341338
this._cleanupIdleInputListener();
@@ -344,24 +341,23 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
344341
if (confirmed) {
345342
// Continue polling as we sent the input
346343
this._logService.trace('OutputMonitor: Option confirmed/sent; continuing polling');
347-
return { shouldContinuePollling: true };
344+
return { shouldContinuePolling: true };
348345
} else {
349346
// User declined
350347
this._logService.trace('OutputMonitor: Option declined; stopping');
351348
this._execution.instance.focus(true);
352-
return { shouldContinuePollling: false };
349+
return { shouldContinuePolling: false };
353350
}
354351
}
355352

356-
// Clean up input listener before custom poll/error assessment
353+
// Clean up input listener before custom poll
357354
this._cleanupIdleInputListener();
358355

359356
// Let custom poller override if provided
360357
const custom = await this._pollFn?.(this._execution, token, this._taskService);
361358
this._logService.trace(`OutputMonitor: Custom poller result: ${custom ? 'provided' : 'none'}`);
362359
const resources = custom?.resources;
363-
const modelOutputEvalResponse = this._pollFn ? undefined : await this._assessOutputForErrors(this._execution.getOutput(), token);
364-
return { resources, modelOutputEvalResponse, shouldContinuePollling: false, output: custom?.output ?? output };
360+
return { resources, shouldContinuePolling: false, output: custom?.output ?? output };
365361
}
366362

367363
private async _handleTimeoutState(_command: string, _invocationContext: IToolInvocationContext | undefined, _extended: boolean, _token: CancellationToken): Promise<boolean> {
@@ -470,28 +466,6 @@ export class OutputMonitor extends Disposable implements IOutputMonitor {
470466
this._userInputListener.clear();
471467
}
472468

473-
private async _assessOutputForErrors(buffer: string, token: CancellationToken): Promise<string | undefined> {
474-
const model = await this._getLanguageModel();
475-
if (!model) {
476-
return 'No models available';
477-
478-
}
479-
480-
const response = await this._languageModelsService.sendChatRequest(
481-
model,
482-
undefined,
483-
[{ role: ChatMessageRole.User, content: [{ type: 'text', value: `Evaluate this terminal output to determine if there were errors. If there are errors, return them. Otherwise, return undefined: ${buffer}.` }] }],
484-
{},
485-
token
486-
);
487-
488-
try {
489-
return await getTextResponseFromStream(response);
490-
} catch (err) {
491-
return 'Error occurred ' + err;
492-
}
493-
}
494-
495469
private async _determineUserInputOptions(execution: IExecution, token: CancellationToken): Promise<IConfirmationPrompt | undefined> {
496470
if (token.isCancellationRequested) {
497471
this._logService.trace('OutputMonitor: determineUserInputOptions cancelled before start');

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/types.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ export interface IExecution {
2929
export interface IPollingResult {
3030
output: string;
3131
resources?: ILinkLocation[];
32-
modelOutputEvalResponse?: string;
3332
state: OutputMonitorState;
3433
}
3534

@@ -43,13 +42,6 @@ export enum OutputMonitorState {
4342
Cancelled = 'Cancelled',
4443
}
4544

46-
export interface IRacePollingOrPromptResult {
47-
output: string;
48-
pollDurationMs?: number;
49-
modelOutputEvalResponse?: string;
50-
state: OutputMonitorState;
51-
}
52-
5345
export const enum PollingConsts {
5446
MinIdleEvents = 2, // Minimum number of idle checks before considering the terminal idle
5547
MinPollingDuration = 500,

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,18 +1186,18 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
11861186
? `Note: The tool simplified the command to \`${command}\`, and that command is now running in terminal with ID=${termId}`
11871187
: `Command is running in terminal with ID=${termId}`
11881188
);
1189-
const backgroundOutput = pollingResult?.modelOutputEvalResponse ?? pollingResult?.output;
1189+
const backgroundOutput = pollingResult?.output;
11901190
const outputAnalyzerMessage = backgroundOutput
11911191
? await this._getOutputAnalyzerMessage(undefined, backgroundOutput, command, didSandboxWrapCommand)
11921192
: undefined;
1193-
if (pollingResult && pollingResult.modelOutputEvalResponse) {
1194-
resultText += `\n\ The command became idle with output:\n`;
1193+
if (pollingResult && pollingResult.state === OutputMonitorState.Idle) {
1194+
resultText += `\n The command became idle with output:\n`;
11951195
if (outputAnalyzerMessage) {
11961196
resultText += `${outputAnalyzerMessage}\n`;
11971197
}
1198-
resultText += pollingResult.modelOutputEvalResponse;
1198+
resultText += pollingResult.output;
11991199
} else if (pollingResult) {
1200-
resultText += `\n\ The command is still running, with output:\n`;
1200+
resultText += `\n The command is still running, with output:\n`;
12011201
if (outputAnalyzerMessage) {
12021202
resultText += `${outputAnalyzerMessage}\n`;
12031203
}

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/outputMonitor.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,8 @@ suite('OutputMonitor', () => {
195195
const timeoutThenIdle = async (): Promise<IPollingResult> => {
196196
pass++;
197197
return pass === 1
198-
? { state: OutputMonitorState.Timeout, output: execution.getOutput(), modelOutputEvalResponse: 'Timed out' }
199-
: { state: OutputMonitorState.Idle, output: execution.getOutput(), modelOutputEvalResponse: 'Done' };
198+
? { state: OutputMonitorState.Timeout, output: execution.getOutput() }
199+
: { state: OutputMonitorState.Idle, output: execution.getOutput() };
200200
};
201201

202202
monitor = store.add(
@@ -301,15 +301,15 @@ suite('OutputMonitor', () => {
301301
monitor = store.add(instantiationService.createInstance(OutputMonitor, execution, undefined, createTestContext('1'), monitorCts.token, 'test command'));
302302

303303
const outputMonitorWithPrivateMethod = monitor as unknown as {
304-
[key: string]: ((token: CancellationToken) => Promise<{ shouldContinuePollling: boolean }>) | undefined;
304+
[key: string]: ((token: CancellationToken) => Promise<{ shouldContinuePolling: boolean }>) | undefined;
305305
};
306306
const idleResult = await outputMonitorWithPrivateMethod['_handleIdleState']!(CancellationToken.None);
307307
await Event.toPromise(monitor.onDidFinishCommand);
308308
monitorCts.dispose();
309309

310310
assert.strictEqual(sendTextCalled, false, 'sendText should not be called when auto reply is enabled for free-form prompts');
311311
assert.strictEqual(sentText, undefined, 'no terminal input should be sent');
312-
assert.strictEqual(idleResult.shouldContinuePollling, false, 'monitor should stop polling for free-form prompts in auto reply mode');
312+
assert.strictEqual(idleResult.shouldContinuePolling, false, 'monitor should stop polling for free-form prompts in auto reply mode');
313313
});
314314

315315
test('auto reply does not propagate free-form input requests without explicit input', async () => {
@@ -335,13 +335,13 @@ suite('OutputMonitor', () => {
335335
return true;
336336
};
337337

338-
const idleResult = await (outputMonitorWithPrivateMethod['_handleIdleState'] as (token: CancellationToken) => Promise<{ shouldContinuePollling: boolean }>)(CancellationToken.None);
338+
const idleResult = await (outputMonitorWithPrivateMethod['_handleIdleState'] as (token: CancellationToken) => Promise<{ shouldContinuePolling: boolean }>)(CancellationToken.None);
339339
await Event.toPromise(monitor.onDidFinishCommand);
340340
monitorCts.dispose();
341341

342342
assert.strictEqual(freeFormRequestShown, false, 'free-form elicitation should not be shown when auto reply is enabled');
343343
assert.strictEqual(sendTextCalled, false, 'sensitive free-form prompt should not be auto-replied');
344-
assert.strictEqual(idleResult.shouldContinuePollling, false, 'monitor should stop instead of propagating free-form prompt');
344+
assert.strictEqual(idleResult.shouldContinuePolling, false, 'monitor should stop instead of propagating free-form prompt');
345345
});
346346

347347
suite('detectsInputRequiredPattern', () => {

0 commit comments

Comments
 (0)