Skip to content

Commit

Permalink
fix: AI history view crashing on first use
Browse files Browse the repository at this point in the history
The AI history view sometimes crashed on first use when the user tried
to select an agent. This was caused by an empty agent list bound in the
initial UI.

Refactors the history view by making sure that the history view is only
instantiated once it is shown instead of early in application start.
This avoids binding a still empty list of agent contributions to the
UI.

Also adds fallback content in case there are no registered agents as
the used 'SelectComponent' crashes with empty options.

Also moves the history sort change tracking outside of the widget.
Previously this was broken as only the initial widget was tracked.
  • Loading branch information
sdirix authored Nov 13, 2024
1 parent 066b2d1 commit 7c05b6f
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 20 deletions.
22 changes: 8 additions & 14 deletions packages/ai-history/src/browser/ai-history-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// *****************************************************************************
import { FrontendApplication, codicon } from '@theia/core/lib/browser';
import { AIViewContribution } from '@theia/ai-core/lib/browser';
import { inject, injectable, postConstruct } from '@theia/core/shared/inversify';
import { inject, injectable } from '@theia/core/shared/inversify';
import { AIHistoryView } from './ai-history-widget';
import { Command, CommandRegistry, Emitter } from '@theia/core';
import { TabBarToolbarContribution, TabBarToolbarRegistry } from '@theia/core/lib/browser/shell/tab-bar-toolbar';
Expand Down Expand Up @@ -49,6 +49,9 @@ export const AI_HISTORY_VIEW_CLEAR = Command.toLocalizedCommand({
export class AIHistoryViewContribution extends AIViewContribution<AIHistoryView> implements TabBarToolbarContribution {
@inject(CommunicationRecordingService) private recordingService: CommunicationRecordingService;

protected readonly chronologicalChangedEmitter = new Emitter<void>();
protected readonly chronologicalStateChanged = this.chronologicalChangedEmitter.event;

constructor() {
super({
widgetId: AIHistoryView.ID,
Expand All @@ -75,6 +78,7 @@ export class AIHistoryViewContribution extends AIViewContribution<AIHistoryView>
isVisible: widget => this.withHistoryWidget(widget, historyView => !historyView.isChronological),
execute: widget => this.withHistoryWidget(widget, historyView => {
historyView.sortHistory(true);
this.chronologicalChangedEmitter.fire();
return true;
})
});
Expand All @@ -83,6 +87,7 @@ export class AIHistoryViewContribution extends AIViewContribution<AIHistoryView>
isVisible: widget => this.withHistoryWidget(widget, historyView => historyView.isChronological),
execute: widget => this.withHistoryWidget(widget, historyView => {
historyView.sortHistory(false);
this.chronologicalChangedEmitter.fire();
return true;
})
});
Expand All @@ -106,31 +111,20 @@ export class AIHistoryViewContribution extends AIViewContribution<AIHistoryView>
return widget instanceof AIHistoryView ? predicate(widget) : false;
}

protected readonly onAIHistoryWidgetStateChangedEmitter = new Emitter<void>();
protected readonly onAIHistoryWidgettStateChanged = this.onAIHistoryWidgetStateChangedEmitter.event;

@postConstruct()
protected override init(): void {
super.init();
this.widget.then(widget => {
widget.onStateChanged(() => this.onAIHistoryWidgetStateChangedEmitter.fire());
});
}

registerToolbarItems(registry: TabBarToolbarRegistry): void {
registry.registerItem({
id: AI_HISTORY_VIEW_SORT_CHRONOLOGICALLY.id,
command: AI_HISTORY_VIEW_SORT_CHRONOLOGICALLY.id,
tooltip: 'Sort chronologically',
isVisible: widget => this.withHistoryWidget(widget),
onDidChange: this.onAIHistoryWidgettStateChanged
onDidChange: this.chronologicalStateChanged
});
registry.registerItem({
id: AI_HISTORY_VIEW_SORT_REVERSE_CHRONOLOGICALLY.id,
command: AI_HISTORY_VIEW_SORT_REVERSE_CHRONOLOGICALLY.id,
tooltip: 'Sort reverse chronologically',
isVisible: widget => this.withHistoryWidget(widget),
onDidChange: this.onAIHistoryWidgettStateChanged
onDidChange: this.chronologicalStateChanged
});
registry.registerItem({
id: AI_HISTORY_VIEW_CLEAR.id,
Expand Down
16 changes: 10 additions & 6 deletions packages/ai-history/src/browser/ai-history-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { inject, injectable, postConstruct } from '@theia/core/shared/inversify'
import * as React from '@theia/core/shared/react';
import { CommunicationCard } from './ai-history-communication-card';
import { SelectComponent, SelectOption } from '@theia/core/lib/browser/widgets/select-component';
import { deepClone, Emitter } from '@theia/core';
import { deepClone } from '@theia/core';

namespace AIHistoryView {
export interface State {
Expand All @@ -40,8 +40,6 @@ export class AIHistoryView extends ReactWidget implements StatefulWidget {
protected selectedAgent?: Agent;

protected _state: AIHistoryView.State = { chronological: false };
protected readonly onStateChangedEmitter = new Emitter<AIHistoryView.State>();
readonly onStateChanged = this.onStateChangedEmitter.event;

constructor() {
super();
Expand All @@ -58,7 +56,7 @@ export class AIHistoryView extends ReactWidget implements StatefulWidget {

protected set state(state: AIHistoryView.State) {
this._state = state;
this.onStateChangedEmitter.fire(this._state);
this.update();
}

storeState(): object {
Expand All @@ -79,7 +77,6 @@ export class AIHistoryView extends ReactWidget implements StatefulWidget {
this.toDispose.push(this.recordingService.onDidRecordRequest(entry => this.historyContentUpdated(entry)));
this.toDispose.push(this.recordingService.onDidRecordResponse(entry => this.historyContentUpdated(entry)));
this.toDispose.push(this.recordingService.onStructuralChange(() => this.update()));
this.toDispose.push(this.onStateChanged(newState => this.update()));
this.selectAgent(this.agentService.getAllAgents()[0]);
}

Expand All @@ -99,10 +96,17 @@ export class AIHistoryView extends ReactWidget implements StatefulWidget {
this.selectedAgent = this.agentService.getAllAgents().find(agent => agent.id === value.value);
this.update();
};
const agents = this.agentService.getAllAgents();
if (agents.length === 0) {
return (
<div className='agent-history-widget'>
<div className='theia-card no-content'>No agent available.</div>
</div >);
}
return (
<div className='agent-history-widget'>
<SelectComponent
options={this.agentService.getAllAgents().map(agent => ({
options={agents.map(agent => ({
value: agent.id,
label: agent.name,
description: agent.description || ''
Expand Down

0 comments on commit 7c05b6f

Please sign in to comment.