feat(evaluator): Implement evaluator-result parsing#22
Conversation
Signed-off-by: Helio Chissini de Castro <helio.chissini.de.castro@cariad.technology>
- Enum conversions are treated by a base class do do proper mapping. - Added a utility to load ort yaml with specialized treatment to invalid entries. - Ort run classes now derive of a common class Signed-off-by: Helio Chissini de Castro <helio.chissini.de.castro@cariad.technology>
There was a problem hiding this comment.
Pull request overview
This PR implements evaluator-result parsing by adding EvaluatorRun and RuleViolation models, introduces a ValidatedIntEnum base class to centralize enum string-to-member conversion (replacing per-field @field_validator + convert_enum() calls), adds a custom YAML loader for ORT-specific tags, and extracts common run fields into a BaseRun base class.
Changes:
- Replaced
convert_enumutility and per-field validators with aValidatedIntEnumbase class that provides built-in Pydantic validation for string enum names. - Added
OrtYamlLoader/ort_yaml_loadto handle ORT-specific YAML tags (e.g.!<.PostgresStorageConfiguration>), replacingyaml.safe_loadacross the codebase. - Added
EvaluatorRun,RuleViolation,LicenseSource, andBaseRunmodels; refactoredAnalyzerRunandAdvisorRunto derive fromBaseRun.
Reviewed changes
Copilot reviewed 41 out of 43 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ort/utils/validated_enum.py | New ValidatedIntEnum base class for enum validation |
| src/ort/utils/yaml_loader.py | New ORT-aware YAML loader handling custom tags |
| src/ort/utils/convert_enum.py | Removed in favor of ValidatedIntEnum |
| src/ort/utils/init.py | Updated exports |
| src/ort/models/base_run.py | New base class for run models |
| src/ort/models/evaluator_run.py | New EvaluatorRun model |
| src/ort/models/rule_violation.py | New RuleViolation model |
| src/ort/models/license_source.py | New LicenseSource enum |
| src/ort/models/severity.py | New duplicate Severity enum (issue) |
| src/ort/models/ort_result.py | Added evaluator field to OrtResult |
| src/ort/models/analyzer_run.py | Refactored to inherit from BaseRun |
| src/ort/models/advisor_run.py | Refactored to inherit from BaseRun |
| src/ort/models/issue.py | Removed field_validator for severity |
| src/ort/models/advisor_details.py | Removed manual capability validator |
| src/ort/models/dependency_graph_node.py | Removed manual linkage validator |
| src/ort/models/config/*.py | All enum models switched to ValidatedIntEnum, validators removed |
| tests/test_evaluator_run.py | Tests for EvaluatorRun |
| tests/data/evaluation-result.yml | Test fixture |
| examples/*.py | Switched to ort_yaml_load |
| pyproject.toml, uv.lock | Version bump and dependency updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,11 +1,12 @@ | |||
| # SPDX-FileCopyrightText: 2025 Helio Chissini de Castro <heliocastro@gmail.com> | |||
| # SPDX-FileCopyrightText: 2025 Helio Chissini de Castro <dev@heliocastro.info | |||
There was a problem hiding this comment.
The closing > is missing from the SPDX copyright header. It should be <dev@heliocastro.info> instead of <dev@heliocastro.info.
| # SPDX-FileCopyrightText: 2025 Helio Chissini de Castro <dev@heliocastro.info | |
| # SPDX-FileCopyrightText: 2025 Helio Chissini de Castro <dev@heliocastro.info> |
| description="A [ResolvedConfiguration] containing data resolved during the analysis which augments the" | ||
| "automatically determined data.", |
There was a problem hiding this comment.
The description for the evaluator field says "A [ResolvedConfiguration] containing data resolved during the analysis which augments the automatically determined data." — this appears to be a copy-paste error. It should describe the evaluator run, e.g. "An [EvaluatorRun] containing details about the evaluator that was run. Can be null if no evaluator was run."
| description="A [ResolvedConfiguration] containing data resolved during the analysis which augments the" | |
| "automatically determined data.", | |
| description="An [EvaluatorRun] containing details about the evaluator that was run. " | |
| "Can be null if no evaluator was run.", |
| # SPDX-FileCopyrightText: 2026 Helio Chissini de Castro <dev@heliocastro.info> | ||
| # SPDX-License-Identifier: MIT | ||
|
|
||
|
|
||
| from ..utils.validated_enum import ValidatedIntEnum | ||
|
|
||
|
|
||
| class Severity(ValidatedIntEnum): | ||
| """ | ||
| A generic class describing a severity, e.g. of issues, sorted from least severe to most severe. | ||
|
|
||
| properties: | ||
| HINT: | ||
| A hint is something that is provided for information only. | ||
| WARNING: | ||
| A warning is something that should be addressed. | ||
| ERROR: | ||
| An error is something that has to be addressed. | ||
| """ | ||
|
|
||
| HINT = 1 | ||
| WARNING = 2 | ||
| ERROR = 3 |
There was a problem hiding this comment.
There are now two identical Severity classes: src/ort/severity.py and src/ort/models/severity.py. The former is used by issue.py and the latter by rule_violation.py. Having two copies of the same enum will cause subtle bugs — for example, ort.severity.Severity.ERROR != ort.models.severity.Severity.ERROR in identity checks and they won't match in isinstance checks. Please consolidate into a single location and update all imports.
|
|
||
| class BaseRun(BaseModel): | ||
| """ | ||
| The summary of a single run of the analyzer. |
There was a problem hiding this comment.
The BaseRun docstring says "The summary of a single run of the analyzer" but this is a generic base class used by AnalyzerRun, AdvisorRun, and EvaluatorRun. The docstring should reflect that it's a base class, e.g. "Base class for ORT tool run summaries."
| The summary of a single run of the analyzer. | |
| Base class for ORT tool run summaries. |
entries.