diff --git a/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-client.ts b/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-client.ts index 6b18efdc33..cf4fe16b3b 100644 --- a/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-client.ts +++ b/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-client.ts @@ -1,5 +1,6 @@ import path from 'path-browserify' import type { + ExecutionErrorType, OutputStream, ProjectFileData, PyodideWorkerRequest, @@ -26,6 +27,8 @@ export type LifecycleCallback = ( success: boolean outputs: string[] failedUploads: string[] + imports: string[] + errorType?: ExecutionErrorType } ) => void @@ -203,6 +206,9 @@ export class PyodideWorkerClient { } } + const errorType = + failedUploads.length > 0 ? 'UploadFileError' : response.errorType + this.lifecycleCallback?.({ type: 'run-finished', fileId: response.fileId, @@ -210,6 +216,8 @@ export class PyodideWorkerClient { success, outputs: response.outputs, failedUploads, + imports: response.imports, + errorType, }) } } diff --git a/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-messages.ts b/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-messages.ts index 8b5f08ad0b..9b448a74c3 100644 --- a/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-messages.ts +++ b/services/web/frontend/js/features/ide-react/components/editor/python/pyodide-worker-messages.ts @@ -49,6 +49,15 @@ export type PyodideWorkerEvent = // Worker -> Main thread ID responses +export type ExecutionErrorType = + | 'SyntaxError' + | 'ModuleNotFoundError' + | 'OutputLimitExceeded' + | 'UploadFileError' + | 'generic' + +export type ExecutionResult = 'success' | 'error' + export type RunCodeResult = { type: 'run-code-result' fileId: string @@ -56,6 +65,8 @@ export type RunCodeResult = { success: boolean outputs: string[] outputFiles: OutputFileData[] + imports: string[] + errorType?: ExecutionErrorType } export type PyodideWorkerResponse = PyodideWorkerEvent | RunCodeResult diff --git a/services/web/frontend/js/features/ide-react/components/editor/python/pyodide.worker.ts b/services/web/frontend/js/features/ide-react/components/editor/python/pyodide.worker.ts index fbe2488ca6..38d8ca577e 100644 --- a/services/web/frontend/js/features/ide-react/components/editor/python/pyodide.worker.ts +++ b/services/web/frontend/js/features/ide-react/components/editor/python/pyodide.worker.ts @@ -2,6 +2,7 @@ import path from 'path-browserify' import type { PyodideInterface } from 'pyodide' import type { + ExecutionErrorType, OutputFileData, InitRequest, ProjectFileData, @@ -20,6 +21,16 @@ const PROJECT_FS_ROOT = '/project' const PROJECT_FS_PREFIX = `${PROJECT_FS_ROOT}/` const PYODIDE_INDEX_PATH = 'js/libs/pyodide/' +function classifyErrorType(errorMessage: string): ExecutionErrorType { + if (errorMessage.includes('ModuleNotFoundError')) { + return 'ModuleNotFoundError' + } + if (errorMessage.includes('SyntaxError')) { + return 'SyntaxError' + } + return 'generic' +} + function ensureDirectoryExists(fs: PyodideFS, filePath: string) { const directory = path.dirname(filePath) if (directory === '.' || directory === '/') { @@ -91,7 +102,17 @@ async function handleInit(msg: InitRequest) { async function handleRunCode(msg: RunCodeRequest) { const { fileId, executionId } = msg - const postFailure = (stream: 'stderr' | 'info', line: string) => { + const writtenPaths = new Set() + const readPaths = new Set() + + const computeImports = () => + [...readPaths].filter(path => !writtenPaths.has(path)) + + const postFailure = ( + stream: 'stderr' | 'info', + line: string, + errorType: ExecutionErrorType = 'generic' + ) => { self.postMessage({ type: 'output-line', stream, @@ -106,6 +127,8 @@ async function handleRunCode(msg: RunCodeRequest) { success: false, outputs: [], outputFiles: [], + imports: computeImports(), + errorType, }) } @@ -119,8 +142,6 @@ async function handleRunCode(msg: RunCodeRequest) { packageBaseUrl: `${pyodideIndexUrl}${pyodideModule.version}/`, }) - const writtenPaths = new Set() - instance.setStdout({ batched: (line: string) => { self.postMessage({ @@ -146,6 +167,7 @@ async function handleRunCode(msg: RunCodeRequest) { const fs = instance.FS const originalWrite = fs.write as PyodideFS['write'] + const originalRead = fs.read as PyodideFS['read'] let runError: unknown = null try { if (msg.files.length > 0) { @@ -165,6 +187,18 @@ async function handleRunCode(msg: RunCodeRequest) { return originalWrite.call(fs, ...args) }) as PyodideFS['write'] + fs.read = ((...args: Parameters) => { + const [stream] = args + if ( + typeof stream?.path === 'string' && + stream.path.startsWith(PROJECT_FS_PREFIX) + ) { + readPaths.add(stream.path) + } + + return originalRead.call(fs, ...args) + }) as PyodideFS['read'] + await instance.loadPackagesFromImports(msg.code) const result = await instance.runPythonAsync(msg.code) if (result !== undefined) { @@ -180,20 +214,20 @@ async function handleRunCode(msg: RunCodeRequest) { runError = e } fs.write = originalWrite + fs.read = originalRead const paths = [...writtenPaths] if (runError) { const errorMessage = runError instanceof Error ? runError.message : String(runError) - - postFailure('stderr', errorMessage) + postFailure('stderr', errorMessage, classifyErrorType(errorMessage)) return } const countViolation = checkOutputCount(paths.length) if (countViolation) { - postFailure('info', countViolation.message) + postFailure('info', countViolation.message, 'OutputLimitExceeded') return } @@ -213,7 +247,7 @@ async function handleRunCode(msg: RunCodeRequest) { const sizeViolation = checkOutputLimits(filesWithSizes) if (sizeViolation) { - postFailure('info', sizeViolation.message) + postFailure('info', sizeViolation.message, 'OutputLimitExceeded') return } @@ -240,6 +274,7 @@ async function handleRunCode(msg: RunCodeRequest) { success: true, outputs: filesWithSizes.map(f => f.path), outputFiles, + imports: computeImports(), }, transferables ) diff --git a/services/web/frontend/js/features/ide-react/components/editor/python/python-output-pane.tsx b/services/web/frontend/js/features/ide-react/components/editor/python/python-output-pane.tsx index 4ee451bdf0..0a7f2f3bbf 100644 --- a/services/web/frontend/js/features/ide-react/components/editor/python/python-output-pane.tsx +++ b/services/web/frontend/js/features/ide-react/components/editor/python/python-output-pane.tsx @@ -3,6 +3,7 @@ import { useTranslation } from 'react-i18next' import OLButton from '@/shared/components/ol/ol-button' import OLButtonToolbar from '@/shared/components/ol/ol-button-toolbar' import MaterialIcon from '@/shared/components/material-icon' +import SplitTestBadge from '@/shared/components/split-test-badge' import { useEditorOpenDocContext } from '@/features/ide-react/context/editor-open-doc-context' import { usePythonExecutionContext } from '@/features/ide-react/context/python-execution-context' import { DEFAULT_STATE } from './python-runner' @@ -58,6 +59,12 @@ export default function PythonOutputPane() { +
+ +
diff --git a/services/web/frontend/js/features/ide-react/components/editor/python/python-runner.ts b/services/web/frontend/js/features/ide-react/components/editor/python/python-runner.ts index bf042339df..f1fab4f0a9 100644 --- a/services/web/frontend/js/features/ide-react/components/editor/python/python-runner.ts +++ b/services/web/frontend/js/features/ide-react/components/editor/python/python-runner.ts @@ -1,8 +1,10 @@ // Per-file Python execution manager. Each PythonRunner owns a PyodideWorkerClient // and exposes a subscribe/getState API for use with useSyncExternalStore, // so React components can reactively read execution status and output. +import path from 'path-browserify' import { v4 as uuid } from 'uuid' import { debugConsole } from '@/utils/debugging' +import { sendMB } from '@/infrastructure/event-tracking' import { PyodideWorkerClient } from './pyodide-worker-client' import type { OutputStream } from './pyodide-worker-messages' import type { @@ -55,7 +57,7 @@ export class PythonRunner { private listeners = new Set() - private activeExecutionId: string | null = null + private activeExecution: { id: string; startedAt: number } | null = null private state: PythonRunnerState = DEFAULT_STATE constructor( @@ -123,20 +125,35 @@ export class PythonRunner { return case 'run-finished': { + const active = this.activeExecution if ( event.fileId !== this.fileId || - this.activeExecutionId !== event.executionId + active?.id !== event.executionId ) { return } - this.activeExecutionId = null + this.activeExecution = null + + sendMB('script-runner-execution-completed', { + result: event.success ? 'success' : 'error', + errorType: event.success ? undefined : event.errorType, + executionTimeMs: Math.round(performance.now() - active.startedAt), + filesImportedCount: event.imports.length, + filesImportedExtensions: collectExtensions(event.imports), + filesWrittenCount: event.outputs.length, + filesWrittenExtensions: collectExtensions(event.outputs), + }) + this.updateState({ status: 'finished' }) } } }, onOutput: (stream, line, fileId, executionId) => { - if (fileId !== this.fileId || this.activeExecutionId !== executionId) { + if ( + fileId !== this.fileId || + this.activeExecution?.id !== executionId + ) { return } this.updateState({ @@ -172,8 +189,12 @@ export class PythonRunner { const { code, files } = context + sendMB('script-runner-run-clicked', { + scriptLineCount: countLines(code), + }) + const executionId = uuid() - this.activeExecutionId = executionId + this.activeExecution = { id: executionId, startedAt: performance.now() } this.updateState({ status: 'running', output: [], error: null }) try { @@ -183,10 +204,10 @@ export class PythonRunner { files, }) } catch (runError) { - if (this.activeExecutionId !== executionId) { + if (this.activeExecution?.id !== executionId) { return } - this.activeExecutionId = null + this.activeExecution = null this.updateState({ status: 'errored', error: formatError(runError) }) } } @@ -196,8 +217,16 @@ export class PythonRunner { return } + if (this.state.status === 'running' && this.activeExecution) { + sendMB('script-runner-stop-clicked', { + timeBeforeStopMs: Math.round( + performance.now() - this.activeExecution.startedAt + ), + }) + } + this.client.reset() - this.activeExecutionId = null + this.activeExecution = null // The worker is terminated and recreated by reset(), so it needs to // reload Pyodide. The 'loaded' lifecycle callback will transition @@ -235,3 +264,25 @@ function formatError(error: unknown): string { } return String(error) } + +function countLines(code: string): number { + if (code.length === 0) { + return 0 + } + return code.split('\n').length +} + +function extractExtension(filePath: string): string { + return path.extname(filePath).slice(1).toLowerCase() +} + +function collectExtensions(filePaths: string[]): string { + const seen = new Set() + for (const filePath of filePaths) { + const ext = extractExtension(filePath) + if (ext) { + seen.add(ext) + } + } + return Array.from(seen).join(',') +} diff --git a/services/web/frontend/stylesheets/pages/editor/ide-redesign.scss b/services/web/frontend/stylesheets/pages/editor/ide-redesign.scss index feb0e750cd..328bd5c682 100644 --- a/services/web/frontend/stylesheets/pages/editor/ide-redesign.scss +++ b/services/web/frontend/stylesheets/pages/editor/ide-redesign.scss @@ -72,6 +72,18 @@ .compile-button-group { border-radius: var(--ds-border-radius-300); } + + .ide-redesign-python-output-pane-toolbar-right { + flex: 1; + display: flex; + justify-content: flex-end; + align-items: center; + padding-right: var(--spacing-02); + + .material-symbols { + font-size: 16px; + } + } } .ide-redesign-python-output-pane-body { diff --git a/services/web/test/frontend/features/ide-react/unit/editor/pyodide-worker-client.spec.ts b/services/web/test/frontend/features/ide-react/unit/editor/pyodide-worker-client.spec.ts index 26b2576567..3d082d62ab 100644 --- a/services/web/test/frontend/features/ide-react/unit/editor/pyodide-worker-client.spec.ts +++ b/services/web/test/frontend/features/ide-react/unit/editor/pyodide-worker-client.spec.ts @@ -140,6 +140,7 @@ describe('PyodideWorkerClient', function () { success: true, outputs: ['/project/output.txt'], outputFiles: [], + imports: [], }) expect(lifecycleEvents).to.deep.equal([ @@ -150,6 +151,8 @@ describe('PyodideWorkerClient', function () { success: true, outputs: ['/project/output.txt'], failedUploads: [], + imports: [], + errorType: undefined, }, ]) }) @@ -170,6 +173,7 @@ describe('PyodideWorkerClient', function () { success: true, outputs: ['/project/fig1.png', '/project/results/data.csv'], outputFiles: [], + imports: [], }) expect(lifecycleEvents).to.deep.equal([ @@ -180,6 +184,8 @@ describe('PyodideWorkerClient', function () { success: true, outputs: ['/project/fig1.png', '/project/results/data.csv'], failedUploads: [], + imports: [], + errorType: undefined, }, ]) }) @@ -200,6 +206,7 @@ describe('PyodideWorkerClient', function () { success: true, outputs: [], outputFiles: [], + imports: [], }) expect(lifecycleEvents).to.deep.equal([ @@ -210,6 +217,8 @@ describe('PyodideWorkerClient', function () { success: true, outputs: [], failedUploads: [], + imports: [], + errorType: undefined, }, ]) }) @@ -235,6 +244,7 @@ describe('PyodideWorkerClient', function () { { relativePath: 'data.csv', content: csvContent }, { relativePath: 'plot.png', content: pngContent }, ], + imports: [], }) await waitFor(() => @@ -249,6 +259,41 @@ describe('PyodideWorkerClient', function () { success: true, outputs: ['/project/data.csv', '/project/plot.png'], failedUploads: [], + imports: [], + errorType: undefined, + }) + }) + + it('propagates ModuleNotFoundError errorType on run-finished', function () { + const { client, worker, lifecycleEvents } = + setupClientWithLifecycleTracking() + + client.runCode('import nope', { + fileId: 'main.py', + executionId: 'exec-mnfe', + files: [], + }) + worker.emitMessage({ + type: 'run-code-result', + fileId: 'main.py', + executionId: 'exec-mnfe', + success: false, + outputs: [], + outputFiles: [], + imports: [], + errorType: 'ModuleNotFoundError', + }) + + const finished = lifecycleEvents.find(e => e.type === 'run-finished') + expect(finished).to.deep.equal({ + type: 'run-finished', + fileId: 'main.py', + executionId: 'exec-mnfe', + success: false, + outputs: [], + failedUploads: [], + imports: [], + errorType: 'ModuleNotFoundError', }) }) @@ -268,6 +313,8 @@ describe('PyodideWorkerClient', function () { success: false, outputs: [], outputFiles: [], + imports: [], + errorType: 'generic', }) const finished = lifecycleEvents.find(e => e.type === 'run-finished') @@ -278,6 +325,8 @@ describe('PyodideWorkerClient', function () { success: false, outputs: [], failedUploads: [], + imports: [], + errorType: 'generic', }) }) @@ -297,6 +346,7 @@ describe('PyodideWorkerClient', function () { success: true, outputs: [], outputFiles: [], + imports: [], }) const finished = lifecycleEvents.find(e => e.type === 'run-finished') @@ -307,6 +357,8 @@ describe('PyodideWorkerClient', function () { success: true, outputs: [], failedUploads: [], + imports: [], + errorType: undefined, }) }) @@ -388,6 +440,7 @@ describe('PyodideWorkerClient', function () { success, outputs: [], outputFiles, + imports: [], }) } @@ -463,6 +516,7 @@ describe('PyodideWorkerClient', function () { expect(finished).to.deep.include({ success: false, failedUploads: ['output/bad.csv'], + errorType: 'UploadFileError', }) // user-facing stderr line surfaced via outputCallback @@ -485,7 +539,10 @@ describe('PyodideWorkerClient', function () { await waitFor(() => Boolean(findFinished(lifecycleEvents))) const finished = findFinished(lifecycleEvents) - expect(finished).to.deep.include({ success: false }) + expect(finished).to.deep.include({ + success: false, + errorType: 'UploadFileError', + }) expect(finished) .to.have.property('failedUploads') .that.has.members(['a.csv', 'b.csv']) diff --git a/services/web/test/frontend/features/ide-react/unit/editor/python-runner.spec.ts b/services/web/test/frontend/features/ide-react/unit/editor/python-runner.spec.ts index 69fc11d931..4fe411c5d1 100644 --- a/services/web/test/frontend/features/ide-react/unit/editor/python-runner.spec.ts +++ b/services/web/test/frontend/features/ide-react/unit/editor/python-runner.spec.ts @@ -137,6 +137,7 @@ describe('PythonRunner', function () { success: true, outputs: [], outputFiles: [], + imports: [], }) await waitForState(runner, s => s.status === 'finished') @@ -163,6 +164,7 @@ describe('PythonRunner', function () { success: true, outputs: [], outputFiles: [], + imports: [], }) expect(runner.getState().output).to.deep.equal([ { stream: 'stdout', line: 'first run output' },