Repository: COVESA/vss-tools

Issue:
v4.2.1: Mandatory Description missing in implicitly created branches in vspec2x
I have a "broken" `.vspec` file where the nodes are not correctly ordered, e.g. a sub-branch comes before its parent branch.

That causes the vss-tools to "implicitly auto-creating branches", which seems to be a really nice feature.
However, since at the time where the branch node is implicitly created there is no description given, subsequent steps error out.

```
Expanding direct VSpec to expanded JSON.../generated/no-covesa/vspec/Vehicle/Cabin/Hvac/Fronthousing.vspec
INFO     VSS-tools version 4.2.1
INFO     Known extended attributes: dbcinfo, dbc, dbc2vss, vss2dbc
INFO     No quantities defined!
INFO     Added 52 units from /generated/no-covesa/vss_custom_units.yaml
INFO     Loading vspec from /generated/no-covesa/vspec/Vehicle/Cabin/Hvac/Fronthousing.vspec...
INFO     Autocreating implicit branch Vehicle
INFO     Autocreating implicit branch Cabin
INFO     Autocreating implicit branch Cabin
INFO     Autocreating implicit branch Hvac
ERROR    Invalid VSS element Cabin, must have description
```

Tested with v4.2.1

This code here is creating a new node, but does not set the required field `description`:
https://github.com/COVESA/vss-tools/blob/release/4.2/vspec/__init__.py#L685

Did not yet test with main branch, as I couldn't find that function.
May be obsolete.

Discussion:
@erikbosch: The implicit branches creation was intended primarily for overlays to avoid the need to add a branch that isn't changed in an overlay just for the sake of it. But it can also be used for cases where the branch does not exist since before and there we need to make up our mind, either make sure it works or create a proper error message like "Implicit branches only allowed in overlays" or so.

VSs-tools has recently been refactored so the error is likely produced elsewhere in the latest version - this is what you get currently with an implicit node.

Thanks for reporting, we should better look into it.

_Note to the community:_

_We cannot just add a default value when we create an implicit node as I then think that note will get priority when we apply an overlay. We could discuss if we should instead relieve the requirement to have a description for branches - it is maybe not that useful in all cases._

```

erik@debian6:~/vehicle_signal_specification$ vspec export yaml --vspec implicit.vspec -o vss.yaml
[12:40:56] WARNING  No 'quantity' files defined. Default not existing:                                         main.py:60
                    /home/erik/vehicle_signal_specification/quantities.yaml                                              
           WARNING  No 'unit' files defined. Default not existing:                                             main.py:66
                    /home/erik/vehicle_signal_specification/units.yaml                                                   
           INFO     VSpecs loaded, amount=1                                                                  vspec.py:122
           CRITICAL 'A.B' has 1 model error(s):                                                               main.py:238
                    [                                                                                                    
                        {                                                                                                
                            'type': 'missing',                                                                           
                            'loc': ('description',),                                                                     
                            'msg': 'Field required',                                                                     
                            'input': {'fqn': 'A.B', 'type': 'branch'}                                                    
                        }                                                                                                
                    ]                                                                                                    
erik@debian6:~/vehicle_signal_specification$ 
```
@sschleemilch: yes, I specifically did not support auto creation of branches. It is not a big deal to support it though if that is what we want. Or to be more specific: Branches are created implicitly but are not prefilled with a description and therefore do not comply to the final model. However, it makes overlays  more powerful since you do not need to define any  additional things in the overlay if the final node has the right format after applying all overlays. That is something that earlier vss-tools could not support in their design.@erikbosch: Let us discuss on Tuesday how we want to handle it, i.e. if we want to allow implicit branches also in "final model".@erikbosch: MoM:

- Ulf: I am not in favor for allowing impliit branches (without description) (except for overlays)
- Sebastian Schildt: Do not use dummy descriptions
- Erik: Please discuss, lets agree next week@erikbosch: 
Trying to list out the alternatives that I see using the example below.

1. Only allow implicit branches for overlays where the branch exist somewhere else already. In this case we should better get an explicit error "Branch A.B must be defined, implicit branches not allowed in final model.", rather than the complaint about that it has no description.
2. Allow it, but then as Sebastian suggests allow empty descriptions for implicit branches (or for all branches, they are not always that useful today)


```
A:
  type: branch
  description: Branch A.


A.B.C:
  datatype: int8
  type: sensor
  unit: km
  description: This is C
```@erikbosch: MoM:

- Ulf: Prefer alt 1 (i.e give better error)
- Decision: For now stick with alt 1, i.e. do not allow implicit branches in final model, but possibly give better error message

Solution patch:
diff --git a/src/vss_tools/model.py b/src/vss_tools/model.py
index f05142a..9216501 100644
--- a/src/vss_tools/model.py
+++ b/src/vss_tools/model.py
@@ -109,7 +109,7 @@ class VSSRaw(BaseModel):
 class VSSData(VSSRaw):
     model_config = ConfigDict(extra="allow")
     type: NodeType
-    description: str
+    description: str = ""
     comment: str | None = None
     delete: bool = False
     deprecation: str | None = None
@@ -126,6 +126,14 @@ class VSSData(VSSRaw):
         assert bool(re.match(pattern, v)), f"'{v}' is not a valid 'constUID'"
         return v
 
+    @model_validator(mode="after")
+    def ensure_description(self) -> Self:
+        """Give better explanation for empty description."""
+        assert (
+            self.description != ""
+        ), "All nodes in the final tree must have a description. Implicit branches are not allowed in final tree!"
+        return self
+
 
 class VSSDataBranch(VSSData):
     instances: Any = None


Test patch:
diff --git a/tests/vspec/test_description_error/test_description_error.py b/tests/vspec/test_description_error/test_description_error.py
index ad26040..79ea35c 100644
--- a/tests/vspec/test_description_error/test_description_error.py
+++ b/tests/vspec/test_description_error/test_description_error.py
@@ -40,5 +40,5 @@ def test_description_error(vspec_file: str, type_file: str, type_out_file: str,
     assert process.returncode != 0
     log_content = log.read_text()
     print(log_content)
-    assert "'type': 'missing'" in log_content
+    assert "'type': 'assertion_error'" in log_content
     assert "1 model error(s):" in log_content
diff --git a/tests/vspec/test_overlay/test_overlay.py b/tests/vspec/test_overlay/test_overlay.py
index 939ffda..5b86e88 100644
--- a/tests/vspec/test_overlay/test_overlay.py
+++ b/tests/vspec/test_overlay/test_overlay.py
@@ -71,5 +71,5 @@ def test_overlay_branch_error(tmp_path):
     assert process.returncode != 0
     log_content = log.read_text()
     assert "'A.AB' has 1 model error(s)" in log_content
-    assert "'type': 'missing'" in log_content
+    assert "'type': 'assertion_error'" in log_content
     assert "description" in log_content