Fix/add contributing guide #264
Conversation
📝 WalkthroughWalkthroughThis pull request addresses a broken documentation link by introducing a new CONTRIBUTING.md file with contributor guidelines and getting started instructions. It also improves error visibility in the graph API by adding explicit logging for index creation failures, and enhances the embedding service with asynchronous encoding support via thread wrapping. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (4 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CONTRIBUTING.md`:
- Around line 15-16: The CONTRIBUTING.md file has an unclosed fenced code block
and truncated content; close the triple-backtick after the pip install line and
append standard contribution guidance sections (e.g., Creating a feature branch,
Running tests, Submitting pull requests, Code style guidelines) so the doc is
complete and self-contained; update the section around the pip install code
block (the fenced block beginning with ```bash and the pip install -r
requirements.txt line) and add short subsections covering branch workflow, test
command (e.g., pytest), PR submission steps, and a brief code style/conventions
note.
🧹 Nitpick comments (1)
backend/app/services/embedding_service/service.py (1)
40-52: Consider thread-safe lazy initialization for the model property.Since
_encode_syncnow runs in a thread pool viaasyncio.to_thread, concurrent calls toget_embedding/get_embeddingscould race on the lazy model initialization if_modelis stillNone. This could cause the model to be loaded multiple times.While unlikely to cause correctness issues (just wasteful), you could add a lock for thread safety:
🔧 Optional: Add thread-safe initialization
+import threading + class EmbeddingService: """Service for generating embeddings and profile summarization for Weaviate integration""" def __init__(self, model_name: str = MODEL_NAME, device: str = EMBEDDING_DEVICE): """Initialize the embedding service with specified model and LLM""" self.model_name = model_name self.device = device self._model = None self._llm = None + self._model_lock = threading.Lock() logger.info(f"Initializing EmbeddingService with model: {model_name} on device: {device}") `@property` def model(self) -> SentenceTransformer: """Lazy-load embedding model to avoid loading during import""" if self._model is None: - try: - logger.info(f"Loading embedding model: {self.model_name}") - self._model = SentenceTransformer(self.model_name, device=self.device) - logger.info( - f"Model loaded successfully. Embedding dimension: {self._model.get_sentence_embedding_dimension()}") - except Exception as e: - logger.error(f"Error loading model {self.model_name}: {str(e)}") - raise + with self._model_lock: + if self._model is None: # Double-check after acquiring lock + try: + logger.info(f"Loading embedding model: {self.model_name}") + self._model = SentenceTransformer(self.model_name, device=self.device) + logger.info( + f"Model loaded successfully. Embedding dimension: {self._model.get_sentence_embedding_dimension()}") + except Exception as e: + logger.error(f"Error loading model {self.model_name}: {str(e)}") + raise return self._model
| ```bash | ||
| pip install -r requirements.txt |
There was a problem hiding this comment.
Code block is not closed and content appears truncated.
The fenced code block starting at Line 15 is missing its closing ```. Additionally, the contribution guide ends abruptly after the install command—consider adding sections for:
- Creating a feature branch
- Running tests
- Submitting pull requests
- Code style guidelines
📝 Suggested completion
```bash
pip install -r requirements.txt
+```
+
+---
+
+## 📋 Submitting Changes
+
+1. Create a new branch for your changes
+2. Make your changes and commit with clear messages
+3. Push your branch and open a pull request
+4. Ensure all checks pass before requesting review
+
+---
+
+## 🧪 Running Tests
+
+```bash
+pytest
+```
+
+---
+
+Thank you for contributing! 🎉📝 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.
| ```bash | |
| pip install -r requirements.txt |
🤖 Prompt for AI Agents
In `@CONTRIBUTING.md` around lines 15 - 16, The CONTRIBUTING.md file has an
unclosed fenced code block and truncated content; close the triple-backtick
after the pip install line and append standard contribution guidance sections
(e.g., Creating a feature branch, Running tests, Submitting pull requests, Code
style guidelines) so the doc is complete and self-contained; update the section
around the pip install code block (the fenced block beginning with ```bash and
the pip install -r requirements.txt line) and add short subsections covering
branch workflow, test command (e.g., pytest), PR submission steps, and a brief
code style/conventions note.
1 participant
Fixes #208
README referenced CONTRIBUTING.md but the file was missing, causing a 404 error.
Added a basic contribution guide to help new contributors.
Summary by CodeRabbit
Documentation
Chores