docs: Add comprehensive documentation to recipe generator

Add detailed docstrings and inline comments to all recipe generator modules explaining classes, functions, and complex logic in plain English.

Files updated:
- recipe-generator/recipe_generator.py: Module, class, and method docs
- recipe-generator/cli.py: CLI modes, arguments, and workflow docs
- recipe-generator/validators.py: Three-layer validation architecture docs
- recipe-generator/manifest.py: Manifest processing and variant expansion docs
- recipe-generator/generate_recipe.py: Entry point documentation

recipe-generator/cli.py

@@ -1,4 +1,22 @@
-"""CLI entry point for recipe generator."""
+"""CLI entry point for recipe generator.
+
+This module handles the command-line interface for the recipe generator. It:
+1. Parses command-line arguments and options
+2. Selects the appropriate mode (validation, batch generation, single generation)
+3. Sets up generators and validators with proper configuration
+4. Orchestrates the workflow based on user input
+5. Handles output and exit codes for integration with CI systems
+
+The CLI supports three main modes:
+- Single generation: Generate one recipe with specific options
+- Batch generation: Generate many recipes from a manifest file
+- Validation mode: Check existing recipes for errors
+
+Exit codes:
+- 0: Success
+- 1: Validation errors or generation failed
+- 2: Invalid arguments or missing files
+"""
 
 from __future__ import annotations
 
@@ -20,13 +38,51 @@ from validators import (
 
 
 def main() -> None:
-    """Main entry point for recipe generator CLI."""
+    """Main entry point for recipe generator CLI.
+    
+    This is the primary function that gets called when running the script.
+    It handles argument parsing, mode selection, and workflow orchestration.
+    
+    Command-line arguments:
+        --ingredients (-i): Directory containing ingredient fragments (default: parent/ingredients)
+        --templates (-t): Path to templates YAML file (default: ./recipe_templates.yaml)
+        
+        Batch mode flags:
+        --manifest (-m): Path to manifest YAML for batch generation
+        --output-dir (-d): Output directory for generated recipes (default: ./recipes)
+        --dry-run (-n): Show what would be generated without writing files
+        
+        Single generation flags:
+        --type (-T): Recipe type to generate (e.g., virtual-desktop)
+        --output (-o): Output file path for single generation
+        
+        Recipe parameters:
+        --version (-v): Fedora version (43 or rawhide, default: 43)
+        --desktop: Desktop environment (gnome or labwc, default: gnome)
+        --storage: Storage type (standard or encrypted, default: standard)
+        --security: Security mode (secure or devel, default: secure)
+        --cpu: CPU optimization (generic, amdcpu, or intelcpu)
+        --gpu: GPU passthrough (none or intelgpu, default: none)
+        
+        Validation mode:
+        --validate (-V): Validate existing recipe files
+        --strict: Treat warnings as errors (CI mode)
+    
+    Modes:
+        1. Validation mode (--validate): Check recipe files without generating new ones
+        2. Batch generation mode (--manifest): Generate many recipes from a manifest
+        3. Single generation mode (--type): Generate one recipe with specified options
+        4. Help mode: No arguments or --help shows usage information
+    """
+    # Set up argument parser with descriptive help text
     parser = argparse.ArgumentParser(
         description='Generate Phyllome OS kickstart recipes from templates',
         formatter_class=argparse.RawDescriptionHelpFormatter
     )
 
-    # Global options
+    # Calculate paths relative to this script's location
+    # SCRIPTS_DIR points to recipe-generator/
+    # PROJECT_ROOT points to phyllomeos/
     SCRIPTS_DIR = Path(__file__).resolve().parent  # noqa: N806 - Constant
     PROJECT_ROOT = SCRIPTS_DIR.parent  # noqa: N806 - Constant
 
@@ -37,7 +93,7 @@ def main() -> None:
                         type=Path, default=SCRIPTS_DIR / 'recipe_templates.yaml',
                         help='Templates YAML file (default: ./recipe_templates.yaml)')
 
-    # Batch mode
+    # Batch mode options
     parser.add_argument('--manifest', '-m',
                         type=Path, help='Manifest YAML for batch generation')
     parser.add_argument('--output-dir', '-d',
@@ -47,13 +103,13 @@ def main() -> None:
                         action='store_true',
                         help='Show what would be generated without writing files')
 
-    # Single generation mode
+    # Single generation mode options
     parser.add_argument('--type', '-T',
                         help='Recipe type (e.g., virtual-desktop)')
     parser.add_argument('--output', '-o',
                         type=Path, help='Output file (single generation)')
 
-    # Recipe parameters
+    # Recipe customization parameters (default to safe/common values)
     parser.add_argument('--version', '-v',
                         choices=['43', 'rawhide'], default='43',
                         help='Fedora version (default: 43)')
@@ -86,58 +142,87 @@ def main() -> None:
                         action='store_true',
                         help='Treat warnings as errors (CI mode)')
 
+    # Parse arguments from command line
     args = parser.parse_args()
 
-    # Initialize generator
+    # Initialize the recipe generator with loaded templates
     generator = RecipeGenerator(args.templates)
 
-    # Initialize validators
+    # Initialize validators - they'll be used throughout the workflow
     template_validator = TemplateValidator(generator.project_root)
     content_validator = ContentValidator(generator.project_root)
     semantic_validator = SemanticValidator()
 
-    # Validation mode
+    # === VALIDATION MODE ===
+    # If --validate is specified, skip generation and just check existing recipes
     if args.validate:
         all_issues = validate_recipes(args.validate, content_validator, semantic_validator, args.strict)
         handle_validation_results(all_issues, args.strict)
         return
 
-    # Batch generation mode
+    # === BATCH GENERATION MODE ===
+    # If --manifest is specified, generate many recipes from a manifest file
     if args.manifest:
         generate_from_manifest(args, generator)
         return
 
-    # Single generation mode
+    # === SINGLE GENERATION MODE ===
+    # If --type is specified, generate one recipe with the given options
     if args.type:
         generate_single(args, generator)
         return
 
-    # No mode specified, show help
+    # === NO MODE SPECIFIED ===
+    # Show help and exit with error
     parser.print_help()
     sys.exit(1)
 
 
 def validate_recipes(recipe_paths: List[str], content_validator: ContentValidator, 
                      semantic_validator: SemanticValidator, strict: bool) -> List:
-    """Validate multiple recipes."""
+    """Validate multiple recipes from file paths.
+    
+    This function iterates through a list of recipe file paths, validates each one
+    for both content issues (duplicates, missing ingredients) and semantic issues
+    (kickstart syntax errors).
+    
+    The validation process for each file:
+    1. Read the file content
+    2. Run ContentValidator to check ingredient existence and duplicates
+    3. Try to extract the Fedora version from content or filename
+    4. Run SemanticValidator (pykickstart parser) to check kickstart syntax
+    5. Collect all issues found
+    
+    Args:
+        recipe_paths: List of file paths to validate
+        content_validator: ContentValidator instance for ingredient checks
+        semantic_validator: SemanticValidator instance for kickstart parsing
+        strict: If True, treat warnings as errors (exit with 1)
+    
+    Returns:
+        List of (path, issues) tuples for files that have issues
+        Example: [('recipes/gnome.cfg', ['ERROR: Missing ingredient', 'Warning: ...'])]
+    """
     all_issues = []
     for recipe_path in recipe_paths:
         try:
-            # Validate content
+            # Validate content - check for duplicates and missing ingredients
             content = Path(recipe_path).read_text(encoding='utf-8')
             issues = content_validator.validate(content)
 
-            # Extract version and validate semantically
+            # Extract version from content or filename for semantic validation
             filename = Path(recipe_path).stem
             version = extract_version(content, filename)
             if version:
                 semantic_issues = semantic_validator.validate(content, version)
                 issues.extend(semantic_issues)
             else:
+                # Could not determine version, can't do semantic validation
                 issues.append("Warning: Could not determine version, "
                             "skipping semantic validation")
 
             if issues:
+                # Only track files that have issues
                 all_issues.append((recipe_path, issues))
         except FileNotFoundError:
             print(f"Error: Recipe not found: {recipe_path}", file=sys.stderr)
