From 909755d9e74ebdd7459aa3ae5400908aac7ffa10 Mon Sep 17 00:00:00 2001 From: mgros Date: Sat, 15 Aug 2026 01:13:48 +0200 Subject: [PATCH] handle definitions.yaml --- AGENTS.md | 1 + PythonScripts/audit_translations/README.md | 8 +- PythonScripts/audit_translations/auditor.py | 153 ++++++++-- PythonScripts/audit_translations/cli.py | 2 +- PythonScripts/audit_translations/differ.py | 2 +- PythonScripts/audit_translations/errors.py | 5 + .../audit_translations/line_resolver.py | 2 +- .../audit_translations/models/__init__.py | 1 + .../audit_translations/models/audit.py | 20 ++ .../audit_translations/models/definitions.py | 52 ++++ .../{models.py => models/rules.py} | 36 +-- PythonScripts/audit_translations/parsers.py | 109 +++++++- PythonScripts/audit_translations/renderer.py | 72 ++++- .../golden/rich/cli_calculus_verbose.golden | 3 + .../audit_translations/tests/test_auditor.py | 66 ++++- .../tests/test_cli_end_to_end.py | 80 ++++++ .../tests/test_definitions.py | 261 ++++++++++++++++++ .../audit_translations/tests/test_differ.py | 2 +- .../tests/test_line_resolver.py | 2 +- .../audit_translations/tests/test_parsers.py | 2 +- 20 files changed, 795 insertions(+), 84 deletions(-) create mode 100644 PythonScripts/audit_translations/errors.py create mode 100644 PythonScripts/audit_translations/models/__init__.py create mode 100644 PythonScripts/audit_translations/models/audit.py create mode 100644 PythonScripts/audit_translations/models/definitions.py rename PythonScripts/audit_translations/{models.py => models/rules.py} (81%) create mode 100644 PythonScripts/audit_translations/tests/test_definitions.py diff --git a/AGENTS.md b/AGENTS.md index 17fd6552e..70a4b943f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,6 +35,7 @@ but add common mistakes of AI agents here instead. - do not do any git commands unless explicitly asked for - Rust coverage is in `target/coverage/`. - When working with GitHub, e.g. looking at PRs and issues, check if the GitHub CLI is installed (`gh --version`). +- When writing tests, add a brief description explaining the purpose and expected behaviour. Avoid complex setups like using heavily parameterized tests (eg `@pytest.mark.parametrize` for Python) ## Fuzzing (`fuzz/` + cargo-fuzz) - Install: `cargo install cargo-fuzz`, use a **nightly** toolchain (`rustup run nightly cargo fuzz …`). diff --git a/PythonScripts/audit_translations/README.md b/PythonScripts/audit_translations/README.md index 240e72300..8aedf1265 100644 --- a/PythonScripts/audit_translations/README.md +++ b/PythonScripts/audit_translations/README.md @@ -10,6 +10,7 @@ The tool analyzes rule files to detect the following issues: * **Extra Rules:** Rules present in the target translation but absent in the source (flagged as potentially intentional language-specific additions). * **Untranslated Text:** Detects text keys that still use **lowercase** formatting, indicating they haven't been verified or translated yet. * **Rule Differences:** Structural changes (match expressions, conditions, variables, or test/replace layout) between the source and target translation. +* **Definition Coverage:** Compares literal `definitions.yaml` entries by name and collection kind (`vector`, `set`, or `map`). Add `# audit-ignore` to a rule block to suppress auditing that rule. @@ -49,10 +50,11 @@ The tool automatically adjusts its matching logic based on the file type: 2. **Unicode Files:** * Matches rules based on character/range keys. * *Examples:* `unicode.yaml`, `unicode-full.yaml` (keys like `a-z`, `!`, `0-9`). +3. **Definition Files:** + * `definitions.yaml` is audited by default and can be selected with `--file definitions.yaml`. + * Definitions are matched by name. Missing definitions and collection-kind mismatches are issues; target-only definitions are informational. + * Definition contents are not compared, includes are not resolved, and translation verification is not available for definitions. -`definitions.yaml` is intentionally excluded from audits *for now*. It does not have the same semantics - as normal rules, so the tool ignores it during automatic file discovery and when it is passed to -`--file`. --- diff --git a/PythonScripts/audit_translations/auditor.py b/PythonScripts/audit_translations/auditor.py index e73bb629c..b67cbf911 100644 --- a/PythonScripts/audit_translations/auditor.py +++ b/PythonScripts/audit_translations/auditor.py @@ -8,9 +8,19 @@ from pathlib import Path from .differ import diff_rules -from .models import AuditError, AuditSummary, ComparisonResult, RuleInfo -from .parsers import parse_yaml_file -from .renderer import console, print_audit_header, print_audit_summary, print_language_list, print_warnings +from .errors import AuditError +from .models.audit import AuditSummary +from .models.definitions import DefinitionComparisonResult, DefinitionInfo, DefinitionTypeMismatch +from .models.rules import ComparisonResult, RuleInfo +from .parsers import parse_definitions_file, parse_yaml_file +from .renderer import ( + console, + print_audit_header, + print_audit_summary, + print_definition_findings, + print_language_list, + print_warnings, +) def split_language_into_base_and_region(language: str) -> tuple[str, str | None]: @@ -32,7 +42,7 @@ def get_rules_dir(rules_dir: str | None = None) -> Path: def is_definitions_file(file_path: str | Path) -> bool: - """Return if the file name is definitions.yaml, which is not yet supported.""" + """Return whether a file needs the dedicated definitions audit path.""" return Path(file_path).name == "definitions.yaml" @@ -44,13 +54,12 @@ def collect_from(directory: Path, root: Path) -> None: if not directory.exists(): return for f in directory.glob("*.yaml"): - if f.name != "prefs.yaml" and not is_definitions_file(f): + if f.name != "prefs.yaml": files.add(f.relative_to(root)) shared_dir = directory / "SharedRules" if shared_dir.exists(): for f in shared_dir.glob("*.yaml"): - if not is_definitions_file(f): - files.add(f.relative_to(root)) + files.add(f.relative_to(root)) collect_from(lang_dir, lang_dir) if region_dir: @@ -142,6 +151,67 @@ def merge_rules(base_rules: list[RuleInfo], region_rules: list[RuleInfo]) -> lis ) +def compare_definition_files( + source_path: Path, + target_path: Path, + issue_filter: set[str] | None = None, + target_region_path: Path | None = None, + source_region_path: Path | None = None, +) -> DefinitionComparisonResult: + """Compare literal definitions by name and collection kind.""" + + def load_definitions(path: Path | None) -> dict[str, DefinitionInfo]: + if path and path.exists(): + definitions, _ = parse_definitions_file(path) + return definitions + return {} + + def merge_definitions( + base_definitions: dict[str, DefinitionInfo], + region_definitions: dict[str, DefinitionInfo], + ) -> dict[str, DefinitionInfo]: + merged = dict(base_definitions) + merged.update(region_definitions) + return merged + + source_definitions = merge_definitions( + load_definitions(source_path), + load_definitions(source_region_path), + ) + target_definitions = merge_definitions( + load_definitions(target_path), + load_definitions(target_region_path), + ) + + include_all = issue_filter is None + include_missing = include_all or "missing" in issue_filter + include_extra = include_all or "extra" in issue_filter + include_diffs = include_all or "diffs" in issue_filter + + missing_definitions = ( + [definition for name, definition in source_definitions.items() if name not in target_definitions] + if include_missing + else [] + ) + extra_definitions = ( + [definition for name, definition in target_definitions.items() if name not in source_definitions] if include_extra else [] + ) + type_mismatches = [] + if include_diffs: + for name, source_definition in source_definitions.items(): + target_definition = target_definitions.get(name) + if target_definition and source_definition.kind is not target_definition.kind: + type_mismatches.append(DefinitionTypeMismatch(source_definition, target_definition)) + + return DefinitionComparisonResult( + missing_definitions=missing_definitions, + extra_definitions=extra_definitions, + type_mismatches=type_mismatches, + source_definition_count=len(source_definitions), + target_definition_count=len(target_definitions), + ) + + def audit_language( language: str, specific_file: str | None = None, @@ -178,10 +248,7 @@ def audit_language( raise AuditError(f"Target region directory not found: {translated_region_dir}") # Get list of files to audit - if specific_file: - files = [] if is_definitions_file(Path(specific_file)) else [specific_file] - else: - files = get_yaml_files(source_dir, source_region_dir) + files = [specific_file] if specific_file else get_yaml_files(source_dir, source_region_dir) print_audit_header(language, len(files), source_language) @@ -190,6 +257,9 @@ def audit_language( total_untranslated = 0 total_extra = 0 total_differences = 0 + total_missing_definitions = 0 + total_extra_definitions = 0 + total_definition_type_mismatches = 0 files_with_issues = 0 files_ok = 0 @@ -203,26 +273,54 @@ def audit_language( console.print(f"\n[yellow]⚠ Warning:[/] Source file not found: {english_path}") continue - result = compare_files( - english_path, - translated_path, - issue_filter, - translated_region_path if translated_region_path and translated_region_path.exists() else None, - english_region_path if english_region_path and english_region_path.exists() else None, + existing_translated_region_path = ( + translated_region_path if translated_region_path and translated_region_path.exists() else None ) - - if result.has_issues: - issues = print_warnings(result, file_name, verbose, language, source_language) + existing_english_region_path = english_region_path if english_region_path and english_region_path.exists() else None + + if is_definitions_file(file_name): + definition_result = compare_definition_files( + english_path, + translated_path, + issue_filter, + existing_translated_region_path, + existing_english_region_path, + ) + issues = print_definition_findings( + definition_result, + file_name, + language, + source_language, + ) if issues > 0: files_with_issues += 1 + else: + files_ok += 1 total_issues += issues + total_missing_definitions += len(definition_result.missing_definitions) + total_extra_definitions += len(definition_result.extra_definitions) + total_definition_type_mismatches += len(definition_result.type_mismatches) else: - files_ok += 1 - - total_missing += len(result.missing_rules) - total_untranslated += sum(len(entries) for _rule, entries in result.untranslated_text) - total_extra += len(result.extra_rules) - total_differences += len(result.rule_differences) + result = compare_files( + english_path, + translated_path, + issue_filter, + existing_translated_region_path, + existing_english_region_path, + ) + + if result.has_issues: + issues = print_warnings(result, file_name, verbose, language, source_language) + if issues > 0: + files_with_issues += 1 + total_issues += issues + else: + files_ok += 1 + + total_missing += len(result.missing_rules) + total_untranslated += sum(len(entries) for _rule, entries in result.untranslated_text) + total_extra += len(result.extra_rules) + total_differences += len(result.rule_differences) print_audit_summary( AuditSummary( @@ -233,6 +331,9 @@ def audit_language( total_untranslated=total_untranslated, total_extra=total_extra, total_differences=total_differences, + total_missing_definitions=total_missing_definitions, + total_extra_definitions=total_extra_definitions, + total_definition_type_mismatches=total_definition_type_mismatches, total_issues=total_issues, ) ) diff --git a/PythonScripts/audit_translations/cli.py b/PythonScripts/audit_translations/cli.py index ee1b48739..6916a636e 100644 --- a/PythonScripts/audit_translations/cli.py +++ b/PythonScripts/audit_translations/cli.py @@ -8,7 +8,7 @@ import sys from .auditor import audit_language, list_languages -from .models import AuditError +from .errors import AuditError from .renderer import console diff --git a/PythonScripts/audit_translations/differ.py b/PythonScripts/audit_translations/differ.py index 5ae4483ce..a2756df08 100644 --- a/PythonScripts/audit_translations/differ.py +++ b/PythonScripts/audit_translations/differ.py @@ -11,7 +11,7 @@ extract_variables, normalize_xpath, ) -from .models import DiffType, RuleDifference, RuleInfo +from .models.rules import DiffType, RuleDifference, RuleInfo def dedup_list(values: list[str]) -> list[str]: diff --git a/PythonScripts/audit_translations/errors.py b/PythonScripts/audit_translations/errors.py new file mode 100644 index 000000000..b4d75ec11 --- /dev/null +++ b/PythonScripts/audit_translations/errors.py @@ -0,0 +1,5 @@ +"""Exceptions raised by the translation audit tool.""" + + +class AuditError(Exception): + """Raised when the audit encounters a configuration or validation error.""" diff --git a/PythonScripts/audit_translations/line_resolver.py b/PythonScripts/audit_translations/line_resolver.py index ee22c59ca..2087a68d4 100644 --- a/PythonScripts/audit_translations/line_resolver.py +++ b/PythonScripts/audit_translations/line_resolver.py @@ -5,7 +5,7 @@ """ from .extractors import extract_structure_elements -from .models import DiffType, RuleDifference, RuleInfo +from .models.rules import DiffType, RuleDifference, RuleInfo def _get_line_map_lines(rule: RuleInfo, kind: DiffType, token: str | None = None) -> list[int]: diff --git a/PythonScripts/audit_translations/models/__init__.py b/PythonScripts/audit_translations/models/__init__.py new file mode 100644 index 000000000..4e5b75bc5 --- /dev/null +++ b/PythonScripts/audit_translations/models/__init__.py @@ -0,0 +1 @@ +"""Domain models used by the translation audit tool.""" diff --git a/PythonScripts/audit_translations/models/audit.py b/PythonScripts/audit_translations/models/audit.py new file mode 100644 index 000000000..3a44a2010 --- /dev/null +++ b/PythonScripts/audit_translations/models/audit.py @@ -0,0 +1,20 @@ +"""Models for aggregate audit output.""" + +from dataclasses import dataclass + + +@dataclass +class AuditSummary: + """Accumulated totals from a full language audit.""" + + files_checked: int + files_with_issues: int + files_ok: int + total_missing: int + total_untranslated: int + total_extra: int + total_differences: int + total_missing_definitions: int + total_extra_definitions: int + total_definition_type_mismatches: int + total_issues: int diff --git a/PythonScripts/audit_translations/models/definitions.py b/PythonScripts/audit_translations/models/definitions.py new file mode 100644 index 000000000..68d2e976a --- /dev/null +++ b/PythonScripts/audit_translations/models/definitions.py @@ -0,0 +1,52 @@ +"""Models for definitions.yaml parsing and comparison.""" + +from dataclasses import dataclass +from enum import StrEnum +from typing import Any + + +class DefinitionKind(StrEnum): + """Collection shapes supported by MathCAT definitions files.""" + + VECTOR = "vector" + SET = "set" + MAP = "map" + + +@dataclass +class DefinitionInfo: + """Information about one literal entry in a definitions file.""" + + name: str + kind: DefinitionKind + line_number: int + raw_content: str + data: Any + + +@dataclass +class DefinitionTypeMismatch: + """A definition whose collection kind differs between source and target.""" + + source_definition: DefinitionInfo + target_definition: DefinitionInfo + + +@dataclass +class DefinitionComparisonResult: + """Results from comparing two literal definitions files.""" + + missing_definitions: list[DefinitionInfo] + extra_definitions: list[DefinitionInfo] + type_mismatches: list[DefinitionTypeMismatch] + source_definition_count: int + target_definition_count: int + + @property + def issue_count(self) -> int: + """Count actionable findings; target-only definitions are informational.""" + return len(self.missing_definitions) + len(self.type_mismatches) + + @property + def has_findings(self) -> bool: + return bool(self.missing_definitions or self.extra_definitions or self.type_mismatches) diff --git a/PythonScripts/audit_translations/models.py b/PythonScripts/audit_translations/models/rules.py similarity index 81% rename from PythonScripts/audit_translations/models.py rename to PythonScripts/audit_translations/models/rules.py index 350ffed33..39069d9db 100644 --- a/PythonScripts/audit_translations/models.py +++ b/PythonScripts/audit_translations/models/rules.py @@ -1,18 +1,10 @@ -""" -Data models for the audit tool. - -Contains dataclasses for representing rules and comparison results. -""" +"""Models for speech, navigation, and unicode rule audits.""" from dataclasses import dataclass, field from enum import StrEnum from typing import Any -class AuditError(Exception): - """Raised when the audit encounters a configuration or validation error.""" - - class IssueType(StrEnum): """Top-level issue categories used by the audit renderer.""" @@ -72,7 +64,7 @@ class RuleInfo: name: str | None # None for unicode entries tag: str | None # None for unicode entries - key: str # For unicode entries, this is the character/range + key: str line_number: int raw_content: str data: Any | None = None @@ -91,7 +83,7 @@ def untranslated_keys(self) -> list[str]: @dataclass class RuleDifference: - """Fine-grained difference between source and translated rule""" + """Fine-grained difference between source and translated rule.""" english_rule: RuleInfo translated_rule: RuleInfo @@ -107,29 +99,15 @@ def __post_init__(self) -> None: @dataclass class ComparisonResult: - """Results from comparing source and translated files""" + """Results from comparing source and translated rule files.""" - missing_rules: list[RuleInfo] # Rules in source but not in translation - extra_rules: list[RuleInfo] # Rules in translation but not in source + missing_rules: list[RuleInfo] + extra_rules: list[RuleInfo] untranslated_text: list[tuple[RuleInfo, list[UntranslatedEntry]]] english_rule_count: int translated_rule_count: int - rule_differences: list[RuleDifference] = field(default_factory=list) # Fine-grained diffs + rule_differences: list[RuleDifference] = field(default_factory=list) @property def has_issues(self) -> bool: return bool(self.missing_rules or self.untranslated_text or self.extra_rules or self.rule_differences) - - -@dataclass -class AuditSummary: - """Accumulated totals from a full language audit.""" - - files_checked: int - files_with_issues: int - files_ok: int - total_missing: int - total_untranslated: int - total_extra: int - total_differences: int - total_issues: int diff --git a/PythonScripts/audit_translations/parsers.py b/PythonScripts/audit_translations/parsers.py index bf64843ec..11dfc5292 100644 --- a/PythonScripts/audit_translations/parsers.py +++ b/PythonScripts/audit_translations/parsers.py @@ -1,7 +1,7 @@ """ YAML file parsing functions. -Handles parsing of rule files and unicode files to extract rule information. +Handles parsing of rule, unicode, and definitions files. """ import re @@ -9,10 +9,13 @@ from typing import Any from ruamel.yaml import YAML +from ruamel.yaml.error import YAMLError from ruamel.yaml.scanner import ScannerError +from .errors import AuditError from .extractors import iter_field_matches, mapping_key_line -from .models import RuleInfo, UntranslatedEntry +from .models.definitions import DefinitionInfo, DefinitionKind +from .models.rules import RuleInfo, UntranslatedEntry _yaml = YAML() _yaml.preserve_quotes = True @@ -23,14 +26,8 @@ def is_unicode_file(file_path: Path) -> bool: return file_path.name in ("unicode.yaml", "unicode-full.yaml") -def parse_yaml_file(file_path: Path, strict: bool = False) -> tuple[list[RuleInfo], str]: - """ - Parse a YAML file and extract rules. - Returns list of RuleInfo and the raw file content. - - For standard rule files: extracts rules with name/tag - For unicode files: extracts entries with character/range keys - """ +def _load_yaml_file(file_path: Path, strict: bool = False) -> tuple[Any, str]: + """Read and load YAML, retaining the tool's existing tab fallback.""" with open(file_path, encoding="utf-8") as f: content = f.read() @@ -45,11 +42,103 @@ def parse_yaml_file(file_path: Path, strict: bool = False) -> tuple[list[RuleInf else: raise exc + return data, content + + +def parse_yaml_file(file_path: Path, strict: bool = False) -> tuple[list[RuleInfo], str]: + """ + Parse a YAML file and extract rules. + Returns list of RuleInfo and the raw file content. + + For standard rule files: extracts rules with name/tag + For unicode files: extracts entries with character/range keys + """ + data, content = _load_yaml_file(file_path, strict) rules = parse_unicode_file(content, data) if is_unicode_file(file_path) else parse_rules_file(content, data) return rules, content +def parse_definitions_file(file_path: Path, strict: bool = False) -> tuple[dict[str, DefinitionInfo], str]: + """Parse and validate one literal ``definitions.yaml`` file. + + Includes are deliberately ignored. Definitions are returned by name so + ordering is irrelevant and a later duplicate replaces an earlier one. + """ + try: + data, content = _load_yaml_file(file_path, strict) + except YAMLError as exc: + raise AuditError(f"Invalid YAML in {file_path}: {exc}") from exc + + return parse_definitions(content, data, file_path), content + + +def parse_definitions(content: str, data: Any, file_path: Path) -> dict[str, DefinitionInfo]: + """Validate parsed YAML and return literal definitions keyed by name.""" + if not isinstance(data, list): + raise AuditError(f"Invalid definitions file {file_path}: expected a YAML sequence") + + lines = content.splitlines() + starts = [data.lc.item(idx)[0] if hasattr(data, "lc") else 0 for idx in range(len(data))] + raw_blocks = build_raw_blocks(lines, starts) + definitions: dict[str, DefinitionInfo] = {} + + for item, line_idx, raw_content in zip(data, starts, raw_blocks, strict=True): + line_number = line_idx + 1 + if not isinstance(item, dict) or len(item) != 1: + raise AuditError(f"Invalid definition in {file_path} at line {line_number}: expected exactly one definition name") + + name, value = next(iter(item.items())) + if not isinstance(name, str): + raise AuditError( + f"Invalid definition name in {file_path} at line {line_number}: expected a string, got {type(name).__name__}" + ) + if name == "include": + continue + + kind = _definition_kind(value, file_path, name, line_number) + definitions[name] = DefinitionInfo( + name=name, + kind=kind, + line_number=line_number, + raw_content=raw_content, + data=value, + ) + + return definitions + + +def _definition_kind(value: Any, file_path: Path, name: str, line_number: int) -> DefinitionKind: + """Validate a definition value and identify its collection kind.""" + + def invalid(message: str) -> AuditError: + return AuditError(f"Invalid definition '{name}' in {file_path} at line {line_number}: {message}") + + if isinstance(value, list): + if not value: + raise invalid("empty sequences have no unambiguous definition kind") + if not all(isinstance(entry, str) for entry in value): + raise invalid("vector entries must all be strings") + return DefinitionKind.VECTOR + + if isinstance(value, dict): + if not value: + raise invalid("empty mappings have no unambiguous definition kind") + if not all(isinstance(key, str) for key in value): + raise invalid("mapping keys must all be strings") + + values = list(value.values()) + if all(entry is None for entry in values): + return DefinitionKind.SET + if all(isinstance(entry, str) for entry in values): + return DefinitionKind.MAP + if any(entry is None for entry in values) and any(isinstance(entry, str) for entry in values): + raise invalid("mixed set/map values are not supported") + raise invalid("mapping values must be all null (set) or all strings (map)") + + raise invalid("value must be a non-empty sequence or mapping") + + def format_tag(tag_value: Any) -> str | None: if tag_value is None: return None diff --git a/PythonScripts/audit_translations/renderer.py b/PythonScripts/audit_translations/renderer.py index c76da6b2c..d79449e80 100644 --- a/PythonScripts/audit_translations/renderer.py +++ b/PythonScripts/audit_translations/renderer.py @@ -13,7 +13,9 @@ from rich.table import Table from .line_resolver import resolve_diff_lines -from .models import AuditSummary, ComparisonResult, DiffType, IssueType, RuleDifference, RuleInfo +from .models.audit import AuditSummary +from .models.definitions import DefinitionComparisonResult +from .models.rules import ComparisonResult, DiffType, IssueType, RuleDifference, RuleInfo console = Console() @@ -152,6 +154,59 @@ def add_issue(rule: RuleInfo, group_key: IssueGroupKey, payload: dict[str, Any]) return issues +def print_definition_findings( + result: DefinitionComparisonResult, + file_name: str | Path, + target_language: str = "tr", + source_language: str = "en", +) -> int: + """Render definition issues and informational extras on a dedicated path.""" + if not result.has_findings: + return 0 + + display_name = Path(file_name).as_posix() + source_label = language_label(source_language) + target_label = language_label(target_language) + style, icon = ("red", "✗") if result.issue_count else ("blue", "i") + + console.print() + console.rule(style="cyan") + console.print(f"[{style}]{icon}[/] [bold]{escape(display_name)}[/]") + console.print( + f" [dim]{source_label}: {result.source_definition_count} definitions → " + f"{target_label}: {result.target_definition_count} definitions[/]" + ) + console.rule(style="cyan") + + if result.issue_count: + console.print(f"\n [magenta]≠[/] [bold]Definition Issues[/] [[magenta]{result.issue_count}[/]]") + + for definition in result.missing_definitions: + console.print(f" [dim]•[/] [cyan]{escape(definition.name)}[/]") + console.print(" [dim]Missing in Translation[/]") + console.print(f" [dim]• (line {definition.line_number} in {source_label})[/]") + + for mismatch in result.type_mismatches: + source_definition = mismatch.source_definition + target_definition = mismatch.target_definition + console.print(f" [dim]•[/] [cyan]{escape(source_definition.name)}[/]") + console.print(" [dim]Definition Type Mismatch[/]") + console.print( + f" [dim]• (line {source_definition.line_number} {source_label}, " + f"{target_definition.line_number} {target_label})[/]" + ) + console.print(f" [green]{source_label}:[/] {source_definition.kind.value}") + console.print(f" [red]{target_label}:[/] {target_definition.kind.value}") + + if result.extra_definitions: + console.print(f"\n [blue]i[/] [bold]Info: Extra Definitions[/] [[blue]{len(result.extra_definitions)}[/]]") + for definition in result.extra_definitions: + console.print(f" [dim]•[/] [cyan]{escape(definition.name)}[/]") + console.print(f" [dim](line {definition.line_number} in {target_label})[/]") + + return result.issue_count + + GREEN_FILE_COUNT_THRESHOLD = 7 YELLOW_FILE_COUNT_THRESHOLD = 4 @@ -185,6 +240,21 @@ def print_audit_summary(summary: AuditSummary) -> None: ("Untranslated text", summary.total_untranslated, "yellow" if summary.total_untranslated else "green"), ("Rule differences", summary.total_differences, "magenta" if summary.total_differences else "green"), ("Extra rules", summary.total_extra, "blue" if summary.total_extra else None), + ( + "Missing definitions", + summary.total_missing_definitions, + "red" if summary.total_missing_definitions else "green", + ), + ( + "Definition type mismatches", + summary.total_definition_type_mismatches, + "magenta" if summary.total_definition_type_mismatches else "green", + ), + ( + "Extra definitions", + summary.total_extra_definitions, + "blue" if summary.total_extra_definitions else None, + ), ]: table.add_row(label, f"[{color}]{value}[/]" if color else str(value)) console.print(Panel(table, style="cyan")) diff --git a/PythonScripts/audit_translations/tests/golden/rich/cli_calculus_verbose.golden b/PythonScripts/audit_translations/tests/golden/rich/cli_calculus_verbose.golden index a75c81d3d..06087cd70 100644 --- a/PythonScripts/audit_translations/tests/golden/rich/cli_calculus_verbose.golden +++ b/PythonScripts/audit_translations/tests/golden/rich/cli_calculus_verbose.golden @@ -65,4 +65,7 @@ │ Untranslated text 6 │ │ Rule differences 6 │ │ Extra rules 0 │ +│ Missing definitions 0 │ +│ Definition type mismatches 0 │ +│ Extra definitions 0 │ ╰──────────────────────────────────────────────────────────────────────────────╯ diff --git a/PythonScripts/audit_translations/tests/test_auditor.py b/PythonScripts/audit_translations/tests/test_auditor.py index a1be8e3d1..38b63694f 100644 --- a/PythonScripts/audit_translations/tests/test_auditor.py +++ b/PythonScripts/audit_translations/tests/test_auditor.py @@ -8,7 +8,7 @@ from ..auditor import audit_language, compare_files, get_yaml_files, list_languages from ..line_resolver import resolve_diff_lines -from ..models import ComparisonResult, DiffType, RuleDifference, RuleInfo, UntranslatedEntry +from ..models.rules import ComparisonResult, DiffType, RuleDifference, RuleInfo, UntranslatedEntry from ..renderer import console, print_warnings from .conftest import strip_ansi @@ -322,32 +322,80 @@ def test_get_yaml_files_includes_region(tmp_path) -> None: assert set(files) == {Path("base.yaml"), Path("SharedRules/shared.yaml"), Path("unicode.yaml")} -def test_get_yaml_files_ignores_definitions(tmp_path) -> None: - """Definitions files are excluded from automatic audit discovery.""" +def test_get_yaml_files_includes_definitions_but_ignores_prefs(tmp_path) -> None: + """Definitions use automatic discovery while prefs remains excluded.""" lang_dir = tmp_path / "lang" shared_dir = lang_dir / "SharedRules" shared_dir.mkdir(parents=True) (lang_dir / "rules.yaml").write_text("---", encoding="utf-8") (lang_dir / "definitions.yaml").write_text("---", encoding="utf-8") + (lang_dir / "prefs.yaml").write_text("---", encoding="utf-8") (shared_dir / "definitions.yaml").write_text("---", encoding="utf-8") - assert get_yaml_files(lang_dir) == [Path("rules.yaml")] + assert get_yaml_files(lang_dir) == [ + Path("SharedRules/definitions.yaml"), + Path("definitions.yaml"), + Path("rules.yaml"), + ] -def test_audit_language_ignores_explicit_definitions_file(tmp_path, fixed_console_width) -> None: - """Passing definitions.yaml through --file produces an empty audit.""" +def test_audit_language_supports_explicit_definitions_file(tmp_path, fixed_console_width) -> None: + """Passing definitions.yaml through --file audits that file only.""" rules_dir = tmp_path / "Rules" / "Languages" (rules_dir / "en").mkdir(parents=True) (rules_dir / "de").mkdir(parents=True) + (rules_dir / "en" / "definitions.yaml").write_text("- Foo: [one]\n", encoding="utf-8") + (rules_dir / "de" / "definitions.yaml").write_text('- include: "other.yaml"\n', encoding="utf-8") with console.capture() as capture: total_issues = audit_language("de", specific_file="definitions.yaml", rules_dir=str(rules_dir)) output = strip_ansi(capture.get()) + assert total_issues == 1 + assert "Files to check: 1" in output + assert "definitions.yaml" in output + assert "Definition Issues [1]" in output + assert "Missing definitions" in output + + +def test_audit_language_discovers_definitions_by_default(tmp_path, fixed_console_width) -> None: + """A normal audit includes definitions.yaml without a feature flag.""" + rules_dir = tmp_path / "Rules" / "Languages" + (rules_dir / "en").mkdir(parents=True) + (rules_dir / "de").mkdir(parents=True) + (rules_dir / "en" / "definitions.yaml").write_text("- Foo: [one]\n", encoding="utf-8") + (rules_dir / "de" / "definitions.yaml").write_text("- Foo: {key: value}\n", encoding="utf-8") + + with console.capture() as capture: + total_issues = audit_language("de", rules_dir=str(rules_dir)) + output = strip_ansi(capture.get()) + + assert total_issues == 1 + assert "Files to check: 1" in output + assert "Definition Type Mismatch" in output + assert "Definition type mismatches" in output + + +def test_extra_definitions_are_informational_only(tmp_path, fixed_console_width) -> None: + """Target-only definitions render without increasing issue or file-issue counts.""" + rules_dir = tmp_path / "Rules" / "Languages" + (rules_dir / "en").mkdir(parents=True) + (rules_dir / "de").mkdir(parents=True) + (rules_dir / "en" / "definitions.yaml").write_text("- Foo: [one]\n", encoding="utf-8") + (rules_dir / "de" / "definitions.yaml").write_text( + "- Foo: [eins]\n- GermanSpecific: {key: value}\n", + encoding="utf-8", + ) + + with console.capture() as capture: + total_issues = audit_language("de", rules_dir=str(rules_dir)) + output = strip_ansi(capture.get()) + assert total_issues == 0 - assert "Files to check: 0" in output - assert "Files checked" in output - assert "definitions.yaml" not in output + assert "Info: Extra Definitions [1]" in output + assert "Files with issues 0" in output + assert "Files OK 1" in output + assert "Extra definitions 1" in output def test_list_languages_includes_region_codes(tmp_path) -> None: diff --git a/PythonScripts/audit_translations/tests/test_cli_end_to_end.py b/PythonScripts/audit_translations/tests/test_cli_end_to_end.py index c4c8364b7..7876bc0de 100644 --- a/PythonScripts/audit_translations/tests/test_cli_end_to_end.py +++ b/PythonScripts/audit_translations/tests/test_cli_end_to_end.py @@ -20,6 +20,86 @@ def fixture_rules_dir() -> Path: return Path(__file__).resolve().parent / "fixtures" / "Rules" / "Languages" +def make_definitions_rules_dir(tmp_path: Path) -> Path: + """Create definitions with one missing, extra, and mismatched entry.""" + rules_dir = tmp_path / "Rules" / "Languages" + source_dir = rules_dir / "en" + target_dir = rules_dir / "de" + source_dir.mkdir(parents=True) + target_dir.mkdir(parents=True) + (source_dir / "definitions.yaml").write_text( + "- Missing: [one]\n- Different: [one]\n", + encoding="utf-8", + ) + (target_dir / "definitions.yaml").write_text( + "- Extra: {key: value}\n- Different: {key: value}\n", + encoding="utf-8", + ) + return rules_dir + + +def run_definitions_cli(tmp_path, capsys, monkeypatch, only: str) -> str: + """Run the definitions CLI fixture with one existing --only category.""" + rules_dir = make_definitions_rules_dir(tmp_path) + args = ["de", "--rules-dir", str(rules_dir), "--file", "definitions.yaml", "--only", only] + monkeypatch.setattr(sys, "argv", ["audit_translations", *args]) + + audit_cli.main() + return strip_ansi(capsys.readouterr().out) + + +def test_cli_definitions_missing_filter_shows_only_missing_findings(tmp_path, capsys, monkeypatch) -> None: + """The CLI missing filter renders missing definitions but no extra or mismatch findings.""" + output = run_definitions_cli(tmp_path, capsys, monkeypatch, "missing") + + assert "Files to check: 1" in output + assert "Missing in Translation" in output + assert "Definition Type Mismatch" not in output + assert "Info: Extra Definitions" not in output + + +def test_cli_definitions_extra_filter_shows_only_extra_findings(tmp_path, capsys, monkeypatch) -> None: + """The CLI extra filter renders informational target-only definitions and no issues.""" + output = run_definitions_cli(tmp_path, capsys, monkeypatch, "extra") + + assert "Files to check: 1" in output + assert "Info: Extra Definitions" in output + assert "Missing in Translation" not in output + assert "Definition Type Mismatch" not in output + + +def test_cli_definitions_diffs_filter_shows_only_type_mismatches(tmp_path, capsys, monkeypatch) -> None: + """The CLI diffs filter renders type mismatches but no coverage findings.""" + output = run_definitions_cli(tmp_path, capsys, monkeypatch, "diffs") + + assert "Files to check: 1" in output + assert "Definition Type Mismatch" in output + assert "Missing in Translation" not in output + assert "Info: Extra Definitions" not in output + + +def test_cli_definitions_untranslated_filter_shows_no_definition_findings(tmp_path, capsys, monkeypatch) -> None: + """The CLI untranslated filter produces no definition-specific findings.""" + output = run_definitions_cli(tmp_path, capsys, monkeypatch, "untranslated") + + assert "Files to check: 1" in output + assert "Missing definitions" in output + assert "Definition type mismatches" in output + assert "Extra definitions" in output + assert "Definition Issues" not in output + assert "Info: Extra Definitions" not in output + + +def test_cli_definitions_all_filter_shows_every_definition_finding(tmp_path, capsys, monkeypatch) -> None: + """The CLI all filter renders missing, extra, and type-mismatch definition findings.""" + output = run_definitions_cli(tmp_path, capsys, monkeypatch, "all") + + assert "Files to check: 1" in output + assert "Missing in Translation" in output + assert "Definition Type Mismatch" in output + assert "Info: Extra Definitions" in output + + def test_cli_main_rich_only_filters_issue_groups(capsys, monkeypatch) -> None: """ Ensure --only also filters visible rich subgroup sections. diff --git a/PythonScripts/audit_translations/tests/test_definitions.py b/PythonScripts/audit_translations/tests/test_definitions.py new file mode 100644 index 000000000..a87fc51d2 --- /dev/null +++ b/PythonScripts/audit_translations/tests/test_definitions.py @@ -0,0 +1,261 @@ +"""Focused tests for definitions.yaml parsing and comparison.""" + +from pathlib import Path + +import pytest +from ruamel.yaml import YAML + +from ..auditor import compare_definition_files +from ..errors import AuditError +from ..models.definitions import DefinitionKind +from ..parsers import parse_definitions + + +def parse(content: str): + """Parse an in-memory definitions document with a stable diagnostic path.""" + return parse_definitions(content, YAML().load(content), Path("definitions.yaml")) + + +def assert_invalid(content: str, message: str) -> None: + """Assert that malformed definition YAML produces a contextual audit error.""" + with pytest.raises(AuditError) as exc: + parse(content) + diagnostic = str(exc.value) + assert "definitions.yaml" in diagnostic + assert "line 1" in diagnostic + assert message in diagnostic + + +def compare(tmp_path, source: str, target: str, issue_filter: set[str] | None = None): + """Compare two in-memory definition documents through temporary files.""" + source_path = tmp_path / "source-definitions.yaml" + target_path = tmp_path / "target-definitions.yaml" + source_path.write_text(source, encoding="utf-8") + target_path.write_text(target, encoding="utf-8") + return compare_definition_files(source_path, target_path, issue_filter) + + +def test_parse_vector_definition() -> None: + """A non-empty YAML sequence of strings is classified as a vector.""" + definitions = parse('- NumbersTens:\n - ""\n - ten\n - twenty\n') + assert definitions["NumbersTens"].kind is DefinitionKind.VECTOR + assert definitions["NumbersTens"].line_number == 1 + + +def test_parse_map_definition() -> None: + """A non-empty string-to-string YAML mapping is classified as a map.""" + definitions = parse('- NavigationParts:\n mfrac: "numerator; denominator"\n') + assert definitions["NavigationParts"].kind is DefinitionKind.MAP + assert definitions["NavigationParts"].line_number == 1 + + +def test_parse_set_definition() -> None: + """A non-empty YAML mapping with all-null values is classified as a set.""" + definitions = parse("- TerseFunctionNames:\n divergence:\n curl:\n") + assert definitions["TerseFunctionNames"].kind is DefinitionKind.SET + assert definitions["TerseFunctionNames"].line_number == 1 + + +def test_parse_definitions_ignores_include() -> None: + """An include entry is omitted from the parsed definition mapping.""" + assert parse('- include: "../../definitions.yaml"\n') == {} + + +def test_parse_definitions_rejects_empty_vector() -> None: + """An empty sequence is rejected because its definition kind is ambiguous.""" + assert_invalid("- Foo: []\n", "empty sequences") + + +def test_parse_definitions_rejects_empty_mapping() -> None: + """An empty mapping is rejected because it could be either a set or a map.""" + assert_invalid("- Foo: {}\n", "empty mappings") + + +def test_parse_definitions_rejects_string_scalar() -> None: + """A string scalar cannot be used as a definition value.""" + assert_invalid("- Foo: bar\n", "non-empty sequence or mapping") + + +def test_parse_definitions_rejects_numeric_scalar() -> None: + """A numeric scalar cannot be used as a definition value.""" + assert_invalid("- Foo: 123\n", "non-empty sequence or mapping") + + +def test_parse_definitions_rejects_nested_mapping_value() -> None: + """A nested object is rejected because map values must be strings.""" + assert_invalid("- Foo:\n nested:\n object: value\n", "all strings") + + +def test_parse_definitions_rejects_mixed_set_and_map_values() -> None: + """A mapping containing both null and string values is rejected.""" + assert_invalid('- Foo:\n a:\n b: "value"\n', "mixed set/map") + + +def test_parse_definitions_rejects_non_string_vector_entry() -> None: + """Every entry in a vector definition must be a string.""" + assert_invalid("- Foo:\n - valid\n - 2\n", "vector entries") + + +def test_parse_definitions_rejects_non_string_mapping_key() -> None: + """Every key in a set or map definition must be a string.""" + assert_invalid('- Foo:\n 1: "value"\n', "mapping keys") + + +def test_parse_definitions_rejects_non_string_definition_name() -> None: + """A definition name must be a string.""" + assert_invalid('- 42:\n - "value"\n', "definition name") + + +def test_parse_definitions_last_duplicate_wins() -> None: + """The final occurrence of a duplicate definition name replaces earlier occurrences.""" + definitions = parse('- Foo:\n - first\n- Foo:\n key: "value"\n') + assert len(definitions) == 1 + assert definitions["Foo"].kind is DefinitionKind.MAP + assert definitions["Foo"].line_number == 3 + + +def test_compare_same_definitions_and_kinds_is_clean(tmp_path) -> None: + """Definitions with matching names and kinds produce no findings.""" + result = compare(tmp_path, "- Foo: [one]\n- Bar: {key: value}\n", "- Foo: [eins]\n- Bar: {taste: wert}\n") + assert not result.has_findings + assert result.issue_count == 0 + + +def test_compare_ignores_definition_order(tmp_path) -> None: + """Reordering definitions does not affect name-based comparison.""" + result = compare(tmp_path, "- Foo: [one]\n- Bar: {key: value}\n", "- Bar: {taste: wert}\n- Foo: [eins]\n") + assert not result.has_findings + + +def test_compare_reports_missing_definition_as_issue(tmp_path) -> None: + """A source-only definition is reported as an audit issue.""" + result = compare(tmp_path, "- Foo: [one]\n- Bar: [two]\n", "- Foo: [eins]\n") + assert [definition.name for definition in result.missing_definitions] == ["Bar"] + assert result.issue_count == 1 + + +def test_compare_reports_extra_definition_as_information(tmp_path) -> None: + """A target-only definition is informational and does not increase the issue count.""" + result = compare(tmp_path, "- Foo: [one]\n", "- Foo: [eins]\n- TargetSpecific: {key: value}\n") + assert [definition.name for definition in result.extra_definitions] == ["TargetSpecific"] + assert result.issue_count == 0 + + +def test_compare_reports_type_mismatch_as_issue(tmp_path) -> None: + """Different kinds for the same definition name produce a type-mismatch issue.""" + result = compare(tmp_path, "- Foo: [one]\n", "- Foo: {key: value}\n") + assert len(result.type_mismatches) == 1 + assert result.type_mismatches[0].source_definition.kind is DefinitionKind.VECTOR + assert result.type_mismatches[0].target_definition.kind is DefinitionKind.MAP + assert result.issue_count == 1 + + +def test_compare_ignores_different_vector_contents(tmp_path) -> None: + """Translated vector strings are not compared when both definitions are vectors.""" + result = compare(tmp_path, "- Foo: [one, two, three]\n", "- Foo: [eins, zwei, drei]\n") + assert not result.has_findings + + +def test_compare_ignores_different_vector_lengths(tmp_path) -> None: + """Vector length differences are outside the scope of definition auditing.""" + result = compare(tmp_path, "- Foo: [one, two, three]\n", "- Foo: [eins, zwei]\n") + assert not result.has_findings + + +def test_compare_ignores_different_map_contents(tmp_path) -> None: + """Map keys and values are not compared when both definitions are maps.""" + result = compare(tmp_path, "- Foo: {one: first}\n", "- Foo: {two: second, three: third}\n") + assert not result.has_findings + + +def test_compare_ignores_different_set_contents(tmp_path) -> None: + """Set members are not compared when both definitions are sets.""" + result = compare(tmp_path, "- Foo: {one, two}\n", "- Foo: {three}\n") + assert not result.has_findings + + +def test_compare_ignores_different_includes(tmp_path) -> None: + """Different include paths do not produce definition findings.""" + result = compare( + tmp_path, + '- include: "../../definitions.yaml"\n- Foo: [one]\n', + '- include: "target-specific.yaml"\n- Foo: [eins]\n', + ) + assert not result.has_findings + + +def test_compare_missing_filter_returns_only_missing_definitions(tmp_path) -> None: + """The missing filter includes missing definitions and suppresses other findings.""" + result = compare( + tmp_path, + "- Missing: [one]\n- Different: [one]\n", + "- Extra: [eins]\n- Different: {key: value}\n", + {"missing"}, + ) + assert len(result.missing_definitions) == 1 + assert result.extra_definitions == [] + assert result.type_mismatches == [] + + +def test_compare_extra_filter_returns_only_extra_definitions(tmp_path) -> None: + """The extra filter includes target-only definitions and suppresses other findings.""" + result = compare( + tmp_path, + "- Missing: [one]\n- Different: [one]\n", + "- Extra: [eins]\n- Different: {key: value}\n", + {"extra"}, + ) + assert result.missing_definitions == [] + assert len(result.extra_definitions) == 1 + assert result.type_mismatches == [] + + +def test_compare_diffs_filter_returns_only_type_mismatches(tmp_path) -> None: + """The diffs filter includes definition type mismatches and suppresses coverage findings.""" + result = compare( + tmp_path, + "- Missing: [one]\n- Different: [one]\n", + "- Extra: [eins]\n- Different: {key: value}\n", + {"diffs"}, + ) + assert result.missing_definitions == [] + assert result.extra_definitions == [] + assert len(result.type_mismatches) == 1 + + +def test_compare_untranslated_filter_returns_no_definition_findings(tmp_path) -> None: + """The untranslated filter never fabricates translation-state findings for definitions.""" + result = compare( + tmp_path, + "- Missing: [one]\n- Different: [one]\n", + "- Extra: [eins]\n- Different: {key: value}\n", + {"untranslated"}, + ) + assert result.missing_definitions == [] + assert result.extra_definitions == [] + assert result.type_mismatches == [] + + +def test_compare_without_filter_returns_all_definition_findings(tmp_path) -> None: + """An unfiltered comparison returns missing, extra, and type-mismatch findings.""" + result = compare( + tmp_path, + "- Missing: [one]\n- Different: [one]\n", + "- Extra: [eins]\n- Different: {key: value}\n", + ) + assert len(result.missing_definitions) == 1 + assert len(result.extra_definitions) == 1 + assert len(result.type_mismatches) == 1 + + +def test_compare_merges_region_definitions_by_name(tmp_path) -> None: + """A regional definition replaces the base definition with the same name.""" + source_path = tmp_path / "source.yaml" + target_path = tmp_path / "target.yaml" + target_region_path = tmp_path / "target-region.yaml" + source_path.write_text("- Base: [one]\n- Override: {key: value}\n", encoding="utf-8") + target_path.write_text("- Base: [eins]\n- Override: [wrong-kind]\n", encoding="utf-8") + target_region_path.write_text("- Override: {translated: value}\n", encoding="utf-8") + + result = compare_definition_files(source_path, target_path, target_region_path=target_region_path) + assert not result.has_findings diff --git a/PythonScripts/audit_translations/tests/test_differ.py b/PythonScripts/audit_translations/tests/test_differ.py index eb7bc3a2c..8ceeb8aed 100644 --- a/PythonScripts/audit_translations/tests/test_differ.py +++ b/PythonScripts/audit_translations/tests/test_differ.py @@ -3,7 +3,7 @@ """ from ..differ import diff_rules -from ..models import RuleDifference, RuleInfo +from ..models.rules import RuleDifference, RuleInfo def make_rule(name: str, tag: str, data) -> RuleInfo: diff --git a/PythonScripts/audit_translations/tests/test_line_resolver.py b/PythonScripts/audit_translations/tests/test_line_resolver.py index 569ce4ef5..133a1d2dc 100644 --- a/PythonScripts/audit_translations/tests/test_line_resolver.py +++ b/PythonScripts/audit_translations/tests/test_line_resolver.py @@ -3,7 +3,7 @@ """ from ..line_resolver import first_structure_mismatch, resolve_diff_lines -from ..models import RuleDifference, RuleInfo +from ..models.rules import RuleDifference, RuleInfo def _make_rule(name: str, line_map: dict, line_number: int = 1) -> RuleInfo: diff --git a/PythonScripts/audit_translations/tests/test_parsers.py b/PythonScripts/audit_translations/tests/test_parsers.py index e65139453..db7f31e1f 100644 --- a/PythonScripts/audit_translations/tests/test_parsers.py +++ b/PythonScripts/audit_translations/tests/test_parsers.py @@ -6,7 +6,7 @@ from ruamel.yaml import YAML from ruamel.yaml.scanner import ScannerError -from ..models import UntranslatedEntry +from ..models.rules import UntranslatedEntry from ..parsers import ( build_line_map, find_untranslated_text_entries,