+
-
-
-
-
-
-
+
-
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 2f898489821..ed2b53d725d 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 eb6d199bd17..2e46f2ee655 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
;
let component: NotebookImportModalComponent;
let notebookMigrationService: NotebookMigrationService;
- let modalRef: { close: ReturnType };
+ let modalRef: { close: ReturnType; updateConfig: ReturnType };
// The opener-supplied gate; tests set its resolved value to drive close vs stay-open.
let requestImport: ReturnType;
@@ -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(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(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 70ee5745d1e..7c9ad86cd66 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;
}
/**
- * 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 | 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 {
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 f85294e42ad..a17d2dbff28 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 2f95ccccfae..b451527498b 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 c4f8ffcfbbc..6a14b98efed 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 40714dd64a5..c1ef22e6afc 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,7 @@
* under the License.
*/
-import { NotebookMigrationLLM, Notebook } from "./migration-llm";
+import { NotebookMigrationLLM, Notebook, LLM_REQUEST_TIMEOUT_MS } 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";
@@ -371,7 +371,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 +386,66 @@ 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")] });
+ // Surface the rejection instead of letting it float as unhandled while timers advance.
+ const assertion = expect(pending).rejects.toThrow(/timed out/);
+ await vi.advanceTimersByTimeAsync(LLM_REQUEST_TIMEOUT_MS);
+ await assertion;
+ } 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();
+ }
+ });
+ });
+
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 6a1ba8b4894..ebfc82907f5 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,8 @@ 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 LLM_REQUEST_TIMEOUT_MS = 10 * 60 * 1000;
+
@Injectable()
export class NotebookMigrationLLM {
private model: any;
@@ -175,7 +177,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 +186,31 @@ 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 });
+ }
+
+ // 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 controller = new AbortController();
+ let timer: ReturnType | undefined;
+ const timeout = new Promise((_resolve, reject) => {
+ timer = setTimeout(() => {
+ controller.abort();
+ reject(new Error(`LLM request timed out after ${LLM_REQUEST_TIMEOUT_MS} ms`));
+ }, LLM_REQUEST_TIMEOUT_MS);
+ });
+ return Promise.race([this.callModel(messages, maxOutputTokens, controller.signal), timeout]).finally(() =>
+ clearTimeout(timer)
+ );
}
/**
@@ -207,7 +227,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 6570e47a00d..2b47325fdbc 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 3d6600c2a2e..5c636240f36 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;
operator_to_cell: Record;
}
@@ -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 {
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 {
+ 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);
+ });
+ }
}