Enhance validation for graph checks error messages #142

Aymen-Soussi-01 · 2025-07-17T10:00:04Z

Adding Attribute explanation to the graph checks in the metamodel
Extract the explanation attribute and add it to the log when we have errors for graph checks
Assert to ensure the explanation attribute is there
Adapt the RST Tests to the new log error format output

close: #115

github-actions · 2025-07-17T10:01:19Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //src:license-check

Status: ✅ Passed

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: ec4b2db3-b488-403a-bf96-7ab82718aca5
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: src
Analyzing: target //src:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //src:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //src:license-check (70 packages loaded, 9 targets configured)

Analyzing: target //src:license-check (121 packages loaded, 2363 targets configured)

Analyzing: target //src:license-check (121 packages loaded, 2363 targets configured)

INFO: Analyzed target //src:license-check (129 packages loaded, 4428 targets configured).
[2 / 11] [Prepa] Writing repo mapping manifest for @@score_dash_license_checker+//tool/formatters:dash_format_converter [for tool]
INFO: Found 1 target...
Target //src:license.check.license_check up-to-date:
  bazel-bin/src/license.check.license_check
  bazel-bin/src/license.check.license_check.jar
INFO: Elapsed time: 14.556s, Critical Path: 0.47s
INFO: 13 processes: 4 disk cache hit, 9 internal.
INFO: Build completed successfully, 13 total actions
INFO: Running command line: bazel-bin/src/license.check.license_check src/formatted.txt <args omitted>
[main] INFO Querying Eclipse Foundation for license data for 84 items.
[main] INFO Found 58 items.
[main] INFO Querying ClearlyDefined for license data for 26 items.
[main] INFO Found 26 items.
[main] INFO Vetted license information was found for all content. No further investigation is required.

github-actions · 2025-07-17T10:03:21Z

The created documentation from the pull request is available at: docu-html

MaximilianSoerenPollak · 2025-07-17T12:48:35Z

src/extensions/score_metamodel/checks/graph_checks.py

        "and": operator.and_,
        "or": operator.or_,
-        "not": operator.not_,
+        "not": lambda x: not x,


Can you expaline why this is needed?

When I was checking if there is other changes I need to make in graph_checks.py to make the tests works I found that Python’s operator.not_ actually expects a single operand, but its behavior can be a bit inconsistent with non-boolean types.
Using lambda x: not x is a safer and more explicit way to handle the logical not operation on evaluated conditions.
I’ve replaced operator.not_ with lambda x: not x for clarity and correctness.

Can you give me some examples?

'operator.not_' works only with boolean inputs. It expects a direct boolean value like True or False.
However, in our function, the evaluated condition might not be a pure boolean — it might be a string, number, or even a None, which Python treats as "falsy" or "truthy" in logical contexts.
Here are some Examples:

