Skip to content
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

Importtemplate #684

Merged
merged 10 commits into from
Sep 3, 2024
Merged

Importtemplate #684

merged 10 commits into from
Sep 3, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Sep 3, 2024

  • 🆕 Added ability to import template prompts using importTemplate function in the genai context (found in packages/core/src/runpromptcontext.ts). This function dynamically loads and interpolates markdown files (found in import-template.genai.mts, basic.prompty).

  • 🧩 Introduced a new importTemplate feature in the ChatTurnGenerationContext interface (found in packages/core/src/types/prompt_template.d.ts).

  • 📄Created new markdown files basic.prompty (under packages/sample/src/) and import-template.genai.mts (under packages/sample/genaisrc/) as part of the testing for the new function.

  • 🌟Updated function createPromptContext in packages/core/src/promptcontext.ts and renderPromptNode in packages/core/src/promptdom.ts to accommodate the new importTemplate feature.

  • 📚 In packages/core/src/promptdom.ts, updated PromptNodeVisitor and PromptNode to manage import templates and their resolutions.

  • 🔄 Updated various package versions in docs/package.json, package.json, packages/cli/package.json and packages/core/package.json.

  • 📁 Added a new type interface for the import template in packages/core/src/types/prompt_type.d.ts.

  • 💡 Added test cases for the new interpolation feature in packages/core/src/mustache.test.ts.

  • 📦 Included mustache.js library for template manipulation in packages/core/package.json and created new usage in packages/core/src/mustache.ts.

  • 🤏 Minor method renaming in packages/core/src/promptdom.ts and packages/core/src/promptcontext.ts (from createFileMergeNode to createFileMerge).

Overall, these changes enhance the ability to dynamically handle and import templates, and improves the flexibility and usability of the API.

generated by pr-describe

Copy link

github-actions bot commented Sep 3, 2024

The changes seem to introduce a new feature to the underlying software. Here's a summary of the changes:

  • A new function, interpolateVariables, has been added which applies mustache to the content of a markdown file. This function is used to interpolate variables in the markdown content based on the provided data.

  • The test file for interpolateVariables function has also been added to validate the expected behavior of the function.

  • The createFileMerge function has been refactored from createFileMergeNode and its usages have been updated in the codebase accordingly.

  • A new type, PromptImportTemplate, has been introduced to handle imported templates.

  • Modifications have been made to handle importTemplate type while visiting or resolving prompt nodes and during rendering.

  • The importTemplate function has been added as a method in the ChatTurnGenerationContext interface. This function is used to import template prompt files and expand arguments in it.

  • The function importTemplate has been introduced in the prompt_type.d.ts file to import and expand the template files.

However, there are a few areas of concern:

  • The interpolateVariables function might include the risk of misinterpreting user data if it contains mustache syntax. It could be improved by validating the data before interpolation.

  • The PromptImportTemplate interface includes args which can be string, number, boolean, or a function returning one of these types. It could be a potential problem if the function passed to the args property contains side effects.

  • Although the error handling has been implemented while resolving or rendering nodes of type importTemplate in the resolvePromptNode and renderPromptNode functions, we should ensure that these errors are properly propagated and handled in the higher execution context.

Here's a possible improvement:

@@ -0,0 +1,29 @@
+import { splitMarkdown } from "./frontmatter"
+import Mustache from "mustache"
+
+/**
+ * Applies mustache to the content of a markdown file.
+ * @param md
+ * @param data
+ * @returns
+ */
+export async function interpolateVariables(
+    md: string,
+    data: Record<string, any>
+): Promise<string> {
+    if (!md || !data) return md
+
+    // Validate the data to avoid misinterpretation of user data
+    if (!validateData(data)) {
+      throw new Error('Invalid data provided')
+    }
+
+    // remove frontmatter
+    let { content } = splitMarkdown(md)
+
+    content = Mustache.render(content, data ?? {})
+
+    return content
+}

generated by pr-review

```

```js title="tool.genai.mjs"
importTemplate("cot.md");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect code usage; importTemplate is not a valid function or method in the provided context.

generated by pr-docs-review-commit incorrect_code_usage

```

```js title="tool.genai.mjs"
importTemplate("time.md", { time: "12:00" });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect code usage; importTemplate is not a valid function or method in the provided context.

generated by pr-docs-review-commit incorrect_code_usage

Mustache supports arguments as functions. This allows you to pass dynamic values to the template.

```js title="tool.genai.mjs"
importTemplate("time.md", { time: () => Date.now() });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect code usage; importTemplate is not a valid function or method in the provided context.

generated by pr-docs-review-commit incorrect_code_usage

You can specify an array of files or glob patterns.

```js
importTemplate("*.prompt")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect code usage; importTemplate is not a valid function or method in the provided context.

generated by pr-docs-review-commit incorrect_code_usage

```

```js title="tool.genai.mjs"
importTemplate("basic.prompty", { question: "what is the capital of France?" });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect code usage; importTemplate is not a valid function or method in the provided context.

generated by pr-docs-review-commit incorrect_code_usage

---
system:
You are an AI assistant who helps people find information.
As the assistant, you answer questions briefly, succinctly.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sentence "As the assistant, you answer questions briefly, succinctly." could be clearer if rephrased to "As the assistant, you should answer questions briefly and succinctly."

generated by pr-docs-review-commit content_clarity

importTemplate(
files: string | string[],
arguments?: Record<string | number | boolean | (() => string | number | boolean)>,
options?: ImportTemplateOptions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of arguments in the importTemplate function is incorrect. It should be Record<string, string | number | boolean | (() => string | number | boolean)>.

generated by pr-review-commit incorrect_type

arguments?: Record<
string | number | boolean | (() => string | number | boolean)
>,
options?: ImportTemplateOptions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ImportTemplateOptions type is used but not defined. This could lead to confusion and potential errors in the future.

generated by pr-review-commit missing_type_definition

@pelikhan pelikhan merged commit c0e3673 into main Sep 3, 2024
11 checks passed
@pelikhan pelikhan deleted the importtemplate branch September 3, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant