fix(translate): apply Excel chart text setters (were silently dropped)
All checks were successful
Deploy to Production / Build and Deploy (push) Successful in 2m35s
All checks were successful
Deploy to Production / Build and Deploy (push) Successful in 2m35s
Chart <a:t> elements (title, axis labels, series names) were collected and sent to the LLM, but the apply loop never invoked their setters. Reason: the apply loop iterated only text_elements[:sheet_name_offset], which excluded the chart-text positions (sheet names were inserted in the middle, pushing chart texts past sheet_name_offset). The LLM correctly returned French for "Revenue by Product" / "Quantity Trend" / "Order #" but the result was thrown away. Fix: include the chart-text slice [sheet_name_offset + N .. total_texts] in the apply loop. Setters for sheet names are still None, so they are naturally skipped. Add test_excel_chart_text_applied.py (2 end-to-end tests) using a fixed provider that pre-translates every known chart text; the test asserts all chart <a:t> values in the output .xlsx come from the FR table, not the original English source.
This commit is contained in:
111
tests/test_excel_chart_text_applied.py
Normal file
111
tests/test_excel_chart_text_applied.py
Normal file
@@ -0,0 +1,111 @@
|
||||
"""Regression test for the bug where chart <a:t> text was never written back
|
||||
to the .xlsx after translation, even though the LLM had returned the
|
||||
correct French translation.
|
||||
|
||||
Root cause: the apply loop only iterated text_elements[:sheet_name_offset]
|
||||
which excluded the chart-text positions (sheet names were added in the
|
||||
middle, pushing chart texts past sheet_name_offset).
|
||||
"""
|
||||
import sys
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
|
||||
|
||||
from lxml import etree
|
||||
from openpyxl import load_workbook
|
||||
import zipfile
|
||||
|
||||
from translators.excel_translator import ExcelTranslator
|
||||
|
||||
NS_A = "http://schemas.openxmlformats.org/drawingml/2006/main"
|
||||
NS_C = "http://schemas.openxmlformats.org/drawingml/2006/chart"
|
||||
|
||||
# Force a known translation for every chart-relevant string.
|
||||
FR = {
|
||||
"Monthly Sales Volume (Units)": "Volume de ventes mensuel (unites)",
|
||||
"Month": "Mois",
|
||||
"Quantity": "Quantite",
|
||||
"Revenue by Product": "Chiffre d'affaires par produit",
|
||||
"Quantity Trend": "Tendance des quantites",
|
||||
"Order #": "Numero de commande",
|
||||
}
|
||||
|
||||
|
||||
class FixedProvider:
|
||||
def translate_batch(self, *args, **kwargs):
|
||||
if args and hasattr(args[0], 'text'):
|
||||
return [type("R", (), {"translated_text": FR.get(req.text, req.text)})() for req in args[0]]
|
||||
texts = args[0] if args else kwargs.get('texts', [])
|
||||
return [FR.get(t, t) for t in texts]
|
||||
|
||||
|
||||
def _read_chart_a_t(xlsx_path: Path):
|
||||
out = {}
|
||||
with zipfile.ZipFile(xlsx_path, 'r') as zf:
|
||||
for name in zf.namelist():
|
||||
if name.startswith('xl/charts/') and name.endswith('.xml'):
|
||||
xml = etree.fromstring(zf.read(name))
|
||||
out[name] = [t.text for t in xml.iter(f'{{{NS_A}}}t') if t.text]
|
||||
return out
|
||||
|
||||
|
||||
def test_chart_texts_get_applied_to_output(tmp_path):
|
||||
src = Path(r'D:\dev1405\office_translator\sample_files\test_corpus\test_excel.xlsx')
|
||||
dst = tmp_path / "out.xlsx"
|
||||
shutil.copy(src, dst)
|
||||
|
||||
tr = ExcelTranslator()
|
||||
tr.set_provider(FixedProvider())
|
||||
tr.translate_file(
|
||||
input_path=src,
|
||||
output_path=dst,
|
||||
target_language="fr",
|
||||
source_language="en",
|
||||
translate_images=False,
|
||||
)
|
||||
|
||||
chart_texts = _read_chart_a_t(dst)
|
||||
# All chart a:t elements should be translated, not still English.
|
||||
for cf, texts in chart_texts.items():
|
||||
for t in texts:
|
||||
assert t in FR.values(), (
|
||||
f"{cf} still has untranslated text: {t!r}. "
|
||||
f"Expected one of: {list(FR.values())}"
|
||||
)
|
||||
|
||||
|
||||
def test_chart_refs_also_rewritten(tmp_path):
|
||||
"""Sheet refs are rewritten as part of the same fix flow."""
|
||||
src = Path(r'D:\dev1405\office_translator\sample_files\test_corpus\test_excel.xlsx')
|
||||
dst = tmp_path / "out2.xlsx"
|
||||
shutil.copy(src, dst)
|
||||
|
||||
tr = ExcelTranslator()
|
||||
tr.set_provider(FixedProvider())
|
||||
tr.translate_file(
|
||||
input_path=src,
|
||||
output_path=dst,
|
||||
target_language="fr",
|
||||
source_language="en",
|
||||
translate_images=False,
|
||||
)
|
||||
|
||||
# Sheet name 'Sales 2024' is not in FR so it should not be renamed.
|
||||
# The chart refs should still point at 'Sales 2024' (no rename happened).
|
||||
with zipfile.ZipFile(dst, 'r') as zf:
|
||||
for name in zf.namelist():
|
||||
if name.startswith('xl/charts/') and name.endswith('.xml'):
|
||||
xml = etree.fromstring(zf.read(name))
|
||||
refs = [f.text for f in xml.iter(f'{{{NS_C}}}f') if f.text]
|
||||
# The chart refs should still be valid (no broken refs)
|
||||
for r in refs:
|
||||
assert '!' in r, f"Invalid chart ref in {name}: {r!r}"
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
import tempfile
|
||||
with tempfile.TemporaryDirectory() as d:
|
||||
test_chart_texts_get_applied_to_output(Path(d))
|
||||
test_chart_refs_also_rewritten(Path(d))
|
||||
print("chart text applied test passed.")
|
||||
@@ -264,11 +264,20 @@ class ExcelTranslator:
|
||||
}
|
||||
)
|
||||
|
||||
# Apply cell translations
|
||||
# Apply cell + chart translations. The structure of
|
||||
# text_elements is:
|
||||
# [0 .. sheet_name_offset) -> cell/header/footer setters
|
||||
# [sheet_name_offset .. +N) -> sheet names (setter=None)
|
||||
# [sheet_name_offset + N .. total_texts) -> chart setters
|
||||
# The old code only iterated the first slice, silently skipping
|
||||
# every chart setter. We now concatenate the two non-None slices.
|
||||
_n_sheet_names = len(sheet_names_to_translate)
|
||||
_chart_slice = text_elements[sheet_name_offset + _n_sheet_names:]
|
||||
_chart_trans_slice = translated_texts[sheet_name_offset + _n_sheet_names:]
|
||||
for i, ((original_text, setter), translated) in enumerate(
|
||||
zip(
|
||||
text_elements[:sheet_name_offset],
|
||||
translated_texts[:sheet_name_offset],
|
||||
text_elements[:sheet_name_offset] + _chart_slice,
|
||||
translated_texts[:sheet_name_offset] + _chart_trans_slice,
|
||||
)
|
||||
):
|
||||
if translated is not None and setter is not None:
|
||||
|
||||
Reference in New Issue
Block a user