-
Notifications
You must be signed in to change notification settings - Fork 1
Rag run #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ved input data handling for tests - Replaced structuredClone and JSON methods with a new smartClone function that deep-clones plain objects and arrays while preserving class instances by reference. - quick versions of tasks as functions now pass input to run not the constructor which means no defaults and cloning
…ng additional input properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a fundamental architectural change to how tasks are instantiated and executed, with the title "Rag run" suggesting this may be related to Retrieval-Augmented Generation workflows that require passing non-serializable objects like repositories.
Key changes:
- Task constructors now receive empty type-asserted objects instead of actual input data
- The
run()method now accepts input as a parameter instead of using constructor-provided input - New
smartClonemethod replacesstructuredCloneto handle class instances and TypedArrays by reference - Removed undefined value checks in
setInputandaddInputfor additional properties
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/task-graph/src/task/Task.ts | Core implementation: adds smartClone method, modifies resetInputData to use it, removes undefined checks in input handling |
| packages/tasks/src/task/SplitTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/tasks/src/task/MergeTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/tasks/src/task/JsonTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/tasks/src/task/JavaScriptTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/tasks/src/task/FileLoaderTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/tasks/src/task/FileLoaderTask.server.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/tasks/src/task/FetchUrlTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/tasks/src/task/DelayTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/tasks/src/task/DebugLogTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/VectorSimilarityTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/VectorQuantizeTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/UnloadModelTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextTranslationTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextSummaryTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextRewriterTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextQuestionAnswerTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextNamedEntityRecognitionTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextLanguageDetectionTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextGenerationTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextFillMaskTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextEmbeddingTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/TextClassificationTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/PoseLandmarkerTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/ObjectDetectionTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/ImageToTextTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/ImageSegmentationTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/ImageEmbeddingTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/ImageClassificationTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/HandLandmarkerTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/GestureRecognizerTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/FaceLandmarkerTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/FaceDetectorTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/DownloadModelTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
| packages/ai/src/task/BackgroundRemovalTask.ts | Updates convenience function to pass empty object to constructor and input to run() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/task-graph/src/task/Task.ts
Outdated
| private smartClone(obj: any): any { | ||
| if (obj === null || obj === undefined) { | ||
| return obj; | ||
| } | ||
|
|
||
| // Preserve TypedArrays (Float32Array, Int8Array, etc.) by reference | ||
| // These are often large and cloning them is expensive | ||
| if (ArrayBuffer.isView(obj)) { | ||
| return obj; | ||
| } | ||
|
|
||
| // Preserve class instances (objects with non-Object/non-Array prototype) | ||
| // This includes repository instances, custom classes, etc. | ||
| if (typeof obj === "object" && !Array.isArray(obj)) { | ||
| const proto = Object.getPrototypeOf(obj); | ||
| if (proto !== Object.prototype && proto !== null) { | ||
| return obj; // Pass by reference | ||
| } | ||
| } | ||
|
|
||
| // Deep clone arrays, preserving class instances within | ||
| if (Array.isArray(obj)) { | ||
| return obj.map((item) => this.smartClone(item)); | ||
| } | ||
|
|
||
| // Deep clone plain objects | ||
| if (typeof obj === "object") { | ||
| const result: Record<string, any> = {}; | ||
| for (const key in obj) { | ||
| if (Object.prototype.hasOwnProperty.call(obj, key)) { | ||
| result[key] = this.smartClone(obj[key]); | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
| // Primitives (string, number, boolean, symbol, bigint) are returned as-is | ||
| return obj; |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The smartClone implementation has a potential issue with circular references in plain objects and arrays. While class instances are passed by reference (avoiding circular reference issues), if a plain object or array contains circular references, this method will cause infinite recursion and stack overflow. Consider adding circular reference detection or documenting this limitation clearly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback. detect circular references
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Initial plan * Add circular reference detection to smartClone method Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com> * Fix circular reference detection to handle shared references correctly Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com> * Refactor TaskEvents to import TaskStatus from TaskTypes and add unit tests for smartClone method - Updated TaskEvents to import TaskStatus from the correct module. - Added comprehensive unit tests for the smartClone method, including cases for circular reference detection and handling various data structures. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com> Co-authored-by: Steven Roussey <sroussey@gmail.com>
No description provided.