-
Notifications
You must be signed in to change notification settings - Fork 25
Feat/edit chat system prompt #247
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
Changes from all commits
a20c1d7
ad4b1f9
ff061e0
bdcb547
5f8d868
192ab6a
21c89ee
ea40f0b
c70d3ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -455,3 +455,103 @@ async def set_memory_provider(provider: str): | |
| except Exception as e: | ||
| logger.exception("Error setting memory provider") | ||
| raise e | ||
|
|
||
|
|
||
| # Chat Configuration Management Functions | ||
|
|
||
| async def get_chat_config_yaml() -> str: | ||
| """Get chat system prompt as plain text.""" | ||
| try: | ||
| config_path = _find_config_path() | ||
|
|
||
| default_prompt = """You are a helpful AI assistant with access to the user's personal memories and conversation history. | ||
|
|
||
| Use the provided memories and conversation context to give personalized, contextual responses. If memories are relevant, reference them naturally in your response. Be conversational and helpful. | ||
|
|
||
| If no relevant memories are available, respond normally based on the conversation context.""" | ||
|
|
||
| if not os.path.exists(config_path): | ||
| return default_prompt | ||
|
|
||
| with open(config_path, 'r') as f: | ||
| full_config = yaml.safe_load(f) or {} | ||
|
|
||
| chat_config = full_config.get('chat', {}) | ||
| system_prompt = chat_config.get('system_prompt', default_prompt) | ||
|
|
||
| # Return just the prompt text, not the YAML structure | ||
| return system_prompt | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error loading chat config: {e}") | ||
| raise | ||
|
|
||
|
|
||
| async def save_chat_config_yaml(prompt_text: str) -> dict: | ||
| """Save chat system prompt from plain text.""" | ||
| try: | ||
| config_path = _find_config_path() | ||
|
|
||
| # Validate plain text prompt | ||
| if not prompt_text or not isinstance(prompt_text, str): | ||
| raise ValueError("Prompt must be a non-empty string") | ||
|
|
||
| prompt_text = prompt_text.strip() | ||
| if len(prompt_text) < 10: | ||
| raise ValueError("Prompt too short (minimum 10 characters)") | ||
| if len(prompt_text) > 10000: | ||
| raise ValueError("Prompt too long (maximum 10000 characters)") | ||
|
|
||
| # Create chat config dict | ||
| chat_config = {'system_prompt': prompt_text} | ||
|
|
||
| # Load full config | ||
| if os.path.exists(config_path): | ||
| with open(config_path, 'r') as f: | ||
| full_config = yaml.safe_load(f) or {} | ||
| else: | ||
| full_config = {} | ||
|
|
||
| # Backup existing config | ||
| if os.path.exists(config_path): | ||
| backup_path = str(config_path) + '.backup' | ||
| shutil.copy2(config_path, backup_path) | ||
| logger.info(f"Created config backup at {backup_path}") | ||
|
|
||
| # Update chat section | ||
| full_config['chat'] = chat_config | ||
|
|
||
| # Save | ||
| with open(config_path, 'w') as f: | ||
| yaml.dump(full_config, f, default_flow_style=False, allow_unicode=True) | ||
|
|
||
| # Reload config in memory (hot-reload) | ||
| load_models_config(force_reload=True) | ||
|
|
||
| logger.info("Chat configuration updated successfully") | ||
|
|
||
| return {"success": True, "message": "Chat configuration updated successfully"} | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error saving chat config: {e}") | ||
| raise | ||
|
Comment on lines
+535
to
+537
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Improve exception handling to follow project patterns. The exception handler should use Based on learnings, this improves debuggability. 🔎 Proposed fix except Exception as e:
- logger.error(f"Error saving chat config: {e}")
- raise
+ logger.exception("Error saving chat config")
+ raise e🧰 Tools🪛 Ruff (0.14.10)536-536: Use Replace with (TRY400) 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| async def validate_chat_config_yaml(prompt_text: str) -> dict: | ||
| """Validate chat system prompt plain text.""" | ||
| try: | ||
| # Validate plain text prompt | ||
| if not isinstance(prompt_text, str): | ||
| return {"valid": False, "error": "Prompt must be a string"} | ||
|
|
||
| prompt_text = prompt_text.strip() | ||
| if len(prompt_text) < 10: | ||
| return {"valid": False, "error": "Prompt too short (minimum 10 characters)"} | ||
| if len(prompt_text) > 10000: | ||
| return {"valid": False, "error": "Prompt too long (maximum 10000 characters)"} | ||
|
|
||
| return {"valid": True, "message": "Configuration is valid"} | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error validating chat config: {e}") | ||
| return {"valid": False, "error": f"Validation error: {str(e)}"} | ||
|
Comment on lines
+555
to
+557
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Use While the catch-all exception handling is appropriate here (since this is a validation function that returns error details), the logging should use Based on learnings, this improves debuggability. 🔎 Proposed fix except Exception as e:
- logger.error(f"Error validating chat config: {e}")
+ logger.exception("Error validating chat config")
return {"valid": False, "error": f"Validation error: {str(e)}"}🧰 Tools🪛 Ruff (0.14.10)555-555: Do not catch blind exception: (BLE001) 556-556: Use Replace with (TRY400) 557-557: Use explicit conversion flag Replace with conversion flag (RUF010) 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,8 @@ | |
| import logging | ||
| from typing import Optional | ||
|
|
||
| from fastapi import APIRouter, Body, Depends, Request | ||
| from fastapi import APIRouter, Body, Depends, HTTPException, Request | ||
| from fastapi.responses import JSONResponse, Response | ||
| from pydantic import BaseModel | ||
|
|
||
| from advanced_omi_backend.auth import current_active_user, current_superuser | ||
|
|
@@ -128,6 +129,53 @@ async def delete_all_user_memories(current_user: User = Depends(current_active_u | |
| return await system_controller.delete_all_user_memories(current_user) | ||
|
|
||
|
|
||
| # Chat Configuration Management Endpoints | ||
|
|
||
| @router.get("/admin/chat/config", response_class=Response) | ||
| async def get_chat_config(current_user: User = Depends(current_superuser)): | ||
| """Get chat configuration as YAML. Admin only.""" | ||
| try: | ||
| yaml_content = await system_controller.get_chat_config_yaml() | ||
| return Response(content=yaml_content, media_type="text/plain") | ||
| except Exception as e: | ||
| logger.error(f"Failed to get chat config: {e}") | ||
| raise HTTPException(status_code=500, detail=str(e)) | ||
|
Comment on lines
+137
to
+142
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve exception handling to follow project patterns. The exception handling should use 🔎 Proposed fix try:
yaml_content = await system_controller.get_chat_config_yaml()
return Response(content=yaml_content, media_type="text/plain")
except Exception as e:
- logger.error(f"Failed to get chat config: {e}")
- raise HTTPException(status_code=500, detail=str(e))
+ logger.exception("Failed to get chat config")
+ raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools🪛 Ruff (0.14.10)140-140: Do not catch blind exception: (BLE001) 141-141: Use Replace with (TRY400) 142-142: Within an (B904) 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| @router.post("/admin/chat/config") | ||
| async def save_chat_config( | ||
| request: Request, | ||
| current_user: User = Depends(current_superuser) | ||
| ): | ||
| """Save chat configuration from YAML. Admin only.""" | ||
| try: | ||
| yaml_content = await request.body() | ||
| yaml_str = yaml_content.decode('utf-8') | ||
| result = await system_controller.save_chat_config_yaml(yaml_str) | ||
| return JSONResponse(content=result) | ||
| except ValueError as e: | ||
| raise HTTPException(status_code=400, detail=str(e)) | ||
| except Exception as e: | ||
| logger.error(f"Failed to save chat config: {e}") | ||
| raise HTTPException(status_code=500, detail=str(e)) | ||
|
Comment on lines
+151
to
+160
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve exception handling to follow project patterns. Both exception handlers should chain exceptions with 🔎 Proposed fix try:
yaml_content = await request.body()
yaml_str = yaml_content.decode('utf-8')
result = await system_controller.save_chat_config_yaml(yaml_str)
return JSONResponse(content=result)
except ValueError as e:
- raise HTTPException(status_code=400, detail=str(e))
+ raise HTTPException(status_code=400, detail=str(e)) from e
except Exception as e:
- logger.error(f"Failed to save chat config: {e}")
- raise HTTPException(status_code=500, detail=str(e))
+ logger.exception("Failed to save chat config")
+ raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools🪛 Ruff (0.14.10)157-157: Within an (B904) 158-158: Do not catch blind exception: (BLE001) 159-159: Use Replace with (TRY400) 160-160: Within an (B904) 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| @router.post("/admin/chat/config/validate") | ||
| async def validate_chat_config( | ||
| request: Request, | ||
| current_user: User = Depends(current_superuser) | ||
| ): | ||
| """Validate chat configuration YAML. Admin only.""" | ||
| try: | ||
| yaml_content = await request.body() | ||
| yaml_str = yaml_content.decode('utf-8') | ||
| result = await system_controller.validate_chat_config_yaml(yaml_str) | ||
| return JSONResponse(content=result) | ||
| except Exception as e: | ||
| logger.error(f"Failed to validate chat config: {e}") | ||
| raise HTTPException(status_code=500, detail=str(e)) | ||
|
Comment on lines
+169
to
+176
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve exception handling to follow project patterns. The exception handler should use 🔎 Proposed fix try:
yaml_content = await request.body()
yaml_str = yaml_content.decode('utf-8')
result = await system_controller.validate_chat_config_yaml(yaml_str)
return JSONResponse(content=result)
except Exception as e:
- logger.error(f"Failed to validate chat config: {e}")
- raise HTTPException(status_code=500, detail=str(e))
+ logger.exception("Failed to validate chat config")
+ raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools🪛 Ruff (0.14.10)174-174: Do not catch blind exception: (BLE001) 175-175: Use Replace with (TRY400) 176-176: Within an (B904) 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| @router.get("/streaming/status") | ||
| async def get_streaming_status(request: Request, current_user: User = Depends(current_superuser)): | ||
| """Get status of active streaming sessions and Redis Streams health. Admin only.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Improve exception handling to follow project patterns.
The exception handler should use
logging.exception()instead oflogger.error()to capture the full stack trace, and should re-raise withfrom eto preserve exception context.Based on learnings, this improves debuggability.
🔎 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.14.10)
486-486: Use
logging.exceptioninstead oflogging.errorReplace with
exception(TRY400)
🤖 Prompt for AI Agents