[Feature] Extract static analysis tests and add multi-template support (#305)#306
Merged
ArthurCRodrigues merged 3 commits intomainfrom May 7, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the autograder to support loading multiple grading templates per assignment, and extracts static-analysis-only checks into a dedicated static_analysis template so they can be combined with execution-based templates like input_output.
Changes:
- Added a new
static_analysistemplate and migratedforbidden_import/forbidden_keywordout ofinput_output. - Updated pipeline steps and services (
TemplateLoaderStep,PipelineExecution,CriteriaTreeService,SandboxStep,GradeStep,BuildTreeStep) to work with multiple loaded templates. - Updated i18n translation files and unit tests to reflect the new template structure.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_test_function_validation.py | Updates CriteriaTreeService usage to pass templates as a list. |
| tests/unit/test_forbidden_keyword.py | Switches registration test to StaticAnalysisTemplate. |
| tests/unit/test_forbidden_import.py | Switches registration test to StaticAnalysisTemplate. |
| tests/unit/test_command_resolution_at_build_time.py | Updates imports to pull ForbiddenImportTest from static_analysis. |
| autograder/translations/pt_br.json | Moves forbidden_import strings under static_analysis and updates io template block. |
| autograder/translations/en.json | Moves forbidden_import strings under static_analysis and updates io template block. |
| autograder/template_library/static_analysis.py | New template implementing forbidden_import + forbidden_keyword. |
| autograder/template_library/input_output.py | Removes static analysis tests from InputOutputTemplate. |
| autograder/steps/structural_analysis_step.py | Changes missing ast-grep-py behavior from pipeline-fail to “skip”. |
| autograder/steps/sandbox_step.py | Skips sandbox creation unless any loaded template requires it. |
| autograder/steps/load_template_step.py | Loads multiple templates from list or comma-separated string. |
| autograder/steps/grade_step.py | Requires sandbox only if any loaded template needs it. |
| autograder/steps/build_tree_step.py | Builds criteria tree from multiple templates’ test registries. |
| autograder/services/template_library_service.py | Registers static_analysis as a built-in template. |
| autograder/services/criteria_tree_service.py | Looks up test implementations across all loaded templates. |
| autograder/models/pipeline_execution.py | Adds get_loaded_templates() and keeps get_loaded_template() for compatibility. |
| autograder/autograder.py | Updates build_pipeline docstring to reflect list/string templates. |
| @@ -1,4 +1,5 @@ | |||
| import logging | |||
| from typing import Union, List, Optional | |||
Comment on lines
+22
to
+24
| self._template_names = [name.strip() for name in template_name.split(",")] | ||
| else: | ||
| self._template_names = template_name |
Comment on lines
+86
to
+87
| for tmpl in templates: | ||
| raw = tmpl.replace('{{lib}}', library).replace('{lib}', re.escape(library)) |
Comment on lines
+31
to
+37
| logger.error("ast-grep-py is not installed; structural analysis will be skipped.") | ||
| return pipeline_exec.add_step_result(StepResult.fail(self.step_name, "ast-grep-py not installed")) | ||
| logger.warning("ast-grep-py is not installed; skipping structural analysis.") | ||
| return pipeline_exec.add_step_result(StepResult.success(self.step_name, StructuralAnalysisResult(roots={}))) |
Comment on lines
+237
to
+285
| if not forbidden_keywords and not custom_ast_grep_rules: | ||
| return TestResult( | ||
| test_name=self.name, | ||
| score=100.0, | ||
| report=t("static_analysis.forbidden_keyword.report.no_rules", locale=locale) | ||
| ) | ||
|
|
||
| if structural_analysis is None: | ||
| return TestResult( | ||
| test_name=self.name, | ||
| score=0.0, | ||
| report=t("static_analysis.forbidden_keyword.report.no_analysis", locale=locale) | ||
| ) | ||
|
|
||
| if submission_language is None: | ||
| return TestResult( | ||
| test_name=self.name, | ||
| score=0.0, | ||
| report=t("static_analysis.forbidden_keyword.report.no_lang", locale=locale) | ||
| ) | ||
|
|
||
| active_rules: List[Dict[str, Any]] = list(custom_ast_grep_rules) | ||
| lang_predefined = self.PREDEFINED_RULES.get(submission_language, {}) | ||
|
|
||
| for kw in forbidden_keywords: | ||
| if kw in lang_predefined: | ||
| active_rules.append(lang_predefined[kw]) | ||
|
|
||
| if not active_rules: | ||
| return TestResult( | ||
| test_name=self.name, | ||
| score=100.0, | ||
| report=t("static_analysis.forbidden_keyword.report.success", locale=locale) | ||
| ) | ||
|
|
||
| all_violations: List[str] = [] | ||
|
|
||
| if not files: | ||
| return TestResult( | ||
| test_name=self.name, | ||
| score=100.0, | ||
| report=t("static_analysis.forbidden_keyword.report.no_files", locale=locale) | ||
| ) | ||
|
|
||
| for sub_file in files: | ||
| root = structural_analysis.roots.get(sub_file.filename) | ||
| if root is None: | ||
| continue | ||
|
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- prevent duplicate submission_language kwargs in grader with runtime precedence - normalize and validate multi-template names in TemplateLoaderStep - remove stale CriteriaTreeService template state - add explicit structural-analysis availability semantics - harden ForbiddenKeywordTest when analysis roots are unavailable - fix ForbiddenImportTest regex escaping for placeholder variants - add regression tests for collision path, template normalization, regex escaping, and analysis-unavailable cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Implemented the remaining fixes for #305 and pushed commit Fixed
Added regression coverage
Validation run
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
forbidden_importandforbidden_keywordtests to a newstatic_analysistemplateInputOutputTemplateto remove static analysis testsbuild_pipelineandTemplateLoaderStepto accept a list of template names or a comma-separated stringCriteriaTreeServiceto lookup test implementations across all loaded templatesSandboxStepandGradeStepto initialize the sandbox only if at least one loaded template requires itStructuralAnalysisStepifast-grep-pyis missing instead of failing the pipelineValidation
Closes #305