feat(knowledge-base): add enabled flag for soft enable/disable

[YonganZhang]

Use case

Currently the only way to "turn off" a knowledge base is to delete it, which loses all indexed documents and embeddings. Users frequently want to temporarily disable a KB (e.g. testing, debugging, A/B comparison) without paying the re-indexing cost later.

Change

Three small additions to the existing schema:

1. KnowledgeBase model gets an enabled: bool = True field.
2. kb_db_sqlite.migrate_to_v2() adds the column to existing DBs via ALTER TABLE ... ADD COLUMN enabled BOOLEAN NOT NULL DEFAULT 1, wrapped in try/except so re-running the migration is a no-op.
3. RetrievalManager.invalidate_sparse_cache(kb_id) exposes a small helper to clear the BM25 cache when a KB is enabled/disabled or its docs change. Useful for the dashboard toggle and any other consumer that mutates KB state.

Backwards compatibility

• New rows default to enabled=True → no behavior change for existing KBs.
• Existing rows get 1 (true) via the migration default → no behavior change.
• Pure additive: no existing field renamed or removed.

Diff size

16 lines across 3 files. Pure model + migration + helper — no API contract change in this PR.

Follow-up

A separate PR can wire this up in the dashboard (toggle button + filter enabled=true in retrieval). Posting this as a small, self-contained schema change first to keep review focused.

Summary by Sourcery

Introduce a soft enable/disable mechanism for knowledge bases and wire supporting schema migration and cache invalidation helper methods.

New Features:

• Add an enabled flag to knowledge bases to support soft enable/disable behavior.
• Expose a retrieval manager helper to invalidate BM25 sparse cache for a specific knowledge base.

Enhancements:

• Add a v2 SQLite migration to backfill the enabled column on existing knowledge base rows without impacting current behavior.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In migrate_to_v2, catching a broad Exception and silently passing can mask real migration issues; consider catching a more specific DB exception (e.g., the 'duplicate column' error) or at least logging unexpected errors while still making re-runs idempotent.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `migrate_to_v2`, catching a broad `Exception` and silently passing can mask real migration issues; consider catching a more specific DB exception (e.g., the 'duplicate column' error) or at least logging unexpected errors while still making re-runs idempotent.

## Individual Comments

### Comment 1
<location path="astrbot/core/knowledge_base/kb_db_sqlite.py" line_range="170-178" />
<code_context>

                 await session.commit()

+    async def migrate_to_v2(self) -> None:
+        """Add enabled column to knowledge_bases table."""
+        async with self.get_db() as session:
+            try:
+                await session.execute(
+                    text("ALTER TABLE knowledge_bases ADD COLUMN enabled BOOLEAN NOT NULL DEFAULT 1")
+                )
+                await session.commit()
+            except Exception:
+                pass  # Column already exists
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid swallowing all exceptions during migration without logging or narrowing the failure mode

Catching a bare `Exception` and then `pass`-ing will hide any migration failure (SQL errors, permission issues, etc.), potentially leaving the DB in an inconsistent state and making problems hard to trace.

Instead, either:
- Catch only the specific error that indicates the column already exists, or
- Log the exception and special-case the "column already exists" scenario.

This keeps the migration idempotent without silently masking real issues.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Avoid swallowing all exceptions during migration without logging or narrowing the failure mode

Catching a bare Exception and then pass-ing will hide any migration failure (SQL errors, permission issues, etc.), potentially leaving the DB in an inconsistent state and making problems hard to trace.

Instead, either:

• Catch only the specific error that indicates the column already exists, or
• Log the exception and special-case the "column already exists" scenario.

This keeps the migration idempotent without silently masking real issues.

[gemini-code-assist]

Code Review

This pull request introduces an enabled field to the knowledge base model and adds a corresponding SQLite migration. It also implements a cache invalidation method for sparse retrievers. However, several critical issues were identified: the migration method is defined but never called, the new cache invalidation logic references a non-existent method in the SparseRetriever class which will lead to runtime errors, and the migration's error handling is overly broad and should be more specific or include logging.

[gemini-code-assist]

The migrate_to_v2 method is defined but never called. To ensure existing databases are updated with the new enabled column, you must invoke this method during the initialization process. Based on the current implementation of KnowledgeBaseManager._init_kb_database in astrbot/core/knowledge_base/kb_mgr.py, a call to await self.kb_db.migrate_to_v2() should be added after migrate_to_v1().

[gemini-code-assist]

The invalidate_sparse_cache method calls self.sparse_retriever.invalidate_cache(kb_id), but the invalidate_cache method is not defined in the SparseRetriever class (see astrbot/core/knowledge_base/retrieval/sparse_retriever.py). This will cause an AttributeError at runtime. Furthermore, the current SparseRetriever implementation does not seem to utilize its _index_cache for BM25 indexing, as it rebuilds the index on every call to _retrieve_with_bm25. You should implement the caching logic and the corresponding invalidation method in SparseRetriever before exposing this helper. Additionally, ensure this new functionality is covered by unit tests.

References

1. New functionality, such as handling attachments, should be accompanied by corresponding unit tests.

[gemini-code-assist]

Catching a broad Exception and silently passing is generally discouraged as it can hide unexpected errors. While this is intended to handle the case where the enabled column already exists (since SQLite's ALTER TABLE doesn't support IF NOT EXISTS), it is safer to catch a more specific exception (e.g., sqlalchemy.exc.OperationalError) and verify the error message. At the very least, consider logging a warning if an exception occurs.