Просмотр исходного кода

Add analytics events for Python script runner

GitOrigin-RevId: 53f0fec09fc2a4ccdf1a6f77345741bed29d8a8b
Domagoj Kriskovic 3 месяцев назад
Родитель
Сommit
5d171066c2

+ 8 - 0
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,
         })
       }
     }

+ 11 - 0
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

+ 42 - 7
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<string>()
+  const readPaths = new Set<string>()
+
+  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<string>()
-
   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<PyodideFS['read']>) => {
+      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
   )

+ 7 - 0
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() {
             </OLButton>
           </div>
         </div>
+        <div className="ide-redesign-python-output-pane-toolbar-right">
+          <SplitTestBadge
+            splitTestName="overleaf-code"
+            displayOnVariants={['enabled']}
+          />
+        </div>
       </OLButtonToolbar>
 
       <div className="ide-redesign-python-output-pane-body">

+ 59 - 8
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<Listener>()
 
-  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<string>()
+  for (const filePath of filePaths) {
+    const ext = extractExtension(filePath)
+    if (ext) {
+      seen.add(ext)
+    }
+  }
+  return Array.from(seen).join(',')
+}

+ 12 - 0
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 {

+ 58 - 1
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'])

+ 2 - 0
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' },