fix(similarity): address code review - column-only queries, configurable batch size, WCAG touch targets
- Use column-only query in embeddings overview to reduce memory for 100K+ files - Add embedding_backfill_batch_size config setting (default 50) - Fix WCAG touch target on backfill button (min-height/min-width 44px) - Add inline comment explaining 3 chars/token truncation estimate - Import settings in compute_embedding task for configurable batch size Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+11
-1
@@ -257,7 +257,17 @@ def get_embeddings_overview(
|
||||
}
|
||||
```
|
||||
"""
|
||||
all_files = db.query(FileRecord).order_by(FileRecord.id.desc()).all()
|
||||
# Use column-only query to avoid loading full ORM objects into memory
|
||||
all_files = (
|
||||
db.query(
|
||||
FileRecord.id,
|
||||
FileRecord.original_filename,
|
||||
FileRecord.ocr_text,
|
||||
FileRecord.embedding,
|
||||
)
|
||||
.order_by(FileRecord.id.desc())
|
||||
.all()
|
||||
)
|
||||
|
||||
files_info = []
|
||||
total_with_ocr = 0
|
||||
|
||||
@@ -339,6 +339,13 @@ class Settings(BaseSettings):
|
||||
"Set this below the model's context window (e.g. 8000 for an 8192-token model)."
|
||||
),
|
||||
)
|
||||
embedding_backfill_batch_size: int = Field(
|
||||
default=50,
|
||||
description=(
|
||||
"Maximum number of files to queue for embedding computation per "
|
||||
"backfill run. Keeps the worker and embedding API load bounded."
|
||||
),
|
||||
)
|
||||
|
||||
# Text quality check - AI-based assessment of embedded PDF text
|
||||
enable_text_quality_check: bool = Field(
|
||||
|
||||
@@ -9,6 +9,7 @@ import logging
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from app.celery_app import celery
|
||||
from app.config import settings
|
||||
from app.database import SessionLocal
|
||||
from app.models import FileRecord
|
||||
from app.tasks.retry_config import BaseTaskWithRetry
|
||||
@@ -145,7 +146,7 @@ def backfill_missing_embeddings(self) -> dict:
|
||||
Returns:
|
||||
A dict with the number of tasks ``queued``.
|
||||
"""
|
||||
batch_size = 50 # max files to queue per run
|
||||
batch_size = settings.embedding_backfill_batch_size
|
||||
task_id = self.request.id
|
||||
logger.info("[%s] Backfill: scanning for files missing embeddings (batch_size=%d)", task_id, batch_size)
|
||||
|
||||
|
||||
@@ -59,8 +59,8 @@ def generate_embedding(text: str, model: str | None = None) -> list[float]:
|
||||
model = settings.embedding_model
|
||||
|
||||
# Truncate to stay within the model's context window.
|
||||
# Use a conservative estimate of ~3 characters per token so that the
|
||||
# resulting text fits comfortably within ``embedding_max_tokens``.
|
||||
# Conservative 3 chars/token estimate (actual ratio varies by language;
|
||||
# English averages ~4 chars/token but 3 gives a safety margin).
|
||||
max_chars = settings.embedding_max_tokens * 3
|
||||
if len(text) > max_chars:
|
||||
logger.debug(
|
||||
|
||||
@@ -133,7 +133,7 @@
|
||||
<strong>{{ files_missing_embedding }}</strong> file(s) have OCR text but no embedding yet.
|
||||
The background task will compute them automatically every 5 minutes, or you can
|
||||
<button onclick="triggerBackfill()" id="backfill-btn"
|
||||
style="background: #d97706; color: white; border: none; padding: 0.2rem 0.6rem; border-radius: 0.25rem; cursor: pointer; font-size: 0.8rem;"
|
||||
style="background: #d97706; color: white; border: none; padding: 0.2rem 0.6rem; border-radius: 0.25rem; cursor: pointer; font-size: 0.8rem; min-height: 44px; min-width: 44px;"
|
||||
aria-label="Trigger embedding computation for all files missing embeddings">
|
||||
trigger it now
|
||||
</button>.
|
||||
|
||||
Reference in New Issue
Block a user