Add template control structure validation to YAML validator
Enhance the activity YAML validator to detect and reject Jinja2 and
Handlebars control structures, enforcing the substitution-only template
system design.
Changes:
- Add regex patterns for Jinja2 ({% %}) and Handlebars ({{# }})
- Add _check_template_syntax() method
- Integrate checks in content_blocks, questions, tokens_for_ai, hints
- Add 7 comprehensive unit tests for template validation
- All 59 activity YAMLs + SPEC.yaml pass validation (0 errors)
This commit is contained in:
Claude
parent
b45712f503
commit
6ce46fc5a0
2 changed files with 391 additions and 16 deletions
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue