Skip to content

[Plugin] AHDA LITE - Actually helpful digital assistant - control PC with OMI#1116

Merged
kodjima33 merged 7 commits intoBasedHardware:mainfrom
ActuallyAdvanced:main
Oct 21, 2024
Merged

[Plugin] AHDA LITE - Actually helpful digital assistant - control PC with OMI#1116
kodjima33 merged 7 commits intoBasedHardware:mainfrom
ActuallyAdvanced:main

Conversation

@neooriginal
Copy link
Copy Markdown
Collaborator

@neooriginal neooriginal commented Oct 21, 2024

See https://github.com/ActuallyAdvanced/OMI-AHDA

Summary by CodeRabbit

  • New Features

    • Introduced two new plugins: "Gen Z/A Translator" for translating sentences into Gen Z or Gen Alpha slang.
    • Added the "AHDA" plugin for voice control of your PC, enhancing user accessibility.
  • Documentation

    • Updated README with a new section for AHDA setup instructions.
  • Chores

    • Implemented webhooks and configuration endpoints for AHDA integration.
    • Added utility functions for managing AHDA-related configurations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 21, 2024

Caution

Review failed

The head commit changed during the review from f65eb50 to 6ae78e0.

Walkthrough

The changes introduce two new plugins: "Gen Z/A Translator" for translating sentences into Gen Z or Gen Alpha slang, and "AHDA," which enables voice control of a PC. The AHDA plugin includes a FastAPI router for handling webhooks and configurations, along with an HTML interface for user input. Additional utility functions for managing AHDA configuration data are added to the database interactions, and the README file is updated to include setup instructions for the new plugins.

Changes

File Path Change Summary
community-plugins.json Added new plugin entries for "Gen Z/A Translator" and "AHDA" with details including ID, name, author, description, image, and capabilities.
plugins/example/ahda/client.py Introduced FastAPI router with endpoints for webhooks and configuration; added methods for handling webhooks and settings.
plugins/example/ahda/index.html Created new HTML file for AHDA integration with form for user input and JavaScript for handling submissions.
plugins/example/db.py Added functions for storing and retrieving AHDA configuration data in Redis.
plugins/instructions/ahda/README.md Added a setup guide section with a link to the GitHub setup instructions for AHDA.

Poem

In the land of bytes and code,
A voice now guides the load.
AHDA hops with joy and cheer,
Controlling PCs far and near.
With a whisper, tasks are done,
A rabbit's work is never shun! 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Info added
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 12

🧹 Outside diff range and nitpick comments (6)
plugins/example/db.py (1)

136-150: LGTM with minor suggestions for improvement

The new AHDA utility functions are well-implemented and consistent with the existing codebase. However, consider the following minor improvements:

  1. Add error handling to catch potential Redis exceptions.
  2. Use a more specific type hint for the os parameter in store_ahda.
  3. Add a docstring to explain the purpose of these functions.

Here's an example of how you could implement these suggestions:

from typing import Literal

def store_ahda(uid: str, url: str, os: Literal["windows", "mac", "linux"]):
    """
    Store AHDA URL and OS information for a given user ID.
    
    :param uid: User ID
    :param url: AHDA URL
    :param os: Operating system (windows, mac, or linux)
    """
    try:
        r.set(f'ahda_url:{uid}', url)
        r.set(f'ahda_os:{uid}', os)
    except redis.RedisError as e:
        print(f"Error storing AHDA data: {e}")

# Similar improvements for get_ahda_url and get_ahda_os
plugins/example/ahda/index.html (3)

4-7: Enhance SEO and accessibility in the HTML head.

Consider adding a meta description tag for better SEO, and ensure the title is descriptive and unique.

Add the following meta tag within the <head> section:

<meta name="description" content="Configure AHDA Integration for OMI - Control your PC with voice commands">

92-96: Improve form accessibility with labels.

The form inputs lack associated labels, which can impact accessibility for screen reader users.

Modify the form structure as follows:

<form id="ahda-form">
    <label for="url">AHDA URL:</label>
    <input type="text" id="url" name="url" placeholder="Enter AHDA URL" required>
    <label for="os">Operating System:</label>
    <input type="text" id="os" name="os" placeholder="Enter Operating System" required>
    <button type="submit">Save Configuration</button>
