Skip to content

Addressing PR comments #7

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

Merged
raux merged 3 commits into from
Feb 23, 2026
Merged

Addressing PR comments #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7abe9a0
Initial plan
Copilot Feb 23, 2026
25fa112
Add Test Connection button to verify LM Studio URL connectivity
Copilot Feb 23, 2026
0683c29
Address code review: add URL validation and aria-labels for accessibi…
Copilot Feb 23, 2026
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
11 changes: 11 additions & 0 deletions client/css/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,14 @@ textarea {

#connectionStatus.connected { background: #d4edda; border-color: #28a745; color: #155724; }
#connectionStatus.disconnected { background: #f8d7da; border-color: #dc3545; color: #721c24; }

/* ---- Connection test status ------------------------------- */
#connectionTestStatus {
font-size: 13px;
padding: 4px 8px;
border-radius: 4px;
}

#connectionTestStatus.connection-testing { color: #856404; background: #fff3cd; }
#connectionTestStatus.connection-success { color: #155724; background: #d4edda; }
#connectionTestStatus.connection-error { color: #721c24; background: #f8d7da; }
4 changes: 3 additions & 1 deletion client/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,15 @@ <h2>Prompts and outputs</h2>
<div id="output"></div>

<hr>
<div style="margin:6px 0; display:flex; flex-wrap:wrap; gap:8px;">
<div style="margin:6px 0; display:flex; flex-wrap:wrap; gap:8px; align-items:center;">
<label>LM Studio URL:
<input id="baseUrlInput" type="text" size="40" placeholder="localhost:1234/v1">
</label>
<label>Model:
<input id="modelInput" type="text" size="40" placeholder="qwen/qwen2.5-coder-3b-instruct">
</label>
<button id="btnTestConnection" title="Test connection to LM Studio" aria-label="Test connection to LM Studio">Test Connection</button>
<span id="connectionTestStatus" aria-live="polite"></span>
</div>

<!-- Cytoscape.js (graph library) -->
Expand Down
19 changes: 19 additions & 0 deletions client/js/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,17 @@ document.getElementById('btnImportJson').addEventListener('click', function () {
vscode.postMessage({ command: 'importJSON' });
});

document.getElementById('btnTestConnection').addEventListener('click', function () {
const baseUrl = document.getElementById('baseUrlInput').value || '';
const model = document.getElementById('modelInput').value || '';
const statusEl = document.getElementById('connectionTestStatus');
if (statusEl) {
statusEl.textContent = 'Testing...';
statusEl.className = 'connection-testing';
}
vscode.postMessage({ command: 'testConnection', baseUrl: baseUrl, model: model });
});

/* =============================================================
3. General helpers
============================================================= */
Expand Down Expand Up @@ -264,6 +275,14 @@ window.addEventListener('message', function (event) {
if (modelInput && message.LLMmodel) { modelInput.value = message.LLMmodel; }
}

if (message.command === 'connectionTestResult') {
const statusEl = document.getElementById('connectionTestStatus');
if (statusEl) {
statusEl.textContent = message.message;
statusEl.className = message.success ? 'connection-success' : 'connection-error';
}
}

if (message.command === 'leafSimilarities') {
renderSimilarities(message.node, message.similarities);
renderLizardMetrics(message.node);
Expand Down
34 changes: 33 additions & 1 deletion media/webview.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,17 @@
.swatch.other {
background: var(--color-other);
}

/* Connection test status */
#connectionTestStatus {
font-size: 13px;
padding: 4px 8px;
border-radius: 4px;
}

#connectionTestStatus.connection-testing { color: #856404; background: #fff3cd; }
#connectionTestStatus.connection-success { color: #155724; background: #d4edda; }
#connectionTestStatus.connection-error { color: #721c24; background: #f8d7da; }
</style>

</head>
Expand Down Expand Up @@ -218,13 +229,15 @@ <h2>Code metrics</h2>
<h2>Prompts and outputs</h2>
<div id="output"></div>
<hr>
<div style="margin:6px 0; display:flex; flex-wrap:wrap; gap:8px;">
<div style="margin:6px 0; display:flex; flex-wrap:wrap; gap:8px; align-items:center;">
<label>LM_Studio URL:
<input id="baseUrlInput" type="text" size="40" placeholder="http://localhost:1234/v1" />
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The placeholder value includes the protocol prefix "http://localhost:1234/v1", but the URL validation regex at line 563 in extension.ts expects URLs without the protocol (host:port/path pattern). This will cause the validation to fail if a user copies the placeholder value. The placeholder should be changed to "localhost:1234/v1" to match the expected format.

Suggested change
<input id="baseUrlInput" type="text" size="40" placeholder="http://localhost:1234/v1" />
<input id="baseUrlInput" type="text" size="40" placeholder="localhost:1234/v1" />

Copilot uses AI. Check for mistakes.
</label>
<label>Model:
<input id="modelInput" type="text" size="40" placeholder="qwen/qwen2.5-coder-3b-instruct" />
</label>
<button id="btnTestConnection" title="Test connection to LM Studio" aria-label="Test connection to LM Studio">Test Connection</button>
<span id="connectionTestStatus" aria-live="polite"></span>
</div>

<script>
Expand Down Expand Up @@ -313,6 +326,17 @@ <h2>Prompts and outputs</h2>
vscode.postMessage({ command: 'importJSON' });
});

