Skip to content

Commit 6da6a8f

Browse files
authored
fix(providers): improve Python provider reliability and logging (#6034)
1 parent 619718b commit 6da6a8f

File tree

7 files changed

+35
-9
lines changed

7 files changed

+35
-9
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
5454
### Fixed
5555

5656
- fix(providers): revert eager template rendering that broke runtime variable substitution (5423f80)
57+
- fix(providers): improve Python provider reliability with automatic python3/python detection, worker cleanup, request count tracking, and reduced logging noise (#6034)
5758
- fix(providers): simulated-user and mischievous-user now respect assistant system prompts in multi-turn conversations (#6020)
5859
- fix(providers): improve MCP tool schema transformation for OpenAI compatibility (#5965)
5960
- fix(providers): sessionId now properly stored in metadata for providers that use server side generated sessionIds (#6016)

src/evaluator.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { selectMaxScore } from './matchers';
2020
import { generateIdFromPrompt } from './models/prompt';
2121
import { CIProgressReporter } from './progress/ciProgressReporter';
2222
import { maybeEmitAzureOpenAiWarning } from './providers/azure/warnings';
23+
import { providerRegistry } from './providers/providerRegistry';
2324
import { isPromptfooSampleTarget } from './providers/shared';
2425
import { strategyDisplayNames } from './redteam/constants';
2526
import { getSessionId, isSimbaTestCase } from './redteam/util';
@@ -1911,6 +1912,9 @@ class Evaluator {
19111912
await sleep(1000);
19121913
}
19131914
await stopOtlpReceiverIfNeeded();
1915+
1916+
// Clean up Python worker pools to prevent resource leaks
1917+
await providerRegistry.shutdownAll();
19141918
}
19151919
}
19161920
}

src/providers/providerRegistry.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,15 @@ class ProviderRegistry {
5757
}
5858

5959
async shutdownAll(): Promise<void> {
60-
await Promise.all(Array.from(this.providers).map((p) => p.shutdown()));
60+
const results = await Promise.allSettled(Array.from(this.providers).map((p) => p.shutdown()));
61+
62+
// Log any failures but don't throw - cleanup should be defensive
63+
for (const result of results) {
64+
if (result.status === 'rejected') {
65+
logger.warn(`Error shutting down provider: ${result.reason}`);
66+
}
67+
}
68+
6169
this.providers.clear();
6270
}
6371
}

src/providers/pythonCompletion.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ export class PythonProvider implements ApiProvider {
199199
parsedResult.tokenUsage = {
200200
cached: total,
201201
total,
202+
numRequests: parsedResult.tokenUsage.numRequests ?? 1,
202203
};
203204
logger.debug(
204205
`Updated token usage for cached result: ${JSON.stringify(parsedResult.tokenUsage)}`,
@@ -310,10 +311,18 @@ export class PythonProvider implements ApiProvider {
310311
);
311312
}
312313

313-
// Set cached=false on fresh results
314+
// Set cached=false on fresh results and ensure numRequests is set
314315
if (typeof result === 'object' && result !== null && apiType === 'call_api') {
315316
logger.debug(`PythonProvider explicitly setting cached=false for fresh result`);
316317
result.cached = false;
318+
319+
// Ensure tokenUsage includes numRequests for fresh results
320+
if (result.tokenUsage && !result.tokenUsage.numRequests) {
321+
result.tokenUsage.numRequests = 1;
322+
logger.debug(
323+
`Added numRequests to fresh result token usage: ${JSON.stringify(result.tokenUsage)}`,
324+
);
325+
}
317326
}
318327

319328
return result;

src/python/persistent_wrapper.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,6 @@ def load_user_module(script_path):
3030
if script_dir not in sys.path:
3131
sys.path.insert(0, script_dir)
3232

33-
print(
34-
f"Loading module {module_name} from {script_path}...",
35-
file=sys.stderr,
36-
flush=True,
37-
)
38-
3933
spec = importlib.util.spec_from_file_location(module_name, script_path)
4034
if spec is None or spec.loader is None:
4135
raise ImportError(f"Cannot load module from {script_path}")

src/python/worker.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import path from 'path';
44
import os from 'os';
55
import logger from '../logger';
66
import { safeJsonStringify } from '../util/json';
7+
import { validatePythonPath } from './pythonUtils';
78

89
export class PythonWorker {
910
private process: PythonShell | null = null;
@@ -33,9 +34,15 @@ export class PythonWorker {
3334
private async startWorker(): Promise<void> {
3435
const wrapperPath = path.join(__dirname, 'persistent_wrapper.py');
3536

37+
// Validate and resolve Python path using smart detection (tries python3, then python)
38+
const resolvedPythonPath = await validatePythonPath(
39+
this.pythonPath || 'python',
40+
typeof this.pythonPath === 'string',
41+
);
42+
3643
this.process = new PythonShell(wrapperPath, {
3744
mode: 'text',
38-
pythonPath: this.pythonPath || 'python',
45+
pythonPath: resolvedPythonPath,
3946
args: [this.scriptPath, this.functionName],
4047
stdio: ['pipe', 'pipe', 'pipe'],
4148
});

test/providers/pythonCompletion.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ describe('PythonProvider', () => {
286286
tokenUsage: {
287287
cached: 25,
288288
total: 25,
289+
numRequests: 1,
289290
},
290291
});
291292
});
@@ -316,6 +317,7 @@ describe('PythonProvider', () => {
316317
prompt: 12,
317318
completion: 18,
318319
total: 30,
320+
numRequests: 1,
319321
},
320322
});
321323
});
@@ -364,6 +366,7 @@ describe('PythonProvider', () => {
364366
expect(result.tokenUsage).toEqual({
365367
cached: 0,
366368
total: 0,
369+
numRequests: 1,
367370
});
368371
expect(result.cached).toBe(true);
369372
});

0 commit comments

Comments
 (0)