Copilot commented on code in PR #8910:
URL: https://github.com/apache/texera/pull/8910#discussion_r4364580152
##########
frontend/src/app/workspace/service/notebook-migration/migration-llm.ts:
##########
@@ -444,10 +481,12 @@ export class NotebookMigrationLLM {
throw new Error("LLM session not initialized");
}
+ const prompts = PROMPT_SETS.python;
const { workflowJSON, udfIdToOperatorId, workflowResponse, reportedRanges
} = await this.requestConversion(
- NotebookMigrationLLM.FOLDER_DOCUMENTATION,
+ PYTHON_FOLDER_DOCUMENTATION,
FOLDER_WORKFLOW_PROMPT,
FOLDER_MAPPING_PROMPT,
+ prompts.operatorType,
Review Comment:
With `MigrationPromptSet` introduced to centralize language-specific
variations, the folder conversion path still hardcodes Python documentation and
prompts outside the prompt set. This leaves an additional place to update when
adding R folder support. Consider adding `folderDocumentation`,
`folderWorkflowPrompt`, and `folderMappingPrompt` to `MigrationPromptSet` now
(even if only the Python entry is populated) and routing this call through
`prompts` for consistency.
##########
frontend/src/app/workspace/service/notebook-migration/migration-llm.ts:
##########
@@ -111,15 +111,66 @@ function toDerivedNotebook(cells: DerivedCell[]):
Notebook {
outputs: [],
execution_count: null,
})),
- metadata: {
- kernelspec: { display_name: "Python 3", language: "python", name:
"python3" },
- language_info: { name: "python" },
- },
+ metadata: structuredClone(metadata),
nbformat: 4,
nbformat_minor: 4,
};
}
+const PYTHON_NOTEBOOK_DOCUMENTATION: string[] = [
+ TEXERA_OVERVIEW,
+ TUPLE_DOCUMENTATION,
+ TABLE_DOCUMENTATION,
+ OPERATOR_DOCUMENTATION,
+ EXAMPLE_OF_GOOD_CONVERSION,
+ VISUALIZER_DOCUMENTATION,
+ UDF_INPUT_PORT_DOCUMENTATION,
+ EXAMPLE_OF_MULTIPLE_UDF_CONVERSION,
+];
+
+// The script prelude differs in exactly one entry. The notebook worked
example is written in
+// `# START CELL1` form, and a system-message example of that weight would
push the model to
+// answer in cell ids for an input that has no cells. The notebook array is
left untouched so
+// existing conversions see byte-identical context.
+const PYTHON_SCRIPT_DOCUMENTATION: string[] =
PYTHON_NOTEBOOK_DOCUMENTATION.map(doc =>
+ doc === EXAMPLE_OF_MULTIPLE_UDF_CONVERSION ?
EXAMPLE_OF_MULTIPLE_UDF_CONVERSION_SCRIPT : doc
+);
+
+// Differs from the script prelude in the same single entry: its worked
example shows banner
+// lines, an entry point calling into other files, and definitions inlined
rather than imported.
+const PYTHON_FOLDER_DOCUMENTATION: string[] =
PYTHON_SCRIPT_DOCUMENTATION.map(doc =>
+ doc === EXAMPLE_OF_MULTIPLE_UDF_CONVERSION_SCRIPT ?
EXAMPLE_OF_MULTIPLE_UDF_CONVERSION_FOLDER : doc
+);
+
+export type MigrationLanguage = "python";
+
+interface MigrationPromptSet {
+ notebookDocumentation: string[];
+ notebookWorkflowPrompt: string;
+ notebookMappingPrompt: string;
+ scriptDocumentation: string[];
+ scriptWorkflowPrompt: string;
+ scriptMappingPrompt: string;
+ operatorType: string;
Review Comment:
`operatorType` being typed as a plain `string` makes it easy to introduce a
typo that only fails at runtime (e.g., when calling `getNewOperatorPredicate`).
Consider tightening the type to an existing operator-type type/union (if
available in the codebase), or deriving it from
`OperatorPredicate[\"operatorType\"]` / a shared operator registry type so
invalid operator types are caught at compile time.
--
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]
