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.

Sign up for GitHub

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

Jump to bottom

Support prompt variants #14487

Merged
merged 4 commits into from
Nov 23, 2024
Merged

Support prompt variants #14487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e770ba6
Support prompt variants
JonasHelming Nov 18, 2024
55e66af
Adress review comments
JonasHelming Nov 21, 2024
31db839
Fix linting
JonasHelming Nov 21, 2024
de67625
Remove deprecated function
JonasHelming Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/ai-chat/src/common/universal-chat-agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ simple solutions.
`
};

export const universalTemplateVariant: PromptTemplate = {
id: 'universal-system-empty',
template: '',
variantOf: universalTemplate.id,
};

@injectable()
export class UniversalChatAgent extends AbstractStreamParsingChatAgent implements ChatAgent {
name: string;
Expand All @@ -96,7 +102,7 @@ export class UniversalChatAgent extends AbstractStreamParsingChatAgent implement
+ 'questions the user might ask. The universal agent currently does not have any context by default, i.e. it cannot '
+ 'access the current user context or the workspace.';
this.variables = [];
this.promptTemplates = [universalTemplate];
this.promptTemplates = [universalTemplate, universalTemplateVariant];
this.functions = [];
this.agentSpecificVariables = [];
}
Expand Down
31 changes: 24 additions & 7 deletions packages/ai-core/src/browser/ai-configuration/agent-configuration-widget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,31 @@ export class AIAgentConfigurationWidget extends ReactWidget {
Enable Agent
</label>
</div>
<div className='ai-templates'>
{agent.promptTemplates?.map(template =>
<TemplateRenderer
key={agent?.id + '.' + template.id}
agentId={agent.id}
template={template}
promptCustomizationService={this.promptCustomizationService} />)}
<div className="settings-section-subcategory-title ai-settings-section-subcategory-title">
Prompt Templates
</div>
<div className="ai-templates">
{(() => {
const defaultTemplates = agent.promptTemplates?.filter(template => !template.variantOf) || [];
return defaultTemplates.length > 0 ? (
defaultTemplates.map(template => (
<div key={agent.id + '.' + template.id}>
<TemplateRenderer
key={agent.id + '.' + template.id}
agentId={agent.id}
template={template}
promptService={this.promptService}
aiSettingsService={this.aiSettingsService}
promptCustomizationService={this.promptCustomizationService}
/>
</div>
))
) : (
<div>No default template available</div>
);
})()}
</div>

<div className='ai-lm-requirements'>
<LanguageModelRenderer
agent={agent}
Expand Down
105 changes: 91 additions & 14 deletions packages/ai-core/src/browser/ai-configuration/template-settings-renderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,103 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************
import * as React from '@theia/core/shared/react';
import { PromptCustomizationService } from '../../common/prompt-service';
import { PromptTemplate } from '../../common';
import { PromptCustomizationService, PromptService } from '../../common/prompt-service';
import { AISettingsService, PromptTemplate } from '../../common';

export interface TemplateSettingProps {
const DEFAULT_VARIANT = 'default';

export interface TemplateRendererProps {
agentId: string;
template: PromptTemplate;
promptCustomizationService: PromptCustomizationService;
promptService: PromptService;
aiSettingsService: AISettingsService;
}

export const TemplateRenderer: React.FC<TemplateSettingProps> = ({ agentId, template, promptCustomizationService }) => {
export const TemplateRenderer: React.FC<TemplateRendererProps> = ({
agentId,
template,
promptCustomizationService,
promptService,
aiSettingsService,
}) => {
const [variantIds, setVariantIds] = React.useState<string[]>([]);
const [selectedVariant, setSelectedVariant] = React.useState<string>(DEFAULT_VARIANT);

React.useEffect(() => {
(async () => {
const variants = promptService.getVariantIds(template.id);
setVariantIds([DEFAULT_VARIANT, ...variants]);

const agentSettings = await aiSettingsService.getAgentSettings(agentId);
const currentVariant =
agentSettings?.selectedVariants?.[template.id] || DEFAULT_VARIANT;
setSelectedVariant(currentVariant);
})();
}, [template.id, promptService, aiSettingsService, agentId]);

const handleVariantChange = async (event: React.ChangeEvent<HTMLSelectElement>) => {
const newVariant = event.target.value;
setSelectedVariant(newVariant);

const agentSettings = await aiSettingsService.getAgentSettings(agentId);
const selectedVariants = agentSettings?.selectedVariants || {};

const updatedVariants = { ...selectedVariants };
if (newVariant === DEFAULT_VARIANT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to handle that case special?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dont understand the question. If the default template is selected for a template ID, we remove the entry from the selected variants?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah i see that, why not just store that the user selected the default variant?
I'm fine also with the current solution was just wondering, whether there is a special reason to do so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I find it cleaner to only store if variants are actually selected.

delete updatedVariants[template.id];
} else {
updatedVariants[template.id] = newVariant;
}

await aiSettingsService.updateAgentSettings(agentId, {
selectedVariants: updatedVariants,
});
};

const openTemplate = React.useCallback(async () => {
promptCustomizationService.editTemplate(template.id, template.template);
}, [template, promptCustomizationService]);
const templateId = selectedVariant === DEFAULT_VARIANT ? template.id : selectedVariant;
const selectedTemplate = promptService.getRawPrompt(templateId);
promptCustomizationService.editTemplate(templateId, selectedTemplate?.template || '');
}, [selectedVariant, template.id, promptService, promptCustomizationService]);

const resetTemplate = React.useCallback(async () => {
promptCustomizationService.resetTemplate(template.id);
}, [promptCustomizationService, template]);

return <>
{template.id}
<button className='theia-button main' onClick={openTemplate}>Edit</button>
<button className='theia-button secondary' onClick={resetTemplate}>Reset</button>
</>;
const templateId = selectedVariant === DEFAULT_VARIANT ? template.id : selectedVariant;
promptCustomizationService.resetTemplate(templateId);
}, [selectedVariant, template.id, promptCustomizationService]);

return (
<div className="template-renderer">
<div className="settings-section-title template-header">
<strong>{template.id}</strong>
</div>
<div className="template-controls">
{variantIds.length > 1 && (
<>
<label htmlFor={`variant-selector-${template.id}`} className="template-select-label">
Select Variant:
</label>
<select
id={`variant-selector-${template.id}`}
className="theia-select template-variant-selector"
value={selectedVariant}
onChange={handleVariantChange}
>
{variantIds.map(variantId => (
<option key={variantId} value={variantId}>
{variantId}
</option>
))}
</select>
</>
)}
<button className="theia-button main" onClick={openTemplate}>
Edit
</button>
<button className="theia-button secondary" onClick={resetTemplate}>
Reset
</button>
</div>
</div>
);
};
38 changes: 33 additions & 5 deletions packages/ai-core/src/browser/style/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,42 @@
margin-left: var(--theia-ui-padding);
}

.theia-settings-container .settings-section-subcategory-title.ai-settings-section-subcategory-title {
padding-left: 0;
}

.ai-templates {
display: grid;
/** Display content in 3 columns */
grid-template-columns: 1fr auto auto;
/** add a 3px gap between rows */
row-gap: 3px;
display: flex;
flex-direction: column;
gap: 5px;
}

.template-renderer {
display: flex;
flex-direction: column;
padding: 10px;
}

.template-header {
margin-bottom: 8px;
}

.template-controls {
display: flex;
align-items: center;
gap: 10px;
}

.template-select-label {
margin-right: 5px;
}

.template-variant-selector {
min-width: 120px;
}



#ai-variable-configuration-container-widget,
#ai-agent-configuration-container-widget {
margin-top: 5px;
Expand Down
2 changes: 1 addition & 1 deletion packages/ai-core/src/common/agent-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export class AgentServiceImpl implements AgentService {
registerAgent(agent: Agent): void {
this._agents.push(agent);
agent.promptTemplates.forEach(
template => this.promptService.storePrompt(template.id, template.template)
template => this.promptService.storePromptTemplate(template)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need the old method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would break the API, I don't know whether people are already using it. For conveniance, I would probably leave it.

Copy link
Contributor

@eneufeld eneufeld Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

mark it as deprecated? with a hint to use the new method instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, it is just a simpler version for all manditory values, I would keep it.

);
this.onDidChangeAgentsEmitter.fire();
}
Expand Down
Loading
Loading