Skip to content

Commit a3fea95

Browse files
committed
refactor(ai): simplify recent AI code and delete verified dead code
Two passes over the multi-provider-support work. Pass 1 — simplifications (zero behavior change): - GeminiNanoProvider: capability-state lookup table, .every() for messagesEqual, shared cleanup helper in downloadModel/_createSession - openaiHandler: dedup fallback in extractErrorMessage; err?.message form kept as (err && err.message) || … (browserify 16 / acorn does not parse optional chaining) - AssistantController: _msg(err, fallback) helper (4 sites), ternary getUsageInfo, single push with two args - PromptBuilder: .filter(Boolean) for section accumulation, ternary helpers, filter().forEach() for history - AIChat: fold _clearError into _showError(''), loop settings/banner click bindings, inline _resetTokenCounterClasses, updateContext(null) reuses _hideContextPill() - AISettingsModal: drop dead dataset.required, data-driven click bindings Pass 2 — dead code, each finding adversarially verified with two independent Explore skeptics (2×14 = 28 refutation attempts). Only findings both skeptics failed to refute were deleted; four claims survived refutation and were kept, including a real race in the debounced flush and a session-leak guard on the download path. - AssistantController._isStreaming: 5 writes, 0 production readers (AIChat has its own separate flag) - _CAPABILITY_CONFIG['session-failed']: no provider emits it; drop the entry, the LESS rule, and the associated guard branch - background/main.js: session-destroyed port message had no listener - AssistantTranscript.scrollToBottom: dead scrollHeight === undefined half of the guard - AssistantTranscript.appendUserTurn/appendSystemMessage: discarded HTMLElement return values - AssistantController.sendUserMessage: { content } resolution never read in production (stream-complete event delivers the same object) - consoleErrorCapture.uninstall: never invoked in production - AssistantTranscript._appendMessage: showCopyButton === true branch unreachable (no caller passes true) Tests: 637 passing (dropped one obsolete session-failed spec). JSHint: clean. Net: 13 files, +93 −195 lines.
1 parent 5e0448a commit a3fea95

13 files changed

Lines changed: 93 additions & 195 deletions

File tree

app/scripts/background/main.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -431,10 +431,6 @@
431431
promptAPIController.abort();
432432
promptAPIController = null;
433433
}
434-
435-
port.postMessage({
436-
type: 'session-destroyed'
437-
});
438434
}
439435

440436
// Listen for long-lived connections for Prompt API

app/scripts/modules/ai/AssistantController.js

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ const PromptBuilder = require('./PromptBuilder.js');
44
const providersRegistry = require('./providers/index.js');
55
const ConversationStore = require('./ConversationStore.js');
66

7+
function _msg(err, fallback) {
8+
return (err && err.message) || fallback;
9+
}
10+
711
/**
812
* Coordinates the AI Assistant: capability state, per-URL conversation memory, inspection context,
913
* streaming, and persistence. Delegates all provider-specific concerns (session lifecycle, prefix
@@ -47,7 +51,6 @@ function AssistantController({
4751
this._currentUrl = null;
4852
this._conversationMemory = [];
4953
this._inspectionContext = null;
50-
this._isStreaming = false;
5154
this._activeAbortController = null;
5255
}
5356

@@ -113,7 +116,7 @@ AssistantController.prototype.initialize = function () {
113116
this._setCapabilityState(capability.status, capability.message, 0, capability.reason);
114117
return this._loadConversationMemory();
115118
}, (err) => {
116-
this._setCapabilityState('unavailable', err && err.message ? err.message : 'Local AI is unavailable', 0);
119+
this._setCapabilityState('unavailable', _msg(err, 'Local AI is unavailable'), 0);
117120
});
118121
};
119122

@@ -122,7 +125,7 @@ AssistantController.prototype.initialize = function () {
122125
* persist both turns, and emit stream events. The current Inspection Context is injected.
123126
*
124127
* @param {string} userMessage
125-
* @returns {Promise<{content: string}>}
128+
* @returns {Promise<void>}
126129
*/
127130
AssistantController.prototype.sendUserMessage = function (userMessage) {
128131
const consoleErrors = this._getConsoleErrors();
@@ -134,7 +137,6 @@ AssistantController.prototype.sendUserMessage = function (userMessage) {
134137
consoleErrors: consoleErrors
135138
});
136139

