Update defImages function to support multiple file types and handle Blob conversion #705
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
The changes in the pull request look good to me. Here is a brief summary of the changes:
The changes seem to be well-implemented and no functional issues are visible. LGTM 🚀
|
| ```js | ||
| const page = await host.browse("https://bing.com") | ||
| const screenshot = await page.screenshot() // returns a node.js Buffer | ||
| defImages(screenshot) |
There was a problem hiding this comment.
The function 'host.browse' is used but not defined in the documentation. There should be a definition or reference for it.
generated by pr-docs-review-commit
missing_function_definition
|
|
||
| ```js | ||
| const page = await host.browse("https://bing.com") | ||
| const screenshot = await page.screenshot() // returns a node.js Buffer |
There was a problem hiding this comment.
The function 'page.screenshot' is used but not defined in the documentation. There should be a definition or reference for it.
generated by pr-docs-review-commit
missing_function_definition
| @@ -1537,7 +1539,7 @@ interface ChatGenerationContext extends ChatTurnGenerationContext { | |||
| options?: DefSchemaOptions | |||
| ): string | |||
| defImages( | |||
| files: StringLike | Buffer | Blob, | |||
| files: ElementOrArray<string | WorkspaceFile | Buffer | Blob>, | |||
There was a problem hiding this comment.
The change in the type of the 'files' parameter from 'StringLike | Buffer | Blob' to 'ElementOrArray<string | WorkspaceFile | Buffer | Blob>' in the 'defImages' function is a breaking change. This can cause issues in the existing code where this function is used.
generated by pr-review-commit
breaking_change
| } | ||
| })() | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The function defImages is missing a return type. It's a good practice to always specify a return type for functions to improve code readability and prevent potential bugs.
generated by pr-review-commit
missing_return_type
| import { Steps } from "@astrojs/starlight/components" | ||
| import source from "../../../../../packages/sample/genaisrc/azure-blobs.genai.mts?raw" | ||
|
|
||
| It is possible to use the Azure Node.JS SDK to download images from Azure Blog Storage |
There was a problem hiding this comment.
There is a typo in "Blog" which should be "Blob".
generated by pr-docs-review-commit
typo
| import source from "../../../../../packages/sample/genaisrc/azure-blobs.genai.mts?raw" | ||
|
|
||
| It is possible to use the Azure Node.JS SDK to download images from Azure Blog Storage | ||
| and use them in the prompt. The `defImages` function support the node.js [Buffer] type. |
There was a problem hiding this comment.
The function name defImages should be defImage to match the function used in the other documentation file.
generated by pr-docs-review-commit
typo
|
|
||
| Open a connection to the Azure Blob Storage and get a client to the container. | ||
| We deconstruct the `account` and `container` from the `env.vars` object | ||
| so that they can be set through the [cli](/genaiscript/reference/cli). |
There was a problem hiding this comment.
The link provided for [cli] is incorrect or broken, it should be a valid URL or path to the CLI reference.
generated by pr-docs-review-commit
broken_link
| If you do not have a specific blob in mind, you can iterate through the blobs, | ||
| and download them into a buffer (`buf`). | ||
|
|
||
| ```ts "image" |
There was a problem hiding this comment.
The code fence "image" is incorrect or unnecessary, it should be removed.
generated by pr-docs-review-commit
incorrect_code_fence
| However since images can be "heavy", you will most likely have to use | ||
| [inline prompts](/genaiscript/reference/prompts/inline-prompts) to split into smaller queries. (Note the use of `_.`) | ||
|
|
||
| ```ts 'await runPrompt(_ =>' '_.' |
There was a problem hiding this comment.
There is a syntax error in the code snippet, the single quotes and backticks are not correctly paired.
generated by pr-docs-review-commit
syntax_error
| However since images can be "heavy", you will most likely have to use | ||
| [inline prompts](/genaiscript/reference/prompts/inline-prompts) to split into smaller queries. (Note the use of `_.`) | ||
|
|
||
| ```ts 'await runPrompt(_ =>' '_.' |
There was a problem hiding this comment.
The code fence 'await runPrompt(_ =>' ' _.' is incorrect or unnecessary, it should be removed.
generated by pr-docs-review-commit
incorrect_code_fence
| defImages(image, { detail: "low" }) | ||
| ``` | ||
|
|
||
| However since images can be "heavy", you will most likely have to use |
There was a problem hiding this comment.
The sentence "However since images can be 'heavy', you will most likely have to use" could be clarified to specify that "heavy" refers to large file sizes which may require more processing time or resources.
generated by pr-docs-review-commit
clarity
|
|
||
| ## Full source | ||
|
|
||
| <Code code={source} wrap={true} lang="js" title="azure-blobs.genai.mts" /> |
There was a problem hiding this comment.
The variable source is used but not defined in the document, which might confuse readers. It should be clearly defined or explained.
generated by pr-docs-review-commit
missing_reference
|
|
||
| ## Buffer, Blob | ||
|
|
||
| The `defImage` function also supports [Buffer](https://nodejs.org/api/buffer.html) |
There was a problem hiding this comment.
The function name defImage should be defImages to match the function used in the other documentation file.
generated by pr-docs-review-commit
typo
…documentation details across various guides.
| If you do not have a specific blob in mind, you can iterate through the blobs, | ||
| and download them into a buffer (`buf`). | ||
|
|
||
| ```ts "image" |
There was a problem hiding this comment.
The code comment "image" seems to be misplaced or incorrect.
generated by pr-docs-review-commit
incorrect_code_comment
| and [Blob](https://developer.mozilla.org/en-US/docs/Web/API/Blob). | ||
|
|
||
|
|
||
| This example takes a screenshot of bing.com and adds it to the images. |
There was a problem hiding this comment.
Typo in the word "bing.com", should be "Bing.com" as it is a proper noun.
generated by pr-docs-review-commit
typo
| } | ||
| })() | ||
| ) | ||
| ) |
There was a problem hiding this comment.
You are creating an async function but not awaiting it, which can lead to unexpected behavior. Consider adding an await keyword or returning the promise.
generated by pr-review-commit
async_without_await
This pull request updates the
defImagesfunction to support multiple file types and handle Blob conversion. Thefilesparameter now accepts an array of strings, WorkspaceFiles, Buffers, or Blobs. This allows for more flexibility when passing files to the function.The
defImagesfunction inrunpromptcontext.tshas been updated to handle Blob instances 👏. Now, when a Blob instance is provided as an input file, it's converted into an 'image' node and appended to the parent node.defImagesfunction parameters across various files have been made more flexible by introducing a new type,ElementOrArray. This meansdefImagescan now handle both individual elements and arrays of elements of the types string, WorkspaceFile, Buffer, or Blob 🔄.These changes are reflected in the relevant type declaration files i.e.
prompt_template.d.tsandprompt_type.d.ts. More specifically, thedefImagesfunction signatures in these files replaceStringLike | Buffer | BlobwithElementOrArray<string | WorkspaceFile | Buffer | Blob>.This shows clarity in the public API regarding what types of inputs
defImagescan accept, enhancing user understanding and control 🎮. The change increases the flexibility of data inputs making the system more robust and user-friendly.Overall, fantastic job on improving flexibility, user compatibility, and making operations more efficient. Keep up the good work! 🚀🙌