</form>

85-98: Enhance accessibility with ARIA attributes.

Add ARIA attributes to improve the structure and meaning of the content for screen readers.

Modify the container div and add roles to key elements:

<div class="container" role="main" aria-labelledby="page-title">
    <div class="logo-container" role="banner">
        <!-- ... logo images ... -->
    </div>
    <p class="plugin-text">AHDA Integration Plugin</p>
    <h1 id="page-title">AHDA Integration</h1>
    <form id="ahda-form" role="form">
        <!-- ... form inputs ... -->
    </form>
    <p id="response-message" role="status" aria-live="polite"></p>
</div>
community-plugins.json (1)

634-658: Summary of AHDA plugin addition

The AHDA plugin has been successfully added to the community-plugins.json file. This new plugin extends the system's capabilities by enabling voice control of PCs, which aligns with the project's goal of enhancing digital assistance.

Key points:

  1. The plugin configuration follows the established structure, maintaining consistency with other entries.
  2. External integration is properly set up with necessary URLs and authentication steps.
  3. The addition doesn't interfere with existing plugin entries.

Recommendations:

  1. Ensure all provided URLs are functional and secure.
  2. Consider adding a brief comment or documentation about the AHDA plugin's specific capabilities and use cases.
  3. Regularly review and update the plugin's configuration as needed, especially if there are changes to the external AHDA project.

As the number of plugins grows, consider implementing a versioning system for the plugins.json file to track changes over time. This could help with backwards compatibility and make it easier to roll back changes if needed.

plugins/example/ahda/client.py (1)

138-143: Declare route handler as asynchronous

For consistency and optimal performance, consider making the route function configure_ahda asynchronous.

Modify the function definition:

-@router.post('/ahda/configure', tags=['ahda'])
-def configure_ahda(uid: str = Form(...), url: str = Form(...), os: str = Form(...)):
+@router.post('/ahda/configure', tags=['ahda'])
+async def configure_ahda(uid: str = Form(...), url: str = Form(...), os: str = Form(...)):

Ensure that any synchronous code within the function is compatible with async context.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between d1639e2 and 5aeca7a.

⛔ Files ignored due to path filters (1)
  • plugins/logos/AHDA.png is excluded by !**/*.png
📒 Files selected for processing (5)
  • community-plugins.json (1 hunks)
  • plugins/example/ahda/client.py (1 hunks)
  • plugins/example/ahda/index.html (1 hunks)
  • plugins/example/db.py (1 hunks)
  • plugins/instructions/ahda/README.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • plugins/instructions/ahda/README.md
🧰 Additional context used
🪛 Ruff
plugins/example/ahda/client.py

2-2: fastapi.responses.JSONResponse imported but unused

Remove unused import: fastapi.responses.JSONResponse

(F401)


6-6: models.RealtimePluginRequest imported but unused

Remove unused import

(F401)


6-6: models.EndpointResponse imported but unused

Remove unused import

(F401)


53-53: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


54-54: Do not perform function call BackgroundTasks in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

🔇 Additional comments (2)
plugins/example/ahda/index.html (1)

1-137: Overall assessment of the AHDA Integration plugin interface.

The HTML file for the AHDA Integration plugin is well-structured and functional. However, there are several areas where improvements can be made:

  1. Enhance accessibility by adding proper labels, ARIA attributes, and improving the semantic structure.
  2. Improve maintainability by moving CSS to an external file and utilizing CSS variables for consistent theming.
  3. Optimize performance by refining the Google Fonts loading strategy.
  4. Enhance JavaScript functionality with better error handling, input validation, and code organization.
  5. Ensure proper server-side validation and sanitization for security.

These improvements will result in a more robust, maintainable, and user-friendly interface for the AHDA Integration plugin.

The current implementation is functional and can be approved, but I strongly recommend addressing the suggested improvements in future iterations to enhance the overall quality and user experience of the plugin.

plugins/example/ahda/client.py (1)

23-23: Verify the model name in ChatOpenAI initialization

The model name 'gpt-4o' may be incorrect. Please verify that 'gpt-4o' is the intended model name.

Comment on lines +11 to +81
<style>
body {
font-family: Assistant, sans-serif;
background-color: #f4f4f4;
margin: 0;
padding: 0;
display: flex;
justify-content: center;
align-items: center;
height: 100vh;
}

.container {
background-color: #fff;
padding: 3rem;
border-radius: 8px;
box-shadow: 0 0 10px rgba(0, 0, 0, 0.1);
text-align: center;
max-width: 400px;
width: 100%;
}

h1 {
margin-bottom: 2rem;
}

.logo-container {
display: flex;
justify-content: center;
gap: 20px;
margin-bottom: 2rem;
}

.logo-container img {
width: 80px;
height: 80px;
object-fit: contain;
}

input[type="text"],
button {
width: 100%;
padding: 10px;
margin-bottom: 1rem;
border: 1px solid #ddd;
border-radius: 4px;
box-sizing: border-box;
}

button {
background-color: #000;
color: #fff;
border: none;
cursor: pointer;
font-size: 1rem;
}

button:hover {
background-color: #333;
}

p {
margin-top: 1rem;
}

.plugin-text {
font-size: 1.2rem;
font-weight: bold;
margin-bottom: 1rem;
}
</style>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve CSS maintainability and performance.

While the current CSS is functional, there are several improvements we can make:

  1. Move CSS to an external file for better separation of concerns and caching.

  2. Use CSS variables for consistent theming and easier maintenance.

  3. Optimize Google Fonts loading for better performance.

  4. Create a new file styles.css and move all CSS there. Then, link it in the HTML:

<link rel="stylesheet" href="styles.css">
  1. In styles.css, add CSS variables for colors and other repeated values:
:root {
    --primary-bg-color: #f4f4f4;
    --secondary-bg-color: #fff;
    --primary-text-color: #000;
    --secondary-text-color: #333;
    --border-color: #ddd;
    --border-radius: 4px;
    --box-shadow: 0 0 10px rgba(0, 0, 0, 0.1);
}

/* Then use these variables in your CSS, e.g.: */
body {
    background-color: var(--primary-bg-color);
}
  1. Optimize Google Fonts loading by adding &display=swap to the font URL and using font-display: swap in your CSS:
<link href="https://fonts.googleapis.com/css2?family=Assistant:wght@200..800&display=swap" rel="stylesheet">
body {
    font-family: Assistant, sans-serif;
    font-display: swap;
}

These changes will improve maintainability, consistency, and performance of your styles.

Comment on lines +100 to +134
<script>
// Extract the uid from the URL query parameters
const urlParams = new URLSearchParams(window.location.search);
const uid = urlParams.get('uid');

if (!uid) {
document.getElementById('response-message').textContent = "UID is missing. Please check the URL.";
document.getElementById('response-message').style.color = 'red';
}

document.getElementById('ahda-form').addEventListener('submit', async function (event) {
event.preventDefault();

const url = document.getElementById('url').value;
const os = document.getElementById('os').value;

const response = await fetch('/ahda/configure', {
method: 'POST',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
},
body: new URLSearchParams({ uid, url, os })
});

const result = await response.json();
const messageElement = document.getElementById('response-message');
if (response.ok) {
messageElement.textContent = result.message;
messageElement.style.color = 'green';
} else {
messageElement.textContent = result.detail || 'An error occurred. Please try again.';
messageElement.style.color = 'red';
}
});
</script>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

Server-side validation for /ahda/configure endpoint is insufficient.

The configure_ahda function does not implement necessary validation or sanitization for user inputs (uid, url, os), which may expose the application to security vulnerabilities.

  • File: plugins/example/ahda/client.py
  • Function: configure_ahda
🔗 Analysis chain

Enhance JavaScript functionality and security.

The current JavaScript implementation is functional, but we can improve its robustness, readability, and security:

  1. Enhance error handling and user feedback.
  2. Improve code organization and readability.
  3. Address potential security concerns.

Consider refactoring the JavaScript as follows:

document.addEventListener('DOMContentLoaded', () => {
    const form = document.getElementById('ahda-form');
    const responseMessage = document.getElementById('response-message');
    const urlInput = document.getElementById('url');
    const osInput = document.getElementById('os');

    const urlParams = new URLSearchParams(window.location.search);
    const uid = urlParams.get('uid');

    if (!uid) {
        displayMessage("UID is missing. Please check the URL.", 'error');
        form.style.display = 'none';
        return;
    }

    form.addEventListener('submit', async (event) => {
        event.preventDefault();
        const url = urlInput.value.trim();
        const os = osInput.value.trim();

        if (!isValidUrl(url)) {
            displayMessage("Please enter a valid URL.", 'error');
            return;
        }

        try {
            const response = await fetch('/ahda/configure', {
                method: 'POST',
                headers: {
                    'Content-Type': 'application/json',
                },
                body: JSON.stringify({ uid, url, os })
            });

            const result = await response.json();

            if (response.ok) {
                displayMessage(result.message, 'success');
                form.reset();
            } else {
                throw new Error(result.detail || 'An error occurred. Please try again.');
            }
        } catch (error) {
            displayMessage(error.message, 'error');
        }
    });

    function displayMessage(message, type) {
        responseMessage.textContent = message;
        responseMessage.style.color = type === 'error' ? 'red' : 'green';
    }

    function isValidUrl(string) {
        try {
            new URL(string);
            return true;
        } catch (_) {
            return false;
        }
    }
});

This refactored version:

  1. Uses event delegation for better performance.
  2. Improves error handling with more specific messages.
  3. Adds URL validation to prevent invalid submissions.
  4. Uses JSON for data submission instead of form-urlencoded for better data handling.
  5. Improves code organization with separate functions for displaying messages and validating URLs.

To ensure the security of the endpoint, please verify that proper server-side validation and sanitization are implemented:

This will help identify if there are any input validation or sanitization measures in place for the /ahda/configure endpoint.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for input validation in the server-side code
rg -i "sanitize|validate|escape" --type py

Length of output: 2611


Script:

#!/bin/bash
# Locate the handler for the '/ahda/configure' endpoint
rg -i "def configure.*" --type py

# Inspect validation and sanitization within the configure handler
rg -A10 "def configure.*" --type py | rg -i "sanitize|validate|escape"

Length of output: 224

Comment thread community-plugins.json Outdated
Comment on lines +632 to +660
},
{
"id": "ahda",
"name": "AHDA",
"author": "Neo",
"description": "Control your PC with your voice",
"image": "/plugins/logos/AHDA.png",
"capabilities": [
"external_integration"
],
"external_integration": {
"triggers_on": "transcript_processed",
"webhook_url": "https://based-hardware--plugins-api.modal.run/ahda/send-webhook",
"setup_completed_url": null,
"setup_instructions_file_path": "/plugins/instructions/ahda/README.md",
"auth_steps": [
{
"name": "Set up and install AHDA on your PC",
"url": "https://github.com/ActuallyAdvanced/OMI-AHDA/blob/main/README.md"
},
{
"name": "Enter URL into OMI",
"url": "https://based-hardware--plugins-api.modal.run/ahda/index"
}
]
},
"deleted": false
}
]
] No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Verify JSON structure integrity

The addition of the new AHDA plugin entry maintains the integrity of the JSON structure. However, there's a minor syntax issue that needs to be addressed.

Please apply the following change to ensure valid JSON syntax:

  }
]
+]

Remove the line number annotation (660~) before the closing square bracket to maintain valid JSON structure.

Committable suggestion was skipped due to low confidence.

Comment on lines +94 to +95

if active_sessions[uid]["timer"]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incomplete conditional blocks

The if statements check for active_sessions[uid]["timer"] but have empty bodies.

Implement the intended logic or remove the empty if statements:

-            if active_sessions[uid]["timer"]:
-                pass
+            if active_sessions[uid]["timer"]:
+                # Cancel the existing timer if necessary
+                # Example: active_sessions[uid]["timer"].cancel()
+                pass  # Replace with appropriate logic

Also applies to: 108-109

