fix(translate): CR1-CR2 migration + H1-H4 code review bugs
CR1: Add needs_review, language_overridden to translation_languages migration CR2: Add per_language_metrics to translation_metric_snapshots migration H1: BulkFindReplaceService.preview() now uses replacement_text (not find pattern) H2: get_run_records now includes per-language data via selectinload H3: Preview per-language approve/reject uses correct (jobId, rowId, lang) args H4: _compute_config_hash now includes target_languages
This commit is contained in:
@@ -0,0 +1,37 @@
|
||||
"""add missing columns needs_review language_overridden per_language_metrics
|
||||
|
||||
Revision ID: c4a3a2f74bfe
|
||||
Revises: ed310b33f02c
|
||||
Create Date: 2026-05-14 17:30:36.865269
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision: str = 'c4a3a2f74bfe'
|
||||
down_revision: Union[str, Sequence[str], None] = 'ed310b33f02c'
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
# CR1: needs_review + language_overridden on translation_languages
|
||||
op.add_column('translation_languages', sa.Column('needs_review', sa.Boolean(), nullable=True,
|
||||
comment='Flagged because source language could not be determined'))
|
||||
op.add_column('translation_languages', sa.Column('language_overridden', sa.Boolean(), nullable=True,
|
||||
comment='User manually overrode the auto-detected source language'))
|
||||
|
||||
# CR1b: needs_review on translation_preview_languages
|
||||
op.add_column('translation_preview_languages', sa.Column('needs_review', sa.Boolean(), nullable=True,
|
||||
comment='Flagged because source language could not be determined'))
|
||||
|
||||
# CR2: per_language_metrics on translation_metric_snapshots
|
||||
op.add_column('translation_metric_snapshots', sa.Column('per_language_metrics', sa.JSON(), nullable=True,
|
||||
comment='Per-language cumulative metrics: {lang: {cumulative_tokens, cumulative_cost, runs}}'))
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_column('translation_metric_snapshots', 'per_language_metrics')
|
||||
op.drop_column('translation_preview_languages', 'needs_review')
|
||||
op.drop_column('translation_languages', 'language_overridden')
|
||||
op.drop_column('translation_languages', 'needs_review')
|
||||
@@ -507,6 +507,7 @@ async def bulk_find_replace(
|
||||
run_id=run_id,
|
||||
pattern=payload.find_pattern,
|
||||
is_regex=payload.is_regex,
|
||||
replacement_text=payload.replacement_text,
|
||||
target_language=payload.target_language,
|
||||
)
|
||||
return {
|
||||
|
||||
@@ -548,9 +548,10 @@ class TestTranslationOrchestrator:
|
||||
record.status = "SUCCESS"
|
||||
record.error_message = None
|
||||
record.created_at = datetime.now(UTC)
|
||||
record.languages = []
|
||||
|
||||
db.query.return_value.filter.return_value.count.return_value = 1
|
||||
db.query.return_value.filter.return_value.order_by.return_value.offset.return_value.limit.return_value.all.return_value = [record]
|
||||
db.query.return_value.options.return_value.filter.return_value.count.return_value = 1
|
||||
db.query.return_value.options.return_value.filter.return_value.order_by.return_value.offset.return_value.limit.return_value.all.return_value = [record]
|
||||
|
||||
orch = TranslationOrchestrator(db, config_manager, "test-user")
|
||||
result = orch.get_run_records("run-1")
|
||||
|
||||
@@ -23,7 +23,7 @@ from collections.abc import Callable
|
||||
from datetime import UTC, datetime
|
||||
from typing import Any
|
||||
|
||||
from sqlalchemy.orm import Session
|
||||
from sqlalchemy.orm import Session, selectinload
|
||||
|
||||
from ...core.config_manager import ConfigManager
|
||||
from ...core.logger import belief_scope, logger
|
||||
@@ -827,8 +827,10 @@ class TranslationOrchestrator:
|
||||
status_filter: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
with belief_scope("TranslationOrchestrator.get_run_records"):
|
||||
query = self.db.query(TranslationRecord).filter(
|
||||
TranslationRecord.run_id == run_id
|
||||
query = (
|
||||
self.db.query(TranslationRecord)
|
||||
.options(selectinload(TranslationRecord.languages))
|
||||
.filter(TranslationRecord.run_id == run_id)
|
||||
)
|
||||
|
||||
if status_filter:
|
||||
@@ -856,6 +858,17 @@ class TranslationOrchestrator:
|
||||
"status": r.status,
|
||||
"error_message": r.error_message,
|
||||
"created_at": r.created_at.isoformat() if r.created_at else None,
|
||||
"languages": [
|
||||
{
|
||||
"language_code": tl.language_code,
|
||||
"translated_value": tl.translated_value,
|
||||
"final_value": tl.final_value or tl.translated_value,
|
||||
"source_language_detected": tl.source_language_detected,
|
||||
"status": tl.status,
|
||||
"needs_review": tl.needs_review or False,
|
||||
}
|
||||
for tl in (r.languages or [])
|
||||
],
|
||||
}
|
||||
for r in records
|
||||
],
|
||||
@@ -995,6 +1008,7 @@ class TranslationOrchestrator:
|
||||
"translation_column": job.translation_column,
|
||||
"context_columns": job.context_columns,
|
||||
"target_language": job.target_language,
|
||||
"target_languages": sorted(job.target_languages) if job.target_languages else [],
|
||||
"provider_id": job.provider_id,
|
||||
"batch_size": job.batch_size,
|
||||
"upsert_strategy": job.upsert_strategy,
|
||||
|
||||
@@ -865,6 +865,7 @@ class BulkFindReplaceService:
|
||||
run_id: str,
|
||||
pattern: str,
|
||||
is_regex: bool,
|
||||
replacement_text: str,
|
||||
target_language: str,
|
||||
) -> list[dict]:
|
||||
"""Scan TranslationLanguage entries and return matching items without applying changes."""
|
||||
@@ -888,7 +889,7 @@ class BulkFindReplaceService:
|
||||
source_term = record.source_sql if record else ""
|
||||
|
||||
new_value = compiled.sub(
|
||||
BulkFindReplaceService._get_replacement(pattern, is_regex),
|
||||
replacement_text,
|
||||
current_value,
|
||||
)
|
||||
if new_value != current_value:
|
||||
@@ -904,10 +905,9 @@ class BulkFindReplaceService:
|
||||
return preview_items
|
||||
|
||||
@staticmethod
|
||||
def _get_replacement(pattern: str, is_regex: bool) -> str:
|
||||
def _get_replacement(replacement_text: str, is_regex: bool) -> str:
|
||||
"""Return replacement pattern (for regex, use backrefs; for plain, use literal)."""
|
||||
# The replacement_text is passed through at apply time
|
||||
return pattern # Placeholder — actual replacement is done in apply()
|
||||
return replacement_text
|
||||
|
||||
@staticmethod
|
||||
def apply(
|
||||
|
||||
@@ -420,7 +420,7 @@
|
||||
<div class="flex gap-1 mt-1 flex-wrap">
|
||||
{#if langStatus !== 'approved' && langStatus !== 'edited'}
|
||||
<button
|
||||
onclick={() => approveRow(row.id, lang)}
|
||||
onclick={() => approveRow(jobId, row.id, lang)}
|
||||
class="px-1.5 py-0.5 text-xs bg-green-100 text-green-700 rounded hover:bg-green-200 transition-colors"
|
||||
title={$t.translate?.preview?.approve}
|
||||
>
|
||||
@@ -436,7 +436,7 @@
|
||||
</button>
|
||||
{#if langStatus !== 'rejected'}
|
||||
<button
|
||||
onclick={() => rejectRow(row.id, lang)}
|
||||
onclick={() => rejectRow(jobId, row.id, lang)}
|
||||
class="px-1.5 py-0.5 text-xs bg-red-100 text-red-700 rounded hover:bg-red-200 transition-colors"
|
||||
title={$t.translate?.preview?.reject}
|
||||
>
|
||||
|
||||
Reference in New Issue
Block a user