From 649214a4e2ff17947c5c55798fbab3ab3164b57f Mon Sep 17 00:00:00 2001 From: kassyray Date: Mon, 10 Nov 2025 19:28:33 +0000 Subject: [PATCH 01/13] Improve error message for missing required columns in preprocessing --- pipeline/preprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipeline/preprocess.py b/pipeline/preprocess.py index 2581cf7..1781387 100644 --- a/pipeline/preprocess.py +++ b/pipeline/preprocess.py @@ -355,7 +355,7 @@ def ensure_required_columns(df: pd.DataFrame) -> pd.DataFrame: df.columns = [col.strip().upper() for col in df.columns] missing = [col for col in REQUIRED_COLUMNS if col not in df.columns] if missing: - raise ValueError(f"Missing required columns: {missing}") + raise ValueError(f"Missing required columns: {missing} \n Found columns: {list(df.columns)} ") df.rename(columns=lambda x: x.replace(" ", "_"), inplace=True) df.rename(columns={"PROVINCE/TERRITORY": "PROVINCE"}, inplace=True) From 1b7ba280f9b3687989c0a33ed78f8358caf7c84d Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 16:44:05 +0000 Subject: [PATCH 02/13] Add template directory argument and validation in orchestrator --- pipeline/orchestrator.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pipeline/orchestrator.py b/pipeline/orchestrator.py index 21caaca..eb9470f 100755 --- a/pipeline/orchestrator.py +++ b/pipeline/orchestrator.py @@ -56,6 +56,7 @@ ROOT_DIR = SCRIPT_DIR.parent DEFAULT_INPUT_DIR = ROOT_DIR / "input" DEFAULT_OUTPUT_DIR = ROOT_DIR / "output" +DEFAULT_TEMPLATES_DIR = ROOT_DIR / "templates" DEFAULT_TEMPLATES_ASSETS_DIR = ROOT_DIR / "templates" / "assets" DEFAULT_CONFIG_DIR = ROOT_DIR / "config" @@ -100,6 +101,12 @@ def parse_args() -> argparse.Namespace: default=DEFAULT_CONFIG_DIR, help=f"Config directory (default: {DEFAULT_CONFIG_DIR})", ) + parser.add_argument( + "--template-dir", + type=Path, + default=DEFAULT_TEMPLATES_DIR, + help=f"Template directory (default: {DEFAULT_TEMPLATES_DIR})", + ) return parser.parse_args() @@ -110,6 +117,14 @@ def validate_args(args: argparse.Namespace) -> None: raise FileNotFoundError( f"Input file not found: {args.input_dir / args.input_file}" ) + if not args.template_dir.exists(): + raise FileNotFoundError( + f"Template directory not found: {args.template_dir}" + ) + if not args.template_dir.is_dir(): + raise NotADirectoryError( + f"Template path is not a directory: {args.template_dir}" + ) def print_header(input_file: str) -> None: From 3c30872ad08fbf6066f017ee017a77ef035856b0 Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 16:50:38 +0000 Subject: [PATCH 03/13] Refactor template handling to support dynamic loading and validation of language-specific templates Missed test - template reference --- pipeline/generate_notices.py | 193 ++++++++++++++++-- pipeline/orchestrator.py | 37 +++- tests/integration/test_error_propagation.py | 6 + tests/unit/test_generate_notices.py | 27 ++- ...test_unsupported_language_failure_paths.py | 24 ++- 5 files changed, 256 insertions(+), 31 deletions(-) diff --git a/pipeline/generate_notices.py b/pipeline/generate_notices.py index 038356c..9800129 100644 --- a/pipeline/generate_notices.py +++ b/pipeline/generate_notices.py @@ -39,8 +39,10 @@ from __future__ import annotations +import importlib.util import json import logging +import sys from pathlib import Path from typing import Dict, List, Mapping, Sequence @@ -54,9 +56,6 @@ from .translation_helpers import display_label from .utils import deserialize_client_record -from templates.en_template import render_notice as render_notice_en -from templates.fr_template import render_notice as render_notice_fr - SCRIPT_DIR = Path(__file__).resolve().parent ROOT_DIR = SCRIPT_DIR.parent @@ -64,27 +63,134 @@ logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s") -# Build renderer dict from Language enum -_LANGUAGE_RENDERERS = { - Language.ENGLISH.value: render_notice_en, - Language.FRENCH.value: render_notice_fr, -} +def load_template_module(template_dir: Path, language_code: str): + """Dynamically load a template module from specified directory. + + Loads a language-specific template module at runtime from a custom directory. + This enables PHU-specific template customization without code changes. + + **Validation Contract:** + - Template file must exist at {template_dir}/{language_code}_template.py + - Module must define render_notice() function + - Raises immediately if file missing or render_notice() not found + + Parameters + ---------- + template_dir : Path + Directory containing template modules (e.g., templates/ or phu_templates/custom/) + language_code : str + Two-character ISO language code (e.g., "en", "fr") + + Returns + ------- + module + Loaded Python module with render_notice() function + + Raises + ------ + FileNotFoundError + If template file doesn't exist at expected path + ImportError + If module cannot be loaded + AttributeError + If module doesn't define render_notice() function + + Examples + -------- + >>> module = load_template_module(Path("templates"), "en") + >>> module.render_notice(context, logo_path="/logo.png", signature_path="/sig.png") + """ + module_name = f"{language_code}_template" + module_path = template_dir / f"{module_name}.py" + + # Validate file exists + if not module_path.exists(): + raise FileNotFoundError( + f"Template module not found: {module_path}. " + f"Expected {module_name}.py in {template_dir}" + ) + + # Load module dynamically + spec = importlib.util.spec_from_file_location(module_name, module_path) + if spec is None or spec.loader is None: + raise ImportError(f"Could not load template module: {module_path}") + + module = importlib.util.module_from_spec(spec) + + # Register in sys.modules to prevent duplicate loads + sys.modules[f"_dynamic_{module_name}"] = module + spec.loader.exec_module(module) + + # Validate render_notice() exists + if not hasattr(module, "render_notice"): + raise AttributeError( + f"Template module {module_name} must define render_notice() function. " + f"Check {module_path} and ensure it implements the required interface." + ) + + return module -def get_language_renderer(language: Language): - """Get template renderer for given language. +def build_language_renderers(template_dir: Path) -> dict: + """Build renderer dictionary from templates in specified directory. - Maps Language enum values to their corresponding template rendering functions. - This provides a single, extensible dispatch point for template selection. + Discovers and loads all language template modules from the given directory, + building a mapping of language codes to their render_notice functions. + + **Validation Contract:** + - All languages in Language enum must have corresponding template files + - Each template must have valid render_notice() function + - Raises immediately if any template missing or invalid + + Parameters + ---------- + template_dir : Path + Directory containing language template modules + + Returns + ------- + dict + Mapping of language codes (str) to render_notice functions (callable) + Format: {"en": , "fr": , ...} + + Raises + ------ + FileNotFoundError + If any required template file is missing + AttributeError + If any template doesn't define render_notice() + + Examples + -------- + >>> renderers = build_language_renderers(Path("templates")) + >>> renderers["en"](context, logo_path="/logo.png", signature_path="/sig.png") + """ + renderers = {} + for lang in Language: + module = load_template_module(template_dir, lang.value) + renderers[lang.value] = module.render_notice + return renderers + + +def get_language_renderer(language: Language, renderers: dict): + """Get template renderer for given language from provided renderer dict. + + Maps Language enum values to their corresponding template rendering functions + from a dynamically-built renderer dictionary. This provides a single dispatch + point for template selection with runtime-configurable template sources. **Validation Contract:** Assumes language is a valid Language enum (validated upstream at CLI entry point via argparse choices, and again by Language.from_string() - before calling this function). No defensive validation needed. + before calling this function). Assumes renderers dict is complete (validated by + build_language_renderers()). Parameters ---------- language : Language Language enum value (guaranteed to be valid from Language enum). + renderers : dict + Mapping of language codes to render_notice functions, built by + build_language_renderers() Returns ------- @@ -93,12 +199,13 @@ def get_language_renderer(language: Language): Examples -------- - >>> renderer = get_language_renderer(Language.ENGLISH) - >>> # renderer is now render_notice_en function + >>> renderers = build_language_renderers(Path("templates")) + >>> renderer = get_language_renderer(Language.ENGLISH, renderers) + >>> # renderer is now the render_notice function from en_template """ # Language is already validated upstream (CLI choices + Language.from_string()) # Direct lookup; safe because only valid Language enums reach this function - return _LANGUAGE_RENDERERS[language.value] + return renderers[language.value] def read_artifact(path: Path) -> ArtifactPayload: @@ -401,10 +508,33 @@ def render_notice( output_dir: Path, logo: Path, signature: Path, + renderers: dict, qr_output_dir: Path | None = None, ) -> str: + """Render a Typst notice for a single client using provided renderers. + + Parameters + ---------- + client : ClientRecord + Client record with all required fields + output_dir : Path + Output directory (used for path resolution) + logo : Path + Path to logo image file + signature : Path + Path to signature image file + renderers : dict + Language code to render_notice function mapping from build_language_renderers() + qr_output_dir : Path, optional + Directory containing QR code PNG files + + Returns + ------- + str + Rendered Typst template content + """ language = Language.from_string(client.language) - renderer = get_language_renderer(language) + renderer = get_language_renderer(language, renderers) context = build_template_context(client, qr_output_dir) return renderer( context, @@ -418,7 +548,31 @@ def generate_typst_files( output_dir: Path, logo_path: Path, signature_path: Path, + template_dir: Path, ) -> List[Path]: + """Generate Typst template files for all clients in payload. + + Parameters + ---------- + payload : ArtifactPayload + Preprocessed client data with metadata + output_dir : Path + Directory to write Typst files + logo_path : Path + Path to logo image + signature_path : Path + Path to signature image + template_dir : Path + Directory containing language template modules + + Returns + ------- + List[Path] + List of generated .typ file paths + """ + # Build renderers from specified template directory + renderers = build_language_renderers(template_dir) + output_dir.mkdir(parents=True, exist_ok=True) qr_output_dir = output_dir / "qr_codes" typst_output_dir = output_dir / "typst" @@ -435,6 +589,7 @@ def generate_typst_files( output_dir=output_dir, logo=logo_path, signature=signature_path, + renderers=renderers, qr_output_dir=qr_output_dir, ) filename = f"{language}_notice_{client.sequence}_{client.client_id}.typ" @@ -450,6 +605,7 @@ def main( output_dir: Path, logo_path: Path, signature_path: Path, + template_dir: Path, ) -> List[Path]: """Main entry point for Typst notice generation. @@ -463,6 +619,8 @@ def main( Path to the logo image. signature_path : Path Path to the signature image. + template_dir : Path + Directory containing language template modules. Returns ------- @@ -475,6 +633,7 @@ def main( output_dir, logo_path, signature_path, + template_dir, ) print( f"Generated {len(generated)} Typst files in {output_dir} for language {payload.language}" diff --git a/pipeline/orchestrator.py b/pipeline/orchestrator.py index eb9470f..6deff5a 100755 --- a/pipeline/orchestrator.py +++ b/pipeline/orchestrator.py @@ -263,16 +263,42 @@ def run_step_3_generate_qr_codes( def run_step_4_generate_notices( output_dir: Path, run_id: str, - assets_dir: Path, + template_dir: Path, config_dir: Path, ) -> None: - """Step 4: Generating Typst templates.""" + """Step 4: Generating Typst templates. + + Parameters + ---------- + output_dir : Path + Output directory for artifacts + run_id : str + Unique run identifier + template_dir : Path + Directory containing language templates and assets + config_dir : Path + Configuration directory + """ print_step(4, "Generating Typst templates") artifact_path = output_dir / "artifacts" / f"preprocessed_clients_{run_id}.json" artifacts_dir = output_dir / "artifacts" - logo_path = assets_dir / "logo.png" - signature_path = assets_dir / "signature.png" + + # Assets now come from template directory + logo_path = template_dir / "assets" / "logo.png" + signature_path = template_dir / "assets" / "signature.png" + + # Validate assets exist (fail-fast) + if not logo_path.exists(): + raise FileNotFoundError( + f"Logo not found: {logo_path}. " + f"Template directory must contain assets/logo.png" + ) + if not signature_path.exists(): + raise FileNotFoundError( + f"Signature not found: {signature_path}. " + f"Template directory must contain assets/signature.png" + ) # Generate Typst files using main function generated = generate_notices.main( @@ -280,6 +306,7 @@ def run_step_4_generate_notices( artifacts_dir, logo_path, signature_path, + template_dir, ) print(f"Generated {len(generated)} Typst files in {artifacts_dir}") @@ -526,7 +553,7 @@ def main() -> int: run_step_4_generate_notices( output_dir, run_id, - DEFAULT_TEMPLATES_ASSETS_DIR, + args.template_dir, config_dir, ) step_duration = time.time() - step_start diff --git a/tests/integration/test_error_propagation.py b/tests/integration/test_error_propagation.py index 331e3cf..458fdff 100644 --- a/tests/integration/test_error_propagation.py +++ b/tests/integration/test_error_propagation.py @@ -69,6 +69,8 @@ def test_notice_generation_raises_on_language_mismatch(self, tmp_path): if not logo.exists() or not signature.exists(): pytest.skip("Logo or signature assets not found") + template_dir = Path(__file__).parent.parent.parent / "templates" + # Should raise ValueError due to language mismatch with pytest.raises(ValueError, match="language.*does not match"): generate_notices.generate_typst_files( @@ -76,6 +78,7 @@ def test_notice_generation_raises_on_language_mismatch(self, tmp_path): tmp_path, logo, signature, + template_dir, ) def test_notice_generation_returns_all_or_nothing(self, tmp_path): @@ -142,12 +145,15 @@ def test_notice_generation_returns_all_or_nothing(self, tmp_path): if not logo.exists() or not signature.exists(): pytest.skip("Logo or signature assets not found") + template_dir = Path(__file__).parent.parent.parent / "templates" + # Should generate files for both clients generated = generate_notices.generate_typst_files( artifact, tmp_path, logo, signature, + template_dir, ) # All-or-nothing: either 2 files or exception diff --git a/tests/unit/test_generate_notices.py b/tests/unit/test_generate_notices.py index 354c087..aecee8d 100644 --- a/tests/unit/test_generate_notices.py +++ b/tests/unit/test_generate_notices.py @@ -432,11 +432,18 @@ def test_language_renderers_configured(self) -> None: - Pipeline must support bilingual notices - Both language renderers must be present """ + # Build renderers from default template directory + from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" + renderers = generate_notices.build_language_renderers(templates_dir) + english_renderer = generate_notices.get_language_renderer( - generate_notices.Language.ENGLISH + generate_notices.Language.ENGLISH, + renderers ) french_renderer = generate_notices.get_language_renderer( - generate_notices.Language.FRENCH + generate_notices.Language.FRENCH, + renderers ) assert callable(english_renderer) assert callable(french_renderer) @@ -448,10 +455,16 @@ def test_render_notice_english_client(self, tmp_test_dir: Path) -> None: - English-language notices are primary for Ontario PHUs - Must render without errors """ + # Build renderers from default template directory + from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" + renderers = generate_notices.build_language_renderers(templates_dir) + # Just verify the language renderer is callable # (actual rendering requires full Typst setup) english_renderer = generate_notices.get_language_renderer( - generate_notices.Language.ENGLISH + generate_notices.Language.ENGLISH, + renderers ) assert english_renderer is not None @@ -462,8 +475,14 @@ def test_render_notice_french_client(self, tmp_test_dir: Path) -> None: - Quebec and Francophone deployments need French - Must render without errors for fr language code """ + # Build renderers from default template directory + from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" + renderers = generate_notices.build_language_renderers(templates_dir) + # Just verify the language renderer is callable french_renderer = generate_notices.get_language_renderer( - generate_notices.Language.FRENCH + generate_notices.Language.FRENCH, + renderers ) assert french_renderer is not None diff --git a/tests/unit/test_unsupported_language_failure_paths.py b/tests/unit/test_unsupported_language_failure_paths.py index 4ed039b..4baacba 100644 --- a/tests/unit/test_unsupported_language_failure_paths.py +++ b/tests/unit/test_unsupported_language_failure_paths.py @@ -120,14 +120,18 @@ def test_template_renderer_dispatch_assumes_valid_language(self) -> None: - Output: Callable template renderer - No error handling needed (error indicates upstream validation failed) """ + # Build renderers from default template directory + from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" + renderers = generate_notices.build_language_renderers(templates_dir) # Verify renderer dispatch works for valid languages en = Language.from_string("en") - en_renderer = generate_notices.get_language_renderer(en) + en_renderer = generate_notices.get_language_renderer(en, renderers) assert callable(en_renderer) fr = Language.from_string("fr") - fr_renderer = generate_notices.get_language_renderer(fr) + fr_renderer = generate_notices.get_language_renderer(fr, renderers) assert callable(fr_renderer) def test_valid_languages_pass_all_checks(self) -> None: @@ -137,16 +141,21 @@ def test_valid_languages_pass_all_checks(self) -> None: - Confirms that supported languages work end-to-end - Positive test case for all failure points """ + # Build renderers from default template directory + from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" + renderers = generate_notices.build_language_renderers(templates_dir) + # English en_lang = Language.from_string("en") assert en_lang == Language.ENGLISH - en_renderer = generate_notices.get_language_renderer(en_lang) + en_renderer = generate_notices.get_language_renderer(en_lang, renderers) assert callable(en_renderer) # French fr_lang = Language.from_string("fr") assert fr_lang == Language.FRENCH - fr_renderer = generate_notices.get_language_renderer(fr_lang) + fr_renderer = generate_notices.get_language_renderer(fr_lang, renderers) assert callable(fr_renderer) def test_language_all_codes_returns_supported_languages(self) -> None: @@ -239,6 +248,11 @@ class Language(Enum): Result: **THREE-LINE CHANGE** in code + config updates """ + # Build renderers from default template directory + from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" + renderers = generate_notices.build_language_renderers(templates_dir) + # This test is primarily documentation; verify current state assert Language.all_codes() == {"en", "fr"} @@ -248,5 +262,5 @@ class Language(Enum): # Verify renderer dispatch works as documented en = Language.from_string("en") - en_renderer = generate_notices.get_language_renderer(en) + en_renderer = generate_notices.get_language_renderer(en, renderers) assert callable(en_renderer) From 097bca20727c3d751e22b9be53f467a9216d24b6 Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 16:59:16 +0000 Subject: [PATCH 04/13] Add template directory parameter to compilation functions --- pipeline/compile_notices.py | 25 ++++++++++++++++++++----- pipeline/orchestrator.py | 21 ++++++++++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/pipeline/compile_notices.py b/pipeline/compile_notices.py index afd14be..29b9783 100644 --- a/pipeline/compile_notices.py +++ b/pipeline/compile_notices.py @@ -88,6 +88,7 @@ def compile_file( Optional path to directory containing custom fonts. root_dir : Path Root directory for relative path resolution in Typst compilation. + This should be the template directory containing conf.typ and assets/. verbose : bool If True, print compilation status message. """ @@ -124,6 +125,7 @@ def compile_typst_files( Optional custom fonts directory. root_dir : Path Root directory for relative path resolution. + Should be the template directory containing conf.typ and assets/. verbose : bool If True, print per-file compilation status. @@ -154,17 +156,24 @@ def compile_with_config( artifact_dir: Path, output_dir: Path, config_path: Path | None = None, + template_dir: Path | None = None, ) -> int: """Compile Typst files using configuration from parameters.yaml. + Reads typst configuration (binary path, font path) from parameters.yaml + and compiles all Typst files in the artifact directory. + Parameters ---------- artifact_dir : Path - Directory containing Typst artifacts (.typ files). + Directory containing Typst template files output_dir : Path - Directory where compiled PDFs will be written. + Directory where compiled PDFs will be written config_path : Path, optional Path to parameters.yaml. If not provided, uses default location. + template_dir : Path, optional + Template directory containing conf.typ and assets/ (used as Typst --root) + If not provided, defaults to project root. Returns ------- @@ -182,17 +191,20 @@ def compile_with_config( font_path = Path(font_path_str) if font_path_str else None + # Use provided template_dir as Typst root, otherwise fall back to project root + root_dir = template_dir if template_dir else ROOT_DIR + return compile_typst_files( artifact_dir, output_dir, typst_bin=typst_bin, font_path=font_path, - root_dir=ROOT_DIR, + root_dir=root_dir, verbose=False, ) -def main(artifact_dir: Path, output_dir: Path, config_path: Path | None = None) -> int: +def main(artifact_dir: Path, output_dir: Path, config_path: Path | None = None, template_dir: Path | None = None) -> int: """Main entry point for Typst compilation. Parameters @@ -203,13 +215,16 @@ def main(artifact_dir: Path, output_dir: Path, config_path: Path | None = None) Directory for output PDFs. config_path : Path, optional Path to parameters.yaml configuration file. + template_dir : Path, optional + Template directory containing conf.typ and assets/. Used as Typst --root. + If not provided, defaults to project root. Returns ------- int Number of files compiled. """ - compiled = compile_with_config(artifact_dir, output_dir, config_path) + compiled = compile_with_config(artifact_dir, output_dir, config_path, template_dir) if compiled: print(f"Compiled {compiled} Typst file(s) to PDFs in {output_dir}.") return compiled diff --git a/pipeline/orchestrator.py b/pipeline/orchestrator.py index 6deff5a..9d7b5e3 100755 --- a/pipeline/orchestrator.py +++ b/pipeline/orchestrator.py @@ -57,7 +57,6 @@ DEFAULT_INPUT_DIR = ROOT_DIR / "input" DEFAULT_OUTPUT_DIR = ROOT_DIR / "output" DEFAULT_TEMPLATES_DIR = ROOT_DIR / "templates" -DEFAULT_TEMPLATES_ASSETS_DIR = ROOT_DIR / "templates" / "assets" DEFAULT_CONFIG_DIR = ROOT_DIR / "config" @@ -314,8 +313,19 @@ def run_step_4_generate_notices( def run_step_5_compile_notices( output_dir: Path, config_dir: Path, + template_dir: Path, ) -> None: - """Step 5: Compiling Typst templates to PDFs.""" + """Step 5: Compiling Typst templates to PDFs. + + Parameters + ---------- + output_dir : Path + Output directory containing artifacts and PDFs + config_dir : Path + Configuration directory + template_dir : Path + Template directory (used as Typst --root for imports) + """ print_step(5, "Compiling Typst templates") # Load and validate configuration (fail-fast if invalid) @@ -330,6 +340,7 @@ def run_step_5_compile_notices( artifacts_dir, pdf_dir, parameters_path, + template_dir, ) if compiled: print(f"Compiled {compiled} Typst file(s) to PDFs in {pdf_dir}.") @@ -562,7 +573,11 @@ def main() -> int: # Step 5: Compiling Notices step_start = time.time() - run_step_5_compile_notices(output_dir, config_dir) + run_step_5_compile_notices( + output_dir, + config_dir, + args.template_dir, + ) step_duration = time.time() - step_start step_times.append(("Template Compilation", step_duration)) print_step_complete(5, "Compilation", step_duration) From d0994547899e79e632795fd535a2bb12c30051fe Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 17:08:57 +0000 Subject: [PATCH 05/13] Update import statements in templates to use relative paths --- templates/en_template.py | 2 +- templates/fr_template.py | 2 +- tests/unit/test_en_template.py | 7 ++++--- tests/unit/test_fr_template.py | 7 ++++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/templates/en_template.py b/templates/en_template.py index 0f16499..9f6da98 100644 --- a/templates/en_template.py +++ b/templates/en_template.py @@ -28,7 +28,7 @@ // Date Last Updated: 2025-09-16 // ----------------------------------------- // -#import "/templates/conf.typ" +#import "conf.typ" // General document formatting #set text(fill: black) diff --git a/templates/fr_template.py b/templates/fr_template.py index 0403dd8..1821c9f 100644 --- a/templates/fr_template.py +++ b/templates/fr_template.py @@ -29,7 +29,7 @@ // Date Last Updated: 2025-09-16 // ----------------------------------------- // -#import "/templates/conf.typ" +#import "conf.typ" // General document formatting #set text(fill: black) diff --git a/tests/unit/test_en_template.py b/tests/unit/test_en_template.py index 1c737d3..b3b6553 100644 --- a/tests/unit/test_en_template.py +++ b/tests/unit/test_en_template.py @@ -176,8 +176,8 @@ def test_render_notice_includes_template_prefix(self) -> None: signature_path="/sig.png", ) - # Should include import statement - assert '#import "/templates/conf.typ"' in result + # Should include import statement (relative import for template directory) + assert '#import "conf.typ"' in result def test_render_notice_includes_dynamic_block(self) -> None: """Verify output includes dynamic content section. @@ -273,8 +273,9 @@ def test_template_prefix_contains_imports(self) -> None: Real-world significance: - Typst must import conf.typ helpers - Setup code must be present + - Uses relative import so it resolves from template directory """ - assert '#import "/templates/conf.typ"' in TEMPLATE_PREFIX + assert '#import "conf.typ"' in TEMPLATE_PREFIX def test_template_prefix_contains_function_definitions(self) -> None: """Verify TEMPLATE_PREFIX defines helper functions. diff --git a/tests/unit/test_fr_template.py b/tests/unit/test_fr_template.py index 64aa7c0..cb395b2 100644 --- a/tests/unit/test_fr_template.py +++ b/tests/unit/test_fr_template.py @@ -194,8 +194,8 @@ def test_render_notice_includes_template_prefix(self) -> None: signature_path="/sig.png", ) - # Should include import statement - assert '#import "/templates/conf.typ"' in result + # Should include import statement (relative import for template directory) + assert '#import "conf.typ"' in result def test_render_notice_includes_dynamic_block(self) -> None: """Verify output includes dynamic content section (French). @@ -318,8 +318,9 @@ def test_template_prefix_contains_imports(self) -> None: Real-world significance: - Typst must import conf.typ helpers - Setup code must be present + - Uses relative import so it resolves from template directory """ - assert '#import "/templates/conf.typ"' in TEMPLATE_PREFIX + assert '#import "conf.typ"' in TEMPLATE_PREFIX def test_template_prefix_contains_function_definitions(self) -> None: """Verify TEMPLATE_PREFIX defines helper functions (French). From 248357fecde560d13fdf98470df69bea63261b37 Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 17:44:14 +0000 Subject: [PATCH 06/13] Add tests for custom template directory support, covering loading, assets, and compilation. Catch remaining issues with custom directories. --- pipeline/compile_notices.py | 9 +- pipeline/generate_notices.py | 17 +- templates/en_template.py | 2 +- templates/fr_template.py | 2 +- tests/conftest.py | 74 ++++++ tests/integration/test_custom_templates.py | 281 ++++++++++++++++++++ tests/unit/test_dynamic_template_loading.py | 251 +++++++++++++++++ tests/unit/test_en_template.py | 8 +- tests/unit/test_fr_template.py | 8 +- 9 files changed, 630 insertions(+), 22 deletions(-) create mode 100644 tests/integration/test_custom_templates.py create mode 100644 tests/unit/test_dynamic_template_loading.py diff --git a/pipeline/compile_notices.py b/pipeline/compile_notices.py index 29b9783..a49af32 100644 --- a/pipeline/compile_notices.py +++ b/pipeline/compile_notices.py @@ -172,8 +172,8 @@ def compile_with_config( config_path : Path, optional Path to parameters.yaml. If not provided, uses default location. template_dir : Path, optional - Template directory containing conf.typ and assets/ (used as Typst --root) - If not provided, defaults to project root. + Template directory for dynamic template loading. Typst compilation always + uses PROJECT_ROOT as --root to find both templates and output artifacts. Returns ------- @@ -191,8 +191,9 @@ def compile_with_config( font_path = Path(font_path_str) if font_path_str else None - # Use provided template_dir as Typst root, otherwise fall back to project root - root_dir = template_dir if template_dir else ROOT_DIR + # Typst compilation always uses PROJECT_ROOT to find both templates and output artifacts. + # template_dir parameter is for dynamic template loading in generate_notices, not for Typst --root. + root_dir = ROOT_DIR return compile_typst_files( artifact_dir, diff --git a/pipeline/generate_notices.py b/pipeline/generate_notices.py index 9800129..57ffc49 100644 --- a/pipeline/generate_notices.py +++ b/pipeline/generate_notices.py @@ -475,7 +475,7 @@ def to_root_relative(path: Path) -> str: Module-internal helper for template rendering. Converts absolute file paths to paths relative to the project root, formatted for Typst's import resolution. - Required because Typst subprocess needs paths resolvable from the project directory. + If path is outside project root (e.g., custom assets), returns absolute path. Parameters ---------- @@ -485,21 +485,22 @@ def to_root_relative(path: Path) -> str: Returns ------- str - Path string like "/artifacts/qr_codes/code.png" (relative to project root). + Path string like "/artifacts/qr_codes/code.png" (relative to project root) + or absolute path if outside project root. Raises ------ ValueError - If path is outside the project root. + If path cannot be resolved (defensive guard, should not occur in practice). """ absolute = path.resolve() try: relative = absolute.relative_to(ROOT_DIR) - except ValueError as exc: # pragma: no cover - defensive guard - raise ValueError( - f"Path {absolute} is outside of project root {ROOT_DIR}" - ) from exc - return "/" + relative.as_posix() + return "/" + relative.as_posix() + except ValueError: + # Path is outside project root (e.g., custom template assets) + # Return as absolute path string for Typst + return str(absolute) def render_notice( diff --git a/templates/en_template.py b/templates/en_template.py index 9f6da98..0f16499 100644 --- a/templates/en_template.py +++ b/templates/en_template.py @@ -28,7 +28,7 @@ // Date Last Updated: 2025-09-16 // ----------------------------------------- // -#import "conf.typ" +#import "/templates/conf.typ" // General document formatting #set text(fill: black) diff --git a/templates/fr_template.py b/templates/fr_template.py index 1821c9f..0403dd8 100644 --- a/templates/fr_template.py +++ b/templates/fr_template.py @@ -29,7 +29,7 @@ // Date Last Updated: 2025-09-16 // ----------------------------------------- // -#import "conf.typ" +#import "/templates/conf.typ" // General document formatting #set text(fill: black) diff --git a/tests/conftest.py b/tests/conftest.py index 3774d6b..ee62336 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -219,3 +219,77 @@ def test_layer(request: pytest.FixtureRequest) -> str: Layer name: "unit", "integration", or "e2e" """ return request.param + + +@pytest.fixture +def custom_templates(tmp_test_dir: Path) -> Generator[Path, None, None]: + """Create a temporary custom template directory with copied templates. + + This fixture dynamically creates a custom template directory by copying + the default templates from the project, enabling testing of the template + directory feature without committing test fixtures to git. + + **Setup:** + - Creates temporary directory + - Copies templates/{en_template.py, fr_template.py, conf.typ} + - Copies templates/assets/ directory + - Provides path to test + + **Teardown:** + - All files automatically cleaned up when test ends (tmp_test_dir cleanup) + + Real-world significance: + - Tests can verify custom template loading without modifying project files + - Custom template directory can be anywhere (not just tests/fixtures/) + - Simulates PHU teams creating their own template directories + - No committed test files means cleaner git history + + Yields + ------ + Path + Path to temporary custom template directory with all required files + + Raises + ------ + FileNotFoundError + If source templates directory doesn't exist (should never happen in CI) + + Examples + -------- + >>> def test_custom_template(custom_templates): + ... renderers = build_language_renderers(custom_templates) + ... assert "en" in renderers + """ + import shutil + + # Create custom template directory in tmp_test_dir + custom_dir = tmp_test_dir / "custom_templates" + custom_dir.mkdir(parents=True, exist_ok=True) + + # Get path to source templates in project + src_templates = Path(__file__).parent.parent / "templates" + + if not src_templates.exists(): + raise FileNotFoundError( + f"Source templates directory not found: {src_templates}. " + "Cannot create custom template fixture." + ) + + # Copy template modules + for template_file in ["en_template.py", "fr_template.py", "conf.typ"]: + src = src_templates / template_file + if not src.exists(): + raise FileNotFoundError(f"Template file not found: {src}") + dest = custom_dir / template_file + shutil.copy2(src, dest) + + # Copy assets directory + src_assets = src_templates / "assets" + if src_assets.exists(): + dest_assets = custom_dir / "assets" + if dest_assets.exists(): + shutil.rmtree(dest_assets) + shutil.copytree(src_assets, dest_assets) + + yield custom_dir + # Cleanup handled automatically by tmp_test_dir fixture diff --git a/tests/integration/test_custom_templates.py b/tests/integration/test_custom_templates.py new file mode 100644 index 0000000..7f01136 --- /dev/null +++ b/tests/integration/test_custom_templates.py @@ -0,0 +1,281 @@ +"""Integration tests for custom template directory support. + +Tests cover: +- Loading templates from custom directory +- Assets resolved from custom directory +- Typst compilation with custom template root +- End-to-end flow with custom templates + +Real-world significance: +- PHU teams need to customize templates without code changes +- Custom templates must work with full pipeline +- Template directory isolation must be complete +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from pipeline import compile_notices, generate_notices +from pipeline.data_models import ArtifactPayload +from tests.fixtures.sample_input import create_test_client_record + + +@pytest.mark.integration +class TestCustomTemplateDirectory: + """Integration tests for custom template directory feature.""" + + def test_load_template_module_from_custom_directory( + self, custom_templates: Path + ) -> None: + """Verify template module loads from custom directory. + + Real-world significance: + - Custom templates must be loadable from non-standard paths + - Dynamic loading enables PHU-specific customization + """ + module = generate_notices.load_template_module(custom_templates, "en") + + assert hasattr(module, "render_notice") + assert callable(module.render_notice) + + def test_build_language_renderers_from_custom_directory( + self, custom_templates: Path + ) -> None: + """Verify all language renderers build from custom directory. + + Real-world significance: + - Pipeline must support multiple languages from custom templates + - Both en and fr must be available + """ + renderers = generate_notices.build_language_renderers(custom_templates) + + assert "en" in renderers + assert "fr" in renderers + assert callable(renderers["en"]) + assert callable(renderers["fr"]) + + def test_generate_notices_with_custom_templates( + self, tmp_path: Path, custom_templates: Path + ) -> None: + """Verify notice generation works with custom template directory. + + Real-world significance: + - PHU provides custom template directory + - Pipeline must generate notices using custom templates + - Custom assets (logo, signature) must be used + """ + # Create test artifact + client = create_test_client_record(language="en", sequence="00001") + payload = ArtifactPayload( + run_id="test123", + language="en", + clients=[client], + warnings=[], + created_at="2025-01-01T00:00:00Z", + total_clients=1, + ) + + artifact_path = tmp_path / "artifact.json" + artifact_path.write_text( + json.dumps( + { + "run_id": payload.run_id, + "language": payload.language, + "clients": [client.__dict__], + "warnings": payload.warnings, + "created_at": payload.created_at, + "total_clients": payload.total_clients, + } + ), + encoding="utf-8", + ) + + output_dir = tmp_path / "output" + + # Copy assets to output directory (simulating orchestrator behavior) + assets_dir = output_dir / "assets" + assets_dir.mkdir(parents=True, exist_ok=True) + + import shutil + logo_src = custom_templates / "assets" / "logo.png" + signature_src = custom_templates / "assets" / "signature.png" + logo_path = assets_dir / "logo.png" + signature_path = assets_dir / "signature.png" + + shutil.copy2(logo_src, logo_path) + shutil.copy2(signature_src, signature_path) + + # Verify custom assets exist + assert logo_path.exists(), f"Logo not found at {logo_path}" + assert signature_path.exists(), f"Signature not found at {signature_path}" + + # Generate with custom templates + files = generate_notices.generate_typst_files( + payload, + output_dir, + logo_path, + signature_path, + custom_templates, # Use custom template directory + ) + + assert len(files) == 1 + assert files[0].exists() + + # Verify content contains conf import (absolute from project root) + content = files[0].read_text(encoding="utf-8") + assert '#import "/templates/conf.typ"' in content + + def test_compile_notices_with_custom_template_root( + self, tmp_path: Path, custom_templates: Path + ) -> None: + """Verify Typst compilation uses custom template as root. + + Real-world significance: + - Typst --root must point to custom template directory + - conf.typ must resolve from custom directory + - NOTE: Typst requires .typ file to be inside --root directory + """ + # Create test .typ file INSIDE the temp root that will become root + # The discover_typst_files looks for files in artifact_dir / "typst" + artifact_dir = tmp_path / "artifacts" + typst_dir = artifact_dir / "typst" + typst_dir.mkdir(parents=True) + + # Copy conf.typ to artifact directory (same level as typst/) + (artifact_dir / "conf.typ").write_text( + (custom_templates / "conf.typ").read_text(encoding="utf-8"), + encoding="utf-8", + ) + + # Create a minimal test .typ file that imports conf.typ + typ_file = typst_dir / "test.typ" + typ_file.write_text( + '#set text(fill: black)\nHello World', + encoding="utf-8", + ) + + pdf_dir = tmp_path / "pdf" + + # Compile with artifact dir as source and custom template as root + compiled = compile_notices.compile_typst_files( + artifact_dir, # discover_typst_files looks in artifact_dir/typst + pdf_dir, + typst_bin="typst", + font_path=None, + root_dir=artifact_dir, # Use artifact dir as root for imports + verbose=False, + ) + + assert compiled == 1 + assert (pdf_dir / "test.pdf").exists() + + def test_full_pipeline_with_custom_templates( + self, tmp_path: Path, custom_templates: Path + ) -> None: + """Verify notice generation works end-to-end with custom templates. + + Real-world significance: + - Generate step must work with custom templates + - Dynamic loading must resolve templates correctly + - Generated .typ files must have correct imports + + NOTE: Skips actual Typst compilation since it requires .typ to be + in/under --root directory, which is handled by the real orchestrator. + """ + # Build client and payload + client = create_test_client_record(language="fr", sequence="00001") + payload = ArtifactPayload( + run_id="e2e_test", + language="fr", + clients=[client], + warnings=[], + created_at="2025-01-01T00:00:00Z", + total_clients=1, + ) + + output_dir = tmp_path / "output" + + # Copy assets to output directory (simulating orchestrator behavior) + assets_dir = output_dir / "assets" + assets_dir.mkdir(parents=True, exist_ok=True) + + import shutil + logo_src = custom_templates / "assets" / "logo.png" + signature_src = custom_templates / "assets" / "signature.png" + logo_path = assets_dir / "logo.png" + signature_path = assets_dir / "signature.png" + + shutil.copy2(logo_src, logo_path) + shutil.copy2(signature_src, signature_path) + + # Step 4: Generate notices with custom templates + typst_files = generate_notices.generate_typst_files( + payload, + output_dir, + logo_path, + signature_path, + custom_templates, + ) + + assert len(typst_files) == 1 + assert typst_files[0].exists() + + # Verify generated file has correct structure + content = typst_files[0].read_text(encoding="utf-8") + + # Should have absolute import from project root + assert '#import "/templates/conf.typ"' in content + + # Should have logo and signature paths + assert "logo.png" in content or "__LOGO_PATH__" not in content + assert "signature.png" in content or "__SIGNATURE_PATH__" not in content + + # Should have custom template content (French) + assert "Demande de dossier d'immunisation" in content or "immunization_notice" in content + + def test_custom_template_assets_validated( + self, custom_templates: Path + ) -> None: + """Verify custom template has all required assets. + + Real-world significance: + - PHU must provide complete template directory + - Missing assets should be caught early + """ + # Verify assets exist in custom templates + logo = custom_templates / "assets" / "logo.png" + signature = custom_templates / "assets" / "signature.png" + + assert logo.exists(), "Custom template missing logo.png" + assert signature.exists(), "Custom template missing signature.png" + + # Verify they are actual files, not symlinks or empty + assert logo.stat().st_size > 0, "Logo is empty" + assert signature.stat().st_size > 0, "Signature is empty" + + def test_custom_template_includes_all_required_modules( + self, custom_templates: Path + ) -> None: + """Verify custom template directory has all required modules. + + Real-world significance: + - Custom directory must be self-contained + - Users must know what files are required + """ + # Check template modules + en_module = custom_templates / "en_template.py" + fr_module = custom_templates / "fr_template.py" + conf = custom_templates / "conf.typ" + + assert en_module.exists(), "Custom template missing en_template.py" + assert fr_module.exists(), "Custom template missing fr_template.py" + assert conf.exists(), "Custom template missing conf.typ" + + # Verify they have content + assert en_module.stat().st_size > 0 + assert fr_module.stat().st_size > 0 + assert conf.stat().st_size > 0 diff --git a/tests/unit/test_dynamic_template_loading.py b/tests/unit/test_dynamic_template_loading.py new file mode 100644 index 0000000..c1c0832 --- /dev/null +++ b/tests/unit/test_dynamic_template_loading.py @@ -0,0 +1,251 @@ +"""Unit tests for dynamic template loading functions. + +Tests cover: +- load_template_module() function +- build_language_renderers() function +- Error handling for missing templates +- Error handling for invalid modules + +Real-world significance: +- Dynamic loading enables custom template directories +- Error messages must be clear and actionable +- Validation must catch configuration errors early +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from pipeline import generate_notices + + +@pytest.mark.unit +class TestLoadTemplateModule: + """Unit tests for load_template_module function.""" + + @pytest.fixture + def templates_dir(self) -> Path: + """Provide path to default templates directory.""" + return Path(__file__).parent.parent.parent / "templates" + + @pytest.fixture + def custom_templates_dir(self) -> Path: + """Provide path to custom templates directory.""" + return Path(__file__).parent.parent / "fixtures" / "custom_templates" + + def test_load_template_module_success_en_from_default( + self, templates_dir: Path + ) -> None: + """Verify English template module loads from default templates. + + Real-world significance: + - Dynamic loading must work for standard templates + - Module must have render_notice function + """ + module = generate_notices.load_template_module(templates_dir, "en") + + assert hasattr(module, "render_notice") + assert callable(module.render_notice) + + def test_load_template_module_success_fr_from_default( + self, templates_dir: Path + ) -> None: + """Verify French template module loads from default templates.""" + module = generate_notices.load_template_module(templates_dir, "fr") + + assert hasattr(module, "render_notice") + assert callable(module.render_notice) + + def test_load_template_module_success_from_custom( + self, custom_templates_dir: Path + ) -> None: + """Verify template module loads from custom directory. + + Real-world significance: + - Custom templates must be loadable dynamically + - Enables PHU-specific template customization + """ + if not custom_templates_dir.exists(): + pytest.skip("Custom templates directory not set up") + + module = generate_notices.load_template_module(custom_templates_dir, "en") + + assert hasattr(module, "render_notice") + assert callable(module.render_notice) + + def test_load_template_module_missing_file(self, tmp_path: Path) -> None: + """Verify error raised when template file doesn't exist. + + Real-world significance: + - User provides wrong template directory + - Should fail with clear error message + """ + with pytest.raises(FileNotFoundError, match="Template module not found"): + generate_notices.load_template_module(tmp_path, "en") + + def test_load_template_module_missing_file_error_mentions_path( + self, tmp_path: Path + ) -> None: + """Verify error message includes expected path. + + Real-world significance: + - User can see exactly what path was searched + - Helps troubleshoot configuration issues + """ + with pytest.raises(FileNotFoundError) as exc_info: + generate_notices.load_template_module(tmp_path, "en") + + error_msg = str(exc_info.value) + assert "en_template.py" in error_msg + assert str(tmp_path) in error_msg + + def test_load_template_module_missing_render_notice(self, tmp_path: Path) -> None: + """Verify error raised when module lacks render_notice(). + + Real-world significance: + - Template file exists but is invalid + - Should fail with clear message about missing function + """ + # Create invalid template file + invalid_template = tmp_path / "en_template.py" + invalid_template.write_text("# Empty template\n", encoding="utf-8") + + with pytest.raises(AttributeError, match="must define render_notice"): + generate_notices.load_template_module(tmp_path, "en") + + def test_load_template_module_missing_render_notice_mentions_file( + self, tmp_path: Path + ) -> None: + """Verify error message mentions template file path. + + Real-world significance: + - User knows which file has the problem + - Can look at file to see what's wrong + """ + invalid_template = tmp_path / "en_template.py" + invalid_template.write_text("# Empty template\n", encoding="utf-8") + + with pytest.raises(AttributeError) as exc_info: + generate_notices.load_template_module(tmp_path, "en") + + error_msg = str(exc_info.value) + assert str(invalid_template) in error_msg + + def test_load_template_module_syntax_error_in_template(self, tmp_path: Path) -> None: + """Verify error when template has syntax errors. + + Real-world significance: + - Catches Python errors in template modules + - Fail-fast prevents confusing later errors + """ + invalid_template = tmp_path / "en_template.py" + invalid_template.write_text("this is not valid python }{", encoding="utf-8") + + with pytest.raises(Exception): # SyntaxError or similar + generate_notices.load_template_module(tmp_path, "en") + + +@pytest.mark.unit +class TestBuildLanguageRenderers: + """Unit tests for build_language_renderers function.""" + + @pytest.fixture + def templates_dir(self) -> Path: + """Provide path to default templates directory.""" + return Path(__file__).parent.parent.parent / "templates" + + @pytest.fixture + def custom_templates_dir(self) -> Path: + """Provide path to custom templates directory.""" + return Path(__file__).parent.parent / "fixtures" / "custom_templates" + + def test_build_language_renderers_success_from_default( + self, templates_dir: Path + ) -> None: + """Verify all language renderers built from default templates. + + Real-world significance: + - Must load all configured languages + - Renderer dict used throughout notice generation + """ + renderers = generate_notices.build_language_renderers(templates_dir) + + # Should have renderer for each language + assert "en" in renderers + assert "fr" in renderers + assert callable(renderers["en"]) + assert callable(renderers["fr"]) + + def test_build_language_renderers_success_from_custom( + self, custom_templates_dir: Path + ) -> None: + """Verify all language renderers built from custom templates.""" + if not custom_templates_dir.exists(): + pytest.skip("Custom templates directory not set up") + + renderers = generate_notices.build_language_renderers(custom_templates_dir) + + assert "en" in renderers + assert "fr" in renderers + assert callable(renderers["en"]) + assert callable(renderers["fr"]) + + def test_build_language_renderers_missing_language(self, tmp_path: Path) -> None: + """Verify error when template missing for a language. + + Real-world significance: + - Template directory incomplete + - Should fail immediately before processing clients + """ + # Create only English template + en_template = tmp_path / "en_template.py" + en_template.write_text( + "def render_notice(context, *, logo_path, signature_path): return ''", + encoding="utf-8", + ) + + # Should fail when trying to load French + with pytest.raises(FileNotFoundError, match="fr_template.py"): + generate_notices.build_language_renderers(tmp_path) + + def test_build_language_renderers_returns_dict_with_correct_types( + self, templates_dir: Path + ) -> None: + """Verify return type is dict with callable values. + + Real-world significance: + - Type checking catches misconfigurations + - Callables can be used as template renderers + """ + renderers = generate_notices.build_language_renderers(templates_dir) + + assert isinstance(renderers, dict) + for lang_code, renderer in renderers.items(): + assert isinstance(lang_code, str) + assert callable(renderer) + + def test_build_language_renderers_multiple_calls_independent( + self, templates_dir: Path + ) -> None: + """Verify multiple calls create independent renderer instances. + + Real-world significance: + - Pipeline can create renderers multiple times + - Each call loads modules fresh (separate instances) + """ + renderers1 = generate_notices.build_language_renderers(templates_dir) + renderers2 = generate_notices.build_language_renderers(templates_dir) + + # Different dicts (not same object) + assert renderers1 is not renderers2 + + # Different function objects (fresh module loads each time) + # This is expected with dynamic loading - each call creates new module instances + assert renderers1["en"] is not renderers2["en"] + assert renderers1["fr"] is not renderers2["fr"] + + # But they should behave the same way (callable with same signature) + assert callable(renderers1["en"]) + assert callable(renderers2["en"]) diff --git a/tests/unit/test_en_template.py b/tests/unit/test_en_template.py index b3b6553..2adbe0f 100644 --- a/tests/unit/test_en_template.py +++ b/tests/unit/test_en_template.py @@ -176,8 +176,8 @@ def test_render_notice_includes_template_prefix(self) -> None: signature_path="/sig.png", ) - # Should include import statement (relative import for template directory) - assert '#import "conf.typ"' in result + # Should include import statement (absolute import from project root) + assert '#import "/templates/conf.typ"' in result def test_render_notice_includes_dynamic_block(self) -> None: """Verify output includes dynamic content section. @@ -273,9 +273,9 @@ def test_template_prefix_contains_imports(self) -> None: Real-world significance: - Typst must import conf.typ helpers - Setup code must be present - - Uses relative import so it resolves from template directory + - Uses absolute import from project root for generated .typ files """ - assert '#import "conf.typ"' in TEMPLATE_PREFIX + assert '#import "/templates/conf.typ"' in TEMPLATE_PREFIX def test_template_prefix_contains_function_definitions(self) -> None: """Verify TEMPLATE_PREFIX defines helper functions. diff --git a/tests/unit/test_fr_template.py b/tests/unit/test_fr_template.py index cb395b2..0959f62 100644 --- a/tests/unit/test_fr_template.py +++ b/tests/unit/test_fr_template.py @@ -194,8 +194,8 @@ def test_render_notice_includes_template_prefix(self) -> None: signature_path="/sig.png", ) - # Should include import statement (relative import for template directory) - assert '#import "conf.typ"' in result + # Should include import statement (absolute import from project root) + assert '#import "/templates/conf.typ"' in result def test_render_notice_includes_dynamic_block(self) -> None: """Verify output includes dynamic content section (French). @@ -318,9 +318,9 @@ def test_template_prefix_contains_imports(self) -> None: Real-world significance: - Typst must import conf.typ helpers - Setup code must be present - - Uses relative import so it resolves from template directory + - Uses absolute import from project root for generated .typ files """ - assert '#import "conf.typ"' in TEMPLATE_PREFIX + assert '#import "/templates/conf.typ"' in TEMPLATE_PREFIX def test_template_prefix_contains_function_definitions(self) -> None: """Verify TEMPLATE_PREFIX defines helper functions (French). From ad7158e207a3ea2176f4ab4a9b75a686026b4b36 Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 17:56:18 +0000 Subject: [PATCH 07/13] Documentation updates --- AGENTS.MD | 2 +- README.md | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/AGENTS.MD b/AGENTS.MD index 1ee4574..6eccb87 100644 --- a/AGENTS.MD +++ b/AGENTS.MD @@ -33,7 +33,7 @@ * **Orchestrator:** `pipeline/orchestrator.py` is the `viper` CLI entry point and coordinates 9 steps. * **Steps:** Modules are organized by steps 1–9, not by functional themes. -* **Templates:** `templates/` contains `en_template.py`, `fr_template.py`. Import via `from templates import ...`. Typesetting is separate from orchestration. +* **Templates:** `templates/` contains default language templates (`en_template.py`, `fr_template.py`), Typst config (`conf.typ`), and assets. Templates are loaded dynamically at runtime via `build_language_renderers()` in `generate_notices.py`, enabling custom template directories via `--template-dir` CLI argument. Each template module must define `render_notice()` function. Template directory isolation: templates, conf.typ, and assets stay together. Typesetting is separate from orchestration. --- diff --git a/README.md b/README.md index cc63bd6..ff1f247 100644 --- a/README.md +++ b/README.md @@ -172,8 +172,28 @@ uv run viper students.xlsx en # Override output directory uv run viper students.xlsx en --output-dir /tmp/output + +# Use custom template directory +uv run viper students.xlsx en --template-dir custom_template +``` + +### Using Custom Templates + +Public Health Units can use custom template directories for organization-specific branding and layouts: + +```bash +uv run viper students.xlsx en --template-dir custom_template ``` +The template directory must contain: +- `en_template.py` - English template module with `render_notice()` function +- `fr_template.py` - French template module with `render_notice()` function +- `conf.typ` - Typst configuration and utility functions +- `assets/logo.png` - Organization logo image if used +- `assets/signature.png` - Signature image if used + +Templates are loaded dynamically at runtime, enabling different organizations to maintain separate template sets without modifying core code. By default, the pipeline uses templates from the `templates/` directory. It's recommended to start custom template work off by copying the `templates/` directory contents into a new custom template directory. + > ℹ️ **Typst preview note:** The WDGPH code-server development environments render Typst files via Tinymist. The shared template at `templates/conf.typ` only defines helper functions, colour tokens, and table layouts that the generated notice `.typ` files import; it doesn't emit any pages on its own, so Tinymist has nothing to preview if attempted on this file. To examine the actual markup that uses these helpers, run the pipeline with `pipeline.keep_intermediate_files: true` in `config/parameters.yaml` so the generated notice `.typ` files stay in `output/artifacts/` for manual inspection. **Outputs:** From 07eb4475d2e06063937f5ac658b6b08ca5eaf0d7 Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 18:24:40 +0000 Subject: [PATCH 08/13] Ruff --- pipeline/compile_notices.py | 7 +++- pipeline/orchestrator.py | 4 +-- pipeline/preprocess.py | 4 ++- tests/conftest.py | 6 ++-- tests/integration/test_custom_templates.py | 33 ++++++++++--------- tests/unit/test_dynamic_template_loading.py | 4 ++- tests/unit/test_generate_notices.py | 21 ++++++------ ...test_unsupported_language_failure_paths.py | 7 ++-- 8 files changed, 49 insertions(+), 37 deletions(-) diff --git a/pipeline/compile_notices.py b/pipeline/compile_notices.py index a49af32..82ee001 100644 --- a/pipeline/compile_notices.py +++ b/pipeline/compile_notices.py @@ -205,7 +205,12 @@ def compile_with_config( ) -def main(artifact_dir: Path, output_dir: Path, config_path: Path | None = None, template_dir: Path | None = None) -> int: +def main( + artifact_dir: Path, + output_dir: Path, + config_path: Path | None = None, + template_dir: Path | None = None, +) -> int: """Main entry point for Typst compilation. Parameters diff --git a/pipeline/orchestrator.py b/pipeline/orchestrator.py index 9d7b5e3..6df01f3 100755 --- a/pipeline/orchestrator.py +++ b/pipeline/orchestrator.py @@ -117,9 +117,7 @@ def validate_args(args: argparse.Namespace) -> None: f"Input file not found: {args.input_dir / args.input_file}" ) if not args.template_dir.exists(): - raise FileNotFoundError( - f"Template directory not found: {args.template_dir}" - ) + raise FileNotFoundError(f"Template directory not found: {args.template_dir}") if not args.template_dir.is_dir(): raise NotADirectoryError( f"Template path is not a directory: {args.template_dir}" diff --git a/pipeline/preprocess.py b/pipeline/preprocess.py index 1781387..1f576e3 100644 --- a/pipeline/preprocess.py +++ b/pipeline/preprocess.py @@ -355,7 +355,9 @@ def ensure_required_columns(df: pd.DataFrame) -> pd.DataFrame: df.columns = [col.strip().upper() for col in df.columns] missing = [col for col in REQUIRED_COLUMNS if col not in df.columns] if missing: - raise ValueError(f"Missing required columns: {missing} \n Found columns: {list(df.columns)} ") + raise ValueError( + f"Missing required columns: {missing} \n Found columns: {list(df.columns)} " + ) df.rename(columns=lambda x: x.replace(" ", "_"), inplace=True) df.rename(columns={"PROVINCE/TERRITORY": "PROVINCE"}, inplace=True) diff --git a/tests/conftest.py b/tests/conftest.py index ee62336..90c700d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -261,14 +261,14 @@ def custom_templates(tmp_test_dir: Path) -> Generator[Path, None, None]: ... assert "en" in renderers """ import shutil - + # Create custom template directory in tmp_test_dir custom_dir = tmp_test_dir / "custom_templates" custom_dir.mkdir(parents=True, exist_ok=True) # Get path to source templates in project src_templates = Path(__file__).parent.parent / "templates" - + if not src_templates.exists(): raise FileNotFoundError( f"Source templates directory not found: {src_templates}. " @@ -290,6 +290,6 @@ def custom_templates(tmp_test_dir: Path) -> Generator[Path, None, None]: if dest_assets.exists(): shutil.rmtree(dest_assets) shutil.copytree(src_assets, dest_assets) - + yield custom_dir # Cleanup handled automatically by tmp_test_dir fixture diff --git a/tests/integration/test_custom_templates.py b/tests/integration/test_custom_templates.py index 7f01136..805cf21 100644 --- a/tests/integration/test_custom_templates.py +++ b/tests/integration/test_custom_templates.py @@ -95,17 +95,18 @@ def test_generate_notices_with_custom_templates( ) output_dir = tmp_path / "output" - + # Copy assets to output directory (simulating orchestrator behavior) assets_dir = output_dir / "assets" assets_dir.mkdir(parents=True, exist_ok=True) - + import shutil + logo_src = custom_templates / "assets" / "logo.png" signature_src = custom_templates / "assets" / "signature.png" logo_path = assets_dir / "logo.png" signature_path = assets_dir / "signature.png" - + shutil.copy2(logo_src, logo_path) shutil.copy2(signature_src, signature_path) @@ -154,7 +155,7 @@ def test_compile_notices_with_custom_template_root( # Create a minimal test .typ file that imports conf.typ typ_file = typst_dir / "test.typ" typ_file.write_text( - '#set text(fill: black)\nHello World', + "#set text(fill: black)\nHello World", encoding="utf-8", ) @@ -182,7 +183,7 @@ def test_full_pipeline_with_custom_templates( - Generate step must work with custom templates - Dynamic loading must resolve templates correctly - Generated .typ files must have correct imports - + NOTE: Skips actual Typst compilation since it requires .typ to be in/under --root directory, which is handled by the real orchestrator. """ @@ -198,17 +199,18 @@ def test_full_pipeline_with_custom_templates( ) output_dir = tmp_path / "output" - + # Copy assets to output directory (simulating orchestrator behavior) assets_dir = output_dir / "assets" assets_dir.mkdir(parents=True, exist_ok=True) - + import shutil + logo_src = custom_templates / "assets" / "logo.png" signature_src = custom_templates / "assets" / "signature.png" logo_path = assets_dir / "logo.png" signature_path = assets_dir / "signature.png" - + shutil.copy2(logo_src, logo_path) shutil.copy2(signature_src, signature_path) @@ -226,20 +228,21 @@ def test_full_pipeline_with_custom_templates( # Verify generated file has correct structure content = typst_files[0].read_text(encoding="utf-8") - + # Should have absolute import from project root assert '#import "/templates/conf.typ"' in content - + # Should have logo and signature paths assert "logo.png" in content or "__LOGO_PATH__" not in content assert "signature.png" in content or "__SIGNATURE_PATH__" not in content - + # Should have custom template content (French) - assert "Demande de dossier d'immunisation" in content or "immunization_notice" in content + assert ( + "Demande de dossier d'immunisation" in content + or "immunization_notice" in content + ) - def test_custom_template_assets_validated( - self, custom_templates: Path - ) -> None: + def test_custom_template_assets_validated(self, custom_templates: Path) -> None: """Verify custom template has all required assets. Real-world significance: diff --git a/tests/unit/test_dynamic_template_loading.py b/tests/unit/test_dynamic_template_loading.py index c1c0832..44fecaf 100644 --- a/tests/unit/test_dynamic_template_loading.py +++ b/tests/unit/test_dynamic_template_loading.py @@ -133,7 +133,9 @@ def test_load_template_module_missing_render_notice_mentions_file( error_msg = str(exc_info.value) assert str(invalid_template) in error_msg - def test_load_template_module_syntax_error_in_template(self, tmp_path: Path) -> None: + def test_load_template_module_syntax_error_in_template( + self, tmp_path: Path + ) -> None: """Verify error when template has syntax errors. Real-world significance: diff --git a/tests/unit/test_generate_notices.py b/tests/unit/test_generate_notices.py index aecee8d..c29d4c4 100644 --- a/tests/unit/test_generate_notices.py +++ b/tests/unit/test_generate_notices.py @@ -434,16 +434,15 @@ def test_language_renderers_configured(self) -> None: """ # Build renderers from default template directory from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" renderers = generate_notices.build_language_renderers(templates_dir) - + english_renderer = generate_notices.get_language_renderer( - generate_notices.Language.ENGLISH, - renderers + generate_notices.Language.ENGLISH, renderers ) french_renderer = generate_notices.get_language_renderer( - generate_notices.Language.FRENCH, - renderers + generate_notices.Language.FRENCH, renderers ) assert callable(english_renderer) assert callable(french_renderer) @@ -457,14 +456,14 @@ def test_render_notice_english_client(self, tmp_test_dir: Path) -> None: """ # Build renderers from default template directory from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" renderers = generate_notices.build_language_renderers(templates_dir) - + # Just verify the language renderer is callable # (actual rendering requires full Typst setup) english_renderer = generate_notices.get_language_renderer( - generate_notices.Language.ENGLISH, - renderers + generate_notices.Language.ENGLISH, renderers ) assert english_renderer is not None @@ -477,12 +476,12 @@ def test_render_notice_french_client(self, tmp_test_dir: Path) -> None: """ # Build renderers from default template directory from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" renderers = generate_notices.build_language_renderers(templates_dir) - + # Just verify the language renderer is callable french_renderer = generate_notices.get_language_renderer( - generate_notices.Language.FRENCH, - renderers + generate_notices.Language.FRENCH, renderers ) assert french_renderer is not None diff --git a/tests/unit/test_unsupported_language_failure_paths.py b/tests/unit/test_unsupported_language_failure_paths.py index 4baacba..ea8a947 100644 --- a/tests/unit/test_unsupported_language_failure_paths.py +++ b/tests/unit/test_unsupported_language_failure_paths.py @@ -122,6 +122,7 @@ def test_template_renderer_dispatch_assumes_valid_language(self) -> None: """ # Build renderers from default template directory from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" renderers = generate_notices.build_language_renderers(templates_dir) @@ -143,9 +144,10 @@ def test_valid_languages_pass_all_checks(self) -> None: """ # Build renderers from default template directory from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" renderers = generate_notices.build_language_renderers(templates_dir) - + # English en_lang = Language.from_string("en") assert en_lang == Language.ENGLISH @@ -250,9 +252,10 @@ class Language(Enum): """ # Build renderers from default template directory from pathlib import Path + templates_dir = Path(__file__).parent.parent.parent / "templates" renderers = generate_notices.build_language_renderers(templates_dir) - + # This test is primarily documentation; verify current state assert Language.all_codes() == {"en", "fr"} From c58d32809a9be4f962ea67f4dfee10de1a2a921f Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 19:01:39 +0000 Subject: [PATCH 09/13] phu_templates directory changes, .gitignore and .gitkeep --- .gitignore | 5 +- phu_templates/.gitkeep | 0 phu_templates/README.md | 103 ++++++++++++++++++++ pipeline/generate_notices.py | 49 +++++++--- pipeline/orchestrator.py | 66 +++++++++---- tests/integration/test_custom_templates.py | 99 +++++++++++++++++++ tests/unit/test_dynamic_template_loading.py | 20 ++-- tests/unit/test_run_pipeline.py | 1 + 8 files changed, 301 insertions(+), 42 deletions(-) create mode 100644 phu_templates/.gitkeep create mode 100644 phu_templates/README.md diff --git a/.gitignore b/.gitignore index 622a784..6203292 100644 --- a/.gitignore +++ b/.gitignore @@ -14,4 +14,7 @@ htmlcov/ coverage.xml coverage.json !input/rodent_dataset.xlsx -input/* \ No newline at end of file +input/* +phu_templates/* +!phu_templates/README.md +!phu_templates/.gitkeep \ No newline at end of file diff --git a/phu_templates/.gitkeep b/phu_templates/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/phu_templates/README.md b/phu_templates/README.md new file mode 100644 index 0000000..65309f5 --- /dev/null +++ b/phu_templates/README.md @@ -0,0 +1,103 @@ +# PHU Templates Directory + +This directory contains Public Health Unit (PHU) specific template customizations. + +## Usage + +Each PHU should create a subdirectory here with their organization-specific templates: + +``` +phu_templates/ +├── my_phu/ +│ ├── en_template.py (required for English output) +│ ├── fr_template.py (required for French output) +│ ├── conf.typ (required) +│ └── assets/ (optional - only if templates reference assets) +│ ├── logo.png (optional) +│ └── signature.png (optional) +``` + +## Running with PHU Templates + +To use a PHU-specific template, specify the template name with `--template`: + +```bash +# Generate English notices +uv run viper students.xlsx en --template my_phu + +# Generate French notices +uv run viper students.xlsx fr --template my_phu +``` + +This will load templates from `phu_templates/my_phu/`. + +## Template File Requirements + +### Core Requirements (Always Required) + +- `conf.typ` - Typst configuration and utility functions + +### Language-Specific Requirements (Based on Output Language) + +- `en_template.py` - Required only if generating English notices (`--language en`) + - Must define `render_notice()` function + - Consulted only when `--language en` is specified + +- `fr_template.py` - Required only if generating French notices (`--language fr`) + - Must define `render_notice()` function + - Consulted only when `--language fr` is specified + +**Note:** A PHU may provide templates for only one language. If a user requests a language your template does not support, the pipeline will fail with a clear error message. If you only support one language, only include that template file (e.g., only `en_template.py`). + +### Asset Requirements (Based on Template Implementation) + +Assets in the `assets/` directory are **optional** and depend entirely on your template implementation: + +- `assets/logo.png` - Only required if your `en_template.py` or `fr_template.py` references a logo +- `assets/signature.png` - Only required if your `en_template.py` or `fr_template.py` references a signature +- Other files - Any additional assets (e.g., `assets/header.png`, `assets/seal.pdf`) may be included and referenced in your templates + +**Note:** If your template references an asset (e.g., `include "assets/logo.png"` in Typst), that asset **must** exist. The pipeline will fail with a clear error if a referenced asset is missing. + +## Creating a PHU Template + +If your PHU supports both English and French: + +```bash +cp -r templates/ phu_templates/my_phu/ +``` + +Then customize: +- Replace `assets/logo.png` with your PHU logo +- Replace `assets/signature.png` with your signature +- Modify `en_template.py` and `fr_template.py` as needed +- Adjust `conf.typ` for organization-specific styling + +### Testing Your Template + +```bash +# Test English generation +uv run viper students.xlsx en --template my_phu + +# Test French generation (if you provided fr_template.py) +uv run viper students.xlsx fr --template my_phu +``` + +If a language template is missing: +``` +FileNotFoundError: Template module not found: /path/to/phu_templates/my_phu/fr_template.py +Expected fr_template.py in /path/to/phu_templates/my_phu +``` + +If an asset referenced by your template is missing: +``` +FileNotFoundError: Logo not found: /path/to/phu_templates/my_phu/assets/logo.png +``` + +## Git Considerations + +**Important:** PHU-specific templates are excluded from version control via `.gitignore`. + +- Templates in this directory will NOT be committed to the main repository +- Each PHU should maintain their templates in their own fork or separate repository +- The `README.md` file and `.gitkeep` are the only tracked files in this directory \ No newline at end of file diff --git a/pipeline/generate_notices.py b/pipeline/generate_notices.py index 57ffc49..e008d53 100644 --- a/pipeline/generate_notices.py +++ b/pipeline/generate_notices.py @@ -77,7 +77,7 @@ def load_template_module(template_dir: Path, language_code: str): Parameters ---------- template_dir : Path - Directory containing template modules (e.g., templates/ or phu_templates/custom/) + Directory containing template modules (e.g., templates/ or phu_templates/my_phu/) language_code : str Two-character ISO language code (e.g., "en", "fr") @@ -134,13 +134,15 @@ def load_template_module(template_dir: Path, language_code: str): def build_language_renderers(template_dir: Path) -> dict: """Build renderer dictionary from templates in specified directory. - Discovers and loads all language template modules from the given directory, + Discovers and loads all available language template modules from the given directory, building a mapping of language codes to their render_notice functions. **Validation Contract:** - - All languages in Language enum must have corresponding template files - - Each template must have valid render_notice() function - - Raises immediately if any template missing or invalid + - Only languages with corresponding template files are included + - Each available template must have valid render_notice() function + - Raises immediately if any template file exists but is invalid + - Does NOT require all Language enum values to be present + - Later validation ensures requested language is available when needed Parameters ---------- @@ -152,23 +154,29 @@ def build_language_renderers(template_dir: Path) -> dict: dict Mapping of language codes (str) to render_notice functions (callable) Format: {"en": , "fr": , ...} + May contain subset of all languages; only includes available templates Raises ------ - FileNotFoundError - If any required template file is missing AttributeError - If any template doesn't define render_notice() + If a template file exists but doesn't define render_notice() Examples -------- >>> renderers = build_language_renderers(Path("templates")) >>> renderers["en"](context, logo_path="/logo.png", signature_path="/sig.png") + + >>> # PHU providing only English + >>> renderers = build_language_renderers(Path("phu_templates/my_phu")) + >>> renderers # May only contain {"en": } """ renderers = {} for lang in Language: - module = load_template_module(template_dir, lang.value) - renderers[lang.value] = module.render_notice + module_path = template_dir / f"{lang.value}_template.py" + # Only load if template file exists + if module_path.exists(): + module = load_template_module(template_dir, lang.value) + renderers[lang.value] = module.render_notice return renderers @@ -181,8 +189,8 @@ def get_language_renderer(language: Language, renderers: dict): **Validation Contract:** Assumes language is a valid Language enum (validated upstream at CLI entry point via argparse choices, and again by Language.from_string() - before calling this function). Assumes renderers dict is complete (validated by - build_language_renderers()). + before calling this function). Checks that language is available in renderers dict; + raises with helpful error if template for requested language is not available. Parameters ---------- @@ -190,21 +198,32 @@ def get_language_renderer(language: Language, renderers: dict): Language enum value (guaranteed to be valid from Language enum). renderers : dict Mapping of language codes to render_notice functions, built by - build_language_renderers() + build_language_renderers(). May only contain subset of all languages. Returns ------- callable Template rendering function for the language. + Raises + ------ + FileNotFoundError + If requested language template is not available in renderers dict. + Provides helpful message listing available languages. + Examples -------- >>> renderers = build_language_renderers(Path("templates")) >>> renderer = get_language_renderer(Language.ENGLISH, renderers) >>> # renderer is now the render_notice function from en_template """ - # Language is already validated upstream (CLI choices + Language.from_string()) - # Direct lookup; safe because only valid Language enums reach this function + if language.value not in renderers: + available = ", ".join(sorted(renderers.keys())) if renderers else "none" + raise FileNotFoundError( + f"Template not available for language: {language.value}\n" + f"Available languages: {available}\n" + f"Ensure your template directory contains {language.value}_template.py" + ) return renderers[language.value] diff --git a/pipeline/orchestrator.py b/pipeline/orchestrator.py index 6df01f3..ae00f91 100755 --- a/pipeline/orchestrator.py +++ b/pipeline/orchestrator.py @@ -57,6 +57,7 @@ DEFAULT_INPUT_DIR = ROOT_DIR / "input" DEFAULT_OUTPUT_DIR = ROOT_DIR / "output" DEFAULT_TEMPLATES_DIR = ROOT_DIR / "templates" +DEFAULT_PHU_TEMPLATES_DIR = ROOT_DIR / "phu_templates" DEFAULT_CONFIG_DIR = ROOT_DIR / "config" @@ -101,10 +102,12 @@ def parse_args() -> argparse.Namespace: help=f"Config directory (default: {DEFAULT_CONFIG_DIR})", ) parser.add_argument( - "--template-dir", - type=Path, - default=DEFAULT_TEMPLATES_DIR, - help=f"Template directory (default: {DEFAULT_TEMPLATES_DIR})", + "--template", + type=str, + default=None, + dest="template_dir", + help="PHU template name within phu_templates/ (e.g., 'wdgph'). " + "If not specified, uses default templates/ directory.", ) return parser.parse_args() @@ -116,8 +119,35 @@ def validate_args(args: argparse.Namespace) -> None: raise FileNotFoundError( f"Input file not found: {args.input_dir / args.input_file}" ) - if not args.template_dir.exists(): - raise FileNotFoundError(f"Template directory not found: {args.template_dir}") + + # Resolve template directory + if args.template_dir is None: + # No custom template specified; use default + args.template_dir = DEFAULT_TEMPLATES_DIR + else: + # Custom PHU template specified; resolve within phu_templates/ + # Validate no path separators (prevent nested directories) + if "/" in args.template_dir or "\\" in args.template_dir: + raise ValueError( + f"Template name cannot contain path separators: {args.template_dir}\n" + f"Expected a simple name like 'wdgph' or 'my_phu', not a path." + ) + + phu_template_path = DEFAULT_PHU_TEMPLATES_DIR / args.template_dir + if not phu_template_path.exists(): + raise FileNotFoundError( + f"PHU template directory not found: {phu_template_path}\n" + f"Expected location: phu_templates/{args.template_dir}\n" + f"Ensure the directory exists and contains required template files." + ) + if not phu_template_path.is_dir(): + raise NotADirectoryError( + f"PHU template path is not a directory: {phu_template_path}" + ) + # Update args.template_dir to resolved Path + args.template_dir = phu_template_path + + # Validate template directory contents if not args.template_dir.is_dir(): raise NotADirectoryError( f"Template path is not a directory: {args.template_dir}" @@ -272,30 +302,28 @@ def run_step_4_generate_notices( run_id : str Unique run identifier template_dir : Path - Directory containing language templates and assets + Directory containing language templates and optional assets config_dir : Path Configuration directory + + Notes + ----- + Assets (logo.png, signature.png) are optional. They are only required if + the template actually references them. If a template references an asset + that doesn't exist, generation will fail with a clear error message. """ print_step(4, "Generating Typst templates") artifact_path = output_dir / "artifacts" / f"preprocessed_clients_{run_id}.json" artifacts_dir = output_dir / "artifacts" - # Assets now come from template directory + # Assets now come from template directory (optional) logo_path = template_dir / "assets" / "logo.png" signature_path = template_dir / "assets" / "signature.png" - # Validate assets exist (fail-fast) - if not logo_path.exists(): - raise FileNotFoundError( - f"Logo not found: {logo_path}. " - f"Template directory must contain assets/logo.png" - ) - if not signature_path.exists(): - raise FileNotFoundError( - f"Signature not found: {signature_path}. " - f"Template directory must contain assets/signature.png" - ) + # Note: Assets are NOT validated here. If a template references an asset + # that doesn't exist, the template rendering will fail with a clear error. + # This allows templates without assets to work without requiring dummy files. # Generate Typst files using main function generated = generate_notices.main( diff --git a/tests/integration/test_custom_templates.py b/tests/integration/test_custom_templates.py index 805cf21..00c8f09 100644 --- a/tests/integration/test_custom_templates.py +++ b/tests/integration/test_custom_templates.py @@ -282,3 +282,102 @@ def test_custom_template_includes_all_required_modules( assert en_module.stat().st_size > 0 assert fr_module.stat().st_size > 0 assert conf.stat().st_size > 0 + + def test_single_language_template_english_only(self, tmp_path: Path) -> None: + """Verify PHU can provide template for only English (no French). + + Real-world significance: + - Not all PHUs need bilingual templates + - Pipeline should work with single-language templates + - Requesting unsupported language should fail with clear error + """ + # Create single-language template (English only) + en_template_dir = tmp_path / "en_only_template" + en_template_dir.mkdir() + + # Copy English template and conf only + import shutil + + shutil.copy2( + Path(__file__).parent.parent.parent / "templates" / "en_template.py", + en_template_dir / "en_template.py", + ) + shutil.copy2( + Path(__file__).parent.parent.parent / "templates" / "conf.typ", + en_template_dir / "conf.typ", + ) + + # Build renderers - should only have English + renderers = generate_notices.build_language_renderers(en_template_dir) + + assert "en" in renderers, "English should be available" + assert "fr" not in renderers, "French should NOT be available" + assert callable(renderers["en"]) + + def test_single_language_template_french_only(self, tmp_path: Path) -> None: + """Verify PHU can provide template for only French (no English). + + Real-world significance: + - Some PHUs may only provide French + - Pipeline should work with single-language templates + - Only the provided language can be requested + """ + # Create single-language template (French only) + fr_template_dir = tmp_path / "fr_only_template" + fr_template_dir.mkdir() + + # Copy French template and conf only + import shutil + + shutil.copy2( + Path(__file__).parent.parent.parent / "templates" / "fr_template.py", + fr_template_dir / "fr_template.py", + ) + shutil.copy2( + Path(__file__).parent.parent.parent / "templates" / "conf.typ", + fr_template_dir / "conf.typ", + ) + + # Build renderers - should only have French + renderers = generate_notices.build_language_renderers(fr_template_dir) + + assert "fr" in renderers, "French should be available" + assert "en" not in renderers, "English should NOT be available" + assert callable(renderers["fr"]) + + def test_missing_language_raises_helpful_error(self, tmp_path: Path) -> None: + """Verify requesting unavailable language gives helpful error. + + Real-world significance: + - Users should understand why generation fails + - Error message should indicate available languages + - Should guide users to provide correct language + """ + # Create single-language template (English only) + en_template_dir = tmp_path / "en_only_template" + en_template_dir.mkdir() + + import shutil + + shutil.copy2( + Path(__file__).parent.parent.parent / "templates" / "en_template.py", + en_template_dir / "en_template.py", + ) + shutil.copy2( + Path(__file__).parent.parent.parent / "templates" / "conf.typ", + en_template_dir / "conf.typ", + ) + + # Build renderers + renderers = generate_notices.build_language_renderers(en_template_dir) + + # Try to get French renderer - should fail + from pipeline.enums import Language + + with pytest.raises(FileNotFoundError) as exc_info: + generate_notices.get_language_renderer(Language.FRENCH, renderers) + + error_msg = str(exc_info.value) + assert "fr" in error_msg, "Error should mention missing 'fr'" + assert "Available languages: en" in error_msg, "Error should list available: en" + assert "fr_template.py" in error_msg, "Error should mention required file name" diff --git a/tests/unit/test_dynamic_template_loading.py b/tests/unit/test_dynamic_template_loading.py index 44fecaf..f27f16a 100644 --- a/tests/unit/test_dynamic_template_loading.py +++ b/tests/unit/test_dynamic_template_loading.py @@ -194,12 +194,15 @@ def test_build_language_renderers_success_from_custom( assert callable(renderers["en"]) assert callable(renderers["fr"]) - def test_build_language_renderers_missing_language(self, tmp_path: Path) -> None: - """Verify error when template missing for a language. + def test_build_language_renderers_missing_language_allowed( + self, tmp_path: Path + ) -> None: + """Verify that missing language template is allowed (not required). Real-world significance: - - Template directory incomplete - - Should fail immediately before processing clients + - PHU templates can support single language (e.g., English only) + - Missing language just means that language isn't available + - Error only occurs when that specific language is requested """ # Create only English template en_template = tmp_path / "en_template.py" @@ -208,9 +211,12 @@ def test_build_language_renderers_missing_language(self, tmp_path: Path) -> None encoding="utf-8", ) - # Should fail when trying to load French - with pytest.raises(FileNotFoundError, match="fr_template.py"): - generate_notices.build_language_renderers(tmp_path) + # Should NOT raise - just returns only English + renderers = generate_notices.build_language_renderers(tmp_path) + + assert "en" in renderers + assert "fr" not in renderers + assert len(renderers) == 1 def test_build_language_renderers_returns_dict_with_correct_types( self, templates_dir: Path diff --git a/tests/unit/test_run_pipeline.py b/tests/unit/test_run_pipeline.py index 9e4ab6b..9a292bf 100644 --- a/tests/unit/test_run_pipeline.py +++ b/tests/unit/test_run_pipeline.py @@ -126,6 +126,7 @@ def test_validate_args_existing_input_file(self, tmp_test_dir: Path) -> None: args = MagicMock() args.input_file = "students.xlsx" args.input_dir = tmp_test_dir + args.template_dir = None # Use default templates # Should not raise orchestrator.validate_args(args) From a9a1951d5f3727b0e06dcfe08e39e955fd66ed29 Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 19:10:49 +0000 Subject: [PATCH 10/13] Documentation updates --- AGENTS.MD | 2 +- README.md | 29 +++++++++++++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/AGENTS.MD b/AGENTS.MD index 6eccb87..9c451c4 100644 --- a/AGENTS.MD +++ b/AGENTS.MD @@ -33,7 +33,7 @@ * **Orchestrator:** `pipeline/orchestrator.py` is the `viper` CLI entry point and coordinates 9 steps. * **Steps:** Modules are organized by steps 1–9, not by functional themes. -* **Templates:** `templates/` contains default language templates (`en_template.py`, `fr_template.py`), Typst config (`conf.typ`), and assets. Templates are loaded dynamically at runtime via `build_language_renderers()` in `generate_notices.py`, enabling custom template directories via `--template-dir` CLI argument. Each template module must define `render_notice()` function. Template directory isolation: templates, conf.typ, and assets stay together. Typesetting is separate from orchestration. +* **Templates:** `templates/` contains default language templates (`en_template.py`, `fr_template.py`), Typst config (`conf.typ`), and assets. `phu_templates/` is the container for PHU-specific template customizations (gitignored). Templates are loaded dynamically at runtime via `build_language_renderers()` in `generate_notices.py`. The `--template` CLI argument expects a template name within `phu_templates/` (e.g., `--template wdgph` → `phu_templates/wdgph/`). Template names cannot contain path separators (no nesting). Each template module must define a `render_notice()` function. Template directory isolation: templates, conf.typ, and assets stay together. Typesetting is separate from orchestration. --- diff --git a/README.md b/README.md index ff1f247..be5943e 100644 --- a/README.md +++ b/README.md @@ -154,6 +154,7 @@ uv run viper [--output-dir PATH] - `--input-dir PATH`: Input directory (default: ../input) - `--output-dir PATH`: Output directory (default: ../output) - `--config-dir PATH`: Configuration directory (default: ../config) +- `--template NAME`: PHU template name within `phu_templates/` (e.g., `wdgph`); defaults to built-in `templates/` when omitted **Configuration:** See the complete configuration reference and examples in `config/README.md`: @@ -173,26 +174,30 @@ uv run viper students.xlsx en # Override output directory uv run viper students.xlsx en --output-dir /tmp/output -# Use custom template directory -uv run viper students.xlsx en --template-dir custom_template +# Use a PHU-specific template (from phu_templates/my_phu/) +uv run viper students.xlsx en --template my_phu ``` -### Using Custom Templates +### Using PHU-Specific Templates -Public Health Units can use custom template directories for organization-specific branding and layouts: +Public Health Units can create custom template directories for organization-specific branding and layouts. All PHU templates live under the `phu_templates/` directory and are gitignored by default. ```bash -uv run viper students.xlsx en --template-dir custom_template +# Create your PHU template directory by copying defaults +cp -r templates/ phu_templates/my_phu/ + +# Customize templates and assets as needed, then run with your PHU template +uv run viper students.xlsx en --template my_phu ``` -The template directory must contain: -- `en_template.py` - English template module with `render_notice()` function -- `fr_template.py` - French template module with `render_notice()` function -- `conf.typ` - Typst configuration and utility functions -- `assets/logo.png` - Organization logo image if used -- `assets/signature.png` - Signature image if used +The `--template` argument expects a template name within `phu_templates/` (flat names only; no `/` or `\`). For example, `--template my_phu` loads from `phu_templates/my_phu/`. + +Each PHU template directory should contain: +- `conf.typ` - Typst configuration and helper functions (required) +- `{lang}_template.py` - Language modules with `render_notice()` for the languages you intend to generate (e.g., `en_template.py` for English, `fr_template.py` for French). Single-language templates are supported. +- `assets/` - Optional directory for images like logos or signatures if your templates reference them -Templates are loaded dynamically at runtime, enabling different organizations to maintain separate template sets without modifying core code. By default, the pipeline uses templates from the `templates/` directory. It's recommended to start custom template work off by copying the `templates/` directory contents into a new custom template directory. +Templates are loaded dynamically at runtime, enabling different organizations to maintain separate template sets without modifying core code. By default (when `--template` is not specified), the pipeline uses the built-in `templates/` directory. It's recommended to start by copying from `templates/` into `phu_templates//` and customizing from there. > ℹ️ **Typst preview note:** The WDGPH code-server development environments render Typst files via Tinymist. The shared template at `templates/conf.typ` only defines helper functions, colour tokens, and table layouts that the generated notice `.typ` files import; it doesn't emit any pages on its own, so Tinymist has nothing to preview if attempted on this file. To examine the actual markup that uses these helpers, run the pipeline with `pipeline.keep_intermediate_files: true` in `config/parameters.yaml` so the generated notice `.typ` files stay in `output/artifacts/` for manual inspection. From d4a8f94b61e6c9dd5c6a5265e8197c8703afefc5 Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 19:21:21 +0000 Subject: [PATCH 11/13] Shorten cli flags --- README.md | 10 +++++----- pipeline/orchestrator.py | 9 ++++++--- tests/e2e/test_full_pipeline.py | 2 +- tests/unit/test_run_pipeline.py | 8 ++++---- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index be5943e..32dd6b3 100644 --- a/README.md +++ b/README.md @@ -143,7 +143,7 @@ The main pipeline orchestrator (`orchestrator.py`) automates the end-to-end work **Usage Example:** ```bash -uv run viper [--output-dir PATH] +uv run viper [--output PATH] ``` **Required Arguments:** @@ -151,9 +151,9 @@ uv run viper [--output-dir PATH] - ``: Language code (`en` or `fr`) **Optional Arguments:** -- `--input-dir PATH`: Input directory (default: ../input) -- `--output-dir PATH`: Output directory (default: ../output) -- `--config-dir PATH`: Configuration directory (default: ../config) +- `--input PATH`: Input directory (default: ../input) +- `--output PATH`: Output directory (default: ../output) +- `--config PATH`: Configuration directory (default: ../config) - `--template NAME`: PHU template name within `phu_templates/` (e.g., `wdgph`); defaults to built-in `templates/` when omitted **Configuration:** @@ -172,7 +172,7 @@ Direct link: [Configuration Reference](./config/README.md) uv run viper students.xlsx en # Override output directory -uv run viper students.xlsx en --output-dir /tmp/output +uv run viper students.xlsx en --output /tmp/output # Use a PHU-specific template (from phu_templates/my_phu/) uv run viper students.xlsx en --template my_phu diff --git a/pipeline/orchestrator.py b/pipeline/orchestrator.py index ae00f91..ab21964 100755 --- a/pipeline/orchestrator.py +++ b/pipeline/orchestrator.py @@ -84,21 +84,24 @@ def parse_args() -> argparse.Namespace: help=f"Language for output ({', '.join(sorted(Language.all_codes()))})", ) parser.add_argument( - "--input-dir", + "--input", type=Path, default=DEFAULT_INPUT_DIR, + dest="input_dir", help=f"Input directory (default: {DEFAULT_INPUT_DIR})", ) parser.add_argument( - "--output-dir", + "--output", type=Path, default=DEFAULT_OUTPUT_DIR, + dest="output_dir", help=f"Output directory (default: {DEFAULT_OUTPUT_DIR})", ) parser.add_argument( - "--config-dir", + "--config", type=Path, default=DEFAULT_CONFIG_DIR, + dest="config_dir", help=f"Config directory (default: {DEFAULT_CONFIG_DIR})", ) parser.add_argument( diff --git a/tests/e2e/test_full_pipeline.py b/tests/e2e/test_full_pipeline.py index 0200c99..89cfcde 100644 --- a/tests/e2e/test_full_pipeline.py +++ b/tests/e2e/test_full_pipeline.py @@ -105,7 +105,7 @@ def run_pipeline( "viper", str(input_file.name), language, - "--input-dir", + "--input", str(input_file.parent), ] diff --git a/tests/unit/test_run_pipeline.py b/tests/unit/test_run_pipeline.py index 9a292bf..cdd4902 100644 --- a/tests/unit/test_run_pipeline.py +++ b/tests/unit/test_run_pipeline.py @@ -56,7 +56,7 @@ def test_parse_args_language_choices(self) -> None: assert args.language == "fr" def test_parse_args_optional_directories(self) -> None: - """Verify optional --input-dir, --output-dir, --config-dir arguments. + """Verify optional --input, --output, --config arguments. Real-world significance: - User can override default directories @@ -68,11 +68,11 @@ def test_parse_args_optional_directories(self) -> None: "viper", "test.xlsx", "en", - "--input-dir", + "--input", "/tmp/input", - "--output-dir", + "--output", "/tmp/output", - "--config-dir", + "--config", "/etc/config", ], ): From b117af612d57ed61f0da830e71d98a4c44520ca7 Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 19:37:18 +0000 Subject: [PATCH 12/13] Exclude timing from codecov --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index bda1a06..f491e95 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,6 +44,7 @@ exclude_lines = [ "raise NotImplementedError", "if __name__ == .__main__.:", "if TYPE_CHECKING:", + "= time\\.time\\(\\)", ] [tool.coverage.html] From 85345f202c5ec3318283fa9ed75c45a69a3f0c40 Mon Sep 17 00:00:00 2001 From: Justin Angevaare Date: Wed, 12 Nov 2025 16:36:57 -0500 Subject: [PATCH 13/13] Update pipeline/orchestrator.py Co-authored-by: Kassy Raymond <44474665+kassyray@users.noreply.github.com> --- pipeline/orchestrator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipeline/orchestrator.py b/pipeline/orchestrator.py index ab21964..8801a7e 100755 --- a/pipeline/orchestrator.py +++ b/pipeline/orchestrator.py @@ -110,7 +110,7 @@ def parse_args() -> argparse.Namespace: default=None, dest="template_dir", help="PHU template name within phu_templates/ (e.g., 'wdgph'). " - "If not specified, uses default templates/ directory.", + "If not specified, pipeline is run in testing mode, defaulting to the templates/ directory.", ) return parser.parse_args()