Comment on lines +14 to +15
active_sessions = {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential concurrency issues with active_sessions

Using a module-level dictionary for session management can lead to concurrency issues in asynchronous or multi-worker environments.

Consider using an external storage solution:

  • Use an in-memory store like Redis to handle session data safely.
  • Utilize thread-safe data structures or synchronization mechanisms.

@@ -0,0 +1,143 @@
from fastapi import APIRouter, Request, HTTPException, Form, Query, BackgroundTasks, Body
from fastapi.responses import HTMLResponse, FileResponse, JSONResponse
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unused import JSONResponse

The JSONResponse import is not used anywhere in the code.

Apply this diff to remove the unused import:

-from fastapi.responses import HTMLResponse, FileResponse, JSONResponse
+from fastapi.responses import HTMLResponse, FileResponse
📝 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
from fastapi.responses import HTMLResponse, FileResponse, JSONResponse
from fastapi.responses import HTMLResponse, FileResponse
🧰 Tools
🪛 Ruff

2-2: fastapi.responses.JSONResponse imported but unused

Remove unused import: fastapi.responses.JSONResponse

(F401)

Comment on lines +139 to +140
if not uid or not url:
raise HTTPException(status_code=400, detail="Both UID, URL AND OS are required")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure all required parameters are validated

The os parameter is required but not checked in the validation logic.

Apply this diff to include os in the validation:

 def configure_ahda(uid: str = Form(...), url: str = Form(...), os: str = Form(...)):
-    if not uid or not url:
-        raise HTTPException(status_code=400, detail="Both UID, URL AND OS are required")
+    if not uid or not url or not os:
+        raise HTTPException(status_code=400, detail="UID, URL, and OS are all required")
📝 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
if not uid or not url:
raise HTTPException(status_code=400, detail="Both UID, URL AND OS are required")
if not uid or not url or not os:
raise HTTPException(status_code=400, detail="UID, URL, and OS are all required")

async def send_ahda_webhook(
uid: str = Query(...),
data: dict = Body(...),
background_tasks: BackgroundTasks = BackgroundTasks()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid setting default value for background_tasks parameter

The BackgroundTasks parameter should not have a default value. FastAPI injects it automatically.

Apply this diff to correct the parameter definition:

-    background_tasks: BackgroundTasks = BackgroundTasks()
+    background_tasks: BackgroundTasks
📝 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
background_tasks: BackgroundTasks = BackgroundTasks()
background_tasks: BackgroundTasks
🧰 Tools
🪛 Ruff

54-54: Do not perform function call BackgroundTasks in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

chat = ChatOpenAI(model='gpt-4o', temperature=0)

# Use requests to get raw text from URL
prompt = requests.get("https://raw.githubusercontent.com/ActuallyAdvanced/OMI-AHDA/main/prompt.txt").text
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid making network requests at module level

Performing a network request during module import can cause performance issues and unhandled exceptions.

Consider moving the request inside a function with proper error handling:

-# Module-level network request
-prompt = requests.get("https://raw.githubusercontent.com/ActuallyAdvanced/OMI-AHDA/main/prompt.txt").text
+
+# Initialize prompt as None
+prompt = None
+
+def get_prompt():
+    global prompt
+    if prompt is None:
+        try:
+            response = requests.get("https://raw.githubusercontent.com/ActuallyAdvanced/OMI-AHDA/main/prompt.txt")
+            response.raise_for_status()
+            prompt = response.text
+        except requests.RequestException as e:
+            logger.error(f"Error fetching prompt: {e}")
+            prompt = ""  # Default to empty string or handle accordingly
+    return prompt

Then, update the usage in your code:

-                ("system", prompt.replace("{os_name}", ahda_os)),
+                ("system", get_prompt().replace("{os_name}", ahda_os)),
📝 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
prompt = requests.get("https://raw.githubusercontent.com/ActuallyAdvanced/OMI-AHDA/main/prompt.txt").text
# Initialize prompt as None
prompt = None
def get_prompt():
global prompt
if prompt is None:
try:
response = requests.get("https://raw.githubusercontent.com/ActuallyAdvanced/OMI-AHDA/main/prompt.txt")
response.raise_for_status()
prompt = response.text
except requests.RequestException as e:
logger.error(f"Error fetching prompt: {e}")
prompt = "" # Default to empty string or handle accordingly
return prompt

Comment on lines +118 to +129
async def call_chatgpt_to_generate_code(command, uid):
try:
ahda_os = get_ahda_os(uid)
messages = [
("system", prompt.replace("{os_name}",ahda_os)),
("human", command),
]
ai_msg = chat.invoke(messages)
sendToPC(uid, ai_msg)
except Exception as e:
logger.error(f"Error calling ChatGPT-4: {e}")
return {"type": "error", "content": str(e)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid blocking the event loop with synchronous calls

The async function call_chatgpt_to_generate_code calls synchronous functions, which can block the event loop.

Consider making the synchronous calls asynchronous:

  • Use asyncio.to_thread to run blocking calls without blocking the event loop.
  • Replace synchronous HTTP requests with asynchronous ones using httpx or aiohttp.

Example adjustment:

 import requests
+import asyncio

 # ...

 async def call_chatgpt_to_generate_code(command, uid):
     try:
         ahda_os = get_ahda_os(uid)
         messages = [
             ("system", get_prompt().replace("{os_name}", ahda_os)),
             ("human", command),
         ]
-        ai_msg = chat.invoke(messages)
-        sendToPC(uid, ai_msg)
+        ai_msg = await asyncio.to_thread(chat.invoke, messages)
+        await sendToPC(uid, ai_msg)
     except Exception as e:
         logger.error(f"Error calling ChatGPT-4: {e}")
         return {"type": "error", "content": str(e)}

Update sendToPC to be asynchronous and use an async HTTP client:

-import requests
+import httpx

 # ...

-   def sendToPC(uid, response):
+   async def sendToPC(uid, response):
        ahda_url = get_ahda_url(uid)
        if not ahda_url:
            raise ValueError('AHDA URL not configured for this UID')
        payload = {
            'uid': uid,
            'response': response
        }
        try:
-           resp = requests.post(ahda_url + "/recieve", json=payload)
-           resp.raise_for_status()
+           async with httpx.AsyncClient() as client:
+               resp = await client.post(ahda_url + "/recieve", json=payload)
+               resp.raise_for_status()
        except httpx.RequestError as e:
            logger.error(f"Error sending webhook: {e}")
            raise
        return {'message': 'Webhook sent successfully'}

Committable suggestion was skipped due to low confidence.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between bb04abe and 524239a.

📒 Files selected for processing (1)
  • community-plugins.json (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
community-plugins.json (1)

672-672: Previous JSON structure issue resolved

The JSON structure issue mentioned in the previous review comment has been successfully addressed. The file now ends with a single closing square bracket, maintaining a valid JSON structure.

Comment thread community-plugins.json
Comment thread community-plugins.json
Comment on lines +648 to +672
"id": "ahda",
"name": "AHDA",
"author": "Neo",
"description": "Control your PC with your voice",
"image": "/plugins/logos/AHDA.png",
"capabilities": [
"external_integration"
],
"external_integration": {
"triggers_on": "transcript_processed",
"webhook_url": "https://based-hardware--plugins-api.modal.run/ahda/send-webhook",
"setup_completed_url": null,
"setup_instructions_file_path": "/plugins/instructions/ahda/README.md",
"auth_steps": [
{
"name": "Set up and install AHDA on your PC",
"url": "https://github.com/ActuallyAdvanced/OMI-AHDA/blob/main/README.md"
},
{
"name": "Enter URL into OMI",
"url": "https://based-hardware--plugins-api.modal.run/ahda/index"
}
]
},
"deleted": false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

AHDA Plugin Configuration URLs Returning 404

🔗 Analysis chain

Verify AHDA Plugin Configuration URLs

The AHDA plugin configuration looks correct and complete. However, it's important to verify that the provided URLs are accessible and functioning correctly.

Please run the following script to check the validity of the URLs:

This script will perform a HEAD request to each URL to verify their accessibility without downloading the full content.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the URLs provided in the AHDA plugin configuration

echo "Checking webhook URL:"
curl -I https://based-hardware--plugins-api.modal.run/ahda/send-webhook

echo "\nChecking setup instructions:"
curl -I https://github.com/ActuallyAdvanced/OMI-AHDA/blob/main/README.md

echo "\nChecking OMI URL entry:"
curl -I https://based-hardware--plugins-api.modal.run/ahda/index

Length of output: 6322

@mdmohsin7 mdmohsin7 self-requested a review October 21, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants