This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7571-e878df3eee7ca3baee7b0c34eceb63548c59e68a in repository https://gitbox.apache.org/repos/asf/texera.git
commit cb6e5c649978215188ea92937ec583be9990c49e Author: Ryan Zhang <[email protected]> AuthorDate: Tue Aug 11 15:42:13 2026 -0700 feat(python-notebook-migration, frontend): remove the workspace toolbar entry point (#7571) ### What changes were proposed in this PR? This is the first PR of moving the AI generate workflow entry point from the workspace toolbar to the workflow dashboard (#7360). It removes the workspace toolbar entry point and the UI that exists only to serve it, so the canvas and dashboard versions never coexist. **Menu toolbar (`menu.component.{ts,html,scss}`)** - Removes the "AI generate workflow" button and the flow it started - Removes the now unused output that signaled the loading overlay, along with the imports and constructor dependencies that only the removed code used. The auto layout action and the workflow modifiable state stay, since other toolbar buttons rely on them. **Workspace (`workspace.component.{ts,html,scss}`)** - Removes the loading overlay and its elapsed time stopwatch, which were driven by the toolbar output. - Keeps the embedded notebook panel host. It displays a workflow's stored notebook and is driven by the workflow id, so it keeps working for the dashboard entry point. **Retained (shared, reused by the dashboard entry point)** - The import modal component and its diagram asset and license attributions. - The notebook to workflow conversion service. - The embedded notebook panel and its per workflow initialization. - The expand Jupyter panel button After this change the tool has no entry point until the dashboard entry point lands. The feature stays behind its existing feature flag, so users see no change. ### Any related issues, documentation, discussions? Closes #7564 Parent issue #4301 ### How was this PR tested? Updated `menu.component.spec.ts` and `workspace.component.spec.ts` to drop the tests for the removed button, the generation pipeline, and the loading timer. No new behavior is added, so no new tests were needed. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) --- .../workspace/component/menu/menu.component.html | 10 - .../component/menu/menu.component.spec.ts | 406 +-------------------- .../app/workspace/component/menu/menu.component.ts | 245 +------------ .../workspace/component/workspace.component.html | 17 +- .../workspace/component/workspace.component.scss | 24 -- .../component/workspace.component.spec.ts | 82 ----- .../app/workspace/component/workspace.component.ts | 44 --- 7 files changed, 9 insertions(+), 819 deletions(-) diff --git a/frontend/src/app/workspace/component/menu/menu.component.html b/frontend/src/app/workspace/component/menu/menu.component.html index 8b5314b49a..34fa3dccc4 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.html +++ b/frontend/src/app/workspace/component/menu/menu.component.html @@ -137,16 +137,6 @@ nz-icon nzType="info-circle"></i> </button> - <button - *ngIf="pythonNotebookMigrationEnabled" - nz-button - [disabled]="!isWorkflowModifiable || isWaitingForLLM" - (click)="openImportNotebookModal()" - title="AI generate workflow"> - <i - nz-icon - nzType="robot"></i> - </button> <button *ngIf="pythonNotebookMigrationEnabled && (jupyterNotebookExists$ | async)" nz-button diff --git a/frontend/src/app/workspace/component/menu/menu.component.spec.ts b/frontend/src/app/workspace/component/menu/menu.component.spec.ts index f35cea2ac1..9a35071cc2 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -38,10 +38,7 @@ import { WorkflowActionService } from "../../service/workflow-graph/model/workfl import { ValidationWorkflowService, ValidationOutput } from "../../service/validation/validation-workflow.service"; import { PanelService } from "../../service/panel/panel.service"; import { WorkflowVersionService } from "../../../dashboard/service/user/workflow-version/workflow-version.service"; -import { - WorkflowPersistService, - DEFAULT_WORKFLOW_NAME, -} from "../../../common/service/workflow-persist/workflow-persist.service"; +import { WorkflowPersistService } from "../../../common/service/workflow-persist/workflow-persist.service"; import { NotificationService } from "../../../common/service/notification/notification.service"; import { ExecutionState } from "../../types/execute-workflow.interface"; import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface"; @@ -52,13 +49,10 @@ import type { ComputingUnitSelectionComponent } from "../power-button/computing- import { WorkflowContent } from "../../../common/type/workflow"; import { Router } from "@angular/router"; import { ReportGenerationService } from "../../service/report-generation/report-generation.service"; -import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; -import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; -import { NotebookMigrationService } from "../../service/notebook-migration/notebook-migration.service"; -import { NotebookImportModalComponent } from "../notebook-import-modal/notebook-import-modal.component"; -import { NzUploadFile } from "ng-zorro-antd/upload"; +import { USER_WORKFLOW } from "../../../app-routing.constant"; import { GuiConfigService } from "../../../common/service/gui-config.service"; import { MockGuiConfigService } from "../../../common/service/gui-config.service.mock"; +import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; import type { Mocked } from "vitest"; vi.mock("file-saver", () => ({ saveAs: vi.fn() })); @@ -918,398 +912,4 @@ describe("MenuComponent", () => { expect(openSpy).toHaveBeenCalled(); }); }); - - // Coverage for the notebook -> workflow import flow: the modal wiring, the - // upload guard, and the read/generate/persist pipeline. The pipeline tests - // double as regressions for the spinner bugs (emit true on start, emit false - // only at the terminal state of every path). - describe("notebook import", () => { - let notebookMigrationService: NotebookMigrationService; - let jupyterPanelService: JupyterPanelService; - - const validNotebook = { - cells: [{ cell_type: "code", source: "print(1)", metadata: {} }], - metadata: {}, - nbformat: 4, - nbformat_minor: 5, - }; - - // A real File is a Blob, so FileReader.readAsText works in jsdom; NzUploadFile - // is the raw File augmented at runtime, matching what nz-upload passes through. - function ipynbFile(content: unknown, name = "my_nb.ipynb"): NzUploadFile { - return new File([JSON.stringify(content)], name, { type: "application/json" }) as unknown as NzUploadFile; - } - - beforeEach(() => { - notebookMigrationService = TestBed.inject(NotebookMigrationService); - jupyterPanelService = TestBed.inject(JupyterPanelService); - }); - - it("openImportNotebookModal opens the NotebookImportModalComponent with a requestImport callback and no menu footer", () => { - const createSpy = vi.spyOn(modalService, "create").mockReturnValue({} as unknown as NzModalRef); - - component.openImportNotebookModal(); - - expect(createSpy).toHaveBeenCalledTimes(1); - const config = createSpy.mock.calls[0][0] as ModalOptions; - expect(config.nzTitle).toBe("AI Generate Workflow from Python Notebook"); - expect(config.nzContent).toBe(NotebookImportModalComponent); - expect(config.nzFooter).toBeNull(); - expect(typeof (config.nzData as { requestImport: unknown }).requestImport).toBe("function"); - }); - - // Opens the modal and returns the requestImport callback the menu handed to it; calling - // it drives the overwrite-confirm + import decision (true => close modal, false => keep open). - function getRequestImport(): (file: NzUploadFile, model: string) => Promise<boolean> { - const createSpy = vi.spyOn(modalService, "create").mockReturnValue({} as unknown as NzModalRef); - component.openImportNotebookModal(); - const config = createSpy.mock.calls[0][0] as ModalOptions; - return (config.nzData as { requestImport: (file: NzUploadFile, model: string) => Promise<boolean> }) - .requestImport; - } - - it("imports directly and resolves true when the current workflow is empty", async () => { - const importSpy = vi.spyOn(component, "onClickImportNotebook").mockReturnValue(false); - const confirmSpy = vi.spyOn(modalService, "confirm").mockImplementation(() => ({}) as NzModalRef); - - const proceed = await getRequestImport()({ name: "x.ipynb" } as NzUploadFile, "gpt-4"); - - expect(confirmSpy).not.toHaveBeenCalled(); - expect(importSpy).toHaveBeenCalledWith({ name: "x.ipynb" }, "gpt-4"); - expect(proceed).toBe(true); - }); - - it("rejects a non-ipynb file: resolves false, errors, and neither confirms nor imports", async () => { - const importSpy = vi.spyOn(component, "onClickImportNotebook").mockReturnValue(false); - const confirmSpy = vi.spyOn(modalService, "confirm").mockImplementation(() => ({}) as NzModalRef); - const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); - - const proceed = await getRequestImport()({ name: "data.txt" } as NzUploadFile, "gpt-4"); - - // Resolving false keeps the modal open with the selection preserved; nothing started. - expect(proceed).toBe(false); - expect(errorSpy).toHaveBeenCalledWith("Please upload a valid Jupyter Notebook (.ipynb) file."); - expect(confirmSpy).not.toHaveBeenCalled(); - expect(importSpy).not.toHaveBeenCalled(); - }); - - it("confirms before overwriting a non-empty workflow, imports and resolves true on confirm", async () => { - workflowActionService.addOperator(mockScanPredicate, mockPoint); - const importSpy = vi.spyOn(component, "onClickImportNotebook").mockReturnValue(false); - const confirmSpy = vi.spyOn(modalService, "confirm").mockImplementation(() => ({}) as NzModalRef); - - const proceedPromise = getRequestImport()({ name: "x.ipynb" } as NzUploadFile, "gpt-4"); - - // Confirmation is shown; the import has not started. - expect(confirmSpy).toHaveBeenCalledTimes(1); - expect(importSpy).not.toHaveBeenCalled(); - - // Confirming ("Overwrite") starts the import and lets the modal close. - const confirmConfig = confirmSpy.mock.calls[0][0] as { nzOnOk: () => void }; - confirmConfig.nzOnOk(); - await expect(proceedPromise).resolves.toBe(true); - expect(importSpy).toHaveBeenCalledWith({ name: "x.ipynb" }, "gpt-4"); - }); - - it("resolves false without importing when the overwrite confirmation is cancelled", async () => { - workflowActionService.addOperator(mockScanPredicate, mockPoint); - const importSpy = vi.spyOn(component, "onClickImportNotebook").mockReturnValue(false); - const confirmSpy = vi.spyOn(modalService, "confirm").mockImplementation(() => ({}) as NzModalRef); - - const proceedPromise = getRequestImport()({ name: "x.ipynb" } as NzUploadFile, "gpt-4"); - - // Backing out keeps the modal open (resolve false) and starts no import. - const confirmConfig = confirmSpy.mock.calls[0][0] as { nzOnCancel: () => void }; - confirmConfig.nzOnCancel(); - await expect(proceedPromise).resolves.toBe(false); - expect(importSpy).not.toHaveBeenCalled(); - }); - - it("rejects a non-ipynb file without entering the loading state", () => { - const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - const result = component.onClickImportNotebook({ name: "data.txt" } as NzUploadFile, "gpt-4"); - - expect(result).toBe(false); - expect(errorSpy).toHaveBeenCalledWith("Please upload a valid Jupyter Notebook (.ipynb) file."); - expect(emitSpy).not.toHaveBeenCalledWith(true); - }); - - // Import always overwrites the current workflow: it reuses the current wid so - // persistWorkflow updates that row in place. When the current workflow was never - // saved (no wid) a new row is created and the wid changes, which routes the notebook - // send + panel open through JupyterPanelService.init() instead of doing it here. - function stubGenerationServices() { - // 1 == the notebook reached Jupyter; the in-place path opens the panel only on 1. - vi.spyOn(notebookMigrationService, "sendNotebookToJupyter").mockResolvedValue(1 as any); - vi.spyOn(notebookMigrationService, "sendToAIGenerateWorkflow").mockResolvedValue({ - workflowContent: { operators: [], links: [], commentBoxes: [], settings: {} } as unknown as WorkflowContent, - mappingContent: {} as any, - }); - vi.spyOn(notebookMigrationService, "setMapping").mockImplementation(() => {}); - vi.spyOn(notebookMigrationService, "storeNotebookAndMapping").mockReturnValue(of({ success: true }) as any); - vi.spyOn(workflowActionService, "reloadWorkflow").mockImplementation(() => {}); - vi.spyOn(jupyterPanelService, "openPanel").mockImplementation(() => {}); - vi.spyOn(notificationService, "success").mockImplementation(() => {}); - // The new-row branch updates the URL via Location.go; stub it out. - vi.spyOn(location, "go").mockImplementation(() => {}); - } - - it("overwrites the saved current workflow in place, reloads it, and opens the panel itself", async () => { - stubGenerationServices(); - // Saved current workflow (wid 7); persist keeps the same wid, so the wid does not change. - vi.spyOn(workflowActionService, "getWorkflow").mockReturnValue({ wid: 7 } as any); - const persistSpy = vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(of({ wid: 7 } as any)); - const autoLayoutSpy = vi.spyOn(component, "onClickAutoLayout").mockImplementation(() => {}); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - component.onClickImportNotebook(ipynbFile(validNotebook), "gpt-4"); - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - - expect(emitSpy).toHaveBeenCalledWith(true); - // Reuses the current wid so the row is overwritten in place; reloads synchronously on the - // live canvas and tidies the layout. - expect(persistSpy.mock.calls[0][0].wid).toBe(7); - expect(workflowActionService.reloadWorkflow).toHaveBeenCalledWith({ wid: 7 }, false); - expect(autoLayoutSpy).toHaveBeenCalled(); - // wid unchanged: we send the notebook + open the panel ourselves (init() does not react). - expect(notebookMigrationService.sendNotebookToJupyter).toHaveBeenCalled(); - expect(jupyterPanelService.openPanel).toHaveBeenCalledWith("JupyterNotebookPanel"); - // Stayed on the same workflow, so the URL is not changed. - expect(location.go).not.toHaveBeenCalled(); - }); - - it("marks isWaitingForLLM true at the start of import and false once the flow settles", async () => { - stubGenerationServices(); - vi.spyOn(workflowActionService, "getWorkflow").mockReturnValue({ wid: 7 } as any); - vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(of({ wid: 7 } as any)); - vi.spyOn(component, "onClickAutoLayout").mockImplementation(() => {}); - - component.onClickImportNotebook(ipynbFile(validNotebook), "gpt-4"); - // emit(true) fires synchronously at the start of the import. - expect(component.isWaitingForLLM).toBe(true); - - await vi.waitFor(() => expect(component.isWaitingForLLM).toBe(false)); - }); - - it("disables the AI-generate button while a conversion is in flight", () => { - const button = () => - fixture.nativeElement.querySelector('button[title="AI generate workflow"]') as HTMLButtonElement; - (TestBed.inject(GuiConfigService) as unknown as MockGuiConfigService).setConfig({ - pythonNotebookMigrationEnabled: true, - }); - // Isolate the waiting flag's effect from the modifiable gate. - component.isWorkflowModifiable = true; - fixture.detectChanges(); - expect(button().disabled).toBe(false); - - component.isWaitingForLLM = true; - fixture.detectChanges(); - expect(button().disabled).toBe(true); - }); - - it("clicking the AI-generate button opens the import modal", () => { - const openSpy = vi.spyOn(component, "openImportNotebookModal").mockImplementation(() => {}); - (TestBed.inject(GuiConfigService) as unknown as MockGuiConfigService).setConfig({ - pythonNotebookMigrationEnabled: true, - }); - component.isWorkflowModifiable = true; // enable the button so the click lands - fixture.detectChanges(); - - const button = fixture.nativeElement.querySelector('button[title="AI generate workflow"]') as HTMLButtonElement; - button.click(); - - expect(openSpy).toHaveBeenCalled(); - }); - - it("does not open the panel when the notebook fails to reach Jupyter", async () => { - stubGenerationServices(); - // sendNotebookToJupyter resolves 0 on failure (it toasts the error itself). - vi.spyOn(notebookMigrationService, "sendNotebookToJupyter").mockResolvedValue(0 as any); - vi.spyOn(workflowActionService, "getWorkflow").mockReturnValue({ wid: 7 } as any); - vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(of({ wid: 7 } as any)); - vi.spyOn(component, "onClickAutoLayout").mockImplementation(() => {}); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - component.onClickImportNotebook(ipynbFile(validNotebook), "gpt-4"); - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - // Let the sendNotebookToJupyter().then(...) microtask settle before asserting. - await Promise.resolve(); - - // The reload still happened, but the panel stays closed since the send failed. - expect(notebookMigrationService.sendNotebookToJupyter).toHaveBeenCalled(); - expect(workflowActionService.reloadWorkflow).toHaveBeenCalledWith({ wid: 7 }, false); - expect(jupyterPanelService.openPanel).not.toHaveBeenCalled(); - }); - - it("creates a new row and points the URL at it when the current workflow was never saved", async () => { - stubGenerationServices(); - // Current workflow has no wid; persist returns a new wid, so the wid changes. - vi.spyOn(workflowActionService, "getWorkflow").mockReturnValue({ wid: undefined } as any); - const persistSpy = vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(of({ wid: 99 } as any)); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - component.onClickImportNotebook(ipynbFile(validNotebook), "gpt-4"); - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - - // No current wid -> the backend inserts a new row. - expect(persistSpy.mock.calls[0][0].wid).toBeUndefined(); - expect(workflowActionService.reloadWorkflow).toHaveBeenCalledWith({ wid: 99 }, false); - expect(location.go).toHaveBeenCalledWith(`${USER_WORKSPACE}/99`); - // wid changed: JupyterPanelService.init() sends the notebook + opens the panel, not us, - // so the "sent to Jupyter" toast fires only once. - expect(notebookMigrationService.sendNotebookToJupyter).not.toHaveBeenCalled(); - expect(jupyterPanelService.openPanel).not.toHaveBeenCalled(); - }); - - it("on LLM error: surfaces an error notification and clears the loading flag", async () => { - vi.spyOn(console, "error").mockImplementation(() => {}); - vi.spyOn(notebookMigrationService, "sendNotebookToJupyter").mockResolvedValue(undefined as any); - vi.spyOn(notebookMigrationService, "sendToAIGenerateWorkflow").mockRejectedValue(new Error("boom")); - const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - component.onClickImportNotebook(ipynbFile(validNotebook), "gpt-4"); - - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - - expect(emitSpy).toHaveBeenCalledWith(true); - expect(errorSpy).toHaveBeenCalledWith("Error while communicating with LLM, check console for details"); - }); - - it("on invalid notebook structure: surfaces an error, clears the loading flag, and never calls jupyter", async () => { - vi.spyOn(console, "error").mockImplementation(() => {}); - const jupyterSpy = vi.spyOn(notebookMigrationService, "sendNotebookToJupyter"); - const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - // No `cells` array -> the structure guard throws before any network call. - component.onClickImportNotebook(ipynbFile({ metadata: {} }), "gpt-4"); - - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - - expect(errorSpy).toHaveBeenCalledWith("Failed to import the notebook."); - expect(jupyterSpy).not.toHaveBeenCalled(); - }); - - it("falls back to the default workflow name when the file has no base name", async () => { - stubGenerationServices(); - vi.spyOn(workflowActionService, "getWorkflow").mockReturnValue({ wid: 7 } as any); - const persistSpy = vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(of({ wid: 7 } as any)); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - // A file named ".ipynb" has an empty base name, so the default name is used. - component.onClickImportNotebook(ipynbFile(validNotebook, ".ipynb"), "gpt-4"); - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - - expect(persistSpy.mock.calls[0][0].name).toBe(`${DEFAULT_WORKFLOW_NAME}_GENERATED_BY_LLM`); - }); - - it("uses the whole file name when it has no dot", async () => { - stubGenerationServices(); - vi.spyOn(workflowActionService, "getWorkflow").mockReturnValue({ wid: 7 } as any); - const persistSpy = vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(of({ wid: 7 } as any)); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - // A file named "ipynb" (no dot) passes the extension check and has no extension to strip, - // so the whole name becomes the base name. - component.onClickImportNotebook(ipynbFile(validNotebook, "ipynb"), "gpt-4"); - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - - expect(persistSpy.mock.calls[0][0].name).toBe("ipynb_GENERATED_BY_LLM"); - }); - - it("tags code cells that arrive without a metadata object", async () => { - stubGenerationServices(); - const persistSpy = vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(of({ wid: 5 } as any)); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - const notebookWithoutCellMetadata = { - cells: [{ cell_type: "code", source: "x = 1" }], - metadata: {}, - nbformat: 4, - }; - component.onClickImportNotebook(ipynbFile(notebookWithoutCellMetadata), "gpt-4"); - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - - expect(persistSpy).toHaveBeenCalledTimes(1); - }); - - it("on persist failure: surfaces an error notification and clears the loading flag", async () => { - vi.spyOn(console, "error").mockImplementation(() => {}); - vi.spyOn(notebookMigrationService, "sendToAIGenerateWorkflow").mockResolvedValue({ - workflowContent: { operators: [], links: [], commentBoxes: [], settings: {} } as unknown as WorkflowContent, - mappingContent: {} as any, - }); - vi.spyOn(workflowPersistService, "persistWorkflow").mockReturnValue(throwError(() => new Error("db down"))); - const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - component.onClickImportNotebook(ipynbFile(validNotebook), "gpt-4"); - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - - expect(errorSpy).toHaveBeenCalledWith("Failed to import notebook, check console for detailed error"); - }); - - it("on file read error: surfaces an error and clears the loading flag", async () => { - const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - // Swap in a FileReader that errors instead of loading, so reader.onerror runs. - const RealFileReader = globalThis.FileReader; - class FakeFileReader { - onerror: ((e: unknown) => void) | null = null; - onload: (() => void) | null = null; - readAsText(): void { - setTimeout(() => this.onerror?.(new Error("read fail")), 0); - } - } - (globalThis as any).FileReader = FakeFileReader; - try { - component.onClickImportNotebook(ipynbFile(validNotebook), "gpt-4"); - await vi.waitFor(() => expect(errorSpy).toHaveBeenCalledWith("Failed to read the notebook file.")); - expect(emitSpy).toHaveBeenCalledWith(false); - } finally { - (globalThis as any).FileReader = RealFileReader; - } - }); - - it("on non-string file content: surfaces an error and clears the loading flag", async () => { - vi.spyOn(console, "error").mockImplementation(() => {}); - const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - // Swap in a FileReader that loads a non-string result, so the string guard throws. - const RealFileReader = globalThis.FileReader; - class FakeFileReader { - result: unknown = null; - onerror: (() => void) | null = null; - onload: (() => void) | null = null; - readAsText(): void { - setTimeout(() => this.onload?.(), 0); - } - } - (globalThis as any).FileReader = FakeFileReader; - try { - component.onClickImportNotebook(ipynbFile(validNotebook), "gpt-4"); - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - expect(errorSpy).toHaveBeenCalledWith("Failed to import the notebook."); - } finally { - (globalThis as any).FileReader = RealFileReader; - } - }); - - it("when the LLM returns no result: surfaces an error, clears the loading flag, and never persists", async () => { - vi.spyOn(console, "error").mockImplementation(() => {}); - vi.spyOn(notebookMigrationService, "sendToAIGenerateWorkflow").mockResolvedValue(undefined as any); - const persistSpy = vi.spyOn(workflowPersistService, "persistWorkflow"); - const errorSpy = vi.spyOn(notificationService, "error").mockImplementation(() => {}); - const emitSpy = vi.spyOn(component.setWaitingForLLM, "emit"); - - component.onClickImportNotebook(ipynbFile(validNotebook), "gpt-4"); - await vi.waitFor(() => expect(emitSpy).toHaveBeenCalledWith(false)); - - expect(errorSpy).toHaveBeenCalledWith("No workflow was generated from the notebook."); - expect(persistSpy).not.toHaveBeenCalled(); - }); - }); }); diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index 8930d9d0bb..b3e44057da 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -18,13 +18,10 @@ */ import { DatePipe, Location, NgIf, NgFor, NgTemplateOutlet, AsyncPipe } from "@angular/common"; -import { Component, ElementRef, Input, OnDestroy, OnInit, ViewChild, Output, EventEmitter } from "@angular/core"; +import { Component, ElementRef, Input, OnDestroy, OnInit, ViewChild } from "@angular/core"; import { Router, RouterLink } from "@angular/router"; import { UserService } from "../../../common/service/user/user.service"; -import { - DEFAULT_WORKFLOW_NAME, - WorkflowPersistService, -} from "../../../common/service/workflow-persist/workflow-persist.service"; +import { WorkflowPersistService } from "../../../common/service/workflow-persist/workflow-persist.service"; import { Workflow, WorkflowContent } from "../../../common/type/workflow"; import { ExecuteWorkflowService } from "../../service/execute-workflow/execute-workflow.service"; import { UndoRedoService } from "../../service/undo-redo/undo-redo.service"; @@ -42,14 +39,14 @@ import { saveAs } from "file-saver"; import { NotificationService } from "src/app/common/service/notification/notification.service"; import { OperatorMenuService } from "../../service/operator-menu/operator-menu.service"; import { CoeditorPresenceService } from "../../service/workflow-graph/model/coeditor-presence.service"; -import { EMPTY, firstValueFrom, of, timer, map } from "rxjs"; +import { EMPTY, firstValueFrom, of, timer } from "rxjs"; import { isDefined } from "../../../common/util/predicate"; import { NzModalService } from "ng-zorro-antd/modal"; import { ResultExportationComponent } from "../result-exportation/result-exportation.component"; import { ReportGenerationService } from "../../service/report-generation/report-generation.service"; import { ShareAccessComponent } from "src/app/dashboard/component/user/share-access/share-access.component"; import { PanelService } from "../../service/panel/panel.service"; -import { USER_WORKFLOW, USER_WORKSPACE } from "../../../app-routing.constant"; +import { USER_WORKFLOW } from "../../../app-routing.constant"; import { ComputingUnitStatusService } from "../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { ComputingUnitState } from "../../../common/type/computing-unit-connection.interface"; import { ComputingUnitSelectionComponent } from "../power-button/computing-unit-selection.component"; @@ -74,14 +71,6 @@ import { NzSwitchComponent } from "ng-zorro-antd/switch"; import { NzBadgeComponent } from "ng-zorro-antd/badge"; import { NzTooltipDirective } from "ng-zorro-antd/tooltip"; import { JupyterPanelService } from "../../service/jupyter-panel/jupyter-panel.service"; -import { v4 as uuidv4 } from "uuid"; -import { Notebook } from "../../service/notebook-migration/migration-llm"; -import { NotebookMigrationService } from "../../service/notebook-migration/notebook-migration.service"; -import { - NotebookImportModalComponent, - NotebookImportModalData, -} from "../notebook-import-modal/notebook-import-modal.component"; -import { NzUploadFile } from "ng-zorro-antd/upload"; /** * MenuComponent is the top level menu bar that shows @@ -155,9 +144,6 @@ export class MenuComponent implements OnInit, OnDestroy { @Input() public currentExecutionName: string = ""; // reset executionName @Input() public particularVersionDate: string = ""; // placeholder for the metadata information of a particular workflow version @ViewChild("workflowNameInput") workflowNameInput: ElementRef<HTMLInputElement> | undefined; - // Emit an event to parent component (workspace) when AI generation starts or stops - @Output() public setWaitingForLLM = new EventEmitter<boolean>(); - public isWaitingForLLM = false; // variable bound with HTML to decide if the running spinner should show public runButtonText = "Run"; @@ -199,8 +185,7 @@ export class MenuComponent implements OnInit, OnDestroy { private computingUnitStatusService: ComputingUnitStatusService, protected config: GuiConfigService, private router: Router, - private jupyterPanelService: JupyterPanelService, - private notebookMigrationService: NotebookMigrationService + private jupyterPanelService: JupyterPanelService ) { workflowWebsocketService .subscribeToEvent("ExecutionDurationUpdateEvent") @@ -614,226 +599,6 @@ export class MenuComponent implements OnInit, OnDestroy { this.jupyterPanelService.openJupyterNotebookPanel(); } - public openImportNotebookModal(): void { - // The modal owns the upload form and the model dropdown. It delegates the decision to - // proceed back here via requestImport so we keep the overwrite-confirm and the - // generation pipeline (and the workflow/persist/jupyter state they touch) in the menu. - this.modalService.create<NotebookImportModalComponent, NotebookImportModalData>({ - nzTitle: "AI Generate Workflow from Python Notebook", - nzContent: NotebookImportModalComponent, - nzWidth: 700, - nzFooter: null, - // Center in the viewport so the overwrite confirm (also centered) overlays this modal's center. - nzCentered: true, - nzData: { - requestImport: (file, model) => this.confirmAndImport(file, model), - }, - }); - } - - // Decides whether an import may proceed, then kicks it off. Resolves true when the import - // has started (the modal should close), false when the user backs out of the overwrite - // confirmation (the modal should stay open with the selection intact). - private confirmAndImport(file: NzUploadFile, model: string): Promise<boolean> { - // Reject a non-notebook file here, before starting anything, so the modal stays open - // with the selection intact (resolving false) instead of closing on a no-op import. - const fileExtension = file.name.split(".").pop()?.toLowerCase(); - if (fileExtension !== "ipynb") { - this.notificationService.error("Please upload a valid Jupyter Notebook (.ipynb) file."); - return Promise.resolve(false); - } - const startImport = () => this.onClickImportNotebook(file, model); - // Generating overwrites the currently open workflow. Confirm first only when there is - // actual content to replace; a fresh empty workflow needs no prompt. - const graph = this.workflowActionService.getTexeraGraph(); - const currentWorkflowHasContent = graph.getAllOperators().length > 0 || graph.getAllCommentBoxes().length > 0; - if (!currentWorkflowHasContent) { - startImport(); - return Promise.resolve(true); - } - return new Promise<boolean>(resolve => { - this.modalService.confirm({ - nzTitle: "Overwrite current workflow?", - nzContent: - "Generating will replace the contents of the workflow you have open. " + - "The previous version is kept in this workflow's version history.", - nzOkText: "Overwrite", - nzOkDanger: true, - // Center over the import modal, and leave only Cancel/Overwrite (no X, no click-outside). - nzCentered: true, - nzClosable: false, - nzMaskClosable: false, - nzOnOk: () => { - startImport(); - resolve(true); - }, - nzOnCancel: () => resolve(false), - }); - }); - } - - public onClickImportNotebook = (file: NzUploadFile, model: string): boolean => { - const reader = new FileReader(); - - // Check if the file is a Jupyter notebook based on its extension - const fileExtension = file.name.split(".").pop()?.toLowerCase(); - if (fileExtension !== "ipynb") { - this.notificationService.error("Please upload a valid Jupyter Notebook (.ipynb) file."); - return false; - } - - this.emitWaitingForLLM(true); // start loading - - // Read the notebook file as text - reader.readAsText(file as any); - reader.onload = async () => { - try { - const result = reader.result; - if (typeof result !== "string") { - throw new Error("File content is not a valid string."); - } - - // Parse the content of the .ipynb file (it's in JSON format) - const notebookContent = JSON.parse(result) as Notebook; - - // Validate the notebook structure - if (!notebookContent || !Array.isArray(notebookContent.cells)) { - throw new Error("Invalid notebook structure."); - } - - // Add UUID's to each cell in the notebook - for (const cell of notebookContent.cells) { - if (!cell.metadata) { - cell.metadata = {}; - } - cell.metadata.uuid = uuidv4(); - } - - // Get workflow and mapping from LLM - await this.notebookMigrationService - .sendToAIGenerateWorkflow(notebookContent, model) - .then(result => { - if (result) { - const { workflowContent, mappingContent } = result; - - const fileExtensionIndex = file.name.lastIndexOf("."); - let workflowName: string; - if (fileExtensionIndex === -1) { - workflowName = file.name; - } else { - workflowName = file.name.substring(0, fileExtensionIndex); - } - if (workflowName.trim() === "") { - workflowName = DEFAULT_WORKFLOW_NAME; - } - - // Always overwrite the current workflow: reuse its wid so persistWorkflow - // updates that row in place instead of inserting a new one (which would leave - // a duplicate behind). Read it now, after generation, so a wid assigned by - // auto-persist during the wait is picked up. If the current workflow was never - // saved, wid is undefined and a new row is created (there is nothing to overwrite). - const reuseWid = this.workflowActionService.getWorkflow().wid; - - const workflow: Workflow = { - content: workflowContent, - name: `${workflowName}_GENERATED_BY_LLM`, - isPublished: 0, - description: undefined, - wid: reuseWid, - creationTime: undefined, - lastModifiedTime: undefined, - readonly: false, - }; - - this.workflowPersistService - .persistWorkflow(workflow) - .pipe( - switchMap((updatedWorkflow: Workflow) => { - const mappingID = "mapping_wid_" + updatedWorkflow.wid; - - this.notebookMigrationService.setMapping(mappingID, mappingContent); - - return this.notebookMigrationService - .storeNotebookAndMapping(updatedWorkflow.wid, 1, mappingContent, notebookContent) - .pipe(map(() => updatedWorkflow)); - }), - untilDestroyed(this) - ) - .subscribe({ - next: updatedWorkflow => { - this.notificationService.success("Successfully generated workflow and mapping from notebook."); - // Reload the generated workflow onto the current (already live) canvas so it - // renders immediately; we never remount the workspace. Render synchronously - // (asyncRendering = false) so the operators exist before auto-layout runs. - this.workflowActionService.reloadWorkflow(updatedWorkflow, false); - // Tidy the LLM-generated layout; the position changes get auto-persisted. - this.onClickAutoLayout(); - if (reuseWid === updatedWorkflow.wid) { - // Overwrote the current workflow in place: the wid did not change, so - // JupyterPanelService.init() does not react. Send the notebook to Jupyter - // and open the panel ourselves. Use openPanel, not openJupyterNotebookPanel: - // init()'s wid-change handler is not involved and openPanel opens - // unconditionally without the hasMapping gate. - // sendNotebookToJupyter never rejects: it resolves 1 on success and 0 on - // failure (it toasts the error itself). Open the panel only on success so we - // do not float it over a blank iframe, matching the init()-driven path which - // opens only when fetchNotebookAndMapping reports the send succeeded. - this.notebookMigrationService.sendNotebookToJupyter(notebookContent).then(result => { - if (result == 1) { - this.jupyterPanelService.openPanel("JupyterNotebookPanel"); - } - }); - } else { - // The current workflow had never been saved, so a new row was created and the - // wid changed. reloadWorkflow's synchronous wid change drives init() to fetch - // the stored notebook/mapping, send it to Jupyter, and open the panel, so we - // do not do that here (doing so would double the "sent to Jupyter" toast). - // Point the URL at the generated workflow. - this.location.go(`${USER_WORKSPACE}/${updatedWorkflow.wid}`); - } - }, - error: (err: unknown) => { - this.notificationService.error("Failed to import notebook, check console for detailed error"); - console.error("Import notebook failed:", err); - this.emitWaitingForLLM(false); - }, - complete: () => { - this.emitWaitingForLLM(false); - }, - }); - } else { - this.notificationService.error("No workflow was generated from the notebook."); - console.error("Result is undefined"); - this.emitWaitingForLLM(false); - } - }) - .catch(error => { - this.notificationService.error("Error while communicating with LLM, check console for details"); - console.error("Error while fetching data from LLM: ", error); - this.emitWaitingForLLM(false); - }); - } catch (error) { - this.notificationService.error("Failed to import the notebook."); - console.error(error); - this.emitWaitingForLLM(false); - } - }; - - reader.onerror = () => { - this.notificationService.error("Failed to read the notebook file."); - this.emitWaitingForLLM(false); - }; - - return false; // Prevent automatic upload handling - }; - - // Keeps the local waiting flag and the parent-facing output in lockstep so the - // AI-generate button can be disabled while a conversion is in flight. - private emitWaitingForLLM(waiting: boolean): void { - this.isWaitingForLLM = waiting; - this.setWaitingForLLM.emit(waiting); - } - public onClickExportWorkflow(): void { const workflowContent: WorkflowContent = this.workflowActionService.getWorkflowContent(); const workflowContentJson = JSON.stringify(workflowContent, null, 2); diff --git a/frontend/src/app/workspace/component/workspace.component.html b/frontend/src/app/workspace/component/workspace.component.html index 78155285d4..b0c300b728 100644 --- a/frontend/src/app/workspace/component/workspace.component.html +++ b/frontend/src/app/workspace/component/workspace.component.html @@ -24,28 +24,13 @@ nzTip="Loading workflow..."> </nz-spin> </div> -<div class="openai-spinner"> - <nz-spin - [nzSize]="'large'" - [nzSpinning]="isWaitingForLLM"></nz-spin> - - @if (isWaitingForLLM) { - <div class="llm-spinner-text"> - <div>Waiting for LLM response...</div> - <div>Estimated time 1-5 minutes</div> - <div class="elapsed-time">Do not close this tab</div> - <div class="elapsed-time">Elapsed time: {{ formattedElapsedTime }}</div> - </div> - } -</div> <div id="result"> <texera-result-panel></texera-result-panel> </div> <texera-workflow-editor></texera-workflow-editor> <texera-menu [writeAccess]="writeAccess" - [pid]="pid" - (setWaitingForLLM)="onWaitingForLLMChanged($event)"> + [pid]="pid"> </texera-menu> <texera-mini-map class="box"></texera-mini-map> <texera-left-panel> </texera-left-panel> diff --git a/frontend/src/app/workspace/component/workspace.component.scss b/frontend/src/app/workspace/component/workspace.component.scss index 012fa3dfea..e01b1dc0f2 100644 --- a/frontend/src/app/workspace/component/workspace.component.scss +++ b/frontend/src/app/workspace/component/workspace.component.scss @@ -63,27 +63,3 @@ texera-workflow-editor { :host { user-select: none; } - -.openai-spinner { - position: absolute; - top: 50%; - left: 50%; - transform: translate(-50%, -50%); - display: flex; - flex-direction: column; - align-items: center; - justify-content: center; - height: 100%; - z-index: 10; - pointer-events: none; -} - -.llm-spinner-text { - margin-top: 20px; - text-align: center; -} - -.elapsed-time { - color: #ff4d4f; - font-weight: 500; -} diff --git a/frontend/src/app/workspace/component/workspace.component.spec.ts b/frontend/src/app/workspace/component/workspace.component.spec.ts index d930af601f..f85294e42a 100644 --- a/frontend/src/app/workspace/component/workspace.component.spec.ts +++ b/frontend/src/app/workspace/component/workspace.component.spec.ts @@ -479,86 +479,4 @@ describe("WorkspaceComponent", () => { expect(typeof codeEditorService.vc.createEmbeddedView).toBe("function"); }); }); - - // The LLM waiting spinner is driven by an elapsed-time timer started/stopped - // from the menu's setWaitingForLLM output. These tests pin the 1s cadence, the - // single-digit minute format, the stop-on-idle behavior, and, crucially, that - // the interval is cleared on destroy so it cannot keep firing detectChanges on - // a torn-down view. - describe("LLM waiting timer", () => { - afterEach(() => { - vi.useRealTimers(); - }); - - it("formattedElapsedTime is 0:00 before the timer starts", async () => { - await createFixture(); - fixture.detectChanges(); - expect(component.formattedElapsedTime).toBe("0:00"); - }); - - it("onWaitingForLLMChanged(true) starts the timer and advances elapsed time each second", async () => { - vi.useFakeTimers(); - await createFixture(); - fixture.detectChanges(); - - component.onWaitingForLLMChanged(true); - expect(component.isWaitingForLLM).toBe(true); - expect(component.formattedElapsedTime).toBe("0:00"); - - vi.advanceTimersByTime(1000); - expect(component.formattedElapsedTime).toBe("0:01"); - - // 1 minute 2 seconds later; minutes are not zero-padded. - vi.advanceTimersByTime(61000); - expect(component.formattedElapsedTime).toBe("1:02"); - }); - - it("onWaitingForLLMChanged(false) stops the timer so elapsed time no longer advances", async () => { - vi.useFakeTimers(); - await createFixture(); - fixture.detectChanges(); - - component.onWaitingForLLMChanged(true); - vi.advanceTimersByTime(1000); - expect(component.formattedElapsedTime).toBe("0:01"); - - component.onWaitingForLLMChanged(false); - expect(component.isWaitingForLLM).toBe(false); - // startTime is reset and the interval cleared, so further ticks do nothing. - vi.advanceTimersByTime(5000); - expect(component.formattedElapsedTime).toBe("0:00"); - }); - - it("clears the interval on destroy so the timer stops firing after teardown", async () => { - vi.useFakeTimers(); - await createFixture(); - fixture.detectChanges(); - - component.onWaitingForLLMChanged(true); - vi.advanceTimersByTime(1000); - - const clearSpy = vi.spyOn(globalThis, "clearInterval"); - component.ngOnDestroy(); - - expect(clearSpy).toHaveBeenCalled(); - }); - - it("clears the previous interval when the timer is started again without stopping", async () => { - vi.useFakeTimers(); - await createFixture(); - fixture.detectChanges(); - - const clearSpy = vi.spyOn(globalThis, "clearInterval"); - - component.onWaitingForLLMChanged(true); - const firstInterval = (component as any).timerInterval; - - // A second start (e.g. a double click) must not leave the first interval running. - component.onWaitingForLLMChanged(true); - const secondInterval = (component as any).timerInterval; - - expect(secondInterval).not.toBe(firstInterval); - expect(clearSpy).toHaveBeenCalledWith(firstInterval); - }); - }); }); diff --git a/frontend/src/app/workspace/component/workspace.component.ts b/frontend/src/app/workspace/component/workspace.component.ts index bdc0cdb8a9..2f95ccccfa 100644 --- a/frontend/src/app/workspace/component/workspace.component.ts +++ b/frontend/src/app/workspace/component/workspace.component.ts @@ -93,10 +93,6 @@ export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { public pid?: number = undefined; public writeAccess: boolean = false; public isLoading: boolean = false; - // variable to track whether we are waiting for AI to finish generating (whether a loading icon should show) - public isWaitingForLLM = false; - private timerInterval: ReturnType<typeof setInterval> | null = null; - private startTime: number | null = null; @ViewChild("codeEditor", { read: ViewContainerRef }) codeEditorViewRef!: ViewContainerRef; /** @@ -204,7 +200,6 @@ export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { // re-entered workflow starts clean instead of reusing the previous one. this.computingUnitStatusService.disconnect(); this.resetWorkflowSessionState(); - this.stopTimer(); } /** @@ -362,43 +357,4 @@ export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { public get copilotEnabled(): boolean { return this.config.env.copilotEnabled; } - - onWaitingForLLMChanged(isWaiting: boolean) { - this.isWaitingForLLM = isWaiting; - - if (isWaiting) { - this.startTimer(); - } else { - this.stopTimer(); - } - } - - startTimer() { - this.stopTimer(); // clear any interval already running so repeated starts don't stack - this.startTime = Date.now(); - this.updateElapsedTime(); - this.timerInterval = setInterval(() => { - this.updateElapsedTime(); - }, 1000); - } - - stopTimer() { - if (this.timerInterval !== null) { - clearInterval(this.timerInterval); - } - this.timerInterval = null; - this.startTime = null; - } - - updateElapsedTime() { - this.changeDetectorRef.detectChanges(); - } - - get formattedElapsedTime(): string { - if (!this.startTime) return "0:00"; - const diff = Date.now() - this.startTime; - const minutes = Math.floor(diff / 60000); - const seconds = Math.floor((diff % 60000) / 1000); - return `${minutes}:${seconds.toString().padStart(2, "0")}`; - } }
