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-7601-d94581ade4f421adcf3e8b58ae4be73afe04bdf1 in repository https://gitbox.apache.org/repos/asf/texera.git
commit f5017f72b776962062b0b540d8f6f831f6131a6a Author: Ryan Zhang <[email protected]> AuthorDate: Fri Aug 14 23:46:27 2026 +0000 feat(python-notebook-migration, frontend): add an AI generate workflow entry point on the dashboard (#7601) ### What changes were proposed in this PR? Adds an "AI generate workflow" entry point on the workflow dashboard for the Python notebook to Texera workflow migration tool. A user can upload a Jupyter notebook (`.ipynb`) and get a generated Texera workflow without opening a workflow first. **Dashboard button (`user-workflow.component.{ts,html}`)** - A new `robot` icon button in the workflows toolbar, styled like the existing "Upload ZIP/JSON" button and gated on `pythonNotebookMigrationEnabled` with a non read-only access level. - `openAiGenerateModal()` opens the same `NotebookImportModalComponent` used elsewhere, with no footer and centered. - `generateWorkflowFromNotebook(file, model)` runs the pipeline in stages, each with its own error toast: validate the `.ipynb` extension, `parseAndTagNotebook(file)` to read and uuid-tag the cells, `sendToAIGenerateWorkflow(notebook, model)` to call the LLM, then `createWorkflow(...)` to persist the result. After the workflow exists, it best-effort adds it to the current project, stores the notebook and cell mapping, and navigates to the new workflow with `?autolayout=1`. Once the workflow is created the flow never re-runs generation and never orphans the workflow. **Import modal (`notebook-import-modal.component.{ts,html,scss}`)** - The modal keeps the diagram at the top and holds the form plus a solid absolute loading overlay, so it does not resize or re-center when it swaps to the loading view. - The loading view shows a spinner, a "Generating your workflow" message, and an elapsed-time stopwatch computed from a wall-clock start time so a backgrounded tab cannot undercount. - While generation runs the modal is locked: it is not closable, mask-closable, or keyboard-dismissable, so a user cannot interrupt an in-flight generation. **Workspace auto-layout (`workspace.component.ts`)** - On load the workspace reads `?autolayout=1`, renders the workflow synchronously so the operators exist, runs auto-layout once, and then strips the query param. The Jupyter notebook panel opens on its own through the wid-driven `JupyterPanelService`. No generation code lives in the workspace. **Shared mapping key (`notebook-migration.service.ts`, `jupyter-panel.service.ts`)** - `notebookMappingKey(wid)` is now the single source of truth for the cache key shared by the dashboard store step and the Jupyter panel lookup. **Behavior note** - A wid is required to open the workspace, so the workflow is created before navigation. If navigation is later blocked, the workflow is still saved and reachable from the dashboard. This is intentional so a completed generation is never lost. #### Demo https://github.com/user-attachments/assets/b42098fb-d240-440a-becb-1674ec81bc7c Note: a mock LLM API was used in this demo so that we don't need to wait for real-time generation. This does not affect any functionality for this PR. ### Any related issues, documentation, discussions? Closes #7360 Parent issue #4301 ### How was this PR tested? Added and updated unit specs, with full line and branch coverage on the changed code: - `user-workflow.component.spec.ts` - `notebook-import-modal.component.spec.ts` - `workspace.component.spec.ts` - `notebook-migration.service.spec.ts` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) --- common/config/src/main/resources/gui.conf | 4 + .../apache/texera/common/config/GuiConfig.scala | 2 + .../texera/common/config/GuiConfigSpec.scala | 3 + .../texera/service/resource/ConfigResource.scala | 3 +- .../app/common/service/gui-config.service.mock.ts | 1 + frontend/src/app/common/type/gui-config.ts | 1 + .../user-workflow/user-workflow.component.html | 14 ++ .../user-workflow/user-workflow.component.spec.ts | 249 ++++++++++++++++++++- .../user/user-workflow/user-workflow.component.ts | 141 +++++++++++- .../notebook-import-modal.component.html | 229 ++++++++++--------- .../notebook-import-modal.component.scss | 53 ++++- .../notebook-import-modal.component.spec.ts | 112 ++++++++- .../notebook-import-modal.component.ts | 67 ++++-- .../component/workspace.component.spec.ts | 26 ++- .../app/workspace/component/workspace.component.ts | 13 +- .../service/jupyter-panel/jupyter-panel.service.ts | 12 +- .../notebook-migration/migration-llm.spec.ts | 99 +++++++- .../service/notebook-migration/migration-llm.ts | 59 ++++- .../notebook-migration.service.spec.ts | 60 ++++- .../notebook-migration.service.ts | 49 +++- 20 files changed, 1022 insertions(+), 175 deletions(-) diff --git a/common/config/src/main/resources/gui.conf b/common/config/src/main/resources/gui.conf index b9908304fb..d0f9f2e8cb 100644 --- a/common/config/src/main/resources/gui.conf +++ b/common/config/src/main/resources/gui.conf @@ -110,6 +110,10 @@ gui { # whether AI python-notebook migration feature is enabled python-notebook-migration-enabled = false python-notebook-migration-enabled = ${?GUI_WORKFLOW_WORKSPACE_PYTHON_NOTEBOOK_MIGRATION_ENABLED} + + # how long a single AI python-notebook migration LLM request may run before it is aborted + python-notebook-migration-timeout-minutes = 10 + python-notebook-migration-timeout-minutes = ${?GUI_WORKFLOW_WORKSPACE_PYTHON_NOTEBOOK_MIGRATION_TIMEOUT_MINUTES} } # whether to show the "Powered by Texera" attribution link in the sidebar diff --git a/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala b/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala index 6897b8e512..9968d97c3b 100644 --- a/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala +++ b/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala @@ -75,4 +75,6 @@ object GuiConfig { conf.getBoolean("gui.deployment-version-check-enabled") val guiWorkflowWorkspacePythonNotebookMigrationEnabled: Boolean = conf.getBoolean("gui.workflow-workspace.python-notebook-migration-enabled") + val guiWorkflowWorkspacePythonNotebookMigrationTimeoutMinutes: Int = + conf.getInt("gui.workflow-workspace.python-notebook-migration-timeout-minutes") } diff --git a/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala b/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala index afb1d908c4..e121a7cc6c 100644 --- a/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala +++ b/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala @@ -100,5 +100,8 @@ class GuiConfigSpec extends AnyFlatSpec with Matchers { ifUnset("GUI_WORKFLOW_WORKSPACE_LIMIT_COLUMNS")( GuiConfig.guiWorkflowWorkspaceLimitColumns shouldBe 15 ) + ifUnset("GUI_WORKFLOW_WORKSPACE_PYTHON_NOTEBOOK_MIGRATION_TIMEOUT_MINUTES")( + GuiConfig.guiWorkflowWorkspacePythonNotebookMigrationTimeoutMinutes shouldBe 10 + ) } } diff --git a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala index a8887cda92..18a35edc0a 100644 --- a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala +++ b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala @@ -94,7 +94,8 @@ class ConfigResource { "activeTimeInMinutes" -> GuiConfig.guiWorkflowWorkspaceActiveTimeInMinutes, "copilotEnabled" -> GuiConfig.guiWorkflowWorkspaceCopilotEnabled, "limitColumns" -> GuiConfig.guiWorkflowWorkspaceLimitColumns, - "pythonNotebookMigrationEnabled" -> GuiConfig.guiWorkflowWorkspacePythonNotebookMigrationEnabled + "pythonNotebookMigrationEnabled" -> GuiConfig.guiWorkflowWorkspacePythonNotebookMigrationEnabled, + "pythonNotebookMigrationTimeoutMinutes" -> GuiConfig.guiWorkflowWorkspacePythonNotebookMigrationTimeoutMinutes ) // Engine configs. diff --git a/frontend/src/app/common/service/gui-config.service.mock.ts b/frontend/src/app/common/service/gui-config.service.mock.ts index 93c8eaf0fd..667a774743 100644 --- a/frontend/src/app/common/service/gui-config.service.mock.ts +++ b/frontend/src/app/common/service/gui-config.service.mock.ts @@ -52,6 +52,7 @@ export class MockGuiConfigService { limitColumns: 15, attributionEnabled: false, pythonNotebookMigrationEnabled: false, + pythonNotebookMigrationTimeoutMinutes: 10, deploymentVersionCheckEnabled: false, }; diff --git a/frontend/src/app/common/type/gui-config.ts b/frontend/src/app/common/type/gui-config.ts index 5da549acd9..d9750d7d3b 100644 --- a/frontend/src/app/common/type/gui-config.ts +++ b/frontend/src/app/common/type/gui-config.ts @@ -43,6 +43,7 @@ export interface GuiConfig { limitColumns: number; attributionEnabled: boolean; pythonNotebookMigrationEnabled: boolean; + pythonNotebookMigrationTimeoutMinutes: number; deploymentVersionCheckEnabled: boolean; } diff --git a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.html b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.html index 492efc5376..d945a21d36 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.html +++ b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.html @@ -51,6 +51,20 @@ nzTheme="outline"></i> </button> </nz-upload> + <button + *ngIf="pythonNotebookMigrationEnabled" + [disabled]="accessLevel === 'READ'" + nz-button + (click)="openAiGenerateModal()" + title="AI generate a workflow from a Python notebook" + nz-tooltip="AI generate a workflow from a Python notebook" + nzTooltipPlacement="bottom" + type="button"> + <i + nz-icon + nzType="robot" + nzTheme="outline"></i> + </button> <button *ngIf="multiWorkflowsOperationButtonEnabled()" (click)="toggleSelection()" diff --git a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts index 64a305aadf..db2c2317b8 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.spec.ts @@ -67,12 +67,18 @@ import { StubSearchService } from "../../../service/user/stub-search.service"; import { SearchResultsComponent } from "../search-results/search-results.component"; import { delay, firstValueFrom, of, throwError } from "rxjs"; import JSZip from "jszip"; -import { NzModalService } from "ng-zorro-antd/modal"; +import { ModalOptions, NzModalRef, NzModalService } from "ng-zorro-antd/modal"; import { NzButtonModule } from "ng-zorro-antd/button"; import { DownloadService } from "../../../service/user/download/download.service"; import { commonTestProviders } from "../../../../common/testing/test-utils"; import { Router } from "@angular/router"; import { USER_WORKSPACE } from "../../../../app-routing.constant"; +import { GuiConfigService } from "../../../../common/service/gui-config.service"; +import { MockGuiConfigService } from "../../../../common/service/gui-config.service.mock"; +import { NotebookMigrationService } from "../../../../workspace/service/notebook-migration/notebook-migration.service"; +import { LlmRequestTimeoutError } from "../../../../workspace/service/notebook-migration/migration-llm"; +import { NotebookImportModalComponent } from "../../../../workspace/component/notebook-import-modal/notebook-import-modal.component"; +import { NzUploadFile } from "ng-zorro-antd/upload"; import type { Mocked } from "vitest"; describe("SavedWorkflowSectionComponent", () => { let component: UserWorkflowComponent; @@ -340,6 +346,247 @@ describe("SavedWorkflowSectionComponent", () => { }); }); + describe("AI generate workflow (dashboard entry point)", () => { + const ipynbFile = { name: "analysis.ipynb" } as NzUploadFile; + const AI_BUTTON_SELECTOR = 'button[title="AI generate a workflow from a Python notebook"]'; + + // Opens the modal and returns the requestImport callback the component handed to it; calling + // it runs the full generation (true => generation succeeded and navigated, false => stay open). + function getRequestImport(): (file: NzUploadFile, model: string) => Promise<boolean> { + const modalService = TestBed.inject(NzModalService); + const createSpy = vi.spyOn(modalService, "create").mockReturnValue({} as unknown as NzModalRef); + component.openAiGenerateModal(); + const config = createSpy.mock.calls[0][0] as ModalOptions; + return (config.nzData as { requestImport: (file: NzUploadFile, model: string) => Promise<boolean> }) + .requestImport; + } + + // Wires the NotebookMigrationService + persistence mocks for a successful generation. + function mockGenerationSuccess(wid = 99) { + const migration = TestBed.inject(NotebookMigrationService); + vi.spyOn(migration, "parseAndTagNotebook").mockResolvedValue({ cells: [] } as any); + vi.spyOn(migration, "sendToAIGenerateWorkflow").mockResolvedValue({ + workflowContent: { operators: [] }, + mappingContent: { operator_to_cell: {}, cell_to_operator: {} }, + } as any); + const storeSpy = vi.spyOn(migration, "storeNotebookAndMapping").mockReturnValue(of({ success: true }) as any); + const persist = TestBed.inject(WorkflowPersistService) as any; + persist.createWorkflow = vi.fn().mockReturnValue(of({ workflow: { wid } })); + return { storeSpy, persist }; + } + + it("openAiGenerateModal opens the NotebookImportModalComponent with a requestImport callback and no footer", () => { + const modalService = TestBed.inject(NzModalService); + const createSpy = vi.spyOn(modalService, "create").mockReturnValue({} as unknown as NzModalRef); + + component.openAiGenerateModal(); + + expect(createSpy).toHaveBeenCalledTimes(1); + const config = createSpy.mock.calls[0][0] as ModalOptions; + expect(config.nzContent).toBe(NotebookImportModalComponent); + expect(config.nzFooter).toBeNull(); + expect(typeof (config.nzData as { requestImport: unknown }).requestImport).toBe("function"); + }); + + it("generates a workflow, stores the notebook and mapping, navigates with autolayout, and resolves true", async () => { + const { storeSpy, persist } = mockGenerationSuccess(99); + const navigateSpy = vi.spyOn(TestBed.inject(Router), "navigate").mockResolvedValue(true); + component.pid = undefined; + + const proceed = await getRequestImport()(ipynbFile, "gpt-4"); + + expect(persist.createWorkflow).toHaveBeenCalledTimes(1); + // Name is derived from the notebook filename with the generated marker. + expect(persist.createWorkflow.mock.calls[0][1]).toBe("analysis_GENERATED_BY_LLM"); + // The dashboard hands off the key + vid ownership to the service in one call. + expect(storeSpy).toHaveBeenCalledWith(99, expect.anything(), expect.anything()); + expect(navigateSpy).toHaveBeenCalledWith([USER_WORKSPACE, 99], { queryParams: { autolayout: 1 } }); + expect(proceed).toBe(true); + }); + + it("truncates a long notebook basename so the generated name fits the 128-char column", async () => { + const { persist } = mockGenerationSuccess(99); + vi.spyOn(TestBed.inject(Router), "navigate").mockResolvedValue(true); + + await getRequestImport()({ name: "a".repeat(200) + ".ipynb" } as NzUploadFile, "gpt-4"); + + const createdName = persist.createWorkflow.mock.calls[0][1] as string; + expect(createdName.length).toBe(128); + expect(createdName.endsWith("_GENERATED_BY_LLM")).toBe(true); + }); + + it("saves but does not navigate when the component was destroyed mid-generation", async () => { + const { persist } = mockGenerationSuccess(99); + const navigateSpy = vi.spyOn(TestBed.inject(Router), "navigate").mockResolvedValue(true); + const infoSpy = vi.spyOn(TestBed.inject(NotificationService), "info").mockImplementation(() => {}); + const requestImport = getRequestImport(); + component.ngOnDestroy(); + + const proceed = await requestImport(ipynbFile, "gpt-4"); + + // The workflow is still created and saved, but the user is not yanked into the workspace. + expect(persist.createWorkflow).toHaveBeenCalledTimes(1); + expect(navigateSpy).not.toHaveBeenCalled(); + expect(infoSpy).toHaveBeenCalledWith("Workflow generated and saved to your dashboard."); + expect(proceed).toBe(true); + }); + + it("adds the new workflow to the current project when opened inside one", async () => { + mockGenerationSuccess(99); + vi.spyOn(TestBed.inject(Router), "navigate").mockResolvedValue(true); + const projectService = TestBed.inject(UserProjectService) as any; + const addSpy = vi.spyOn(projectService, "addWorkflowToProject").mockReturnValue(of(undefined)); + component.pid = 5; + + const proceed = await getRequestImport()(ipynbFile, "gpt-4"); + + expect(addSpy).toHaveBeenCalledWith(5, 99); + expect(proceed).toBe(true); + }); + + it("rejects a non-ipynb file: errors, resolves false, and generates nothing", async () => { + const parseSpy = vi.spyOn(TestBed.inject(NotebookMigrationService), "parseAndTagNotebook"); + const errorSpy = vi.spyOn(TestBed.inject(NotificationService), "error").mockImplementation(() => {}); + + const proceed = await getRequestImport()({ name: "data.txt" } as NzUploadFile, "gpt-4"); + + expect(proceed).toBe(false); + expect(errorSpy).toHaveBeenCalledWith("Please upload a valid Jupyter Notebook (.ipynb) file."); + expect(parseSpy).not.toHaveBeenCalled(); + }); + + it("reports a parse failure and resolves false without calling the LLM", async () => { + const migration = TestBed.inject(NotebookMigrationService); + vi.spyOn(migration, "parseAndTagNotebook").mockRejectedValue(new Error("bad json")); + const llmSpy = vi.spyOn(migration, "sendToAIGenerateWorkflow"); + const errorSpy = vi.spyOn(TestBed.inject(NotificationService), "error").mockImplementation(() => {}); + + const proceed = await getRequestImport()(ipynbFile, "gpt-4"); + + expect(proceed).toBe(false); + expect(errorSpy).toHaveBeenCalledWith("Failed to read the notebook file. Please upload a valid .ipynb file."); + expect(llmSpy).not.toHaveBeenCalled(); + }); + + it("reports an LLM failure and resolves false without creating a workflow", async () => { + const migration = TestBed.inject(NotebookMigrationService); + vi.spyOn(migration, "parseAndTagNotebook").mockResolvedValue({ cells: [] } as any); + vi.spyOn(migration, "sendToAIGenerateWorkflow").mockRejectedValue(new Error("LLM down")); + const persist = TestBed.inject(WorkflowPersistService) as any; + persist.createWorkflow = vi.fn(); + const errorSpy = vi.spyOn(TestBed.inject(NotificationService), "error").mockImplementation(() => {}); + + const proceed = await getRequestImport()(ipynbFile, "gpt-4"); + + expect(proceed).toBe(false); + expect(errorSpy).toHaveBeenCalledWith("Error while communicating with the LLM, check console for details."); + expect(persist.createWorkflow).not.toHaveBeenCalled(); + }); + + it("reports a distinct timeout message when generation times out", async () => { + const migration = TestBed.inject(NotebookMigrationService); + vi.spyOn(migration, "parseAndTagNotebook").mockResolvedValue({ cells: [] } as any); + vi.spyOn(migration, "sendToAIGenerateWorkflow").mockRejectedValue(new LlmRequestTimeoutError(10)); + const persist = TestBed.inject(WorkflowPersistService) as any; + persist.createWorkflow = vi.fn(); + const errorSpy = vi.spyOn(TestBed.inject(NotificationService), "error").mockImplementation(() => {}); + + const proceed = await getRequestImport()(ipynbFile, "gpt-4"); + + expect(proceed).toBe(false); + expect(errorSpy).toHaveBeenCalledWith( + "Generation timed out after 10 minutes. Try again, choose a faster model, or simplify the notebook." + ); + expect(persist.createWorkflow).not.toHaveBeenCalled(); + }); + + it("reports a save failure and resolves false when the created workflow has no wid", async () => { + const migration = TestBed.inject(NotebookMigrationService); + vi.spyOn(migration, "parseAndTagNotebook").mockResolvedValue({ cells: [] } as any); + vi.spyOn(migration, "sendToAIGenerateWorkflow").mockResolvedValue({ + workflowContent: {}, + mappingContent: {}, + } as any); + const storeSpy = vi.spyOn(migration, "storeNotebookAndMapping"); + const persist = TestBed.inject(WorkflowPersistService) as any; + persist.createWorkflow = vi.fn().mockReturnValue(of({ workflow: {} })); + const errorSpy = vi.spyOn(TestBed.inject(NotificationService), "error").mockImplementation(() => {}); + + const proceed = await getRequestImport()(ipynbFile, "gpt-4"); + + expect(proceed).toBe(false); + expect(errorSpy).toHaveBeenCalledWith("Failed to save the generated workflow, check console for details."); + expect(storeSpy).not.toHaveBeenCalled(); + }); + + it("still opens the workflow when adding it to the project fails (best effort)", async () => { + mockGenerationSuccess(99); + const navigateSpy = vi.spyOn(TestBed.inject(Router), "navigate").mockResolvedValue(true); + const projectService = TestBed.inject(UserProjectService) as any; + vi.spyOn(projectService, "addWorkflowToProject").mockReturnValue(throwError(() => new Error("project down"))); + component.pid = 5; + + const proceed = await getRequestImport()(ipynbFile, "gpt-4"); + + expect(navigateSpy).toHaveBeenCalledWith([USER_WORKSPACE, 99], { queryParams: { autolayout: 1 } }); + expect(proceed).toBe(true); + }); + + it("warns but still opens the workflow when storing the notebook fails (no re-generation)", async () => { + const { storeSpy, persist } = mockGenerationSuccess(99); + storeSpy.mockReturnValue(throwError(() => new Error("store down")) as any); + const navigateSpy = vi.spyOn(TestBed.inject(Router), "navigate").mockResolvedValue(true); + const warnSpy = vi.spyOn(TestBed.inject(NotificationService), "warning").mockImplementation(() => {}); + component.pid = undefined; + + const proceed = await getRequestImport()(ipynbFile, "gpt-4"); + + // The created workflow is kept and opened; the LLM call is not re-run. + expect(persist.createWorkflow).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + "Workflow created, but the notebook could not be attached; the Jupyter panel may not open." + ); + expect(navigateSpy).toHaveBeenCalledWith([USER_WORKSPACE, 99], { queryParams: { autolayout: 1 } }); + expect(proceed).toBe(true); + }); + + it("warns but resolves true when navigation is blocked after the workflow is created", async () => { + mockGenerationSuccess(99); + vi.spyOn(TestBed.inject(Router), "navigate").mockRejectedValue(new Error("blocked")); + const warnSpy = vi.spyOn(TestBed.inject(NotificationService), "warning").mockImplementation(() => {}); + component.pid = undefined; + + const proceed = await getRequestImport()(ipynbFile, "gpt-4"); + + expect(warnSpy).toHaveBeenCalledWith("Workflow created. You can open it from your dashboard."); + expect(proceed).toBe(true); + }); + + it("shows the button only when the migration flag is enabled", () => { + expect(fixture.nativeElement.querySelector(AI_BUTTON_SELECTOR)).toBeNull(); + + (TestBed.inject(GuiConfigService) as unknown as MockGuiConfigService).setConfig({ + pythonNotebookMigrationEnabled: true, + }); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector(AI_BUTTON_SELECTOR)).not.toBeNull(); + }); + + it("clicking the toolbar button opens the AI generate modal", () => { + (TestBed.inject(GuiConfigService) as unknown as MockGuiConfigService).setConfig({ + pythonNotebookMigrationEnabled: true, + }); + fixture.detectChanges(); + const openSpy = vi.spyOn(component, "openAiGenerateModal").mockImplementation(() => {}); + + const button = fixture.nativeElement.querySelector(AI_BUTTON_SELECTOR) as HTMLButtonElement; + button.click(); + + expect(openSpy).toHaveBeenCalled(); + }); + }); + it("downloads checked files", async () => { // If multiple workflows in a single batch download have name conflicts, rename them as workflow-1, workflow-2, etc. component.searchResultsComponent.entries = component.searchResultsComponent.entries.concat( diff --git a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts index e46582ea1d..4bd6acda2b 100644 --- a/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts +++ b/frontend/src/app/dashboard/component/user/user-workflow/user-workflow.component.ts @@ -17,7 +17,7 @@ * under the License. */ -import { AfterViewInit, Component, Input, ViewChild } from "@angular/core"; +import { AfterViewInit, Component, Input, OnDestroy, ViewChild } from "@angular/core"; import { Router } from "@angular/router"; import { NzModalService } from "ng-zorro-antd/modal"; import { firstValueFrom, from, lastValueFrom, Observable, of } from "rxjs"; @@ -46,6 +46,15 @@ import { DashboardWorkflow } from "../../../type/dashboard-workflow.interface"; import { DownloadService } from "../../../service/user/download/download.service"; import { USER_WORKSPACE } from "../../../../app-routing.constant"; import { GuiConfigService } from "../../../../common/service/gui-config.service"; +import { + MappingContent, + NotebookMigrationService, +} from "../../../../workspace/service/notebook-migration/notebook-migration.service"; +import { LlmRequestTimeoutError, Notebook } from "../../../../workspace/service/notebook-migration/migration-llm"; +import { + NotebookImportModalComponent, + NotebookImportModalData, +} from "../../../../workspace/component/notebook-import-modal/notebook-import-modal.component"; import { NzCardComponent } from "ng-zorro-antd/card"; import { NzSpaceCompactItemDirective, NzSpaceCompactComponent } from "ng-zorro-antd/space"; import { NzButtonComponent } from "ng-zorro-antd/button"; @@ -112,8 +121,10 @@ import { FormsModule } from "@angular/forms"; NzSpaceCompactComponent, ], }) -export class UserWorkflowComponent implements AfterViewInit { +export class UserWorkflowComponent implements AfterViewInit, OnDestroy { private static readonly VIEW_MODE_STORAGE_KEY = "texera.userWorkflow.viewMode"; + // Set on teardown so a generation that finishes after the user leaves does not navigate them back. + private destroyed = false; private _searchResultsComponent?: SearchResultsComponent; public isLogin = this.userService.isLogin(); private includePublic = false; @@ -157,7 +168,8 @@ export class UserWorkflowComponent implements AfterViewInit { private router: Router, private downloadService: DownloadService, private searchService: SearchService, - private config: GuiConfigService + private config: GuiConfigService, + private notebookMigrationService: NotebookMigrationService ) { this.userService .userChanged() @@ -199,6 +211,10 @@ export class UserWorkflowComponent implements AfterViewInit { .subscribe(() => this.search()); } + ngOnDestroy(): void { + this.destroyed = true; + } + /** * open the Modal to add workflow(s) to project */ @@ -312,6 +328,118 @@ export class UserWorkflowComponent implements AfterViewInit { }); } + public get pythonNotebookMigrationEnabled(): boolean { + return this.config.env.pythonNotebookMigrationEnabled; + } + + /** Open the AI-generate import modal, wiring its submit to generateWorkflowFromNotebook. */ + public openAiGenerateModal(): void { + this.modalService.create<NotebookImportModalComponent, NotebookImportModalData>({ + nzTitle: "AI Generate Workflow from Python Notebook", + nzContent: NotebookImportModalComponent, + nzWidth: 700, + nzFooter: null, + nzCentered: true, + nzData: { + requestImport: (file, model) => this.generateWorkflowFromNotebook(file, model), + }, + }); + } + + /** + * Parse the notebook, generate a workflow via the LLM, save it, store the cell mapping, and open it. + * Resolves true on success (modal closes), false to keep the modal open on a bad file or a failure. + */ + private async generateWorkflowFromNotebook(file: NzUploadFile, model: string): Promise<boolean> { + const fileExtension = file.name.split(".").pop()?.toLowerCase(); + if (fileExtension !== "ipynb") { + this.notificationService.error("Please upload a valid Jupyter Notebook (.ipynb) file."); + return false; + } + let notebook: Notebook; + try { + notebook = await this.notebookMigrationService.parseAndTagNotebook(file as unknown as File); + } catch (error) { + this.notificationService.error("Failed to read the notebook file. Please upload a valid .ipynb file."); + console.error("Notebook parse failed:", error); + return false; + } + + let generated: { workflowContent: WorkflowContent; mappingContent: MappingContent }; + try { + generated = await this.notebookMigrationService.sendToAIGenerateWorkflow(notebook, model); + } catch (error) { + if (error instanceof LlmRequestTimeoutError) { + this.notificationService.error( + `Generation timed out after ${error.minutes} minutes. Try again, choose a faster model, or simplify the notebook.` + ); + } else { + this.notificationService.error("Error while communicating with the LLM, check console for details."); + } + console.error("LLM generation failed:", error); + return false; + } + + // Commit point: persisting captures the expensive LLM result. On failure nothing was created, + // so returning false to let the user retry is safe. + let wid: number; + try { + // workflow.name is VARCHAR(128); cap the base so base + suffix fits the column. + const generatedSuffix = "_GENERATED_BY_LLM"; + const generatedName = this.deriveWorkflowName(file.name).slice(0, 128 - generatedSuffix.length); + const createdWorkflow = await firstValueFrom( + this.workflowPersistService.createWorkflow(generated.workflowContent, generatedName + generatedSuffix) + ); + if (!createdWorkflow.workflow.wid) { + throw new Error("Created workflow has no wid."); + } + wid = createdWorkflow.workflow.wid; + } catch (error) { + this.notificationService.error("Failed to save the generated workflow, check console for details."); + console.error("Saving the generated workflow failed:", error); + return false; + } + + // Best-effort follow-ups: never discard the created workflow, so log/warn and still open it. + if (this.pid) { + try { + await firstValueFrom(this.userProjectService.addWorkflowToProject(this.pid, wid)); + } catch (error) { + console.error("Adding the generated workflow to the project failed:", error); + } + } + try { + await firstValueFrom( + this.notebookMigrationService.storeNotebookAndMapping(wid, generated.mappingContent, notebook) + ); + } catch (error) { + this.notificationService.warning( + "Workflow created, but the notebook could not be attached; the Jupyter panel may not open." + ); + console.error("Storing the notebook and mapping failed:", error); + } + + if (this.destroyed) { + this.notificationService.info("Workflow generated and saved to your dashboard."); + return true; + } + + const navigated = await this.router + .navigate([USER_WORKSPACE, wid], { queryParams: { autolayout: 1 } }) + .catch(() => false); + if (!navigated) { + this.notificationService.warning("Workflow created. You can open it from your dashboard."); + } + return true; + } + + // Strips the extension from a file name, falling back to DEFAULT_WORKFLOW_NAME when empty. + private deriveWorkflowName(fileName: string): string { + const extensionIndex = fileName.lastIndexOf("."); + const baseName = extensionIndex === -1 ? fileName : fileName.substring(0, extensionIndex); + return baseName.trim() === "" ? DEFAULT_WORKFLOW_NAME : baseName; + } + /** * duplicate the current workflow. A new record will appear in frontend * workflow list and backend database. @@ -444,13 +572,8 @@ export class UserWorkflowComponent implements AfterViewInit { throw new Error("Incorrect format: file is not a string"); } const workflowContent = JSON.parse(result) as WorkflowContent; - const fileExtensionIndex = name.lastIndexOf("."); - let workflowName = fileExtensionIndex === -1 ? name : name.substring(0, fileExtensionIndex); - if (workflowName.trim() === "") { - workflowName = DEFAULT_WORKFLOW_NAME; - } this.workflowPersistService - .createWorkflow(workflowContent, workflowName) + .createWorkflow(workflowContent, this.deriveWorkflowName(name)) .pipe(untilDestroyed(this)) .subscribe({ next: uploadedWorkflow => { diff --git a/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.html b/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.html index 4e6a3d6037..814b92069c 100644 --- a/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.html +++ b/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.html @@ -17,122 +17,139 @@ under the License. --> -<form - class="import-modal-form" - [formGroup]="importForm" - nz-form> - <div class="import-modal-diagram"> - <img - ngSrc="assets/notebook_migration_tool/tool_popup_diagram.png" - alt="Notebook to Workflow" - width="1132" - height="290" /> - </div> +<div class="import-modal-diagram"> + <img + ngSrc="assets/notebook_migration_tool/tool_popup_diagram.png" + alt="Notebook to Workflow" + width="1132" + height="290" /> +</div> - <nz-alert - class="import-modal-warning" - nzType="warning" - nzShowIcon - nzMessage="Generating overwrites your current workflow; the previous version is kept in version history."></nz-alert> +<div class="import-modal-content"> + <form + class="import-modal-form" + [formGroup]="importForm" + [attr.inert]="isSubmitting ? '' : null" + nz-form> + <nz-form-item> + <p class="import-modal-text"> + This tool converts a Python Jupyter Notebook into a Texera workflow using LLM capabilities. After you submit a + notebook, the LLM service generates a corresponding Texera workflow. The conversion time depends on the + notebook's complexity and can take 1-5 minutes. Once generation finishes, you are taken to the new workflow, + which opens with: + </p> + <ol class="import-modal-list"> + <li> + The generated workflow ready to use (Note: you will still need to upload the dataset and connect it to the + workflow). + </li> + <li>A floating Jupyter window containing the uploaded notebook for reference.</li> + </ol> + <p class="import-modal-text"> + Generation runs here after you submit. Please keep this window open while you wait. + </p> + </nz-form-item> - <nz-form-item> - <p class="import-modal-text"> - This tool converts a Python Jupyter Notebook into a Texera workflow using LLM capabilities. After you submit a - notebook, the LLM service will generate a corresponding Texera workflow. The conversion time depends on the - notebook’s complexity and can take 1–5 minutes. Once the process is complete, the workflow workspace will reload - with: - </p> - <ol class="import-modal-list"> - <li> - The generated workflow ready to use (Note: you will still need to upload the dataset and connect it to the - workflow). - </li> - <li>A floating Jupyter window containing the uploaded notebook for reference.</li> - </ol> - <p class="import-modal-text"> - Feel free to navigate away from this tab while you wait for the workflow to generate. Please do not close the - window. - </p> - </nz-form-item> + <nz-form-item> + <nz-form-label [nzNoColon]="true"> + <span class="import-modal-label"> Upload Python Jupyter Notebook </span> + </nz-form-label> + <nz-form-control> + <div class="import-modal-upload-row"> + <nz-upload + nzAccept=".ipynb" + [nzBeforeUpload]="beforeUpload" + [nzShowUploadList]="false"> + <button + nz-button + type="button" + title="Upload notebook" + aria-label="Upload notebook"> + <i + nz-icon + nzType="upload"></i> + </button> + </nz-upload> - <nz-form-item> - <nz-form-label [nzNoColon]="true"> - <span class="import-modal-label"> Upload Python Jupyter Notebook </span> - </nz-form-label> - <nz-form-control> - <div class="import-modal-upload-row"> - <nz-upload - nzAccept=".ipynb" - [nzBeforeUpload]="beforeUpload" - [nzShowUploadList]="false"> - <button - nz-button - type="button" - title="Upload notebook" - aria-label="Upload notebook"> - <i - nz-icon - nzType="upload"></i> - </button> - </nz-upload> + <span + *ngIf="importForm.get('file')?.value?.name as fileName" + class="import-modal-selected-file" + [title]="fileName"> + Selected file: {{ fileName }} + </span> + </div> + </nz-form-control> + </nz-form-item> - <span *ngIf="importForm.get('file')?.value?.name"> - Selected file: {{ importForm.get('file')?.value?.name }} - </span> - </div> - </nz-form-control> - </nz-form-item> + <nz-form-item> + <nz-form-label [nzNoColon]="true"> + <span class="import-modal-label"> Select Model Type </span> + </nz-form-label> - <nz-form-item> - <nz-form-label [nzNoColon]="true"> - <span class="import-modal-label"> Select Model Type </span> - </nz-form-label> + <nz-form-control> + <ng-container *ngIf="models$ | async as models; else loadingTpl"> + <nz-select + *ngIf="models.length > 0; else noModelsTpl" + class="import-modal-select" + formControlName="model" + nzPlaceHolder="Select a model"> + <nz-option + *ngFor="let model of models" + [nzValue]="model.name" + [nzLabel]="model.name"></nz-option> + </nz-select> + <ng-template #noModelsTpl> + <nz-select + class="import-modal-select" + nzPlaceHolder="No models available" + [nzDisabled]="true"></nz-select> + </ng-template> + </ng-container> - <nz-form-control> - <ng-container *ngIf="models$ | async as models; else loadingTpl"> - <nz-select - *ngIf="models.length > 0; else noModelsTpl" - class="import-modal-select" - formControlName="model" - nzPlaceHolder="Select a model"> - <nz-option - *ngFor="let model of models" - [nzValue]="model.name" - [nzLabel]="model.name"></nz-option> - </nz-select> - <ng-template #noModelsTpl> + <ng-template #loadingTpl> <nz-select class="import-modal-select" - nzPlaceHolder="No models available" + nzPlaceHolder="Loading models..." + [nzLoading]="true" [nzDisabled]="true"></nz-select> </ng-template> - </ng-container> + </nz-form-control> + </nz-form-item> + </form> - <ng-template #loadingTpl> - <nz-select - class="import-modal-select" - nzPlaceHolder="Loading models..." - [nzLoading]="true" - [nzDisabled]="true"></nz-select> - </ng-template> - </nz-form-control> - </nz-form-item> -</form> + <div + class="import-modal-footer" + [attr.inert]="isSubmitting ? '' : null"> + <button + nz-button + type="button" + [disabled]="isSubmitting" + (click)="onCancel()"> + Cancel + </button> + <button + nz-button + type="button" + nzType="primary" + [disabled]="!importForm.valid || isSubmitting" + (click)="onSubmit()"> + Submit + </button> + </div> -<div class="import-modal-footer"> - <button - nz-button - type="button" - [disabled]="isSubmitting" - (click)="onCancel()"> - Cancel - </button> - <button - nz-button - type="button" - nzType="primary" - [disabled]="!importForm.valid || isSubmitting" - (click)="onSubmit()"> - Submit - </button> + <div + *ngIf="isSubmitting" + class="import-modal-loading" + role="status" + aria-live="polite"> + <nz-spin + nzSimple + [nzSize]="'large'"></nz-spin> + <p class="import-modal-loading-title">Generating your workflow</p> + <p class="import-modal-loading-elapsed">Elapsed time: {{ formattedElapsedTime }}</p> + <p class="import-modal-loading-text">This can take 1-5 minutes depending on the notebook's complexity.</p> + <p class="import-modal-loading-text"> + Please keep this window open. You will be taken to the new workflow when it is ready. + </p> + </div> </div> diff --git a/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.scss b/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.scss index 2f89848982..ed2b53d725 100644 --- a/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.scss +++ b/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.scss @@ -57,22 +57,67 @@ } &-upload-row { - display: inline-flex; + display: flex; align-items: center; gap: 8px; button { white-space: normal; + flex: none; } } + &-selected-file { + flex: 1; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + &-select { width: 50%; } - &-warning { - display: block; - margin-bottom: 12px; + // Positioning context for the loading overlay, so covering the form does not resize the modal. + &-content { + position: relative; + } + + &-loading { + position: absolute; + inset: 0; + z-index: 1; + // Solid background so the form underneath is fully hidden. + background-color: #fff; + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + gap: 14px; + padding: 16px 24px 28px; + text-align: center; + } + + &-loading-title { + margin: 8px 0 0; + font-size: 20px; + font-weight: 700; + } + + &-loading-elapsed { + margin: 0; + font-size: 18px; + font-weight: 600; + font-variant-numeric: tabular-nums; + } + + &-loading-text { + margin: 0; + max-width: 480px; + font-size: 16px; + line-height: 1.5; + color: rgba(0, 0, 0, 0.65); } &-footer { diff --git a/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.spec.ts b/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.spec.ts index eb6d199bd1..2e46f2ee65 100644 --- a/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.spec.ts +++ b/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.spec.ts @@ -32,7 +32,7 @@ describe("NotebookImportModalComponent", () => { let fixture: ComponentFixture<NotebookImportModalComponent>; let component: NotebookImportModalComponent; let notebookMigrationService: NotebookMigrationService; - let modalRef: { close: ReturnType<typeof vi.fn> }; + let modalRef: { close: ReturnType<typeof vi.fn>; updateConfig: ReturnType<typeof vi.fn> }; // The opener-supplied gate; tests set its resolved value to drive close vs stay-open. let requestImport: ReturnType<typeof vi.fn>; @@ -56,19 +56,47 @@ describe("NotebookImportModalComponent", () => { } beforeEach(() => { - modalRef = { close: vi.fn() }; + modalRef = { close: vi.fn(), updateConfig: vi.fn() }; requestImport = vi.fn().mockResolvedValue(true); }); - it("renders the warning, diagram, and a usable model select once models load", async () => { + it("renders the diagram and a usable model select once models load", async () => { await createWith(of([{ name: "gpt-4" }])); const root = fixture.nativeElement as HTMLElement; - expect(root.querySelector(".import-modal-warning")).not.toBeNull(); expect(root.querySelector("img[alt='Notebook to Workflow']")).not.toBeNull(); expect(root.querySelector("nz-select")).not.toBeNull(); expect(root.textContent).toContain("Select a model"); }); + it("shows the loading spinner only while a submission is in flight", async () => { + await createWith(of([{ name: "gpt-4" }])); + const spinning = () => (fixture.nativeElement as HTMLElement).querySelector(".ant-spin-spinning") !== null; + expect(spinning()).toBe(false); + + component.isSubmitting = true; + fixture.detectChanges(); + expect(spinning()).toBe(true); + }); + + it("makes the form and footer inert and announces the overlay while submitting", async () => { + await createWith(of([{ name: "gpt-4" }])); + const root = fixture.nativeElement as HTMLElement; + const form = () => root.querySelector(".import-modal-form"); + const footer = () => root.querySelector(".import-modal-footer"); + + expect(form()?.hasAttribute("inert")).toBe(false); + expect(footer()?.hasAttribute("inert")).toBe(false); + + component.isSubmitting = true; + fixture.detectChanges(); + + // While generating, the covered form and footer are pulled out of the focus/a11y tree, + // and the overlay is a live region so its status is announced. + expect(form()?.hasAttribute("inert")).toBe(true); + expect(footer()?.hasAttribute("inert")).toBe(true); + expect(root.querySelector(".import-modal-loading")?.getAttribute("role")).toBe("status"); + }); + it("shows the disabled 'no models available' select when the list is empty", async () => { await createWith(of([])); expect((fixture.nativeElement as HTMLElement).textContent).toContain("No models available"); @@ -125,7 +153,7 @@ describe("NotebookImportModalComponent", () => { }); it("onSubmit keeps the modal open when the opener declines", async () => { - // e.g. the user backed out of the overwrite confirmation. + // e.g. generation failed and the user can retry. requestImport.mockResolvedValue(false); await createWith(of([{ name: "gpt-4" }])); component.importForm.setValue({ file: { name: "x.ipynb" } as NzUploadFile, model: "gpt-4" }); @@ -136,6 +164,80 @@ describe("NotebookImportModalComponent", () => { expect(modalRef.close).not.toHaveBeenCalled(); }); + it("locks the modal shut while generating and restores the close controls on failure", async () => { + let resolveRequest!: (proceed: boolean) => void; + requestImport.mockReturnValue(new Promise<boolean>(resolve => (resolveRequest = resolve))); + await createWith(of([{ name: "gpt-4" }])); + component.importForm.setValue({ file: { name: "x.ipynb" } as NzUploadFile, model: "gpt-4" }); + + const submitting = component.onSubmit(); + // While generation is pending, the X, mask click, and ESC are disabled. + expect(modalRef.updateConfig).toHaveBeenCalledWith({ + nzClosable: false, + nzMaskClosable: false, + nzKeyboard: false, + }); + + resolveRequest(false); // generation failed + await submitting; + // The modal stayed open, so the close controls are restored. + expect(modalRef.updateConfig).toHaveBeenLastCalledWith({ + nzClosable: true, + nzMaskClosable: true, + nzKeyboard: true, + }); + expect(modalRef.close).not.toHaveBeenCalled(); + }); + + it("computes the elapsed time from the start timestamp as mm:ss", async () => { + await createWith(of([{ name: "gpt-4" }])); + // No generation started yet -> no start timestamp -> zero. + expect(component.formattedElapsedTime).toBe("0:00"); + (component as any).startTime = 1000; + vi.spyOn(Date, "now").mockReturnValue(1000 + 62_000); + expect(component.formattedElapsedTime).toBe("1:02"); + }); + + it("runs the stopwatch off wall-clock time while generating and stops the interval when done", async () => { + let resolveRequest!: (proceed: boolean) => void; + requestImport.mockReturnValue(new Promise<boolean>(resolve => (resolveRequest = resolve))); + await createWith(of([{ name: "gpt-4" }])); + component.importForm.setValue({ file: { name: "x.ipynb" } as NzUploadFile, model: "gpt-4" }); + + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date(0)); + const submitting = component.onSubmit(); + expect(component.formattedElapsedTime).toBe("0:00"); + + // Advancing the clock (even if the interval were throttled) yields the correct elapsed time. + vi.advanceTimersByTime(75_000); + expect(component.formattedElapsedTime).toBe("1:15"); + + resolveRequest(false); + await submitting; + expect((component as any).timerHandle).toBeNull(); + } finally { + vi.useRealTimers(); + } + }); + + it("has a visibilitychange handler that is safe to call (repaint is driven by the zone event)", async () => { + await createWith(of([{ name: "gpt-4" }])); + expect(() => component.onVisibilityChange()).not.toThrow(); + }); + + it("clears the stopwatch interval on destroy", async () => { + await createWith(of([{ name: "gpt-4" }])); + const clearSpy = vi.spyOn(globalThis, "clearInterval"); + (component as any).timerHandle = setInterval(() => {}, 1000); + + component.ngOnDestroy(); + + expect(clearSpy).toHaveBeenCalled(); + expect((component as any).timerHandle).toBeNull(); + }); + it("ignores a second submit while the first is still pending", async () => { // A pending requestImport models the opener still showing its overwrite confirmation. let resolveRequest!: (proceed: boolean) => void; diff --git a/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.ts b/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.ts index 70ee5745d1..7c9ad86cd6 100644 --- a/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.ts +++ b/frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.ts @@ -17,7 +17,7 @@ * under the License. */ -import { Component, inject } from "@angular/core"; +import { Component, HostListener, inject, OnDestroy } from "@angular/core"; import { FormBuilder, FormGroup, Validators, ReactiveFormsModule } from "@angular/forms"; import { NZ_MODAL_DATA, NzModalRef } from "ng-zorro-antd/modal"; import { NzUploadComponent, NzUploadFile } from "ng-zorro-antd/upload"; @@ -25,26 +25,20 @@ import { Observable } from "rxjs"; import { AsyncPipe, NgIf, NgFor, NgOptimizedImage } from "@angular/common"; import { NzFormModule } from "ng-zorro-antd/form"; import { NzSelectModule } from "ng-zorro-antd/select"; -import { NzAlertModule } from "ng-zorro-antd/alert"; +import { NzSpinComponent } from "ng-zorro-antd/spin"; import { NzButtonComponent } from "ng-zorro-antd/button"; import { NzIconDirective } from "ng-zorro-antd/icon"; import { NotebookMigrationService } from "../../service/notebook-migration/notebook-migration.service"; -// Passed in via nzData. The modal delegates "may I proceed?" to the opener so the -// opener can keep the overwrite-confirm and generation logic (and the workflow state it -// needs) without the modal knowing about them. Resolve true to close the modal (the -// import has started), false to keep it open with the user's selection intact. +// Passed in via nzData. requestImport resolves true to close the modal, false to keep it open +// with the user's selection intact (bad file or a retryable failure). export interface NotebookImportModalData { requestImport: (file: NzUploadFile, model: string) => Promise<boolean>; } /** - * The "AI Generate Workflow from Python Notebook" modal body. It owns the upload form and - * the three model-dropdown states (loading / has models / none). On Submit it delegates the - * decision to proceed to its opener via the requestImport callback (passed in through - * nzData); the opener runs the overwrite-confirm and generation pipeline and the modal - * closes itself only when that resolves true. Mirrors the component-as-nzContent pattern - * used by the other modals opened from the menu (ResultExportationComponent, ...). + * The "AI Generate Workflow from Python Notebook" modal body: the upload form and model dropdown. + * On Submit it hands the file and model to requestImport and shows a loading state until it resolves. */ @Component({ selector: "texera-notebook-import-modal", @@ -58,13 +52,13 @@ export interface NotebookImportModalData { ReactiveFormsModule, NzFormModule, NzSelectModule, - NzAlertModule, + NzSpinComponent, NzUploadComponent, NzButtonComponent, NzIconDirective, ], }) -export class NotebookImportModalComponent { +export class NotebookImportModalComponent implements OnDestroy { private readonly fb = inject(FormBuilder); private readonly modalRef = inject(NzModalRef); private readonly notebookMigrationService = inject(NotebookMigrationService); @@ -90,24 +84,59 @@ export class NotebookImportModalComponent { this.modalRef.close(); } - // Guards against a second submit while the opener callback (which may show an - // overwrite confirmation) is still pending, so a double-click cannot start two imports. + // True while generation runs: guards against a second submit and drives the loading overlay. public isSubmitting = false; + private startTime: number | null = null; + private timerHandle: ReturnType<typeof setInterval> | null = null; + + public ngOnDestroy(): void { + this.stopTimer(); + } + + public get formattedElapsedTime(): string { + const diffMs = this.startTime === null ? 0 : Date.now() - this.startTime; + const totalSeconds = Math.floor(diffMs / 1000); + const minutes = Math.floor(totalSeconds / 60); + const seconds = totalSeconds % 60; + return `${minutes}:${seconds.toString().padStart(2, "0")}`; + } + + // Empty body on purpose: the zone-patched event firing is itself what repaints the stopwatch, + // so it catches up when the user returns to a backgrounded tab. Same reason as the timer below. + @HostListener("document:visibilitychange") + public onVisibilityChange(): void {} + + private startTimer(): void { + this.stopTimer(); + this.startTime = Date.now(); + // Empty body: elapsed is computed from startTime; the zone-patched tick just triggers a repaint. + this.timerHandle = setInterval(() => {}, 1000); + } + + private stopTimer(): void { + if (this.timerHandle !== null) { + clearInterval(this.timerHandle); + this.timerHandle = null; + } + } public async onSubmit(): Promise<void> { if (this.isSubmitting || !this.importForm.valid) return; const file: NzUploadFile = this.importForm.get("file")?.value; const model: string = this.importForm.get("model")?.value; this.isSubmitting = true; + this.startTimer(); + this.modalRef.updateConfig({ nzClosable: false, nzMaskClosable: false, nzKeyboard: false }); try { - // Ask the opener whether to proceed; close only if it does, so cancelling the - // overwrite-confirm leaves this modal open with the selection preserved. + // Close only on success, so a failure leaves the modal open with the selection preserved. if (await this.data.requestImport(file, model)) { this.modalRef.close(); + return; } + this.modalRef.updateConfig({ nzClosable: true, nzMaskClosable: true, nzKeyboard: true }); } finally { - // Re-enable submit if the modal is still open (import declined or it threw). this.isSubmitting = false; + this.stopTimer(); } } } diff --git a/frontend/src/app/workspace/component/workspace.component.spec.ts b/frontend/src/app/workspace/component/workspace.component.spec.ts index f85294e42a..a17d2dbff2 100644 --- a/frontend/src/app/workspace/component/workspace.component.spec.ts +++ b/frontend/src/app/workspace/component/workspace.component.spec.ts @@ -106,6 +106,7 @@ describe("WorkspaceComponent", () => { disableWorkflowModification: vi.fn(), enableWorkflowModification: vi.fn(), reloadWorkflow: vi.fn(), + autoLayoutWorkflow: vi.fn(), setNewSharedModel: vi.fn(), setWorkflowMetadata: vi.fn(), clearWorkflow: vi.fn(), @@ -253,7 +254,7 @@ describe("WorkspaceComponent", () => { await createFixture(configureRoute({ id: "42" })); fixture.detectChanges(); expect(workflowActionService.setNewSharedModel).toHaveBeenCalledWith(42, { uid: 7 }); - expect(workflowActionService.reloadWorkflow).toHaveBeenCalledWith(stubWorkflow); + expect(workflowActionService.reloadWorkflow).toHaveBeenCalledWith(stubWorkflow, undefined); expect(undoRedoService.clearUndoStack).toHaveBeenCalled(); expect(undoRedoService.clearRedoStack).toHaveBeenCalled(); expect(component.isLoading).toBe(false); @@ -283,7 +284,28 @@ describe("WorkspaceComponent", () => { fixture.detectChanges(); expect(notificationService.error).toHaveBeenCalledWith(expect.stringContaining("broken")); // Workflow still flows through reload — the error is informational, not blocking. - expect(workflowActionService.reloadWorkflow).toHaveBeenCalledWith(brokenWorkflow); + expect(workflowActionService.reloadWorkflow).toHaveBeenCalledWith(brokenWorkflow, undefined); + }); + + it("with autolayout=1: renders synchronously and lays the workflow out once", async () => { + await createFixture(configureRoute({ id: "42" }, { autolayout: "1" })); + const registerSpy = vi.spyOn(component, "registerAutoPersistWorkflow"); + fixture.detectChanges(); + // asyncRendering=false so the operators exist in the graph before layout runs. + expect(workflowActionService.reloadWorkflow).toHaveBeenCalledWith(stubWorkflow, false); + expect(workflowActionService.autoLayoutWorkflow).toHaveBeenCalledTimes(1); + // Auto-persistence must be registered before the layout runs, otherwise the layout's + // position-change events fire into no subscriber and the tidied layout is never saved. + expect(registerSpy.mock.invocationCallOrder[0]).toBeLessThan( + workflowActionService.autoLayoutWorkflow.mock.invocationCallOrder[0] + ); + }); + + it("without autolayout: uses the default rendering and does not lay out", async () => { + await createFixture(configureRoute({ id: "42" })); + fixture.detectChanges(); + expect(workflowActionService.reloadWorkflow).toHaveBeenCalledWith(stubWorkflow, undefined); + expect(workflowActionService.autoLayoutWorkflow).not.toHaveBeenCalled(); }); it("when URL fragment matches an element in the graph, highlights it", async () => { diff --git a/frontend/src/app/workspace/component/workspace.component.ts b/frontend/src/app/workspace/component/workspace.component.ts index 2f95ccccfa..b451527498 100644 --- a/frontend/src/app/workspace/component/workspace.component.ts +++ b/frontend/src/app/workspace/component/workspace.component.ts @@ -259,9 +259,17 @@ export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { this.workflowActionService.setNewSharedModel(wid, this.userService.getCurrentUser()); // remember URL fragment const fragment = this.route.snapshot.fragment; - // load the fetched workflow - this.workflowActionService.reloadWorkflow(workflow); + // An AI-generated workflow arrives with autolayout=1. Render synchronously + // (asyncRendering = false) so the operators exist before the one-shot layout runs. + const shouldAutoLayout = this.route.snapshot.queryParams.autolayout === "1"; + this.workflowActionService.reloadWorkflow(workflow, shouldAutoLayout ? false : undefined); this.workflowActionService.enableWorkflowModification(); + // Register before autoLayoutWorkflow(): workflowChanged() streams are hot, so subscribing + // afterward would drop the layout's position events and the tidied layout would never save. + this.registerAutoPersistWorkflow(); + if (shouldAutoLayout) { + this.workflowActionService.autoLayoutWorkflow(); + } // set the URL fragment to previous value // because reloadWorkflow will highlight/unhighlight all elements // which will change the URL fragment @@ -284,7 +292,6 @@ export class WorkspaceComponent implements AfterViewInit, OnInit, OnDestroy { this.undoRedoService.clearUndoStack(); this.undoRedoService.clearRedoStack(); this.setLoadingState(false); - this.registerAutoPersistWorkflow(); this.triggerCenter(); }, () => { diff --git a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts index c4f8ffcfbb..6a14b98efe 100644 --- a/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts +++ b/frontend/src/app/workspace/service/jupyter-panel/jupyter-panel.service.ts @@ -25,7 +25,7 @@ import { HttpClient, HttpHeaders } from "@angular/common/http"; import { NotificationService } from "src/app/common/service/notification/notification.service"; import { distinctUntilChanged, switchMap } from "rxjs/operators"; import { AppSettings } from "../../../common/app-setting"; -import { NotebookMigrationService } from "../notebook-migration/notebook-migration.service"; +import { NotebookMigrationService, notebookMappingKey } from "../notebook-migration/notebook-migration.service"; import { GuiConfigService } from "../../../common/service/gui-config.service"; @Injectable({ @@ -137,7 +137,7 @@ export class JupyterPanelService { switchMap(async (response: any) => { // Only load mapping and workflow if they exist if (response.exists) { - this.notebookMigrationService.setMapping("mapping_wid_" + workflowID, response.mapping); + this.notebookMigrationService.setMapping(notebookMappingKey(workflowID), response.mapping); if ((await this.notebookMigrationService.sendNotebookToJupyter(response.notebook)) == 1) { return 1; @@ -166,7 +166,7 @@ export class JupyterPanelService { console.warn("Workflow ID is undefined. Cannot compute highlight mapping."); return; } - const mappingKey = "mapping_wid_" + wid; + const mappingKey = notebookMappingKey(wid); const mapping = this.notebookMigrationService.getMapping(mappingKey); if (mapping == undefined) { @@ -253,7 +253,7 @@ export class JupyterPanelService { this.jupyterNotebookPanelVisible.next(false); const wid = this.workflowActionService.getWorkflow().wid; if (wid != undefined) { - this.notebookMigrationService.deleteMapping("mapping_wid_" + wid); + this.notebookMigrationService.deleteMapping(notebookMappingKey(wid)); } } @@ -285,7 +285,7 @@ export class JupyterPanelService { public openJupyterNotebookPanel(): void { if (!this.enabled) return; const wid = this.workflowActionService.getWorkflow().wid; - const mappingKey = "mapping_wid_" + wid; + const mappingKey = notebookMappingKey(wid); // Check if there is corresponding mapping data if (wid === undefined || !this.notebookMigrationService.hasMapping(mappingKey)) { this.notificationService.warning("No Jupyter notebook associated with this workflow."); @@ -359,7 +359,7 @@ export class JupyterPanelService { return; } - const mappingKey = "mapping_wid_" + wid; + const mappingKey = notebookMappingKey(wid); const mappingEntry = this.notebookMigrationService.getMapping(mappingKey); if (!mappingEntry) { diff --git a/frontend/src/app/workspace/service/notebook-migration/migration-llm.spec.ts b/frontend/src/app/workspace/service/notebook-migration/migration-llm.spec.ts index 40714dd64a..da1961efa3 100644 --- a/frontend/src/app/workspace/service/notebook-migration/migration-llm.spec.ts +++ b/frontend/src/app/workspace/service/notebook-migration/migration-llm.spec.ts @@ -17,7 +17,12 @@ * under the License. */ -import { NotebookMigrationLLM, Notebook } from "./migration-llm"; +import { + NotebookMigrationLLM, + Notebook, + DEFAULT_LLM_REQUEST_TIMEOUT_MINUTES, + LlmRequestTimeoutError, +} from "./migration-llm"; import { GuiConfigService } from "../../../common/service/gui-config.service"; import { WorkflowUtilService } from "../workflow-graph/util/workflow-util.service"; import { AuthService } from "../../../common/service/user/auth.service"; @@ -37,6 +42,7 @@ describe("NotebookMigrationLLM", () => { const stubConfig = { env: { pythonNotebookMigrationEnabled: true, + pythonNotebookMigrationTimeoutMinutes: 10, defaultDataTransferBatchSize: 400, defaultExecutionMode: "PIPELINED", }, @@ -371,7 +377,8 @@ describe("NotebookMigrationLLM", () => { it("returns true and pings the model with a capped token budget on success", async () => { const ok = await makeLLM().verifyConnection(); expect(ok).toBe(true); - expect(callModelSpy).toHaveBeenCalledWith([{ role: "user", content: "ping" }], 10); + // The ping goes through the timeout wrapper, so it also carries an abort signal. + expect(callModelSpy).toHaveBeenCalledWith([{ role: "user", content: "ping" }], 10, expect.any(AbortSignal)); }); it("returns false and logs the error when the ping fails", async () => { @@ -385,6 +392,94 @@ describe("NotebookMigrationLLM", () => { }); }); + describe("callModel transport", () => { + it("forwards messages and the abort signal to generateText and returns its text", async () => { + // Exercise the real callModel body (the ai-SDK seam every other test stubs) with a minimal + // LanguageModelV2 fake, so no network call and no "ai" module mock is involved. + callModelSpy.mockRestore(); + const llm = makeLLM(); + (llm as any).model = { + specificationVersion: "v2", + provider: "mock", + modelId: "mock", + supportedUrls: {}, + doGenerate: async () => ({ + content: [{ type: "text", text: "pong" }], + finishReason: "stop", + usage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 }, + warnings: [], + }), + }; + + const result = await (llm as any).callModel( + [{ role: "user", content: "ping" }], + 10, + new AbortController().signal + ); + + expect(result.text).toBe("pong"); + }); + }); + + describe("request timeout", () => { + it("rejects a stalled model call once the timeout elapses so the caller can recover", async () => { + vi.useFakeTimers(); + try { + // A request that never settles: only the timeout can end it. + callModelSpy.mockReturnValue(new Promise<{ text: string }>(() => {})); + const pending = makeLLM().convertNotebookToWorkflow({ cells: [codeCell("AAA", "a = 1")] }); + // Reject with a typed error carrying the configured minutes, so callers can message precisely. + const captured = pending.catch(error => error); + // stubConfig sets pythonNotebookMigrationTimeoutMinutes to 10. + await vi.advanceTimersByTimeAsync(10 * 60 * 1000); + const error = await captured; + expect(error).toBeInstanceOf(LlmRequestTimeoutError); + expect(error.minutes).toBe(10); + } finally { + vi.useRealTimers(); + } + }); + + it("does not reject a call that resolves before the timeout", async () => { + vi.useFakeTimers(); + try { + mockResponses( + JSON.stringify({ code: { UDF1: "code1" }, edges: [], outputs: { UDF1: ["a"] } }), + JSON.stringify({ UDF1: ["AAA"] }) + ); + const result = await makeLLM().convertNotebookToWorkflow({ cells: [codeCell("AAA", "a = 1")] }); + expect(JSON.parse(result).workflowJSON.operators).toHaveLength(1); + } finally { + vi.useRealTimers(); + } + }); + + it("falls back to the default timeout when the configured value is non-positive", async () => { + vi.useFakeTimers(); + try { + const stubConfig = { + env: { + pythonNotebookMigrationEnabled: true, + pythonNotebookMigrationTimeoutMinutes: 0, + defaultDataTransferBatchSize: 400, + defaultExecutionMode: "PIPELINED", + }, + } as unknown as GuiConfigService; + const util = { getNewOperatorPredicate: vi.fn() } as unknown as WorkflowUtilService; + const llm = new NotebookMigrationLLM(stubConfig, util); + llm.initialize("gpt-5-mini", "test-token"); + + callModelSpy.mockReturnValue(new Promise<{ text: string }>(() => {})); + const pending = llm.convertNotebookToWorkflow({ cells: [codeCell("AAA", "a = 1")] }); + const assertion = expect(pending).rejects.toThrow(/timed out/); + await vi.advanceTimersByTimeAsync(DEFAULT_LLM_REQUEST_TIMEOUT_MINUTES * 60 * 1000); + await assertion; + } finally { + vi.useRealTimers(); + } + }); + }); + describe("initialization guards", () => { it("convertNotebookToWorkflow() rejects when the session is enabled but not initialized", async () => { const llm = makeUninitializedLLM(); diff --git a/frontend/src/app/workspace/service/notebook-migration/migration-llm.ts b/frontend/src/app/workspace/service/notebook-migration/migration-llm.ts index 6a1ba8b489..4bafa095e2 100644 --- a/frontend/src/app/workspace/service/notebook-migration/migration-llm.ts +++ b/frontend/src/app/workspace/service/notebook-migration/migration-llm.ts @@ -79,6 +79,19 @@ interface CombinedMapping { * Terminal UDFs (no outgoing edge) declare their outputs as `string` so the result panel * renders viewable values rather than opaque binary blobs. */ +export const DEFAULT_LLM_REQUEST_TIMEOUT_MINUTES = 10; + +// Thrown when a model request exceeds the configured timeout, so callers can tell a slow-but-timed-out +// request apart from a genuine transport error and message the user accordingly. +export class LlmRequestTimeoutError extends Error { + constructor(public readonly minutes: number) { + super(`LLM request timed out after ${minutes} minutes`); + this.name = "LlmRequestTimeoutError"; + // Keep instanceof correct regardless of the compile target. + Object.setPrototypeOf(this, LlmRequestTimeoutError.prototype); + } +} + @Injectable() export class NotebookMigrationLLM { private model: any; @@ -175,7 +188,7 @@ export class NotebookMigrationLLM { } try { - await this.callModel([{ role: "user", content: "ping" }], 10); + await this.callModelWithTimeout([{ role: "user", content: "ping" }], 10); return true; } catch (err) { @@ -184,13 +197,41 @@ export class NotebookMigrationLLM { } } - // Seam over the `ai` transport. Specs stub this by spying the method, instead of - // mocking the "ai" module — module mocks are unreliable in the Angular unit-test - // builder when "ai" is also loaded by a sibling spec (e.g. via - // NotebookMigrationService), which silently breaks the mock and hangs these - // tests on a real network call. - protected callModel(messages: ModelMessage[], maxOutputTokens?: number): Promise<{ text: string }> { - return generateText({ model: this.model, messages, maxOutputTokens }); + // Seam over the `ai` transport. Specs spy this method rather than mocking the "ai" module: + // a module mock leaks across specs that share the "ai" import and hangs on a real network call. + protected callModel( + messages: ModelMessage[], + maxOutputTokens?: number, + abortSignal?: AbortSignal + ): Promise<{ text: string }> { + return generateText({ model: this.model, messages, maxOutputTokens, abortSignal }); + } + + // Deployment-configurable bound (in minutes), falling back to the default when unset or non-positive. + private get timeoutMinutes(): number { + const configured = this.config.env.pythonNotebookMigrationTimeoutMinutes; + return configured > 0 ? configured : DEFAULT_LLM_REQUEST_TIMEOUT_MINUTES; + } + + // Wraps callModel with a hard timeout so a stalled request cannot hang forever. The abort + // cancels the underlying request when the transport honors it; the race guarantees rejection + // even if it does not, so the caller's error path always runs. + private callModelWithTimeout(messages: ModelMessage[], maxOutputTokens?: number): Promise<{ text: string }> { + const minutes = this.timeoutMinutes; + const controller = new AbortController(); + let timer: ReturnType<typeof setTimeout> | undefined; + const timeout = new Promise<never>((_resolve, reject) => { + timer = setTimeout( + () => { + controller.abort(); + reject(new LlmRequestTimeoutError(minutes)); + }, + minutes * 60 * 1000 + ); + }); + return Promise.race([this.callModel(messages, maxOutputTokens, controller.signal), timeout]).finally(() => + clearTimeout(timer) + ); } /** @@ -207,7 +248,7 @@ export class NotebookMigrationLLM { content: prompt, }); - const result = await this.callModel(this.messages); + const result = await this.callModelWithTimeout(this.messages); this.messages.push({ role: "assistant", diff --git a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts index 6570e47a00..2b47325fdb 100644 --- a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts +++ b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.spec.ts @@ -18,7 +18,7 @@ */ import { TestBed } from "@angular/core/testing"; -import { NotebookMigrationService } from "./notebook-migration.service"; +import { NotebookMigrationService, notebookMappingKey } from "./notebook-migration.service"; import { HttpClient } from "@angular/common/http"; import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; import { NotificationService } from "src/app/common/service/notification/notification.service"; @@ -220,15 +220,21 @@ describe("NotebookMigrationService", () => { }); // storeNotebookAndMapping - it("should call storeNotebookAndMapping API", () => { - service.storeNotebookAndMapping(1, 1, {}, {}).subscribe(); + it("should call storeNotebookAndMapping API with the default vid", () => { + service.storeNotebookAndMapping(1, {}, {}).subscribe(); const req = httpMock.expectOne(req => req.url.includes("/notebook-migration/store-notebook-and-mapping")); expect(req.request.method).toBe("POST"); + expect(req.request.body.wid).toBe(1); + expect(req.request.body.vid).toBe(1); req.flush({ success: true, message: "stored" }); }); + it("notebookMappingKey builds the in-memory cache key from the wid", () => { + expect(notebookMappingKey(42)).toBe("mapping_wid_42"); + }); + // deleteNotebookAndMapping it("should call deleteNotebookAndMapping API with the wid", () => { let result: any; @@ -332,7 +338,7 @@ describe("NotebookMigrationService", () => { }); it("storeNotebookAndMapping emits a disabled result without making an HTTP call", async () => { - const result = await firstValueFrom(service.storeNotebookAndMapping(1, 1, {}, {})); + const result = await firstValueFrom(service.storeNotebookAndMapping(1, {}, {})); expect(result.success).toBe(false); httpMock.expectNone(req => req.url.includes("/notebook-migration/store-notebook-and-mapping")); }); @@ -343,4 +349,50 @@ describe("NotebookMigrationService", () => { httpMock.expectNone(req => req.url.includes("/notebook-migration/delete-notebook-and-mapping")); }); }); + + // parseAndTagNotebook (reads + validates + uuid-tags an uploaded .ipynb) + describe("parseAndTagNotebook", () => { + it("parses a valid notebook and tags every cell with a uuid", async () => { + const notebook = { + cells: [ + { cell_type: "code", source: "print(1)", metadata: {} }, + // No metadata: the parser must create it before setting the uuid. + { cell_type: "markdown", source: "# title" }, + ], + }; + const file = new File([JSON.stringify(notebook)], "analysis.ipynb"); + + const parsed = await service.parseAndTagNotebook(file); + + expect(parsed.cells.length).toBe(2); + expect(parsed.cells[0].metadata?.uuid).toBeTruthy(); + expect(parsed.cells[1].metadata?.uuid).toBeTruthy(); + }); + + it("rejects when the notebook is not valid JSON", async () => { + const file = new File(["not json"], "x.ipynb"); + await expect(service.parseAndTagNotebook(file)).rejects.toThrow(); + }); + + it("rejects a notebook without a cells array", async () => { + const file = new File([JSON.stringify({ nbformat: 4 })], "x.ipynb"); + await expect(service.parseAndTagNotebook(file)).rejects.toThrow(/Invalid notebook structure/); + }); + + it("rejects when the file content is not a string", async () => { + vi.spyOn(FileReader.prototype, "readAsText").mockImplementation(function (this: any) { + // result is a getter-only property, so shadow it with an own non-string value. + Object.defineProperty(this, "result", { value: null, configurable: true }); + this.onload?.(new ProgressEvent("load")); + }); + await expect(service.parseAndTagNotebook(new File([""], "x.ipynb"))).rejects.toThrow(/not a valid string/); + }); + + it("rejects when the file cannot be read", async () => { + vi.spyOn(FileReader.prototype, "readAsText").mockImplementation(function (this: any) { + this.onerror?.(new ProgressEvent("error")); + }); + await expect(service.parseAndTagNotebook(new File([""], "x.ipynb"))).rejects.toThrow(/Failed to read/); + }); + }); }); diff --git a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts index 3d6600c2a2..5c636240f3 100644 --- a/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts +++ b/frontend/src/app/workspace/service/notebook-migration/notebook-migration.service.ts @@ -24,7 +24,9 @@ import { HttpClient, HttpHeaders } from "@angular/common/http"; import { NotificationService } from "src/app/common/service/notification/notification.service"; import { GuiConfigService } from "../../../common/service/gui-config.service"; import { WorkflowUtilService } from "../workflow-graph/util/workflow-util.service"; +import { WorkflowContent } from "../../../common/type/workflow"; import { catchError, firstValueFrom, map, Observable, of } from "rxjs"; +import { v4 as uuidv4 } from "uuid"; interface LiteLLMModel { id: string; @@ -38,7 +40,7 @@ interface LiteLLMModelsResponse { object: string; } -interface MappingContent { +export interface MappingContent { cell_to_operator: Record<string, string[]>; operator_to_cell: Record<string, string[]>; } @@ -54,6 +56,11 @@ interface DeleteNotebookResponse { message?: string; } +// Single source of truth for the mapping cache key, shared with JupyterPanelService so it can't drift. +export function notebookMappingKey(wid: number | undefined): string { + return "mapping_wid_" + wid; +} + @Injectable({ providedIn: "root", }) @@ -86,7 +93,10 @@ export class NotebookMigrationService { ); } - public async sendToAIGenerateWorkflow(notebookContent: Notebook, modelType: string) { + public async sendToAIGenerateWorkflow( + notebookContent: Notebook, + modelType: string + ): Promise<{ workflowContent: WorkflowContent; mappingContent: MappingContent }> { if (!this.enabled) throw new Error("Notebook migration feature is disabled"); const migrationLLM = this.createMigrationLLM(); // initialize() defaults to the user's Texera JWT via AuthService.getAccessToken(). @@ -195,9 +205,9 @@ export class NotebookMigrationService { public storeNotebookAndMapping( wid: number | undefined, - vid: number = 1, mappingContent: any, - notebookContent: any + notebookContent: any, + vid: number = 1 ): Observable<StoreNotebookResponse> { if (!this.enabled) { return of({ success: false, message: "Notebook migration feature is disabled" }); @@ -246,4 +256,35 @@ export class NotebookMigrationService { public deleteMapping(id: string): void { delete this.mapping[id]; } + + // Reads and parses an .ipynb file, then tags each cell with a uuid (the mapping keys off these). + // Rejects on a read error, invalid JSON, or a missing cells array. Uses FileReader rather than + // file.text() because jsdom (the test environment) does not implement Blob/File.text(). + public parseAndTagNotebook(file: File): Promise<Notebook> { + return new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onerror = () => reject(new Error("Failed to read the notebook file.")); + reader.onload = () => { + try { + if (typeof reader.result !== "string") { + throw new Error("File content is not a valid string."); + } + const notebook = JSON.parse(reader.result) as Notebook; + if (!notebook || !Array.isArray(notebook.cells)) { + throw new Error("Invalid notebook structure."); + } + for (const cell of notebook.cells) { + if (!cell.metadata) { + cell.metadata = {}; + } + cell.metadata.uuid = uuidv4(); + } + resolve(notebook); + } catch (error) { + reject(error); + } + }; + reader.readAsText(file); + }); + } }
