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.

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

AI Connectivity Tester command and page #76

Merged
merged 5 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ isn't too helpful for most cases.
- **AI: Search**: Ask the user for a search query, and then navigate to the search results page.
Search results are provided by calculating the cosine similarity between the
query embedding and each indexed embedding.
- **AI: Connectivity Test**: Command to navigate to the AI Connectivity Test page, which runs various tests against the currently selected models.

<!-- end-commands-and-functions -->

Expand Down
2 changes: 2 additions & 0 deletions docs/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ This page is a brief overview of each version.

## Unreleased
- Better logging when SSE events have errors
- Add support for retrieving list of models from openai and ollama providers
- Add a Connectivity Test command and page to test whether an api is working

---
## 0.4.1 (2024-11-15)
Expand Down
8 changes: 8 additions & 0 deletions docs/Commands/AI: Connectivity Test.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
tags: commands
commandName: "AI: Connectivity Test"
commandSummary: "Command to navigate to the AI Connectivity Test page, which runs various tests against the currently selected models."
---

This command redirects the client to a page which runs various tests against the current configuration. It's meant to
locate issues with the currently selected models.
6,971 changes: 6,879 additions & 92 deletions silverbullet-ai.plug.js

Large diffs are not rendered by default.

26 changes: 25 additions & 1 deletion silverbullet-ai.plug.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -139,4 +139,28 @@ functions:
path: src/embeddings.ts:updateSearchPage
events:
- editor:pageLoaded
- editor:pageReloaded
- editor:pageReloaded
readPageConnectivityTest:
path: src/connectivity.ts:readFileConnectivityTest
pageNamespace:
pattern: "🛰️ AI Connectivity Test"
operation: readFile
writePageConnectivityTest:
path: src/connectivity.ts:writeFileConnectivityTest
pageNamespace:
pattern: "🛰️ AI Connectivity Test"
operation: writeFile
getPageMetaConnectivityTest:
path: src/connectivity.ts:getFileMetaConnectivityTest
pageNamespace:
pattern: "🛰️ AI Connectivity Test"
operation: getFileMeta
connectivityTestCommand:
path: src/connectivity.ts:connectivityTestCommand
command:
name: "AI: Connectivity Test"
updateConnectivityTestPage:
path: src/connectivity.ts:updateConnectivityTestPage
events:
- editor:pageLoaded
- editor:pageReloaded
267 changes: 267 additions & 0 deletions src/connectivity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,267 @@
import type { FileMeta } from "@silverbulletmd/silverbullet/types";
import { editor } from "@silverbulletmd/silverbullet/syscalls";
import {
aiSettings,
configureSelectedModel,
configureSelectedEmbeddingModel,
initIfNeeded,
currentEmbeddingProvider,
getSelectedTextModel,
getSelectedImageModel,
getSelectedEmbeddingModel
} from "./init.ts";

const connectivityTestPage = "🛰️ AI Connectivity Test";

export function readFileConnectivityTest(
name: string,
): { data: Uint8Array; meta: FileMeta } {
return {
data: new TextEncoder().encode(""),
meta: {
name,
contentType: "text/markdown",
size: 0,
created: 0,
lastModified: 0,
perm: "ro",
},
};
}

export function getFileMetaConnectivityTest(name: string): FileMeta {
return {
name,
contentType: "text/markdown",
size: -1,
created: 0,
lastModified: 0,
perm: "ro",
};
}

export function writeFileConnectivityTest(
name: string,
): FileMeta {
return getFileMetaConnectivityTest(name);
}

export async function updateConnectivityTestPage() {
const page = await editor.getCurrentPage();
if (page === connectivityTestPage) {
await initIfNeeded();
let text = `# 🛰️ AI Connectivity Test

## Status Overview

`;

// Get currently selected models
const textModel = await getSelectedTextModel();
const imageModel = await getSelectedImageModel();
const embeddingModel = await getSelectedEmbeddingModel();

if (!textModel && !imageModel && !embeddingModel) {
text += `> ⚠️ **No models currently selected**

## Available Models

`;

if (aiSettings.textModels.length > 0) {
text += "### 💬 Text Models\n\n";
aiSettings.textModels.forEach(m => text += `* ${m.name}\n`);
text += "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid assignments within expressions for clarity

Assigning to text within the forEach callback can be confusing and may lead to unintended side effects. Consider refactoring using a loop to improve readability.

Apply this diff to refactor the code:

-aiSettings.textModels.forEach(m => text += `* ${m.name}\n`);
+for (const m of aiSettings.textModels) {
+  text += `* ${m.name}\n`;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
aiSettings.textModels.forEach(m => text += `* ${m.name}\n`);
text += "\n";
for (const m of aiSettings.textModels) {
text += `* ${m.name}\n`;
}
text += "\n";
🧰 Tools
🪛 Biome (1.9.4)

[error] 73-74: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

🪛 GitHub Actions: Deno build test and push

[error] File has formatting issues with inconsistent indentation and line breaks

} else {
text += "### 💬 Text Models\n\n_No text models configured._\n\n";
}

if (aiSettings.imageModels.length > 0) {
text += "### 🎨 Image Models\n\n";
aiSettings.imageModels.forEach(m => text += `* ${m.name}\n`);
text += "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid assignments within expressions for clarity

Similar to the previous comment, assigning to text within the forEach callback can be unclear. Refactor using a loop for better readability.

Apply this diff:

-aiSettings.imageModels.forEach(m => text += `* ${m.name}\n`);
+for (const m of aiSettings.imageModels) {
+  text += `* ${m.name}\n`;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
aiSettings.imageModels.forEach(m => text += `* ${m.name}\n`);
text += "\n";
for (const m of aiSettings.imageModels) {
text += `* ${m.name}\n`;
}
text += "\n";
🧰 Tools
🪛 Biome (1.9.4)

[error] 81-82: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

🪛 GitHub Actions: Deno build test and push

[error] File has formatting issues with inconsistent indentation and line breaks

} else {
text += "### 🎨 Image Models\n\n_No image models configured._\n\n";
}

if (aiSettings.embeddingModels.length > 0) {
text += "### 🔤 Embedding Models\n\n";
aiSettings.embeddingModels.forEach(m => text += `* ${m.name}\n`);
text += "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid assignments within expressions for clarity

Again, avoid assigning to text within the forEach callback. Refactoring will enhance clarity.

Apply this diff:

-aiSettings.embeddingModels.forEach(m => text += `* ${m.name}\n`);
+for (const m of aiSettings.embeddingModels) {
+  text += `* ${m.name}\n`;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
aiSettings.embeddingModels.forEach(m => text += `* ${m.name}\n`);
text += "\n";
for (const m of aiSettings.embeddingModels) {
text += `* ${m.name}\n`;
}
text += "\n";
🧰 Tools
🪛 Biome (1.9.4)

[error] 89-90: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

🪛 GitHub Actions: Deno build test and push

[error] File has formatting issues with inconsistent indentation and line breaks

} else {
text += "### 🔤 Embedding Models\n\n_No embedding models configured._\n\n";
}

text += `## Quick Setup

Use these commands to select your models:

* \`AI: Select Text Model from Config\`
* \`AI: Select Image Model from Config\`
* \`AI: Select Embedding Model from Config\`
`;
} else {
const showModelDetails = (model: any, type: string, emoji: string) => {
text += `## ${emoji} ${type} Configuration

### Model Details

| Setting | Value |
|---------|-------|
| Name | ${model.name} |
| Description | ${model.description || "_No description provided_"} |
| Provider | ${model.provider} |
| Model Name | \`${model.modelName}\` |
| Authentication | ${model.requireAuth ? "Required" : "Not Required"} |
| Secret Name | ${model.secretName ? `\`${model.secretName}\`` : "_Not provided_"} |${model.baseUrl ? `\n| API Endpoint | \`${model.baseUrl}\` |` : ""}

`;
};

if (textModel) {
showModelDetails(textModel, "Text Model", "💬");
text += "\n> 🔄 Starting connectivity tests...\n\n";
await editor.setText(text);

// Test text model connectivity
text += "### 🔌 Provider Setup\n\n";
try {
const provider = await configureSelectedModel(textModel);
text += "> ✅ Provider successfully configured\n\n";

// Test model availability
text += "### 📋 Model Availability\n\n";
try {
const availableModels = await provider.listModels();
if (availableModels.includes(textModel.modelName)) {
text += "> ✅ Selected model is available\n\n";
} else {
text += "> ⚠️ Selected model not found in available models\n\n";
text += "#### Available Models\n\n";
availableModels.forEach(model => text += `* \`${model}\`\n`);
text += "\n";
}
Comment on lines +145 to +146
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid assignments within expressions for clarity

Using assignment within the forEach can lead to confusion. Refactor to a loop to make the code more understandable.

Apply this diff:

-availableModels.forEach(model => text += `* \`${model}\`\n`);
+for (const model of availableModels) {
+  text += `* \`${model}\`\n`;
+}

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome (1.9.4)

[error] 142-143: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

🪛 GitHub Actions: Deno build test and push

[error] File has formatting issues with inconsistent indentation and line breaks

} catch (error) {
text += `> ❌ Failed to fetch available models: ${error}\n\n`;
}

// Test API connectivity
text += "### 🔌 API Connectivity\n\n";
try {
// Test non-streaming API connectivity
text += "#### 📡 Non-Streaming Test\n\n";
const response = await provider.singleMessageChat("This is a connectivity test. Respond with exactly 'CONNECTED' (no quotes, no other text).");
if (response && response.trim() === "CONNECTED") {
text += "> ✅ Successfully connected to API and received expected response\n\n";
} else {
text += "> ⚠️ Connected to API but received unexpected response\n\n";
text += "```diff\n";
text += "- Expected: CONNECTED\n";
text += `+ Received: ${response}\n`;
text += "```\n\n";
text += "_Note: The API is accessible but may not be following instructions precisely._\n\n";
}

// Test streaming API connectivity
text += "#### 📡 Streaming Test\n\n";
try {
const chunks: string[] = [];
const streamingResult = await new Promise<string>((resolve, reject) => {
let fullResponse = "";
provider.chatWithAI({
messages: [{
role: "user",
content: "This is a streaming connectivity test. Respond with exactly 'CONNECTED' (no quotes, no other text)."
}],
stream: true,
onDataReceived: (chunk) => {
console.log("Streaming chunk received:", chunk);
chunks.push(chunk);
fullResponse += chunk;
},
onResponseComplete: (finalResponse) => {
resolve(finalResponse);
}
}).catch(reject);
});

if (streamingResult.trim() === "CONNECTED") {
text += "> ✅ Successfully connected to streaming API and received expected response\n\n";
} else {
text += "> ⚠️ Connected to streaming API but received unexpected response\n\n";
text += "```diff\n";
text += "- Expected: CONNECTED\n";
text += `+ Received: ${streamingResult}\n`;
text += "```\n\n";
text += "_Note: The streaming API is accessible but may not be following instructions precisely._\n\n";
}

text += "Received chunks: \n```\n";
chunks.forEach((chunk, index) => {
text += `Chunk ${index + 1}: "${chunk}"\n`;
});
text += "```\n\n\n";
} catch (streamError) {
text += `> ❌ Failed to connect to streaming API: ${streamError}\n\n`;
text += "**Troubleshooting Tips:**\n\n";
text += "* Verify your provider supports streaming\n";
text += "* Ensure there isn't a proxy affecting streaming\n\n";
}
} catch (error) {
text += `> ❌ Failed to connect to API: ${error}\n\n`;
text += "**Troubleshooting Tips:**\n\n";
text += "* Check your API key if needed\n";
text += "* Ensure the API endpoint is accessible\n";
text += "* Check if you have exceeded API rate limits\n";
text += "* Verify you are not using https on silverbullet and connecting to regular http for the api endpoint\n\n";
}
} catch (error) {
text += `> **error** ⚠️ Failed to configure provider: ${error}\n\n`;
}
}

if (imageModel) {
showModelDetails(imageModel, "Image Model", "🎨");
text += "> ℹ️ Image generation testing is disabled to avoid unnecessary API usage\n\n";
}

if (embeddingModel) {
showModelDetails(embeddingModel, "Embedding Model", "🔤");

// Test embedding model connectivity
text += "### 🔌 Embedding Provider Setup\n\n";
try {
await configureSelectedEmbeddingModel(embeddingModel);
text += "> ✅ Embedding provider successfully configured\n\n";

// Test embedding generation
text += "### 🧮 Embedding Generation\n\n";
try {
const testText = "This is a connectivity test.";
const embeddings = await currentEmbeddingProvider.generateEmbeddings({ text: testText });
if (embeddings && embeddings.length > 0) {
text += "> ✅ Successfully generated embeddings\n\n";
text += `\`\`\`\nGenerated ${embeddings.length}-dimensional embedding vector\n\`\`\`\n\n`;
} else {
text += "> ⚠️ Connected to API but received empty embeddings\n\n";
}
} catch (error) {
text += `> ❌ Failed to generate embeddings: ${error}\n\n`;
}
} catch (error) {
text += `> ❌ Failed to configure embedding provider: ${error}\n\n`;
}
}
}

