Merge pull request #31 from russellballestrini/claude/template-system-documentation-011CUzhHbaPXsG7W92zZ1Dci

OpenCompletion Template System Documentation

activity_yaml_validator.py

@@ -40,6 +40,47 @@ class ActivityYAMLValidator:
         self.warnings = []
         self.current_file = None
 
+        # Regex patterns for template validation
+        # Jinja2 control structures (NOT ALLOWED)
+        self.jinja2_control_pattern = re.compile(r'\{%\s*(if|for|elif|else|endif|endfor|block|endblock|macro|endmacro|set|include|extends)\s')
+        # Handlebars control structures (NOT ALLOWED)
+        self.handlebars_control_pattern = re.compile(r'\{\{#(if|each|unless|with)|\{\{/(if|each|unless|with)\}\}|\{\{else\}\}')
+        # Valid substitution patterns (ALLOWED)
+        self.valid_substitution_pattern = re.compile(r'\{\{[a-zA-Z_][a-zA-Z0-9_\.]*\}\}')
+
+    def _check_template_syntax(self, text: str, location: str):
+        """
+        Check text for invalid template control structures.
+
+        OpenCompletion uses a substitution-only template system:
+        - ALLOWED: {{variable}}, {{metadata.key}}, {{current_attempt}}
+        - NOT ALLOWED: {% if %}, {{#if}}, loops, conditionals
+
+        Args:
+            text: The text content to check
+            location: Human-readable location string for error messages
+        """
+        if not isinstance(text, str):
+            return
+
+        # Check for Jinja2 control structures
+        jinja2_match = self.jinja2_control_pattern.search(text)
+        if jinja2_match:
+            self.errors.append(
+                f"{location}: Jinja2 control structures ({{%% %}}) are NOT supported. "
+                f"Found: '{jinja2_match.group(0)}...'. "
+                f"Use 'show_if' conditions or pre-compute values in scripts instead."
+            )
+
+        # Check for Handlebars control structures
+        handlebars_match = self.handlebars_control_pattern.search(text)
+        if handlebars_match:
+            self.errors.append(
+                f"{location}: Handlebars control structures ({{{{#}}}}) are NOT supported. "
+                f"Found: '{handlebars_match.group(0)}...'. "
+                f"Use 'show_if' conditions or pre-compute values in scripts instead."
+            )
+
     def validate_file(self, file_path: str) -> Tuple[bool, List[str], List[str]]:
         """
         Validate a YAML file and return results
@@ -239,8 +280,11 @@ class ActivityYAMLValidator:
 
         for i, block in enumerate(content_blocks):
             if isinstance(block, str):
-                # Simple string block - always valid
-                continue
+                # Simple string block - check for control structures
+                self._check_template_syntax(
+                    block,
+                    f"Section {section_id}, step {step_id}: content_blocks[{i}]"
+                )
             elif isinstance(block, dict):
                 # Conditional block (v2.0)
                 if 'text' not in block:
@@ -251,6 +295,12 @@ class ActivityYAMLValidator:
                     self.errors.append(
                         f"Section {section_id}, step {step_id}: content_blocks[{i}]['text'] must be a string"
                     )
+                else:
+                    # Check text for control structures
+                    self._check_template_syntax(
+                        block['text'],
+                        f"Section {section_id}, step {step_id}: content_blocks[{i}]['text']"
+                    )
 
                 if 'show_if' in block:
                     if not isinstance(block['show_if'], dict):
@@ -266,10 +316,17 @@ class ActivityYAMLValidator:
         self, step: Dict[str, Any], section_id: str, step_id: str
     ):
         """Validate question-type step"""
-        if "question" in step and not isinstance(step["question"], str):
-            self.errors.append(
-                f"Section {section_id}, step {step_id}: 'question' must be a string"
-            )
+        if "question" in step:
+            if not isinstance(step["question"], str):
+                self.errors.append(
+                    f"Section {section_id}, step {step_id}: 'question' must be a string"
+                )
+            else:
+                # Check question for control structures
+                self._check_template_syntax(
+                    step["question"],
+                    f"Section {section_id}, step {step_id}: 'question'"
+                )
 
         # Validate AI tokens
         if "tokens_for_ai" in step:
@@ -277,12 +334,24 @@ class ActivityYAMLValidator:
                 self.errors.append(
                     f"Section {section_id}, step {step_id}: 'tokens_for_ai' must be a string"
                 )
+            else:
+                # Check tokens_for_ai for control structures
+                self._check_template_syntax(
+                    step["tokens_for_ai"],
+                    f"Section {section_id}, step {step_id}: 'tokens_for_ai'"
+                )
 
         if "feedback_tokens_for_ai" in step:
             if not isinstance(step["feedback_tokens_for_ai"], str):
                 self.errors.append(
                     f"Section {section_id}, step {step_id}: 'feedback_tokens_for_ai' must be a string"
                 )
+            else:
+                # Check feedback_tokens_for_ai for control structures
+                self._check_template_syntax(
+                    step["feedback_tokens_for_ai"],
+                    f"Section {section_id}, step {step_id}: 'feedback_tokens_for_ai'"
+                )
 
         # Validate feedback_prompts (new multi-prompt system)
         if "feedback_prompts" in step:
@@ -360,10 +429,16 @@ class ActivityYAMLValidator:
                     self.errors.append(
                         f"Section {section_id}, step {step_id}: feedback_prompts[{i}].tokens_for_ai must be a string"
                     )
-                # Check for STFU token usage (informational)
-                elif "STFU" in prompt["tokens_for_ai"]:
-                    # This is valid - STFU token is used to suppress empty feedback messages
-                    pass
+                else:
+                    # Check for control structures
+                    self._check_template_syntax(
+                        prompt["tokens_for_ai"],
+                        f"Section {section_id}, step {step_id}: feedback_prompts[{i}].tokens_for_ai"
+                    )
+                    # Check for STFU token usage (informational)
+                    if "STFU" in prompt["tokens_for_ai"]:
+                        # This is valid - STFU token is used to suppress empty feedback messages
+                        pass
 
             # Validate metadata_filter (optional)
             if "metadata_filter" in prompt:
@@ -573,12 +648,17 @@ class ActivityYAMLValidator:
                 self.errors.append(
                     f"Section {section_id}, step {step_id}, bucket {bucket}: 'ai_feedback' must be a dictionary"
                 )
-            elif "tokens_for_ai" in ai_feedback and not isinstance(
-                ai_feedback["tokens_for_ai"], str
-            ):
-                self.errors.append(
-                    f"Section {section_id}, step {step_id}, bucket {bucket}: ai_feedback.tokens_for_ai must be a string"
-                )
+            elif "tokens_for_ai" in ai_feedback:
+                if not isinstance(ai_feedback["tokens_for_ai"], str):
+                    self.errors.append(
+                        f"Section {section_id}, step {step_id}, bucket {bucket}: ai_feedback.tokens_for_ai must be a string"
+                    )
+                else:
+                    # Check ai_feedback tokens for control structures
+                    self._check_template_syntax(
+                        ai_feedback["tokens_for_ai"],
+                        f"Section {section_id}, step {step_id}, bucket {bucket}: ai_feedback.tokens_for_ai"
+                    )
 
         if "content_blocks" in transition:
             if not isinstance(transition["content_blocks"], list):
@@ -628,6 +708,12 @@ class ActivityYAMLValidator:
                 self.errors.append(
                     f"Section {section_id}, step {step_id}: hints[{i}]['text'] must be a string"
                 )
+            else:
+                # Check hint text for control structures
+                self._check_template_syntax(
+                    hint['text'],
+                    f"Section {section_id}, step {step_id}: hints[{i}]['text']"
+                )
 
             # Validate optional fields
             if 'counts_as_attempt' in hint and not isinstance(hint['counts_as_attempt'], bool):

research/SPEC.yaml

@@ -976,13 +976,12 @@ question: "Try {{current_attempt}}: What's your answer?"
 ai_feedback:
   tokens_for_ai: |
     This is attempt {{current_attempt}} of {{max_attempts}}.
-    {% if attempts_remaining == 1 %}
-      This is their last chance! Be clear and helpful.
-    {% elif attempts_remaining == 2 %}
-      They still have time. Provide a gentle hint.
-    {% else %}
-      Encourage them to think carefully.
-    {% endif %}
+    They have {{attempts_remaining}} attempts remaining.
+
+    Adjust your feedback based on the attempt number:
+    - On their last attempt: Be clear and helpful
+    - With 2 attempts left: Provide a gentle hint
+    - With more attempts: Encourage them to think carefully
 
 # Conditional content based on attempts:
 content_blocks:

tests/unit/test_activity_yaml_validator.py

@@ -813,6 +813,295 @@ sections:
             os.unlink(warning_file)
 
 
+    def test_jinja2_control_structures_rejected(self):
+        """Test that Jinja2 control structures are rejected"""
+        jinja2_control_yaml = """
+sections:
+  - section_id: "section_1"
+    title: "Test"
+    steps:
+      - step_id: "step_1"
+        title: "Test Step"
+        content_blocks:
+          - "Valid content"
+          - "{% if score > 80 %}High score{% else %}Low score{% endif %}"
+        question: "Test question {% for item in items %}{{item}}{% endfor %}"
+        tokens_for_ai: |
+          {% if attempts_remaining == 1 %}
+            Last chance
+          {% else %}
+            Keep trying
+          {% endif %}
+        buckets:
+          - test
+        transitions:
+          test:
+            content_blocks: ["Done"]
+"""
+        temp_file = self.create_temp_yaml(jinja2_control_yaml)
+        try:
+            is_valid, errors, warnings = self.validator.validate_file(temp_file)
+            self.assertFalse(is_valid)
+            # Should have multiple errors for different Jinja2 control structures
+            jinja2_errors = [e for e in errors if "Jinja2" in e]
+            self.assertGreater(len(jinja2_errors), 0)
+            # Check that error messages mention the right thing
+            self.assertTrue(any("NOT supported" in error for error in jinja2_errors))
+            self.assertTrue(any("show_if" in error or "pre-compute" in error for error in jinja2_errors))
+        finally:
+            os.unlink(temp_file)
+
+    def test_handlebars_control_structures_rejected(self):
+        """Test that Handlebars control structures are rejected"""
+        handlebars_yaml = """
+sections:
+  - section_id: "section_1"
+    title: "Test"
+    steps:
+      - step_id: "step_1"
+        title: "Test Step"
+        content_blocks:
+          - "{{#if premium}}Premium content{{else}}Free content{{/if}}"
+          - "{{#each items}}Item: {{name}}{{/each}}"
+        question: "{{#unless answered}}Please answer{{/unless}}"
+        feedback_tokens_for_ai: "{{#if correct}}Good job{{else}}Try again{{/if}}"
+        buckets:
+          - test
+        transitions:
+          test:
+            ai_feedback:
+              tokens_for_ai: "{{#with user}}Hello {{name}}{{/with}}"
+            content_blocks: ["Done"]
+"""
+        temp_file = self.create_temp_yaml(handlebars_yaml)
+        try:
+            is_valid, errors, warnings = self.validator.validate_file(temp_file)
+            self.assertFalse(is_valid)
+            # Should have multiple errors for different Handlebars control structures
+            handlebars_errors = [e for e in errors if "Handlebars" in e]
+            self.assertGreater(len(handlebars_errors), 0)
+            # Check that error messages mention the right thing
+            self.assertTrue(any("NOT supported" in error for error in handlebars_errors))
+        finally:
+            os.unlink(temp_file)
+
+    def test_valid_substitutions_allowed(self):
+        """Test that valid {{variable}} substitutions are allowed"""
+        valid_substitutions_yaml = """
+sections:
+  - section_id: "section_1"
+    title: "Test"
+    steps:
+      - step_id: "step_1"
+        title: "Test Step"
+        content_blocks:
+          - "Hello {{username}}!"
+          - "Score: {{metadata.score}}"
+          - "Attempt {{current_attempt}} of {{max_attempts}}"
+          - "You have {{attempts_remaining}} attempts left"
+        question: "Ready {{username}}? Try {{current_attempt}}"
+        tokens_for_ai: |
+          User {{username}} is on attempt {{current_attempt}}.
+          Their score is {{metadata.score}}.
+        feedback_tokens_for_ai: |
+          Provide feedback to {{username}}.
+          Reference their {{metadata.last_answer}}.
+        buckets:
+          - test
+        transitions:
+          test:
+            ai_feedback:
+              tokens_for_ai: "Great job {{username}}! Score: {{metadata.score}}"
+            content_blocks:
+              - "Well done {{username}}!"
+              - "Final score: {{metadata.score}}"
+            next_section_and_step: "section_1:step_2"
+
+      - step_id: "step_2"
+        title: "Final"
+        content_blocks:
+          - "Goodbye {{username}}!"
+"""
+        temp_file = self.create_temp_yaml(valid_substitutions_yaml)
+        try:
+            is_valid, errors, warnings = self.validator.validate_file(temp_file)
+            self.assertTrue(is_valid, f"Valid substitutions should be allowed but got errors: {errors}")
+            self.assertEqual(len(errors), 0)
+        finally:
+            os.unlink(temp_file)
+
+    def test_control_structures_in_hints(self):
+        """Test that control structures in hints are rejected"""
+        hints_with_control_yaml = """
+sections:
+  - section_id: "section_1"
+    title: "Test"
+    steps:
+      - step_id: "step_1"
+        title: "Test Step"
+        question: "What is 2+2?"
+        hints:
+          - attempt: 2
+            text: "{% if score > 50 %}Think harder{% else %}You can do it{% endif %}"
+          - attempt: 3
+            text: "{{#if last_try}}This is your last chance{{/if}}"
+        buckets:
+          - test
+        transitions:
+          test:
+            content_blocks: ["Done"]
+"""
+        temp_file = self.create_temp_yaml(hints_with_control_yaml)
+        try:
+            is_valid, errors, warnings = self.validator.validate_file(temp_file)
+            self.assertFalse(is_valid)
+            # Should catch control structures in hints
+            hint_errors = [e for e in errors if "hints" in e]
+            self.assertGreater(len(hint_errors), 0)
+        finally:
+            os.unlink(temp_file)
+
+    def test_control_structures_in_feedback_prompts(self):
+        """Test that control structures in feedback_prompts are rejected"""
+        feedback_prompts_control_yaml = """
+sections:
+  - section_id: "section_1"
+    title: "Test"
+    steps:
+      - step_id: "step_1"
+        title: "Test Step"
+        question: "Test?"
+        feedback_prompts:
+          - name: "status"
+            tokens_for_ai: "{% if health > 50 %}Healthy{% else %}Injured{% endif %}"
+          - name: "items"
+            tokens_for_ai: "{{#each inventory}}{{item}}{{/each}}"
+        buckets:
+          - test
+        transitions:
+          test:
+            content_blocks: ["Done"]
+"""
+        temp_file = self.create_temp_yaml(feedback_prompts_control_yaml)
+        try:
+            is_valid, errors, warnings = self.validator.validate_file(temp_file)
+            self.assertFalse(is_valid)
+            # Should catch control structures in feedback_prompts
+            feedback_errors = [e for e in errors if "feedback_prompts" in e]
+            self.assertGreater(len(feedback_errors), 0)
+        finally:
+            os.unlink(temp_file)
+
+    def test_control_structures_in_conditional_content_blocks(self):
+        """Test that control structures in conditional content_blocks are rejected"""
+        conditional_blocks_yaml = """
+sections:
+  - section_id: "section_1"
+    title: "Test"
+    steps:
+      - step_id: "step_1"
+        title: "Test Step"
+        content_blocks:
+          - text: "{% if score > 90 %}Excellent!{% endif %}"
+            show_if:
+              score_gte: 90
+          - text: "{{#if premium}}Premium user{{/if}}"
+            show_if:
+              premium: true
+        question: "Test?"
+        buckets:
+          - test
+        transitions:
+          test:
+            content_blocks:
+              - text: "{% for i in range(5) %}Step {{i}}{% endfor %}"
+            next_section_and_step: "section_1:step_2"
+
+      - step_id: "step_2"
+        title: "Final"
+        content_blocks:
+          - "Done"
+"""
+        temp_file = self.create_temp_yaml(conditional_blocks_yaml)
+        try:
+            is_valid, errors, warnings = self.validator.validate_file(temp_file)
+            self.assertFalse(is_valid)
+            # Should catch control structures in conditional content blocks
+            control_errors = [e for e in errors if "Jinja2" in e or "Handlebars" in e]
+            self.assertGreater(len(control_errors), 0)
+        finally:
+            os.unlink(temp_file)
+
+    def test_mixed_valid_and_invalid_templates(self):
+        """Test file with both valid substitutions and invalid control structures"""
+        mixed_yaml = """
+sections:
+  - section_id: "section_1"
+    title: "Test"
+    steps:
+      - step_id: "step_1"
+        title: "Test Step"
+        content_blocks:
+          - "Hello {{username}}!"  # VALID
+          - "Score: {{metadata.score}}"  # VALID
+          - "{% if score > 80 %}High{% else %}Low{% endif %}"  # INVALID
+        question: "Ready {{username}}?"  # VALID
+        tokens_for_ai: |
+          User {{username}} on attempt {{current_attempt}}.  # VALID
+          {% if attempts_remaining == 1 %}Last chance{% endif %}  # INVALID
+        buckets:
+          - test
+        transitions:
+          test:
+            content_blocks: ["Done"]
+"""
+        temp_file = self.create_temp_yaml(mixed_yaml)
+        try:
+            is_valid, errors, warnings = self.validator.validate_file(temp_file)
+            self.assertFalse(is_valid)
+            # Should only have errors for the control structures, not the valid substitutions
+            control_errors = [e for e in errors if "Jinja2" in e or "Handlebars" in e]
+            self.assertGreater(len(control_errors), 0)
+            # Should have exactly 2 errors (one for content_block, one for tokens_for_ai)
+            self.assertEqual(len(control_errors), 2)
+        finally:
+            os.unlink(temp_file)
+
+    def test_various_jinja2_statements(self):
+        """Test detection of various Jinja2 statement types"""
+        various_jinja2_yaml = """
+sections:
+  - section_id: "section_1"
+    title: "Test"
+    steps:
+      - step_id: "step_1"
+        title: "Test with various Jinja2"
+        content_blocks:
+          - "{% if x %}test{% endif %}"
+          - "{% for item in list %}{{item}}{% endfor %}"
+          - "{% elif condition %}branch{% endif %}"
+          - "{% else %}default{% endif %}"
+          - "{% set var = value %}"
+          - "{% block content %}test{% endblock %}"
+        question: "Test?"
+        buckets:
+          - test
+        transitions:
+          test:
+            content_blocks: ["Done"]
+"""
+        temp_file = self.create_temp_yaml(various_jinja2_yaml)
+        try:
+            is_valid, errors, warnings = self.validator.validate_file(temp_file)
+            self.assertFalse(is_valid)
+            # Should catch all the different Jinja2 statement types
+            jinja2_errors = [e for e in errors if "Jinja2" in e]
+            # Should have multiple errors for different statements
+            self.assertGreaterEqual(len(jinja2_errors), 5)
+        finally:
+            os.unlink(temp_file)
+
+
 if __name__ == "__main__":
     # Run the tests
     unittest.main(verbosity=2)