```' # Example 1 print(operator.not_(True)) # ✅ Works → False print(operator.not_(False)) # ✅ Works → True # Example 2 - Non-boolean print(operator.not_("status")) # ✅ Works → False print(operator.not_("")) # ✅ Works → True # BUT... # Example 3 - What if we accidentally give a list or dict? print(operator.not_({"key": "value"})) # ❌ Technically works (False), but semantically risky '''

'operator.not_ 'doesn’t do type checking, and in more complex condition trees, it's harder to ensure only boolean values are passed.
In our recursive eval_need_condition, the return value might not always be strictly boolean, leading to unclear behavior or silent logic errors.

'lambda x: not x' is safer as it ensures you're explicitly controlling the context — Python's native not works on any type and follows standard truthy/falsy semantics just like Python developers expect. It also reads more clearly when debugging or tracing the logic.

MaximilianSoerenPollak · 2025-07-17T12:48:44Z

src/extensions/score_metamodel/checks/graph_checks.py

+    if cond == "not":
+        if not isinstance(vals, list) or len(vals) != 1:
+            raise ValueError("Operator 'not' requires exactly one operand.")
+        return oper["not"](eval_need_condition(need, vals[0], log))


Same as above

For sure it's a suggestion and I can roll back the changes if needed.

MaximilianSoerenPollak · 2025-07-17T12:49:26Z

src/extensions/score_metamodel/checks/graph_checks.py

                    continue
+
                parent_ids = need[parent_relation]
+                if not isinstance(parent_ids, list):


I don't quiet understand this?

What's the pourpes of this?

Just to ensure it's a list, as we are going to loop through it, but I have removed this if statement now

Theoretically it makes sense, though if this weren't a list then we have much bigger problems, as that means the Metamodel loaded wrong.
If you truly want you can make this as an 'assert' here.

I will just keep it simple and remove the if there is no need for an assert I guess

MaximilianSoerenPollak · 2025-07-17T12:50:23Z

src/extensions/score_metamodel/metamodel.yaml

      condition: safety == QM
    check:
      satisfies: safety == QM
+    explanation: An ASIL requirement must link at least one parent/upstream ASIL requirement for correct decomposition. Please ensure the parent’s safety level is QM and its status is valid.


Status valid is not checked here.
Though it will be in the future as far as I know. So I think we should keep this Explanation for now

MaximilianSoerenPollak · 2025-07-17T12:50:57Z

src/extensions/score_metamodel/metamodel.yaml

      condition: safety == ASIL_B
    check:
      satisfies: safety != ASIL_D
+    explanation: An ASIL requirement must link at least one parent/upstream ASIL requirement for correct decomposition. Please ensure the parent’s safety level is not ASIL_D and its status is valid.


Missing that this only applies when current requirement is ASIL_B, not whne it's QM

MaximilianSoerenPollak · 2025-07-17T12:52:50Z

src/extensions/score_metamodel/metamodel.yaml

      condition: safety == ASIL_B
    check:
      mitigates: safety != QM
+    explanation: An ASIL requirement must link at least one parent/upstream ASIL requirement for correct decomposition. Please ensure the parent’s safety level is not QM and its status is valid.


Doesn't make sense.

Suggested change

explanation: An ASIL requirement must link at least one parent/upstream ASIL requirement for correct decomposition. Please ensure the parent’s safety level is not QM and its status is valid.

explanation: An ASIL_B safety requirement must link to a ASIL_B requirement. Please ensure that the linked requirements safety level is not QM and it's status is valid.

but here also we it can be equal or higher so must link to ASIL_B or ASIL_D

ASIL_D is no longer available to be choosen.

MaximilianSoerenPollak

Some questions. See Comments

Edit: Also, formatting missing.

MaximilianSoerenPollak

Some comments

AlexanderLanin · 2025-07-29T13:57:25Z

@Aymen-Soussi-01 can you resolve the conflicts?

It seems we can drop req_safety_linkage_asil_b as ASIL_D is now gone
In the other conflict change mitigates to mitigated_by
Update tests etc for those two changes

…ion description

Aymen-Soussi-01 requested review from AlexanderLanin, MaximilianSoerenPollak and dcalavrezo-qorix as code owners July 17, 2025 10:00

github-project-automation bot added this to Infrastructure Jul 17, 2025

github-project-automation bot moved this to Draft in Infrastructure Jul 17, 2025

Aymen-Soussi-01 requested a review from nicu1989 July 17, 2025 10:00

Aymen-Soussi-01 self-assigned this Jul 17, 2025

MaximilianSoerenPollak reviewed Jul 17, 2025

View reviewed changes

MaximilianSoerenPollak reviewed Jul 18, 2025

View reviewed changes

Aymen-Soussi-01 added 4 commits July 30, 2025 15:21

add explanation to graph checks

01261b0

Correct tests and adapt them to checks

4a4062a

Correct ruff format errors and rollback the function eval_need_condit…

b500140

…ion description

Fix review comments

1c4f656

Aymen-Soussi-01 force-pushed the enhance-validation-for-graph-checks-error-messages branch from 9e60df9 to 1c4f656 Compare July 30, 2025 13:27

Fix review comment Explanation message

8b843f9

AlexanderLanin approved these changes Jul 30, 2025

View reviewed changes

AlexanderLanin merged commit 2e6cf97 into eclipse-score:main Jul 30, 2025
7 checks passed

github-project-automation bot moved this from Draft to Done in Infrastructure Jul 30, 2025

	explanation: An ASIL requirement must link at least one parent/upstream ASIL requirement for correct decomposition. Please ensure the parent’s safety level is not QM and its status is valid.
	explanation: An ASIL_B safety requirement must link to a ASIL_B requirement. Please ensure that the linked requirements safety level is not QM and it's status is valid.

Enhance validation for graph checks error messages #142

Enhance validation for graph checks error messages #142

Uh oh!

Conversation

Aymen-Soussi-01 commented Jul 17, 2025

Uh oh!

github-actions bot commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions bot commented Jul 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaximilianSoerenPollak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaximilianSoerenPollak left a comment

Choose a reason for hiding this comment

Uh oh!

AlexanderLanin commented Jul 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Jul 17, 2025 •

edited

Loading

MaximilianSoerenPollak left a comment •

edited

Loading