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

support for system prompts in runPrompt #622

Merged
merged 7 commits into from
Aug 16, 2024
Merged

support for system prompts in runPrompt #622

merged 7 commits into from
Aug 16, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Aug 16, 2024

  • 🦚 Improved system prompt identification in resolveSystems method (in systems.ts file).
    • Refined with word boundary check to prevent partial word matches.
    • For example, earlier 'diff' could have matched 'different', but with this change it will not.
  • 🧬 Adjusted handling of system and model options in the context creator (createPromptContext in promptcontext.ts).
    • Now allows a set of system prompts to be executed before the main prompt.
    • System prompts become part of the conversation with their outputs used to supplement the main prompt's output.
  • 🔍 Enhanced detection of presence of schema in script source in resolveSystems method
    • Additional check for separated 'defschema' word in the source.
    • Consequential addition of 'system.schema' and 'system.files_schema' to systems if schema usage detected.
  • 🔧 Modified expandTemplate function in expander.ts to call the expander for each system in the systems array.
    • Each system's function adds its output (if any) to the overall RunPromptResult for the prompt.
  • 🛠 Added the system field to the PromptGeneratorOptions interface in prompt_template.d.ts file.
    • This field is meant to allow system prompts to be specified when calling runPrompt.
  • 🏗 Refining error handling in expandTemplate and createPromptContext by replacing some abrupt returns with throwing of descriptive error messages.
  • 🖼 If a system prompt produces an image, it throws a NotSupportedError.
  • 📄 runprompt.genai.js in the package/sample/genaisrc/style/ directory has been updated to demonstrate a usage of the new system property.
  • Two different runPrompt calls are made, one for generating a poem and another for generating JSON. The system option is used in the first call. Each runPrompt has its own label.

generated by pr-describe

@@ -8,7 +8,7 @@ import {
CancellationToken,
checkCancelled,
} from "./cancellation"
import { assert, logError } from "./util"
import { assert, logError, logVerbose } from "./util"

Choose a reason for hiding this comment

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

Unused import 'logVerbose'. If it's not being used, it should be removed.

generated by pr-review-commit unused_import

@@ -27,15 +27,16 @@ import {
} from "./chattypes"
import { promptParametersSchemaToJSONSchema } from "./parameters"

async function callExpander(
export async function callExpander(

Choose a reason for hiding this comment

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

The visibility of the function 'callExpander' has been changed from private to public. This could potentially expose internal logic and should be reviewed.

generated by pr-review-commit function_visibility_change

import { CancelError, isCancelError, serializeError } from "./error"
import {
CancelError,
isCancelError,

Choose a reason for hiding this comment

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

Unused import 'ChatCompletionMessageParam'. If it's not being used, it should be removed.

generated by pr-review-commit unused_import

Copy link

The pull request introduces several changes across multiple files. Here are the main points:

  1. In chat.ts, logVerbose is being imported from ./util but it's not used anywhere in the file.

  2. In expander.ts, there's a change of function definition for callExpander. The function now accepts an additional parameter prj: Project. This function is then being invoked in expandTemplate with prj as the first argument. This modification has also led to changes in the return type of several parts of the code, which are now more structured.

  3. In promptcontext.ts, the createPromptContext function is also updated to accept one more parameter prj: Project. This change affects the runPrompt function where now it also takes into account 'system' in runOptions and throws NotSupportedError for several unsupported cases.

  4. In systems.ts, there's a change in how systems are resolved. It uses more strict regular expressions to match keywords in jsSource for system identification.

  5. In prompt_template.d.ts, a new property system is added to the interface PromptGeneratorOptions.

Concerns:

  • Unused import logVerbose in chat.ts may lead to linting warnings.

Suggestions:

--- a/packages/core/src/chat.ts
+++ b/packages/core/src/chat.ts
@@ -8,7 +8,7 @@ import {
     CancellationToken,
     checkCancelled,
 } from "./cancellation"
-import { assert, logError, logVerbose } from "./util"
+import { assert, logError } from "./util"

Overall, this pull request looks good to me (LGTM). 🚀 Please consider the suggestion regarding the unused import.

generated by pr-review

@pelikhan pelikhan merged commit fcc9701 into main Aug 16, 2024
9 checks passed
@pelikhan pelikhan deleted the runpromptsystem branch August 16, 2024 18:12
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