document.getElementById('btnTestConnection').addEventListener('click', () => {
const baseUrl = document.getElementById('baseUrlInput').value || '';
const model = document.getElementById('modelInput').value || '';
const statusEl = document.getElementById('connectionTestStatus');
if (statusEl) {
statusEl.textContent = 'Testing...';
statusEl.className = 'connection-testing';
}
vscode.postMessage({ command: 'testConnection', baseUrl, model });
});

function sendPrompt() {
const prompt = document.getElementById('prompt').value;
const code = document.getElementById('code').value;
Expand Down Expand Up @@ -445,6 +469,14 @@ <h2>Prompts and outputs</h2>
if (modelInput && message.LLMmodel) modelInput.value = message.LLMmodel;
}

if (message.command === 'connectionTestResult') {
const statusEl = document.getElementById('connectionTestStatus');
if (statusEl) {
statusEl.textContent = message.message;
statusEl.className = message.success ? 'connection-success' : 'connection-error';
}
}

if (message.command === 'leafSimilarities') {
// message.node: comlete node selected
// message.similarities: [{ id, similarity }, ...]
Expand Down
43 changes: 43 additions & 0 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,49 @@ export function activate(context: vscode.ExtensionContext) {
return;
}

if (message.command === 'testConnection') {
const testUrl = message.baseUrl || baseUrl;
const testModel = message.model || LLMmodel;
bonsaiLog('Testing connection to:', testUrl, 'with model:', testModel);
panel.webview.postMessage({ command: 'loading', text: 'Testing connection...' });

try {
// Validate URL format (should be host:port/path pattern)
if (!/^[\w.-]+(:\d+)?(\/[\w./]*)?$/.test(testUrl)) {
throw new Error('Invalid URL format. Expected format: host:port/path (e.g., localhost:1234/v1)');
}

const res = await fetch(`http://${testUrl}/chat/completions`, {
Comment on lines +562 to +567
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The URL validation regex rejects URLs that contain the protocol prefix. However, looking at extension.ts line 899, there's code that uses a default URL with the "http://" protocol prefix ("http://10.184.201.141:1234/v1"). The regex should either be updated to optionally accept the protocol prefix, or all code should be consistent about expecting URLs without the protocol. Consider updating the regex to: /^(https?://)?[\w.-]+(:\d+)?(/[\w./]*)?$/ and strip the protocol before prepending "http://" in the fetch call.

Suggested change
// Validate URL format (should be host:port/path pattern)
if (!/^[\w.-]+(:\d+)?(\/[\w./]*)?$/.test(testUrl)) {
throw new Error('Invalid URL format. Expected format: host:port/path (e.g., localhost:1234/v1)');
}
const res = await fetch(`http://${testUrl}/chat/completions`, {
// Validate URL format (allows optional http/https protocol and host:port/path pattern)
if (!/^(https?:\/\/)?[\w.-]+(:\d+)?(\/[\w./]*)?$/.test(testUrl)) {
throw new Error('Invalid URL format. Expected format: host:port/path (e.g., localhost:1234/v1) or with protocol (e.g., http://localhost:1234/v1)');
}
const normalizedUrl = testUrl.replace(/^https?:\/\//, '');
const res = await fetch(`http://${normalizedUrl}/chat/completions`, {

Copilot uses AI. Check for mistakes.
method: 'POST',
headers: {
'Content-Type': 'application/json',
'Authorization': 'Bearer lm-studio',
},
body: JSON.stringify({
model: testModel,
messages: [{ role: 'user', content: 'Say "connected" in one word.' }],
temperature: 0,
max_tokens: 10,
stream: false,
}),
});

Comment on lines +567 to +581
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The connection test fetch request has no timeout configured. If the LM Studio server is unreachable or not responding, the user may have to wait for the default fetch timeout (which can be very long). Consider adding an AbortController with a reasonable timeout (e.g., 10 seconds) to provide better user experience.

Suggested change
const res = await fetch(`http://${testUrl}/chat/completions`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'Authorization': 'Bearer lm-studio',
},
body: JSON.stringify({
model: testModel,
messages: [{ role: 'user', content: 'Say "connected" in one word.' }],
temperature: 0,
max_tokens: 10,
stream: false,
}),
});
const controller = new AbortController();
const timeoutMs = 10_000;
const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
const res = await (async () => {
try {
return await fetch(`http://${testUrl}/chat/completions`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'Authorization': 'Bearer lm-studio',
},
body: JSON.stringify({
model: testModel,
messages: [{ role: 'user', content: 'Say "connected" in one word.' }],
temperature: 0,
max_tokens: 10,
stream: false,
}),
signal: controller.signal,
});
} finally {
clearTimeout(timeoutId);
}
})();

Copilot uses AI. Check for mistakes.
if (!res.ok) {
const text = await res.text().catch(() => '');
throw new Error(`HTTP ${res.status} ${res.statusText}${text ? ` - ${text}` : ''}`);
}

const json: any = await res.json();
const output = json?.choices?.[0]?.message?.content?.trim?.() ?? '';
bonsaiLog('Connection test successful. Response:', output);
panel.webview.postMessage({ command: 'connectionTestResult', success: true, message: `✓ Connected! Model responded: "${output}"` });
} catch (err: any) {
bonsaiLog('Connection test failed:', err?.message || err);
panel.webview.postMessage({ command: 'connectionTestResult', success: false, message: `✗ Connection failed: ${err?.message || err}` });
}
return;
}
Comment on lines +555 to +596
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The testConnection handler code in extension.ts (lines 555-596) is nearly identical to the code in server.ts (lines 493-534). This duplication makes maintenance harder and increases the risk of bugs when changes are made to one but not the other. Consider extracting this logic into a shared utility function that both files can use, or at minimum document this duplication so future maintainers are aware they need to update both places.

Copilot uses AI. Check for mistakes.

if (message.command === 'generate') {
const selectedNodeIdForPrompt = selectedNodeId;
let code = message.code;
Expand Down
43 changes: 43 additions & 0 deletions src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,49 @@ async function handleMessage(message: any): Promise<void> {
bonsaiLog('Config updated – baseUrl:', baseUrl, 'model:', LLMmodel);
return;
}

if (message.command === 'testConnection') {
const testUrl = message.baseUrl || baseUrl;
const testModel = message.model || LLMmodel;
bonsaiLog('Testing connection to:', testUrl, 'with model:', testModel);
broadcast({ command: 'loading', text: 'Testing connection...' });

try {
// Validate URL format (should be host:port/path pattern)
if (!/^[\w.-]+(:\d+)?(\/[\w./]*)?$/.test(testUrl)) {
throw new Error('Invalid URL format. Expected format: host:port/path (e.g., localhost:1234/v1)');
}

const res = await fetch(`http://${testUrl}/chat/completions`, {
Comment on lines +500 to +505
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The URL validation regex rejects URLs that contain the protocol prefix. However, the media/webview.html placeholder shows "http://localhost:1234/v1" (with protocol). The regex should either be updated to optionally accept the protocol prefix, or all code should be consistent about expecting URLs without the protocol. Consider updating the regex to: /^(https?://)?[\w.-]+(:\d+)?(/[\w./]*)?$/ and strip the protocol before prepending "http://" in the fetch call.

Suggested change
// Validate URL format (should be host:port/path pattern)
if (!/^[\w.-]+(:\d+)?(\/[\w./]*)?$/.test(testUrl)) {
throw new Error('Invalid URL format. Expected format: host:port/path (e.g., localhost:1234/v1)');
}
const res = await fetch(`http://${testUrl}/chat/completions`, {
// Normalize and validate URL format (accept optional protocol, ensure host:port/path pattern)
const normalizedTestUrl = testUrl.replace(/^https?:\/\//, '');
if (!/^(https?:\/\/)?[\w.-]+(:\d+)?(\/[\w./]*)?$/.test(testUrl)) {
throw new Error('Invalid URL format. Expected format similar to: localhost:1234/v1 or http://localhost:1234/v1');
}
const res = await fetch(`http://${normalizedTestUrl}/chat/completions`, {

Copilot uses AI. Check for mistakes.
method: 'POST',
headers: {
'Content-Type': 'application/json',
'Authorization': 'Bearer lm-studio',
},
body: JSON.stringify({
model: testModel,
messages: [{ role: 'user', content: 'Say "connected" in one word.' }],
temperature: 0,
max_tokens: 10,
stream: false,
}),
});
Comment on lines +505 to +518
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The connection test fetch request has no timeout configured. If the LM Studio server is unreachable or not responding, the user may have to wait for the default fetch timeout (which can be very long). Consider adding an AbortController with a reasonable timeout (e.g., 10 seconds) to provide better user experience.

Copilot uses AI. Check for mistakes.

if (!res.ok) {
const text = await res.text().catch(() => '');
throw new Error(`HTTP ${res.status} ${res.statusText}${text ? ` - ${text}` : ''}`);
}

const json: any = await res.json();
const output = json?.choices?.[0]?.message?.content?.trim?.() ?? '';
bonsaiLog('Connection test successful. Response:', output);
broadcast({ command: 'connectionTestResult', success: true, message: `✓ Connected! Model responded: "${output}"` });
} catch (err: any) {
bonsaiLog('Connection test failed:', err?.message || err);
broadcast({ command: 'connectionTestResult', success: false, message: `✗ Connection failed: ${err?.message || err}` });
}
return;
}
}

// ---------------------------------------------------------------------------
Expand Down