Remove system prompt from playground creation page (#1020)

* frontend: remove system prompt from creation form Signed-off-by: Philippe Martin <[email protected]> * backend: remove systemPrompt parameter for playground creation Signed-off-by: Philippe Martin <[email protected]> --------- Signed-off-by: Philippe Martin <[email protected]>
containers · Apr 30, 2024 · ab0caf4 · ab0caf4
1 parent 7595c87
commit ab0caf4
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 87 deletions.
diff --git a/packages/backend/src/managers/playgroundV2Manager.spec.ts b/packages/backend/src/managers/playgroundV2Manager.spec.ts
@@ -81,7 +81,7 @@ test('submit should throw an error if the server is stopped', async () => {
     } as unknown as InferenceServer,
   ]);
   const manager = new PlaygroundV2Manager(webviewMock, inferenceManagerMock, taskRegistryMock, telemetryMock);
-  await manager.createPlayground('playground 1', { id: 'model1' } as ModelInfo, '', 'tracking-1');
+  await manager.createPlayground('playground 1', { id: 'model1' } as ModelInfo, 'tracking-1');
 
   vi.mocked(inferenceManagerMock.getServers).mockReturnValue([
     {
@@ -114,7 +114,7 @@ test('submit should throw an error if the server is unhealthy', async () => {
     } as unknown as InferenceServer,
   ]);
   const manager = new PlaygroundV2Manager(webviewMock, inferenceManagerMock, taskRegistryMock, telemetryMock);
-  await manager.createPlayground('p1', { id: 'model1' } as ModelInfo, '', 'tracking-1');
+  await manager.createPlayground('p1', { id: 'model1' } as ModelInfo, 'tracking-1');
   const playgroundId = manager.getConversations()[0].id;
   await expect(manager.submit(playgroundId, 'dummyUserInput')).rejects.toThrowError(
     'Inference server is not healthy, currently status: unhealthy.',
@@ -140,7 +140,7 @@ test('create playground should create conversation.', async () => {
   ]);
   const manager = new PlaygroundV2Manager(webviewMock, inferenceManagerMock, taskRegistryMock, telemetryMock);
   expect(manager.getConversations().length).toBe(0);
-  await manager.createPlayground('playground 1', { id: 'model-1' } as ModelInfo, '', 'tracking-1');
+  await manager.createPlayground('playground 1', { id: 'model-1' } as ModelInfo, 'tracking-1');
 
   const conversations = manager.getConversations();
   expect(conversations.length).toBe(1);
@@ -176,7 +176,7 @@ test('valid submit should create IPlaygroundMessage and notify the webview', asy
   } as unknown as OpenAI);
 
   const manager = new PlaygroundV2Manager(webviewMock, inferenceManagerMock, taskRegistryMock, telemetryMock);
-  await manager.createPlayground('playground 1', { id: 'dummyModelId' } as ModelInfo, undefined, 'tracking-1');
+  await manager.createPlayground('playground 1', { id: 'dummyModelId' } as ModelInfo, 'tracking-1');
 
   const date = new Date(2000, 1, 1, 13);
   vi.setSystemTime(date);
@@ -245,7 +245,7 @@ test('submit should send options', async () => {
   } as unknown as OpenAI);
 
   const manager = new PlaygroundV2Manager(webviewMock, inferenceManagerMock, taskRegistryMock, telemetryMock);
-  await manager.createPlayground('playground 1', { id: 'dummyModelId' } as ModelInfo, undefined, 'tracking-1');
+  await manager.createPlayground('playground 1', { id: 'dummyModelId' } as ModelInfo, 'tracking-1');
 
   const playgrounds = manager.getConversations();
   await manager.submit(playgrounds[0].id, 'dummyUserInput', { temperature: 0.123, max_tokens: 45, top_p: 0.345 });
@@ -282,7 +282,6 @@ test('creating a new playground should send new playground to frontend', async (
       id: 'model-1',
       name: 'Model 1',
     } as unknown as ModelInfo,
-    '',
     'tracking-1',
   );
   expect(webviewMock.postMessage).toHaveBeenCalledWith({
@@ -307,7 +306,6 @@ test('creating a new playground with no name should send new playground to front
       id: 'model-1',
       name: 'Model 1',
     } as unknown as ModelInfo,
-    '',
     'tracking-1',
   );
   expect(webviewMock.postMessage).toHaveBeenCalledWith({
@@ -333,7 +331,6 @@ test('creating a new playground with no model served should start an inference s
       id: 'model-1',
       name: 'Model 1',
     } as unknown as ModelInfo,
-    '',
     'tracking-1',
   );
   expect(createInferenceServerMock).toHaveBeenCalledWith(
@@ -370,7 +367,6 @@ test('creating a new playground with the model already served should not start a
       id: 'model-1',
       name: 'Model 1',
     } as unknown as ModelInfo,
-    '',
     'tracking-1',
   );
   expect(createInferenceServerMock).not.toHaveBeenCalled();
@@ -399,7 +395,6 @@ test('creating a new playground with the model server stopped should start the i
       id: 'model-1',
       name: 'Model 1',
     } as unknown as ModelInfo,
-    '',
     'tracking-1',
   );
   expect(createInferenceServerMock).not.toHaveBeenCalled();
@@ -417,7 +412,6 @@ test('delete conversation should delete the conversation', async () => {
       id: 'model-1',
       name: 'Model 1',
     } as unknown as ModelInfo,
-    '',
     'tracking-1',
   );
 
@@ -437,7 +431,6 @@ test('creating a new playground with an existing name shoud fail', async () => {
       id: 'model-1',
       name: 'Model 1',
     } as unknown as ModelInfo,
-    '',
     'tracking-1',
   );
   await expect(
@@ -447,7 +440,6 @@ test('creating a new playground with an existing name shoud fail', async () => {
         id: 'model-2',
         name: 'Model 2',
       } as unknown as ModelInfo,
-      '',
       'tracking-2',
     ),
   ).rejects.toThrowError('a playground with the name a name already exists');
@@ -465,9 +457,9 @@ test('requestCreatePlayground should call createPlayground and createTask, then
   });
   const createPlaygroundSpy = vi.spyOn(manager, 'createPlayground').mockResolvedValue('playground-1');
 
-  const id = await manager.requestCreatePlayground('a name', { id: 'model-1' } as ModelInfo, '');
+  const id = await manager.requestCreatePlayground('a name', { id: 'model-1' } as ModelInfo);
 
-  expect(createPlaygroundSpy).toHaveBeenCalledWith('a name', { id: 'model-1' } as ModelInfo, '', expect.any(String));
+  expect(createPlaygroundSpy).toHaveBeenCalledWith('a name', { id: 'model-1' } as ModelInfo, expect.any(String));
   expect(createTaskMock).toHaveBeenCalledWith('Creating Playground environment', 'loading', {
     trackingId: id,
   });
@@ -494,9 +486,9 @@ test('requestCreatePlayground should call createPlayground and createTask, then
   });
   const createPlaygroundSpy = vi.spyOn(manager, 'createPlayground').mockRejectedValue(new Error('an error'));
 
-  const id = await manager.requestCreatePlayground('a name', { id: 'model-1' } as ModelInfo, '');
+  const id = await manager.requestCreatePlayground('a name', { id: 'model-1' } as ModelInfo);
 
-  expect(createPlaygroundSpy).toHaveBeenCalledWith('a name', { id: 'model-1' } as ModelInfo, '', expect.any(String));
+  expect(createPlaygroundSpy).toHaveBeenCalledWith('a name', { id: 'model-1' } as ModelInfo, expect.any(String));
   expect(createTaskMock).toHaveBeenCalledWith('Creating Playground environment', 'loading', {
     trackingId: id,
   });
@@ -520,27 +512,6 @@ test('requestCreatePlayground should call createPlayground and createTask, then
 });
 
 describe('system prompt', () => {
-  test('create playground with system prompt should init the conversation with one message', async () => {
-    vi.mocked(inferenceManagerMock.getServers).mockReturnValue([
-      {
-        status: 'running',
-        models: [
-          {
-            id: 'model1',
-          },
-        ],
-      } as unknown as InferenceServer,
-    ]);
-    const manager = new PlaygroundV2Manager(webviewMock, inferenceManagerMock, taskRegistryMock, telemetryMock);
-    await manager.createPlayground('playground 1', { id: 'model1' } as ModelInfo, 'dummySystemPrompt', 'tracking-1');
-
-    const conversations = manager.getConversations();
-    expect(conversations.length).toBe(1);
-    expect(conversations[0].messages.length).toBe(1);
-    expect(conversations[0].messages[0].role).toBe('system');
-    expect(conversations[0].messages[0].content).toBe('dummySystemPrompt');
-  });
-
   test('set system prompt on non existing conversation should throw an error', async () => {
     vi.mocked(inferenceManagerMock.getServers).mockReturnValue([
       {
@@ -559,25 +530,6 @@ describe('system prompt', () => {
     }).toThrowError('conversation with id invalid does not exist.');
   });
 
-  test('set system prompt should overwrite existing system prompt', async () => {
-    vi.mocked(inferenceManagerMock.getServers).mockReturnValue([
-      {
-        status: 'running',
-        models: [
-          {
-            id: 'model1',
-          },
-        ],
-      } as unknown as InferenceServer,
-    ]);
-    const manager = new PlaygroundV2Manager(webviewMock, inferenceManagerMock, taskRegistryMock, telemetryMock);
-    await manager.createPlayground('playground 1', { id: 'model1' } as ModelInfo, 'dummySystemPrompt', 'tracking-1');
-
-    const conversations = manager.getConversations();
-    manager.setSystemPrompt(conversations[0].id, 'newSystemPrompt');
-    expect(manager.getConversations()[0].messages[0].content).toBe('newSystemPrompt');
-  });
-
   test('set system prompt should throw an error if user already submit message', async () => {
     vi.mocked(inferenceManagerMock.getServers).mockReturnValue([
       {
@@ -608,7 +560,7 @@ describe('system prompt', () => {
     } as unknown as OpenAI);
 
     const manager = new PlaygroundV2Manager(webviewMock, inferenceManagerMock, taskRegistryMock, telemetryMock);
-    await manager.createPlayground('playground 1', { id: 'dummyModelId' } as ModelInfo, undefined, 'tracking-1');
+    await manager.createPlayground('playground 1', { id: 'dummyModelId' } as ModelInfo, 'tracking-1');
 
     const date = new Date(2000, 1, 1, 13);
     vi.setSystemTime(date);

diff --git a/packages/backend/src/managers/playgroundV2Manager.ts b/packages/backend/src/managers/playgroundV2Manager.ts
@@ -50,18 +50,17 @@ export class PlaygroundV2Manager implements Disposable {
     this.#conversationRegistry.deleteConversation(conversationId);
   }
 
-  async requestCreatePlayground(name: string, model: ModelInfo, systemPrompt?: string): Promise<string> {
+  async requestCreatePlayground(name: string, model: ModelInfo): Promise<string> {
     const trackingId: string = getRandomString();
     const task = this.taskRegistry.createTask('Creating Playground environment', 'loading', {
       trackingId: trackingId,
     });
 
     const telemetry: Record<string, unknown> = {
       hasName: !!name,
-      hasSystemPrompt: !!systemPrompt,
       modelId: model.id,
     };
-    this.createPlayground(name, model, systemPrompt, trackingId)
+    this.createPlayground(name, model, trackingId)
       .then((playgroundId: string) => {
         this.taskRegistry.updateTask({
           ...task,
@@ -100,12 +99,7 @@ export class PlaygroundV2Manager implements Disposable {
     return trackingId;
   }
 
-  async createPlayground(
-    name: string,
-    model: ModelInfo,
-    systemPrompt: string | undefined,
-    trackingId: string,
-  ): Promise<string> {
+  async createPlayground(name: string, model: ModelInfo, trackingId: string): Promise<string> {
     if (!name) {
       name = this.getFreeName();
     }
@@ -116,11 +110,6 @@ export class PlaygroundV2Manager implements Disposable {
     // Create conversation
     const conversationId = this.#conversationRegistry.createConversation(name, model.id);
 
-    // If system prompt let's add it to the conversation
-    if (systemPrompt !== undefined && systemPrompt.length > 0) {
-      this.submitSystemPrompt(conversationId, 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));

diff --git a/packages/backend/src/studio-api-impl.ts b/packages/backend/src/studio-api-impl.ts
@@ -78,9 +78,9 @@ export class StudioApiImpl implements StudioAPI {
       });
   }
 
-  async requestCreatePlayground(name: string, model: ModelInfo, systemPrompt?: string): Promise<string> {
+  async requestCreatePlayground(name: string, model: ModelInfo): Promise<string> {
     try {
-      return this.playgroundV2.requestCreatePlayground(name, model, systemPrompt);
+      return this.playgroundV2.requestCreatePlayground(name, model);
     } catch (err: unknown) {
       console.error('Something went wrong while trying to create playground environment', err);
       throw err;

diff --git a/packages/frontend/src/pages/PlaygroundCreate.svelte b/packages/frontend/src/pages/PlaygroundCreate.svelte
@@ -19,7 +19,6 @@ 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;
 let errorMsg: string | undefined = undefined;
@@ -58,7 +57,7 @@ async function submit() {
   submitted = true;
   try {
     // Using || and not && as we want to have the empty string systemPrompt passed as undefined
-    trackingId = await studioClient.requestCreatePlayground(playgroundName, model, systemPrompt || undefined);
+    trackingId = await studioClient.requestCreatePlayground(playgroundName, model);
   } catch (err: unknown) {
     trackingId = undefined;
     console.error('Something wrong while trying to create the playground.', err);
@@ -173,16 +172,6 @@ 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}"
-            disabled="{submitted}"
-            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>
         {#if errorMsg !== undefined}
           <ErrorMessage error="{errorMsg}" />

diff --git a/packages/shared/src/StudioAPI.ts b/packages/shared/src/StudioAPI.ts
@@ -151,7 +151,7 @@ export abstract class StudioAPI {
    */
   abstract createSnippet(options: RequestOptions, language: string, variant: string): Promise<string>;
 
-  abstract requestCreatePlayground(name: string, model: ModelInfo, systemPrompt?: string): Promise<string>;
+  abstract requestCreatePlayground(name: string, model: ModelInfo): Promise<string>;
 
   /**
    * Delete a conversation