Skip to content

Commit

Permalink
Fix incorrect empty string handling (#360)
Browse files Browse the repository at this point in the history
This fixes a bug where passing an empty string in a conversational
prompt would cause errors to be thrown.
  • Loading branch information
petersalas committed Oct 2, 2023
1 parent af8bb9a commit b57ed36
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 22 deletions.
2 changes: 1 addition & 1 deletion packages/ai-jsx/package.json
Expand Up @@ -4,7 +4,7 @@
"repository": "fixie-ai/ai-jsx",
"bugs": "https://github.com/fixie-ai/ai-jsx/issues",
"homepage": "https://ai-jsx.com",
"version": "0.18.2",
"version": "0.18.3",
"volta": {
"extends": "../../package.json"
},
Expand Down
32 changes: 12 additions & 20 deletions packages/ai-jsx/src/core/conversation.tsx
Expand Up @@ -174,7 +174,7 @@ export function isConversationalComponent(element: AI.Element<any>): boolean {
).includes(element.tag);
}

function assertAllElementsAreConversationalComponents(partialRendering: AI.PartiallyRendered[]) {
function assertNoMeaningfulStringContent(partialRendering: AI.PartiallyRendered[]): AI.Element<any>[] {
const invalidChildren = partialRendering.filter((el) => typeof el === 'string' && el.trim()) as string[];
if (invalidChildren.length) {
throw new AIJSXError(
Expand All @@ -188,12 +188,12 @@ function assertAllElementsAreConversationalComponents(partialRendering: AI.Parti
}
);
}

return partialRendering.filter(AI.isElement);
}

function toConversationMessages(partialRendering: AI.PartiallyRendered[]): ConversationMessage[] {
assertAllElementsAreConversationalComponents(partialRendering);

return (partialRendering as AI.Element<any>[]).map<ConversationMessage>((e) => {
return assertNoMeaningfulStringContent(partialRendering).map<ConversationMessage>((e) => {
switch (e.tag) {
case UserMessage:
return { type: 'user', element: e };
Expand Down Expand Up @@ -436,18 +436,14 @@ export async function ShrinkConversation(

/** Converts a conversational `AI.Node` into a shrinkable tree. */
async function conversationToTreeRoots(conversation: AI.Node): Promise<TreeNode[]> {
const rendered = await render(conversation, {
stop: (e) => isConversationalComponent(e) || e.tag === InternalShrinkable,
});

assertAllElementsAreConversationalComponents(rendered);

const asTreeNodes = await Promise.all(
rendered.map<Promise<TreeNode | null>>(async (value) => {
if (typeof value === 'string') {
return null;
}
const rendered = assertNoMeaningfulStringContent(
await render(conversation, {
stop: (e) => isConversationalComponent(e) || e.tag === InternalShrinkable,
})
);

return Promise.all(
rendered.map<Promise<TreeNode>>(async (value) => {
if (value.tag === InternalShrinkable) {
const children = await conversationToTreeRoots(value.props.children);
return { type: 'shrinkable', element: value, cost: aggregateCost(children), children };
Expand All @@ -460,8 +456,6 @@ export async function ShrinkConversation(
};
})
);

return asTreeNodes.filter((n): n is TreeNode => n !== null);
}

/** Finds the least important node in any of the trees, considering cost as a second factor. */
Expand Down Expand Up @@ -531,10 +525,8 @@ export async function ShrinkConversation(
stop: (e) => isConversationalComponent(e) || e.tag === InternalShrinkable,
});

assertAllElementsAreConversationalComponents(rendered);

// If there are no shrinkable elements, there's no need to evaluate the cost.
const shrinkableOrConversationElements = rendered.filter(AI.isElement);
const shrinkableOrConversationElements = assertNoMeaningfulStringContent(rendered);
if (!shrinkableOrConversationElements.find((value) => value.tag === InternalShrinkable)) {
return shrinkableOrConversationElements;
}
Expand Down
6 changes: 5 additions & 1 deletion packages/docs/docs/changelog.md
@@ -1,6 +1,10 @@
# Changelog

## 0.18.2
## 0.18.3

- Fix an issue where empty strings in conversational prompts cause errors to be thrown.

## [0.18.2](https://github.com/fixie-ai/ai-jsx/commit/fc8ada2d9900b179252d377292835dc28998b86f)

- Modified `lib/openai` to preload the tokenizer to avoid a stall on first use
- Fixed an issue where `debug(component)` would throw an exception if a component had a prop that could not be JSON-serialized.
Expand Down

3 comments on commit b57ed36

@vercel
Copy link

@vercel vercel bot commented on b57ed36 Oct 2, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

ai-jsx-docs – ./packages/docs

ai-jsx-docs-git-main-fixie-ai.vercel.app
ai-jsx-docs.vercel.app
ai-jsx-docs-fixie-ai.vercel.app
docs.ai-jsx.com

@vercel
Copy link

@vercel vercel bot commented on b57ed36 Oct 2, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

ai-jsx-tutorial-nextjs – ./packages/tutorial-nextjs

ai-jsx-tutorial-nextjs-fixie-ai.vercel.app
ai-jsx-tutorial-nextjs-git-main-fixie-ai.vercel.app
ai-jsx-tutorial-nextjs.vercel.app

@vercel
Copy link

@vercel vercel bot commented on b57ed36 Oct 2, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

ai-jsx-nextjs-demo – ./packages/nextjs-demo

ai-jsx-nextjs-demo.vercel.app
ai-jsx-nextjs-demo-git-main-fixie-ai.vercel.app
ai-jsx-nextjs-demo-fixie-ai.vercel.app

Please sign in to comment.