Skip to content

local llm #357

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+16 −3
Draft

local llm #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions api/config/embedder.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
"encoding_format": "float"
}
},
"embedder_ollama": {
"client_class": "OllamaClient",
"model_kwargs": {
"model": "nomic-embed-text"
}
},
"retriever": {
"top_k": 20
},
Expand Down
3 changes: 3 additions & 0 deletions api/config/generator.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@
},
"o4-mini": {
"temperature": 1.0
},
"claude-3.7-sonnet": {
"temperature": 1.0
}
}
},
Expand Down
2 changes: 2 additions & 0 deletions api/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import logging
from dotenv import load_dotenv

sys.path.insert(0, os.path.normpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")))
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Manipulating sys.path directly is generally discouraged as it can make dependency management fragile and code harder to understand. It can lead to issues with module resolution, especially in larger projects or when deployed in different environments. A better approach is to structure your project as a proper Python package and use relative imports, or to install it in editable mode (pip install -e .) for development. This ensures that imports are explicit and reliable.


# Load environment variables from .env file
load_dotenv()

Expand Down
2 changes: 1 addition & 1 deletion api/openai_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def __init__(
self._api_key = api_key
self._env_api_key_name = env_api_key_name
self._env_base_url_name = env_base_url_name
self.base_url = base_url or os.getenv(self._env_base_url_name, "https://api.openai.com/v1")
self.base_url = base_url or os.getenv(self._env_base_url_name, "http://10.239.142.219:8012/")
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

Hardcoding a private IP address as a default value is a critical issue. It makes the application non-portable, difficult to configure for other developers or deployment environments, and leaks internal network details. It's much better to use a generic, safe default like localhost or not provide a default at all, forcing the configuration via the OPENAI_BASE_URL environment variable. This ensures the application is secure and easy to set up in any environment.

Suggested change
self.base_url = base_url or os.getenv(self._env_base_url_name, "http://10.239.142.219:8012/")
self.base_url = base_url or os.getenv(self._env_base_url_name, "http://localhost:8012/")

self.sync_client = self.init_sync_client()
self.async_client = None # only initialize if the async call is called
self.chat_completion_parser = (
Expand Down
4 changes: 3 additions & 1 deletion api/websocket_wiki.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,10 @@ async def handle_websocket_chat(websocket: WebSocket):
model_kwargs = {
"model": request.model,
"stream": True,
"temperature": model_config["temperature"]
#"temperature": model_config["temperature"]
}
if "temperature" in model_config:
model_kwargs["temperature"] = model_config["temperature"]
Comment on lines 459 to +465
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

While the logic to conditionally add temperature is correct, the implementation leaves a commented-out line of code. This can be confusing for future maintainers. It's cleaner to initialize the dictionary with common parameters and then conditionally add optional ones. This improves readability and maintainability.

Suggested change
model_kwargs = {
"model": request.model,
"stream": True,
"temperature": model_config["temperature"]
#"temperature": model_config["temperature"]
}
if "temperature" in model_config:
model_kwargs["temperature"] = model_config["temperature"]
model_kwargs = {
"model": request.model,
"stream": True,
}
if "temperature" in model_config:
model_kwargs["temperature"] = model_config["temperature"]

# Only add top_p if it exists in the model config
if "top_p" in model_config:
model_kwargs["top_p"] = model_config["top_p"]
Expand Down
2 changes: 1 addition & 1 deletion src/components/Markdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import ReactMarkdown from 'react-markdown';
import remarkGfm from 'remark-gfm';
import rehypeRaw from 'rehype-raw';
import { Prism as SyntaxHighlighter } from 'react-syntax-highlighter';
import { tomorrow } from 'react-syntax-highlighter/dist/cjs/styles/prism';
import { tomorrow } from 'react-syntax-highlighter/dist/esm/styles/prism';
import Mermaid from './Mermaid';

interface MarkdownProps {
Expand Down