Conversation
Co-authored-by: Fu-Jie <33599649+Fu-Jie@users.noreply.github.com>
Co-authored-by: Fu-Jie <33599649+Fu-Jie@users.noreply.github.com>
Co-authored-by: Fu-Jie <33599649+Fu-Jie@users.noreply.github.com>
Co-authored-by: Fu-Jie <33599649+Fu-Jie@users.noreply.github.com>
Co-authored-by: Fu-Jie <33599649+Fu-Jie@users.noreply.github.com>
Co-authored-by: Fu-Jie <33599649+Fu-Jie@users.noreply.github.com>
Fu-Jie
left a comment
Fu-Jie
left a comment
There was a problem hiding this comment.
def _extract_chat_id(self, body: dict, metadata: Optional[dict]) -> str:
"""Extract chat_id from body or metadata"""
if isinstance(body, dict):
chat_id = body.get("chat_id")
if isinstance(chat_id, str) and chat_id.strip():
return chat_id.strip()
body_metadata = body.get("metadata", {})
if isinstance(body_metadata, dict):
chat_id = body_metadata.get("chat_id")
if isinstance(chat_id, str) and chat_id.strip():
return chat_id.strip()
if isinstance(metadata, dict):
chat_id = metadata.get("chat_id")
if isinstance(chat_id, str) and chat_id.strip():
return chat_id.strip()
return ""
chat_id,统一改为调用这个函数
Co-authored-by: Fu-Jie <33599649+Fu-Jie@users.noreply.github.com>
Updated inlet/outlet to use a shared _extract_chat_id helper per request. Commit abbe3fb. |
- Enhanced error reporting via status bar and console - Robust model ID handling - Open WebUI v0.7.x compatibility (dynamic DB session) - Updated documentation and version bumps
✅ Plugin Version Check / 插件版本检查版本更新检测通过！PR 包含版本变化和更新说明。 Version check passed! PR contains version changes and update description. 版本变化 / Version Changes插件更新 / Plugin Updates
This comment was generated automatically. / 此评论由自动生成。 |
There was a problem hiding this comment.
Pull request overview
This pull request addresses critical database compatibility issues affecting the Async Context Compression filter plugin after Open WebUI upgraded from v0.6.x to v0.7.x. The primary issue is that open_webui.internal.db.Session changed from being a scoped session instance to the SQLAlchemy Session class itself, causing "Could not locate a bind configured" errors. Additionally, the PR adds safety guards for missing metadata and invalid model IDs.
Changes:
- Database compatibility layer with dynamic engine/schema discovery and multi-level session factory fallbacks
- Safety guards for missing chat_id metadata and empty/invalid model IDs
- Enhanced error reporting with status bar notifications for background errors
- Version bump from 1.1.1 to 1.1.2 with updated documentation
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
async_context_compression.py |
Added database discovery functions, context-managed session factory, chat_id extractor, model ID cleaner, and enhanced error handling |
async_context_compression_cn.py |
Partially updated: Added model ID validation and error handling but missing critical database compatibility fixes |
README.md |
Updated version to 1.1.2 and documented new compatibility features |
README_CN.md |
Updated version to 1.1.2 with Chinese documentation of changes |
docs/plugins/filters/index.md |
Updated version number in plugin listing |
docs/plugins/filters/async-context-compression.md |
Updated version badge and added feature bullets |
docs/plugins/filters/async-context-compression.zh.md |
Updated Chinese version badge and features |
Comments suppressed due to low confidence (6)
plugins/filters/async-context-compression/async_context_compression.py:246
- Import of 'model_validator' is not used.
from pydantic import BaseModel, Field, model_validator
plugins/filters/async-context-compression/async_context_compression.py:247
- Import of 'Union' is not used.
from typing import Optional, Dict, Any, List, Union, Callable, Awaitable
plugins/filters/async-context-compression/async_context_compression_cn.py:245
- Import of 'model_validator' is not used.
from pydantic import BaseModel, Field, model_validator
plugins/filters/async-context-compression/async_context_compression_cn.py:246
- Import of 'Union' is not used.
from typing import Optional, Dict, Any, List, Union, Callable, Awaitable
plugins/filters/async-context-compression/async_context_compression_cn.py:249
- Import of 'hashlib' is not used.
import hashlib
plugins/filters/async-context-compression/async_context_compression_cn.py:250
- Import of 'time' is not used.
import time
| with db_context() as session: | ||
| try: | ||
| return session.get_bind() | ||
| except AttributeError: | ||
| return getattr(session, "bind", None) or getattr( | ||
| session, "engine", None | ||
| ) | ||
| except Exception as exc: | ||
| print(f"[DB Discover] get_db_context failed: {exc}") |
There was a problem hiding this comment.
The _discover_owui_engine function attempts to call session.get_bind() which may not exist on all session types. While there is a fallback that uses getattr for bind and engine attributes, the initial try block could fail with an AttributeError that gets caught in the outer exception handler. This can mask the actual problem.
Consider restructuring to avoid calling potentially non-existent methods directly. The inner try-except should catch AttributeError specifically and fall through to the getattr fallback, while the outer exception handler should only catch context manager errors.
| with db_context() as session: | |
| try: | |
| return session.get_bind() | |
| except AttributeError: | |
| return getattr(session, "bind", None) or getattr( | |
| session, "engine", None | |
| ) | |
| except Exception as exc: | |
| print(f"[DB Discover] get_db_context failed: {exc}") | |
| ctx = db_context() | |
| except Exception as exc: | |
| print(f"[DB Discover] get_db_context failed: {exc}") | |
| else: | |
| with ctx as session: | |
| try: | |
| return session.get_bind() | |
| except AttributeError: | |
| return getattr(session, "bind", None) or getattr( | |
| session, "engine", None | |
| ) |
| try: | ||
| from open_webui import env as owui_env | ||
|
|
||
| candidate = getattr(owui_env, "DATABASE_SCHEMA", None) | ||
| if isinstance(candidate, str) and candidate.strip(): | ||
| return candidate.strip() | ||
| except Exception as exc: | ||
| print(f"[DB Discover] env schema lookup failed: {exc}") |
There was a problem hiding this comment.
The table schema discovery logic tries to import from open_webui.env, but this import may fail in environments where the module structure is different. The exception is caught and logged to print, but the error handling should be more specific.
Consider checking if the module exists before attempting the import, or using importlib for more controlled imports. Also, the fallback chain might benefit from a final check that validates the discovered schema is actually used by the database.
| if self._fallback_session_factory is None: | ||
| raise RuntimeError( | ||
| "Open WebUI database session is unavailable. Ensure Open WebUI's database layer is initialized." | ||
| ) |
There was a problem hiding this comment.
The _db_session context manager has a potential issue in the fallback path. If self._fallback_session_factory is None, it raises a RuntimeError, but this check happens after attempting to use the session factories from the db_module. If the db_module factories exist but fail during session creation or usage, the error handling might not provide clear diagnostics about which factory failed.
Consider adding more specific error messages that indicate which fallback level failed (e.g., "get_db_context failed", "SessionLocal factory failed", "fallback session factory unavailable") to help with debugging.
|
|
||
| # Check if table exists using SQLAlchemy inspect | ||
| inspector = inspect(self._db_engine) | ||
| if not inspector.has_table("chat_summary"): |
There was a problem hiding this comment.
The _init_database method checks for table existence using inspector.has_table("chat_summary"), but when a schema is configured (via owui_schema), the table might be in that schema. The has_table method may need a schema parameter to correctly check for the table in the right schema.
Verify that inspector.has_table("chat_summary") correctly finds the table when a schema is configured. You may need to pass the schema explicitly: inspector.has_table("chat_summary", schema=owui_schema).
| if not inspector.has_table("chat_summary"): | |
| # Try to detect the configured schema (if any) from Open WebUI's internal DB module. | |
| # This keeps behavior identical when no schema is configured, while correctly handling | |
| # deployments that use a dedicated schema (e.g. PostgreSQL with owui_schema set). | |
| owui_schema = None | |
| try: | |
| from open_webui.internal.db import schema as owui_schema # type: ignore | |
| except Exception: | |
| owui_schema = None | |
| if owui_schema: | |
| table_exists = inspector.has_table("chat_summary", schema=owui_schema) | |
| else: | |
| table_exists = inspector.has_table("chat_summary") | |
| if not table_exists: |
| """Cleans the model ID by removing whitespace and quotes.""" | ||
| if not model_id: | ||
| return None | ||
| cleaned = model_id.strip().strip('"').strip("'") | ||
| return cleaned if cleaned else None |
There was a problem hiding this comment.
The _clean_model_id method strips both double and single quotes from model IDs, but this could potentially cause issues if a model ID legitimately contains quotes as part of its identifier. While this is unlikely, the current implementation might be too aggressive.
Consider validating that the quotes are actually wrapping the entire string rather than being part of the model ID itself. A more conservative approach would only strip quotes if they appear at both the beginning and end of the string.
| """Cleans the model ID by removing whitespace and quotes.""" | |
| if not model_id: | |
| return None | |
| cleaned = model_id.strip().strip('"').strip("'") | |
| return cleaned if cleaned else None | |
| """Cleans the model ID by removing surrounding whitespace and wrapping quotes.""" | |
| if not model_id: | |
| return None | |
| # First trim surrounding whitespace | |
| cleaned = model_id.strip() | |
| # Only strip quotes if they wrap the entire string, e.g. "model-id" or 'model-id' | |
| if len(cleaned) >= 2 and cleaned[0] == cleaned[-1] and cleaned[0] in ('"', "'"): | |
| cleaned = cleaned[1:-1] | |
| return cleaned or None |
| @@ -249,6 +249,7 @@ | |||
| import json | |||
| import hashlib | |||
There was a problem hiding this comment.
Import of 'hashlib' is not used.
| import hashlib |
| @@ -249,6 +249,7 @@ | |||
| import json | |||
| import hashlib | |||
| import time | |||
There was a problem hiding this comment.
Import of 'time' is not used.
| import time |
Async Context Compression failed on newer Open WebUI builds because
Sessionis no longer a scoped, engine-bound alias; it also crashed on missing metadata and empty summary models.get_db_context/get_db/engine attributes with fallbacks.chat_idmetadata is absent, now centralized through a shared extractor.Example (session usage):
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.