Repository: stateful/vscode-runme
Pull request #1027 (2023-12-29 17:54:23)

Set execution order according to session
Will count execution continuously across notebooks per session ID (starting new sessions will become available with Session Outputs currently opt-in). Skips "Configure" and non-cell executions.

<img width="915" alt="Screenshot 2023-12-29 at 3 15 58 PM" src="https://github.com/stateful/vscode-runme/assets/250527/a131d96d-f9cc-4578-85ec-9d5b4c5c709a">

sonarcloud[bot]: ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=stateful_vscode-runme&pullRequest=1027) **Quality Gate failed**  
Failed conditions

[0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=stateful_vscode-runme&pullRequest=1027&metric=new_coverage&view=list) (required ≥ 80%)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=stateful_vscode-runme&pullRequest=1027)


sonarcloud[bot]: ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=stateful_vscode-runme&pullRequest=1027) **Quality Gate failed**  
Failed conditions

[0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=stateful_vscode-runme&pullRequest=1027&metric=new_coverage&view=list) (required ≥ 80%)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=stateful_vscode-runme&pullRequest=1027)


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=stateful_vscode-runme&pullRequest=1027) **Quality Gate passed**  
The SonarCloud Quality Gate passed, but some issues were introduced.

[1 New issue](https://sonarcloud.io/project/issues?id=stateful_vscode-runme&pullRequest=1027&resolved=false&inNewCodePeriod=true)  
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=stateful_vscode-runme&pullRequest=1027&resolved=false&inNewCodePeriod=true)  
[91.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=stateful_vscode-runme&pullRequest=1027&metric=new_coverage&view=list)  
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=stateful_vscode-runme&pullRequest=1027&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=stateful_vscode-runme&pullRequest=1027)



Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 9d67d2de..5382e662 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -179,6 +179,12 @@ export NODE_OPTIONS="--experimental-specifier-resolution=node"
 npx vitest -c ./vitest.conf.ts --run
 ```
 
+The test coverage report is easy to access at:
+
+```sh {"id":"01HJVVP86RWEMH7QM80EPK266B","name":"test:report"}
+open coverage/lcov-report/index.html
+```
+
 #### E2E Testing
 
 We use WebdriverIO to run e2e tests on the VS Code extension:
diff --git a/package-lock.json b/package-lock.json
index 5859ee89..8b3eff25 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
 {
   "name": "runme",
-  "version": "2.1.7-edge.1",
+  "version": "2.1.7",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "runme",
-      "version": "2.1.7-edge.1",
+      "version": "2.1.7",
       "hasInstallScript": true,
       "license": "Apache-2.0",
       "dependencies": {
diff --git a/package.json b/package.json
index e675914a..9339ed10 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "runme",
-  "version": "2.1.7-edge.1",
+  "version": "2.1.7",
   "publisher": "stateful",
   "displayName": "Runme runs Markdown",
   "description": "Execute your runbooks, docs, and READMEs.",
@@ -614,6 +614,11 @@
             ],
             "markdownDescription": "Default behavior of Open In Editor action button"
           },
+          "runme.notebook.executionOrder": {
+            "type": "boolean",
+            "default": true,
+            "markdownDescription": "Enable execution order counter per cell in notebook UX"
+          },
           "runme.codelens.enable": {
             "type": "boolean",
             "default": true,
diff --git a/src/extension/cell.ts b/src/extension/cell.ts
index 02491a2b..c9a71aab 100644
--- a/src/extension/cell.ts
+++ b/src/extension/cell.ts
@@ -28,11 +28,15 @@ import {
   XTermState,
 } from './terminal/terminalState'
 import ContextState from './contextState'
+import { IRunnerEnvironment } from './runner'
 
 const NOTEBOOK_SELECTION_COMMAND = '_notebook.selectKernel'
 
 export class NotebookCellManager {
   #data = new WeakMap<NotebookCell, NotebookCellOutputManager>()
+  #runnerEnv: IRunnerEnvironment | undefined
+  #executionSessionOrder = new Map<string, number>()
+  #mruSessionId = ''
 
   constructor(
     protected controller: NotebookController,
@@ -51,8 +55,23 @@ export class NotebookCellManager {
     return outputs
   }
 
-  async createNotebookCellExecution(cell: NotebookCell) {
+  setRunnerEnv(runnerEnv: IRunnerEnvironment) {
+    this.#runnerEnv = runnerEnv
+    this.#mruSessionId = runnerEnv.getSessionId()
+    this.#executionSessionOrder.set(this.#mruSessionId, 0)
+  }
+
+  incrementExecutionOrder(outputs: NotebookCellOutputManager): void {
+    let order = this.#executionSessionOrder.get(this.#mruSessionId) ?? 0
+    this.#executionSessionOrder.set(this.#mruSessionId, ++order)
+    outputs.setSessionExecutionOrder(this.#mruSessionId, order)
+  }
+
+  async createNotebookCellExecution(
+    cell: NotebookCell,
+  ): Promise<RunmeNotebookCellExecution | undefined> {
     const outputs = await this.getNotebookOutputs(cell)
+    this.incrementExecutionOrder(outputs)
     return outputs.createNotebookCellExecution()
   }
 
@@ -60,6 +79,8 @@ export class NotebookCellManager {
     const outputs = this.#data.get(cell)
     if (!outputs) {
       console.error(`cell at index ${cell.index} has not been registered!`)
+    } else {
+      outputs.setMruSessionId(this.#mruSessionId)
     }
 
     return outputs ?? this.registerCell(cell)
@@ -88,6 +109,9 @@ export class NotebookCellOutputManager {
     [OutputType.github, false],
   ])
 
+  protected sessionExecutionOrder = new Map<string, number | undefined>()
+  protected mruSessionId = ''
+
   protected cellState?: ICellState
 
   protected mutex: Mutex = new Mutex()
@@ -311,6 +335,7 @@ export class NotebookCellOutputManager {
         return undefined
       }
 
+      execution.executionOrder = this.currentExecutionOrder()
       this.execution = execution
 
       const wrapper = new RunmeNotebookCellExecution(execution)
@@ -460,7 +485,7 @@ export class NotebookCellOutputManager {
   }
 
   /**
-   * Internal refresh ouptut function. Runs under mutex.
+   * Internal refresh output function. Runs under mutex.
    *
    * @param mutater Optional callback, which runs after enabled outputs are
    * retrieved, but before outputs are replaced. This function should be used to
@@ -474,6 +499,7 @@ export class NotebookCellOutputManager {
   protected async refreshOutputInternal(mutater?: () => Promise<boolean | void> | boolean | void) {
     await this.withLock(async () => {
       await this.getExecutionUnsafe(async (exec) => {
+        exec.executionOrder = this.currentExecutionOrder()
         for (const key of [...this.enabledOutputs.keys()]) {
           this.enabledOutputs.set(key, this.hasOutputTypeUnsafe(key))
         }
@@ -533,6 +559,24 @@ export class NotebookCellOutputManager {
     }
   }
 
+  setSessionExecutionOrder(sessionId: string, order: number | undefined): number | undefined {
+    this.sessionExecutionOrder.set(sessionId, order)
+    this.setMruSessionId(sessionId)
+    return order
+  }
+
+  setMruSessionId(sessionId: string): void {
+    if (this.mruSessionId === sessionId) {
+      return
+    }
+    this.mruSessionId = sessionId
+  }
+
+  protected currentExecutionOrder(): number | undefined {
+    const mruExecOrder = this.sessionExecutionOrder.get(this.mruSessionId)
+    return mruExecOrder
+  }
+
   setState(state: ICellState) {
     this.cellState = state
   }
diff --git a/src/extension/kernel.ts b/src/extension/kernel.ts
index 81e8bf93..f488a6b7 100644
--- a/src/extension/kernel.ts
+++ b/src/extension/kernel.ts
@@ -28,6 +28,7 @@ import {
 } from '../constants'
 import { API } from '../utils/deno/api'
 import { postClientMessage } from '../utils/messaging'
+import { getNotebookExecutionOrder } from '../utils/configuration'
 
 import * as survey from './survey'
 import getLogger from './logger'
@@ -111,7 +112,7 @@ export class Kernel implements Disposable {
       this.#controller,
       this.hasExperimentEnabled('outputPersistence') ?? false,
     )
-    this.#controller.supportsExecutionOrder = false
+    this.#controller.supportsExecutionOrder = getNotebookExecutionOrder()
     this.#controller.description = 'Run your Markdown'
     this.#controller.executeHandler = this._executeAll.bind(this)
 
@@ -652,12 +653,14 @@ export class Kernel implements Disposable {
 
       const runnerEnv = await this.runner.createEnvironment(
         // copy env from process naively for now
-        // later we might want a more sophisticated approach/to bring this serverside
+        // later we might want a more sophisticated approach/to bring this server-side
         processEnviron(),
       )
 
       this.runnerEnv = runnerEnv
 
+      this.cellManager.setRunnerEnv(runnerEnv)
+
       // runs this last to not overwrite previous outputs
       await commands.executeCommand('notebook.clearAllCellsOutputs')
 
diff --git a/src/utils/configuration.ts b/src/utils/configuration.ts
index 68c2ba3d..c768b499 100644
--- a/src/utils/configuration.ts
+++ b/src/utils/configuration.ts
@@ -13,6 +13,7 @@ const ACTIONS_SECTION_NAME = 'runme.actions'
 const SERVER_SECTION_NAME = 'runme.server'
 const TERMINAL_SECTION_NAME = 'runme.terminal'
 const CODELENS_SECTION_NAME = 'runme.codelens'
+const NOTEBOOK_SECTION_NAME = 'runme.notebook'
 const ENV_SECTION_NAME = 'runme.env'
 const CLI_SECTION_NAME = 'runme.cli'
 const APP_SECTION_NAME = 'runme.app'
@@ -64,6 +65,9 @@ const configurationSchema = {
   codelens: {
     enable: z.boolean().default(true),
   },
+  notebook: {
+    executionOrder: z.boolean().default(true),
+  },
   env: {
     workspaceFileOrder: z.array(z.string()).default(DEFAULT_WORKSPACE_FILE_ORDER),
     loadWorkspaceFiles: z.boolean().default(true),
@@ -141,6 +145,19 @@ const getCodeLensConfigurationValue = <T>(
   return defaultValue
 }
 
+const getNotebookConfigurationValue = <T>(
+  configName: keyof typeof configurationSchema.notebook,
+  defaultValue: T,
+) => {
+  const configurationSection = workspace.getConfiguration(NOTEBOOK_SECTION_NAME)
+  const configurationValue = configurationSection.get<T>(configName)!
+  const parseResult = configurationSchema.notebook[configName].safeParse(configurationValue)
+  if (parseResult.success) {
+    return parseResult.data as T
+  }
+  return defaultValue
+}
+
 const getEnvConfigurationValue = <T>(
   configName: keyof typeof configurationSchema.env,
   defaultValue: T,
@@ -257,6 +274,10 @@ const getCodeLensEnabled = (): boolean => {
   return getCodeLensConfigurationValue<boolean>('enable', true)
 }
 
+const getNotebookExecutionOrder = (): boolean => {
+  return getNotebookConfigurationValue<boolean>('executionOrder', true)
+}
+
 const registerExtensionEnvironmentVariables = (context: ExtensionContext): void => {
   context.environmentVariableCollection.prepend(
     'PATH',
@@ -357,6 +378,7 @@ export {
   getTLSDir,
   getNotebookTerminalConfigurations,
   getCodeLensEnabled,
+  getNotebookExecutionOrder,
   registerExtensionEnvironmentVariables,
   getCustomServerAddress,
   getActionsOpenViewInEditor,


Test patch:
diff --git a/tests/extension/cell.test.ts b/tests/extension/cell.test.ts
index ef3161e8..8aa58bec 100644
--- a/tests/extension/cell.test.ts
+++ b/tests/extension/cell.test.ts
@@ -7,6 +7,7 @@ import {
   NotebookCellOutputManager,
   RunmeNotebookCellExecution,
 } from '../../src/extension/cell'
+import { IRunnerEnvironment } from '../../src/extension/runner'
 
 function mockedNotebookCellExecution(): NotebookCellExecution {
   return {
@@ -28,18 +29,66 @@ vi.mock('../../src/extension/grpc/client', () => ({}))
 vi.mock('../../src/extension/grpc/runnerTypes', () => ({}))
 
 describe('NotebookCellManager', () => {
-  it('can register cells', () => {
+  it('can register cells', async () => {
     const manager = new NotebookCellManager({} as any, false)
     const cell = {} as any
 
     manager.registerCell(cell)
-    expect(manager.getNotebookOutputs(cell)).toBeTruthy()
+    expect(await manager.getNotebookOutputs(cell)).toBeTruthy()
 
     const errorMock = vi.spyOn(console, 'error')
 
     expect(manager.getNotebookOutputs({} as any)).resolves.toBeInstanceOf(NotebookCellOutputManager)
     expect(errorMock.mock.calls[0][0]).toMatch('has not been registered')
   })
+
+  it('returns existing outputs on cell re-register', async () => {
+    const manager = new NotebookCellManager({} as any, false)
+    const cell = {} as any
+
+    const outputs1 = manager.registerCell(cell)
+    expect(outputs1).toBeTruthy()
+    const outputs2 = manager.registerCell(cell)
+    expect(outputs2).toBeTruthy()
+
+    expect(outputs1).toStrictEqual(outputs2)
+  })
+
+  it('set mru session id on outputs whenever they are being used', async () => {
+    const manager = new NotebookCellManager({} as any, false)
+    manager.setRunnerEnv(<IRunnerEnvironment>{ getSessionId: () => 'session-id1' })
+    const cell = {} as any
+
+    manager.registerCell(cell)
+    let outputs = await manager.getNotebookOutputs(cell)
+    expect((outputs as any).mruSessionId).toStrictEqual('session-id1')
+    expect(outputs).toBeTruthy()
+
+    manager.setRunnerEnv(<IRunnerEnvironment>{ getSessionId: () => 'session-id2' })
+
+    const outputsP = manager.getNotebookOutputs(cell)
+    expect(outputsP).resolves.toBeInstanceOf(NotebookCellOutputManager)
+    expect(((await outputsP) as any).mruSessionId).toStrictEqual('session-id2')
+  })
+
+  it('increment order for session id when an execution is created', async () => {
+    const createNotebookCellExecution = vi.fn()
+    const manager = new NotebookCellManager({ createNotebookCellExecution } as any, false)
+    manager.setRunnerEnv(<IRunnerEnvironment>{ getSessionId: () => 'session-id1' })
+    const cell = {} as any
+
+    manager.registerCell(cell)
+    const outputs = await manager.getNotebookOutputs(cell)
+    expect((outputs as any).mruSessionId).toStrictEqual('session-id1')
+    expect(outputs).toBeTruthy()
+
+    await manager.createNotebookCellExecution(cell)
+    await manager.createNotebookCellExecution(cell)
+    await manager.createNotebookCellExecution(cell)
+
+    expect((outputs as any).currentExecutionOrder()).toStrictEqual(3)
+    expect(createNotebookCellExecution).toBeCalledTimes(3)
+  })
 })
 
 function mockCellExecution(cell: NotebookCell) {
@@ -363,6 +412,59 @@ describe('NotebookCellOutputManager', () => {
     await outputs.refreshOutput(OutputType.annotations)
     expect(replaceOutput).toHaveBeenCalledTimes(1)
   })
+
+  describe('exec and order', () => {
+    const cell = mockCell()
+
+    const { controller, createExecution } = mockNotebookController(cell)
+
+    const outputs = new NotebookCellOutputManager(cell, controller, false)
+
+    it('sets mru session ID and order', () => {
+      outputs.setSessionExecutionOrder('session-id1', 3641)
+      expect((outputs as any).currentExecutionOrder()).toStrictEqual(3641)
+    })
+
+    it('resets order when mru session ID changes', () => {
+      outputs.setMruSessionId('session-id2')
+      expect((outputs as any).currentExecutionOrder()).toStrictEqual(undefined)
+    })
+
+    it('applies current session order to execution', async () => {
+      outputs.setSessionExecutionOrder('session-id1', 1337)
+      const exec1 = mockCellExecution(cell)
+      createExecution.mockReturnValue(exec1)
+
+      const runmeExec = await outputs.createNotebookCellExecution()
+      expect(runmeExec).toBeTruthy()
+
+      expect(runmeExec!.underlyingExecution.executionOrder).toStrictEqual(1337)
+    })
+  })
+
+  describe('outputs and order', () => {
+    it('retains session order for outputs', async () => {
+      const cell = mockCell()
+
+      const { controller, createExecution } = mockNotebookController(cell)
+
+      const outputs = new NotebookCellOutputManager(cell, controller, false)
+      outputs.setSessionExecutionOrder('session-id', 1337)
+
+      const exec1 = mockCellExecution(cell)
+      createExecution.mockReturnValue(exec1)
+
+      const runmeExec = await outputs.createNotebookCellExecution()
+      expect(runmeExec).toBeTruthy()
+
+      runmeExec!.start()
+
+      await outputs.toggleOutput(OutputType.annotations)
+      expect(exec1.executionOrder).toStrictEqual(1337)
+
+      runmeExec!.end(undefined)
+    })
+  })
 })
 
 describe('RunmeNotebookCellExecution', () => {
diff --git a/tests/extension/configuration.test.ts b/tests/extension/configuration.test.ts
index 1e120a22..8bfdc3da 100644
--- a/tests/extension/configuration.test.ts
+++ b/tests/extension/configuration.test.ts
@@ -14,6 +14,7 @@ import {
   getNotebookTerminalConfigurations,
   getCodeLensEnabled,
   getCLIUseIntegratedRunme,
+  getNotebookExecutionOrder,
 } from '../../src/utils/configuration'
 import { SERVER_PORT } from '../../src/constants'
 import { RunmeIdentity } from '../../src/extension/grpc/serializerTypes'
@@ -142,6 +143,10 @@ suite('Configuration', () => {
     expect(getCodeLensEnabled()).toStrictEqual(true)
   })
 
+  test('getNotebookExecutionOrder should return true by default', () => {
+    expect(getNotebookExecutionOrder()).toStrictEqual(true)
+  })
+
   test('getCLIUseIntegratedRunme should return false by default', () => {
     expect(getCLIUseIntegratedRunme()).toStrictEqual(false)
   })