feat: make systemPrompt sync and optional #29
Conversation
mdjastrzebski
force-pushed
the
feat/do-not-require-system-prompt
branch
from
3cea2f1 to
c0096a1
Compare
There was a problem hiding this comment.
it folder name intentional? Shouldn't be tests ?
There was a problem hiding this comment.
lol, good catch ;-)
| const { languageModel, calls } = createMockVercelModel({ text: 'Hello, world!' }); | ||
| const app = createApp({ | ||
| chatModel: new VercelChatModelAdapter({ languageModel }), | ||
| systemPrompt: 'This is system prompt', |
There was a problem hiding this comment.
Why did this test pass? Shouldn't systemPrompt be a function that returns string?
There was a problem hiding this comment.
You can have it both ways, function or just a string.
packages/core/src/ai/vercel.ts
Outdated
| const messages = context.messages; | ||
|
|
||
| const systemPrompt = await context.systemPrompt(); | ||
| let systemPrompt = context.systemPrompt(); | ||
| const entitiesPrompt = formatResolvedEntities(context.resolvedEntities); |
There was a problem hiding this comment.
let's put that logic inside "if(systemPrompt)" below. No need to check for entities if we add them only when systemPrompt is present.
| @@ -33,7 +33,7 @@ export type ErrorHandler = ( | |||
|
|
|||
| export type ApplicationConfig = { | |||
| chatModel: ChatModel; | |||
| systemPrompt: (context: RequestContext) => Promise<string> | string; | |||
| systemPrompt?: ((context: RequestContext) => string | null) | string; | |||
There was a problem hiding this comment.
Oh, ok. So please fix the type in docs as it does not match.
| @@ -30,7 +30,7 @@ export type RequestContext = { | |||
| tools: ApplicationTool[]; | |||
| references: ReferenceStorage; | |||
| resolvedEntities: EntityInfo; | |||
| systemPrompt: () => Promise<string> | string; | |||
| systemPrompt: () => string | null; | |||
There was a problem hiding this comment.
Same case. It can also be a string.
There was a problem hiding this comment.
inside context, it's already standardised to a function
mdjastrzebski
force-pushed
the
feat/do-not-require-system-prompt
branch
from
22a4c3a to
76a28ea
Compare
mdjastrzebski
force-pushed
the
feat/do-not-require-system-prompt
branch
from
76a28ea to
8d58f19
Compare
Summary
Two changes to
systemPrompt:nullvalue from the function form to indicate that you do not want to have a system prompt. This is required to support OpenAI's o1 model which throws error when receiving system prompt.Test plan
Added automated tests. Created another test model, this time on the Vercel Model/Adapter level, as system message transformation happens there.