await editor.setText(text);
}
}

/**
* Command to navigate to the AI Connectivity Test page, which runs various tests against the currently selected models.
*/
export async function connectivityTestCommand() {
await initIfNeeded();
await editor.navigate({ page: connectivityTestPage });
}
2 changes: 2 additions & 0 deletions src/interfaces/Provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface ProviderInterface {
systemPrompt?: string,
enrichMessages?: boolean,
) => Promise<string>;
listModels: () => Promise<string[]>;
}

export abstract class AbstractProvider implements ProviderInterface {
Expand All @@ -44,6 +45,7 @@ export abstract class AbstractProvider implements ProviderInterface {
}

abstract chatWithAI(options: StreamChatOptions): Promise<any>;
abstract listModels(): Promise<string[]>;

async streamChatIntoEditor(
options: StreamChatOptions,
Expand Down
9 changes: 9 additions & 0 deletions src/mocks/mockproviders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ export class MockProvider extends AbstractProvider {
}
return mockResponse;
}

async listModels(): Promise<string[]> {
return [
"mock-gpt-3.5",
"mock-gpt-4",
"mock-claude-2",
this.modelName // Include the currently configured model
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing comma and adjust comment placement

There's a missing comma after this.modelName, which can cause a syntax error. Also, the inline comment should be placed correctly to avoid confusion.

Apply this diff to fix the issue:

         "mock-claude-2",
-        this.modelName // Include the currently configured model
+        this.modelName, // Include the currently configured model

Alternatively, move the comment to its own line:

         "mock-claude-2",
+        // Include the currently configured model
         this.modelName,

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 GitHub Actions: Deno build test and push

[error] 35-35: Missing comma after this.modelName in comment line

];
}
}

export class MockImageProvider extends AbstractImageProvider {
Expand Down
Loading
Loading