zyratlo commented on code in PR #8767:
URL: https://github.com/apache/texera/pull/8767#discussion_r4159781105
##########
frontend/src/app/workspace/component/notebook-import-modal/notebook-import-modal.component.ts:
##########
@@ -30,20 +30,34 @@ import { NzButtonComponent } from "ng-zorro-antd/button";
import { NzIconDirective } from "ng-zorro-antd/icon";
import { NzTabsComponent, NzTabComponent, NzTabDirective } from
"ng-zorro-antd/tabs";
import { NotebookMigrationService } from
"../../service/notebook-migration/notebook-migration.service";
+import { folderRootName, pickedFilePath } from
"../../service/notebook-migration/folder-assembly";
+import { NotificationService } from
"../../../common/service/notification/notification.service";
// 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).
+// with the user's selection intact (bad file or a retryable failure). A
folder is handed over as
+// the whole picked list, which is also how the opener tells a folder from a
single file.
export interface NotebookImportModalData {
- requestImport: (file: NzUploadFile, model: string) => Promise<boolean>;
+ requestImport: (selection: NzUploadFile | NzUploadFile[], model: string) =>
Promise<boolean>;
+}
+
+// Falls back to a count when the picker reported no path, so the row always
says something
Review Comment:
Fixed in 7fd196d4a. Moved the two lines back above describeFolderSelection;
the latch comment keeps its constant.
##########
frontend/src/app/workspace/service/notebook-migration/migration-llm.spec.ts:
##########
@@ -696,4 +697,218 @@ describe("NotebookMigrationLLM", () => {
expect(callModelSpy).not.toHaveBeenCalled();
});
});
+ describe("convertFolderToWorkflow", () => {
+ // Two files behind banner lines: a.py on document lines 1-3, b.py on
lines 4-5.
+ const folder = buildFolderDocument(
+ [
+ { path: "a.py", source: "import os\nx = compute()" },
+ { path: "b.py", source: "print(x)" },
+ ],
+ { rootName: "proj", otherPaths: ["requirements.txt"] }
+ );
+
+ const workflowResponse = JSON.stringify({
+ code: { UDF1: "# UDF1", UDF2: "# UDF2" },
+ edges: [["UDF1", "UDF2"]],
+ outputs: { UDF1: ["value"], UDF2: ["result"] },
+ });
+
+ // Same reason as the script suite: sendPrompt mutates the array it handed
to callModel, so
+ // index by the conversion's first call and read the whole exchange.
+ function contentsFor(role: string, firstCallIndex = 0): string[] {
+ return (callModelSpy.mock.calls[firstCallIndex][0] as { role: string;
content: string }[])
+ .filter(message => message.role === role)
+ .map(message => message.content);
+ }
+
+ it("sends one document whose line numbers run across both files", async ()
=> {
+ const llm = makeLLM();
+ mockResponses(workflowResponse, JSON.stringify({ UDF1: [[2, 3]] }));
+
+ await llm.convertFolderToWorkflow(folder);
+
+ const prompt = contentsFor("user").join("\n");
+ expect(prompt).toContain("1| # ===== FILE: a.py =====");
+ expect(prompt).toContain("2| import os");
+ // b.py restarts no numbering: its first line is 4, not 1.
+ expect(prompt).toContain("4| # ===== FILE: b.py =====");
+ expect(prompt).toContain("5| print(x)");
+ });
+
+ it("sends the layout ahead of the code, and never numbers it", async () =>
{
Review Comment:
Fixed in 15ffd1b66, two indexOf assertions. Confirmed they have teeth: with
preamble and numberScriptLines swapped the test fails, where before all 57
passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]