@@ -147,13 +232,37 @@ def validate_recipes(recipe_paths: List[str], content_validator: ContentValidato
 
 
 def extract_version(content: str, filename: str) -> str | None:
-    """Extract Fedora version from recipe content or filename."""
+    """Extract Fedora version from recipe content or filename.
+    
+    This function tries multiple heuristics to determine which Fedora version
+    a recipe is targeting. It checks the filename first, then falls back to
+    searching the content for version-specific patterns.
+    
+    The method checks for:
+    1. Version in filename: patterns like _43_, -rawhide.cfg, _43.cfg
+    2. Version in content: repository paths like 'core-fedora-repo-43'
+    
+    This is useful for batch operations where the version might not be
+    explicitly stated but can be inferred from naming conventions or content.
+    
+    Args:
+        content: The content of the recipe file (as a string)
+        filename: Just the filename without path
+    
+    Returns:
+        Version string ('43' or 'rawhide') if found, None otherwise
+    """
     import re
 
+    # Try to extract from filename first using regex
+    # Matches patterns like _43, -43, _rawhide, -rawhide, etc.
+    # The lookahead for .cfg|.yaml|$ ensures we don't match 43 in '431'
     filename_match = re.search(r'(?:_|-)(43|rawhide)(?:_|-|.cfg|.yaml|$)', filename)
     if filename_match:
         return filename_match.group(1)
 
+    # Fall back to content inspection
+    # Check for Fedora repository paths that contain version info
     for line in content.split('\n'):
         if 'core-fedora-repo-43' in line:
             return '43'
@@ -168,52 +277,105 @@ def extract_version(content: str, filename: str) -> str | None:
 
 
 def handle_validation_results(all_issues: List, strict: bool) -> None:
-    """Handle validation results and exit with appropriate code."""
+    """Handle validation results and exit with appropriate code.
+    
+    This function formats and displays validation results to the user,
+    then exits with the appropriate code based on the results and strict mode.
+    
+    The output format:
+    - Group issues by file
+    - Show errors first (separate from warnings)
+    - Show summary totals at the end
+    
+    Exit codes:
+    - 0: All files passed validation
+    - 1: Any issues found (or warnings in strict mode)
+    - 2: Error during processing
+    
+    Args:
+        all_issues: List of (path, issues) tuples from validate_recipes()
+        strict: If True, warnings are treated as errors
+    """
     if all_issues:
+        # Display issues for each file that has problems
         for path, issues in all_issues:
             print(f"\n{path}:", file=sys.stderr)
+            
+            # Count errors and warnings separately
             error_count = sum(1 for i in issues if 'ERROR' in i or 'error' in i.lower())
             warning_count = sum(1 for i in issues if 'Warning' in i or 'warning' in i.lower())
 
+            # Display errors (if any)
             if error_count > 0:
                 for issue in issues:
                     if 'ERROR' in issue or 'error' in issue.lower():
                         print(f"  {issue}", file=sys.stderr)
 
+            # Display warnings (if any)
             if warning_count > 0:
                 for issue in issues:
                     if 'Warning' in issue or 'warning' in issue.lower():
                         print(f"  {issue}", file=sys.stderr)
 
+            # Handle case where no issues match expected patterns
             if error_count == 0 and warning_count == 0:
                 print(f"  No issues found (file exists)", file=sys.stderr)
 
+        # Print summary
         print(f"\nSummary:", file=sys.stderr)
         print(f"  - {len(all_issues)} recipe(s) checked", file=sys.stderr)
 
+        # Calculate totals across all files
         total_errors = sum(len([i for i in issues if 'ERROR' in i or 'error' in i.lower()]) 
                          for _, issues in all_issues)
         total_warnings = sum(len([i for i in issues if 'Warning' in i or 'warning' in i.lower()]) 
                            for _, issues in all_issues)
         print(f"  - {total_errors} error(s), {total_warnings} warning(s)", file=sys.stderr)
 
+        # In strict mode, warnings are treated as errors
         if strict and total_warnings > 0:
             print("\nStrict mode: Warnings treated as errors", file=sys.stderr)
             sys.exit(1)
 
-        if total_errors > 0:
-            sys.exit(1)
-        if all_issues:
+        # Exit with error if any issues found
+        if total_errors > 0 or all_issues:
             sys.exit(1)
     else:
+        # All recipes validated successfully
         print("All recipes validated successfully")
         sys.exit(0)
 
 
 def generate_from_manifest(args: argparse.Namespace, generator: RecipeGenerator) -> None:
-    """Generate recipes from manifest."""
+    """Generate recipes in batch from a manifest file.
+    
+    This function handles batch generation mode. It:
+    1. Loads and parses the manifest YAML file
+    2. Validates the manifest structure
+    3. Iterates through each recipe configuration in the manifest
+    4. Expands any variants with list values (cartesian product)
+    5. Generates each variant and writes to output files
+    6. Optionally runs validation or dry-run checks
+    
+    The manifest YAML format:
+        recipes:
+          - name: virtual-desktop
+            variants:
+              - version: 43
+                desktop: gnome
+                storage: encrypted
+              - version: ["43", "rawhide"]
+                security: secure
+    
+    The list-valued variants (like version) get expanded into all combinations.
+    
+    Args:
+        args: Parsed command-line arguments containing paths and settings
+        generator: Pre-configured RecipeGenerator instance
+    """
     manifest_path = args.manifest
     try:
+        # Load manifest YAML file
         with open(manifest_path, encoding='utf-8') as f:
             manifest = yaml.safe_load(f)
     except FileNotFoundError:
@@ -223,7 +385,7 @@ def generate_from_manifest(args: argparse.Namespace, generator: RecipeGenerator)
         print(f"Error: Invalid YAML in manifest: {e}", file=sys.stderr)
         sys.exit(2)
 
-    # Validate manifest
+    # Validate manifest structure before processing
     manifest_processor = ManifestProcessor(generator.project_root)
     errors = manifest_processor.validate(manifest)
     if errors:
@@ -232,32 +394,36 @@ def generate_from_manifest(args: argparse.Namespace, generator: RecipeGenerator)
             print(f"  - {error}", file=sys.stderr)
         sys.exit(1)
 
-    # Track seen filenames to avoid overwriting duplicates
-
-    # Generate all recipes
+    # Generate all recipes from the manifest
     for recipe_config in manifest.get('recipes', []):
         recipe_type = recipe_config['name']
         if recipe_type not in generator.templates:
             print(f"Error: Unknown recipe type in manifest: {recipe_type}", file=sys.stderr)
             sys.exit(1)
 
+        # Get variants for this recipe type
         variants = recipe_config.get('variants', [])
+        
+        # Expand list-valued variants (e.g., version: ["43", "rawhide"])
         variants = generator.expand_variants(variants)
+        
         for variant in variants:
+            # Extract version and other modifiers from variant
             version = variant['version']
             modifiers = {k: v for k, v in variant.items() if k not in ['name', 'version']}
             variant_subname = variant.get('name', '')
 
+            # Add variant name as modifier if present
             if variant_subname:
                 modifiers['variant_type'] = variant_subname
                 modifiers['variant_subname'] = variant_subname
 
+            # Generate the recipe content
             content = generator.generate(recipe_type, version, **modifiers)
 
+            # Optional validation on generated content
             if args.validate and not args.dry_run:
-                content_validator = ContentValidator(generator.project_root)
                 issues = content_validator.validate(content)
-                semantic_validator = SemanticValidator()
                 semantic_issues = semantic_validator.validate(content, version)
                 all_issues = issues + semantic_issues
                 if all_issues:
@@ -266,20 +432,45 @@ def generate_from_manifest(args: argparse.Namespace, generator: RecipeGenerator)
                         print(f"  - {issue}", file=sys.stderr)
                     sys.exit(1)
 
+            # Generate output filename based on recipe parameters
             filename = generator.generate_filename(recipe_type, version, **modifiers)
             output_path = args.output_dir / filename
 
-
+            # Handle dry-run mode
             if args.dry_run:
                 print(f"Would generate: {output_path}")
             else:
+                # Actually write the file
                 print(f"Generating: {output_path}")
                 with open(output_path, 'w', encoding='utf-8') as f:
                     f.write(content)
 
 
 def generate_single(args: argparse.Namespace, generator: RecipeGenerator) -> None:
-    """Generate a single recipe."""
+    """Generate a single recipe with the specified parameters.
+    
+    This function handles single recipe generation mode. It:
+    1. Constructs a modifiers dictionary from command-line arguments
+    2. Only includes non-default values (e.g., only adds 'encrypted' if storage is encrypted)
+    3. Generates the recipe content
+    4. Optionally validates the content
+    5. Outputs to a file or stdout based on arguments
+    
+    Command-line parameter to modifier mapping:
+        --type -> recipe_type (required)
+        --version -> version (43 or rawhide)
+        --desktop -> desktop (gnome or labwc)
+        --storage -> storage (standard or encrypted)
+        --security -> security (secure or devel)
+        --cpu -> cpu (generic, amdcpu, or intelcpu)
+        --gpu -> gpu (none or intelgpu)
+    
+    Args:
+        args: Parsed command-line arguments
+        generator: Pre-configured RecipeGenerator instance
+    """
+    # Build modifiers dictionary from command-line arguments
+    # Only include non-default values to keep filenames clean
     modifiers = {
         'variant_type': 'desktop',
         'desktop': args.desktop if args.desktop else None,
@@ -288,10 +479,13 @@ def generate_single(args: argparse.Namespace, generator: RecipeGenerator) -> Non
         'cpu': args.cpu if args.cpu and args.cpu != 'generic' else None,
         'gpu': args.gpu if args.gpu and args.gpu != 'none' else None,
     }
+    # Remove Nones from modifiers
     modifiers = {k: v for k, v in modifiers.items() if v is not None}
 
+    # Generate the recipe
     content = generator.generate(args.type, args.version, **modifiers)
 
+    # Optional validation
     if args.validate:
         content_validator = ContentValidator(generator.project_root)
         issues = content_validator.validate(content)
@@ -306,7 +500,9 @@ def generate_single(args: argparse.Namespace, generator: RecipeGenerator) -> Non
         else:
             print("Validation passed")
 
+    # Output the recipe
     if args.output:
+        # Write to file
         if args.dry_run:
             print(f"Would write to: {args.output}")
         else:
@@ -314,4 +510,5 @@ def generate_single(args: argparse.Namespace, generator: RecipeGenerator) -> Non
                 f.write(content)
             print(f"Generated: {args.output}")
     else:
+        # Print to stdout
         print(content)

recipe-generator/generate_recipe.py

@@ -1,5 +1,23 @@
-#!/usr/bin/env python3
-"""Entry point for the recipe generator."""
+"""Entry point for the recipe generator.
+
+This is a simple wrapper script that provides the main entry point for running
+the recipe generator. It follows the common Python pattern of having a script
+that can be run directly or imported as a module.
+
+Usage:
+    # Run as script
+    python generate_recipe.py --type virtual-desktop --version 43 --output output.cfg
+    
+    # Or from another Python script
+    from generate_recipe import main
+    main()
+
+This module doesn't do any processing itself - it delegates to the cli.main()
+function which handles all the actual work. The separation allows for:
+- Easy command-line execution (this file is the entry point)
+- Module imports without triggering execution
+- Clean separation of concerns (cli.py handles CLI, this just delegates)
+"""
 
 from cli import main
 

recipe-generator/manifest.py

@@ -1,4 +1,32 @@
-"""Manifest loading and variant expansion."""
+"""Manifest loading and variant expansion.
+
+This module provides functionality for processing manifest YAML files, which
+are used in batch recipe generation mode. It has two main responsibilities:
+
+1. Loading and Parsing Manifests
+   - Reads YAML files containing recipe configurations
+   - Validates basic structure of the manifest
+
+2. Variant Expansion
+   - Converts compact manifest entries with list values into individual variants
+   - Uses cartesian product to generate all combinations
+   - Enables generating many recipes from a single manifest entry
+
+Manifest Example:
+    recipes:
+      - name: virtual-desktop
+        variants:
+          - version: 43
+            desktop: gnome
+          - version: ["43", "rawhide"]
+            storage: ["standard", "encrypted"]
+    
+    The above would generate 4 recipes (2 versions × 2 storage types).
+
+The key insight is that list values in variants represent "multiple values for
+this field", and we want to generate one recipe for each combination. This is
+the classic cartesian product problem.
+"""
 
 from __future__ import annotations
 
@@ -10,27 +38,79 @@ import yaml
 
 
 class ManifestProcessor:
-    """Load and process recipe manifests."""
+    """Load and process recipe manifests.
+    
+    This class handles the loading and processing of manifest YAML files.
+    A manifest is a YAML file that defines which recipes to generate and
+    what variations (variants) of each recipe to create.
+    
+    Key features:
+    - Loads and parses YAML manifest files
+    - Validates manifest structure (schema-level checks)
+    - Expands variants with list values into all combinations
+    
+    The variant expansion is particularly useful for generating multiple
+    versions (e.g., both Fedora 43 and rawhide) or multiple configurations
+    (e.g., both standard and encrypted storage) in a compact format.
+    """
 
     def __init__(self, project_root: Path):
+        """Initialize the processor with project root path.
+        
+        Args:
+            project_root: Path to the phyllomeos project root directory.
+                         Not currently used but available for future enhancements.
+        """
         self.project_root = project_root
 
     def load(self, path: Path) -> Dict:
-        """Load and parse manifest YAML file."""
+        """Load and parse manifest YAML file.
+        
+        This method reads a YAML file from disk and parses it into a Python
+        dictionary. It's a simple wrapper around yaml.safe_load that provides
+        error handling and a consistent interface.
+        
+        Args:
+            path: Path to the manifest YAML file
+        
+        Returns:
+            Parsed manifest as a dictionary
+        
+        Raises:
+            FileNotFoundError: If the file doesn't exist
+            yaml.YAMLError: If the file contains invalid YAML
+        """
         with open(path, encoding='utf-8') as f:
             return yaml.safe_load(f)
 
     def validate(self, manifest: Dict) -> List[str]:
         """Validate manifest structure.
         
-        Returns a list of validation warnings (not errors).
+        This method performs schema-level validation of a manifest dictionary.
+        It checks that:
+        1. The top-level 'recipes' key exists
+        2. Each recipe configuration has a 'name' field
+        3. Each recipe configuration has a 'variants' field
+        4. Each variant has a 'version' field
+        
+        This is a basic structural check that catches obvious errors before
+        trying to process the manifest. It doesn't validate the content of
+        individual recipes (that's done by RecipeGenerator and validators).
+        
+        Args:
+            manifest: The manifest dictionary to validate
+        
+        Returns:
+            List of validation error strings (empty if valid)
         """
         errors = []
 
+        # Check for required top-level key
         if 'recipes' not in manifest:
             errors.append("Manifest missing 'recipes' key")
-            return errors
+            return errors  # Can't proceed without recipes
 
+        # Validate each recipe configuration
         for recipe_config in manifest.get('recipes', []):
             if 'name' not in recipe_config:
                 errors.append("Recipe config missing 'name' key")
@@ -43,15 +123,52 @@ class ManifestProcessor:
     def expand_variants(self, variants: List[Dict]) -> List[Dict]:
         """Expand variants with list values into individual variants.
         
-        Converts variants like:
-            - version: ["43", "rawhide"]
-            - storage: ["standard", "encrypted"]
+        This is the core functionality of the manifest processor. It converts
+        a compact representation with list values into a complete list of
+        individual variants by generating the cartesian product.
         
-        Into cartesian product of all combinations.
+        Example Input:
+            [
+                {
+                    'version': ['43', 'rawhide'],
+                    'storage': ['standard', 'encrypted'],
+                    'desktop': 'gnome'
+                },
+                {
+                    'version': '43',
+                    'security': 'devel'
+                }
+            ]
+        
+        Example Output:
+            [
+                {'version': '43', 'storage': 'standard', 'desktop': 'gnome'},
+                {'version': '43', 'storage': 'encrypted', 'desktop': 'gnome'},
+                {'version': 'rawhide', 'storage': 'standard', 'desktop': 'gnome'},
+                {'version': 'rawhide', 'storage': 'encrypted', 'desktop': 'gnome'},
+                {'version': '43', 'security': 'devel'}
+            ]
+        
+        How it works:
+        1. For each variant dictionary, separate list-valued keys from scalar keys
+        2. If there are no lists, keep the variant as-is
+        3. If there are lists, generate the cartesian product of all list values
+        4. For each combination, create a new variant with scalar values preserved
+        5. Return the complete list of expanded variants
+        
+        This enables generating many recipes from a single compact manifest entry,
+        making it easy to generate variations across multiple dimensions.
+        
+        Args:
+            variants: List of variant dictionaries, where some values may be lists
+        
+        Returns:
+            Expanded list where each variant has only scalar (non-list) values
         """
         expanded = []
 
         for variant in variants:
+            # Separate list-valued keys from scalar keys
             list_keys = {}
             scalar_keys = {}
 
@@ -62,12 +179,15 @@ class ManifestProcessor:
                     scalar_keys[key] = value
 
             if not list_keys:
+                # No lists to expand, keep variant as-is
                 expanded.append(variant)
                 continue
 
+            # Get the cartesian product of all list values
             keys = list(list_keys.keys())
             values_product = product(*[list_keys[k] for k in keys])
 
+            # Create a new variant for each combination
             for combo in values_product:
                 new_variant = scalar_keys.copy()
                 for i, key in enumerate(keys):

recipe-generator/recipe_generator.py

@@ -1,9 +1,32 @@
-"""Core recipe generation logic."""
+"""Core recipe generation logic.
+
+This module is the heart of the Phyllome OS kickstart recipe generation system.
+It works like a sophisticated template engine that:
+
+1. Reads template definitions from recipe_templates.yaml
+2. Each template defines:
+   - Required ingredients (always included, like base system components)
+   - Modifiers (user choices like desktop environment, storage type)
+   - Optional ingredients (included based on modifier values)
+   - Versioned ingredients (Fedora version-specific paths)
+   - Conditional ingredients (complex logic based on multiple factors)
+   - Flags (boolean on/off switches for features)
+
+3. When you call generate(), it:
+   - Looks up the template for your recipe type (e.g., "virtual-desktop")
+   - Processes all the modifier values you passed in
+   - Builds a list of %include directives pointing to ingredient fragments
+   - Returns a complete kickstart file with header and includes
+
+Think of it as a "recipe assembler" - the templates are the master recipes,
+modifiers are your customizations, and the output is a complete installation
+script that pulls in the right ingredient fragments.
+"""
 
 from __future__ import annotations
 
 from pathlib import Path
-from typing import Dict, List, Optional
+from typing import Dict, List
 
 from validators import (
     TemplateValidator,
@@ -14,6 +37,9 @@ from validators import (
 
 import yaml
 
+# ASCII art banner displayed at the top of every generated recipe file.
+# This decorative header identifies the output as a Phyllome OS kickstart recipe.
+# The backslashes are escaped because they're inside a Python string.
 HEADER_ASCII_ART = [
     "#            __          ____                        ____  _____",
     "#     ____  / /_  __  __/ / /___  ____ ___  ___     / __ \\/ ___/",
@@ -26,37 +52,71 @@ HEADER_ASCII_ART = [
 
 
 class RecipeGenerator:
-    """Generate kickstart recipes from templates and modifiers."""
+    """Generate kickstart recipes from templates and modifiers.
     
-    def __init__(self, ingredients_dir_or_templates: Optional[Path] = None, templates_file: Optional[Path] = None):
+    This is the core class that powers the recipe generation system. It works by:
+    1. Loading template definitions from a YAML file (recipe_templates.yaml)
+    2. Taking a recipe type (like "virtual-desktop") and version (like "43")
+    3. Accepting optional modifiers (like desktop environment, storage type, security mode)
+    4. Building a list of %include directives that reference ingredient fragments
+    5. Outputting a complete kickstart file with header and include statements
+    
+    The templates define which ingredients are required, optional, conditional, or version-specific.
+    Modifiers control which optional ingredients get included based on user preferences.
+    
+    Example usage:
+        generator = RecipeGenerator(Path('recipe_templates.yaml'))
+        recipe = generator.generate('virtual-desktop', '43', desktop='gnome', storage='encrypted')
+        # Returns a string containing the complete kickstart recipe
+    """
+
+    def __init__(self, templates_file: Path):
         """Initialize RecipeGenerator.
         
         Args:
-            ingredients_dir_or_templates: Either ingredients_dir (deprecated) or templates_file
-            templates_file: Path to the templates YAML file (if ingredients_dir provided)
+            templates_file: Path to the templates YAML file.
+                           This should be the full path to recipe_templates.yaml.
         """
-        # Handle both positional arg patterns:
-        # RecipeGenerator(templates_file) - new style
-        # RecipeGenerator(ingredients_dir, templates_file) - old style
-        if templates_file is None:
-            # Old style: single arg which is actually templates_file
-            templates_file = ingredients_dir_or_templates
-        else:
-            # New style: both args provided (old style with ingredients_dir)
-            pass
-        
         self.project_root = templates_file.parent.parent
         self.templates = self._load_templates(templates_file)
 
     def _load_templates(self, path: Path) -> Dict:
-        """Load recipe templates from YAML file."""
+        """Load recipe templates from YAML file.
+        
+        This method reads the templates YAML file and extracts the 'templates' section.
+        The YAML file contains a top-level 'templates' key with all recipe type definitions.
+        
+        Args:
+            path: Path to the templates YAML file (can be absolute or relative)
+        
+        Returns:
+            Dictionary mapping recipe types to their template definitions.
+            Example structure:
+            {
+                'virtual-desktop': {
+                    'description': 'Virtual machine with desktop',
+                    'required': ['ingredients/base.cfg', ...],
+                    'modifiers': {'desktop': {...}, 'storage': {...}},
+                    'optional': {...},
+                    'versioned': {...},
+                    'conditional': {...},
+                    'flags': {...}
+                },
+                ...
+            }
+        
+        Raises:
+            SystemExit: If the file is not found or contains invalid YAML
+        """
         try:
             if path.is_absolute():
                 template_path = path
             else:
+                # Resolve relative paths from project root
                 template_path = self.project_root / path
             with open(template_path, encoding='utf-8') as f:
                 data = yaml.safe_load(f)
+            # Extract the 'templates' section - the YAML file has this as top-level key
             return data['templates']
         except FileNotFoundError:
             print(f"Error: Templates file not found: {template_path}")
@@ -66,64 +126,158 @@ class RecipeGenerator:
             exit(2)
 
     def generate(self, recipe_type: str, version: str, **modifiers) -> str:
-        """Generate a recipe from template with modifiers."""
+        """Generate a complete kickstart recipe from a template with modifiers.
+        
+        This is the main entry point for recipe generation. It takes a recipe type
+        (like "virtual-desktop"), a Fedora version, and any number of modifier options,
+        then returns a complete kickstart file as a string.
+        
+        Args:
+            recipe_type: The type of recipe to generate. Must match a key in the templates.
+                        Examples: 'virtual-desktop', 'server', 'hypervisor'
+            version: Fedora version string, typically '43' or 'rawhide'.
+                    This affects which versioned ingredients are included.
+            **modifiers: Keyword arguments for recipe customization:
+                        - desktop: 'gnome' or 'labwc'
+                        - storage: 'standard' or 'encrypted'
+                        - security: 'secure' or 'devel'
+                        - cpu: 'generic', 'amdcpu', or 'intelcpu'
+                        - gpu: 'none' or 'intelgpu'
+                        - guest_agents: True/False for virtualization tools
+                        - And many others...
+        
+        Returns:
+            A string containing the complete kickstart recipe, including:
+            - ASCII art header
+            - Description comment
+            - %include directives for all required and selected ingredients
+            
+        Raises:
+            SystemExit: If the recipe_type is not found in templates
+        """
         if recipe_type not in self.templates:
             print(f"Error: Unknown recipe type: {recipe_type}")
             exit(1)
 
         template = self.templates[recipe_type]
 
+        # Build the output in two parts:
+        # 1. Header with ASCII art and description
+        # 2. All the %include lines for ingredients
         lines = self._build_header(template['description'])
         lines.extend(self._build_includes(template, version, modifiers))
 
+        # Join all lines with newlines to create the final recipe string
         return '\n'.join(lines)
     
-    generate_recipe = generate  # Compatibility alias
+    generate_recipe = generate  # Compatibility alias - old code may use this name
 
     def _build_header(self, description: str) -> List[str]:
-        """Build the ASCII art header and description."""
+        """Build the ASCII art header and description for the recipe.
+        
+        Creates the decorative banner that appears at the top of every generated
+        kickstart file. This helps identify the file as a Phyllome OS recipe and
+        describes what type of system it will install.
+        
+        Args:
+            description: Human-readable description of the recipe type
+                        (e.g., "Virtual desktop environment")
+        
+        Returns:
+            List of strings representing the header lines, including:
+            - The ASCII art banner (from HEADER_ASCII_ART constant)
+            - A comment line with the recipe description
+            - An empty line to separate header from content
+        """
         header = HEADER_ASCII_ART.copy()
         header.append(f"# {description}")
         header.append("")
         return header
 
     def _build_includes(self, template: Dict, version: str, modifiers: Dict) -> List[str]:
-        """Build %include lines from template and modifiers."""
-        includes = []
-        seen = set()
+        """Build %include lines from template and modifiers.
         
-        # Add version to modifiers for template processing
+        This is the most complex method in the class - it's the core logic that
+        determines which ingredient fragments get included in the final recipe.
+        
+        The method processes six different types of ingredient sources:
+        
+        1. REQUIRED: Always included, no conditions
+           Example: base system packages, partition layout
+        
+        2. MODIFIERS: User-specified options that map to ingredient paths
+           Example: desktop='gnome' → ingredients/desktops/gnome.cfg
+        
+        3. OPTIONAL: Included only if the modifier is present
+           Example: gpu='intelgpu' → ingredients/gpu/intel.cfg
+        
+        4. VERSIONED: Paths with {version} placeholder substituted
+           Example: ingredients/repo/f{version}.cfg → ingredients/repo/f43.cfg
+        
+        5. CONDITIONAL: Complex logic based on modifier values
+           Example: If security='devel', include development tools
+        
+        6. FLAGS: Boolean on/off switches
+           Example: hardware_support=True → ingredients/hardware-detection.cfg
+        
+        The 'seen' set prevents duplicate includes if multiple paths reference
+        the same ingredient.
+        
+        Args:
+            template: The template dictionary for this recipe type
+            version: Fedora version string for versioned paths
+            modifiers: Dictionary of user-specified options
+        
+        Returns:
+            List of %include directive strings, one per ingredient
+        """
+        includes = []
+        seen = set()  # Track which ingredients we've already added
+
+        # Add version to modifiers so templates can reference it
         modifiers = modifiers.copy()
         modifiers['version'] = version
 
-        # Add required includes (all ingredients listed under 'required')
+        # === 1. REQUIRED INGREDIENTS ===
+        # These are always included, regardless of user options
+        # Example: base system, bootloader, partition scheme
         for item in template.get('required', []):
             if isinstance(item, dict):
+                # Dict format allows conditional logic within required
                 fragment_path = list(item.values())[0]
             else:
+                # Simple string path
                 fragment_path = item
 
             if fragment_path not in seen:
                 includes.append(f"%include {fragment_path}")
                 seen.add(fragment_path)
 
-        # Add modifiers section includes
+        # === 2. MODIFIER INGREDIENTS ===
+        # Process user-specified options like desktop, storage, security
         for mod_key, mod_value in modifiers.items():
             # Normalize key: convert underscores to hyphens for template lookup
+            # This allows Python-style snake_case (guest_agents) to match
+            # YAML-style kebab-case (guest-agents)
             mod_key_normalized = mod_key.replace("_", "-")
             if mod_key_normalized in template.get("modifiers", {}):
                 mod_key_to_use = mod_key_normalized
             elif mod_key in template.get("modifiers", {}):
                 mod_key_to_use = mod_key
             else:
+                # This modifier doesn't have a mapping in the template, skip it
                 continue
             mod_config = template["modifiers"][mod_key_to_use]
             
             # Handle nested dict modifiers with string values
+            # Example: mod_config = {'gnome': 'ingredients/gnome.cfg', 'labwc': 'ingredients/labwc.cfg'}
+            #          mod_value = 'gnome'
+            # Result: include ingredients/gnome.cfg
             if isinstance(mod_config, dict) and isinstance(mod_value, str):
                 if mod_value in mod_config:
                     fragment_path = mod_config[mod_value]
                     if isinstance(fragment_path, list):
+                        # Some modifiers map to multiple ingredients
                         for fp in fragment_path:
                             if fp is not None and fp not in seen:
                                 includes.append(f"%include {fp}")
@@ -133,6 +287,7 @@ class RecipeGenerator:
                         seen.add(fragment_path)
             
             # Handle list modifiers
+            # Example: User passes multiple values like hypervisor_type=['kvm', 'xen']
             elif isinstance(mod_config, dict) and isinstance(mod_value, list):
                 for item in mod_value:
                     if item in mod_config:
@@ -146,12 +301,16 @@ class RecipeGenerator:
                             includes.append(f"%include {fragment_path}")
                             seen.add(fragment_path)
 
-        # Add optional includes based on modifiers
+        # === 3. OPTIONAL INGREDIENTS ===
+        # Included based on modifier presence and value
+        # More flexible than modifiers - can handle complex nested structures
         for opt_key, opt_config in template.get("optional", {}).items():
             if opt_key in modifiers:
                 value = modifiers[opt_key]
                 
                 # Case 1: opt_config is nested dict, value is string/int
+                # Example: opt_config = {'gnome': 'ingredients/gnome.cfg'}
+                #          value = 'gnome'
                 if isinstance(opt_config, dict) and not isinstance(value, (dict, list)):
                     if value in opt_config:
                         fragment_path = opt_config[value]
@@ -165,6 +324,8 @@ class RecipeGenerator:
                             seen.add(fragment_path)
                 
                 # Case 2: opt_config is nested dict, value is dict
+                # Example: value = {'kvm': True, 'xen': False}
+                # Include ingredients for keys that are present
                 elif isinstance(opt_config, dict) and isinstance(value, dict):
                     for nested_key in value:
                         if nested_key in opt_config:
@@ -178,7 +339,8 @@ class RecipeGenerator:
                                 includes.append(f"%include {nested_value}")
                                 seen.add(nested_value)
                 
-                # Case 3: opt_config is nested dict, value is boolean
+                # Case 3: opt_config is nested dict, value is boolean True
+                # Include ALL options when value is True
                 elif isinstance(opt_config, dict) and isinstance(value, bool) and value:
                     for nested_key, nested_value in opt_config.items():
                         if isinstance(nested_value, list):
@@ -190,32 +352,41 @@ class RecipeGenerator:
                             includes.append(f"%include {nested_value}")
                             seen.add(nested_value)
                 
-                # Case 4: opt_config is list, value is boolean
+                # Case 4: opt_config is list, value is boolean True
+                # Include all items in the list
                 elif isinstance(opt_config, list) and value is True:
                     for fragment_path in opt_config:
                         if fragment_path is not None and fragment_path not in seen:
                             includes.append(f"%include {fragment_path}")
                             seen.add(fragment_path)
 
-        # Add versioned includes
+        # === 4. VERSIONED INGREDIENTS ===
+        # Paths that change based on Fedora version
+        # Example: 'repo': 'ingredients/repo/f{version}.cfg'
+        # Becomes: ingredients/repo/f43.cfg or ingredients/repo/frawhide.cfg
         versioned = template.get('versioned', {})
         for fragment_path in versioned.values():
-            # Substitute {version} placeholder
+            # Substitute {version} placeholder with actual version
             resolved_path = fragment_path.format(version=version)
             if resolved_path not in seen:
                 includes.append(f"%include {resolved_path}")
                 seen.add(resolved_path)
 
-        # Add conditional includes based on modifiers
+        # === 5. CONDITIONAL INGREDIENTS ===
+        # Similar to optional, but with different template structure
+        # Used for more complex conditional logic
         conditional = template.get('conditional', {})
         for mod_key, mod_config in conditional.items():
+            # Support both hyphenated and underscored key names
             if mod_key.replace('-', '_') in modifiers:
                 value = modifiers[mod_key.replace('-', '_')]
             elif mod_key in modifiers:
                 value = modifiers[mod_key]
             else:
+                # This modifier wasn't provided, skip
                 continue
 
+            # Same four cases as optional section above
             if isinstance(mod_config, dict) and not isinstance(value, (dict, list)):
                 if value in mod_config:
                     fragment_path = mod_config[value]
@@ -258,7 +429,10 @@ class RecipeGenerator:
                         includes.append(f"%include {fragment_path}")
                         seen.add(fragment_path)
 
-        # Add flag includes (boolean toggles)
+        # === 6. FLAG INGREDIENTS ===
+        # Simple boolean on/off switches
+        # Example: hardware_support=True → include hardware detection
+        # Only includes when value is exactly True (not False or None)
         for mod_key, mod_value in modifiers.items():
             mod_key_normalized = mod_key.replace('_', '-')
             if mod_key_normalized in template.get('flags', {}):
@@ -270,7 +444,22 @@ class RecipeGenerator:
         return includes
 
     def _get_modifier(self, modifiers: dict, key: str, default=None):
-        """Get modifier value, checking both hyphenated and underscored versions."""
+        """Get modifier value, checking both hyphenated and underscored versions.
+        
+        This helper method handles the naming convention mismatch between:
+        - Python code: uses snake_case (e.g., guest_agents)
+        - YAML templates: use kebab-case (e.g., guest-agents)
+        
+        It tries both versions so users can pass modifiers using either convention.
+        
+        Args:
+            modifiers: Dictionary of modifier values
+            key: The key to look up (will try both this and the hyphenated version)
+            default: Value to return if key is not found
+        
+        Returns:
+            The modifier value if found, otherwise the default
+        """
         if key in modifiers:
             return modifiers[key]
         alt_key = key.replace('_', '-')
@@ -279,17 +468,43 @@ class RecipeGenerator:
         return default
 
     def generate_filename(self, recipe_type: str, version: str, **modifiers) -> str:
-        """Generate recipe filename from parameters."""
+        """Generate recipe filename from parameters.
         
+        Creates a descriptive filename that encodes the recipe's configuration.
+        This allows users to identify what a recipe does just from its name.
+        
+        Filename format:
+        {recipe_type}-{guest_type}-{variant}-{desktop}-{security}-{storage}-{version}.cfg
+        
+        Examples:
+        - virtual-desktop_gnome_43.cfg
+        - server_encrypted_43.cfg
+        - hypervisor_kvm_devel_43.cfg
+        
+        The method only adds suffixes for non-default values:
+        - GNOME is default → only add if different (labwc)
+        - secure is default → only add if devel
+        - standard is default → only add if encrypted
+        
+        Args:
+            recipe_type: Base recipe type (e.g., 'virtual-desktop')
+            version: Fedora version (e.g., '43' or 'rawhide')
+            **modifiers: All the modifier values that affect the recipe
+        
+        Returns:
+            Filename string ending in .cfg
+        """
         # Extract variant subname if present
+        # Used to distinguish between desktop/server/hypervisor variants
         variant_subname = modifiers.get('variant_subname', '')
         if not variant_subname:
             variant_subname = modifiers.get('variant_type', '')
 
-        # Build base parts
+        # Build base parts - start with recipe type
         parts = [recipe_type.replace('_', '-')]
 
         # Add guest_agents suffix (for both True and False)
+        # Distinguishes between virtual machines and bare metal installs
         guest_agents = self._get_modifier(modifiers, 'guest_agents')
         if guest_agents is True:
             parts.append('virtual')
@@ -297,61 +512,103 @@ class RecipeGenerator:
             parts.append('bare-metal')
 
         # Add variant_subname for install variants
+        # Only include recognized variant types
         if variant_subname and variant_subname in ['desktop', 'server', 'hypervisor', 'hypervisor-desktop']:
             parts.append(variant_subname)
 
         # Add hypervisor_type suffix
+        # For hypervisors, include the type (kvm, xen, etc.)
         if modifiers.get('hypervisor_type'):
             ht = modifiers['hypervisor_type']
             if isinstance(ht, list):
+                # Multiple hypervisor types
                 for h in ht:
                     if h:
                         parts.append(h)
             elif ht:
+                # Single hypervisor type
                 parts.append(ht)
 
         # Add desktop (non-GNOME only, since GNOME is default)
+        # GNOME is the default desktop, so we only note alternatives
         desktop = self._get_modifier(modifiers, 'desktop')
         if desktop and desktop != 'gnome':
             parts.append(desktop)
 
         # Add security suffix (devel only, since secure is default)
+        # Security defaults to 'secure', so only 'devel' gets noted
         security = self._get_modifier(modifiers, 'security')
         if security == 'off':
             parts.append('devel')
 
         # Add storage suffix (encrypted only, since standard is default)
+        # Standard storage is default, encrypted gets noted
         storage = self._get_modifier(modifiers, 'storage')
         if storage == 'encrypted':
             parts.append('encrypted')
 
         # Add hardware_support suffix (for True only)
+        # Hardware support detection is optional
         hardware_support = self._get_modifier(modifiers, 'hardware_support')
         if hardware_support is True:
             parts.append('hw')
 
         # Add initial_setup suffix (non-server values)
+        # Server is default, other setup types get noted
         initial_setup = self._get_modifier(modifiers, 'initial_setup')
         if initial_setup and initial_setup != 'server':
             parts.append(f'{initial_setup}-setup')
 
         # Add bootloader suffix (systemd-boot only)
+        # Default bootloader doesn't get noted
         bootloader = self._get_modifier(modifiers, 'bootloader')
         if bootloader == 'systemd-boot':
             parts.append('systemd-boot')
 
-        # Add version
+        # Add version - always included
         parts.append(str(version))
 
+        # Join all parts with underscores and add .cfg extension
         return '_'.join(parts) + '.cfg'
 
     def expand_variants(self, variants: List[Dict]) -> List[Dict]:
-        """Expand variants with list values into individual variants."""
+        """Expand variants with list values into individual variants.
+        
+        This method enables batch generation by converting compact manifest
+        entries with list values into all possible combinations.
+        
+        Example input:
+            [
+                {
+                    'version': ['43', 'rawhide'],
+                    'storage': ['standard', 'encrypted'],
+                    'desktop': 'gnome'
+                }
+            ]
+        
+        Example output (cartesian product = 2×2×1 = 4 variants):
+            [
+                {'version': '43', 'storage': 'standard', 'desktop': 'gnome'},
+                {'version': '43', 'storage': 'encrypted', 'desktop': 'gnome'},
+                {'version': 'rawhide', 'storage': 'standard', 'desktop': 'gnome'},
+                {'version': 'rawhide', 'storage': 'encrypted', 'desktop': 'gnome'}
+            ]
+        
+        This uses itertools.product to generate the cartesian product of all
+        list-valued keys, while preserving scalar values across all combinations.
+        
+        Args:
+            variants: List of variant dictionaries, where some values may be lists
+        
+        Returns:
+            Expanded list where each variant has only scalar (non-list) values
+        """
         from itertools import product as itertools_product
 
         expanded = []
 
         for variant in variants:
+            # Separate list-valued keys from scalar keys
             list_keys = {}
             scalar_keys = {}
 
@@ -362,12 +619,15 @@ class RecipeGenerator:
                     scalar_keys[key] = value
 
             if not list_keys:
+                # No lists to expand, keep as-is
                 expanded.append(variant)
                 continue
 
+            # Get cartesian product of all list values
             keys = list(list_keys.keys())
             values_product = itertools_product(*[list_keys[k] for k in keys])
 
+            # Create a new variant for each combination
             for combo in values_product:
                 new_variant = scalar_keys.copy()
                 for i, key in enumerate(keys):
@@ -376,37 +636,112 @@ class RecipeGenerator:
 
         return expanded
 
+    # === VALIDATION METHODS ===
+    # These are convenience wrappers that delegate to the validators module.
+    # They provide a unified API so callers can validate through the generator.
+
     def validate_template(self, template: Dict) -> List[str]:
-        """Validate template structure and fragment existence."""
+        """Validate template structure and fragment existence.
+        
+        Checks that:
+        - Required keys are present (description, required)
+        - All referenced ingredient files actually exist on disk
+        - Versioned, conditional, and flag paths are valid
+        
+        Args:
+            template: Template dictionary to validate
+        
+        Returns:
+            List of validation error strings (empty if valid)
+        """
         validator = TemplateValidator(self.project_root)
         return validator.validate(template)
 
     def validate_manifest(self, manifest: Dict) -> List[str]:
-        """Validate manifest structure."""
+        """Validate manifest structure.
+        
+        Checks that the manifest has the required 'recipes' key and that
+        each recipe has required fields (name, variants with version).
+        
+        Args:
+            manifest: Manifest dictionary to validate
+        
+        Returns:
+            List of validation error strings (empty if valid)
+        """
         return validate_manifest(manifest)
 
     def validate_recipe(self, content: str) -> List[str]:
-        """Validate recipe content."""
+        """Validate recipe content.
+        
+        Checks that:
+        - No duplicate %include directives
+        - All referenced ingredient files exist
+        
+        Args:
+            content: Recipe content string to validate
+        
+        Returns:
+            List of validation error strings (empty if valid)
+        """
         validator = ContentValidator(self.project_root)
         return validator.validate(content)
 
     def validate_recipe_semantic(self, content: str, version: str) -> List[str]:
-        """Validate recipe using pykickstart parser."""
+        """Validate recipe using pykickstart parser.
+        
+        Uses the official pykickstart library to parse the recipe and check
+        for syntax errors, invalid directives, or semantic issues.
+        
+        Args:
+            content: Recipe content string to validate
+            version: Fedora version for version-specific validation rules
+        
+        Returns:
+            List of validation error strings (empty if valid)
+        """
         validator = SemanticValidator()
         return validator.validate(content, version)
 
     def get_ksversion(self, version: str) -> Optional[str]:
-        """Map Phyllome OS version to pykickstart version string."""
+        """Map Phyllome OS version to pykickstart version string.
+        
+        Pykickstart uses Fedora version naming (F42, F43, etc.).
+        This method converts Phyllome OS versions to the corresponding
+        kickstart version string.
+        
+        Args:
+            version: Phyllome OS version (e.g., '43' or 'rawhide')
+        
+        Returns:
+            Pykickstart version string (e.g., 'F42' for version 43)
+            or None for rawhide (uses latest development version)
+        """
         return SemanticValidator().get_ksversion(version)
 
     def extract_version(self, content: str, filename: str) -> Optional[str]:
-        """Extract Fedora version from recipe content or filename."""
+        """Extract Fedora version from recipe content or filename.
+        
+        Uses multiple heuristics to determine which Fedora version a recipe
+        targets:
+        1. Check filename for version patterns (e.g., _43_, -rawhide.cfg)
+        2. Check content for version-specific repository paths
+        
+        Args:
+            content: Recipe content string
+            filename: Recipe filename
+        
+        Returns:
+            Version string ('43' or 'rawhide') or None if undetermined
+        """
         import re
 
+        # Try to extract from filename first
         filename_match = re.search(r'(?:_|-)(43|rawhide)(?:_|-|.cfg|.yaml|$)', filename)
         if filename_match:
             return filename_match.group(1)
 
+        # Fall back to content inspection
         for line in content.split('\n'):
             if 'core-fedora-repo-43' in line:
                 return '43'
@@ -420,16 +755,46 @@ class RecipeGenerator:
         return None
 
     def extract_version_from_file(self, recipe_path: str, filename: str) -> Optional[str]:
-        """Extract version from recipe file."""
+        """Extract version from recipe file.
+        
+        Convenience wrapper that reads the file and delegates to extract_version.
+        
+        Args:
+            recipe_path: Path to the recipe file
+            filename: Filename (used for pattern matching)
+        
+        Returns:
+            Version string or None
+        """
         content = Path(recipe_path).read_text(encoding='utf-8')
         return self.extract_version(content, filename)
 
     def validate_recipe_content(self, recipe_path: str) -> List[str]:
-        """Validate recipe content from file path."""
+        """Validate recipe content from file path.
+        
+        Reads a recipe file and validates its content for ingredient existence
+        and duplicate includes.
+        
+        Args:
+            recipe_path: Path to the recipe file
+        
+        Returns:
+            List of validation error strings
+        """
         content = Path(recipe_path).read_text(encoding='utf-8')
         return self.validate_recipe(content)
 
     def validate_recipe_semantic_from_file(self, recipe_path: str, version: str) -> List[str]:
-        """Validate recipe from file path using pykickstart."""
+        """Validate recipe from file path using pykickstart.
+        
+        Reads a recipe file and validates it with the pykickstart parser.
+        
+        Args:
+            recipe_path: Path to the recipe file
+            version: Fedora version for validation rules
+        
+        Returns:
+            List of validation error strings
+        """
         content = Path(recipe_path).read_text(encoding='utf-8')
         return self.validate_recipe_semantic(content, version)

recipe-generator/validators.py

@@ -1,4 +1,31 @@
-"""Validation logic for recipes and templates."""
+"""Validation logic for recipes and templates.
+
+This module provides three layers of validation for Phyllome OS kickstart recipes:
+
+1. TemplateValidator
+   - Validates the template YAML structure itself
+   - Checks that all referenced ingredient files actually exist on disk
+   - Validates required, versioned, conditional, optional, and flag paths
+
+2. ContentValidator
+   - Validates the generated recipe content
+   - Checks for duplicate %include directives
+   - Verifies all referenced ingredients exist on disk
+
+3. SemanticValidator
+   - Uses the official pykickstart library to parse recipes
+   - Checks for syntax errors and invalid kickstart directives
+   - Validates against Fedora version-specific kickstart rules
+
+All validators return lists of warning/error strings. They don't raise exceptions
+for minor issues; instead, they collect and report issues so callers can decide
+how to handle them (warn, error, exit, etc.)
+
+The separation allows for:
+- Early detection of template issues (TemplateValidator)
+- Detection of generation issues (ContentValidator)  
+- Detection of actual syntax errors (SemanticValidator)
+"""
 
 from __future__ import annotations
 
@@ -10,36 +37,94 @@ from pykickstart.version import makeVersion, DEVEL
 
 
 class TemplateValidator:
-    """Validate template structure and fragment existence."""
+    """Validate template structure and fragment existence.
+    
+    This validator checks that:
+    1. Required template keys are present (description, required)
+    2. All ingredient paths referenced in the template exist on disk
+    3. The template structure matches expected format
+    
+    It's typically used:
+    - During development to catch template errors early
+    - In CI/CD pipelines to validate templates before use
+    - Programmatically when loading templates
+    
+    The validator doesn't check for semantic issues in the ingredients themselves -
+    it only verifies that they exist and are properly referenced.
+    """
 
     def __init__(self, project_root: Path):
+        """Initialize the validator with project root path.
+        
+        Args:
+            project_root: Path to the phyllomeos project root directory.
+                         All ingredient paths in templates are relative to this.
+        """
         self.project_root = project_root
 
     def validate(self, template: Dict) -> List[str]:
         """Validate template structure and fragment existence.
         
-        Returns a list of validation warnings (not errors).
+        This method performs a comprehensive check of the template dictionary:
+        
+        1. Required Keys Check:
+           - Verifies 'description' key exists (used in header)
+           - Verifies 'required' key exists (base ingredients)
+        
+        2. Required Ingredients Check:
+           - Iterates through template['required'] list
+           - Resolves each path relative to project_root
+           - Checks if the file actually exists
+           - Reports missing files as errors
+        
+        3. Versioned Ingredients Check:
+           - Checks paths in template['versioned']
+           - If the path contains {version}, it's deferred to generation time
+           - Otherwise checks if the resolved file exists
+        
+        4. Conditional Ingredients Check:
+           - Iterates through template['conditional']
+           - For each modifier and value, checks referenced files exist
+           - Handles both single paths and lists of paths
+        
+        5. Flag Ingredients Check:
+           - Checks all paths in template['flags']
+           - Flags are boolean on/off switches
+        
+        Args:
+            template: The template dictionary to validate
+                     Structure: {'description': str, 'required': [...], ...}
+        
+        Returns:
+            List of validation error strings (empty if valid)
+            Example: ['Missing required key: description', 'Required ingredient not found: ingredients/gnome.cfg']
         """
         errors = []
 
-        # Check required keys
+        # Check required keys at the template level
         required_keys = ['description', 'required']
         for key in required_keys:
             if key not in template:
                 errors.append(f"Missing required key: {key}")
 
-        # Validate required ingredients exist
+        # Validate required ingredients exist on disk
         for item in template.get('required', []):
             if isinstance(item, dict):
+                # Dict format: {priority: path} or {condition: path}
                 fragment_path = list(item.values())[0]
             else:
+                # Simple string path
                 fragment_path = item
 
+            # Resolve relative path from project root
             full_path = self.project_root / fragment_path
             if not full_path.exists():
                 errors.append(f"Required ingredient not found: {fragment_path}")
 
-        # Validate versioned ingredients
+        # Validate versioned ingredients exist on disk
+        # These are paths like 'ingredients/repo/f{version}.cfg'
+        # If the path has {version}, it'll be resolved at generation time
+        # Otherwise, check if it exists now
         for key, fragment_path in template.get('versioned', {}).items():
             if '{version}' in fragment_path:
                 # Will be resolved at generation time
@@ -48,14 +133,18 @@ class TemplateValidator:
             if not full_path.exists():
                 errors.append(f"Versioned ingredient not found: {fragment_path}")
 
-        # Validate conditional ingredients
+        # Validate conditional ingredients exist on disk
+        # Conditional ingredients depend on modifier values
         conditional = template.get('conditional', {})
         for modifier, modifier_config in conditional.items():
             if isinstance(modifier_config, dict):
+                # Dict format: {'value': 'path'} or {'value': ['path1', 'path2']}
                 for value, fragment_path in modifier_config.items():
                     if fragment_path is None:
+                        # None means "don't include anything for this value"
                         continue
                     if isinstance(fragment_path, list):
+                        # List of paths (e.g., multiple ingredients for this value)
                         for fp in fragment_path:
                             if fp is not None:
                                 full_path = self.project_root / fp
@@ -66,7 +155,8 @@ class TemplateValidator:
                         if not full_path.exists():
                             errors.append(f"Conditional ingredient not found: {fragment_path} (for {modifier}={value})")
 
-        # Validate flag ingredients
+        # Validate flag ingredients exist on disk
+        # Flags are simple boolean on/off switches
         for key, fragment_path in template.get('flags', {}).items():
             if fragment_path is None:
                 continue
@@ -78,15 +168,63 @@ class TemplateValidator:
 
 
 class ContentValidator:
-    """Validate recipe content for ingredient existence and duplicates."""
+    """Validate recipe content for ingredient existence and duplicates.
+    
+    This validator checks the content of a generated recipe file:
+    
+    1. Duplicate Include Detection:
+       - Scans for all %include directives
+       - Tracks which ingredient paths have been seen
+       - Reports if the same path appears multiple times
+       - This prevents redundant loading and potential conflicts
+    
+    2. Ingredient Existence Check:
+       - For each %include directive, Verifies the referenced file exists
+       - Uses project_root to resolve relative paths
+       - Reports missing files so generators can catch issues early
+    
+    The validator is designed to run on generated recipes, not templates.
+    It's lighter than TemplateValidator and runs during generation to
+    catch issues immediately.
+    """
 
     def __init__(self, project_root: Path):
+        """Initialize the validator with project root path.
+        
+        Args:
+            project_root: Path to the phyllomeos project root directory.
+                         All ingredient paths are relative to this.
+        """
         self.project_root = project_root
 
     def validate(self, content: str) -> List[str]:
-        """Validate recipe content.
+        """Validate recipe content for issues.
         
-        Returns a list of validation warnings (not errors).
+        This method parses the recipe content string and checks for:
+        
+        1. Duplicate Includes:
+           - Extracts all %include directives
+           - Tracks seen paths in a set
+           - Reports if a path appears more than once
+           - Duplicates are problematic because they slow down generation
+             and can cause conflicts in the final kickstart file
+        
+        2. Missing Ingredients:
+           - For each %include directive, checks if the file exists
+           - Path is resolved relative to project_root
+           - Reports missing files that would cause generation to fail
+           - This catches typos in template paths before runtime
+        
+        The validation is "best effort" - it doesn't raise exceptions for
+        minor issues but collects all problems to report them together.
+        
+        Args:
+            content: The recipe content string to validate
+                    Contains %include directives and other kickstart code
+        
+        Returns:
+            List of validation warning strings (empty if valid)
+            Example: ['Duplicate include: ingredients/gnome.cfg', 'Missing ingredient: ingredients/unknown.cfg']
         """
         issues = []
         includes = [line for line in content.split('\n') if line.startswith('%include')]
@@ -96,6 +234,7 @@ class ContentValidator:
         for inc in includes:
             parts = inc.split()
             if len(parts) < 2:
+                # Malformed include line, skip
                 continue
             path = parts[1]
             if path in seen:
@@ -116,60 +255,160 @@ class ContentValidator:
 
 
 class SemanticValidator:
-    """Validate recipe using pykickstart parser."""
+    """Validate recipe using pykickstart parser.
+    
+    This is the most thorough validator - it actually parses the recipe
+    as a kickstart file and checks for syntax errors and semantic issues.
+    
+    It uses the pykickstart library, which is the same library Anaconda uses
+    to parse kickstart files. This means it catches:
+    - Syntax errors in kickstart directives
+    - Invalid options or values
+    - Incompatible directives for the target Fedora version
+    - Other structural issues
+    
+    The validator is lenient about include resolution issues (file not found)
+    because those are expected - the actual ingredient files are included
+    during the installation, not at generation time.
+    
+    Version Mapping:
+    - '43' -> pykickstart F42 (Phyllome OS 43 is based on Fedora 42)
+    - 'rawhide' -> DEVEL (development version, uses latest rules)
+    - This accounts for the fact that Phyllome OS lags Fedora by one version
+    """
 
     def validate(self, content: str, version: str) -> List[str]:
         """Validate recipe using pykickstart parser.
         
-        Returns a list of validation warnings (not errors).
+        This method attempts to parse the recipe content as a kickstart file
+        using the pykickstart library. It:
+        
+        1. Maps Phyllome OS version to pykickstart version string
+        2. Creates a KickstartParser with the appropriate version
+        3. Attempts to parse the content
+        4. Catches and reports any parsing errors
+        
+        The parser is configured to be lenient about missing includes
+        (file not found errors) because those are expected - the ingredients
+        are resolved at installation time, not generation time.
+        
+        Args:
+            content: The recipe content string to validate
+            version: Phyllome OS version string ('43' or 'rawhide')
+                    Determines which kickstart version rules to apply
+        
+        Returns:
+            List of validation error strings (empty if valid)
+            Only reports actual validation errors, not file missing errors
         """
         issues = []
 
         try:
+            # Get pykickstart version string
             ks_version_str = self.get_ksversion(version)
             if ks_version_str:
+                # Create version-specific parser
                 ks_version = makeVersion(ks_version_str)
             else:
+                # Use development version for rawhide
                 ks_version = makeVersion(DEVEL)
 
+            # Parse the content
             parser = KickstartParser(ks_version)
             parser.readKickstartFromString(content)
         except Exception as e:
             # Only report actual validation errors, not include resolution issues
             err_str = str(e)
             if 'Unable to open input kickstart file' not in err_str:
+                # This is a real validation error, not a missing include
                 issues.append(f"Validation error: {err_str}")
 
         return issues
 
     def get_ksversion(self, version: str) -> Optional[str]:
-        """Map Phyllome OS version to pykickstart version string."""
+        """Map Phyllome OS version to pykickstart version string.
+        
+        This helper method converts Phyllome OS version strings to the
+        corresponding pykickstart version strings.
+        
+        Pykickstart uses Fedora version naming:
+        - F42, F43, etc. for stable releases
+        - DEVEL for development versions
+        
+        Phyllome OS version mapping:
+        - Phyllome OS 43 is based on Fedora 42, so use F42
+        - Phyllome OS 44 is based on Fedora 43, so use F43
+        - The pattern: F(Phyllome_OS_version - 1)
+        
+        For rawhide, we use None which tells pykickstart to use the
+        latest development version.
+        
+        Args:
+            version: Phyllome OS version string ('43' or 'rawhide')
+        
+        Returns:
+            Pykickstart version string ('F42', 'F43', etc.) or None for rawhide
+        """
         if version == 'rawhide':
             return None
         else:
+            # Phyllome OS 43 -> Fedora 42 -> F42
             return f'F{int(version) - 1}'
 
 
 def validate_manifest(manifest: Dict) -> List[str]:
-    """Validate manifest structure."""
+    """Validate manifest structure.
+    
+    This function validates the high-level structure of a manifest YAML file.
+    It checks that the manifest has the required sections and that each recipe
+    configuration has the necessary fields.
+    
+    The manifest structure:
+        recipes:
+          - name: virtual-desktop
+            variants:
+              - version: 43
+                desktop: gnome
+              - version: rawhide
+                storage: encrypted
+    
+    Validation checks:
+    1. 'recipes' key exists at top level
+    2. Each recipe config has 'name' key
+    3. Each recipe config has 'variants' key
+    4. Each variant has 'version' key
+    
+    This is a schema-level validation that catches structural errors before
+    trying to process the manifest. It's less detailed than template validation
+    but catches the most obvious problems early.
+    
+    Args:
+        manifest: The manifest dictionary to validate
+    
+    Returns:
+        List of validation error strings (empty if valid)
+        Example: ["Manifest missing 'recipes' key", "Recipe 'virtual-desktop' variant missing 'version'"]
+    """
     errors = []
 
+    # Check for required top-level key
     if 'recipes' not in manifest:
         errors.append("Manifest missing 'recipes' key")
-        return errors
+        return errors  # Can't continue without recipes
 
+    # Validate each recipe configuration
     for recipe_config in manifest.get('recipes', []):
         if 'name' not in recipe_config:
             errors.append("Recipe config missing 'name' key")
+        
         if 'variants' not in recipe_config:
+            # Recipe doesn't have variants, can't validate further
             errors.append(f"Recipe '{recipe_config.get('name', 'unnamed')}' "
                         "missing 'variants' key")
         else:
+            # Each variant must have version
             for variant in recipe_config.get('variants', []):
                 if 'version' not in variant:
-                    if 'name' in recipe_config:
-                        errors.append(f"Recipe '{recipe_config['name']}' variant missing 'version'")
-                    else:
-                        errors.append("Recipe variant missing 'version'")
+                    errors.append(f"Recipe '{recipe_config['name']}' variant missing 'version'")
 
     return errors