Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request shifts DeepWiki towards a local-first architecture by defaulting to Ollama for LLM and embedding services and introducing a local repository type for data processing. It integrates promptfoo for regression testing, accompanied by new VS Code tasks and documentation. Review feedback identifies several hardcoded absolute paths that limit portability across environments. Additionally, there is a potential TypeError in the local file handling logic when repo_url is missing, redundant string replacements in the streaming chat implementation, and a regression in the frontend where language preferences are no longer persisted in local storage.
| @@ -0,0 +1,214 @@ | |||
| const DEFAULT_API_BASE_URL = 'http://127.0.0.1:8001'; | |||
| const DEFAULT_REPO_URL = '/Users/samroku/dev/deepwiki-open'; | |||
There was a problem hiding this comment.
The DEFAULT_REPO_URL is hardcoded to a specific local path (/Users/samroku/dev/deepwiki-open). This will cause the provider to fail for any other user. Consider using an environment variable or a relative path.
| label: deepwiki-local-ollama | ||
| config: | ||
| apiBaseUrl: http://127.0.0.1:8001 | ||
| repoUrl: /Users/samroku/dev/deepwiki-open |
There was a problem hiding this comment.
Hardcoded absolute path detected. This configuration is not portable and will fail on other environments.
| 3. Run `npm run promptfoo:ci` for the full local regression suite | ||
| 4. Optional: inspect the latest run with `npm run promptfoo:view` | ||
|
|
||
| The default eval config lives in `promptfooconfig.yaml` and targets the local repository path `/Users/samroku/dev/deepwiki-open`, so it stays inside your local-first workflow by default. |
There was a problem hiding this comment.
The path /Users/samroku/dev/deepwiki-open is hardcoded. This makes the documentation and the default configuration specific to your local machine. Please use a placeholder or a relative path to ensure it works for other contributors.
| if repo_type == "local" or ( | ||
| repo_url | ||
| and not repo_url.startswith("http://") | ||
| and not repo_url.startswith("https://") | ||
| ): |
There was a problem hiding this comment.
If repo_type is set to "local" but repo_url is missing or null, os.path.abspath(repo_url) will raise a TypeError. A check should be added to ensure repo_url is provided when repo_type is "local".
| if repo_type == "local" or ( | |
| repo_url | |
| and not repo_url.startswith("http://") | |
| and not repo_url.startswith("https://") | |
| ): | |
| if (repo_type == "local" or ( | |
| repo_url | |
| and not repo_url.startswith("http://") | |
| and not repo_url.startswith("https://") | |
| )) and repo_url: |
| text = extract_ollama_stream_text(chunk).replace('<think>', '').replace('</think>', '') | ||
| if text: | ||
| logger.debug("Streaming Ollama chunk with %s chars", len(text)) | ||
| text = text.replace('<think>', '').replace('</think>', '') | ||
| yield text |
There was a problem hiding this comment.
The <think> and </think> tags are being replaced twice. They are already removed during the assignment of text on line 516, making the replacement on line 519 redundant.
| text = extract_ollama_stream_text(chunk).replace('<think>', '').replace('</think>', '') | |
| if text: | |
| logger.debug("Streaming Ollama chunk with %s chars", len(text)) | |
| text = text.replace('<think>', '').replace('</think>', '') | |
| yield text | |
| text = extract_ollama_stream_text(chunk).replace('<think>', '').replace('</think>', '') | |
| if text: | |
| logger.debug("Streaming Ollama chunk with %s chars", len(text)) | |
| yield text |
| text = extract_ollama_stream_text(chunk).replace('<think>', '').replace('</think>', '') | ||
| if text: | ||
| text = text.replace('<think>', '').replace('</think>', '') | ||
| yield text |
There was a problem hiding this comment.
Redundant replacement of <think> tags. The tags are already removed on line 641.
| text = extract_ollama_stream_text(chunk).replace('<think>', '').replace('</think>', '') | |
| if text: | |
| text = text.replace('<think>', '').replace('</think>', '') | |
| yield text | |
| text = extract_ollama_stream_text(chunk).replace('<think>', '').replace('</think>', '') | |
| if text: | |
| yield text |
| const storedLanguage = | ||
| typeof window !== 'undefined' ? detectBrowserLanguage() : 'en'; |
There was a problem hiding this comment.
The removal of localStorage logic for language persistence is a regression. Users will now lose their language preference whenever the page is refreshed or the session ends.
1 participant
No description provided.