Streamline App Console runme commands (clear/runAll/rerun)#82
Streamline App Console runme commands (clear/runAll/rerun)#82hamelsmu merged 3 commits intorunmedev:mainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dee203d1f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
app/src/lib/runtime/runmeConsole.ts
Outdated
| cellData.run(); | ||
| const runID = cellData.getRunID(); | ||
| if (runID) { |
There was a problem hiding this comment.
This success check can misreport failed starts as successful because it reads cellData.getRunID() after cellData.run(). In the real model, getRunID() returns the cell’s stored LastRunID metadata (app/src/lib/notebookData.ts:913-916), and runCodeCell returns "" without clearing that metadata when no runner is available (app/src/lib/notebookData.ts:537-545), so a cell with an old run ID is counted as started even when the new run did not launch (for example, backend unavailable after a prior successful run).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This pull request adds console helper functions for running all cells and clearing outputs in notebooks through the App Console. The implementation follows the established pattern of namespace-based console APIs and provides a clean programmatic interface for notebook operations that were previously only accessible through UI interactions.
Changes:
- Added
runmenamespace with helpers:getCurrentNotebook(),clearOutputs([notebook|uri]),runAll([notebook|uri]), andhelp() - Updated top-level and
runmehelp functions to display output in the console for better visibility - Added comprehensive unit tests for the new console API behavior
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/lib/runtime/runmeConsole.ts | Core implementation of the runme console API with functions to get notebook, clear outputs, run all cells, and display help |
| app/src/lib/runtime/runmeConsole.test.ts | Unit tests covering getCurrentNotebook, clearOutputs, and runAll functions with various scenarios |
| app/src/lib/runtime/jsKernel.ts | Modified to support custom help() function from globals while maintaining backward compatibility |
| app/src/components/AppConsole/AppConsole.tsx | Integrated the runme namespace into the console, added resolveNotebookData for flexible notebook resolution, and updated top-level help |
| app/src/components/Actions/Actions.tsx | Added quick-start console snippets demonstrating the new runme commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(model.getCell("cell-a")?.calls).toBe(1); | ||
| expect(model.getCell("cell-b")?.calls).toBe(0); | ||
| expect(model.getCell("cell-d")?.calls).toBe(1); | ||
| }); |
There was a problem hiding this comment.
The test suite is missing coverage for several important edge cases:
- Testing
clearOutputs()when no notebook is found (returns "No active notebook found.") - Testing
clearOutputs()when there are no outputs to clear - Testing
runAll()when no notebook is found - Testing
runAll()when there are no runnable cells - Testing the
help()function to ensure it returns the expected message - Testing with cells that have no refId
- Testing with cells that have metadata but no outputs
These edge cases are covered by the implementation but not verified in tests, which could lead to undetected regressions.
| }); | |
| }); | |
| it("returns a helpful message when clearOutputs is called with no active notebook", () => { | |
| const api = createRunmeConsoleApi({ | |
| // Simulate no active notebook being available | |
| resolveNotebook: () => undefined as unknown as NotebookDataLike, | |
| }); | |
| const message = api.clearOutputs(); | |
| expect(typeof message).toBe("string"); | |
| expect(message).toContain("No active notebook found."); | |
| }); | |
| it("handles clearOutputs when there are no outputs to clear", () => { | |
| const notebook = create(parser_pb.NotebookSchema, { | |
| cells: [ | |
| // Code cell with no outputs or metadata | |
| codeCell("cell-a", "echo nothing"), | |
| ], | |
| }); | |
| const model = new FakeNotebookData( | |
| "local://two", | |
| "Notebook Two", | |
| notebook, | |
| ); | |
| const api = createRunmeConsoleApi({ | |
| resolveNotebook: () => model, | |
| }); | |
| const message = api.clearOutputs(); | |
| // Follows the same reporting pattern as the existing test, but for zero items. | |
| expect(message).toContain("Cleared 0 output item group(s) across 0 cell(s)"); | |
| expect(model.updates).toHaveLength(0); | |
| }); | |
| it("returns a helpful message when runAll is called with no active notebook", () => { | |
| const api = createRunmeConsoleApi({ | |
| resolveNotebook: () => undefined as unknown as NotebookDataLike, | |
| }); | |
| const message = api.runAll(); | |
| expect(typeof message).toBe("string"); | |
| expect(message).toContain("No active notebook found."); | |
| }); | |
| it("handles runAll when there are no runnable code cells", () => { | |
| const notebook = create(parser_pb.NotebookSchema, { | |
| cells: [ | |
| // Empty code cell (non-runnable) | |
| codeCell("cell-a", " "), | |
| // Markup cell | |
| create(parser_pb.CellSchema, { | |
| refId: "cell-b", | |
| kind: parser_pb.CellKind.MARKUP, | |
| languageId: "markdown", | |
| value: "# title", | |
| }), | |
| ], | |
| }); | |
| const model = new FakeNotebookData( | |
| "local://three", | |
| "Notebook Three", | |
| notebook, | |
| ); | |
| const api = createRunmeConsoleApi({ | |
| resolveNotebook: () => model, | |
| }); | |
| const message = api.runAll(); | |
| // Should report that there were zero runnable code cells. | |
| expect(message).toContain("Started 0/0 code cell(s)"); | |
| }); | |
| it("returns a descriptive help message", () => { | |
| const api = createRunmeConsoleApi({ | |
| // The help text should not depend on an active notebook | |
| resolveNotebook: () => undefined as unknown as NotebookDataLike, | |
| }); | |
| const message = api.help(); | |
| expect(typeof message).toBe("string"); | |
| expect(message.length).toBeGreaterThan(0); | |
| // The help text should describe the main commands. | |
| expect(message).toContain("runAll"); | |
| expect(message).toContain("clearOutputs"); | |
| }); | |
| it("handles cells without refId when running all cells", () => { | |
| const notebook = create(parser_pb.NotebookSchema, { | |
| cells: [ | |
| // Code cell without a refId | |
| create(parser_pb.CellSchema, { | |
| kind: parser_pb.CellKind.CODE, | |
| languageId: "bash", | |
| value: "echo no-ref", | |
| }), | |
| ], | |
| }); | |
| const model = new FakeNotebookData( | |
| "local://four", | |
| "Notebook Four", | |
| notebook, | |
| ); | |
| const api = createRunmeConsoleApi({ | |
| resolveNotebook: () => model, | |
| }); | |
| const message = api.runAll(); | |
| // The API should handle this gracefully and still return a status message. | |
| expect(typeof message).toBe("string"); | |
| expect(message).toContain("Started"); | |
| }); | |
| it("clears run metadata even when cells have metadata but no outputs", () => { | |
| const notebook = create(parser_pb.NotebookSchema, { | |
| cells: [ | |
| // Cell with run-related metadata but no outputs | |
| codeCell("cell-a", "echo hi", { | |
| metadata: { | |
| [RunmeMetadataKey.LastRunID]: "run-cell-a", | |
| [RunmeMetadataKey.Pid]: "42", | |
| [RunmeMetadataKey.ExitCode]: "0", | |
| }, | |
| }), | |
| ], | |
| }); | |
| const model = new FakeNotebookData( | |
| "local://five", | |
| "Notebook Five", | |
| notebook, | |
| ); | |
| const api = createRunmeConsoleApi({ | |
| resolveNotebook: () => model, | |
| }); | |
| const message = api.clearOutputs(); | |
| expect(message).toContain("Cleared 0 output item group(s) across 0 cell(s)"); | |
| const updated = notebook.cells.find((cell) => cell.refId === "cell-a"); | |
| expect(updated?.metadata?.[RunmeMetadataKey.LastRunID]).toBeUndefined(); | |
| expect(updated?.metadata?.[RunmeMetadataKey.Pid]).toBeUndefined(); | |
| expect(updated?.metadata?.[RunmeMetadataKey.ExitCode]).toBeUndefined(); | |
| }); |
| import { create } from "@bufbuild/protobuf"; | ||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| import { RunmeMetadataKey, parser_pb } from "../../contexts/CellContext"; |
There was a problem hiding this comment.
The test file imports RunmeMetadataKey and parser_pb from ../../contexts/CellContext, while the main implementation file imports them from ../../runme/client. Although CellContext re-exports these from runme/client, it's better to import directly from the source to maintain consistency and avoid potential issues if the re-export is removed in the future. Consider changing this import to match the implementation file.
| import { RunmeMetadataKey, parser_pb } from "../../contexts/CellContext"; | |
| import { RunmeMetadataKey, parser_pb } from "../../runme/client"; |
| [ | ||
| "runme.getCurrentNotebook() - Return the active notebook handle", | ||
| "runme.clearOutputs([notebook]) - Clear all outputs in a notebook", | ||
| "runme.runAll([notebook]) - Run all non-empty code cells", | ||
| "runme.help() - Show this help", |
There was a problem hiding this comment.
The help messages describe the parameter as [notebook], which might mislead users into thinking they must pass a notebook object. However, the implementation accepts either a NotebookDataLike object, a URI string, or nothing (to use the current notebook). Consider updating the help messages to clarify this, for example: runme.clearOutputs([notebook|uri]) or add a comment explaining that the parameter can be a notebook handle, URI string, or omitted to use the current notebook.
hamelsmu
changed the title
1 participant
Summary
runme.clear()runme.runAll()runme.rerun()runme.clear(target)runme.runAll(target)runme.rerun(target)targetcan be a notebook handle (runme.getCurrentNotebook()) or a notebook URIrunme.clearOutputs()(alias ofrunme.clear())runme.clear()repaints notebook output immediately (output area disappears without manual refresh)Help Text Behavior
runme.help()now emphasizes zero-argument default usage first and markstargetforms as advanced/optional.Testing
cd web/app && npx vitest run src/lib/runtime/runmeConsole.test.tscd web/app && npx vitest run src/storage/contents.test.tscd web && pnpm run buildcd web/app && bash test/browser/test-smoke.sh 5173runme.runAll()shows outputrunme.clear()removes output area immediatelyrunme.rerun()clears + reruns current visible notebookCloses #46