-
Notifications
You must be signed in to change notification settings - Fork 22
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
ask the system prompt from the Playground creation form #643
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -22,7 +22,7 @@ import type { ChatCompletionChunk, ChatCompletionMessageParam } from 'openai/src | |||||
import type { ModelOptions } from '@shared/src/models/IModelOptions'; | ||||||
import type { Stream } from 'openai/streaming'; | ||||||
import { ConversationRegistry } from '../registries/conversationRegistry'; | ||||||
import type { Conversation, PendingChat, UserChat } from '@shared/src/models/IPlaygroundMessage'; | ||||||
import type { Conversation, PendingChat, SystemPrompt, UserChat } from '@shared/src/models/IPlaygroundMessage'; | ||||||
import type { PlaygroundV2 } from '@shared/src/models/IPlaygroundV2'; | ||||||
import { Publisher } from '../utils/Publisher'; | ||||||
import { Messages } from '@shared/Messages'; | ||||||
|
@@ -54,13 +54,13 @@ export class PlaygroundV2Manager extends Publisher<PlaygroundV2[]> implements Di | |||||
this.notify(); | ||||||
} | ||||||
|
||||||
async requestCreatePlayground(name: string, model: ModelInfo): Promise<string> { | ||||||
async requestCreatePlayground(name: string, model: ModelInfo, systemPrompt: string): Promise<string> { | ||||||
const trackingId: string = getRandomString(); | ||||||
const task = this.taskRegistry.createTask('Creating Playground environment', 'loading', { | ||||||
trackingId: trackingId, | ||||||
}); | ||||||
|
||||||
this.createPlayground(name, model, trackingId) | ||||||
this.createPlayground(name, model, systemPrompt, trackingId) | ||||||
.then((playgroundId: string) => { | ||||||
this.taskRegistry.updateTask({ | ||||||
...task, | ||||||
|
@@ -94,7 +94,7 @@ export class PlaygroundV2Manager extends Publisher<PlaygroundV2[]> implements Di | |||||
return trackingId; | ||||||
} | ||||||
|
||||||
async createPlayground(name: string, model: ModelInfo, trackingId: string): Promise<string> { | ||||||
async createPlayground(name: string, model: ModelInfo, systemPrompt: string, trackingId: string): Promise<string> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
const id = `${this.#playgroundCounter++}`; | ||||||
|
||||||
if (!name) { | ||||||
|
@@ -103,6 +103,15 @@ export class PlaygroundV2Manager extends Publisher<PlaygroundV2[]> implements Di | |||||
|
||||||
this.#conversationRegistry.createConversation(id); | ||||||
|
||||||
if (systemPrompt) { | ||||||
this.#conversationRegistry.submit(id, { | ||||||
content: systemPrompt, | ||||||
role: 'system', | ||||||
id: this.getUniqueId(), | ||||||
timestamp: Date.now(), | ||||||
} as SystemPrompt); | ||||||
} | ||||||
|
||||||
// create/start inference server if necessary | ||||||
const servers = this.inferenceManager.getServers(); | ||||||
const server = servers.find(s => s.models.map(mi => mi.id).includes(model.id)); | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -74,9 +74,9 @@ export class StudioApiImpl implements StudioAPI { | |||||
}); | ||||||
} | ||||||
|
||||||
async requestCreatePlayground(name: string, model: ModelInfo): Promise<string> { | ||||||
async requestCreatePlayground(name: string, model: ModelInfo, systemPrompt: string): Promise<string> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
try { | ||||||
return this.playgroundV2.requestCreatePlayground(name, model); | ||||||
return this.playgroundV2.requestCreatePlayground(name, model, systemPrompt); | ||||||
} catch (err: unknown) { | ||||||
console.error('Something went wrong while trying to create playground environment', err); | ||||||
throw err; | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -18,6 +18,7 @@ let localModels: ModelInfo[]; | |||||
$: localModels = $modelsInfo.filter(model => model.file); | ||||||
$: availModels = $modelsInfo.filter(model => !model.file); | ||||||
let modelId: string | undefined = undefined; | ||||||
let systemPrompt: string | undefined = undefined; | ||||||
let submitted: boolean = false; | ||||||
let playgroundName: string; | ||||||
|
||||||
|
@@ -55,7 +56,7 @@ async function submit() { | |||||
// disable submit button | ||||||
submitted = true; | ||||||
try { | ||||||
trackingId = await studioClient.requestCreatePlayground(playgroundName, model); | ||||||
trackingId = await studioClient.requestCreatePlayground(playgroundName, model, systemPrompt ?? ''); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was my first attempt, but the problem I can see is that systemPrompt can be either undefined or an empty string, depending on if the user lets the textarea untouched, or edits its content and removes it. I wanted to have only one value (the empty string) to indicate the systemPrompt is empty. But as the two values are handled correctly on the other side, we can do like this if you prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that the empty value should not be allowed, and be replaced with |
||||||
} catch (err: unknown) { | ||||||
trackingId = undefined; | ||||||
console.error('Something wrong while trying to create the playground.', err); | ||||||
|
@@ -161,6 +162,15 @@ onDestroy(() => { | |||||
</div> | ||||||
</div> | ||||||
{/if} | ||||||
|
||||||
<label for="model" class="pt-4 block mb-2 text-sm font-bold text-gray-400">System prompt</label> | ||||||
<textarea | ||||||
aria-label="system-prompt-textarea" | ||||||
bind:value="{systemPrompt}" | ||||||
class="w-full p-2 outline-none text-sm bg-charcoal-600 rounded-sm text-gray-700 placeholder-gray-700" | ||||||
rows="4" | ||||||
placeholder="Optionally provide system prompt to define general context, instructions or guidelines to be used with each query" | ||||||
></textarea> | ||||||
</div> | ||||||
<footer> | ||||||
<div class="w-full flex flex-col"> | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -143,7 +143,7 @@ export abstract class StudioAPI { | |||||
*/ | ||||||
abstract createSnippet(options: RequestOptions, language: string, variant: string): Promise<string>; | ||||||
|
||||||
abstract requestCreatePlayground(name: string, model: ModelInfo): Promise<string>; | ||||||
abstract requestCreatePlayground(name: string, model: ModelInfo, systemPrompt: string): Promise<string>; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
abstract getPlaygroundsV2(): Promise<PlaygroundV2[]>; | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.