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

Prompt reorg for caching + usage collection #743

Merged
merged 9 commits into from
Oct 1, 2024
Merged

Prompt reorg for caching + usage collection #743

merged 9 commits into from
Oct 1, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Oct 1, 2024

This pull request optimizes chat caching by moving definitions to the back of the prompt structure. The layoutPromptNode function is introduced to sort the children of a prompt node, ensuring that definitions are placed after other types of nodes. This change improves the efficiency of chat caching with OpenAI.


  • 🧩 A new function layoutPromptNode has been added which aims to optimize chat caching with OpenAI by reordering nodes inside the prompt. It places the "def" type nodes towards the end. This function is triggered before the node tracing functionality.
    • The function works by sorting child nodes based on their type. Nodes with type "def" are pushed towards the end.
    • It maps the preview field of children nodes before and after the sorting operation. Any changes detected in the preview result in the function returning true.
    • The returned value indicates whether there was a change in the order of the nodes.
  • 🔄 The order in which nodes are processed during a prompt rendering operation has been updated. After resolving a prompt node, the new layoutPromptNode function is called. If it identifies a change in the order of the nodes, the trace of the prompt node is called again with the label "layout".
  • 🔃 The prompt rendering operation is further adjusted to accommodate this new structural change.

All these modifications will help to better leverage OpenAI's chat caching functionality, thus potentially improving performance.

generated by pr-describe

Copy link

github-actions bot commented Oct 1, 2024

The changes in this diff primarily focus on the introduction of a new function layoutPromptNode. The purpose of this function is to optimize chat caching with OpenAI by moving "defs" to the back of the prompt.

Concerns and Suggestions:

  1. ⚠️ The function layoutPromptNode is being called after resolvePromptNode and tracePromptNode in renderPromptNode function. If the layout of the node is changing, it might be better to call layoutPromptNode before these functions. This would ensure that the layout is finalized before resolving and tracing the node.
   export async function renderPromptNode(
     ...
-    await resolvePromptNode(model, node)
-    await tracePromptNode(trace, node)
 
+    if (await layoutPromptNode(model, node))
+        await tracePromptNode(trace, node, { label: "layout" })
+
+    await resolvePromptNode(model, node)
+    await tracePromptNode(trace, node)

     ...
  1. ✨ As an enhancement, consider adding error handling in layoutPromptNode. If there are any issues with sorting or rearranging the children nodes, it would be beneficial to have error messaging or fallback procedures in place.

  2. 🌟 It's great to see that the changes are well-documented with comments. Documentation is crucial to maintaining the readability and maintainability of the codebase, so keep it up!

generated by pr-review

@@ -874,6 +900,9 @@ export async function renderPromptNode(
await resolvePromptNode(model, node)
await tracePromptNode(trace, node)

if (await layoutPromptNode(model, node))
await tracePromptNode(trace, node, { label: "layout" })

Copy link

Choose a reason for hiding this comment

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

The function layoutPromptNode is called with await but it does not return a promise. This might lead to unexpected behavior. Please ensure that the function returns a promise or remove the await keyword if it's not needed. 😊

generated by pr-review-commit async_function

packages/core/src/chat.ts Outdated Show resolved Hide resolved
@pelikhan pelikhan changed the title Optimize chat caching by repositioning definitions in prompt structure Prompt reorg for caching + usage collection Oct 1, 2024
Copy link

github-actions bot commented Oct 1, 2024

Investigator report

Summary of Root Cause

The failure is caused by a TypeError related to trying to convert undefined or null to an object. This error occurs in the runScript function within genaiscript.cjs at line 87676.

Key Points:

  • The error message is: TypeError: Cannot convert undefined or null to object.
  • This occurs in two subtests: slides greeter and parameters.
  • Both errors result in an exit code of 255, which is unexpected as the tests are designed to pass with an exit code of 0.

Code Causing the Issue

The error is occurring in:

at Function.entries (<anonymous>)
at runScript (/home/runner/work/genaiscript/genaiscript/packages/cli/built/genaiscript.cjs:87676:37)

This suggests a problem with object manipulation, likely related to incorrect assumptions about data structure or missing data.

Suggested Fix

--- a/packages/cli/src/genaiscript.ts
+++ b/packages/cli/src/genaiscript.ts
@@ -87673,7 +87673,11 @@ export function runScript() {
     // Existing code...

-    Object.entries(someVar).forEach(([key, value]) => {
+    if (someVar && typeof someVar === 'object') {
+        Object.entries(someVar).forEach(([key, value]) => {
+            // Existing logic...
+        });
+    }

     // Existing code...

Explanation:

  • Ensure someVar is not undefined or null and is indeed an object before attempting to use Object.entries on it.

generated by gai

@pelikhan pelikhan merged commit a64b1be into main Oct 1, 2024
11 checks passed
@pelikhan pelikhan deleted the def-back branch October 1, 2024 19:42
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