137-
this._isStreaming = true;
138140
const abortController = new AbortController();
139141
this._activeAbortController = abortController;
140142

@@ -155,27 +157,26 @@ AssistantController.prototype.sendUserMessage = function (userMessage) {
155157
content: fullText
156158
});
157159
}).then(() => {
158-
this._conversationMemory.push({ role: 'user', content: userMessage });
159-
this._conversationMemory.push({ role: 'assistant', content: fullText });
160-
this._isStreaming = false;
160+
this._conversationMemory.push(
161+
{ role: 'user', content: userMessage },
162+
{ role: 'assistant', content: fullText }
163+
);
161164
if (this._activeAbortController === abortController) {
162165
this._activeAbortController = null;
163166
}
164167
if (this._capabilityState.status === 'streaming-failed') {
165168
this._setCapabilityState('ready', this._lastReadyMessage, 0);
166169
}
167170
this._emit('stream-complete', { content: fullText });
168-
return { content: fullText };
169171
});
170172
}, (err) => {
171-
this._isStreaming = false;
172173
if (this._activeAbortController === abortController) {
173174
this._activeAbortController = null;
174175
}
175176
if (err && err.name === 'AbortError') {
176177
throw err;
177178
}
178-
this._setCapabilityState('streaming-failed', err && err.message ? err.message : 'Streaming failed', 0);
179+
this._setCapabilityState('streaming-failed', _msg(err, 'Streaming failed'), 0);
179180
this._emit('stream-failed', err);
180181
throw err;
181182
});
@@ -262,7 +263,7 @@ AssistantController.prototype.downloadModel = function () {
262263
}).then(() => {
263264
this._setCapabilityState('ready', 'Model ready', 1);
264265
}, (err) => {
265-
this._setCapabilityState('unavailable', err && err.message ? err.message : 'Download failed', 0);
266+
this._setCapabilityState('unavailable', _msg(err, 'Download failed'), 0);
266267
throw err;
267268
});
268269
};
@@ -271,10 +272,9 @@ AssistantController.prototype.downloadModel = function () {
271272
* @returns {Promise<Object|null>}
272273
*/
273274
AssistantController.prototype.getUsageInfo = function () {
274-
if (typeof this._provider.getUsageInfo !== 'function') {
275-
return Promise.resolve(null);
276-
}
277-
return this._provider.getUsageInfo();
275+
return typeof this._provider.getUsageInfo === 'function' ?
276+
this._provider.getUsageInfo() :
277+
Promise.resolve(null);
278278
};
279279

280280
/**
@@ -304,15 +304,14 @@ AssistantController.prototype.setProvider = function (name, config) {
304304
this._activeAbortController.abort();
305305
this._activeAbortController = null;
306306
}
307-
this._isStreaming = false;
308307

309308
this._provider.destroy();
310309
this._provider = this._createProvider(name, config || {});
311310

312311
return this._provider.checkAvailability().then((capability) => {
313312
this._setCapabilityState(capability.status, capability.message, 0, capability.reason);
314313
}, (err) => {
315-
this._setCapabilityState('unavailable', err && err.message ? err.message : 'Provider unavailable', 0);
314+
this._setCapabilityState('unavailable', _msg(err, 'Provider unavailable'), 0);
316315
});
317316
};
318317

app/scripts/modules/ai/AssistantTranscript.js

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,16 @@ AssistantTranscript.prototype._renderEmptyState = function () {
4343

4444
/**
4545
* @param {string} content - Raw user input, escaped before insertion.
46-
* @returns {HTMLElement}
4746
*/
4847
AssistantTranscript.prototype.appendUserTurn = function (content) {
49-
return this._appendMessage('user', content);
48+
this._appendMessage('user', content);
5049
};
5150

5251
/**
5352
* @param {string} message - Plain text, escaped before insertion.
54-
* @returns {HTMLElement}
5553
*/
5654
AssistantTranscript.prototype.appendSystemMessage = function (message) {
57-
return this._appendMessage('system', message);
55+
this._appendMessage('system', message);
5856
};
5957

6058
/**
@@ -163,9 +161,6 @@ AssistantTranscript.prototype.reset = function (turns) {
163161
*/
164162
AssistantTranscript.prototype.scrollToBottom = function (force) {
165163
const container = this._container;
166-
if (!container || container.scrollHeight === undefined) {
167-
return;
168-
}
169164
if (force || this._isScrolledToBottom()) {
170165
container.scrollTop = container.scrollHeight;
171166
}
@@ -198,7 +193,7 @@ AssistantTranscript.prototype._appendMessage = function (role, content, showCopy
198193
messageElement.className = 'ai-message message-' + role;
199194

200195
const formattedContent = role === 'assistant' ? this._parseMarkdown(content) : this._escapeHtml(content);
201-
const shouldShowCopyButton = role === 'assistant' && (showCopyButton === undefined || showCopyButton === true);
196+
const shouldShowCopyButton = role === 'assistant' && showCopyButton !== false;
202197
const roleLabel = role === 'user' ? 'You' : role === 'assistant' ? 'AI' : 'System';
203198

204199
// Safe innerHTML: roleLabel is from a fixed set, formattedContent is either escaped or markdown-parsed (which itself escapes anything it does not turn into a known formatting tag).

app/scripts/modules/ai/GeminiNanoProvider.js

Lines changed: 23 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -63,20 +63,15 @@ GeminiNanoProvider.prototype._send = function (message) {
6363
this._port.postMessage(message);
6464
};
6565

66+
const CAPABILITY_STATE_BY_PORT_STATUS = {
67+
'ready': 'ready',
68+
'needs-download': 'downloadable',
69+
'downloading': 'downloading',
70+
'unsupported': 'unsupported'
71+
};
72+
6673
function toCanonicalCapabilityState(portStatus) {
67-
if (portStatus === 'ready') {
68-
return 'ready';
69-
}
70-
if (portStatus === 'needs-download') {
71-
return 'downloadable';
72-
}
73-
if (portStatus === 'downloading') {
74-
return 'downloading';
75-
}
76-
if (portStatus === 'unsupported') {
77-
return 'unsupported';
78-
}
79-
return 'unavailable';
74+
return CAPABILITY_STATE_BY_PORT_STATUS[portStatus] || 'unavailable';
8075
}
8176

8277
function abortError() {
@@ -92,12 +87,7 @@ function messagesEqual(a, b) {
9287
if (!a || !b || a.length !== b.length) {
9388
return false;
9489
}
95-
for (let i = 0; i < a.length; i++) {
96-
if (a[i].role !== b[i].role || a[i].content !== b[i].content) {
97-
return false;
98-
}
99-
}
100-
return true;
90+
return a.every((m, i) => m.role === b[i].role && m.content === b[i].content);
10191
}
10292

10393
/**
@@ -131,25 +121,19 @@ GeminiNanoProvider.prototype.downloadModel = function (onProgress) {
131121
return new Promise((resolve, reject) => {
132122
this._connect();
133123

134-
this._on('download-progress', (message) => {
135-
if (typeof onProgress === 'function') {
136-
onProgress(message.progress);
137-
}
138-
});
139-
140-
this._on('download-complete', () => {
124+
const cleanup = () => {
141125
this._off('download-progress');
142126
this._off('download-complete');
143127
this._off('error');
144-
resolve();
145-
});
128+
};
146129

147-
this._on('error', (message) => {
148-
this._off('download-progress');
149-
this._off('download-complete');
150-
this._off('error');
151-
reject(new Error(message.message));
130+
this._on('download-progress', (message) => {
131+
if (typeof onProgress === 'function') {
132+
onProgress(message.progress);
133+
}
152134
});
135+
this._on('download-complete', () => { cleanup(); resolve(); });
136+
this._on('error', (message) => { cleanup(); reject(new Error(message.message)); });
153137

154138
this._send({ type: 'download-model' });
155139
});
@@ -164,18 +148,17 @@ GeminiNanoProvider.prototype._createSession = function (prefix) {
164148
return new Promise((resolve, reject) => {
165149
this._connect();
166150

167-
this._on('session-created', () => {
151+
const cleanup = () => {
168152
this._off('session-created');
169153
this._off('error');
154+
};
155+
156+
this._on('session-created', () => {
157+
cleanup();
170158
this._sessionPrefix = prefix.slice();
171159
resolve();
172160
});
173-
174-
this._on('error', (message) => {
175-
this._off('session-created');
176-
this._off('error');
177-
reject(new Error(message.message));
178-
});
161+
this._on('error', (message) => { cleanup(); reject(new Error(message.message)); });
179162

180163
this._send({
181164
type: 'create-session',

app/scripts/modules/ai/PromptBuilder.js

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,7 @@ function _stringifyValue(value) {
3838
*/
3939
function _stringifyBindingValue(value) {
4040
const rendered = _stringifyValue(value);
41-
if (rendered.length > BINDING_VALUE_CAP) {
42-
return rendered.substring(0, BINDING_VALUE_CAP) + '...';
43-
}
44-
return rendered;
41+
return rendered.length > BINDING_VALUE_CAP ? rendered.substring(0, BINDING_VALUE_CAP) + '...' : rendered;
4542
}
4643

4744
/**
@@ -112,12 +109,7 @@ PromptBuilder.prototype.buildSystemPrompt = function (appInfo) {
112109
'Bad: "Yes, sap.m.Slider has a `flashOnClick` property that lights it up."\n' +
113110
'Good: "I\'m not certain flashOnClick exists on sap.m.Slider — verify in the API reference."';
114111

115-
const zones = [role];
116-
const appContext = this._buildAppContext(appInfo);
117-
if (appContext) {
118-
zones.push(appContext);
119-
}
120-
zones.push(rules, style, example);
112+
const zones = [role, this._buildAppContext(appInfo), rules, style, example].filter(Boolean);
121113

122114
return zones.join('\n\n');
123115
};
@@ -221,19 +213,11 @@ PromptBuilder.prototype._buildControlContextBlock = function (control) {
221213
'- ID: ' + (control.id || 'None')
222214
];
223215

224-
const sections = [];
225-
const propertiesSection = this._renderPropertiesSection(control.properties);
226-
if (propertiesSection) {
227-
sections.push(propertiesSection);
228-
}
229-
const bindingsSection = this._renderBindingsSection(control.bindings);
230-
if (bindingsSection) {
231-
sections.push(bindingsSection);
232-
}
233-
const aggregationsSection = this._renderAggregationsSection(control.aggregations);
234-
if (aggregationsSection) {
235-
sections.push(aggregationsSection);
236-
}
216+
const sections = [
217+
this._renderPropertiesSection(control.properties),
218+
this._renderBindingsSection(control.bindings),
219+
this._renderAggregationsSection(control.aggregations)
220+
].filter(Boolean);
237221

238222
const contextBody = identityLines.concat(sections).join('\n');
239223
return 'Current UI5 Control Context:\n' + contextBody;
@@ -266,10 +250,8 @@ PromptBuilder.prototype._buildConsoleErrorsBlock = function (consoleErrors) {
266250
* @private
267251
*/
268252
PromptBuilder.prototype._capSection = function (header, body, maxLength) {
269-
if (body.length > maxLength) {
270-
return header + '\n' + body.substring(0, maxLength) + '... [truncated]';
271-
}
272-
return header + '\n' + body;
253+
const capped = body.length > maxLength ? body.substring(0, maxLength) + '... [truncated]' : body;
254+
return header + '\n' + capped;
273255
};
274256

275257
/**
@@ -396,14 +378,13 @@ PromptBuilder.prototype.buildMessages = function (params) {
396378
{ role: 'system', content: this.buildSystemPrompt(p.appInfo) }
397379
];
398380

399-
if (p.history && p.history.length) {
400-
for (let i = 0; i < p.history.length; i++) {
401-
const turn = p.history[i];
402-
if ((turn.role === 'user' || turn.role === 'assistant') && turn.content) {
403-
messages.push({ role: turn.role, content: turn.content });
404-
}
405-
}
406-
}
381+
(p.history || [])
382+
.filter(function (turn) {
383+
return (turn.role === 'user' || turn.role === 'assistant') && turn.content;
384+
})
385+
.forEach(function (turn) {
386+
messages.push({ role: turn.role, content: turn.content });
387+
});
407388

408389
messages.push({
409390
role: 'user',

0 commit comments

Comments
 (0)