diff --git a/tests/test_excel_chart_text_applied.py b/tests/test_excel_chart_text_applied.py new file mode 100644 index 0000000..36a2022 --- /dev/null +++ b/tests/test_excel_chart_text_applied.py @@ -0,0 +1,111 @@ +"""Regression test for the bug where chart 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.") diff --git a/translators/excel_translator.py b/translators/excel_translator.py index 9f8de44..dee144d 100644 --- a/translators/excel_translator.py +++ b/translators/excel_translator.py @@ -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: