From 345d3f05601505dfd4005567f3f725ea20a6155d Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Thu, 5 Feb 2026 14:44:47 +0100 Subject: [PATCH 01/18] feat: Enhance enum value handling: sanitize values for GraphQL compliance and annotate modified values with metadata Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- docs/s2dm.md | 21 +++ .../predefined_elements/directives.graphql | 2 +- src/vss_tools/exporters/s2dm/type_builders.py | 72 +++++++- .../utils/graphql_directive_processor.py | 45 ++++- tests/test_s2dm_exporter.py | 171 +++++++++++++++++- .../test_s2dm/test_camelcase_enums.vspec | 36 ++++ .../test_s2dm/test_enum_sanitization.vspec | 38 ++++ 7 files changed, 365 insertions(+), 20 deletions(-) create mode 100644 tests/vspec/test_s2dm/test_camelcase_enums.vspec create mode 100644 tests/vspec/test_s2dm/test_enum_sanitization.vspec diff --git a/docs/s2dm.md b/docs/s2dm.md index 22b30b24..3add9fc7 100644 --- a/docs/s2dm.md +++ b/docs/s2dm.md @@ -44,6 +44,8 @@ type Vehicle_Cabin @vspec(element: BRANCH, fqn: "Vehicle.Cabin") { In this example, the metadata shows that the `Vehicle_Cabin` type was derived from the Fully-Qualified Name (FQN) `Vehicle.Cabin`, and that is was a `BRANCH`. Likewise, the `driverPosition` was derived from `Vehicle.Cabin.DriverPosition` and it was an `ATTRIBUTE`. +The `@vspec` directive is also used to annotate original names when they have been modified for GraphQL compliance (for example, see [Enum Value Sanitization](#enum-value-sanitization)). + ### VSS Data Types Support The exporter handles all `vspec` data types as follows: - **Strings** → GraphQL String @@ -52,6 +54,25 @@ The exporter handles all `vspec` data types as follows: - **Arrays** → GraphQL Lists - **Allowed values** → GraphQL Enums +#### Enum Value Sanitization + +GraphQL enum values must follow strict naming rules (alphanumeric + underscore only, cannot start with a digit). The S2DM exporter automatically sanitizes VSS enum values to comply with GraphQL requirements: + +- **Spaces & special characters** → Converted to underscores (`"some value"` → `SOME_VALUE`) +- **CamelCase** → Converted to SCREAMING_SNAKE_CASE (`"HTTPSProtocol"` → `HTTPS_PROTOCOL`) +- **Leading digits** → Prefixed with underscore (`"123abc"` → `_123ABC`) + +When enum values are modified, the original VSS value is preserved using `@vspec` metadata: + +```graphql +enum Vehicle_Connection_Protocol_Enum @vspec(element: SENSOR, fqn: "Vehicle.Connection.Protocol", metadata: [{key: "allowed", value: "['HTTPSProtocol', 'TCPProtocol']"}]) { + HTTPS_PROTOCOL @vspec(metadata: [{key: "originalName", value: "HTTPSProtocol"}]) + TCP_PROTOCOL @vspec(metadata: [{key: "originalName", value: "TCPProtocol"}]) +} +``` + +This ensures complete traceability between the VSS source and the generated GraphQL schema. + ### VSS Instances Become GraphQL Structures When your `vspec` has instances (like multiple seats), the exporter creates proper GraphQL types: diff --git a/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql b/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql index ebcdb866..bfd20e36 100644 --- a/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql +++ b/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql @@ -1,7 +1,7 @@ """Annotation directive for Vspec mapping information.""" directive @vspec( """The element of the Vspec language to which the item in the GraphQL schema maps.""" - element: VspecElement!, + element: VspecElement, """The Fully Qualified Name (FQN) of the related element in Vspec (aka., path). Example: Vehicle.Cabin.Door.Window.position""" fqn: String """Additional metadata associated with the mapping.""" diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index f7111654..9f178695 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -18,8 +18,10 @@ from __future__ import annotations +import re from typing import Any +import caseconverter import pandas as pd from graphql import ( GraphQLArgument, @@ -171,29 +173,85 @@ def create_allowed_enums( for fqn, row in leaves_df[leaves_df["allowed"].notna()].iterrows(): if allowed := row.get("allowed"): enum_name = f"{convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS)}_Enum" - values = {_clean_enum_name(str(v)): GraphQLEnumValue(v) for v in allowed} + + # Track values and their modifications + values = {} + modified_values = {} + + for v in allowed: + sanitized, was_modified = _sanitize_enum_value_for_graphql(str(v)) + values[sanitized] = GraphQLEnumValue(v) + + # Track if value was modified for metadata annotation + if was_modified: + modified_values[sanitized] = str(v) + enums[enum_name] = GraphQLEnumType(enum_name, values, description=f"Allowed values for {fqn}.") vss_type = row.get("type", "").upper() if vss_type not in VSS_LEAF_TYPES: vss_type = "ATTRIBUTE" - allowed_values_graphql = {_clean_enum_name(str(v)): str(v).replace('"', "'") for v in allowed} + allowed_values_graphql = { + _sanitize_enum_value_for_graphql(str(v))[0]: str(v).replace('"', "'") for v in allowed + } metadata[enum_name] = { "fqn": fqn, "vss_type": vss_type, "allowed_values": allowed_values_graphql, + "modified_values": modified_values, # Store modified values for directive annotations } return enums, metadata -def _clean_enum_name(value: str) -> str: - """Sanitize enum value names for GraphQL.""" - if value[0].isdigit(): - value = f"_{value}" - return value.replace(".", "_DOT_").replace("-", "_DASH_") +def _sanitize_enum_value_for_graphql(original_value: str) -> tuple[str, bool]: + """ + Sanitize enum value for GraphQL schema compliance. + + Converts values with spaces, camelCase, or other invalid characters to valid GraphQL enum values. + Uses caseconverter to properly handle camelCase word boundaries. + + Examples: + "some value" -> "SOME_VALUE" + "SOME VALUE" -> "SOME_VALUE" + "PbCa" -> "PB_CA" + "HTTPSConnection" -> "HTTPS_CONNECTION" + "value-with-dash" -> "VALUE_WITH_DASH" + + Args: + original_value: The original enum value from VSS + + Returns: + Tuple of (sanitized_value, was_modified) + - sanitized_value: Valid GraphQL enum value name + - was_modified: True if the value was changed, False otherwise + """ + + # Handle empty or whitespace-only strings + if not original_value or not original_value.strip(): + raise ValueError(f"Cannot create GraphQL enum value from empty or whitespace-only string: {original_value!r}") + + # Replace with underscore all the special characters that are not allowed in GraphQL enum names + sanitized = re.sub(r"[^a-zA-Z0-9_]", "_", original_value) + + # Convert to caseconverter MACRO_CASE (i.e., SCREAMING_SNAKE_CASE) + if re.search(r"[A-Z]{2,}", sanitized) and re.search(r"[a-z]", sanitized): + words: list[str] = [] + for segment in sanitized.split("_"): + words.extend(re.findall(r"[A-Z]+(?![a-z])|[A-Z]?[a-z]+|[0-9]+", segment)) + sanitized = "_".join(caseconverter.macrocase(word, strip_punctuation=False) for word in words if word) + else: + sanitized = caseconverter.macrocase(sanitized, strip_punctuation=False) + + # Handle enum values starting with digits + sanitized = f"_{sanitized}" if sanitized[0].isdigit() else sanitized + + # Check if modification occurred + was_modified = sanitized != original_value + + return sanitized, was_modified def create_struct_types( diff --git a/src/vss_tools/utils/graphql_directive_processor.py b/src/vss_tools/utils/graphql_directive_processor.py index 053d536d..d7904cc9 100644 --- a/src/vss_tools/utils/graphql_directive_processor.py +++ b/src/vss_tools/utils/graphql_directive_processor.py @@ -117,28 +117,53 @@ def _process_allowed_enum_directives( Process allowed value enum directives. Annotates the enum type itself with @vspec(element, fqn, metadata), - but does NOT annotate individual enum values. + and annotates individual enum values that were modified with @vspec(metadata). """ for enum_name, enum_data in allowed_enums_metadata.items(): fqn = enum_data.get("fqn", "") vss_type = enum_data.get("vss_type", "ATTRIBUTE") allowed_values_dict = enum_data.get("allowed_values", {}) + modified_values = enum_data.get("modified_values", {}) # Build the allowed values list for metadata # GraphQL requires: value: "['val1', 'val2']" (double quotes outside, single quotes inside) allowed_values_list = list(allowed_values_dict.values()) allowed_str = ", ".join([f"'{v}'" for v in allowed_values_list]) + in_target_enum = False for i, line in enumerate(lines): - if line.strip().startswith(f"enum {enum_name}") and "@vspec" not in line: - # Annotate the enum type (not individual values) - # Format: @vspec(element: ATTRIBUTE, fqn: "...", metadata: [{key: "allowed", value: "[...]"}]) - directive = ( - f'@vspec(element: {vss_type}, fqn: "{fqn}", ' - f'metadata: [{{key: "allowed", value: "[{allowed_str}]"}}])' - ) - lines[i] = line.replace(" {", f" {directive} {{") - break + if line.strip().startswith(f"enum {enum_name}"): + if "@vspec" not in line: + # Annotate the enum type + directive = ( + f'@vspec(element: {vss_type}, fqn: "{fqn}", ' + f'metadata: [{{key: "allowed", value: "[{allowed_str}]"}}])' + ) + lines[i] = line.replace(" {", f" {directive} {{") + in_target_enum = True + continue + elif line.strip().startswith("enum ") and in_target_enum: + in_target_enum = False + continue + elif line.strip() == "}" and in_target_enum: + in_target_enum = False + continue + + # Process individual enum values that were modified + if in_target_enum and line.strip() and not line.strip().startswith('"'): + stripped_line = line.strip() + + for enum_value_name, original_value in modified_values.items(): + enum_value_key = f"{enum_name}.{enum_value_name}" + if stripped_line.startswith(enum_value_name) and enum_value_key not in processed_values: + if "@vspec" not in line: + indent = line[: len(line) - len(line.lstrip())] + # Annotate modified enum value with original value in metadata + directive = f'@vspec(metadata: [{{key: "originalName", value: "{original_value}"}}])' + lines[i] = f"{indent}{enum_value_name} {directive}" + + processed_values.add(enum_value_key) + break return lines diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index 62cc3ea5..bb39a705 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -8,6 +8,7 @@ from pathlib import Path +import pytest from graphql import build_schema, print_schema from vss_tools.exporters.s2dm import ( S2DM_CONVERSIONS, @@ -347,8 +348,8 @@ def test_allowed_value_enums_generation(self): # Check for float field with allowed values [1.0, 2.5, 4.0, 5.0] assert "Vehicle_Performance_Rating_Enum" in schema_sdl - assert "_1" in schema_sdl # 1.0 becomes _1 - assert "_2_DOT_5" in schema_sdl # 2.5 should use _DOT_ + assert "_1_0" in schema_sdl # 1.0 becomes _1_0 + assert "_2_5" in schema_sdl # 2.5 becomes _2_5 assert "_4" in schema_sdl assert "_5" in schema_sdl @@ -500,3 +501,169 @@ def test_non_instantiated_property_hoisting(self, tmp_path: Path): assert 'metadata: [{key: "instantiate", value: "false"}]' in cabin_type_content # And door_s array field should also be there assert "door_s" in cabin_type_content + + def test_enum_value_sanitization_with_spaces(self): + """Test that enum values with spaces are properly sanitized and annotated.""" + from vss_tools.exporters.s2dm.type_builders import _sanitize_enum_value_for_graphql + + # Test the sanitization function + assert _sanitize_enum_value_for_graphql("some value") == ("SOME_VALUE", True) + assert _sanitize_enum_value_for_graphql("SOME VALUE") == ("SOME_VALUE", True) + assert _sanitize_enum_value_for_graphql("another-value") == ("ANOTHER_VALUE", True) + assert _sanitize_enum_value_for_graphql("YET_ANOTHER") == ("YET_ANOTHER", False) + assert _sanitize_enum_value_for_graphql("front left") == ("FRONT_LEFT", True) + assert _sanitize_enum_value_for_graphql("1value") == ("_1VALUE", True) + + def test_enum_sanitization_in_schema_generation(self): + """Test that enum values with spaces generate proper schema with metadata.""" + # Load the test vspec with spaces in enum values + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/test_enum_sanitization.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + schema, _, allowed_metadata, _ = generate_s2dm_schema(tree) + + # Check that schema is valid + assert schema is not None + + # Check that enum type for LightMode exists + light_mode_enum_name = "Vehicle_Cabin_LightMode_Enum" + assert light_mode_enum_name in schema.type_map + + # Check metadata includes modified values + assert light_mode_enum_name in allowed_metadata + metadata = allowed_metadata[light_mode_enum_name] + assert "modified_values" in metadata + + # Verify specific modifications + modified = metadata["modified_values"] + assert "SOME_VALUE" in modified + assert modified["SOME_VALUE"] in ["some value", "SOME VALUE"] # One of them + assert "ANOTHER_VALUE" in modified + assert modified["ANOTHER_VALUE"] == "another-value" + + # YET_ANOTHER should not be in modified (no modification needed) + assert "YET_ANOTHER" not in modified + + def test_enum_sanitization_schema_output_with_directives(self): + """Test that the schema output includes proper @vspec directives for modified enum values.""" + # Load the test vspec with spaces in enum values + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/test_enum_sanitization.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree) + schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) + + # Check that enum type has @vspec directive with element + assert "enum Vehicle_Cabin_LightMode_Enum @vspec" in schema_str + + # Check that modified enum values have @vspec directives with originalName metadata + # "some value" -> SOME_VALUE + assert ( + 'SOME_VALUE @vspec(metadata: [{key: "originalName", value: "some value"}])' in schema_str + or 'SOME_VALUE @vspec(metadata: [{key: "originalName", value: "SOME VALUE"}])' in schema_str + ) + + # "another-value" -> ANOTHER_VALUE + assert 'ANOTHER_VALUE @vspec(metadata: [{key: "originalName", value: "another-value"}])' in schema_str + + # YET_ANOTHER should not have originalName metadata (wasn't modified) + assert 'YET_ANOTHER @vspec(metadata: [{key: "originalName"' not in schema_str + + # Check SeatPosition enum + assert "enum Vehicle_Cabin_SeatPosition_Enum @vspec" in schema_str + assert 'FRONT_LEFT @vspec(metadata: [{key: "originalName", value: "front left"}])' in schema_str + assert 'FRONT_RIGHT @vspec(metadata: [{key: "originalName", value: "front right"}])' in schema_str + + def test_enum_camelcase_sanitization(self): + """Test that camelCase enum values are properly converted using caseconverter.""" + from vss_tools.exporters.s2dm.type_builders import _sanitize_enum_value_for_graphql + + # Test camelCase word boundary detection + test_cases = [ + ("PbCa", "PB_CA", True), # Mixed case acronyms + ("HTTPSConnection", "HTTPS_CONNECTION", True), # Acronym + word + ("someAPIKey", "SOME_API_KEY", True), # word + acronym + word + ("XMLParser", "XML_PARSER", True), # Acronym + word + ("myValue", "MY_VALUE", True), # Simple camelCase + ("IOError", "IO_ERROR", True), # Two-letter acronym + ("AGM", "AGM", False), # Already uppercase, no change + ("EFB", "EFB", False), # Already uppercase, no change + ("already_snake", "ALREADY_SNAKE", True), # snake_case to SCREAMING_SNAKE_CASE + ("ALREADY_SCREAMING", "ALREADY_SCREAMING", False), # No change needed + ("mixed-Case", "MIXED_CASE", True), # Mixed with hyphen + ("some value", "SOME_VALUE", True), # Spaces + ("value.with.dots", "VALUE_WITH_DOTS", True), # Dots + ("123value", "_123VALUE", True), # Starts with number + ] + + error_cases = [ + "", # Empty string + " ", # Whitespace only + ] + + for input_val, expected_output, expected_modified in test_cases: + result, was_modified = _sanitize_enum_value_for_graphql(input_val) + assert result == expected_output, f"Failed for '{input_val}': expected '{expected_output}', got '{result}'" + assert was_modified == expected_modified, f"Failed modification flag for '{input_val}'" + + for input_val in error_cases: + with pytest.raises(ValueError): + _sanitize_enum_value_for_graphql(input_val) + + def test_camelcase_enums_schema_generation(self): + """Test that camelCase enum values in vspec generate proper schema with directives.""" + # Load the test vspec with camelCase enum values + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/test_camelcase_enums.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree) + schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) + + # Check Component.Type enum with AbCd + assert "enum Vehicle_Component_Type_Enum @vspec" in schema_str + + # AbCd should be converted to AB_CD with originalName annotation + assert 'AB_CD @vspec(metadata: [{key: "originalName", value: "AbCd"}])' in schema_str + + # AAA, BBB, CCC, DDD should not have originalName (no change needed) + assert 'AAA @vspec(metadata: [{key: "originalName"' not in schema_str + assert 'BBB @vspec(metadata: [{key: "originalName"' not in schema_str + + # Check Connection.Protocol enum + assert "enum Vehicle_Connection_Protocol_Enum @vspec" in schema_str + assert 'HTTPS_PROTOCOL @vspec(metadata: [{key: "originalName", value: "HTTPSProtocol"}])' in schema_str + assert 'TCP_PROTOCOL @vspec(metadata: [{key: "originalName", value: "TCPProtocol"}])' in schema_str + assert 'UDP_PROTOCOL @vspec(metadata: [{key: "originalName", value: "UDPProtocol"}])' in schema_str + + # Check Status.Code enum + assert "enum Vehicle_Status_Code_Enum @vspec" in schema_str + assert 'IO_ERROR @vspec(metadata: [{key: "originalName", value: "IOError"}])' in schema_str + assert 'XML_PARSER @vspec(metadata: [{key: "originalName", value: "XMLParser"}])' in schema_str + assert 'SOME_API_KEY @vspec(metadata: [{key: "originalName", value: "someAPIKey"}])' in schema_str diff --git a/tests/vspec/test_s2dm/test_camelcase_enums.vspec b/tests/vspec/test_s2dm/test_camelcase_enums.vspec new file mode 100644 index 00000000..7426a9e2 --- /dev/null +++ b/tests/vspec/test_s2dm/test_camelcase_enums.vspec @@ -0,0 +1,36 @@ +# +# S2DM test file for camelCase enum value sanitization +# +Vehicle: + type: branch + description: High-level vehicle data. + +Vehicle.Component: + type: branch + description: Component related signals. + +Vehicle.Component.Type: + datatype: string + type: attribute + allowed: ['AAA', 'BBB', 'CCC', 'DDD', 'AbCd'] + description: Component type with mixed case values including AbCd. + +Vehicle.Connection: + type: branch + description: Connection related signals. + +Vehicle.Connection.Protocol: + datatype: string + type: sensor + allowed: ['HTTPSProtocol', 'TCPProtocol', 'UDPProtocol'] + description: Connection protocol with camelCase values. + +Vehicle.Status: + type: branch + description: Status related signals. + +Vehicle.Status.Code: + datatype: string + type: attribute + allowed: ['IOError', 'XMLParser', 'someAPIKey'] + description: Status codes with various camelCase patterns. diff --git a/tests/vspec/test_s2dm/test_enum_sanitization.vspec b/tests/vspec/test_s2dm/test_enum_sanitization.vspec new file mode 100644 index 00000000..349fc23e --- /dev/null +++ b/tests/vspec/test_s2dm/test_enum_sanitization.vspec @@ -0,0 +1,38 @@ +# +# S2DM test file for enum value sanitization with spaces +# +Vehicle: + type: branch + description: High-level vehicle data. + +Vehicle.Cabin: + type: branch + description: Cabin related signals. + +Vehicle.Cabin.LightMode: + datatype: string + type: actuator + allowed: ['some value', 'SOME VALUE', 'another-value', 'YET_ANOTHER'] + description: Light mode with values containing spaces. + +Vehicle.Cabin.SeatPosition: + datatype: string + type: sensor + allowed: ['front left', 'front right', 'rear left', 'rear right'] + description: Seat position descriptors with spaces. + +Vehicle.Cabin.AirflowDirection: + datatype: string + type: actuator + allowed: ['up and down', 'left-right', 'CIRCULAR'] + description: Airflow direction with mixed case and spaces. + +Vehicle.Status: + type: branch + description: Status related signals. + +Vehicle.Status.Priority: + datatype: uint8 + type: attribute + allowed: [1, 2, 3] + description: Priority level without spaces (should not be modified). From d74a87a2615fc134ca7febaa17a06e3616baedac Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Fri, 6 Feb 2026 12:30:40 +0100 Subject: [PATCH 02/18] feat: Implement instance dimension enum sanitization and metadata annotation Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- docs/s2dm.md | 19 ++++++-- src/vss_tools/exporters/s2dm/type_builders.py | 21 +++++++- .../utils/graphql_directive_processor.py | 48 +++++++++++++++++++ tests/test_s2dm_exporter.py | 35 ++++++++++++++ .../test_instance_sanitization.vspec | 28 +++++++++++ 5 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 tests/vspec/test_s2dm/test_instance_sanitization.vspec diff --git a/docs/s2dm.md b/docs/s2dm.md index 3add9fc7..78d22e1f 100644 --- a/docs/s2dm.md +++ b/docs/s2dm.md @@ -62,8 +62,9 @@ GraphQL enum values must follow strict naming rules (alphanumeric + underscore o - **CamelCase** → Converted to SCREAMING_SNAKE_CASE (`"HTTPSProtocol"` → `HTTPS_PROTOCOL`) - **Leading digits** → Prefixed with underscore (`"123abc"` → `_123ABC`) -When enum values are modified, the original VSS value is preserved using `@vspec` metadata: +When enum values are modified, the original VSS value is preserved using `@vspec` metadata for complete traceability. This applies to both **allowed value enums** and **instance dimension enums**. +**Allowed Value Enum Example:** ```graphql enum Vehicle_Connection_Protocol_Enum @vspec(element: SENSOR, fqn: "Vehicle.Connection.Protocol", metadata: [{key: "allowed", value: "['HTTPSProtocol', 'TCPProtocol']"}]) { HTTPS_PROTOCOL @vspec(metadata: [{key: "originalName", value: "HTTPSProtocol"}]) @@ -71,6 +72,14 @@ enum Vehicle_Connection_Protocol_Enum @vspec(element: SENSOR, fqn: "Vehicle.Conn } ``` +**Instance Dimension Enum Example:** +```graphql +enum Vehicle_Cabin_Seat_InstanceTag_Dimension2 { + DRIVER_SIDE @vspec(metadata: [{key: "originalName", value: "DriverSide"}]) + PASSENGER_SIDE @vspec(metadata: [{key: "originalName", value: "PassengerSide"}]) +} +``` + This ensures complete traceability between the VSS source and the generated GraphQL schema. ### VSS Instances Become GraphQL Structures @@ -84,13 +93,13 @@ type Vehicle_Cabin_Seat_InstanceTag @instanceTag { } enum Vehicle_Cabin_Seat_InstanceTag_Dimension1 { - Row1 - Row2 + ROW1 @vspec(metadata: [{key: "originalName", value: "Row1"}]) + ROW2 @vspec(metadata: [{key: "originalName", value: "Row2"}]) } enum Vehicle_Cabin_Seat_InstanceTag_Dimension2 { - DriverSide - PassengerSide + DRIVER_SIDE @vspec(metadata: [{key: "originalName", value: "DriverSide"}]) + PASSENGER_SIDE @vspec(metadata: [{key: "originalName", value: "PassengerSide"}]) } ``` Such an structure is then usable by any other type like: diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index 9f178695..9aafe6e2 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -110,6 +110,8 @@ def create_instance_types( Mapping of type names to GraphQL enum or object types """ types: dict[str, GraphQLEnumType | GraphQLObjectType] = {} + vspec_comments.setdefault("instance_dimension_enums", {}) + for fqn, row in branches_df[branches_df["instances"].notna()].iterrows(): if instances := row.get("instances"): base_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) @@ -119,11 +121,28 @@ def create_instance_types( fields = {} for i, values in enumerate(dimensions, 1): enum_name = f"{tag_name}_Dimension{i}" + + # Sanitize enum values and track modifications + enum_values = {} + modified_values = {} + + for v in values: + sanitized, was_modified = _sanitize_enum_value_for_graphql(str(v)) + enum_values[sanitized] = GraphQLEnumValue(v) + + if was_modified: + modified_values[sanitized] = str(v) + types[enum_name] = GraphQLEnumType( enum_name, - {v: GraphQLEnumValue(v) for v in values}, + enum_values, description=f"Dimensional enum for VSS instance dimension {i}.", ) + + # Store metadata for directive processor + if modified_values: + vspec_comments["instance_dimension_enums"][enum_name] = {"modified_values": modified_values} + fields[f"dimension{i}"] = GraphQLField(types[enum_name]) types[tag_name] = GraphQLObjectType(tag_name, fields) diff --git a/src/vss_tools/utils/graphql_directive_processor.py b/src/vss_tools/utils/graphql_directive_processor.py index d7904cc9..74ef4922 100644 --- a/src/vss_tools/utils/graphql_directive_processor.py +++ b/src/vss_tools/utils/graphql_directive_processor.py @@ -54,6 +54,9 @@ def process_schema( lines = self._process_unit_enum_directives(lines, unit_enums_metadata, processed_enum_values) lines = self._process_allowed_enum_directives(lines, allowed_enums_metadata, processed_enum_values) + lines = self._process_instance_dimension_enum_directives( + lines, vspec_comments.get("instance_dimension_enums", {}), processed_enum_values + ) lines = self._process_field_directives(lines, vspec_comments) lines = self._process_deprecated_directives(lines, vspec_comments.get("field_deprecated", {})) lines = self._process_range_directives(lines, vspec_comments.get("field_ranges", {})) @@ -167,6 +170,51 @@ def _process_allowed_enum_directives( return lines + def _process_instance_dimension_enum_directives( + self, lines: list[str], instance_dimension_enums: dict, processed_values: set + ) -> list[str]: + """ + Process instance dimension enum directives. + + Annotates enum values that were modified during sanitization with their original names. + """ + for enum_name, enum_data in instance_dimension_enums.items(): + modified_values = enum_data.get("modified_values", {}) + if not modified_values: + continue + + in_target_enum = False + + for i, line in enumerate(lines): + # Detect enum start + if line.strip().startswith(f"enum {enum_name}"): + in_target_enum = True + continue + elif line.strip().startswith("enum ") and in_target_enum: + in_target_enum = False + continue + elif line.strip() == "}" and in_target_enum: + in_target_enum = False + continue + + # Process enum values within target enum + if in_target_enum and line.strip() and not line.strip().startswith('"'): + stripped_line = line.strip() + + for enum_value_name, original_value in modified_values.items(): + enum_value_key = f"{enum_name}.{enum_value_name}" + + if stripped_line.startswith(enum_value_name) and enum_value_key not in processed_values: + if "@vspec" not in line: + indent = line[: len(line) - len(line.lstrip())] + directive = f'@vspec(metadata: [{{key: "originalName", value: "{original_value}"}}])' + lines[i] = f"{indent}{enum_value_name} {directive}" + + processed_values.add(enum_value_key) + break + + return lines + def _process_field_directives(self, lines: list[str], vspec_comments: dict) -> list[str]: """Process consolidated field @vspec directives (element + fqn + optional metadata).""" # Process VSS type information (element + fqn + metadata) diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index bb39a705..cdba1282 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -667,3 +667,38 @@ def test_camelcase_enums_schema_generation(self): assert 'IO_ERROR @vspec(metadata: [{key: "originalName", value: "IOError"}])' in schema_str assert 'XML_PARSER @vspec(metadata: [{key: "originalName", value: "XMLParser"}])' in schema_str assert 'SOME_API_KEY @vspec(metadata: [{key: "originalName", value: "someAPIKey"}])' in schema_str + + def test_instance_dimension_enum_sanitization(self): + """Test that instance dimension enum values are properly sanitized and annotated.""" + # Load the test vspec with instances that need sanitization + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/test_instance_sanitization.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree) + schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) + + # Check that Row instance enum is created and values are sanitized + assert "enum Vehicle_Cabin_InstanceTag_Dimension1" in schema_str + assert 'ROW1 @vspec(metadata: [{key: "originalName", value: "Row1"}])' in schema_str + assert 'ROW2 @vspec(metadata: [{key: "originalName", value: "Row2"}])' in schema_str + + # Check that DriverSide/PassengerSide instance enum is created and values are sanitized + assert "enum Vehicle_Cabin_Seat_InstanceTag_Dimension1" in schema_str + assert 'DRIVER_SIDE @vspec(metadata: [{key: "originalName", value: "DriverSide"}])' in schema_str + assert 'PASSENGER_SIDE @vspec(metadata: [{key: "originalName", value: "PassengerSide"}])' in schema_str + + # Check that FrontLeft/FrontRight/RearLeft/RearRight instance enum is created and values are sanitized + assert "enum Vehicle_Cabin_Seat_Position_InstanceTag_Dimension1" in schema_str + assert 'FRONT_LEFT @vspec(metadata: [{key: "originalName", value: "FrontLeft"}])' in schema_str + assert 'FRONT_RIGHT @vspec(metadata: [{key: "originalName", value: "FrontRight"}])' in schema_str + assert 'REAR_LEFT @vspec(metadata: [{key: "originalName", value: "RearLeft"}])' in schema_str + assert 'REAR_RIGHT @vspec(metadata: [{key: "originalName", value: "RearRight"}])' in schema_str diff --git a/tests/vspec/test_s2dm/test_instance_sanitization.vspec b/tests/vspec/test_s2dm/test_instance_sanitization.vspec new file mode 100644 index 00000000..74424462 --- /dev/null +++ b/tests/vspec/test_s2dm/test_instance_sanitization.vspec @@ -0,0 +1,28 @@ +# Test vspec for instance dimension enum sanitization + +Vehicle: + type: branch + description: High-level vehicle data. + +Vehicle.Cabin: + type: branch + description: Cabin information. + instances: + - Row[1,2] + +Vehicle.Cabin.Seat: + type: branch + description: Seat information. + instances: + - ["DriverSide", "PassengerSide"] + +Vehicle.Cabin.Seat.Position: + type: branch + description: Seat position. + instances: + - ["FrontLeft", "FrontRight", "RearLeft", "RearRight"] + +Vehicle.Cabin.Seat.Position.IsOccupied: + datatype: boolean + type: actuator + description: Is the seat position occupied. From 79e4b4db93bb384c4d1a1d558d312e4355454db2 Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Fri, 6 Feb 2026 13:48:39 +0100 Subject: [PATCH 03/18] feat(s2dm): add extended attributes metadata annotations Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- docs/s2dm.md | 31 ++++++++ .../exporters/s2dm/schema_generator.py | 2 +- src/vss_tools/exporters/s2dm/type_builders.py | 57 ++++++++++++-- .../utils/graphql_directive_processor.py | 41 ++++++++-- src/vss_tools/utils/modular_export_utils.py | 3 + tests/test_s2dm_exporter.py | 76 +++++++++++++++++++ .../test_s2dm/test_extended_attributes.vspec | 33 ++++++++ 7 files changed, 227 insertions(+), 16 deletions(-) create mode 100644 tests/vspec/test_s2dm/test_extended_attributes.vspec diff --git a/docs/s2dm.md b/docs/s2dm.md index 78d22e1f..739a7a02 100644 --- a/docs/s2dm.md +++ b/docs/s2dm.md @@ -46,6 +46,37 @@ Likewise, the `driverPosition` was derived from `Vehicle.Cabin.DriverPosition` a The `@vspec` directive is also used to annotate original names when they have been modified for GraphQL compliance (for example, see [Enum Value Sanitization](#enum-value-sanitization)). +#### Extended Attributes + +If the VSpec model uses extended attributes (custom metadata), the exporter add them to `@vspec` metadata annotations. + +**Example VSS with extended attributes `source` and `quality`:** +```yaml +Vehicle.Speed: + datatype: float + type: sensor + unit: km/h + source: ecu0xAA # Extended attribute + quality: 100 # Extended attribute +``` + +**Generated GraphQL:** +```graphql +type Vehicle { + speed(unit: VelocityUnitEnum = KILOMETERS_PER_HOUR): Float + @vspec( + element: SENSOR, + fqn: "Vehicle.Speed", + metadata: [ + {key: "source", value: "ecu0xAA"}, + {key: "quality", value: "100"} + ] + ) +} +``` + +Extended attributes work on all VSS elements: branches, sensors, actuators, attributes, and structs. + ### VSS Data Types Support The exporter handles all `vspec` data types as follows: - **Strings** → GraphQL String diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index 419741fe..453d7449 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -100,7 +100,7 @@ def generate_s2dm_schema( for fqn in branches_df.index: if fqn not in types_registry: types_registry[fqn] = create_object_type( - fqn, branches_df, leaves_df, types_registry, unit_enums, vspec_comments + fqn, branches_df, leaves_df, types_registry, unit_enums, vspec_comments, extended_attributes ) # Assemble complete schema diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index 9aafe6e2..78787819 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -46,6 +46,24 @@ from .metadata_tracker import build_field_path +def _extract_extended_attributes(row: pd.Series, extended_attributes: tuple[str, ...]) -> dict[str, Any]: + """ + Extract extended attributes from a DataFrame row. + + Args: + row: pandas Series (DataFrame row) containing VSS node data + extended_attributes: Tuple of extended attribute names to extract + + Returns: + Dictionary containing only the extended attributes that exist and are not NA + """ + extracted = {} + for ext_attr in extended_attributes: + if ext_attr in row.index and pd.notna(row.get(ext_attr)): + extracted[ext_attr] = row[ext_attr] + return extracted + + def create_unit_enums() -> tuple[dict[str, GraphQLEnumType], dict[str, dict[str, dict[str, str]]]]: """ Create GraphQL enum types for VSS units grouped by quantity. @@ -307,7 +325,12 @@ def create_struct_types( fields[field_name] = GraphQLField(GraphQLNonNull(base_type), description=prop_row.get("description", "")) field_path = build_field_path(type_name, field_name) - vspec_comments["field_vss_types"][field_path] = {"element": "STRUCT_PROPERTY", "fqn": prop_fqn} + field_metadata = {"element": "STRUCT_PROPERTY", "fqn": prop_fqn} + + # Capture extended attributes if present + field_metadata.update(_extract_extended_attributes(prop_row, extended_attributes)) + + vspec_comments["field_vss_types"][field_path] = field_metadata if pd.notna(prop_row.get("min")) or pd.notna(prop_row.get("max")): vspec_comments["field_ranges"][field_path] = { @@ -321,7 +344,11 @@ def create_struct_types( struct_types[type_name] = GraphQLObjectType( name=type_name, fields=fields, description=struct_row.get("description", "") ) - vspec_comments["vss_types"][type_name] = {"element": "STRUCT", "fqn": fqn} + + # Store type-level metadata including extended attributes + type_metadata = {"element": "STRUCT", "fqn": fqn} + type_metadata.update(_extract_extended_attributes(struct_row, extended_attributes)) + vspec_comments["vss_types"][type_name] = type_metadata return struct_types @@ -367,6 +394,7 @@ def create_object_type( types_registry: dict[str, Any], unit_enums: dict[str, GraphQLEnumType], vspec_comments: dict[str, dict[str, Any]], + extended_attributes: tuple[str, ...] = (), ) -> GraphQLObjectType: """ Create GraphQL object type for a VSS branch. @@ -381,6 +409,7 @@ def create_object_type( types_registry: Registry of already-created types unit_enums: Unit enum types vspec_comments: Metadata tracking dictionary + extended_attributes: Extended attribute names from CLI Returns: GraphQL object type for the branch @@ -415,7 +444,12 @@ def get_fields() -> dict[str, GraphQLField]: field_path = build_field_path(type_name, field_name) if leaf_type := _get_vss_type_if_valid(leaf_row): - vspec_comments["field_vss_types"][field_path] = {"element": leaf_type, "fqn": child_fqn} + field_metadata = {"element": leaf_type, "fqn": child_fqn} + + # Capture extended attributes if present + field_metadata.update(_extract_extended_attributes(leaf_row, extended_attributes)) + + vspec_comments["field_vss_types"][field_path] = field_metadata if pd.notna(leaf_row.get("min")) or pd.notna(leaf_row.get("max")): vspec_comments["field_ranges"][field_path] = { @@ -434,12 +468,12 @@ def get_fields() -> dict[str, GraphQLField]: for child_fqn, child_row in branches_df[branches_df["parent"] == fqn].iterrows(): field_name = convert_name_for_graphql_schema(child_row["name"], GraphQLElementType.FIELD, S2DM_CONVERSIONS) child_type = types_registry.get(child_fqn) or create_object_type( - child_fqn, branches_df, leaves_df, types_registry, unit_enums, vspec_comments + child_fqn, branches_df, leaves_df, types_registry, unit_enums, vspec_comments, extended_attributes ) types_registry[child_fqn] = child_type hoisted_fields = get_hoisted_fields( - child_fqn, child_row, leaves_df, types_registry, unit_enums, vspec_comments + child_fqn, child_row, leaves_df, types_registry, unit_enums, vspec_comments, extended_attributes ) fields.update(hoisted_fields) @@ -450,7 +484,10 @@ def get_fields() -> dict[str, GraphQLField]: return fields - vspec_comments["vss_types"][type_name] = {"element": "BRANCH", "fqn": fqn} + # Store type-level metadata including extended attributes + type_metadata = {"element": "BRANCH", "fqn": fqn} + type_metadata.update(_extract_extended_attributes(branch_row, extended_attributes)) + vspec_comments["vss_types"][type_name] = type_metadata return GraphQLObjectType(name=type_name, fields=get_fields, description=branch_row.get("description", "")) @@ -462,6 +499,7 @@ def get_hoisted_fields( types_registry: dict[str, Any], unit_enums: dict[str, GraphQLEnumType], vspec_comments: dict[str, dict[str, Any]], + extended_attributes: tuple[str, ...] = (), ) -> dict[str, GraphQLField]: """Get fields to hoist from instantiated child branch to parent.""" hoisted: dict[str, GraphQLField] = {} @@ -492,12 +530,17 @@ def get_hoisted_fields( field_path = build_field_path(parent_type_name, hoisted_field_name) if leaf_type := _get_vss_type_if_valid(leaf_row): - vspec_comments["field_vss_types"][field_path] = { + field_metadata = { "element": leaf_type, "fqn": leaf_fqn, "instantiate": False, } + # Capture extended attributes if present + field_metadata.update(_extract_extended_attributes(leaf_row, extended_attributes)) + + vspec_comments["field_vss_types"][field_path] = field_metadata + if pd.notna(leaf_row.get("min")) or pd.notna(leaf_row.get("max")): vspec_comments["field_ranges"][field_path] = { "min": leaf_row.get("min") if pd.notna(leaf_row.get("min")) else None, diff --git a/src/vss_tools/utils/graphql_directive_processor.py b/src/vss_tools/utils/graphql_directive_processor.py index 74ef4922..71d2a781 100644 --- a/src/vss_tools/utils/graphql_directive_processor.py +++ b/src/vss_tools/utils/graphql_directive_processor.py @@ -237,13 +237,24 @@ def _process_field_directives(self, lines: list[str], vspec_comments: dict) -> l continue if in_type and line.strip().startswith(f"{field_name}") and "@vspec" not in line: - # Build directive with element (mandatory), fqn, and optional metadata + # Build metadata array from extended attributes + metadata_entries = [] + + # Add instantiate metadata if present if instantiate is False: - # Add metadata for hoisted non-instantiated fields - directive = ( - f'@vspec(element: {element}, fqn: "{fqn}", ' - f'metadata: [{{key: "instantiate", value: "false"}}])' - ) + metadata_entries.append('{key: "instantiate", value: "false"}') + + # Add extended attributes metadata + for key, value in vss_info.items(): + if key not in ["element", "fqn", "instantiate"]: + # Escape quotes in value + escaped_value = str(value).replace('"', '\\\\"') + metadata_entries.append(f'{{key: "{key}", value: "{escaped_value}"}}') + + # Build directive + if metadata_entries: + metadata_str = ", ".join(metadata_entries) + directive = f'@vspec(element: {element}, fqn: "{fqn}", metadata: [{metadata_str}])' else: # Standard directive without metadata directive = f'@vspec(element: {element}, fqn: "{fqn}")' @@ -375,11 +386,25 @@ def _process_type_directives(self, lines: list[str], vspec_comments: dict) -> li ) new_line += f" {directive}" elif needs_vss_type and "@vspec" not in line: - # Regular types get element + fqn only + # Regular types get element + fqn + extended attributes metadata vss_info = vspec_comments["vss_types"][type_name] element = vss_info["element"] fqn = vss_info["fqn"] - directive = f'@vspec(element: {element}, fqn: "{fqn}")' + + # Build metadata array from extended attributes + metadata_entries = [] + for key, value in vss_info.items(): + if key not in ["element", "fqn"]: + # Escape quotes in value + escaped_value = str(value).replace('"', '\\\\"') + metadata_entries.append(f'{{key: "{key}", value: "{escaped_value}"}}') + + # Build directive + if metadata_entries: + metadata_str = ", ".join(metadata_entries) + directive = f'@vspec(element: {element}, fqn: "{fqn}", metadata: [{metadata_str}])' + else: + directive = f'@vspec(element: {element}, fqn: "{fqn}")' new_line += f" {directive}" new_line += " {" diff --git a/src/vss_tools/utils/modular_export_utils.py b/src/vss_tools/utils/modular_export_utils.py index 37bd0195..f4528e34 100644 --- a/src/vss_tools/utils/modular_export_utils.py +++ b/src/vss_tools/utils/modular_export_utils.py @@ -270,6 +270,9 @@ def write_domain_files( # Apply vspec directives to instance tag files if directive_processor and hasattr(directive_processor, "process_schema"): lines = file_content.split("\n") + lines = directive_processor._process_instance_dimension_enum_directives( + lines, vspec_comments.get("instance_dimension_enums", {}), set() + ) lines = directive_processor._process_type_directives(lines, vspec_comments) file_content = "\n".join(lines) diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index cdba1282..7895d5e1 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -457,6 +457,43 @@ def test_modular_export_nested_domains(self, tmp_path): seat_content = (output_dir / "domain" / "Vehicle" / "Cabin" / "Seat" / "_Seat.graphql").read_text() assert "Vehicle_Cabin_Seat" in seat_content + def test_modular_export_instance_enum_directives(self, tmp_path): + """Test that instance dimension enums get @vspec directives in modular export.""" + from vss_tools.exporters.s2dm import write_modular_schema + + # Load test vspec with instances that need sanitization + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/test_instance_sanitization.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + # Generate schema + schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema(tree) + + # Test modular export with flat domains + output_dir = tmp_path / "modular_instance_test" + write_modular_schema( + schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments, output_dir, flat_domains=True + ) + + # Check that instance files were created + instance_file = output_dir / "instances" / "Vehicle_Cabin_Seat_InstanceTag.graphql" + assert instance_file.exists() + + # Verify that instance dimension enums have @vspec directives with originalName + instance_content = instance_file.read_text() + + # Check for sanitized enum values with @vspec directives + assert 'DRIVER_SIDE @vspec(metadata: [{key: "originalName", value: "DriverSide"}])' in instance_content + assert 'PASSENGER_SIDE @vspec(metadata: [{key: "originalName", value: "PassengerSide"}])' in instance_content + def test_non_instantiated_property_hoisting(self, tmp_path: Path): """Test that properties with instantiate=false are hoisted to parent type.""" # Load the test vspec with non-instantiated properties @@ -702,3 +739,42 @@ def test_instance_dimension_enum_sanitization(self): assert 'FRONT_RIGHT @vspec(metadata: [{key: "originalName", value: "FrontRight"}])' in schema_str assert 'REAR_LEFT @vspec(metadata: [{key: "originalName", value: "RearLeft"}])' in schema_str assert 'REAR_RIGHT @vspec(metadata: [{key: "originalName", value: "RearRight"}])' in schema_str + + def test_extended_attributes_in_metadata(self): + """Test that extended attributes are captured and added to @vspec metadata.""" + # Load the test vspec with extended attributes + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/test_extended_attributes.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=("source", "quality", "calibration", "customMetadata", "anotherAttribute"), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema( + tree, extended_attributes=("source", "quality", "calibration", "customMetadata", "anotherAttribute") + ) + schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) + + # Check Vehicle.Speed has source and quality in metadata + assert "speed(unit: RelationUnitEnum = PERCENT): Float" in schema_str + assert '@vspec(element: SENSOR, fqn: "Vehicle.Speed"' in schema_str + assert '{key: "source", value: "ecu0xAA"}' in schema_str + assert '{key: "quality", value: "100"}' in schema_str + + # Check Vehicle.Temperature has source, quality, and calibration + assert "temperature(unit: AngleUnitEnum = DEGREE): Int16" in schema_str + assert '@vspec(element: SENSOR, fqn: "Vehicle.Temperature"' in schema_str + assert '{key: "source", value: "ecu0xBB"}' in schema_str + assert '{key: "quality", value: "95"}' in schema_str + assert '{key: "calibration", value: "factory"}' in schema_str + + # Check Vehicle.Info.Model has customMetadata and anotherAttribute + assert "model: String" in schema_str + assert '@vspec(element: ATTRIBUTE, fqn: "Vehicle.Info.Model"' in schema_str + assert '{key: "customMetadata", value: "test_value"}' in schema_str + assert '{key: "anotherAttribute", value: "42"}' in schema_str diff --git a/tests/vspec/test_s2dm/test_extended_attributes.vspec b/tests/vspec/test_s2dm/test_extended_attributes.vspec new file mode 100644 index 00000000..00188c79 --- /dev/null +++ b/tests/vspec/test_s2dm/test_extended_attributes.vspec @@ -0,0 +1,33 @@ +# Test vspec for extended attributes in S2DM exporter + +Vehicle: + type: branch + description: High-level vehicle data. + +Vehicle.Speed: + datatype: float + type: sensor + unit: percent + description: Vehicle speed. + source: ecu0xAA + quality: 100 + +Vehicle.Temperature: + datatype: int16 + type: sensor + unit: degrees + description: Ambient temperature. + source: ecu0xBB + quality: 95 + calibration: factory + +Vehicle.Info: + type: branch + description: Vehicle information. + +Vehicle.Info.Model: + datatype: string + type: attribute + description: Vehicle model. + customMetadata: test_value + anotherAttribute: 42 From d0200784305527cd423322b7dc42ee86612cfecb Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Sat, 7 Feb 2026 00:02:39 +0100 Subject: [PATCH 04/18] feat(s2dm): enhance pluralization handling and naming conventions in GraphQL schema generation Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- docs/s2dm.md | 74 ++++++++++++++++++- src/vss_tools/exporters/s2dm/__init__.py | 10 ++- .../exporters/s2dm/reference_generator.py | 60 ++++++++++++++- src/vss_tools/exporters/s2dm/type_builders.py | 52 ++++++++++++- tests/test_s2dm_exporter.py | 13 ++-- 5 files changed, 194 insertions(+), 15 deletions(-) diff --git a/docs/s2dm.md b/docs/s2dm.md index 739a7a02..9bcfe14b 100644 --- a/docs/s2dm.md +++ b/docs/s2dm.md @@ -82,9 +82,34 @@ The exporter handles all `vspec` data types as follows: - **Strings** → GraphQL String - **Numbers** → GraphQL Int, Float, or custom scalars (Int8, UInt16, etc.) - **Booleans** → GraphQL Boolean -- **Arrays** → GraphQL Lists +- **Arrays** → GraphQL Lists (with natural plural field names) - **Allowed values** → GraphQL Enums +#### List Field Names (Automatic Pluralization) + +When VSS branches have instances (like `Seat` with multiple rows/positions), the parent type gets a list field. The S2DM exporter automatically generates **natural plural names** using the inflect library: + +- `seat` → `seats: [Vehicle_Cabin_Seat]` +- `door` → `doors: [Vehicle_Cabin_Door]` +- `window` → `windows: [Vehicle_Cabin_Window]` +- `battery` → `batteries: [Vehicle_Battery]` +- `mirror` → `mirrors: [Vehicle_Body_Mirror]` + +This improves GraphQL schema readability by following common naming conventions instead of using mechanical suffixes like `_s`. + +**Example:** +```graphql +type Vehicle_Cabin { + """Cabin seats for passengers.""" + seats: [Vehicle_Cabin_Seat] + + """Cabin doors.""" + doors: [Vehicle_Cabin_Door] +} +``` + +**Tracking:** All pluralized field names are logged to `vspec_reference/pluralized_field_names.yaml` showing the original VSS FQN, the plural field name used, and its location in the GraphQL schema. + #### Enum Value Sanitization GraphQL enum values must follow strict naming rules (alphanumeric + underscore only, cannot start with a digit). The S2DM exporter automatically sanitizes VSS enum values to comply with GraphQL requirements: @@ -113,6 +138,46 @@ enum Vehicle_Cabin_Seat_InstanceTag_Dimension2 { This ensures complete traceability between the VSS source and the generated GraphQL schema. +### GraphQL Naming Convention Warnings + +GraphQL best practices recommend using **singular names for types** (e.g., `User` not `Users`, `Product` not `Products`). The S2DM exporter automatically detects VSS branches with plural names and generates warnings to help identify potential naming convention violations. + +**Detection:** The exporter uses the inflect library to identify potential plural type names. It maintains a whitelist of known exceptions (acronyms like "ADAS", "ABS", Latin words like "Status", "Chassis") to reduce false positives. + +**Warning Output:** When plural type names are detected, two files are generated in `vspec_reference/`: + +1. **`plural_type_warnings.yaml`** - Lists all detected plural type names: + ```yaml + # WARNING: These elements in the reference model seem to have a plural name... + + Vehicle.Cabin.Lights: + singular: Light + currentNameInGraphQLModel: Vehicle_Cabin_Lights + + Vehicle.Body.Mirrors: + singular: Mirror + currentNameInGraphQLModel: Vehicle_Body_Mirrors + + # Whitelisted words (excluded from plural detection): + whitelisted_non_plurals: + - ADAS + - ABS + - Status + - Chassis + # ... etc + ``` + +2. **Console warnings:** During export, warnings are logged for immediate visibility: + ``` + WARNING: Type 'Vehicle_Cabin_Lights' uses potential plural name 'Lights'. + Suggested singular: 'Light' (VSS FQN: Vehicle.Cabin.Lights) + ``` + +**Review Process:** These warnings help VSS maintainers identify: +- **True plurals** - VSS branches that should be renamed to singular form +- **False positives** - Words that end in 's' but aren't actually plural (add to whitelist) +- **Acceptable exceptions** - Cases where plural names are intentional + ### VSS Instances Become GraphQL Structures When your `vspec` has instances (like multiple seats), the exporter creates proper GraphQL types: @@ -186,7 +251,9 @@ myOutput/ ├── README.md # Documentation and provenance info ├── vspec_lookup_spec.yaml # Complete VSS tree (fully expanded) ├── vspec_units.yaml # Units used (if provided via -u or implicit) - └── vspec_quantities.yaml # Quantities used (if provided via -q or implicit) + ├── vspec_quantities.yaml # Quantities used (if provided via -q or implicit) + ├── plural_type_warnings.yaml # Plural type names detected (if any) + └── pluralized_field_names.yaml # Fields changed to plural form (if any instances) ``` ### VSS Reference Files @@ -197,11 +264,14 @@ The `vspec_reference/` directory provides complete traceability: - **vspec_lookup_spec.yaml** - Complete VSS specification tree (fully processed and expanded) in YAML format - **vspec_units.yaml** - Unit definitions used during generation (included if units were provided via `-u` flag or implicitly loaded) - **vspec_quantities.yaml** - Quantity definitions used during generation (included if quantities were provided via `-q` flag or implicitly loaded) +- **plural_type_warnings.yaml** - VSS branches with plural type names that may violate GraphQL naming conventions (generated if any detected) +- **pluralized_field_names.yaml** - Fields whose names were changed to plural form for list fields (generated if any instances exist) These files allow you to: 1. Trace GraphQL elements back to their VSS source using the FQN in `@vspec` directives 2. Reproduce the exact GraphQL schema by re-running the exporter 3. Understand which input files were used for generation +4. Review naming convention warnings and pluralization changes diff --git a/src/vss_tools/exporters/s2dm/__init__.py b/src/vss_tools/exporters/s2dm/__init__.py index 6444ddce..cdac0799 100644 --- a/src/vss_tools/exporters/s2dm/__init__.py +++ b/src/vss_tools/exporters/s2dm/__init__.py @@ -109,21 +109,21 @@ def cli( log.info("Generating S2DM GraphQL schema...") # Generate the schema - schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema( + schema, unit_enums_metadata, allowed_enums_metadata, mapping_metadata = generate_s2dm_schema( tree, data_type_tree, extended_attributes=extended_attributes ) if modular: # Write modular files write_modular_schema( - schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments, output_dir, flat_domains + schema, unit_enums_metadata, allowed_enums_metadata, mapping_metadata, output_dir, flat_domains ) log.info(f"Modular GraphQL schema written to {output_dir}/") else: # Single file export: write to outputDir/outputDir.graphql graphql_file = output_dir / f"{output_dir.name}.graphql" full_schema_str = print_schema_with_vspec_directives( - schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments + schema, unit_enums_metadata, allowed_enums_metadata, mapping_metadata ) with open(graphql_file, "w", encoding="utf-8") as outfile: outfile.write(full_schema_str) @@ -131,7 +131,9 @@ def cli( log.info(f"GraphQL schema written to {graphql_file}") # Generate VSS reference files (will check for implicit files) - generate_vspec_reference(tree, data_type_tree, output_dir, extended_attributes, vspec, units, quantities) + generate_vspec_reference( + tree, data_type_tree, output_dir, extended_attributes, vspec, units, quantities, mapping_metadata + ) except S2DMExporterException as e: log.error(e) diff --git a/src/vss_tools/exporters/s2dm/reference_generator.py b/src/vss_tools/exporters/s2dm/reference_generator.py index 486990e4..e8ac5fcc 100644 --- a/src/vss_tools/exporters/s2dm/reference_generator.py +++ b/src/vss_tools/exporters/s2dm/reference_generator.py @@ -34,6 +34,7 @@ def generate_vspec_reference( vspec_file: Path, units_files: tuple[Path, ...], quantities_files: tuple[Path, ...], + mapping_metadata: dict[str, dict] | None = None, ) -> None: """ Generate VSS reference files alongside GraphQL output. @@ -49,6 +50,7 @@ def generate_vspec_reference( vspec_file: Path to vspec file (to find implicit units/quantities) units_files: Unit files from CLI quantities_files: Quantity files from CLI + mapping_metadata: Optional metadata including plural type warnings Raises: S2DMExporterException: If reference file generation fails @@ -152,8 +154,58 @@ def generate_vspec_reference( except (PermissionError, OSError) as e: raise S2DMExporterException(f"Failed to write quantities file {quantities_output}: {e}") from e + # Write plural type warnings if any were collected + if mapping_metadata and mapping_metadata.get("plural_type_warnings"): + warnings_output = reference_dir / "plural_type_warnings.yaml" + try: + with open(warnings_output, "w") as f: + # Write header comment + f.write( + "# WARNING: These elements in the reference model seem to have a plural name, " + "while GraphQL best practices suggest the use of the singular form for type names.\n" + ) + f.write("# Consider replacing plural forms and whitelisting false positives.\n\n") + + # Write each warning as FQN with nested fields + for warning in mapping_metadata["plural_type_warnings"]: + f.write(f"{warning['fqn']}:\n") + f.write(f" suggested_singular: {warning['suggested_singular']}\n") + f.write(f" current_name_in_graphql_model: {warning['type_name']}\n") + f.write("\n") + + warning_count = len(mapping_metadata["plural_type_warnings"]) + log.info(f" - Plural warnings: {warnings_output.name} ({warning_count} warning(s))") + except (PermissionError, OSError) as e: + raise S2DMExporterException(f"Failed to write plural warnings file {warnings_output}: {e}") from e + + # Write pluralized field names if any were collected + if mapping_metadata and mapping_metadata.get("pluralized_field_names"): + pluralized_output = reference_dir / "pluralized_field_names.yaml" + try: + with open(pluralized_output, "w") as f: + # Write header comment + f.write( + "# Following names were changed to their plural form as they resolve to an output type " + "with a List type modifier.\n\n" + ) + + # Write each pluralized field as FQN with nested fields + for entry in mapping_metadata["pluralized_field_names"]: + f.write(f"{entry['fqn']}:\n") + f.write(f" plural_field_name: {entry['plural_field_name']}\n") + f.write(f" path_in_graphql_model: {entry['path_in_graphql_model']}\n") + f.write("\n") + + log.info( + f" - Pluralized fields: {pluralized_output.name} " + f"({len(mapping_metadata['pluralized_field_names'])} field(s))" + ) + except (PermissionError, OSError) as e: + raise S2DMExporterException(f"Failed to write pluralized fields file {pluralized_output}: {e}") from e + # Generate README.md for provenance documentation - generate_reference_readme(reference_dir, vspec_file, actual_units, actual_quantities) + has_plural_warnings = mapping_metadata and bool(mapping_metadata.get("plural_type_warnings")) + generate_reference_readme(reference_dir, vspec_file, actual_units, actual_quantities, has_plural_warnings) except S2DMExporterException: # Re-raise our custom exceptions @@ -168,6 +220,7 @@ def generate_reference_readme( vspec_file: Path, units_files: tuple[Path, ...] | None, quantities_files: tuple[Path, ...] | None, + has_plural_warnings: bool = False, ) -> None: """ Generate README.md documenting the provenance of reference files. @@ -177,6 +230,7 @@ def generate_reference_readme( vspec_file: Original vspec input file units_files: Units files used (explicit or implicit) quantities_files: Quantities files used (explicit or implicit) + has_plural_warnings: Whether plural type warnings were generated Raises: S2DMExporterException: If README generation fails @@ -211,6 +265,10 @@ def generate_reference_readme( readme_content += """ * **vspec_quantities.yaml** - Quantity definitions categorizing measurements.""" + if has_plural_warnings: + readme_content += """ +* **plural_type_warnings.txt** - VSS branches with plural type names (GraphQL prefers singular).""" + readme_content += """ ## Documentation diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index 78787819..f86bdfe3 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -22,6 +22,7 @@ from typing import Any import caseconverter +import inflect import pandas as pd from graphql import ( GraphQLArgument, @@ -64,6 +65,42 @@ def _extract_extended_attributes(row: pd.Series, extended_attributes: tuple[str, return extracted +# Initialize inflect engine for pluralization (singleton) +_inflect_engine = inflect.engine() + + +def _check_and_collect_plural_type_name( + converted_type_name: str, fqn: str, original_name: str, plural_name_warnings: dict[str, dict[str, Any]] +) -> None: + """ + Check if a name appears to be plural and collect for reporting. + + Uses inflect library to detect potential plural forms. Reports all cases where + a singular form is detected, without filtering. This allows downstream tools + to decide which cases are true issues vs. acceptable exceptions. + + Args: + converted_type_name: The converted type name (for schema) + fqn: The fully qualified VSS name for context + original_name: The original VSS branch/struct name (before conversion) + plural_name_warnings: Dictionary to store metadata (adds to "plural_type_warnings" key) + """ + # Check if inflect detects a singular form + # Returns False if already singular, or the singular form if plural + singular = _inflect_engine.singular_noun(original_name) + + if singular: + plural_name_warnings.setdefault("plural_type_warnings", []).append( + {"type_name": converted_type_name, "fqn": fqn, "plural_word": original_name, "suggested_singular": singular} + ) + + # Also log to console for immediate visibility + log.warning( + f"Type '{converted_type_name}' uses potential plural name '{original_name}'. " + f"Suggested singular: '{singular}' (VSS FQN: {fqn})" + ) + + def create_unit_enums() -> tuple[dict[str, GraphQLEnumType], dict[str, dict[str, dict[str, str]]]]: """ Create GraphQL enum types for VSS units grouped by quantity. @@ -415,8 +452,13 @@ def create_object_type( GraphQL object type for the branch """ branch_row = branches_df.loc[fqn] + original_name = branch_row["name"] type_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) + # Check if branch name is plural and collect for reporting + # Pass original VSS branch name (before any conversion) + _check_and_collect_plural_type_name(type_name, fqn, original_name, vspec_comments) + def get_fields() -> dict[str, GraphQLField]: fields = {} @@ -478,7 +520,15 @@ def get_fields() -> dict[str, GraphQLField]: fields.update(hoisted_fields) if child_row.get("instances"): - fields[f"{field_name}_s"] = GraphQLField(GraphQLList(child_type)) + # Use natural plural form for list fields (using inflect directly) + plural_field_name = _inflect_engine.plural(field_name) + fields[plural_field_name] = GraphQLField(GraphQLList(child_type)) + + # Collect pluralized field name for reporting + field_path = build_field_path(type_name, plural_field_name) + vspec_comments.setdefault("pluralized_field_names", []).append( + {"fqn": child_fqn, "plural_field_name": plural_field_name, "path_in_graphql_model": field_path} + ) else: fields[field_name] = GraphQLField(child_type) diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index 7895d5e1..45a23379 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -16,6 +16,7 @@ get_metadata_df, print_schema_with_vspec_directives, ) +from vss_tools.exporters.s2dm.type_builders import _sanitize_enum_value_for_graphql from vss_tools.main import get_trees from vss_tools.utils.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema @@ -308,8 +309,8 @@ def test_instance_tag_support(self): assert "id: ID!" in sdl # Verify the complete structure matches the reference pattern - # The seat should be a list field (seat_s) because it has instances - assert "seat_s: [Vehicle_Cabin_Seat]" in sdl + # The seat should be a list field (seats) with natural plural because it has instances + assert "seats: [Vehicle_Cabin_Seat]" in sdl def test_allowed_value_enums_generation(self): """Test that allowed value enums are generated correctly.""" @@ -536,13 +537,11 @@ def test_non_instantiated_property_hoisting(self, tmp_path: Path): assert "someSignal" in cabin_type_content # Verify it has the instantiate=false metadata assert 'metadata: [{key: "instantiate", value: "false"}]' in cabin_type_content - # And door_s array field should also be there - assert "door_s" in cabin_type_content + # And doors array field should also be there (natural plural) + assert "doors" in cabin_type_content def test_enum_value_sanitization_with_spaces(self): - """Test that enum values with spaces are properly sanitized and annotated.""" - from vss_tools.exporters.s2dm.type_builders import _sanitize_enum_value_for_graphql - + """Test that enum values with spaces are sanitized correctly.""" # Test the sanitization function assert _sanitize_enum_value_for_graphql("some value") == ("SOME_VALUE", True) assert _sanitize_enum_value_for_graphql("SOME VALUE") == ("SOME_VALUE", True) From a4b3b50b543320dc309fc15079487814de5d8871 Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Mon, 9 Feb 2026 16:07:43 +0100 Subject: [PATCH 05/18] refactor(s2dm): move GraphQL utilities into s2dm exporter Move graphql_utils.py, graphql_scalars.py, graphql_directive_processor.py, and modular_export_utils.py from utils/ to exporters/s2dm/ since they are exclusively used by the s2dm exporter. Update all imports accordingly. Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- src/vss_tools/exporters/s2dm/constants.py | 5 +++-- .../s2dm}/graphql_directive_processor.py | 2 +- .../s2dm}/graphql_scalars.py | 0 .../s2dm}/graphql_utils.py | 0 .../s2dm}/modular_export_utils.py | 2 +- .../exporters/s2dm/reference_generator.py | 18 ++++++++--------- .../exporters/s2dm/schema_generator.py | 14 ++++++------- src/vss_tools/exporters/s2dm/type_builders.py | 20 ++++++++++--------- tests/test_conversions.py | 6 +++++- tests/test_graphql_naming.py | 2 +- tests/test_graphql_utils.py | 2 +- tests/test_s2dm_exporter.py | 2 +- tests/test_s2dm_structs.py | 4 ++-- 13 files changed, 42 insertions(+), 35 deletions(-) rename src/vss_tools/{utils => exporters/s2dm}/graphql_directive_processor.py (99%) rename src/vss_tools/{utils => exporters/s2dm}/graphql_scalars.py (100%) rename src/vss_tools/{utils => exporters/s2dm}/graphql_utils.py (100%) rename src/vss_tools/{utils => exporters/s2dm}/modular_export_utils.py (99%) diff --git a/src/vss_tools/exporters/s2dm/constants.py b/src/vss_tools/exporters/s2dm/constants.py index 150639de..cc300d36 100644 --- a/src/vss_tools/exporters/s2dm/constants.py +++ b/src/vss_tools/exporters/s2dm/constants.py @@ -15,12 +15,13 @@ from caseconverter import DELIMITERS, pascalcase -from vss_tools.utils.graphql_utils import ( +from vss_tools.utils.string_conversion_utils import handle_fqn_conversion + +from .graphql_utils import ( DEFAULT_CONVERSIONS, GraphQLElementType, load_predefined_schema_elements, ) -from vss_tools.utils.string_conversion_utils import handle_fqn_conversion # VSS leaf types to track in field metadata (corresponds to VspecElement enum in directives.graphql) # Note: BRANCH is excluded as it's handled separately for object types diff --git a/src/vss_tools/utils/graphql_directive_processor.py b/src/vss_tools/exporters/s2dm/graphql_directive_processor.py similarity index 99% rename from src/vss_tools/utils/graphql_directive_processor.py rename to src/vss_tools/exporters/s2dm/graphql_directive_processor.py index 71d2a781..a2c53095 100644 --- a/src/vss_tools/utils/graphql_directive_processor.py +++ b/src/vss_tools/exporters/s2dm/graphql_directive_processor.py @@ -18,7 +18,7 @@ from graphql import GraphQLSchema, print_schema -from vss_tools.utils.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema +from .graphql_utils import GraphQLElementType, convert_name_for_graphql_schema class GraphQLDirectiveProcessor: diff --git a/src/vss_tools/utils/graphql_scalars.py b/src/vss_tools/exporters/s2dm/graphql_scalars.py similarity index 100% rename from src/vss_tools/utils/graphql_scalars.py rename to src/vss_tools/exporters/s2dm/graphql_scalars.py diff --git a/src/vss_tools/utils/graphql_utils.py b/src/vss_tools/exporters/s2dm/graphql_utils.py similarity index 100% rename from src/vss_tools/utils/graphql_utils.py rename to src/vss_tools/exporters/s2dm/graphql_utils.py diff --git a/src/vss_tools/utils/modular_export_utils.py b/src/vss_tools/exporters/s2dm/modular_export_utils.py similarity index 99% rename from src/vss_tools/utils/modular_export_utils.py rename to src/vss_tools/exporters/s2dm/modular_export_utils.py index f4528e34..057410d4 100644 --- a/src/vss_tools/utils/modular_export_utils.py +++ b/src/vss_tools/exporters/s2dm/modular_export_utils.py @@ -367,7 +367,7 @@ def write_common_files( """ from graphql import is_scalar_type, print_type - from vss_tools.utils.graphql_utils import extract_custom_directives_from_schema + from .graphql_utils import extract_custom_directives_from_schema # Ensure output directory exists output_dir.mkdir(parents=True, exist_ok=True) diff --git a/src/vss_tools/exporters/s2dm/reference_generator.py b/src/vss_tools/exporters/s2dm/reference_generator.py index e8ac5fcc..9c5ea8e8 100644 --- a/src/vss_tools/exporters/s2dm/reference_generator.py +++ b/src/vss_tools/exporters/s2dm/reference_generator.py @@ -101,15 +101,15 @@ def generate_vspec_reference( shutil.copy2(actual_units[0], units_output) else: merged = {} - for f in actual_units: + for unit_file in actual_units: try: - with open(f) as inf: + with open(unit_file) as inf: if data := yaml.safe_load(inf): merged.update(data) except yaml.YAMLError as e: - raise S2DMExporterException(f"Invalid YAML in units file {f}: {e}") from e + raise S2DMExporterException(f"Invalid YAML in units file {unit_file}: {e}") from e except FileNotFoundError: - raise S2DMExporterException(f"Units file not found: {f}") from None + raise S2DMExporterException(f"Units file not found: {unit_file}") from None with open(units_output, "w", encoding="utf-8") as outf: yaml.dump(merged, outf, default_flow_style=False, sort_keys=True) @@ -135,15 +135,15 @@ def generate_vspec_reference( shutil.copy2(actual_quantities[0], quantities_output) else: merged = {} - for f in actual_quantities: + for qty_file in actual_quantities: try: - with open(f) as inf: + with open(qty_file) as inf: if data := yaml.safe_load(inf): merged.update(data) except yaml.YAMLError as e: - raise S2DMExporterException(f"Invalid YAML in quantities file {f}: {e}") from e + raise S2DMExporterException(f"Invalid YAML in quantities file {qty_file}: {e}") from e except FileNotFoundError: - raise S2DMExporterException(f"Quantities file not found: {f}") from None + raise S2DMExporterException(f"Quantities file not found: {qty_file}") from None with open(quantities_output, "w", encoding="utf-8") as outf: yaml.dump(merged, outf, default_flow_style=False, sort_keys=True) @@ -204,7 +204,7 @@ def generate_vspec_reference( raise S2DMExporterException(f"Failed to write pluralized fields file {pluralized_output}: {e}") from e # Generate README.md for provenance documentation - has_plural_warnings = mapping_metadata and bool(mapping_metadata.get("plural_type_warnings")) + has_plural_warnings = bool(mapping_metadata and mapping_metadata.get("plural_type_warnings")) generate_reference_readme(reference_dir, vspec_file, actual_units, actual_quantities, has_plural_warnings) except S2DMExporterException: diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index 453d7449..e2382f4d 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -22,18 +22,18 @@ from graphql import GraphQLField, GraphQLObjectType, GraphQLSchema, GraphQLString from vss_tools.tree import VSSNode -from vss_tools.utils.graphql_directive_processor import GraphQLDirectiveProcessor -from vss_tools.utils.graphql_scalars import get_vss_scalar_types -from vss_tools.utils.modular_export_utils import ( +from vss_tools.utils.pandas_utils import get_metadata_df + +from .constants import CUSTOM_DIRECTIVES, S2DMExporterException +from .graphql_directive_processor import GraphQLDirectiveProcessor +from .graphql_scalars import get_vss_scalar_types +from .metadata_tracker import init_vspec_comments +from .modular_export_utils import ( analyze_schema_for_flat_domains, analyze_schema_for_nested_domains, write_common_files, write_domain_files, ) -from vss_tools.utils.pandas_utils import get_metadata_df - -from .constants import CUSTOM_DIRECTIVES, S2DMExporterException -from .metadata_tracker import init_vspec_comments from .type_builders import ( create_allowed_enums, create_instance_types, diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index f86bdfe3..72bc4522 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -19,7 +19,7 @@ from __future__ import annotations import re -from typing import Any +from typing import Any, cast import caseconverter import inflect @@ -39,11 +39,11 @@ from vss_tools import log from vss_tools.datatypes import dynamic_units from vss_tools.tree import VSSNode, expand_string -from vss_tools.utils.graphql_scalars import VSS_DATATYPE_MAP -from vss_tools.utils.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema from vss_tools.utils.pandas_utils import get_metadata_df from .constants import S2DM_CONVERSIONS, VSS_LEAF_TYPES +from .graphql_scalars import VSS_DATATYPE_MAP +from .graphql_utils import GraphQLElementType, convert_name_for_graphql_schema from .metadata_tracker import build_field_path @@ -70,7 +70,7 @@ def _extract_extended_attributes(row: pd.Series, extended_attributes: tuple[str, def _check_and_collect_plural_type_name( - converted_type_name: str, fqn: str, original_name: str, plural_name_warnings: dict[str, dict[str, Any]] + converted_type_name: str, fqn: str, original_name: str, plural_name_warnings: dict[str, Any] ) -> None: """ Check if a name appears to be plural and collect for reporting. @@ -90,7 +90,8 @@ def _check_and_collect_plural_type_name( singular = _inflect_engine.singular_noun(original_name) if singular: - plural_name_warnings.setdefault("plural_type_warnings", []).append( + warnings_list = cast(list[dict[str, Any]], plural_name_warnings.setdefault("plural_type_warnings", [])) + warnings_list.append( {"type_name": converted_type_name, "fqn": fqn, "plural_word": original_name, "suggested_singular": singular} ) @@ -152,7 +153,7 @@ def _get_quantity_units() -> dict[str, dict[str, dict[str, str]]]: def create_instance_types( - branches_df: pd.DataFrame, vspec_comments: dict[str, dict[str, Any]] + branches_df: pd.DataFrame, vspec_comments: dict[str, Any] ) -> dict[str, GraphQLEnumType | GraphQLObjectType]: """ Create GraphQL types for VSS instance-based branches. @@ -330,7 +331,7 @@ def _sanitize_enum_value_for_graphql(original_value: str) -> tuple[str, bool]: def create_struct_types( data_type_tree: VSSNode | None, - vspec_comments: dict[str, dict[str, Any]], + vspec_comments: dict[str, Any], extended_attributes: tuple[str, ...] = (), ) -> dict[str, GraphQLObjectType]: """ @@ -430,7 +431,7 @@ def create_object_type( leaves_df: pd.DataFrame, types_registry: dict[str, Any], unit_enums: dict[str, GraphQLEnumType], - vspec_comments: dict[str, dict[str, Any]], + vspec_comments: dict[str, Any], extended_attributes: tuple[str, ...] = (), ) -> GraphQLObjectType: """ @@ -526,7 +527,8 @@ def get_fields() -> dict[str, GraphQLField]: # Collect pluralized field name for reporting field_path = build_field_path(type_name, plural_field_name) - vspec_comments.setdefault("pluralized_field_names", []).append( + pluralized_list = cast(list[dict[str, Any]], vspec_comments.setdefault("pluralized_field_names", [])) + pluralized_list.append( {"fqn": child_fqn, "plural_field_name": plural_field_name, "path_in_graphql_model": field_path} ) else: diff --git a/tests/test_conversions.py b/tests/test_conversions.py index 7f7eb0d3..8d408971 100644 --- a/tests/test_conversions.py +++ b/tests/test_conversions.py @@ -12,7 +12,11 @@ sys.path.insert(0, "src") -from vss_tools.utils.graphql_utils import DEFAULT_CONVERSIONS, GraphQLElementType, convert_name_for_graphql_schema +from vss_tools.exporters.s2dm.graphql_utils import ( + DEFAULT_CONVERSIONS, + GraphQLElementType, + convert_name_for_graphql_schema, +) from vss_tools.utils.string_conversion_utils import handle_fqn_conversion print("=== Creating S2DM conversions in same scope ===") diff --git a/tests/test_graphql_naming.py b/tests/test_graphql_naming.py index 50e69519..33cbd731 100644 --- a/tests/test_graphql_naming.py +++ b/tests/test_graphql_naming.py @@ -6,7 +6,7 @@ # # SPDX-License-Identifier: MPL-2.0 -from vss_tools.utils.graphql_utils import ( +from vss_tools.exporters.s2dm.graphql_utils import ( GraphQLElementType, convert_fqn_to_graphql_type_name, convert_name_for_graphql_schema, diff --git a/tests/test_graphql_utils.py b/tests/test_graphql_utils.py index d5fb3ab4..fc944a7a 100644 --- a/tests/test_graphql_utils.py +++ b/tests/test_graphql_utils.py @@ -11,7 +11,7 @@ import pytest from graphql import build_schema -from vss_tools.utils.graphql_utils import ( +from vss_tools.exporters.s2dm.graphql_utils import ( GraphQLUtilsException, extract_custom_directives_from_schema, load_graphql_schema_from_path, diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index 45a23379..9a4128a9 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -16,9 +16,9 @@ get_metadata_df, print_schema_with_vspec_directives, ) +from vss_tools.exporters.s2dm.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema from vss_tools.exporters.s2dm.type_builders import _sanitize_enum_value_for_graphql from vss_tools.main import get_trees -from vss_tools.utils.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema class TestS2DMExporter: diff --git a/tests/test_s2dm_structs.py b/tests/test_s2dm_structs.py index 449b25e7..546d4cee 100644 --- a/tests/test_s2dm_structs.py +++ b/tests/test_s2dm_structs.py @@ -13,9 +13,9 @@ import pytest from graphql import GraphQLList, GraphQLNonNull, GraphQLObjectType, is_object_type from vss_tools.exporters.s2dm import S2DM_CONVERSIONS, generate_s2dm_schema +from vss_tools.exporters.s2dm.graphql_scalars import VSS_DATATYPE_MAP +from vss_tools.exporters.s2dm.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema from vss_tools.main import get_trees -from vss_tools.utils.graphql_scalars import VSS_DATATYPE_MAP -from vss_tools.utils.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema class TestS2DMStructs: From 9441d162c561db0bb72b66ef45caaa01318f68b9 Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Mon, 9 Feb 2026 23:12:23 +0100 Subject: [PATCH 06/18] feat: Implement progressive qualification for GraphQL type naming in S2DM exporter - Introduced short name collision detection and resolution strategy in the S2DM exporter. - Added support for generating concise GraphQL type names using the last segment of the VSS path. - Implemented progressive qualification for name collisions, allowing for parent and ancestor names to be added as needed. - Updated the CLI options to include a flag for using fully qualified names. - Enhanced the schema generation process to accommodate both short and fully qualified names based on user preference. - Added comprehensive logging and reporting for name collisions, including a detailed YAML report. - Created new tests to validate the functionality of short name collision detection and resolution. Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- docs/s2dm.md | 112 ++++++++++++- src/vss_tools/cli_options.py | 7 + src/vss_tools/exporters/s2dm/__init__.py | 4 +- .../exporters/s2dm/reference_generator.py | 105 +++++++++++++ .../exporters/s2dm/schema_generator.py | 45 +++++- src/vss_tools/exporters/s2dm/type_builders.py | 25 ++- src/vss_tools/utils/pandas_utils.py | 147 ++++++++++++++++++ tests/test_s2dm_exporter.py | 42 +++-- tests/test_s2dm_short_names.py | 76 +++++++++ tests/test_s2dm_structs.py | 26 ++-- 10 files changed, 548 insertions(+), 41 deletions(-) create mode 100644 tests/test_s2dm_short_names.py diff --git a/docs/s2dm.md b/docs/s2dm.md index 9bcfe14b..fdafd4b1 100644 --- a/docs/s2dm.md +++ b/docs/s2dm.md @@ -138,6 +138,108 @@ enum Vehicle_Cabin_Seat_InstanceTag_Dimension2 { This ensures complete traceability between the VSS source and the generated GraphQL schema. +### GraphQL Type Naming: Short Names with Collision Resolution + +**By default**, the S2DM exporter generates **clean, short GraphQL type names** using the last segment of the VSS path: + +- `Vehicle.Cabin.Door.Window` → `type Window` +- `Vehicle.Body.Lights` → `type Lights` +- `Vehicle.Chassis.Axle` → `type Axle` + +This improves readability and follows GraphQL best practices for concise type names. + +#### Progressive Qualification for Name Collisions + +Since VSS does not enforce branch name uniqueness (only Fully Qualified Names are unique), the exporter uses **progressive qualification** when collisions are detected: + +1. **Try short name first**: `Window` +2. **If collision, add parent**: `Door_Window`, `Windshield_Window` +3. **If still collision, add more ancestors**: `Cabin_Door_Window`, `Body_Windshield_Window` +4. **Last resort: use full FQN**: `Vehicle_Cabin_Door_Window` + +**Example from real VSS collisions:** + +```graphql +# Vehicle.Cabin.Door.Shade and Vehicle.Cabin.Sunroof.Shade both have short name "Shade" +# Resolution: qualify with parent name +type Door_Shade @vspec(element: BRANCH, fqn: "Vehicle.Cabin.Door.Shade") { + position: Int +} + +type Sunroof_Shade @vspec(element: BRANCH, fqn: "Vehicle.Cabin.Sunroof.Shade") { + position: Int +} +``` + +**Console Output:** During export, collision statistics are logged: + +``` +[INFO] Short name collision resolution: + ✓ 245 types use short names (no collisions) + ⚠ 8 types qualified with parent (e.g., Parent_Name) + ⚠ 2 types qualified with multiple ancestors (e.g., GrandParent_Parent_Name) + → See vspec_reference/short_name_collisions.yaml for 5 collision groups +``` + +**Collision Report:** The `vspec_reference/short_name_collisions.yaml` file documents all name collisions and resolutions in a compact format: + +```yaml +# Short Name Collision Resolution Report +# +# This file documents how VSS branch names were converted to GraphQL type names. +# Resolution strategy: Progressive parent qualification +# 1. Try short name (e.g., 'Window') +# 2. If collision: add parent (e.g., 'Door_Window', 'Windshield_Window') +# 3. If still collision: add more ancestors (e.g., 'Cabin_Door_Window') +# 4. Last resort: use full FQN with underscores + +summary: + total_branches: 137 + no_collisions: 120 + parent_qualified: 15 + multi_ancestor_qualified: 2 + full_fqn_fallback: 0 + +# Collisions resolved by adding immediate parent name (e.g., Parent_Name) +parent_qualified: + - { fqn: Vehicle.Body.Windshield.Shade, type: Windshield_Shade } + - { fqn: Vehicle.Cabin.Door.Shade, type: Door_Shade } + - { fqn: Vehicle.Cabin.Sunroof.Shade, type: Sunroof_Shade } + +# Collisions resolved by adding multiple ancestor names (e.g., GrandParent_Parent_Name) +multi_ancestor_qualified: + - { fqn: Vehicle.Powertrain.Engine.Brake, type: Engine_Brake } + - { fqn: Vehicle.Chassis.Axle.Wheel.Brake, type: Axle_Wheel_Brake } + +# Detailed collision groups: branches that share the same short name +collision_groups: + Axle: # 4 branches share this name + - { fqn: Vehicle.Chassis.Axle, type: Chassis_Axle } + - { fqn: Vehicle.Trailer.Axle, type: Trailer_Axle } + - { fqn: Vehicle.Chassis.Axle.Wheel.Axle, type: Wheel_Axle } + - { fqn: Vehicle.Body.Hood.Axle, type: Hood_Axle } + Shade: # 2 branches share this name + - { fqn: Vehicle.Cabin.Door.Shade, type: Door_Shade } + - { fqn: Vehicle.Cabin.Sunroof.Shade, type: Sunroof_Shade } +``` + +#### Opting Out: Full FQN Names + +To use traditional fully-qualified names with underscores (legacy behavior), use the `--fqn-type-names` flag: + +```bash +vspec export s2dm --vspec spec.vspec --output myOutput/ --fqn-type-names +``` + +This generates: +- `Vehicle.Cabin.Door.Window` → `type Vehicle_Cabin_Door_Window` +- `Vehicle.Body.Lights` → `type Vehicle_Body_Lights` + +**When to use `--fqn-type-names`:** +- You need backward compatibility with existing schemas +- Your tooling expects the old naming convention +- You prefer explicit full paths over short names + ### GraphQL Naming Convention Warnings GraphQL best practices recommend using **singular names for types** (e.g., `User` not `Users`, `Product` not `Products`). The S2DM exporter automatically detects VSS branches with plural names and generates warnings to help identify potential naming convention violations. @@ -152,11 +254,11 @@ GraphQL best practices recommend using **singular names for types** (e.g., `User Vehicle.Cabin.Lights: singular: Light - currentNameInGraphQLModel: Vehicle_Cabin_Lights + currentNameInGraphQLModel: Lights # or Vehicle_Cabin_Lights if using --fqn-type-names Vehicle.Body.Mirrors: singular: Mirror - currentNameInGraphQLModel: Vehicle_Body_Mirrors + currentNameInGraphQLModel: Mirrors # or Vehicle_Body_Mirrors if using --fqn-type-names # Whitelisted words (excluded from plural detection): whitelisted_non_plurals: @@ -169,7 +271,7 @@ GraphQL best practices recommend using **singular names for types** (e.g., `User 2. **Console warnings:** During export, warnings are logged for immediate visibility: ``` - WARNING: Type 'Vehicle_Cabin_Lights' uses potential plural name 'Lights'. + WARNING: Type 'Lights' uses potential plural name 'Lights'. Suggested singular: 'Light' (VSS FQN: Vehicle.Cabin.Lights) ``` @@ -252,6 +354,7 @@ myOutput/ ├── vspec_lookup_spec.yaml # Complete VSS tree (fully expanded) ├── vspec_units.yaml # Units used (if provided via -u or implicit) ├── vspec_quantities.yaml # Quantities used (if provided via -q or implicit) + ├── short_name_collisions.yaml # Name collision resolutions (if any detected) ├── plural_type_warnings.yaml # Plural type names detected (if any) └── pluralized_field_names.yaml # Fields changed to plural form (if any instances) ``` @@ -264,6 +367,7 @@ The `vspec_reference/` directory provides complete traceability: - **vspec_lookup_spec.yaml** - Complete VSS specification tree (fully processed and expanded) in YAML format - **vspec_units.yaml** - Unit definitions used during generation (included if units were provided via `-u` flag or implicitly loaded) - **vspec_quantities.yaml** - Quantity definitions used during generation (included if quantities were provided via `-q` flag or implicitly loaded) +- **short_name_collisions.yaml** - Branch name collisions and progressive qualification resolutions (generated if collisions detected) - **plural_type_warnings.yaml** - VSS branches with plural type names that may violate GraphQL naming conventions (generated if any detected) - **pluralized_field_names.yaml** - Fields whose names were changed to plural form for list fields (generated if any instances exist) @@ -314,6 +418,8 @@ enum VelocityUnitEnum @vspec(element: QUANTITY_KIND, metadata: [{key: "quantity" } ``` +**Note:** Type names use short names by default (`Vehicle` instead of `Vehicle`). For types deeper in the tree like `Vehicle.Cabin.Door.Window`, the short name `Window` is used unless a collision is detected. Use `--fqn-type-names` to get fully-qualified names like `Vehicle_Cabin_Door_Window`. + ## Output Structures The S2DM exporter supports three output modes. All modes generate a `vspec_reference/` directory for traceability. diff --git a/src/vss_tools/cli_options.py b/src/vss_tools/cli_options.py index 41e54663..c2d77f48 100644 --- a/src/vss_tools/cli_options.py +++ b/src/vss_tools/cli_options.py @@ -195,3 +195,10 @@ def validate_attribute(value): default=True, # Default to flat for simplicity show_default=True, ) + +fqn_type_names_opt = option( + "--fqn-type-names/--short-type-names", + help="Use fully qualified names for GraphQL types (default: use short names with collision resolution).", + default=False, + show_default=True, +) diff --git a/src/vss_tools/exporters/s2dm/__init__.py b/src/vss_tools/exporters/s2dm/__init__.py index cdac0799..b101d832 100644 --- a/src/vss_tools/exporters/s2dm/__init__.py +++ b/src/vss_tools/exporters/s2dm/__init__.py @@ -46,6 +46,7 @@ @clo.types_opt @clo.modular_opt @clo.flat_domains_opt +@clo.fqn_type_names_opt @clo.strict_exceptions_opt def cli( vspec: Path, @@ -60,6 +61,7 @@ def cli( types: tuple[Path, ...], modular: bool, flat_domains: bool, + fqn_type_names: bool, strict_exceptions: Path | None, ) -> None: """ @@ -110,7 +112,7 @@ def cli( # Generate the schema schema, unit_enums_metadata, allowed_enums_metadata, mapping_metadata = generate_s2dm_schema( - tree, data_type_tree, extended_attributes=extended_attributes + tree, data_type_tree, extended_attributes=extended_attributes, use_short_names=not fqn_type_names ) if modular: diff --git a/src/vss_tools/exporters/s2dm/reference_generator.py b/src/vss_tools/exporters/s2dm/reference_generator.py index 9c5ea8e8..0216e9fe 100644 --- a/src/vss_tools/exporters/s2dm/reference_generator.py +++ b/src/vss_tools/exporters/s2dm/reference_generator.py @@ -178,6 +178,111 @@ def generate_vspec_reference( except (PermissionError, OSError) as e: raise S2DMExporterException(f"Failed to write plural warnings file {warnings_output}: {e}") from e + # Write short name collisions only if short names were actually used + # (short_name_mapping will be None when --fqn-type-names is used) + if mapping_metadata and mapping_metadata.get("short_name_mapping") is not None: + collisions_output = reference_dir / "short_name_collisions.yaml" + try: + with open(collisions_output, "w") as f: + # Write header comment + f.write("# Short Name Collision Resolution Report\n") + f.write("#\n") + f.write("# This file documents how VSS branch names were converted to GraphQL type names.\n") + f.write("# Resolution strategy: Progressive parent qualification\n") + f.write("# 1. Try short name (e.g., 'Window')\n") + f.write("# 2. If collision: add parent (e.g., 'Door_Window', 'Windshield_Window')\n") + f.write("# 3. If still collision: add more ancestors (e.g., 'Cabin_Door_Window')\n") + f.write("# 4. Last resort: use full FQN with underscores\n\n") + + collision_list = mapping_metadata.get("short_name_collisions", []) + name_mapping = mapping_metadata.get("short_name_mapping", {}) + stats = mapping_metadata.get("short_name_stats", {}) + + # Organize FQNs by resolution strategy + parent_qualified_fqns = [] + multi_qualified_fqns = [] + full_fqn_fqns = [] + + for fqn, assigned_name in name_mapping.items(): + fqn_parts = fqn.split(".") + short_name = fqn_parts[-1] + + if assigned_name == short_name: + # No collision - skip (we'll only report collisions) + continue + elif assigned_name == "_".join(fqn_parts): + # Full FQN used + full_fqn_fqns.append(fqn) + else: + # Qualified name - check depth + assigned_parts = assigned_name.split("_") + if len(assigned_parts) == 2: + # Parent-qualified (e.g., Parent_Name) + parent_qualified_fqns.append(fqn) + else: + # Multi-ancestor qualified + multi_qualified_fqns.append(fqn) + + # Write summary section + f.write("summary:\n") + f.write(f" total_branches: {len(name_mapping)}\n") + f.write(f" no_collisions: {stats.get('no_collision', 0)}\n") + f.write(f" parent_qualified: {stats.get('parent_qualified', 0)}\n") + f.write(f" multi_ancestor_qualified: {stats.get('multi_parent_qualified', 0)}\n") + f.write(f" full_fqn_fallback: {stats.get('full_fqn', 0)}\n\n") + + # Section 1: Parent-qualified names + if parent_qualified_fqns: + f.write("# Collisions resolved by adding immediate parent name (e.g., Parent_Name)\n") + f.write("parent_qualified:\n") + for fqn in sorted(parent_qualified_fqns): + assigned_name = name_mapping[fqn] + f.write(f" - {{ fqn: {fqn}, type: {assigned_name} }}\n") + f.write("\n") + + # Section 2: Multi-ancestor qualified names + if multi_qualified_fqns: + f.write( + "# Collisions resolved by adding multiple ancestor names (e.g., GrandParent_Parent_Name)\n" + ) + f.write("multi_ancestor_qualified:\n") + for fqn in sorted(multi_qualified_fqns): + assigned_name = name_mapping[fqn] + f.write(f" - {{ fqn: {fqn}, type: {assigned_name} }}\n") + f.write("\n") + + # Section 3: Full FQN names (last resort) + if full_fqn_fqns: + f.write("# Even with qualification, collisions persisted - using full FQN\n") + f.write("full_fqn_fallback:\n") + for fqn in sorted(full_fqn_fqns): + assigned_name = name_mapping[fqn] + f.write(f" - {{ fqn: {fqn}, type: {assigned_name} }}\n") + f.write("\n") + + # Section 4: Collision groups (detailed view by short name) + if collision_list: + f.write("# Detailed collision groups: branches that share the same short name\n") + f.write("collision_groups:\n") + for collision in sorted(collision_list, key=lambda x: x["short_name"]): + short_name = collision["short_name"] + f.write(f" {short_name}: # {collision['collision_count']} branches share this name\n") + for fqn in sorted(collision["fqns"]): + assigned_name = collision["assigned_names"][fqn] + f.write(f" - {{ fqn: {fqn}, type: {assigned_name} }}\n") + f.write("\n") + + total_count = len(name_mapping) if name_mapping else 0 + collision_count = len(collision_list) if collision_list else 0 + log.info( + f" - Short name resolution: {collisions_output.name} ({total_count} branches, " + + f"{collision_count} collision groups)" + ) + except (PermissionError, OSError) as e: + raise S2DMExporterException( + f"Failed to write short name collisions file {collisions_output}: {e}" + ) from e + # Write pluralized field names if any were collected if mapping_metadata and mapping_metadata.get("pluralized_field_names"): pluralized_output = reference_dir / "pluralized_field_names.yaml" diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index e2382f4d..f761196e 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -22,7 +22,7 @@ from graphql import GraphQLField, GraphQLObjectType, GraphQLSchema, GraphQLString from vss_tools.tree import VSSNode -from vss_tools.utils.pandas_utils import get_metadata_df +from vss_tools.utils.pandas_utils import detect_and_resolve_short_name_collisions, get_metadata_df from .constants import CUSTOM_DIRECTIVES, S2DMExporterException from .graphql_directive_processor import GraphQLDirectiveProcessor @@ -55,6 +55,7 @@ def generate_s2dm_schema( tree: VSSNode, data_type_tree: VSSNode | None = None, extended_attributes: tuple[str, ...] = (), + use_short_names: bool = True, ) -> tuple[ GraphQLSchema, dict[str, dict[str, dict[str, str]]], @@ -66,14 +67,17 @@ def generate_s2dm_schema( Orchestrates the complete schema generation process: 1. Extract metadata from VSS tree - 2. Create unit enums, instance types, allowed value enums, struct types - 3. Create object types for all branches - 4. Assemble complete GraphQL schema + 2. Optionally detect and resolve short name collisions + 3. Create unit enums, instance types, allowed value enums, struct types + 4. Create object types for all branches + 5. Assemble complete GraphQL schema Args: tree: Main VSS tree with vehicle signals data_type_tree: Optional user-defined struct types extended_attributes: Extended attribute names from CLI flags + use_short_names: If True, use short names with collision resolution (default: True). + If False, use fully qualified names with underscores. Returns: Tuple of (schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments) @@ -85,13 +89,33 @@ def generate_s2dm_schema( branches_df, leaves_df = get_metadata_df(tree, extended_attributes=extended_attributes) vspec_comments = init_vspec_comments() + # Detect and resolve short name collisions if requested + short_name_mapping: dict[str, str] | None = None + if use_short_names: + short_name_mapping, collision_warnings, collision_stats = detect_and_resolve_short_name_collisions( + branches_df + ) + vspec_comments["short_name_mapping"] = short_name_mapping + vspec_comments["short_name_collisions"] = collision_warnings + vspec_comments["short_name_stats"] = collision_stats + else: + # When using FQN names, store empty mapping to indicate FQN mode + vspec_comments["short_name_mapping"] = None + vspec_comments["short_name_collisions"] = [] + vspec_comments["short_name_stats"] = {} + # Create all types in logical order unit_enums, unit_metadata = create_unit_enums() - instance_types = create_instance_types(branches_df, vspec_comments) + instance_types = create_instance_types(branches_df, vspec_comments, short_name_mapping) allowed_enums, allowed_metadata = create_allowed_enums(leaves_df) # Create struct types from data type tree - struct_types = create_struct_types(data_type_tree, vspec_comments, extended_attributes=extended_attributes) + struct_types = create_struct_types( + data_type_tree, + vspec_comments, + extended_attributes=extended_attributes, + short_name_mapping=short_name_mapping, + ) # Combine all types types_registry = {**instance_types, **allowed_enums, **struct_types} @@ -100,7 +124,14 @@ def generate_s2dm_schema( for fqn in branches_df.index: if fqn not in types_registry: types_registry[fqn] = create_object_type( - fqn, branches_df, leaves_df, types_registry, unit_enums, vspec_comments, extended_attributes + fqn, + branches_df, + leaves_df, + types_registry, + unit_enums, + vspec_comments, + extended_attributes, + short_name_mapping, ) # Assemble complete schema diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index 72bc4522..c6322a96 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -153,7 +153,7 @@ def _get_quantity_units() -> dict[str, dict[str, dict[str, str]]]: def create_instance_types( - branches_df: pd.DataFrame, vspec_comments: dict[str, Any] + branches_df: pd.DataFrame, vspec_comments: dict[str, Any], short_name_mapping: dict[str, str] | None = None ) -> dict[str, GraphQLEnumType | GraphQLObjectType]: """ Create GraphQL types for VSS instance-based branches. @@ -161,6 +161,7 @@ def create_instance_types( Args: branches_df: VSS branch node metadata vspec_comments: Dictionary to store instance tag metadata + short_name_mapping: Optional mapping from FQN to short type names Returns: Mapping of type names to GraphQL enum or object types @@ -170,7 +171,11 @@ def create_instance_types( for fqn, row in branches_df[branches_df["instances"].notna()].iterrows(): if instances := row.get("instances"): - base_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) + # Use short name if mapping exists, otherwise fall back to FQN conversion + if short_name_mapping and fqn in short_name_mapping: + base_name = short_name_mapping[fqn] + else: + base_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) tag_name = f"{base_name}_InstanceTag" dimensions = _parse_instances_simple(instances) @@ -333,6 +338,7 @@ def create_struct_types( data_type_tree: VSSNode | None, vspec_comments: dict[str, Any], extended_attributes: tuple[str, ...] = (), + short_name_mapping: dict[str, str] | None = None, ) -> dict[str, GraphQLObjectType]: """ Convert VSS struct definitions to GraphQL object types. @@ -341,6 +347,7 @@ def create_struct_types( data_type_tree: VSS tree containing user-defined struct types vspec_comments: Dictionary to store struct metadata extended_attributes: Extended attribute names from CLI + short_name_mapping: Optional mapping from FQN to short type names Returns: Dictionary mapping struct type names to GraphQL object types @@ -353,7 +360,11 @@ def create_struct_types( struct_nodes = branches_df[branches_df["type"] == "struct"] for fqn, struct_row in struct_nodes.iterrows(): - type_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) + # Use short name if mapping exists, otherwise fall back to FQN conversion + if short_name_mapping and fqn in short_name_mapping: + type_name = short_name_mapping[fqn] + else: + type_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) properties = leaves_df[leaves_df["parent"] == fqn] fields = {} @@ -433,6 +444,7 @@ def create_object_type( unit_enums: dict[str, GraphQLEnumType], vspec_comments: dict[str, Any], extended_attributes: tuple[str, ...] = (), + short_name_mapping: dict[str, str] | None = None, ) -> GraphQLObjectType: """ Create GraphQL object type for a VSS branch. @@ -448,13 +460,18 @@ def create_object_type( unit_enums: Unit enum types vspec_comments: Metadata tracking dictionary extended_attributes: Extended attribute names from CLI + short_name_mapping: Optional mapping from FQN to short type names Returns: GraphQL object type for the branch """ branch_row = branches_df.loc[fqn] original_name = branch_row["name"] - type_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) + # Use short name if mapping exists, otherwise fall back to FQN conversion + if short_name_mapping and fqn in short_name_mapping: + type_name = short_name_mapping[fqn] + else: + type_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) # Check if branch name is plural and collect for reporting # Pass original VSS branch name (before any conversion) diff --git a/src/vss_tools/utils/pandas_utils.py b/src/vss_tools/utils/pandas_utils.py index 054f16ba..074b472c 100644 --- a/src/vss_tools/utils/pandas_utils.py +++ b/src/vss_tools/utils/pandas_utils.py @@ -92,3 +92,150 @@ def get_metadata_df(root: VSSNode, extended_attributes: tuple[str, ...] = ()) -> log.debug(f" Extended attributes found: {', '.join(sorted(found_extended_attrs))}") return branches_df, leaves_df + + +def detect_and_resolve_short_name_collisions( + branches_df: pd.DataFrame, +) -> tuple[dict[str, str], list[dict[str, any]], dict[str, int]]: + """ + Detect name collisions in branch names and resolve using progressive parent qualification. + + Uses a progressive qualification strategy: + 1. Try short name (e.g., "Window") + 2. If collision, try parent.name (e.g., "Door_Window") + 3. If still collision, try grandparent.parent.name, etc. + 4. As last resort, use full FQN with underscores + + Args: + branches_df: DataFrame with branch metadata (must have 'name' and 'parent' columns, FQN as index) + + Returns: + tuple: (fqn_to_short_name_mapping, collision_warnings, statistics) + - fqn_to_short_name_mapping: Dict mapping FQN to assigned GraphQL type name + - collision_warnings: List of collision details for reporting + - statistics: Dict with counts for each resolution strategy + + Example: + >>> df = pd.DataFrame({ + ... 'name': ['Window', 'Window', 'Seat'], + ... 'parent': ['Vehicle.Cabin.Door', 'Vehicle.Body.Windshield', 'Vehicle.Cabin'] + ... }, index=['Vehicle.Cabin.Door.Window', 'Vehicle.Body.Windshield.Window', 'Vehicle.Cabin.Seat']) + >>> mapping, warnings, stats = detect_and_resolve_short_name_collisions(df) + >>> mapping + {'Vehicle.Cabin.Door.Window': 'Door_Window', 'Vehicle.Body.Windshield.Window': 'Windshield_Window', ...} + """ + from collections import defaultdict + + # Sort by FQN for deterministic processing + sorted_df = branches_df.sort_index() + + # Track assigned names to detect collisions + assigned_names: dict[str, str] = {} # short_name -> fqn (currently using this name) + fqn_to_short_name: dict[str, str] = {} # fqn -> assigned short name + collision_groups: dict[str, list[str]] = defaultdict(list) # short_name -> list of colliding FQNs + + # First pass: detect collisions at the short name level + short_name_groups = sorted_df.groupby("name") + for short_name, group in short_name_groups: + fqns = list(group.index) + if len(fqns) > 1: + collision_groups[short_name] = fqns + + # Track statistics + stats = { + "no_collision": 0, # Clean short names + "parent_qualified": 0, # Needed parent.name + "multi_parent_qualified": 0, # Needed grandparent.parent.name or deeper + "full_fqn": 0, # Had to use full FQN + } + + # Second pass: assign names using progressive qualification + for fqn in sorted_df.index: + short_name = sorted_df.loc[fqn, "name"] + + # No collision - use short name directly + if short_name not in collision_groups: + fqn_to_short_name[fqn] = short_name + assigned_names[short_name] = fqn + stats["no_collision"] += 1 + continue + + # Collision detected - try progressive qualification + assigned_name = _resolve_collision_with_qualification( + fqn, short_name, sorted_df.loc[fqn, "parent"], assigned_names, stats + ) + fqn_to_short_name[fqn] = assigned_name + assigned_names[assigned_name] = fqn + + # Build collision warnings for reporting + collision_warnings = [] + for short_name, fqns in collision_groups.items(): + warning = { + "short_name": short_name, + "collision_count": len(fqns), + "fqns": fqns, + "assigned_names": {fqn: fqn_to_short_name[fqn] for fqn in fqns}, + } + collision_warnings.append(warning) + + # Log statistics + len(sorted_df) + log.info("Short name collision resolution:") + log.info(f" ✓ {stats['no_collision']} types use short names (no collisions)") + if stats["parent_qualified"] > 0: + log.info(f" ⚠ {stats['parent_qualified']} types qualified with parent (e.g., Parent_Name)") + if stats["multi_parent_qualified"] > 0: + log.info(f" ⚠ {stats['multi_parent_qualified']} types qualified with multiple nested parents") + if stats["full_fqn"] > 0: + log.warning(f" ⚠ {stats['full_fqn']} types use full FQN due to deep collisions") + + if collision_warnings: + log.info(f" → See vspec_reference/short_name_collisions.yaml for {len(collision_warnings)} collision groups") + + # Return mapping, warnings, and statistics for detailed reporting + return fqn_to_short_name, collision_warnings, stats + + +def _resolve_collision_with_qualification( + fqn: str, short_name: str, parent_fqn: str, assigned_names: dict[str, str], stats: dict[str, int] +) -> str: + """ + Resolve a name collision by progressively adding parent qualifiers. + + Tries: parent.name, grandparent.parent.name, ..., full FQN + + Args: + fqn: The fully qualified name to resolve + short_name: The base short name (last segment of FQN) + parent_fqn: The parent's FQN + assigned_names: Dict of already assigned names (to detect further collisions) + stats: Statistics dict to update + + Returns: + The assigned qualified name + """ + # Build list of parent segments for progressive qualification + fqn_parts = fqn.split(".") + if len(fqn_parts) == 1: + # Root node, no qualification possible - use as-is (shouldn't happen in collision scenario) + stats["no_collision"] += 1 + return short_name + + # Try progressive qualification: parent.name, grandparent.parent.name, etc. + for depth in range(1, len(fqn_parts)): + # Take last 'depth + 1' segments (depth parents + name) + qualified_parts = fqn_parts[-(depth + 1) :] + candidate_name = "_".join(qualified_parts) + + # Check if this qualified name is available + if candidate_name not in assigned_names: + if depth == 1: + stats["parent_qualified"] += 1 + else: + stats["multi_parent_qualified"] += 1 + return candidate_name + + # All qualified names taken - use full FQN as last resort + full_fqn_name = "_".join(fqn_parts) + stats["full_fqn"] += 1 + return full_fqn_name diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index 9a4128a9..bd25ee3b 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -84,7 +84,7 @@ def test_generate_s2dm_schema_basic_structure(self): expand=False, ) - schema, _, _, _ = generate_s2dm_schema(tree) + schema, _, _, _ = generate_s2dm_schema(tree, use_short_names=False) # Check that schema is valid assert schema is not None @@ -123,7 +123,7 @@ def test_schema_can_be_printed(self): expand=False, ) - schema, _, _, _ = generate_s2dm_schema(tree) + schema, _, _, _ = generate_s2dm_schema(tree, use_short_names=False) schema_str = print_schema(schema) # Check that output contains expected elements @@ -151,7 +151,7 @@ def test_unit_enums_generation(self): expand=False, ) - schema, _, _, _ = generate_s2dm_schema(tree) + schema, _, _, _ = generate_s2dm_schema(tree, use_short_names=False) schema_str = print_schema(schema) # Check that unit enums are generated @@ -184,7 +184,9 @@ def test_vspec_comment_directives(self): expand=False, ) - schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema( + tree, use_short_names=False + ) schema_str = print_schema_with_vspec_directives( schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments ) @@ -222,7 +224,9 @@ def test_range_and_deprecation_directives(self): expand=False, ) - schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema( + tree, use_short_names=False + ) sdl = print_schema_with_vspec_directives(schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments) # Test @deprecated directive for massage field @@ -268,7 +272,9 @@ def test_instance_tag_support(self): expand=False, ) - schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema( + tree, use_short_names=False + ) sdl = print_schema_with_vspec_directives(schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments) # Test that instance tag type is created @@ -330,7 +336,7 @@ def test_allowed_value_enums_generation(self): ) # Generate schema - schema, _, _, _ = generate_s2dm_schema(tree) + schema, _, _, _ = generate_s2dm_schema(tree, use_short_names=False) schema_sdl = print_schema(schema) # Check that allowed value enums were generated @@ -380,7 +386,9 @@ def test_modular_export_flat_domains(self, tmp_path): ) # Generate schema - schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema( + tree, use_short_names=False + ) # Test modular export with flat domains (default) output_dir = tmp_path / "modular_flat" @@ -427,7 +435,9 @@ def test_modular_export_nested_domains(self, tmp_path): ) # Generate schema - schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema( + tree, use_short_names=False + ) # Test modular export with nested domains output_dir = tmp_path / "modular_nested" @@ -476,7 +486,9 @@ def test_modular_export_instance_enum_directives(self, tmp_path): ) # Generate schema - schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema( + tree, use_short_names=False + ) # Test modular export with flat domains output_dir = tmp_path / "modular_instance_test" @@ -512,7 +524,7 @@ def test_non_instantiated_property_hoisting(self, tmp_path: Path): ) # Generate schema - schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=False) schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) # Verify that Vehicle_Cabin_Door type doesn't have someSignal @@ -565,7 +577,7 @@ def test_enum_sanitization_in_schema_generation(self): expand=False, ) - schema, _, allowed_metadata, _ = generate_s2dm_schema(tree) + schema, _, allowed_metadata, _ = generate_s2dm_schema(tree, use_short_names=False) # Check that schema is valid assert schema is not None @@ -604,7 +616,7 @@ def test_enum_sanitization_schema_output_with_directives(self): expand=False, ) - schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=False) schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) # Check that enum type has @vspec directive with element @@ -679,7 +691,7 @@ def test_camelcase_enums_schema_generation(self): expand=False, ) - schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=False) schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) # Check Component.Type enum with AbCd @@ -719,7 +731,7 @@ def test_instance_dimension_enum_sanitization(self): expand=False, ) - schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree) + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=False) schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) # Check that Row instance enum is created and values are sanitized diff --git a/tests/test_s2dm_short_names.py b/tests/test_s2dm_short_names.py new file mode 100644 index 00000000..e584aa8d --- /dev/null +++ b/tests/test_s2dm_short_names.py @@ -0,0 +1,76 @@ +# Copyright (c) 2025 Contributors to COVESA +# +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 + +"""Tests for S2DM short name collision detection and resolution. + +These tests are based on real-world collision scenarios documented in: +https://github.com/COVESA/vehicle_signal_specification/issues/790 +""" + +from pathlib import Path + +from vss_tools.exporters.s2dm import generate_s2dm_schema, print_schema_with_vspec_directives +from vss_tools.main import get_trees + + +class TestS2DMShortNames: + """Test short name collision detection and resolution in S2DM exporter.""" + + def test_basic_short_name_no_collision(self): + """Test that unique branch names use short names without collision.""" + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/example_seat.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + # Generate schema with short names enabled (default) + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=True) + schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) + + # Should use short names for unique branches + assert "type Vehicle @vspec" in schema_str + assert "type Cabin @vspec" in schema_str + assert "type Seat @vspec" in schema_str + + # Should NOT use FQN-style names + assert "type Vehicle_Cabin @vspec" not in schema_str + assert "type Vehicle_Cabin_Seat @vspec" not in schema_str + + def test_fqn_type_names_flag(self): + """Test that use_short_names=False uses full FQN instead of short names.""" + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/example_seat.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + # Generate schema with short names disabled (use_short_names=False) + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=False) + schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) + + # Should use FQN-style names + assert "type Vehicle @vspec" in schema_str + assert "type Vehicle_Cabin @vspec" in schema_str + assert "type Vehicle_Cabin_Seat @vspec" in schema_str + + # Short name collision detection should be skipped + assert vspec_comments["short_name_mapping"] is None + assert vspec_comments["short_name_collisions"] == [] diff --git a/tests/test_s2dm_structs.py b/tests/test_s2dm_structs.py index 546d4cee..faba18c9 100644 --- a/tests/test_s2dm_structs.py +++ b/tests/test_s2dm_structs.py @@ -48,7 +48,7 @@ def test_data_type_tree_is_captured(self, struct_trees): def test_struct_types_are_created(self, struct_trees): """Test that struct types are converted to GraphQL object types.""" tree, data_type_tree = struct_trees - schema, _, _, vspec_comments = generate_s2dm_schema(tree, data_type_tree) + schema, _, _, vspec_comments = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) # Check that struct types exist in the schema nested_struct_name = convert_name_for_graphql_schema( @@ -72,7 +72,7 @@ def test_struct_types_are_created(self, struct_trees): def test_struct_properties_are_non_null(self, struct_trees): """Test that all struct properties are non-null fields.""" tree, data_type_tree = struct_trees - schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree) + schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) # Get NestedStruct type nested_struct_name = convert_name_for_graphql_schema( @@ -88,7 +88,7 @@ def test_struct_properties_are_non_null(self, struct_trees): def test_struct_properties_have_correct_types(self, struct_trees): """Test that struct properties map to correct GraphQL types.""" tree, data_type_tree = struct_trees - schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree) + schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) # Get NestedStruct type nested_struct_name = convert_name_for_graphql_schema( @@ -115,7 +115,7 @@ def test_struct_properties_have_correct_types(self, struct_trees): def test_nested_struct_references(self, struct_trees): """Test that structs can reference other structs.""" tree, data_type_tree = struct_trees - schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree) + schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) # Get ParentStruct type parent_struct_name = convert_name_for_graphql_schema( @@ -147,7 +147,7 @@ def test_nested_struct_references(self, struct_trees): def test_struct_array_properties(self, struct_trees): """Test that struct array properties are correctly handled.""" tree, data_type_tree = struct_trees - schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree) + schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) # Get ParentStruct type parent_struct_name = convert_name_for_graphql_schema( @@ -181,7 +181,7 @@ def test_struct_array_properties(self, struct_trees): def test_signal_with_struct_datatype(self, struct_trees): """Test that signals can use struct datatypes.""" tree, data_type_tree = struct_trees - schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree) + schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) # Get the TestRoot type root_type_name = convert_name_for_graphql_schema("TestRoot", GraphQLElementType.TYPE, S2DM_CONVERSIONS) @@ -212,7 +212,7 @@ def test_signal_with_struct_datatype(self, struct_trees): def test_struct_metadata_and_comments(self, struct_trees): """Test that struct metadata and comments are preserved.""" tree, data_type_tree = struct_trees - schema, _, _, vspec_comments = generate_s2dm_schema(tree, data_type_tree) + schema, _, _, vspec_comments = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) # Get struct type name nested_struct_name = convert_name_for_graphql_schema( @@ -231,7 +231,7 @@ def test_struct_metadata_and_comments(self, struct_trees): def test_struct_property_range_constraints(self, struct_trees): """Test that range constraints on struct properties are preserved.""" tree, data_type_tree = struct_trees - schema, _, _, vspec_comments = generate_s2dm_schema(tree, data_type_tree) + schema, _, _, vspec_comments = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) # Get struct type name nested_struct_name = convert_name_for_graphql_schema( @@ -252,7 +252,7 @@ def test_struct_property_range_constraints(self, struct_trees): def test_primitive_property_in_struct(self, struct_trees): """Test that primitive (non-struct) properties work correctly.""" tree, data_type_tree = struct_trees - schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree) + schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) # Get ParentStruct type parent_struct_name = convert_name_for_graphql_schema( @@ -292,7 +292,9 @@ def test_modular_output_structs_in_separate_folder(self, tmp_path): ) # Generate schema - schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, data_type_tree) + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema( + tree, data_type_tree, use_short_names=False + ) # Write modular output output_dir = tmp_path / "modular_output" @@ -340,7 +342,9 @@ def test_modular_output_regular_types_not_in_structs_folder(self, tmp_path): ) # Generate schema - schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, data_type_tree) + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema( + tree, data_type_tree, use_short_names=False + ) # Write modular output output_dir = tmp_path / "modular_output" From 957baa206c55751e3e652f7ea1323f9a623a26be Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Thu, 12 Feb 2026 10:27:48 +0100 Subject: [PATCH 07/18] feat(s2dm): enhance name collision detection including tree and structs Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- .../exporters/s2dm/schema_generator.py | 10 ++- src/vss_tools/utils/pandas_utils.py | 42 +++++++++---- tests/test_s2dm_short_names.py | 62 +++++++++++++++++++ tests/test_s2dm_structs.py | 2 + .../test.vspec | 46 ++++++++++++++ .../types.vspec | 48 ++++++++++++++ 6 files changed, 198 insertions(+), 12 deletions(-) create mode 100644 tests/vspec/test_s2dm_cross_tree_collisions/test.vspec create mode 100644 tests/vspec/test_s2dm_cross_tree_collisions/types.vspec diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index f761196e..c50e5403 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -19,6 +19,7 @@ from pathlib import Path from typing import Any +import pandas as pd from graphql import GraphQLField, GraphQLObjectType, GraphQLSchema, GraphQLString from vss_tools.tree import VSSNode @@ -89,11 +90,18 @@ def generate_s2dm_schema( branches_df, leaves_df = get_metadata_df(tree, extended_attributes=extended_attributes) vspec_comments = init_vspec_comments() + # Combine branches from both main tree and data type tree for joint collision detection + # since they share the same GraphQL type namespace + combined_branches_df = branches_df + if data_type_tree: + struct_branches_df, _ = get_metadata_df(data_type_tree, extended_attributes=extended_attributes) + combined_branches_df = pd.concat([branches_df, struct_branches_df], axis=0, verify_integrity=True) + # Detect and resolve short name collisions if requested short_name_mapping: dict[str, str] | None = None if use_short_names: short_name_mapping, collision_warnings, collision_stats = detect_and_resolve_short_name_collisions( - branches_df + combined_branches_df ) vspec_comments["short_name_mapping"] = short_name_mapping vspec_comments["short_name_collisions"] = collision_warnings diff --git a/src/vss_tools/utils/pandas_utils.py b/src/vss_tools/utils/pandas_utils.py index 074b472c..799a676a 100644 --- a/src/vss_tools/utils/pandas_utils.py +++ b/src/vss_tools/utils/pandas_utils.py @@ -4,6 +4,8 @@ from __future__ import annotations +from typing import Any + import pandas as pd from anytree import PreOrderIter @@ -96,7 +98,7 @@ def get_metadata_df(root: VSSNode, extended_attributes: tuple[str, ...] = ()) -> def detect_and_resolve_short_name_collisions( branches_df: pd.DataFrame, -) -> tuple[dict[str, str], list[dict[str, any]], dict[str, int]]: +) -> tuple[dict[str, str], list[dict[str, Any]], dict[str, int]]: """ Detect name collisions in branch names and resolve using progressive parent qualification. @@ -150,19 +152,37 @@ def detect_and_resolve_short_name_collisions( } # Second pass: assign names using progressive qualification - for fqn in sorted_df.index: - short_name = sorted_df.loc[fqn, "name"] + # For branches with collisions, process by depth (shallowest first) to prioritize root-level branches + # For branches without collisions, process alphabetically + + # Separate colliding and non-colliding branches + colliding_fqns = set() + for fqns in collision_groups.values(): + colliding_fqns.update(fqns) + + non_colliding_df = sorted_df[~sorted_df.index.isin(colliding_fqns)] + colliding_df = sorted_df[sorted_df.index.isin(colliding_fqns)].copy() + + # Add depth column for colliding branches (count dots in FQN) + colliding_df["depth"] = colliding_df.index.str.count(r"\.") + + # Sort colliding branches by depth first (shallowest gets priority), then alphabetically + colliding_df = colliding_df.sort_values(["depth", colliding_df.index.name or "fqn"]) + + # Process non-colliding branches first (they get clean short names) + for fqn in non_colliding_df.index: + short_name = non_colliding_df.loc[fqn, "name"] + fqn_to_short_name[fqn] = short_name + assigned_names[short_name] = fqn + stats["no_collision"] += 1 - # No collision - use short name directly - if short_name not in collision_groups: - fqn_to_short_name[fqn] = short_name - assigned_names[short_name] = fqn - stats["no_collision"] += 1 - continue + # Then process colliding branches (depth priority means shallowest gets short name first) + for fqn in colliding_df.index: + short_name = colliding_df.loc[fqn, "name"] - # Collision detected - try progressive qualification + # Try progressive qualification assigned_name = _resolve_collision_with_qualification( - fqn, short_name, sorted_df.loc[fqn, "parent"], assigned_names, stats + fqn, short_name, colliding_df.loc[fqn, "parent"], assigned_names, stats ) fqn_to_short_name[fqn] = assigned_name assigned_names[assigned_name] = fqn diff --git a/tests/test_s2dm_short_names.py b/tests/test_s2dm_short_names.py index e584aa8d..189cecba 100644 --- a/tests/test_s2dm_short_names.py +++ b/tests/test_s2dm_short_names.py @@ -74,3 +74,65 @@ def test_fqn_type_names_flag(self): # Short name collision detection should be skipped assert vspec_comments["short_name_mapping"] is None assert vspec_comments["short_name_collisions"] == [] + + def test_cross_tree_collision_detection(self): + """Test collision detection works across main tree and struct types.""" + tree, data_type_tree = get_trees( + vspec=Path("tests/vspec/test_s2dm_cross_tree_collisions/test.vspec"), + types=(Path("tests/vspec/test_s2dm_cross_tree_collisions/types.vspec"),), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_quantities.yaml"),), + units=(Path("tests/vspec/test_units.yaml"),), + overlays=(), + expand=False, + ) + + # Generate schema with short names enabled + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema( + tree, data_type_tree, use_short_names=True + ) + schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) + + # Verify collision detection ran and found collisions + assert vspec_comments["short_name_mapping"] is not None + assert len(vspec_comments["short_name_collisions"]) > 0 + + # Window collisions: 3 branches with "Window" name + # - Vehicle.Body.Window + # - Vehicle.Cabin.Window + # - VehicleDataTypes.Window (struct) + # Should get parent-qualified names + assert "type Body_Window @vspec" in schema_str + assert "type Cabin_Window @vspec" in schema_str + assert "type VehicleDataTypes_Window @vspec" in schema_str + + # Status collisions: 2 branches with "Status" name + # - Vehicle.Powertrain.Status + # - VehicleDataTypes.Status (struct) + # Should get parent-qualified names + assert "type Powertrain_Status @vspec" in schema_str + assert "type VehicleDataTypes_Status @vspec" in schema_str + + # Sensor is unique (only in structs) - should use short name + assert "type Sensor @vspec" in schema_str + + # Verify collision groups contain both main tree and struct types + collision_groups = {c["short_name"]: c for c in vspec_comments["short_name_collisions"]} + + # Window collision group should have 3 FQNs + assert "Window" in collision_groups + window_group = collision_groups["Window"] + assert window_group["collision_count"] == 3 + assert "Vehicle.Body.Window" in window_group["fqns"] + assert "Vehicle.Cabin.Window" in window_group["fqns"] + assert "VehicleDataTypes.Window" in window_group["fqns"] + + # Status collision group should have 2 FQNs + assert "Status" in collision_groups + status_group = collision_groups["Status"] + assert status_group["collision_count"] == 2 + assert "Vehicle.Powertrain.Status" in status_group["fqns"] + assert "VehicleDataTypes.Status" in status_group["fqns"] diff --git a/tests/test_s2dm_structs.py b/tests/test_s2dm_structs.py index faba18c9..4e09239b 100644 --- a/tests/test_s2dm_structs.py +++ b/tests/test_s2dm_structs.py @@ -387,6 +387,7 @@ def test_cli_with_types_option(self, tmp_path): "tests/vspec/test_structs/VehicleDataTypes.vspec", "--output", str(output_dir), + "--fqn-type-names", ], capture_output=True, text=True, @@ -430,6 +431,7 @@ def test_cli_modular_output_with_structs(self, tmp_path): "--output", str(output_dir), "--modular", + "--fqn-type-names", ], capture_output=True, text=True, diff --git a/tests/vspec/test_s2dm_cross_tree_collisions/test.vspec b/tests/vspec/test_s2dm_cross_tree_collisions/test.vspec new file mode 100644 index 00000000..daf2d560 --- /dev/null +++ b/tests/vspec/test_s2dm_cross_tree_collisions/test.vspec @@ -0,0 +1,46 @@ +# Test cross-tree collision detection between main tree and struct types +# Both main tree branches and struct definitions have "Window" and "Status" names + +Vehicle: + type: branch + description: Top-level vehicle branch + +Vehicle.Body: + type: branch + description: Body branch + +Vehicle.Body.Window: + type: branch + description: Body window - collides with Cabin.Window and struct Window + +Vehicle.Body.Window.Position: + datatype: float + type: sensor + unit: percent + description: Window position + +Vehicle.Cabin: + type: branch + description: Cabin branch + +Vehicle.Cabin.Window: + type: branch + description: Cabin window - collides with Body.Window and struct Window + +Vehicle.Cabin.Window.IsOpen: + datatype: boolean + type: actuator + description: Window open state + +Vehicle.Powertrain: + type: branch + description: Powertrain branch + +Vehicle.Powertrain.Status: + type: branch + description: Powertrain status - collides with struct Status + +Vehicle.Powertrain.Status.IsRunning: + datatype: boolean + type: sensor + description: Engine running state diff --git a/tests/vspec/test_s2dm_cross_tree_collisions/types.vspec b/tests/vspec/test_s2dm_cross_tree_collisions/types.vspec new file mode 100644 index 00000000..cfed5dcb --- /dev/null +++ b/tests/vspec/test_s2dm_cross_tree_collisions/types.vspec @@ -0,0 +1,48 @@ +# Struct type definitions that collide with main tree branch names +# These will share the same GraphQL type namespace + +VehicleDataTypes: + type: branch + description: Root for struct definitions + +VehicleDataTypes.Window: + type: struct + description: Window info struct - collides with Vehicle.Body.Window and Vehicle.Cabin.Window + +VehicleDataTypes.Window.height: + datatype: float + type: property + description: Window height in cm + +VehicleDataTypes.Window.width: + datatype: float + type: property + description: Window width in cm + +VehicleDataTypes.Status: + type: struct + description: Status struct - collides with Vehicle.Powertrain.Status + +VehicleDataTypes.Status.code: + datatype: int32 + type: property + description: Status code + +VehicleDataTypes.Status.message: + datatype: string + type: property + description: Status message + +VehicleDataTypes.Sensor: + type: struct + description: Sensor struct - unique, no collision + +VehicleDataTypes.Sensor.value: + datatype: float + type: property + description: Sensor value + +VehicleDataTypes.Sensor.timestamp: + datatype: uint64 + type: property + description: Timestamp in ms From c02536d02906b2513c4b2d2d0f432a523e91d777 Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Wed, 18 Feb 2026 16:10:07 +0100 Subject: [PATCH 08/18] fix(s2dm): Simplify directive handling in modular mode and resolve mypy errors Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- .../exporters/s2dm/metadata_tracker.py | 2 +- .../exporters/s2dm/modular_export_utils.py | 63 ++----------------- .../exporters/s2dm/reference_generator.py | 5 +- .../exporters/s2dm/schema_generator.py | 2 +- 4 files changed, 12 insertions(+), 60 deletions(-) diff --git a/src/vss_tools/exporters/s2dm/metadata_tracker.py b/src/vss_tools/exporters/s2dm/metadata_tracker.py index 7eb98aca..640a496d 100644 --- a/src/vss_tools/exporters/s2dm/metadata_tracker.py +++ b/src/vss_tools/exporters/s2dm/metadata_tracker.py @@ -13,7 +13,7 @@ from typing import Any -def init_vspec_comments() -> dict[str, dict[str, Any]]: +def init_vspec_comments() -> dict[str, Any]: """ Initialize dictionary for storing VSS metadata for directives. diff --git a/src/vss_tools/exporters/s2dm/modular_export_utils.py b/src/vss_tools/exporters/s2dm/modular_export_utils.py index 057410d4..25226aa9 100644 --- a/src/vss_tools/exporters/s2dm/modular_export_utils.py +++ b/src/vss_tools/exporters/s2dm/modular_export_utils.py @@ -13,7 +13,7 @@ from pathlib import Path from typing import Any, cast -from graphql import GraphQLEnumType, GraphQLObjectType, GraphQLSchema, is_enum_type, is_object_type, print_type +from graphql import GraphQLObjectType, GraphQLSchema, is_enum_type, is_object_type, print_type def analyze_schema_for_flat_domains(schema: GraphQLSchema) -> dict[str, list[str]]: @@ -367,8 +367,6 @@ def write_common_files( """ from graphql import is_scalar_type, print_type - from .graphql_utils import extract_custom_directives_from_schema - # Ensure output directory exists output_dir.mkdir(parents=True, exist_ok=True) @@ -376,61 +374,12 @@ def write_common_files( other_dir = output_dir / "other" other_dir.mkdir(parents=True, exist_ok=True) - # Extract and write directives using GraphQL introspection - custom_directives = extract_custom_directives_from_schema(schema) - - if custom_directives: - directives_content = ["# GraphQL directive definitions\n"] - - # For each custom directive, use GraphQL's print function or reconstruct the SDL - for directive_name, directive_obj in custom_directives.items(): - # Build directive definition string - args_parts = [] - for arg_name, arg in directive_obj.args.items(): - arg_type_str = str(arg.type) - - # Check if default_value is actually set (not None and not Undefined sentinel) - # GraphQL uses a special Undefined sentinel, check by string representation - has_default = arg.default_value is not None and str(arg.default_value) != "Undefined" - - if has_default: - args_parts.append(f" {arg_name}: {arg_type_str} = {arg.default_value}") - else: - args_parts.append(f" {arg_name}: {arg_type_str}") - - # Add description if available - if arg.description: - # Use triple-quoted string for description, ensure proper formatting - desc_lines = arg.description.split("\n") - if len(desc_lines) == 1: - args_parts[-1] = f' """{arg.description}"""\n {args_parts[-1].strip()}' - else: - # Multi-line description - desc = "\n ".join(desc_lines) - args_parts[-1] = f' """\n {desc}\n """\n {args_parts[-1].strip()}' - - # Build locations string - locations = " | ".join([loc.name for loc in directive_obj.locations]) - - # Construct the directive - directive_def = f"directive @{directive_name}" - if args_parts: - directive_def += f"(\n{chr(10).join(args_parts)}\n)" - directive_def += f" on {locations}" - - directives_content.append(directive_def) - - # Add schema enums that belong with directives (VspecElement, etc.) - schema_enums = [] - for type_name, type_def in schema.type_map.items(): - if isinstance(type_def, GraphQLEnumType) and type_name in ["VspecElement"]: - schema_enums.append(print_type(type_def)) - - if schema_enums: - directives_content.extend(schema_enums) - + # Copy predefined directives file directly instead of reconstructing from introspection + predefined_directives_file = Path(__file__).parent / "predefined_elements" / "directives.graphql" + if predefined_directives_file.exists(): + directives_content = predefined_directives_file.read_text(encoding="utf-8") with open(other_dir / "directives.graphql", "w") as f: - f.write("\n\n".join(directives_content)) + f.write(directives_content) # Extract custom scalars using GraphQL introspection custom_scalars = [] diff --git a/src/vss_tools/exporters/s2dm/reference_generator.py b/src/vss_tools/exporters/s2dm/reference_generator.py index 0216e9fe..181ebeee 100644 --- a/src/vss_tools/exporters/s2dm/reference_generator.py +++ b/src/vss_tools/exporters/s2dm/reference_generator.py @@ -19,6 +19,7 @@ from __future__ import annotations from pathlib import Path +from typing import Any, cast from vss_tools import log from vss_tools.tree import VSSNode @@ -194,7 +195,9 @@ def generate_vspec_reference( f.write("# 3. If still collision: add more ancestors (e.g., 'Cabin_Door_Window')\n") f.write("# 4. Last resort: use full FQN with underscores\n\n") - collision_list = mapping_metadata.get("short_name_collisions", []) + collision_list: list[dict[str, Any]] = cast( + list[dict[str, Any]], mapping_metadata.get("short_name_collisions", []) + ) name_mapping = mapping_metadata.get("short_name_mapping", {}) stats = mapping_metadata.get("short_name_stats", {}) diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index c50e5403..766c57a6 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -108,7 +108,7 @@ def generate_s2dm_schema( vspec_comments["short_name_stats"] = collision_stats else: # When using FQN names, store empty mapping to indicate FQN mode - vspec_comments["short_name_mapping"] = None + vspec_comments["short_name_mapping"] = {} vspec_comments["short_name_collisions"] = [] vspec_comments["short_name_stats"] = {} From f5471159350ecbc3c2b063688ceb37f1854ee0c5 Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Wed, 18 Feb 2026 21:34:20 +0100 Subject: [PATCH 09/18] feat(s2dm): add unit enums metadata handling in modular mode Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- src/vss_tools/exporters/s2dm/modular_export_utils.py | 5 +++++ src/vss_tools/exporters/s2dm/schema_generator.py | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/vss_tools/exporters/s2dm/modular_export_utils.py b/src/vss_tools/exporters/s2dm/modular_export_utils.py index 25226aa9..baef5c15 100644 --- a/src/vss_tools/exporters/s2dm/modular_export_utils.py +++ b/src/vss_tools/exporters/s2dm/modular_export_utils.py @@ -206,6 +206,7 @@ def write_domain_files( output_dir: Path, vspec_comments: dict[str, Any], directive_processor: Any, + unit_enums_metadata: dict[str, Any], allowed_enums_metadata: dict[str, Any], ) -> None: """ @@ -217,6 +218,7 @@ def write_domain_files( output_dir: Output directory vspec_comments: VSS comments for directive processing directive_processor: Processor for adding @vspec directives + unit_enums_metadata: Metadata for unit enums allowed_enums_metadata: Metadata for allowed value enums """ for file_path, type_names in domain_structure.items(): @@ -335,6 +337,9 @@ def write_domain_files( # Process the SDL string to add directives # We need to split into lines and process lines = file_content.split("\n") + # Process unit enum directives for units file + if file_path == "other/units.graphql": + lines = directive_processor._process_unit_enum_directives(lines, unit_enums_metadata, set()) lines = directive_processor._process_allowed_enum_directives(lines, allowed_enums_metadata, set()) lines = directive_processor._process_field_directives(lines, vspec_comments) lines = directive_processor._process_deprecated_directives( diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index 766c57a6..67a1746b 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -236,7 +236,13 @@ def write_modular_schema( # Write domain-specific files write_domain_files( - domain_structure, schema, output_dir, vspec_comments, directive_processor, allowed_enums_metadata + domain_structure, + schema, + output_dir, + vspec_comments, + directive_processor, + unit_enums_metadata, + allowed_enums_metadata, ) except Exception as e: From 5f587d319d8bfcff3048dfcccbab4c92661a46aa Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Thu, 19 Feb 2026 12:51:45 +0100 Subject: [PATCH 10/18] fix(s2dm): Use the correct output type Problem appeared when field connects to type produced from vspec struct and it was resolving always to String due to name shortening missmatches. Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- .../exporters/s2dm/schema_generator.py | 12 +++- src/vss_tools/exporters/s2dm/type_builders.py | 72 ++++++++++++++----- tests/test_s2dm_structs.py | 53 ++++++++++++++ 3 files changed, 115 insertions(+), 22 deletions(-) diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index 67a1746b..ea77f47d 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -22,6 +22,7 @@ import pandas as pd from graphql import GraphQLField, GraphQLObjectType, GraphQLSchema, GraphQLString +from vss_tools.exporters.s2dm.graphql_utils import GraphQLElementType from vss_tools.tree import VSSNode from vss_tools.utils.pandas_utils import detect_and_resolve_short_name_collisions, get_metadata_df @@ -108,7 +109,7 @@ def generate_s2dm_schema( vspec_comments["short_name_stats"] = collision_stats else: # When using FQN names, store empty mapping to indicate FQN mode - vspec_comments["short_name_mapping"] = {} + vspec_comments["short_name_mapping"] = None vspec_comments["short_name_collisions"] = [] vspec_comments["short_name_stats"] = {} @@ -143,8 +144,13 @@ def generate_s2dm_schema( ) # Assemble complete schema - vehicle_type = types_registry.get("Vehicle", GraphQLString) - query = GraphQLObjectType("Query", {"vehicle": GraphQLField(vehicle_type)}) + from .graphql_utils import sanitize_graphql_name + + root_type_name = sanitize_graphql_name(tree.name, element_type=GraphQLElementType.TYPE) + root_type = types_registry.get(root_type_name, GraphQLString) + query = GraphQLObjectType( + "Query", {sanitize_graphql_name(tree.name, element_type=GraphQLElementType.FIELD): GraphQLField(root_type)} + ) schema = GraphQLSchema( query=query, types=get_vss_scalar_types() + list(types_registry.values()) + list(unit_enums.values()), diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index c6322a96..f1b7a8d7 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -370,7 +370,7 @@ def create_struct_types( fields = {} for prop_fqn, prop_row in properties.iterrows(): field_name = convert_name_for_graphql_schema(prop_row["name"], GraphQLElementType.FIELD, S2DM_CONVERSIONS) - base_type = _get_graphql_type_for_property(prop_row, struct_types) + base_type = _get_graphql_type_for_property(prop_row, struct_types, short_name_mapping) fields[field_name] = GraphQLField(GraphQLNonNull(base_type), description=prop_row.get("description", "")) field_path = build_field_path(type_name, field_name) @@ -402,38 +402,70 @@ def create_struct_types( return struct_types -def _get_graphql_type_for_property(prop_row: pd.Series, struct_types: dict[str, GraphQLObjectType]) -> Any: +def _get_graphql_type_for_property( + prop_row: pd.Series, struct_types: dict[str, GraphQLObjectType], short_name_mapping: dict[str, str] | None = None +) -> Any: """Map VSS property datatype to GraphQL type.""" datatype = prop_row.get("datatype", "string") - return resolve_datatype_to_graphql(datatype, struct_types) + return resolve_datatype_to_graphql(datatype, struct_types, short_name_mapping) -def resolve_datatype_to_graphql(datatype: str, types_registry: dict[str, Any]) -> Any: +def resolve_datatype_to_graphql( + datatype: str, types_registry: dict[str, Any], short_name_mapping: dict[str, str] | None = None +) -> Any: """ Resolve a VSS datatype string to its corresponding GraphQL type. Args: datatype: VSS datatype string (e.g., "uint8", "MyStruct", "MyStruct[]") types_registry: Dictionary of custom types + short_name_mapping: Optional mapping from FQN to short type names Returns: Corresponding GraphQL type + + Raises: + ValueError: If datatype references a struct that doesn't exist in types_registry """ - if datatype.endswith("[]"): - base_datatype = datatype[:-2] + is_array = datatype.endswith("[]") + base_datatype = datatype[:-2] if is_array else datatype + + # First check if it's a primitive type + if base_datatype in VSS_DATATYPE_MAP: + base_type = VSS_DATATYPE_MAP[base_datatype] + if is_array: + return GraphQLList(GraphQLNonNull(base_type)) + return base_type + + # Not a primitive, so it should be a struct type + # Try to resolve using short_name_mapping first if available + struct_type_name = None + if short_name_mapping and base_datatype in short_name_mapping: + struct_type_name = short_name_mapping[base_datatype] + else: + # Fall back to FQN conversion struct_type_name = convert_name_for_graphql_schema(base_datatype, GraphQLElementType.TYPE, S2DM_CONVERSIONS) - if struct_type_name in types_registry: - return GraphQLList(GraphQLNonNull(types_registry[struct_type_name])) - - base_type = VSS_DATATYPE_MAP.get(base_datatype, GraphQLString) - return GraphQLList(GraphQLNonNull(base_type)) - - struct_type_name = convert_name_for_graphql_schema(datatype, GraphQLElementType.TYPE, S2DM_CONVERSIONS) if struct_type_name in types_registry: - return types_registry[struct_type_name] - - return VSS_DATATYPE_MAP.get(datatype, GraphQLString) + struct_type = types_registry[struct_type_name] + if is_array: + return GraphQLList(GraphQLNonNull(struct_type)) + return struct_type + + # Type not found - this is a translation error + available_structs = [ + name for name in types_registry.keys() if not name.endswith("_Enum") and not name.endswith("UnitEnum") + ] + log.error( + f"Failed to resolve datatype '{datatype}' to GraphQL type. " + f"This datatype is not a primitive and doesn't match any struct type in the registry. " + f"Available struct types: {available_structs}. " + f"Check that the struct is defined in your types vspec file." + ) + raise ValueError( + f"Cannot resolve datatype '{datatype}': not a primitive type and not found in types registry. " + f"Available struct types: {available_structs}" + ) def create_object_type( @@ -520,7 +552,7 @@ def get_fields() -> dict[str, GraphQLField]: if pd.notna(leaf_row.get("deprecation")) and leaf_row.get("deprecation").strip(): vspec_comments["field_deprecated"][field_path] = leaf_row["deprecation"] - field_type = get_graphql_type_for_leaf(leaf_row, types_registry) + field_type = get_graphql_type_for_leaf(leaf_row, types_registry, short_name_mapping) unit_args = _get_unit_args(leaf_row, unit_enums) fields[field_name] = GraphQLField(field_type, args=unit_args, description=leaf_row.get("description", "")) @@ -657,7 +689,9 @@ def _get_unit_args(leaf_row: pd.Series, unit_enums: dict[str, GraphQLEnumType]) return {"unit": GraphQLArgument(type_=unit_enum, default_value=unit)} -def get_graphql_type_for_leaf(leaf_row: pd.Series, types_registry: dict[str, Any] | None = None) -> Any: +def get_graphql_type_for_leaf( + leaf_row: pd.Series, types_registry: dict[str, Any] | None = None, short_name_mapping: dict[str, str] | None = None +) -> Any: """Map VSS leaf to GraphQL type.""" if types_registry: try: @@ -675,6 +709,6 @@ def get_graphql_type_for_leaf(leaf_row: pd.Series, types_registry: dict[str, Any datatype = leaf_row.get("datatype", "string") if types_registry: - return resolve_datatype_to_graphql(datatype, types_registry) + return resolve_datatype_to_graphql(datatype, types_registry, short_name_mapping) return VSS_DATATYPE_MAP.get(datatype, GraphQLString) diff --git a/tests/test_s2dm_structs.py b/tests/test_s2dm_structs.py index 4e09239b..c4334ba6 100644 --- a/tests/test_s2dm_structs.py +++ b/tests/test_s2dm_structs.py @@ -269,6 +269,59 @@ def test_primitive_property_in_struct(self, struct_trees): assert isinstance(z_property_type, GraphQLNonNull) assert z_property_type.of_type == VSS_DATATYPE_MAP["double"] + def test_signal_with_struct_datatype_short_names(self, struct_trees): + """Test that signals correctly reference structs when using short names (default mode).""" + tree, data_type_tree = struct_trees + schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=True) + + # Get the TestRoot type + root_type = schema.type_map["TestRoot"] # Short name mode + + # Get struct type names (should be short names) + parent_struct_name = "ParentStruct" + nested_struct_name = "NestedStruct" + + # Verify struct types exist with short names + assert parent_struct_name in schema.type_map + assert nested_struct_name in schema.type_map + + # Check fields + assert isinstance(root_type, GraphQLObjectType) + fields = root_type.fields + + # ParentStructSensor should reference ParentStruct (short name) + assert "parentStructSensor" in fields + parent_sensor_type = fields["parentStructSensor"].type + assert ( + parent_sensor_type == schema.type_map[parent_struct_name] + ), f"Expected ParentStruct but got {parent_sensor_type}" + + # NestedStructSensor should reference NestedStruct (short name) + assert "nestedStructSensor" in fields + nested_sensor_type = fields["nestedStructSensor"].type + assert ( + nested_sensor_type == schema.type_map[nested_struct_name] + ), f"Expected NestedStruct but got {nested_sensor_type}" + + def test_struct_in_property_short_names(self, struct_trees): + """Test that struct properties correctly reference other structs with short names.""" + tree, data_type_tree = struct_trees + schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=True) + + # Get ParentStruct type (short name) + parent_struct_type = schema.type_map["ParentStruct"] + nested_struct_type = schema.type_map["NestedStruct"] + + # Check that x_property and y_property reference NestedStruct + assert isinstance(parent_struct_type, GraphQLObjectType) + fields = parent_struct_type.fields + + # x_property should reference NestedStruct + assert "xProperty" in fields + x_property_type = fields["xProperty"].type + assert isinstance(x_property_type, GraphQLNonNull) + assert x_property_type.of_type == nested_struct_type, f"Expected NestedStruct but got {x_property_type.of_type}" + class TestS2DMStructsModular: """Test class for modular output with struct support.""" From 4baef441d21ad091b84636beca7682308d063a51 Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:05:32 +0100 Subject: [PATCH 11/18] feat(s2dm): implement handling for skipped empty branches during export Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- .../exporters/s2dm/metadata_tracker.py | 2 + .../exporters/s2dm/reference_generator.py | 39 ++++- .../exporters/s2dm/schema_generator.py | 32 +++- src/vss_tools/exporters/s2dm/type_builders.py | 18 +- tests/test_s2dm_exporter.py | 162 ++++++++++++++++++ .../vspec/test_s2dm/test_empty_branches.vspec | 27 +++ 6 files changed, 276 insertions(+), 4 deletions(-) create mode 100644 tests/vspec/test_s2dm/test_empty_branches.vspec diff --git a/src/vss_tools/exporters/s2dm/metadata_tracker.py b/src/vss_tools/exporters/s2dm/metadata_tracker.py index 640a496d..a1daf134 100644 --- a/src/vss_tools/exporters/s2dm/metadata_tracker.py +++ b/src/vss_tools/exporters/s2dm/metadata_tracker.py @@ -24,6 +24,7 @@ def init_vspec_comments() -> dict[str, Any]: - field_deprecated: field_path -> "deprecation reason" for @deprecated directive - instance_tags: tag_name -> {"element": "BRANCH", "fqn": "...", "instances": "..."} - instance_tag_types: type_name -> tag_name + - skipped_empty_branches: list of branches skipped during export (no children) Returns: Initialized metadata tracking dictionary @@ -35,6 +36,7 @@ def init_vspec_comments() -> dict[str, Any]: "field_vss_types": {}, "field_ranges": {}, "field_deprecated": {}, + "skipped_empty_branches": [], } diff --git a/src/vss_tools/exporters/s2dm/reference_generator.py b/src/vss_tools/exporters/s2dm/reference_generator.py index 181ebeee..c199a0eb 100644 --- a/src/vss_tools/exporters/s2dm/reference_generator.py +++ b/src/vss_tools/exporters/s2dm/reference_generator.py @@ -311,9 +311,40 @@ def generate_vspec_reference( except (PermissionError, OSError) as e: raise S2DMExporterException(f"Failed to write pluralized fields file {pluralized_output}: {e}") from e + # Write skipped empty branches if any were collected + if mapping_metadata and mapping_metadata.get("skipped_empty_branches"): + not_mapped_output = reference_dir / "not_mapped.yaml" + try: + with open(not_mapped_output, "w") as f: + # Write header comment + f.write("# Branches Skipped During S2DM Export\n") + f.write("#\n") + f.write("# This file lists VSS branches that were not mapped to GraphQL types because\n") + f.write("# they have no children (no properties or sub-branches). Empty branches cannot\n") + f.write("# be represented as GraphQL types since GraphQL requires types to have at least\n") + f.write("# one field.\n") + f.write("#\n") + f.write("# To include these branches in the schema, add properties or sub-branches to them\n") + f.write("# in the VSS specification.\n\n") + + f.write("skipped_empty_branches:\n") + for entry in mapping_metadata["skipped_empty_branches"]: + f.write(f" - fqn: {entry['fqn']}\n") + f.write(f" graphql_type_name: {entry['graphql_type_name']}\n") + f.write(f" reason: {entry['reason']}\n") + f.write("\n") + + count = len(mapping_metadata["skipped_empty_branches"]) + log.info(f" - Not mapped (empty branches): {not_mapped_output.name} ({count} branch(es))") + except (PermissionError, OSError) as e: + raise S2DMExporterException(f"Failed to write not mapped file {not_mapped_output}: {e}") from e + # Generate README.md for provenance documentation has_plural_warnings = bool(mapping_metadata and mapping_metadata.get("plural_type_warnings")) - generate_reference_readme(reference_dir, vspec_file, actual_units, actual_quantities, has_plural_warnings) + has_skipped_branches = bool(mapping_metadata and mapping_metadata.get("skipped_empty_branches")) + generate_reference_readme( + reference_dir, vspec_file, actual_units, actual_quantities, has_plural_warnings, has_skipped_branches + ) except S2DMExporterException: # Re-raise our custom exceptions @@ -329,6 +360,7 @@ def generate_reference_readme( units_files: tuple[Path, ...] | None, quantities_files: tuple[Path, ...] | None, has_plural_warnings: bool = False, + has_skipped_branches: bool = False, ) -> None: """ Generate README.md documenting the provenance of reference files. @@ -339,6 +371,7 @@ def generate_reference_readme( units_files: Units files used (explicit or implicit) quantities_files: Quantities files used (explicit or implicit) has_plural_warnings: Whether plural type warnings were generated + has_skipped_branches: Whether empty branches were skipped during export Raises: S2DMExporterException: If README generation fails @@ -377,6 +410,10 @@ def generate_reference_readme( readme_content += """ * **plural_type_warnings.txt** - VSS branches with plural type names (GraphQL prefers singular).""" + if has_skipped_branches: + readme_content += """ +* **not_mapped.yaml** - VSS branches skipped during export (empty branches with no children).""" + readme_content += """ ## Documentation diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index ea77f47d..6efc4463 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -22,11 +22,12 @@ import pandas as pd from graphql import GraphQLField, GraphQLObjectType, GraphQLSchema, GraphQLString -from vss_tools.exporters.s2dm.graphql_utils import GraphQLElementType +from vss_tools import log +from vss_tools.exporters.s2dm.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema from vss_tools.tree import VSSNode from vss_tools.utils.pandas_utils import detect_and_resolve_short_name_collisions, get_metadata_df -from .constants import CUSTOM_DIRECTIVES, S2DMExporterException +from .constants import CUSTOM_DIRECTIVES, S2DM_CONVERSIONS, S2DMExporterException from .graphql_directive_processor import GraphQLDirectiveProcessor from .graphql_scalars import get_vss_scalar_types from .metadata_tracker import init_vspec_comments @@ -132,6 +133,33 @@ def generate_s2dm_schema( # Create object types for all branches for fqn in branches_df.index: if fqn not in types_registry: + # Check if branch is empty (has no children) + node = tree.get_node_with_fqn(fqn) + if node is None: + log.error(f"Branch '{fqn}' not found in tree but present in DataFrame. Skipping.") + continue + + if node.is_leaf: + # Branch has no children - skip creating empty type + # Use same logic as create_object_type for type name + if short_name_mapping and fqn in short_name_mapping: + type_name = short_name_mapping[fqn] + else: + type_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) + log.warning( + f"Branch '{fqn}' (GraphQL type: '{type_name}') was skipped because " + f"the reference vspec does not define any sub-elements inside." + ) + # Track skipped branch for reporting + vspec_comments["skipped_empty_branches"].append( + { + "fqn": fqn, + "graphql_type_name": type_name, + "reason": "Empty branch (no children)", + } + ) + continue + types_registry[fqn] = create_object_type( fqn, branches_df, diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index f1b7a8d7..18c64811 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -558,9 +558,25 @@ def get_fields() -> dict[str, GraphQLField]: # Branch fields for child_fqn, child_row in branches_df[branches_df["parent"] == fqn].iterrows(): + # Check if child branch is empty (has no children) + has_leaf_children = not leaves_df[leaves_df["parent"] == child_fqn].empty + has_branch_children = not branches_df[branches_df["parent"] == child_fqn].empty + + if not has_leaf_children and not has_branch_children: + # Skip creating field for empty branches + log.debug(f"Skipping field for empty branch '{child_fqn}' in type '{type_name}'") + continue + field_name = convert_name_for_graphql_schema(child_row["name"], GraphQLElementType.FIELD, S2DM_CONVERSIONS) child_type = types_registry.get(child_fqn) or create_object_type( - child_fqn, branches_df, leaves_df, types_registry, unit_enums, vspec_comments, extended_attributes + child_fqn, + branches_df, + leaves_df, + types_registry, + unit_enums, + vspec_comments, + extended_attributes, + short_name_mapping, ) types_registry[child_fqn] = child_type diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index bd25ee3b..cb481077 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -789,3 +789,165 @@ def test_extended_attributes_in_metadata(self): assert '@vspec(element: ATTRIBUTE, fqn: "Vehicle.Info.Model"' in schema_str assert '{key: "customMetadata", value: "test_value"}' in schema_str assert '{key: "anotherAttribute", value: "42"}' in schema_str + + def test_empty_branches_are_skipped(self, caplog): + """Test that empty branches (branches with no children) are skipped during export.""" + # Load vspec with empty branches + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/test_empty_branches.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + # Generate schema + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=True) + + # Check that empty branches are NOT in the schema type_map + assert "EmptyBranch" not in schema.type_map + assert "AnotherEmpty" not in schema.type_map + assert "EmptyNested" not in schema.type_map + + # Check that non-empty branches ARE in the schema + assert "Vehicle" in schema.type_map + assert "HasContent" in schema.type_map + + # Generate the actual GraphQL SDL output + schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) + + # Verify empty types don't appear in the SDL output + assert "type EmptyBranch" not in schema_str + assert "type AnotherEmpty" not in schema_str + assert "type EmptyNested" not in schema_str + + # Verify non-empty types DO appear in the SDL output + assert "type Vehicle" in schema_str + assert "type HasContent" in schema_str + + # Verify HasContent has the expected speed field + assert "speed" in schema_str.lower() + + # Check that warnings were logged for empty branches + assert "Vehicle.EmptyBranch" in caplog.text + assert "was skipped because the reference vspec does not define any sub-elements inside" in caplog.text + assert "Vehicle.AnotherEmpty" in caplog.text + assert "Vehicle.HasContent.EmptyNested" in caplog.text + + def test_empty_branches_skipped_with_fqn_names(self, caplog, tmp_path): + """Test that empty branches are skipped when using FQN names (use_short_names=False).""" + # Load vspec with empty branches + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/test_empty_branches.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + # Generate schema with FQN names + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=False) + + # Check that empty branches are NOT in the schema type_map (FQN format) + assert "Vehicle_EmptyBranch" not in schema.type_map + assert "Vehicle_AnotherEmpty" not in schema.type_map + assert "Vehicle_HasContent_EmptyNested" not in schema.type_map + + # Check that non-empty branches ARE in the schema + assert "Vehicle" in schema.type_map + assert "Vehicle_HasContent" in schema.type_map + + # Generate the actual GraphQL SDL output + schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) + + # Write to file for debugging + output_file = tmp_path / "test_empty_branches_fqn.graphql" + output_file.write_text(schema_str) + + # Verify empty types don't appear in the SDL output (FQN format) + assert "type Vehicle_EmptyBranch" not in schema_str + assert "type Vehicle_AnotherEmpty" not in schema_str + assert "type Vehicle_HasContent_EmptyNested" not in schema_str + + # Verify non-empty types DO appear in the SDL output + assert "type Vehicle " in schema_str or "type Vehicle {" in schema_str or "type Vehicle\n" in schema_str + assert "type Vehicle_HasContent" in schema_str + + # Check that warnings were logged for empty branches + assert "Vehicle.EmptyBranch" in caplog.text + assert "Vehicle.AnotherEmpty" in caplog.text + assert "Vehicle.HasContent.EmptyNested" in caplog.text + + def test_empty_branches_reported_in_not_mapped_yaml(self, tmp_path): + """Test that empty branches are reported in vspec_reference/not_mapped.yaml.""" + from vss_tools.exporters.s2dm import generate_vspec_reference + + # Load vspec with empty branches + tree, _ = get_trees( + vspec=Path("tests/vspec/test_s2dm/test_empty_branches.vspec"), + include_dirs=(), + aborts=(), + strict=False, + extended_attributes=(), + quantities=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + units=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + overlays=(), + expand=False, + ) + + # Generate schema (captures skipped branches in vspec_comments) + schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=True) + + # Verify that skipped branches were tracked + assert "skipped_empty_branches" in vspec_comments + skipped = vspec_comments["skipped_empty_branches"] + assert len(skipped) == 3 + + # Verify the FQNs of skipped branches + skipped_fqns = [entry["fqn"] for entry in skipped] + assert "Vehicle.EmptyBranch" in skipped_fqns + assert "Vehicle.AnotherEmpty" in skipped_fqns + assert "Vehicle.HasContent.EmptyNested" in skipped_fqns + + # Generate vspec_reference with the metadata + generate_vspec_reference( + tree=tree, + data_type_tree=None, + output_dir=tmp_path, + extended_attributes=(), + vspec_file=Path("tests/vspec/test_s2dm/test_empty_branches.vspec"), + units_files=(Path("tests/vspec/test_s2dm/test_units.yaml"),), + quantities_files=(Path("tests/vspec/test_s2dm/test_quantities.yaml"),), + mapping_metadata=vspec_comments, + ) + + # Verify not_mapped.yaml was created + not_mapped_file = tmp_path / "vspec_reference" / "not_mapped.yaml" + assert not_mapped_file.exists() + + # Verify contents of not_mapped.yaml + content = not_mapped_file.read_text() + assert "skipped_empty_branches:" in content + assert "Vehicle.EmptyBranch" in content + assert "Vehicle.AnotherEmpty" in content + assert "Vehicle.HasContent.EmptyNested" in content + assert "Empty branch (no children)" in content + + # Verify header comments are present + assert "Branches Skipped During S2DM Export" in content + assert "no children (no properties or sub-branches)" in content + + # Verify README mentions the file + readme_file = tmp_path / "vspec_reference" / "README.md" + assert readme_file.exists() + readme_content = readme_file.read_text() + assert "not_mapped.yaml" in readme_content + assert "empty branches" in readme_content.lower() diff --git a/tests/vspec/test_s2dm/test_empty_branches.vspec b/tests/vspec/test_s2dm/test_empty_branches.vspec new file mode 100644 index 00000000..e65ce63c --- /dev/null +++ b/tests/vspec/test_s2dm/test_empty_branches.vspec @@ -0,0 +1,27 @@ +# Test vspec with empty branches to verify they are skipped during S2DM export + +Vehicle: + type: branch + description: High-level vehicle data. + +Vehicle.EmptyBranch: + type: branch + description: A branch that does not have any sub-elements. + +Vehicle.HasContent: + type: branch + description: A branch that has some elements inside. + +Vehicle.HasContent.Speed: + type: sensor + datatype: float + unit: percent + description: Vehicle speed. + +Vehicle.AnotherEmpty: + type: branch + description: Another empty branch for testing. + +Vehicle.HasContent.EmptyNested: + type: branch + description: An empty branch nested inside a non-empty branch. From ebecad2c17794fbdf0003bf46c29e5c9e105138c Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Wed, 11 Mar 2026 23:43:07 +0100 Subject: [PATCH 12/18] feat(s2dm): simplify vspec metadata annotation with sidecar lookup spec file Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- .../s2dm/graphql_directive_processor.py | 83 ++++--------------- .../predefined_elements/directives.graphql | 7 -- src/vss_tools/exporters/s2dm/type_builders.py | 54 ++---------- tests/test_s2dm_exporter.py | 29 ++++--- 4 files changed, 37 insertions(+), 136 deletions(-) diff --git a/src/vss_tools/exporters/s2dm/graphql_directive_processor.py b/src/vss_tools/exporters/s2dm/graphql_directive_processor.py index a2c53095..6f1bfd8f 100644 --- a/src/vss_tools/exporters/s2dm/graphql_directive_processor.py +++ b/src/vss_tools/exporters/s2dm/graphql_directive_processor.py @@ -70,8 +70,8 @@ def _process_unit_enum_directives( """ Process unit enum directives. - Annotates enum type with @vspec(element: QUANTITY_KIND, metadata: [{key: "quantity", value: "..."}]) - and individual enum values with @vspec(element: UNIT, metadata: [{key: "unit", value: "..."}]) + Annotates enum type with @vspec(element: QUANTITY_KIND) + and individual enum values with @vspec(element: UNIT). """ for quantity, units_data in unit_enums_metadata.items(): enum_name = f"{convert_name_for_graphql_schema(quantity, GraphQLElementType.TYPE)}UnitEnum" @@ -80,11 +80,7 @@ def _process_unit_enum_directives( for i, line in enumerate(lines): if line.strip().startswith(f"enum {enum_name}"): if "@vspec" not in line: - # Annotate enum type with QUANTITY_KIND - directive = ( - f"@vspec(element: QUANTITY_KIND, " f'metadata: [{{key: "quantity", value: "{quantity}"}}])' - ) - lines[i] = line.replace(" {", f" {directive} {{") + lines[i] = line.replace(" {", " @vspec(element: QUANTITY_KIND) {") in_target_enum = True continue elif line.strip().startswith("enum ") and in_target_enum: @@ -105,9 +101,7 @@ def _process_unit_enum_directives( if stripped_line.startswith(enum_value_name) and enum_value_key not in processed_values: if "@vspec" not in line: indent = line[: len(line) - len(line.lstrip())] - # Annotate enum value with UNIT - directive = f'@vspec(element: UNIT, metadata: [{{key: "unit", value: "{unit_key}"}}])' - lines[i] = f"{indent}{enum_value_name} {directive}" + lines[i] = f"{indent}{enum_value_name} @vspec(element: UNIT)" processed_values.add(enum_value_key) break @@ -119,29 +113,19 @@ def _process_allowed_enum_directives( """ Process allowed value enum directives. - Annotates the enum type itself with @vspec(element, fqn, metadata), - and annotates individual enum values that were modified with @vspec(metadata). + Annotates the enum type itself with @vspec(element, fqn). + Annotates individual enum values that were sanitized with @vspec(metadata: originalName). """ for enum_name, enum_data in allowed_enums_metadata.items(): fqn = enum_data.get("fqn", "") vss_type = enum_data.get("vss_type", "ATTRIBUTE") - allowed_values_dict = enum_data.get("allowed_values", {}) modified_values = enum_data.get("modified_values", {}) - # Build the allowed values list for metadata - # GraphQL requires: value: "['val1', 'val2']" (double quotes outside, single quotes inside) - allowed_values_list = list(allowed_values_dict.values()) - allowed_str = ", ".join([f"'{v}'" for v in allowed_values_list]) - in_target_enum = False for i, line in enumerate(lines): if line.strip().startswith(f"enum {enum_name}"): if "@vspec" not in line: - # Annotate the enum type - directive = ( - f'@vspec(element: {vss_type}, fqn: "{fqn}", ' - f'metadata: [{{key: "allowed", value: "[{allowed_str}]"}}])' - ) + directive = f'@vspec(element: {vss_type}, fqn: "{fqn}")' lines[i] = line.replace(" {", f" {directive} {{") in_target_enum = True continue @@ -216,13 +200,12 @@ def _process_instance_dimension_enum_directives( return lines def _process_field_directives(self, lines: list[str], vspec_comments: dict) -> list[str]: - """Process consolidated field @vspec directives (element + fqn + optional metadata).""" - # Process VSS type information (element + fqn + metadata) + """Process consolidated field @vspec directives (element + fqn + optional instantiate metadata).""" for field_path, vss_info in vspec_comments.get("field_vss_types", {}).items(): type_name, field_name = field_path.split(".", 1) # Use maxsplit=1 to handle field names with dots element = vss_info["element"] fqn = vss_info["fqn"] - instantiate = vss_info.get("instantiate") # Check if this is a hoisted non-instantiated field + instantiate = vss_info.get("instantiate") # Only False for hoisted non-instantiated fields in_type = False for i, line in enumerate(lines): @@ -237,26 +220,12 @@ def _process_field_directives(self, lines: list[str], vspec_comments: dict) -> l continue if in_type and line.strip().startswith(f"{field_name}") and "@vspec" not in line: - # Build metadata array from extended attributes - metadata_entries = [] - - # Add instantiate metadata if present if instantiate is False: - metadata_entries.append('{key: "instantiate", value: "false"}') - - # Add extended attributes metadata - for key, value in vss_info.items(): - if key not in ["element", "fqn", "instantiate"]: - # Escape quotes in value - escaped_value = str(value).replace('"', '\\\\"') - metadata_entries.append(f'{{key: "{key}", value: "{escaped_value}"}}') - - # Build directive - if metadata_entries: - metadata_str = ", ".join(metadata_entries) - directive = f'@vspec(element: {element}, fqn: "{fqn}", metadata: [{metadata_str}])' + directive = ( + f'@vspec(element: {element}, fqn: "{fqn}", ' + + 'metadata: [{key: "instantiate", value: "false"}])' + ) else: - # Standard directive without metadata directive = f'@vspec(element: {element}, fqn: "{fqn}")' lines[i] = line.rstrip() + f" {directive}" @@ -376,36 +345,14 @@ def _process_type_directives(self, lines: list[str], vspec_comments: dict) -> li # Add @vspec directive if needs_instance_tag and "@vspec" not in line: - # Instance tag types get special metadata with instances element = instance_tag_info["element"] fqn = instance_tag_info["fqn"] - instances = instance_tag_info["instances"] - directive = ( - f'@vspec(element: {element}, fqn: "{fqn}", ' - f'metadata: [{{key: "instances", value: "{instances}"}}])' - ) - new_line += f" {directive}" + new_line += f' @vspec(element: {element}, fqn: "{fqn}")' elif needs_vss_type and "@vspec" not in line: - # Regular types get element + fqn + extended attributes metadata vss_info = vspec_comments["vss_types"][type_name] element = vss_info["element"] fqn = vss_info["fqn"] - - # Build metadata array from extended attributes - metadata_entries = [] - for key, value in vss_info.items(): - if key not in ["element", "fqn"]: - # Escape quotes in value - escaped_value = str(value).replace('"', '\\\\"') - metadata_entries.append(f'{{key: "{key}", value: "{escaped_value}"}}') - - # Build directive - if metadata_entries: - metadata_str = ", ".join(metadata_entries) - directive = f'@vspec(element: {element}, fqn: "{fqn}", metadata: [{metadata_str}])' - else: - directive = f'@vspec(element: {element}, fqn: "{fqn}")' - new_line += f" {directive}" + new_line += f' @vspec(element: {element}, fqn: "{fqn}")' new_line += " {" lines[i] = new_line # No extra indentation diff --git a/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql b/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql index bfd20e36..45e75f6e 100644 --- a/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql +++ b/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql @@ -35,13 +35,6 @@ enum VspecElement { } -#enum FuelTypeEnum @vspec(element:"ATTRIBUTE", fqn:"Vehicle.Powertrain.FuelType", metadata:[{key:"allowed", value:"[GASOLINE, DIESEL, ELECTRIC, HYBRID]"}]) { -# GASOLINE -# DIESEL -# ELECTRIC -# HYBRID -#} - directive @range(min: Float, max: Float) on FIELD_DEFINITION directive @instanceTag on OBJECT diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index 18c64811..d6f46397 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -29,7 +29,6 @@ GraphQLEnumType, GraphQLEnumValue, GraphQLField, - GraphQLID, GraphQLList, GraphQLNonNull, GraphQLObjectType, @@ -46,25 +45,6 @@ from .graphql_utils import GraphQLElementType, convert_name_for_graphql_schema from .metadata_tracker import build_field_path - -def _extract_extended_attributes(row: pd.Series, extended_attributes: tuple[str, ...]) -> dict[str, Any]: - """ - Extract extended attributes from a DataFrame row. - - Args: - row: pandas Series (DataFrame row) containing VSS node data - extended_attributes: Tuple of extended attribute names to extract - - Returns: - Dictionary containing only the extended attributes that exist and are not NA - """ - extracted = {} - for ext_attr in extended_attributes: - if ext_attr in row.index and pd.notna(row.get(ext_attr)): - extracted[ext_attr] = row[ext_attr] - return extracted - - # Initialize inflect engine for pluralization (singleton) _inflect_engine = inflect.engine() @@ -375,10 +355,6 @@ def create_struct_types( field_path = build_field_path(type_name, field_name) field_metadata = {"element": "STRUCT_PROPERTY", "fqn": prop_fqn} - - # Capture extended attributes if present - field_metadata.update(_extract_extended_attributes(prop_row, extended_attributes)) - vspec_comments["field_vss_types"][field_path] = field_metadata if pd.notna(prop_row.get("min")) or pd.notna(prop_row.get("max")): @@ -394,10 +370,7 @@ def create_struct_types( name=type_name, fields=fields, description=struct_row.get("description", "") ) - # Store type-level metadata including extended attributes - type_metadata = {"element": "STRUCT", "fqn": fqn} - type_metadata.update(_extract_extended_attributes(struct_row, extended_attributes)) - vspec_comments["vss_types"][type_name] = type_metadata + vspec_comments["vss_types"][type_name] = {"element": "STRUCT", "fqn": fqn} return struct_types @@ -513,8 +486,6 @@ def get_fields() -> dict[str, GraphQLField]: fields = {} # System fields - if type_name == "Vehicle" or branch_row.get("instances"): - fields["id"] = GraphQLField(GraphQLNonNull(GraphQLID)) if instance_tag_type := vspec_comments.get("instance_tag_types", {}).get(type_name): if instance_tag_type in types_registry: fields["instanceTag"] = GraphQLField(types_registry[instance_tag_type]) @@ -536,12 +507,7 @@ def get_fields() -> dict[str, GraphQLField]: field_path = build_field_path(type_name, field_name) if leaf_type := _get_vss_type_if_valid(leaf_row): - field_metadata = {"element": leaf_type, "fqn": child_fqn} - - # Capture extended attributes if present - field_metadata.update(_extract_extended_attributes(leaf_row, extended_attributes)) - - vspec_comments["field_vss_types"][field_path] = field_metadata + vspec_comments["field_vss_types"][field_path] = {"element": leaf_type, "fqn": child_fqn} if pd.notna(leaf_row.get("min")) or pd.notna(leaf_row.get("max")): vspec_comments["field_ranges"][field_path] = { @@ -597,14 +563,15 @@ def get_fields() -> dict[str, GraphQLField]: {"fqn": child_fqn, "plural_field_name": plural_field_name, "path_in_graphql_model": field_path} ) else: + field_path = build_field_path(type_name, field_name) fields[field_name] = GraphQLField(child_type) + # Annotate field with @vspec directive (shared for both cases) + vspec_comments["field_vss_types"][field_path] = {"element": "BRANCH", "fqn": child_fqn} + return fields - # Store type-level metadata including extended attributes - type_metadata = {"element": "BRANCH", "fqn": fqn} - type_metadata.update(_extract_extended_attributes(branch_row, extended_attributes)) - vspec_comments["vss_types"][type_name] = type_metadata + vspec_comments["vss_types"][type_name] = {"element": "BRANCH", "fqn": fqn} return GraphQLObjectType(name=type_name, fields=get_fields, description=branch_row.get("description", "")) @@ -647,17 +614,12 @@ def get_hoisted_fields( field_path = build_field_path(parent_type_name, hoisted_field_name) if leaf_type := _get_vss_type_if_valid(leaf_row): - field_metadata = { + vspec_comments["field_vss_types"][field_path] = { "element": leaf_type, "fqn": leaf_fqn, "instantiate": False, } - # Capture extended attributes if present - field_metadata.update(_extract_extended_attributes(leaf_row, extended_attributes)) - - vspec_comments["field_vss_types"][field_path] = field_metadata - if pd.notna(leaf_row.get("min")) or pd.notna(leaf_row.get("max")): vspec_comments["field_ranges"][field_path] = { "min": leaf_row.get("min") if pd.notna(leaf_row.get("min")) else None, diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index cb481077..3fb26182 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -751,8 +751,12 @@ def test_instance_dimension_enum_sanitization(self): assert 'REAR_LEFT @vspec(metadata: [{key: "originalName", value: "RearLeft"}])' in schema_str assert 'REAR_RIGHT @vspec(metadata: [{key: "originalName", value: "RearRight"}])' in schema_str - def test_extended_attributes_in_metadata(self): - """Test that extended attributes are captured and added to @vspec metadata.""" + def test_extended_attributes_not_in_schema_metadata(self): + """Test that extended attributes are not annotated in @vspec schema metadata. + + Extended attributes are available via the sidecar vspec lookup, so they are + intentionally omitted from the schema to keep it minimal. + """ # Load the test vspec with extended attributes tree, _ = get_trees( vspec=Path("tests/vspec/test_s2dm/test_extended_attributes.vspec"), @@ -771,24 +775,19 @@ def test_extended_attributes_in_metadata(self): ) schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) - # Check Vehicle.Speed has source and quality in metadata + # Fields are still annotated with element + fqn assert "speed(unit: RelationUnitEnum = PERCENT): Float" in schema_str assert '@vspec(element: SENSOR, fqn: "Vehicle.Speed"' in schema_str - assert '{key: "source", value: "ecu0xAA"}' in schema_str - assert '{key: "quality", value: "100"}' in schema_str - - # Check Vehicle.Temperature has source, quality, and calibration - assert "temperature(unit: AngleUnitEnum = DEGREE): Int16" in schema_str assert '@vspec(element: SENSOR, fqn: "Vehicle.Temperature"' in schema_str - assert '{key: "source", value: "ecu0xBB"}' in schema_str - assert '{key: "quality", value: "95"}' in schema_str - assert '{key: "calibration", value: "factory"}' in schema_str - - # Check Vehicle.Info.Model has customMetadata and anotherAttribute assert "model: String" in schema_str assert '@vspec(element: ATTRIBUTE, fqn: "Vehicle.Info.Model"' in schema_str - assert '{key: "customMetadata", value: "test_value"}' in schema_str - assert '{key: "anotherAttribute", value: "42"}' in schema_str + + # Extended attributes are NOT annotated in @vspec metadata (available via sidecar instead) + assert '{key: "source"' not in schema_str + assert '{key: "quality"' not in schema_str + assert '{key: "calibration"' not in schema_str + assert '{key: "customMetadata"' not in schema_str + assert '{key: "anotherAttribute"' not in schema_str def test_empty_branches_are_skipped(self, caplog): """Test that empty branches (branches with no children) are skipped during export.""" From 4e97abe98e5caa6e1fb7d9fef64b9545c106480d Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Tue, 14 Apr 2026 14:10:24 +0200 Subject: [PATCH 13/18] feat(s2dm): fix struct processing and remove hoisted properties Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- src/vss_tools/exporters/s2dm/type_builders.py | 121 ++++++------------ 1 file changed, 40 insertions(+), 81 deletions(-) diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index d6f46397..617f0818 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -314,6 +314,43 @@ def _sanitize_enum_value_for_graphql(original_value: str) -> tuple[str, bool]: return sanitized, was_modified +def _sort_structs_by_dependencies(struct_fqns: list[str], leaves_df: pd.DataFrame) -> list[str]: + """ + Sort struct FQNs so dependencies come before dependents (topological order). + + Inspects each struct's property datatypes to detect struct-to-struct references, + then uses Kahn's algorithm to produce a safe processing order. + """ + struct_fqn_set = set(struct_fqns) + + deps: dict[str, set[str]] = {fqn: set() for fqn in struct_fqns} + for fqn in struct_fqns: + for _, prop_row in leaves_df[leaves_df["parent"] == fqn].iterrows(): + datatype = str(prop_row.get("datatype", "")) + base = datatype[:-2] if datatype.endswith("[]") else datatype + if base in struct_fqn_set and base != fqn: + deps[fqn].add(base) + + # Kahn's algorithm + result: list[str] = [] + ready = [f for f, d in deps.items() if not d] + remaining = {f: d.copy() for f, d in deps.items() if d} + while ready: + fqn = ready.pop(0) + result.append(fqn) + for other in list(remaining): + remaining[other].discard(fqn) + if not remaining[other]: + ready.append(other) + del remaining[other] + + if remaining: + log.warning(f"Circular struct dependencies detected, processing in original order: {list(remaining)}") + result.extend(remaining) + + return result + + def create_struct_types( data_type_tree: VSSNode | None, vspec_comments: dict[str, Any], @@ -339,7 +376,8 @@ def create_struct_types( branches_df, leaves_df = get_metadata_df(data_type_tree, extended_attributes=extended_attributes) struct_nodes = branches_df[branches_df["type"] == "struct"] - for fqn, struct_row in struct_nodes.iterrows(): + for fqn in _sort_structs_by_dependencies(list(struct_nodes.index), leaves_df): + struct_row = struct_nodes.loc[fqn] # Use short name if mapping exists, otherwise fall back to FQN conversion if short_name_mapping and fqn in short_name_mapping: type_name = short_name_mapping[fqn] @@ -354,8 +392,7 @@ def create_struct_types( fields[field_name] = GraphQLField(GraphQLNonNull(base_type), description=prop_row.get("description", "")) field_path = build_field_path(type_name, field_name) - field_metadata = {"element": "STRUCT_PROPERTY", "fqn": prop_fqn} - vspec_comments["field_vss_types"][field_path] = field_metadata + vspec_comments["field_vss_types"][field_path] = {"element": "STRUCT_PROPERTY", "fqn": prop_fqn} if pd.notna(prop_row.get("min")) or pd.notna(prop_row.get("max")): vspec_comments["field_ranges"][field_path] = { @@ -492,17 +529,6 @@ def get_fields() -> dict[str, GraphQLField]: # Leaf fields for child_fqn, leaf_row in leaves_df[leaves_df["parent"] == fqn].iterrows(): - if branch_row.get("instances"): - instantiate = leaf_row.get("instantiate") - if instantiate is False: - parent_fqn = branch_row.get("parent") - if parent_fqn is None or parent_fqn == "" or pd.isna(parent_fqn): - log.warning( - f"Property '{child_fqn}' has instantiate=false but '{fqn}' has no parent. " - f"Property will be omitted from schema." - ) - continue - field_name = convert_name_for_graphql_schema(leaf_row["name"], GraphQLElementType.FIELD, S2DM_CONVERSIONS) field_path = build_field_path(type_name, field_name) @@ -546,11 +572,6 @@ def get_fields() -> dict[str, GraphQLField]: ) types_registry[child_fqn] = child_type - hoisted_fields = get_hoisted_fields( - child_fqn, child_row, leaves_df, types_registry, unit_enums, vspec_comments, extended_attributes - ) - fields.update(hoisted_fields) - if child_row.get("instances"): # Use natural plural form for list fields (using inflect directly) plural_field_name = _inflect_engine.plural(field_name) @@ -576,68 +597,6 @@ def get_fields() -> dict[str, GraphQLField]: return GraphQLObjectType(name=type_name, fields=get_fields, description=branch_row.get("description", "")) -def get_hoisted_fields( - child_branch_fqn: str, - child_branch_row: pd.Series, - leaves_df: pd.DataFrame, - types_registry: dict[str, Any], - unit_enums: dict[str, GraphQLEnumType], - vspec_comments: dict[str, dict[str, Any]], - extended_attributes: tuple[str, ...] = (), -) -> dict[str, GraphQLField]: - """Get fields to hoist from instantiated child branch to parent.""" - hoisted: dict[str, GraphQLField] = {} - - if not child_branch_row.get("instances"): - return hoisted - - child_leaves = leaves_df[leaves_df["parent"] == child_branch_fqn] - - for leaf_fqn, leaf_row in child_leaves.iterrows(): - instantiate = leaf_row.get("instantiate") - if instantiate is False: - leaf_name = leaf_row["name"] - parent_fqn = child_branch_row["parent"] - - if parent_fqn is None or parent_fqn == "" or pd.isna(parent_fqn): - log.warning( - f"Property '{leaf_fqn}' has instantiate=false but '{child_branch_fqn}' has no parent. " - f"Property will be omitted." - ) - continue - - hoisted_field_name = convert_name_for_graphql_schema( - f"{leaf_name}", GraphQLElementType.FIELD, S2DM_CONVERSIONS - ) - - parent_type_name = convert_name_for_graphql_schema(parent_fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS) - field_path = build_field_path(parent_type_name, hoisted_field_name) - - if leaf_type := _get_vss_type_if_valid(leaf_row): - vspec_comments["field_vss_types"][field_path] = { - "element": leaf_type, - "fqn": leaf_fqn, - "instantiate": False, - } - - if pd.notna(leaf_row.get("min")) or pd.notna(leaf_row.get("max")): - vspec_comments["field_ranges"][field_path] = { - "min": leaf_row.get("min") if pd.notna(leaf_row.get("min")) else None, - "max": leaf_row.get("max") if pd.notna(leaf_row.get("max")) else None, - } - - if pd.notna(leaf_row.get("deprecation")) and leaf_row.get("deprecation").strip(): - vspec_comments["field_deprecated"][field_path] = leaf_row["deprecation"] - - field_type = get_graphql_type_for_leaf(leaf_row, types_registry) - unit_args = _get_unit_args(leaf_row, unit_enums) - hoisted[hoisted_field_name] = GraphQLField( - field_type, args=unit_args, description=leaf_row.get("description", "") - ) - - return hoisted - - def _get_vss_type_if_valid(row: pd.Series) -> str | None: """Get VSS type if valid, else None.""" if vss_type := row.get("type", "").upper(): From 373b6959a784c6d64e38827850b202e152fdb01a Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Fri, 17 Apr 2026 13:37:24 +0200 Subject: [PATCH 14/18] refactor(s2dm): Simplify vspec directive and include instantiate metadata in the lookup file Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- .../s2dm/graphql_directive_processor.py | 18 +---- .../predefined_elements/directives.graphql | 10 +-- .../exporters/s2dm/reference_generator.py | 9 +++ tests/test_s2dm_exporter.py | 79 +++++++++---------- 4 files changed, 52 insertions(+), 64 deletions(-) diff --git a/src/vss_tools/exporters/s2dm/graphql_directive_processor.py b/src/vss_tools/exporters/s2dm/graphql_directive_processor.py index 6f1bfd8f..d9a09376 100644 --- a/src/vss_tools/exporters/s2dm/graphql_directive_processor.py +++ b/src/vss_tools/exporters/s2dm/graphql_directive_processor.py @@ -145,8 +145,7 @@ def _process_allowed_enum_directives( if stripped_line.startswith(enum_value_name) and enum_value_key not in processed_values: if "@vspec" not in line: indent = line[: len(line) - len(line.lstrip())] - # Annotate modified enum value with original value in metadata - directive = f'@vspec(metadata: [{{key: "originalName", value: "{original_value}"}}])' + directive = f'@vspec(originalName: "{original_value}")' lines[i] = f"{indent}{enum_value_name} {directive}" processed_values.add(enum_value_key) @@ -191,7 +190,7 @@ def _process_instance_dimension_enum_directives( if stripped_line.startswith(enum_value_name) and enum_value_key not in processed_values: if "@vspec" not in line: indent = line[: len(line) - len(line.lstrip())] - directive = f'@vspec(metadata: [{{key: "originalName", value: "{original_value}"}}])' + directive = f'@vspec(originalName: "{original_value}")' lines[i] = f"{indent}{enum_value_name} {directive}" processed_values.add(enum_value_key) @@ -200,12 +199,11 @@ def _process_instance_dimension_enum_directives( return lines def _process_field_directives(self, lines: list[str], vspec_comments: dict) -> list[str]: - """Process consolidated field @vspec directives (element + fqn + optional instantiate metadata).""" + """Process field @vspec directives (element + fqn).""" for field_path, vss_info in vspec_comments.get("field_vss_types", {}).items(): type_name, field_name = field_path.split(".", 1) # Use maxsplit=1 to handle field names with dots element = vss_info["element"] fqn = vss_info["fqn"] - instantiate = vss_info.get("instantiate") # Only False for hoisted non-instantiated fields in_type = False for i, line in enumerate(lines): @@ -220,15 +218,7 @@ def _process_field_directives(self, lines: list[str], vspec_comments: dict) -> l continue if in_type and line.strip().startswith(f"{field_name}") and "@vspec" not in line: - if instantiate is False: - directive = ( - f'@vspec(element: {element}, fqn: "{fqn}", ' - + 'metadata: [{key: "instantiate", value: "false"}])' - ) - else: - directive = f'@vspec(element: {element}, fqn: "{fqn}")' - - lines[i] = line.rstrip() + f" {directive}" + lines[i] = line.rstrip() + f' @vspec(element: {element}, fqn: "{fqn}")' break return lines diff --git a/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql b/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql index 45e75f6e..7dfc1320 100644 --- a/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql +++ b/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql @@ -4,16 +4,10 @@ directive @vspec( element: VspecElement, """The Fully Qualified Name (FQN) of the related element in Vspec (aka., path). Example: Vehicle.Cabin.Door.Window.position""" fqn: String - """Additional metadata associated with the mapping.""" - metadata: [KeyValue] + """Original name before sanitization for GraphQL compliance (e.g. for enum values with spaces or special characters).""" + originalName: String ) on OBJECT | FIELD_DEFINITION | ENUM | ENUM_VALUE -"""Key-value pair for storing metadata.""" -input KeyValue { - key: String! - value: String! -} - """Kinds of Vspec elements that can be annotated.""" enum VspecElement { """A groupping entity that contains multiple properties whose values are meant to be read and written independently.""" diff --git a/src/vss_tools/exporters/s2dm/reference_generator.py b/src/vss_tools/exporters/s2dm/reference_generator.py index c199a0eb..fc029814 100644 --- a/src/vss_tools/exporters/s2dm/reference_generator.py +++ b/src/vss_tools/exporters/s2dm/reference_generator.py @@ -77,7 +77,16 @@ def generate_vspec_reference( # Generate VSS lookup spec vspec_file_out = reference_dir / "vspec_lookup_spec.yaml" try: + from anytree import PreOrderIter + tree_data = tree.as_flat_dict(with_extra_attributes=False, extended_attributes=extended_attributes) + # Inject `instantiate: false` for nodes that opt out of instance expansion, + # since `instantiate` is excluded from as_flat_dict by EXPORT_EXCLUDE_ATTRIBUTES. + for node in PreOrderIter(tree): + if hasattr(node, "data") and hasattr(node.data, "instantiate") and node.data.instantiate is False: + fqn = node.get_fqn() + if fqn in tree_data: + tree_data[fqn]["instantiate"] = False if data_type_tree: tree_data["ComplexDataTypes"] = data_type_tree.as_flat_dict( with_extra_attributes=False, extended_attributes=extended_attributes diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index 3fb26182..88d2cdd8 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -504,12 +504,11 @@ def test_modular_export_instance_enum_directives(self, tmp_path): instance_content = instance_file.read_text() # Check for sanitized enum values with @vspec directives - assert 'DRIVER_SIDE @vspec(metadata: [{key: "originalName", value: "DriverSide"}])' in instance_content - assert 'PASSENGER_SIDE @vspec(metadata: [{key: "originalName", value: "PassengerSide"}])' in instance_content + assert 'DRIVER_SIDE @vspec(originalName: "DriverSide")' in instance_content + assert 'PASSENGER_SIDE @vspec(originalName: "PassengerSide")' in instance_content - def test_non_instantiated_property_hoisting(self, tmp_path: Path): - """Test that properties with instantiate=false are hoisted to parent type.""" - # Load the test vspec with non-instantiated properties + def test_non_instantiated_property_stays_in_parent_type(self, tmp_path: Path): + """Test that properties with instantiate=false remain in their defining type.""" tree, _ = get_trees( vspec=Path("tests/vspec/test_non_instantiated_props/test.vspec"), include_dirs=(), @@ -527,29 +526,25 @@ def test_non_instantiated_property_hoisting(self, tmp_path: Path): schema, unit_metadata, allowed_metadata, vspec_comments = generate_s2dm_schema(tree, use_short_names=False) schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) - # Verify that Vehicle_Cabin_Door type doesn't have someSignal + # Verify Vehicle_Cabin_Door type contains someSignal (not hoisted) assert "type Vehicle_Cabin_Door @vspec" in schema_str door_type_start = schema_str.find("type Vehicle_Cabin_Door @vspec") - door_type_end = schema_str.find("\n}", door_type_start) + 2 # Include closing brace + door_type_end = schema_str.find("\n}", door_type_start) + 2 door_type_content = schema_str[door_type_start:door_type_end] - # someSignal should NOT be on Door type - assert "someSignal" not in door_type_content - # But isOpen and isLocked should be + # someSignal stays in Door type + assert "someSignal" in door_type_content assert "isOpen" in door_type_content assert "isLocked" in door_type_content - # Verify that Vehicle_Cabin type has doorSomeSignal (hoisted) + # Vehicle_Cabin type should NOT have someSignal (no hoisting) assert "type Vehicle_Cabin @vspec" in schema_str cabin_type_start = schema_str.find("type Vehicle_Cabin @vspec") - cabin_type_end = schema_str.find("\n}", cabin_type_start) + 2 # Include closing brace + cabin_type_end = schema_str.find("\n}", cabin_type_start) + 2 cabin_type_content = schema_str[cabin_type_start:cabin_type_end] - # someSignal should be on Cabin type (hoisted without branch prefix) - assert "someSignal" in cabin_type_content - # Verify it has the instantiate=false metadata - assert 'metadata: [{key: "instantiate", value: "false"}]' in cabin_type_content - # And doors array field should also be there (natural plural) + assert "someSignal" not in cabin_type_content + # doors array field should still be there assert "doors" in cabin_type_content def test_enum_value_sanitization_with_spaces(self): @@ -622,23 +617,23 @@ def test_enum_sanitization_schema_output_with_directives(self): # Check that enum type has @vspec directive with element assert "enum Vehicle_Cabin_LightMode_Enum @vspec" in schema_str - # Check that modified enum values have @vspec directives with originalName metadata + # Check that modified enum values have @vspec directives with originalName argument # "some value" -> SOME_VALUE assert ( - 'SOME_VALUE @vspec(metadata: [{key: "originalName", value: "some value"}])' in schema_str - or 'SOME_VALUE @vspec(metadata: [{key: "originalName", value: "SOME VALUE"}])' in schema_str + 'SOME_VALUE @vspec(originalName: "some value")' in schema_str + or 'SOME_VALUE @vspec(originalName: "SOME VALUE")' in schema_str ) # "another-value" -> ANOTHER_VALUE - assert 'ANOTHER_VALUE @vspec(metadata: [{key: "originalName", value: "another-value"}])' in schema_str + assert 'ANOTHER_VALUE @vspec(originalName: "another-value")' in schema_str - # YET_ANOTHER should not have originalName metadata (wasn't modified) - assert 'YET_ANOTHER @vspec(metadata: [{key: "originalName"' not in schema_str + # YET_ANOTHER should not have originalName (wasn't modified) + assert "YET_ANOTHER @vspec(originalName:" not in schema_str # Check SeatPosition enum assert "enum Vehicle_Cabin_SeatPosition_Enum @vspec" in schema_str - assert 'FRONT_LEFT @vspec(metadata: [{key: "originalName", value: "front left"}])' in schema_str - assert 'FRONT_RIGHT @vspec(metadata: [{key: "originalName", value: "front right"}])' in schema_str + assert 'FRONT_LEFT @vspec(originalName: "front left")' in schema_str + assert 'FRONT_RIGHT @vspec(originalName: "front right")' in schema_str def test_enum_camelcase_sanitization(self): """Test that camelCase enum values are properly converted using caseconverter.""" @@ -698,23 +693,23 @@ def test_camelcase_enums_schema_generation(self): assert "enum Vehicle_Component_Type_Enum @vspec" in schema_str # AbCd should be converted to AB_CD with originalName annotation - assert 'AB_CD @vspec(metadata: [{key: "originalName", value: "AbCd"}])' in schema_str + assert 'AB_CD @vspec(originalName: "AbCd")' in schema_str # AAA, BBB, CCC, DDD should not have originalName (no change needed) - assert 'AAA @vspec(metadata: [{key: "originalName"' not in schema_str - assert 'BBB @vspec(metadata: [{key: "originalName"' not in schema_str + assert "AAA @vspec(originalName:" not in schema_str + assert "BBB @vspec(originalName:" not in schema_str # Check Connection.Protocol enum assert "enum Vehicle_Connection_Protocol_Enum @vspec" in schema_str - assert 'HTTPS_PROTOCOL @vspec(metadata: [{key: "originalName", value: "HTTPSProtocol"}])' in schema_str - assert 'TCP_PROTOCOL @vspec(metadata: [{key: "originalName", value: "TCPProtocol"}])' in schema_str - assert 'UDP_PROTOCOL @vspec(metadata: [{key: "originalName", value: "UDPProtocol"}])' in schema_str + assert 'HTTPS_PROTOCOL @vspec(originalName: "HTTPSProtocol")' in schema_str + assert 'TCP_PROTOCOL @vspec(originalName: "TCPProtocol")' in schema_str + assert 'UDP_PROTOCOL @vspec(originalName: "UDPProtocol")' in schema_str # Check Status.Code enum assert "enum Vehicle_Status_Code_Enum @vspec" in schema_str - assert 'IO_ERROR @vspec(metadata: [{key: "originalName", value: "IOError"}])' in schema_str - assert 'XML_PARSER @vspec(metadata: [{key: "originalName", value: "XMLParser"}])' in schema_str - assert 'SOME_API_KEY @vspec(metadata: [{key: "originalName", value: "someAPIKey"}])' in schema_str + assert 'IO_ERROR @vspec(originalName: "IOError")' in schema_str + assert 'XML_PARSER @vspec(originalName: "XMLParser")' in schema_str + assert 'SOME_API_KEY @vspec(originalName: "someAPIKey")' in schema_str def test_instance_dimension_enum_sanitization(self): """Test that instance dimension enum values are properly sanitized and annotated.""" @@ -736,20 +731,20 @@ def test_instance_dimension_enum_sanitization(self): # Check that Row instance enum is created and values are sanitized assert "enum Vehicle_Cabin_InstanceTag_Dimension1" in schema_str - assert 'ROW1 @vspec(metadata: [{key: "originalName", value: "Row1"}])' in schema_str - assert 'ROW2 @vspec(metadata: [{key: "originalName", value: "Row2"}])' in schema_str + assert 'ROW1 @vspec(originalName: "Row1")' in schema_str + assert 'ROW2 @vspec(originalName: "Row2")' in schema_str # Check that DriverSide/PassengerSide instance enum is created and values are sanitized assert "enum Vehicle_Cabin_Seat_InstanceTag_Dimension1" in schema_str - assert 'DRIVER_SIDE @vspec(metadata: [{key: "originalName", value: "DriverSide"}])' in schema_str - assert 'PASSENGER_SIDE @vspec(metadata: [{key: "originalName", value: "PassengerSide"}])' in schema_str + assert 'DRIVER_SIDE @vspec(originalName: "DriverSide")' in schema_str + assert 'PASSENGER_SIDE @vspec(originalName: "PassengerSide")' in schema_str # Check that FrontLeft/FrontRight/RearLeft/RearRight instance enum is created and values are sanitized assert "enum Vehicle_Cabin_Seat_Position_InstanceTag_Dimension1" in schema_str - assert 'FRONT_LEFT @vspec(metadata: [{key: "originalName", value: "FrontLeft"}])' in schema_str - assert 'FRONT_RIGHT @vspec(metadata: [{key: "originalName", value: "FrontRight"}])' in schema_str - assert 'REAR_LEFT @vspec(metadata: [{key: "originalName", value: "RearLeft"}])' in schema_str - assert 'REAR_RIGHT @vspec(metadata: [{key: "originalName", value: "RearRight"}])' in schema_str + assert 'FRONT_LEFT @vspec(originalName: "FrontLeft")' in schema_str + assert 'FRONT_RIGHT @vspec(originalName: "FrontRight")' in schema_str + assert 'REAR_LEFT @vspec(originalName: "RearLeft")' in schema_str + assert 'REAR_RIGHT @vspec(originalName: "RearRight")' in schema_str def test_extended_attributes_not_in_schema_metadata(self): """Test that extended attributes are not annotated in @vspec schema metadata. From 1a3a88a3aa50f4b14e4acf89768e78fd09277398 Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Tue, 28 Apr 2026 21:35:27 +0200 Subject: [PATCH 15/18] feat(s2dm): map units to qudt references Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- src/vss_tools/exporters/s2dm/constants.py | 1 + .../s2dm/graphql_directive_processor.py | 29 ++- .../exporters/s2dm/modular_export_utils.py | 4 +- .../predefined_elements/directives.graphql | 3 + .../s2dm/predefined_elements/qudt_mappings.py | 205 ++++++++++++++++++ .../exporters/s2dm/schema_generator.py | 3 +- src/vss_tools/exporters/s2dm/type_builders.py | 100 +++++---- tests/test_s2dm_exporter.py | 20 +- .../desired_output_seat_example.graphql | 52 +++-- 9 files changed, 333 insertions(+), 84 deletions(-) create mode 100644 src/vss_tools/exporters/s2dm/predefined_elements/qudt_mappings.py diff --git a/src/vss_tools/exporters/s2dm/constants.py b/src/vss_tools/exporters/s2dm/constants.py index cc300d36..50e62b05 100644 --- a/src/vss_tools/exporters/s2dm/constants.py +++ b/src/vss_tools/exporters/s2dm/constants.py @@ -63,6 +63,7 @@ def get_s2dm_conversions() -> Dict[GraphQLElementType, Callable[[str], str]]: VSpecDirective = CUSTOM_DIRECTIVES["vspec"] RangeDirective = CUSTOM_DIRECTIVES["range"] InstanceTagDirective = CUSTOM_DIRECTIVES["instanceTag"] +ReferenceDirective = CUSTOM_DIRECTIVES["reference"] class S2DMExporterException(Exception): diff --git a/src/vss_tools/exporters/s2dm/graphql_directive_processor.py b/src/vss_tools/exporters/s2dm/graphql_directive_processor.py index d9a09376..3ae21fdd 100644 --- a/src/vss_tools/exporters/s2dm/graphql_directive_processor.py +++ b/src/vss_tools/exporters/s2dm/graphql_directive_processor.py @@ -70,17 +70,21 @@ def _process_unit_enum_directives( """ Process unit enum directives. - Annotates enum type with @vspec(element: QUANTITY_KIND) - and individual enum values with @vspec(element: UNIT). + Annotates enum type with @vspec(element: QUANTITY_KIND, originalName: "{vss_quantity}") + and individual enum values with @vspec(element: UNIT, originalName: "{vss_key}") and, + when available, @reference(uri: "{qudt_uri}"). """ - for quantity, units_data in unit_enums_metadata.items(): - enum_name = f"{convert_name_for_graphql_schema(quantity, GraphQLElementType.TYPE)}UnitEnum" + for qudt_quantity_kind, quantity_data in unit_enums_metadata.items(): + enum_name = f"{convert_name_for_graphql_schema(qudt_quantity_kind, GraphQLElementType.TYPE)}Unit" + vss_quantity = quantity_data.get("vss_quantity", "") + units_data: dict = quantity_data.get("units", {}) in_target_enum = False for i, line in enumerate(lines): if line.strip().startswith(f"enum {enum_name}"): if "@vspec" not in line: - lines[i] = line.replace(" {", " @vspec(element: QUANTITY_KIND) {") + directive = f'@vspec(element: QUANTITY_KIND, originalName: "{vss_quantity}")' + lines[i] = line.replace(" {", f" {directive} {{") in_target_enum = True continue elif line.strip().startswith("enum ") and in_target_enum: @@ -93,15 +97,18 @@ def _process_unit_enum_directives( if in_target_enum and line.strip() and not line.strip().startswith('"'): stripped_line = line.strip() - for unit_key, unit_info in units_data.items(): - unit_name = unit_info["name"] - enum_value_name = convert_name_for_graphql_schema(unit_name, GraphQLElementType.ENUM_VALUE) + for vss_key, unit_info in units_data.items(): + qudt_unit = unit_info["qudt_unit"] + qudt_uri = unit_info.get("qudt_uri") - enum_value_key = f"{enum_name}.{enum_value_name}" - if stripped_line.startswith(enum_value_name) and enum_value_key not in processed_values: + enum_value_key = f"{enum_name}.{qudt_unit}" + if stripped_line.startswith(qudt_unit) and enum_value_key not in processed_values: if "@vspec" not in line: indent = line[: len(line) - len(line.lstrip())] - lines[i] = f"{indent}{enum_value_name} @vspec(element: UNIT)" + directive = f'@vspec(element: UNIT, originalName: "{vss_key}")' + if qudt_uri: + directive += f' @reference(uri: "{qudt_uri}")' + lines[i] = f"{indent}{qudt_unit} {directive}" processed_values.add(enum_value_key) break diff --git a/src/vss_tools/exporters/s2dm/modular_export_utils.py b/src/vss_tools/exporters/s2dm/modular_export_utils.py index baef5c15..933d42f9 100644 --- a/src/vss_tools/exporters/s2dm/modular_export_utils.py +++ b/src/vss_tools/exporters/s2dm/modular_export_utils.py @@ -57,7 +57,7 @@ def analyze_schema_for_flat_domains(schema: GraphQLSchema) -> dict[str, list[str domain_files[file_name] = [type_name] elif is_enum_type(graphql_type): # Group enums by category - if "UnitEnum" in type_name: + if type_name.endswith("Unit") and "_" not in type_name: # Unit enums go to other/units.graphql enum_file = "other/units.graphql" if enum_file not in domain_files: @@ -170,7 +170,7 @@ def analyze_schema_for_nested_domains(schema: GraphQLSchema) -> dict[str, list[s type_groups[domain_path].append(type_name) elif is_enum_type(graphql_type): # Group enums by category - if "UnitEnum" in type_name: + if type_name.endswith("Unit") and "_" not in type_name: # Unit enums go to other/units.graphql enum_file = "other/units.graphql" if enum_file not in type_groups: diff --git a/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql b/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql index 7dfc1320..02fb213d 100644 --- a/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql +++ b/src/vss_tools/exporters/s2dm/predefined_elements/directives.graphql @@ -32,3 +32,6 @@ enum VspecElement { directive @range(min: Float, max: Float) on FIELD_DEFINITION directive @instanceTag on OBJECT + +"""Reference directive for linking to an external ontology resource (e.g. a QUDT unit URI).""" +directive @reference(uri: String) on ENUM_VALUE diff --git a/src/vss_tools/exporters/s2dm/predefined_elements/qudt_mappings.py b/src/vss_tools/exporters/s2dm/predefined_elements/qudt_mappings.py new file mode 100644 index 00000000..dec09a29 --- /dev/null +++ b/src/vss_tools/exporters/s2dm/predefined_elements/qudt_mappings.py @@ -0,0 +1,205 @@ +# vspec unit key → QUDT QuantityKind and unit +QUDT_MAPPING: dict[str, dict[str, str]] = { + # length + "mm": {"qudt_uri": "http://qudt.org/vocab/unit/MilliM", "qudt_quantity_kind": "Length", "qudt_unit": "MILLIM"}, + "cm": {"qudt_uri": "http://qudt.org/vocab/unit/CentiM", "qudt_quantity_kind": "Length", "qudt_unit": "CENTIM"}, + "m": {"qudt_uri": "http://qudt.org/vocab/unit/M", "qudt_quantity_kind": "Length", "qudt_unit": "M"}, + "km": {"qudt_uri": "http://qudt.org/vocab/unit/KiloM", "qudt_quantity_kind": "Length", "qudt_unit": "KILOM"}, + "inch": {"qudt_uri": "http://qudt.org/vocab/unit/IN", "qudt_quantity_kind": "Length", "qudt_unit": "IN"}, + # velocity + "km/h": { + "qudt_uri": "http://qudt.org/vocab/unit/KiloM-PER-HR", + "qudt_quantity_kind": "Velocity", + "qudt_unit": "KILOM_PER_HR", + }, + "m/s": { + "qudt_uri": "http://qudt.org/vocab/unit/M-PER-SEC", + "qudt_quantity_kind": "Velocity", + "qudt_unit": "M_PER_SEC", + }, + # acceleration + "m/s^2": { + "qudt_uri": "http://qudt.org/vocab/unit/M-PER-SEC2", + "qudt_quantity_kind": "Acceleration", + "qudt_unit": "M_PER_SEC2", + }, + "cm/s^2": { + "qudt_uri": "http://qudt.org/vocab/unit/CentiM-PER-SEC2", + "qudt_quantity_kind": "Acceleration", + "qudt_unit": "CENTIM_PER_SEC2", + }, + # volume + "ml": {"qudt_uri": "http://qudt.org/vocab/unit/MilliL", "qudt_quantity_kind": "Volume", "qudt_unit": "MILLIL"}, + "l": {"qudt_uri": "http://qudt.org/vocab/unit/L", "qudt_quantity_kind": "Volume", "qudt_unit": "L"}, + "cm^3": {"qudt_uri": "http://qudt.org/vocab/unit/CentiM3", "qudt_quantity_kind": "Volume", "qudt_unit": "CENTIM3"}, + # temperature + "Celsius": { + "qudt_uri": "http://qudt.org/vocab/unit/DEG_C", + "qudt_quantity_kind": "Temperature", + "qudt_unit": "DEG_C", + }, + "Fahrenheit": { + "qudt_uri": "http://qudt.org/vocab/unit/DEG_F", + "qudt_quantity_kind": "Temperature", + "qudt_unit": "DEG_F", + }, + # angle + "degrees": {"qudt_uri": "http://qudt.org/vocab/unit/Degree", "qudt_quantity_kind": "Angle", "qudt_unit": "DEG"}, + # angular speed + "degrees/s": { + "qudt_uri": "http://qudt.org/vocab/unit/DEG-PER-SEC", + "qudt_quantity_kind": "AngularVelocity", + "qudt_unit": "DEG_PER_SEC", + }, + "rad/s": { + "qudt_uri": "http://qudt.org/vocab/unit/RAD-PER-SEC", + "qudt_quantity_kind": "AngularVelocity", + "qudt_unit": "RAD_PER_SEC", + }, + # power + "W": {"qudt_uri": "http://qudt.org/vocab/unit/W", "qudt_quantity_kind": "Power", "qudt_unit": "W"}, + "kW": {"qudt_uri": "http://qudt.org/vocab/unit/KiloW", "qudt_quantity_kind": "Power", "qudt_unit": "KILOW"}, + "PS": {"qudt_uri": "http://qudt.org/vocab/unit/HP", "qudt_quantity_kind": "Power", "qudt_unit": "HP"}, + # energy / work + "kWh": {"qudt_uri": "http://qudt.org/vocab/unit/KiloW-HR", "qudt_quantity_kind": "Energy", "qudt_unit": "KILOW_HR"}, + # mass + "g": {"qudt_uri": "http://qudt.org/vocab/unit/GM", "qudt_quantity_kind": "Mass", "qudt_unit": "GM"}, + "kg": {"qudt_uri": "http://qudt.org/vocab/unit/KiloGM", "qudt_quantity_kind": "Mass", "qudt_unit": "KILOGM"}, + "lbs": {"qudt_uri": "http://qudt.org/vocab/unit/LB", "qudt_quantity_kind": "Mass", "qudt_unit": "LB"}, + # voltage + "V": {"qudt_uri": "http://qudt.org/vocab/unit/V", "qudt_quantity_kind": "Voltage", "qudt_unit": "V"}, + # electric current + "A": {"qudt_uri": "http://qudt.org/vocab/unit/A", "qudt_quantity_kind": "ElectricCurrent", "qudt_unit": "A"}, + # electric charge + "Ah": {"qudt_uri": "http://qudt.org/vocab/unit/A_HR", "qudt_quantity_kind": "ElectricCharge", "qudt_unit": "A_HR"}, + # duration + "ns": {"qudt_uri": "http://qudt.org/vocab/unit/NanoSEC", "qudt_quantity_kind": "Time", "qudt_unit": "NANOSEC"}, + "ms": {"qudt_uri": "http://qudt.org/vocab/unit/MilliSEC", "qudt_quantity_kind": "Time", "qudt_unit": "MILLISEC"}, + "s": {"qudt_uri": "http://qudt.org/vocab/unit/SEC", "qudt_quantity_kind": "Time", "qudt_unit": "SEC"}, + "min": {"qudt_uri": "http://qudt.org/vocab/unit/MIN", "qudt_quantity_kind": "Time", "qudt_unit": "MIN"}, + "h": {"qudt_uri": "http://qudt.org/vocab/unit/HR", "qudt_quantity_kind": "Time", "qudt_unit": "HR"}, + "day": {"qudt_uri": "http://qudt.org/vocab/unit/DAY", "qudt_quantity_kind": "Time", "qudt_unit": "DAY"}, + "weeks": {"qudt_uri": "http://qudt.org/vocab/unit/WK", "qudt_quantity_kind": "Time", "qudt_unit": "WK"}, + "months": {"qudt_uri": "http://qudt.org/vocab/unit/MO", "qudt_quantity_kind": "Time", "qudt_unit": "MO"}, + "years": {"qudt_uri": "http://qudt.org/vocab/unit/YR", "qudt_quantity_kind": "Time", "qudt_unit": "YR"}, + # pressure + "mbar": { + "qudt_uri": "http://qudt.org/vocab/unit/MilliBAR", + "qudt_quantity_kind": "VaporPressure", + "qudt_unit": "MILLIBAR", + }, + "Pa": {"qudt_uri": "http://qudt.org/vocab/unit/PA", "qudt_quantity_kind": "VaporPressure", "qudt_unit": "PA"}, + "kPa": { + "qudt_uri": "http://qudt.org/vocab/unit/KiloPA", + "qudt_quantity_kind": "VaporPressure", + "qudt_unit": "KILOPA", + }, + "psi": {"qudt_uri": "http://qudt.org/vocab/unit/PSI", "qudt_quantity_kind": "VaporPressure", "qudt_unit": "PSI"}, + # mass flow rate + "g/s": { + "qudt_uri": "http://qudt.org/vocab/unit/GM-PER-SEC", + "qudt_quantity_kind": "MassFlowRate", + "qudt_unit": "GM_PER_SEC", + }, + # mass per length + "g/km": { + "qudt_uri": "http://qudt.org/vocab/unit/GM-PER-KiloM", + "qudt_quantity_kind": "MassPerLength", + "qudt_unit": "GM_PER_KILOM", + }, + # volume flow rate + "l/h": { + "qudt_uri": "http://qudt.org/vocab/unit/L-PER-HR", + "qudt_quantity_kind": "VolumeFlowRate", + "qudt_unit": "L_PER_HR", + }, + # force + "N": {"qudt_uri": "http://qudt.org/vocab/unit/N", "qudt_quantity_kind": "Force", "qudt_unit": "N"}, + "kN": {"qudt_uri": "http://qudt.org/vocab/unit/KiloN", "qudt_quantity_kind": "Force", "qudt_unit": "KILON"}, + # torque + "Nm": {"qudt_uri": "http://qudt.org/vocab/unit/N-M", "qudt_quantity_kind": "Torque", "qudt_unit": "N_M"}, + # rotational speed + "rpm": { + "qudt_uri": "http://qudt.org/vocab/unit/REV-PER-MIN", + "qudt_quantity_kind": "RotationalVelocity", + "qudt_unit": "REV_PER_MIN", + }, + # frequency + "Hz": {"qudt_uri": "http://qudt.org/vocab/unit/HZ", "qudt_quantity_kind": "Frequency", "qudt_unit": "HZ"}, + "cpm": {"qudt_quantity_kind": "RotationalFrequency", "qudt_unit": "CYC_PER_MIN"}, # Custom (not present in QUDT) + "bpm": { + "qudt_uri": "http://qudt.org/vocab/unit/BEAT-PER-MIN", + "qudt_quantity_kind": "HeartRate", + "qudt_unit": "BEAT_PER_MIN", + }, + # relation + "ratio": { + "qudt_uri": "http://qudt.org/vocab/unit/ONE-PER-ONE", + "qudt_quantity_kind": "DimensionlessRatio", + "qudt_unit": "ONE_PER_ONE", + }, + "percent": { + "qudt_uri": "http://qudt.org/vocab/unit/PERCENT", + "qudt_quantity_kind": "DimensionlessRatio", + "qudt_unit": "PERCENT", + }, + "nm/km": { + "qudt_quantity_kind": "DimensionlessRatio", + "qudt_unit": "NANOM_PER_KILOM", + }, # Custom (not present in QUDT) + "dBm": { + "qudt_uri": "http://qudt.org/vocab/unit/DeciB-MilliW", + "qudt_quantity_kind": "Unknown", + "qudt_unit": "DECIB_MILLIW", + }, + "dB": { + "qudt_uri": "http://qudt.org/vocab/unit/DeciB", + "qudt_quantity_kind": "SoundPowerLevel", + "qudt_unit": "DECIB", + }, + # resistance + "Ohm": {"qudt_uri": "http://qudt.org/vocab/unit/OHM", "qudt_quantity_kind": "Resistance", "qudt_unit": "OHM"}, + # iluminance + "lx": { + "qudt_uri": "http://qudt.org/vocab/unit/LUX", + "qudt_quantity_kind": "LuminousFluxPerArea", + "qudt_unit": "LUX", + }, + # OTHERS + # ------ + # rating + "stars": {"qudt_quantity_kind": "Rating", "qudt_unit": "STARS"}, # Custom (not present in QUDT) + # datetime + "unix-time": {"qudt_quantity_kind": "DateTime", "qudt_unit": "UNIX_TIME"}, # Custom (not present in QUDT) + "iso8601": {"qudt_quantity_kind": "DateTime", "qudt_unit": "ISO8601"}, # Custom (not present in QUDT) + # energy-consumption-per-distance + "kWh/km": { + "qudt_quantity_kind": "EnergyPerDistance", + "qudt_unit": "KILOW_HR_PER_100KILOM", + }, # Custom (not present in QUDT) + "Wh/km": {"qudt_quantity_kind": "EnergyPerDistance", "qudt_unit": "W_HR_PER_KILOM"}, # Custom (not present in QUDT) + # volume-per-distance + "ml/100km": { + "qudt_quantity_kind": "VolumePerDistance", + "qudt_unit": "MILLIL_PER_100KILOM", + }, # Custom (not present in QUDT) + "l/100km": { + "qudt_quantity_kind": "VolumePerDistance", + "qudt_unit": "L_PER_100KILOM", + }, # Custom (not present in QUDT) + # distance-per-volume + "mpg-us": { + "qudt_quantity_kind": "DistancePerVolume", + "qudt_unit": "MILE_PER_GAL_US", + }, # Custom (not present in QUDT) + "mpg-uk": { + "qudt_quantity_kind": "DistancePerVolume", + "qudt_unit": "MILE_PER_GAL_UK", + }, # Custom (not present in QUDT) + "mpge": { + "qudt_quantity_kind": "DistancePerVolume", + "qudt_unit": "MILE_PER_GAL_US_EQ", + }, # Custom (not present in QUDT) + "mpg": {"qudt_quantity_kind": "DistancePerVolume", "qudt_unit": "MILE_PER_GAL"}, # Custom (not present in QUDT) + "km/l": {"qudt_quantity_kind": "DistancePerVolume", "qudt_unit": "KILOM_PER_L"}, # Custom (not present in QUDT) +} diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index 6efc4463..38ee45cf 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -49,6 +49,7 @@ VSpecDirective = CUSTOM_DIRECTIVES["vspec"] RangeDirective = CUSTOM_DIRECTIVES["range"] InstanceTagDirective = CUSTOM_DIRECTIVES["instanceTag"] +ReferenceDirective = CUSTOM_DIRECTIVES["reference"] # Initialize directive processor directive_processor = GraphQLDirectiveProcessor() @@ -182,7 +183,7 @@ def generate_s2dm_schema( schema = GraphQLSchema( query=query, types=get_vss_scalar_types() + list(types_registry.values()) + list(unit_enums.values()), - directives=[VSpecDirective, RangeDirective, InstanceTagDirective], + directives=[VSpecDirective, RangeDirective, InstanceTagDirective, ReferenceDirective], ) return schema, unit_metadata, allowed_metadata, vspec_comments diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index 617f0818..0bc99815 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -44,6 +44,7 @@ from .graphql_scalars import VSS_DATATYPE_MAP from .graphql_utils import GraphQLElementType, convert_name_for_graphql_schema from .metadata_tracker import build_field_path +from .predefined_elements.qudt_mappings import QUDT_MAPPING # Initialize inflect engine for pluralization (singleton) _inflect_engine = inflect.engine() @@ -82,52 +83,69 @@ def _check_and_collect_plural_type_name( ) -def create_unit_enums() -> tuple[dict[str, GraphQLEnumType], dict[str, dict[str, dict[str, str]]]]: +def create_unit_enums() -> tuple[dict[str, GraphQLEnumType], dict[str, dict]]: """ - Create GraphQL enum types for VSS units grouped by quantity. + Create GraphQL enum types for VSS units grouped by QUDT quantity kind. - Generates enums like LengthUnitEnum containing all length units (km, m, cm, etc.). + Generates enums like LengthUnit containing all length units (MILLIM, M, etc.) using + QUDT unit codes as enum value names. Each enum is keyed by its QUDT quantity kind + (e.g. "Length"), and enum values carry the VSS unit key as their internal GraphQL value. Returns: - Tuple of (unit_enums, unit_metadata) + Tuple of (unit_enums, unit_metadata) where unit_metadata maps + QUDT quantity kind → {"vss_quantity": str, "units": {vss_key → {qudt_unit, qudt_uri?}}} """ - unit_enums = {} - unit_metadata = {} - - for quantity, units in _get_quantity_units().items(): - enum_name = f"{convert_name_for_graphql_schema(quantity, GraphQLElementType.ENUM, S2DM_CONVERSIONS)}UnitEnum" - values = { - convert_name_for_graphql_schema( - info["name"], GraphQLElementType.ENUM_VALUE, S2DM_CONVERSIONS - ): GraphQLEnumValue(key) - for key, info in units.items() - } - unit_enums[quantity] = GraphQLEnumType(enum_name, values, description=f'Units for "{quantity}"') - unit_metadata[quantity] = units + unit_enums: dict[str, GraphQLEnumType] = {} + unit_metadata: dict[str, dict] = {} + + for qudt_quantity_kind, quantity_data in _get_quantity_units().items(): + enum_name = ( + f"{convert_name_for_graphql_schema(qudt_quantity_kind, GraphQLElementType.ENUM, S2DM_CONVERSIONS)}Unit" + ) + values = {info["qudt_unit"]: GraphQLEnumValue(vss_key) for vss_key, info in quantity_data["units"].items()} + unit_enums[qudt_quantity_kind] = GraphQLEnumType( + enum_name, values, description=f'Units for "{qudt_quantity_kind}"' + ) + unit_metadata[qudt_quantity_kind] = quantity_data return unit_enums, unit_metadata -def _get_quantity_units() -> dict[str, dict[str, dict[str, str]]]: - """Extract and organize units from VSS registry by quantity.""" - quantity_units: dict[str, dict[str, dict[str, str]]] = {} - processed_units = set() +def _get_quantity_units() -> dict[str, dict]: + """ + Build a mapping from QUDT quantity kind to units, sourced from QUDT_MAPPING. + + Cross-references VSS dynamic_units to obtain the VSS quantity key (used as + originalName on the GraphQL enum type). Units whose VSS key is not registered + in dynamic_units are skipped with a warning. + + Returns: + dict mapping qudt_quantity_kind → + {"vss_quantity": str, "units": {vss_key → {"qudt_unit": str, "qudt_uri": str|None}}} + """ + quantity_units: dict[str, dict] = {} - for unit_key, unit_data in dynamic_units.items(): - unit_id = id(unit_data) - if unit_id in processed_units: + for vss_key, qudt_info in QUDT_MAPPING.items(): + qudt_quantity_kind = qudt_info.get("qudt_quantity_kind", "") + qudt_unit = qudt_info.get("qudt_unit", "") + if not qudt_quantity_kind or not qudt_unit: continue - quantity = unit_data.quantity - unit_display_name = unit_data.unit - actual_unit_key = unit_data.key or unit_key + # Cross-reference VSS dynamic_units for the VSS quantity key + vss_unit_data = dynamic_units.get(vss_key) + if vss_unit_data is None: + log.debug(f"QUDT unit '{vss_key}' not found in loaded dynamic_units; skipping.") + continue - if quantity and unit_display_name: - if quantity not in quantity_units: - quantity_units[quantity] = {} + vss_quantity = vss_unit_data.quantity or "" - quantity_units[quantity][actual_unit_key] = {"name": unit_display_name, "key": actual_unit_key} - processed_units.add(unit_id) + if qudt_quantity_kind not in quantity_units: + quantity_units[qudt_quantity_kind] = {"vss_quantity": vss_quantity, "units": {}} + + quantity_units[qudt_quantity_kind]["units"][vss_key] = { + "qudt_unit": qudt_unit, + "qudt_uri": qudt_info.get("qudt_uri"), + } return quantity_units @@ -606,23 +624,31 @@ def _get_vss_type_if_valid(row: pd.Series) -> str | None: def _get_unit_args(leaf_row: pd.Series, unit_enums: dict[str, GraphQLEnumType]) -> dict[str, GraphQLArgument]: - """Generate unit argument for fields with units.""" + """Generate unit argument for fields with units, using QUDT-based enum values.""" unit = leaf_row.get("unit", "") if not unit: return {} - unit_data = dynamic_units[unit.lower()] - if not unit_data.quantity: + qudt_info = QUDT_MAPPING.get(unit) + if qudt_info is None: + log.warning(f"Unit '{unit}' has no QUDT mapping; unit argument will not be generated.") + return {} + + qudt_quantity_kind = qudt_info.get("qudt_quantity_kind", "") + qudt_unit = qudt_info.get("qudt_unit", "") + if not qudt_quantity_kind or not qudt_unit: return {} - unit_enum = unit_enums.get(unit_data.quantity) + unit_enum = unit_enums.get(qudt_quantity_kind) if not unit_enum: log.warning( - f"Unit '{unit}' with quantity '{unit_data.quantity}' has no corresponding GraphQL enum. " + f"Unit '{unit}' with QUDT quantity kind '{qudt_quantity_kind}' has no corresponding GraphQL enum. " "Unit argument will not be generated." ) return {} + # default_value must be the internal Python value stored in GraphQLEnumValue (the VSS key). + # graphql-core's serializer maps that internal value → enum name (qudt_unit) when printing SDL. return {"unit": GraphQLArgument(type_=unit_enum, default_value=unit)} diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index 88d2cdd8..0a3cfa83 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -155,19 +155,19 @@ def test_unit_enums_generation(self): schema_str = print_schema(schema) # Check that unit enums are generated - assert "enum LengthUnitEnum" in schema_str - assert "enum AngleUnitEnum" in schema_str - assert "enum RelationUnitEnum" in schema_str + assert "enum LengthUnit" in schema_str + assert "enum AngleUnit" in schema_str + assert "enum DimensionlessRatioUnit" in schema_str - # Check that enum values use uppercase unit names - assert "MILLIMETER" in schema_str - assert "DEGREE" in schema_str + # Check that enum values use QUDT unit codes + assert "MILLIM" in schema_str + assert "DEG" in schema_str assert "PERCENT" in schema_str # Check that unit arguments are added to fields with proper defaults - assert "unit: LengthUnitEnum = MILLIMETER" in schema_str - assert "unit: AngleUnitEnum = DEGREE" in schema_str - assert "unit: RelationUnitEnum = PERCENT" in schema_str + assert "unit: LengthUnit = MILLIM" in schema_str + assert "unit: AngleUnit = DEG" in schema_str + assert "unit: DimensionlessRatioUnit = PERCENT" in schema_str def test_vspec_comment_directives(self): """Test that @vspec comment directives are generated correctly.""" @@ -771,7 +771,7 @@ def test_extended_attributes_not_in_schema_metadata(self): schema_str = print_schema_with_vspec_directives(schema, unit_metadata, allowed_metadata, vspec_comments) # Fields are still annotated with element + fqn - assert "speed(unit: RelationUnitEnum = PERCENT): Float" in schema_str + assert "speed(unit: DimensionlessRatioUnit = PERCENT): Float" in schema_str assert '@vspec(element: SENSOR, fqn: "Vehicle.Speed"' in schema_str assert '@vspec(element: SENSOR, fqn: "Vehicle.Temperature"' in schema_str assert "model: String" in schema_str diff --git a/tests/vspec/test_s2dm/desired_output_seat_example.graphql b/tests/vspec/test_s2dm/desired_output_seat_example.graphql index 1c1735bd..cfb86674 100644 --- a/tests/vspec/test_s2dm/desired_output_seat_example.graphql +++ b/tests/vspec/test_s2dm/desired_output_seat_example.graphql @@ -39,11 +39,11 @@ type Vehicle_Cabin_Seat { instanceTag: Vehicle_Cabin_Seat_InstanceTag """Heating or Cooling requsted for the Item. -100 = Maximum cooling, 0 = Heating/cooling deactivated, 100 = Maximum heating.""" - heatingCooling(unit: RelationUnitEnum = PERCENT): Int8 + heatingCooling(unit: DimensionlessRatioUnit = PERCENT): Int8 @range(min: -100, max: 100) """Seat position on vehicle z-axis. Position is relative within available movable range of the seating. 0 = Lowermost position supported.""" - height(unit: LengthUnitEnum = MILLIMETER): UInt16 + height(unit: LengthUnit = MILLIM): UInt16 """Seat backward switch engaged.""" isBackwardSwitchEngaged: Boolean @@ -92,22 +92,22 @@ type Vehicle_Cabin_Seat { @vspec(comment: "Affects the property (SingleSeat.Heating).") """Seat massage level. 0 = off. 100 = max massage.""" - massage(unit: RelationUnitEnum = PERCENT): UInt8 + massage(unit: DimensionlessRatioUnit = PERCENT): UInt8 @deprecated(reason: "v5.0 - refactored to Seat.MassageLevel") @range(max: 100) """Seat massage level. 0 = off. 100 = max massage.""" - massageLevel(unit: RelationUnitEnum = PERCENT): UInt8 + massageLevel(unit: DimensionlessRatioUnit = PERCENT): UInt8 @range(max: 100) """Seat position on vehicle x-axis. Position is relative to the frontmost position supported by the seat. 0 = Frontmost position supported.""" - position(unit: LengthUnitEnum = MILLIMETER): UInt16 + position(unit: LengthUnit = MILLIM): UInt16 """Seat belt position on vehicle z-axis. Position is relative within available movable range of the seat belt. 0 = Lowermost position supported.""" - seatBeltHeight(unit: LengthUnitEnum = MILLIMETER): UInt16 + seatBeltHeight(unit: LengthUnit = MILLIM): UInt16 """Tilting of seat (seating and backrest) relative to vehicle x-axis. 0 = seat bottom is flat, seat bottom and vehicle x-axis are parallel. Positive degrees = seat tilted backwards, seat x-axis tilted upward, seat z-axis is tilted backward.""" - tilt(unit: AngleUnitEnum = DEGREE): Float + tilt(unit: AngleUnit = DEG): Float @vspec(comment: "In VSS it is assumed that tilting a seat affects both seating (seat bottom) and backrest, i.e. the angle between seating and backrest will not be affected when changing Tilt.") airbag: Vehicle_Cabin_Seat_Airbag backrest: Vehicle_Cabin_Seat_Backrest @@ -171,29 +171,29 @@ type Vehicle_Cabin_Seat_Backrest { @vspec(comment: "Affects the property (SingleSeat.Backrest.Recline).") """Height of lumbar support. Position is relative within available movable range of the lumbar support. 0 = Lowermost position supported.""" - lumbarHeight(unit: LengthUnitEnum = MILLIMETER): UInt8 + lumbarHeight(unit: LengthUnit = MILLIM): UInt8 """Lumbar support (in/out position). 0 = Innermost position. 100 = Outermost position.""" - lumbarSupport(unit: RelationUnitEnum = PERCENT): Float + lumbarSupport(unit: DimensionlessRatioUnit = PERCENT): Float @range(max: 100) """Backrest recline compared to seat z-axis (seat vertical axis). 0 degrees = Upright/Vertical backrest. Negative degrees for forward recline. Positive degrees for backward recline.""" - recline(unit: AngleUnitEnum = DEGREE): Float + recline(unit: AngleUnit = DEG): Float @vspec(comment: "Seat z-axis depends on seat tilt. This means that movement of backrest due to seat tilting will not affect Backrest.Recline as long as the angle between Seating and Backrest are constant. Absolute recline relative to vehicle z-axis can be calculated as Tilt + Backrest.Recline.") """Side bolster support. 0 = Minimum support (widest side bolster setting). 100 = Maximum support.""" - sideBolsterSupport(unit: RelationUnitEnum = PERCENT): Float + sideBolsterSupport(unit: DimensionlessRatioUnit = PERCENT): Float @range(max: 100) } """Headrest settings.""" type Vehicle_Cabin_Seat_Headrest { """Headrest angle, relative to backrest.""" - angle(unit: AngleUnitEnum = DEGREE): Float + angle(unit: AngleUnit = DEG): Float @vspec(comment: "Headrest angle, relative to backrest, 0 degrees if parallel to backrest, Positive degrees = tilted forward.") """Position of headrest relative to movable range.""" - height(unit: LengthUnitEnum = MILLIMETER): UInt8 + height(unit: LengthUnit = MILLIM): UInt8 @vspec(comment: "Position of headrest relative to movable range of the head rest. 0 = Bottommost position supported.") """Head rest backward switch engaged.""" @@ -223,7 +223,7 @@ type Vehicle_Cabin_Seat_Seating @vspec(comment: "Describes signals related to th isForwardSwitchEngaged: Boolean """Length adjustment of seating. 0 = Adjustable part of seating in rearmost position (Shortest length of seating).""" - length(unit: LengthUnitEnum = MILLIMETER): UInt16 + length(unit: LengthUnit = MILLIM): UInt16 } """Attributes that identify a vehicle.""" @@ -238,17 +238,23 @@ enum Vehicle_Cabin_DriverPosition_Enum { RIGHT } -"""Set of units for the quantity kind "angle". NOTE: Taken from VSS specification.""" -enum AngleUnitEnum @vspec(quantityKindKey: "angle") { - DEGREE @vspec(unitKey: "degrees", unitName: "degree") +""" +Units for "Angle" +""" +enum AngleUnit @vspec(element: QUANTITY_KIND, originalName: "angle") { + DEG @vspec(element: UNIT, originalName: "degrees") @reference(uri: "http://qudt.org/vocab/unit/Degree") } -"""Set of units for the quantity kind "length". NOTE: Taken from VSS specification.""" -enum LengthUnitEnum @vspec(quantityKindKey: "length") { - MILLIMETER @vspec(unitKey: "mm", unitName: "millimeter") +""" +Units for "Length" +""" +enum LengthUnit @vspec(element: QUANTITY_KIND, originalName: "length") { + MILLIM @vspec(element: UNIT, originalName: "mm") @reference(uri: "http://qudt.org/vocab/unit/MilliM") } -"""Set of units for the quantity kind "relation". NOTE: Taken from VSS specification.""" -enum RelationUnitEnum @vspec(quantityKindKey: "relation") { - PERCENT @vspec(unitKey: "percent", unitName: "percent") +""" +Units for "DimensionlessRatio" +""" +enum DimensionlessRatioUnit @vspec(element: QUANTITY_KIND, originalName: "relation") { + PERCENT @vspec(element: UNIT, originalName: "percent") @reference(uri: "http://qudt.org/vocab/unit/PERCENT") } From 91ceaeb036cf6d9171439ef33ff32c5510a5d107 Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Tue, 28 Apr 2026 21:41:54 +0200 Subject: [PATCH 16/18] fix(s2dm): Update test for instance tag check without id fields Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- tests/test_s2dm_exporter.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_s2dm_exporter.py b/tests/test_s2dm_exporter.py index 0a3cfa83..cb2e97da 100644 --- a/tests/test_s2dm_exporter.py +++ b/tests/test_s2dm_exporter.py @@ -311,9 +311,6 @@ def test_instance_tag_support(self): assert len(main_type_lines) == 1 assert "@instanceTag" not in main_type_lines[0] - # Test that types with instances get ID field - assert "id: ID!" in sdl - # Verify the complete structure matches the reference pattern # The seat should be a list field (seats) with natural plural because it has instances assert "seats: [Vehicle_Cabin_Seat]" in sdl From eff603bdd1d5c6f3d80685835622d0faf85035a5 Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Wed, 29 Apr 2026 10:26:57 +0200 Subject: [PATCH 17/18] fix(s2dm): Add alias for old vspec unit keys for qudt mappings Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- .../s2dm/predefined_elements/qudt_mappings.py | 22 ++++- src/vss_tools/exporters/s2dm/type_builders.py | 11 ++- tests/test_qudt_unit_resolution.py | 86 +++++++++++++++++++ 3 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 tests/test_qudt_unit_resolution.py diff --git a/src/vss_tools/exporters/s2dm/predefined_elements/qudt_mappings.py b/src/vss_tools/exporters/s2dm/predefined_elements/qudt_mappings.py index dec09a29..d6b72060 100644 --- a/src/vss_tools/exporters/s2dm/predefined_elements/qudt_mappings.py +++ b/src/vss_tools/exporters/s2dm/predefined_elements/qudt_mappings.py @@ -1,3 +1,10 @@ +# NOTE: This mapping approach could be replaced/simplified if the VSPEC units file would already contain the QUDT info. +# For example, +# mm: +# ... +# qudt-unit: http://qudt.org/vocab/unit/MilliM +# qudt-quantity-kind: https://qudt.org/vocab/quantitykind/Length + # vspec unit key → QUDT QuantityKind and unit QUDT_MAPPING: dict[str, dict[str, str]] = { # length @@ -159,6 +166,11 @@ }, # resistance "Ohm": {"qudt_uri": "http://qudt.org/vocab/unit/OHM", "qudt_quantity_kind": "Resistance", "qudt_unit": "OHM"}, + "mOhm": { + "qudt_uri": "http://qudt.org/vocab/unit/MilliOHM", + "qudt_quantity_kind": "Resistance", + "qudt_unit": "MILLIOHM", + }, # iluminance "lx": { "qudt_uri": "http://qudt.org/vocab/unit/LUX", @@ -173,7 +185,7 @@ "unix-time": {"qudt_quantity_kind": "DateTime", "qudt_unit": "UNIX_TIME"}, # Custom (not present in QUDT) "iso8601": {"qudt_quantity_kind": "DateTime", "qudt_unit": "ISO8601"}, # Custom (not present in QUDT) # energy-consumption-per-distance - "kWh/km": { + "kWh/100km": { "qudt_quantity_kind": "EnergyPerDistance", "qudt_unit": "KILOW_HR_PER_100KILOM", }, # Custom (not present in QUDT) @@ -202,4 +214,12 @@ }, # Custom (not present in QUDT) "mpg": {"qudt_quantity_kind": "DistancePerVolume", "qudt_unit": "MILE_PER_GAL"}, # Custom (not present in QUDT) "km/l": {"qudt_quantity_kind": "DistancePerVolume", "qudt_unit": "KILOM_PER_L"}, # Custom (not present in QUDT) + "g/Ah": {"qudt_quantity_kind": "MassPerElectricCharge", "qudt_unit": "GM_PER_A_HR"}, # Custom (not present in QUDT) +} + +# Deprecated VSS unit key → canonical key in QUDT_MAPPING. +# Used to support models that have not yet migrated to the new unit name. +QUDT_ALIASES: dict[str, str] = { + "days": "day", # renamed in VSS 5.x + "celsius": "Celsius", # renamed in VSS 5.x } diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index 0bc99815..f96b980c 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -44,7 +44,7 @@ from .graphql_scalars import VSS_DATATYPE_MAP from .graphql_utils import GraphQLElementType, convert_name_for_graphql_schema from .metadata_tracker import build_field_path -from .predefined_elements.qudt_mappings import QUDT_MAPPING +from .predefined_elements.qudt_mappings import QUDT_ALIASES, QUDT_MAPPING # Initialize inflect engine for pluralization (singleton) _inflect_engine = inflect.engine() @@ -131,8 +131,14 @@ def _get_quantity_units() -> dict[str, dict]: if not qudt_quantity_kind or not qudt_unit: continue - # Cross-reference VSS dynamic_units for the VSS quantity key + # Cross-reference VSS dynamic_units for the VSS quantity key. + # If the canonical key is not found, check whether any alias points to it + # (supports models whose units.yaml still uses a deprecated unit name). vss_unit_data = dynamic_units.get(vss_key) + if vss_unit_data is None: + alias_key = next((a for a, canonical in QUDT_ALIASES.items() if canonical == vss_key), None) + if alias_key is not None: + vss_unit_data = dynamic_units.get(alias_key) if vss_unit_data is None: log.debug(f"QUDT unit '{vss_key}' not found in loaded dynamic_units; skipping.") continue @@ -629,6 +635,7 @@ def _get_unit_args(leaf_row: pd.Series, unit_enums: dict[str, GraphQLEnumType]) if not unit: return {} + unit = QUDT_ALIASES.get(unit, unit) qudt_info = QUDT_MAPPING.get(unit) if qudt_info is None: log.warning(f"Unit '{unit}' has no QUDT mapping; unit argument will not be generated.") diff --git a/tests/test_qudt_unit_resolution.py b/tests/test_qudt_unit_resolution.py new file mode 100644 index 00000000..d758dda4 --- /dev/null +++ b/tests/test_qudt_unit_resolution.py @@ -0,0 +1,86 @@ +# Copyright (c) 2025 Contributors to COVESA +# +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 + +import pandas as pd +import pytest +import vss_tools.exporters.s2dm.type_builders as type_builders_module +from vss_tools.exporters.s2dm.type_builders import _get_unit_args, create_unit_enums +from vss_tools.model import VSSUnit + + +def _make_unit(key: str, quantity: str) -> VSSUnit: + """Build a minimal VSSUnit for testing without triggering quantity validation.""" + return VSSUnit.model_construct(definition="test", unit=key, quantity=quantity, key=key) + + +def _leaf(unit: str) -> pd.Series: + return pd.Series({"unit": unit}) + + +class TestUnitAliasResolution: + """Tests for QUDT_ALIASES backward-compatibility in unit enum building and _get_unit_args. + + Two scenarios are covered: + - Current model: units.yaml registers the new canonical names ("day", "Celsius"). + - Legacy model: units.yaml still registers the old names ("days", "celsius") and + the vspec also still uses those old names. + """ + + @pytest.fixture() + def current_dynamic_units(self, monkeypatch): + """Simulate a current model whose units.yaml uses the new canonical names.""" + monkeypatch.setattr( + type_builders_module, + "dynamic_units", + { + "day": _make_unit("day", "time"), + "Celsius": _make_unit("Celsius", "temperature"), + }, + ) + + @pytest.fixture() + def legacy_dynamic_units(self, monkeypatch): + """Simulate a legacy model whose units.yaml still uses the old names.""" + monkeypatch.setattr( + type_builders_module, + "dynamic_units", + { + "days": _make_unit("days", "time"), + "celsius": _make_unit("celsius", "temperature"), + }, + ) + + def test_canonical_day_resolves(self, current_dynamic_units): + """Current models using 'day' produce a TimeUnit enum and a unit argument.""" + unit_enums, _ = create_unit_enums() + assert "Time" in unit_enums, "TimeUnit enum should be built from 'day'" + args = _get_unit_args(_leaf("day"), unit_enums) + assert "unit" in args + assert args["unit"].default_value == "day" + + def test_canonical_celsius_resolves(self, current_dynamic_units): + """Current models using 'Celsius' produce a TemperatureUnit enum and a unit argument.""" + unit_enums, _ = create_unit_enums() + assert "Temperature" in unit_enums, "TemperatureUnit enum should be built from 'Celsius'" + args = _get_unit_args(_leaf("Celsius"), unit_enums) + assert "unit" in args + assert args["unit"].default_value == "Celsius" + + def test_alias_days_resolves(self, legacy_dynamic_units): + """Legacy models with 'days' in units.yaml and vspec still get a TimeUnit enum and unit argument.""" + unit_enums, _ = create_unit_enums() + assert "Time" in unit_enums, "TimeUnit enum should be built even when dynamic_units has 'days'" + args = _get_unit_args(_leaf("days"), unit_enums) + assert "unit" in args, "'days' should resolve via alias and produce a unit argument" + + def test_alias_celsius_resolves(self, legacy_dynamic_units): + """Legacy models with 'celsius' in units.yaml and vspec still get a TemperatureUnit enum and unit argument.""" + unit_enums, _ = create_unit_enums() + assert "Temperature" in unit_enums, "TemperatureUnit enum should be built even when dynamic_units has 'celsius'" + args = _get_unit_args(_leaf("celsius"), unit_enums) + assert "unit" in args, "'celsius' should resolve via alias and produce a unit argument" From 9150db1ccc95df5d072f5f68bbed09828fcf576a Mon Sep 17 00:00:00 2001 From: JD Alvarez <8550265+jdacoello@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:39:50 +0200 Subject: [PATCH 18/18] fix(s2dm): Export allowed values inside struct as enums Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com> --- .../exporters/s2dm/schema_generator.py | 12 ++++- src/vss_tools/exporters/s2dm/type_builders.py | 7 ++- tests/test_s2dm_structs.py | 49 +++++++++++++++++++ tests/vspec/test_structs/TestBranch1.vspec | 9 ++++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/vss_tools/exporters/s2dm/schema_generator.py b/src/vss_tools/exporters/s2dm/schema_generator.py index 38ee45cf..e73719b7 100644 --- a/src/vss_tools/exporters/s2dm/schema_generator.py +++ b/src/vss_tools/exporters/s2dm/schema_generator.py @@ -96,8 +96,11 @@ def generate_s2dm_schema( # Combine branches from both main tree and data type tree for joint collision detection # since they share the same GraphQL type namespace combined_branches_df = branches_df + struct_leaves_df: pd.DataFrame | None = None if data_type_tree: - struct_branches_df, _ = get_metadata_df(data_type_tree, extended_attributes=extended_attributes) + struct_branches_df, struct_leaves_df = get_metadata_df( + data_type_tree, extended_attributes=extended_attributes + ) combined_branches_df = pd.concat([branches_df, struct_branches_df], axis=0, verify_integrity=True) # Detect and resolve short name collisions if requested @@ -120,12 +123,19 @@ def generate_s2dm_schema( instance_types = create_instance_types(branches_df, vspec_comments, short_name_mapping) allowed_enums, allowed_metadata = create_allowed_enums(leaves_df) + # Also create allowed enums for struct properties (properties with `allowed` values) + if struct_leaves_df is not None: + struct_allowed_enums, struct_allowed_metadata = create_allowed_enums(struct_leaves_df) + allowed_enums.update(struct_allowed_enums) + allowed_metadata.update(struct_allowed_metadata) + # Create struct types from data type tree struct_types = create_struct_types( data_type_tree, vspec_comments, extended_attributes=extended_attributes, short_name_mapping=short_name_mapping, + allowed_enums=allowed_enums, ) # Combine all types diff --git a/src/vss_tools/exporters/s2dm/type_builders.py b/src/vss_tools/exporters/s2dm/type_builders.py index f96b980c..5ea0694b 100644 --- a/src/vss_tools/exporters/s2dm/type_builders.py +++ b/src/vss_tools/exporters/s2dm/type_builders.py @@ -380,6 +380,7 @@ def create_struct_types( vspec_comments: dict[str, Any], extended_attributes: tuple[str, ...] = (), short_name_mapping: dict[str, str] | None = None, + allowed_enums: dict[str, GraphQLEnumType] | None = None, ) -> dict[str, GraphQLObjectType]: """ Convert VSS struct definitions to GraphQL object types. @@ -389,6 +390,7 @@ def create_struct_types( vspec_comments: Dictionary to store struct metadata extended_attributes: Extended attribute names from CLI short_name_mapping: Optional mapping from FQN to short type names + allowed_enums: Pre-built allowed-value enum types for struct properties Returns: Dictionary mapping struct type names to GraphQL object types @@ -412,7 +414,10 @@ def create_struct_types( fields = {} for prop_fqn, prop_row in properties.iterrows(): field_name = convert_name_for_graphql_schema(prop_row["name"], GraphQLElementType.FIELD, S2DM_CONVERSIONS) - base_type = _get_graphql_type_for_property(prop_row, struct_types, short_name_mapping) + # Use get_graphql_type_for_leaf so that properties with `allowed` values resolve + # to their enum type rather than the raw scalar. + combined_registry: dict[str, Any] = {**(allowed_enums or {}), **struct_types} + base_type = get_graphql_type_for_leaf(prop_row, combined_registry, short_name_mapping) fields[field_name] = GraphQLField(GraphQLNonNull(base_type), description=prop_row.get("description", "")) field_path = build_field_path(type_name, field_name) diff --git a/tests/test_s2dm_structs.py b/tests/test_s2dm_structs.py index c4334ba6..33fc6779 100644 --- a/tests/test_s2dm_structs.py +++ b/tests/test_s2dm_structs.py @@ -322,6 +322,55 @@ def test_struct_in_property_short_names(self, struct_trees): assert isinstance(x_property_type, GraphQLNonNull) assert x_property_type.of_type == nested_struct_type, f"Expected NestedStruct but got {x_property_type.of_type}" + def test_struct_property_with_allowed_values_resolves_to_enum(self, struct_trees): + """Test that struct properties with `allowed` values resolve to an enum type, not a scalar.""" + tree, data_type_tree = struct_trees + schema, _, allowed_metadata, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=False) + + nested_struct_name = convert_name_for_graphql_schema( + "VehicleDataTypes.TestBranch1.NestedStruct", GraphQLElementType.TYPE, S2DM_CONVERSIONS + ) + enum_name = ( + convert_name_for_graphql_schema( + "VehicleDataTypes.TestBranch1.NestedStruct.mode", GraphQLElementType.TYPE, S2DM_CONVERSIONS + ) + + "_Enum" + ) + + # Enum type must be present in schema + assert enum_name in schema.type_map, f"Expected enum '{enum_name}' in schema" + + # The `mode` field on NestedStruct must resolve to that enum (wrapped NonNull) + nested_struct_type = schema.type_map[nested_struct_name] + assert isinstance(nested_struct_type, GraphQLObjectType) + assert "mode" in nested_struct_type.fields, "Expected 'mode' field in NestedStruct" + mode_field_type = nested_struct_type.fields["mode"].type + assert isinstance(mode_field_type, GraphQLNonNull) + assert mode_field_type.of_type == schema.type_map[enum_name] + + # Allowed enum metadata must be populated for this property + assert enum_name in allowed_metadata + assert set(allowed_metadata[enum_name]["allowed_values"].values()) == {"ACTIVE", "INACTIVE", "ERROR"} + + def test_struct_property_allowed_enum_short_names(self, struct_trees): + """Test allowed-value enum resolution for struct properties when using short names.""" + tree, data_type_tree = struct_trees + schema, _, _, _ = generate_s2dm_schema(tree, data_type_tree, use_short_names=True) + + nested_struct_type = schema.type_map.get("NestedStruct") + assert nested_struct_type is not None + assert isinstance(nested_struct_type, GraphQLObjectType) + assert "mode" in nested_struct_type.fields + + mode_field_type = nested_struct_type.fields["mode"].type + assert isinstance(mode_field_type, GraphQLNonNull) + # The wrapped type must be an enum (not a scalar like String) + from graphql import GraphQLEnumType + + assert isinstance( + mode_field_type.of_type, GraphQLEnumType + ), f"Expected GraphQLEnumType for 'mode' field, got {type(mode_field_type.of_type)}" + class TestS2DMStructsModular: """Test class for modular output with struct support.""" diff --git a/tests/vspec/test_structs/TestBranch1.vspec b/tests/vspec/test_structs/TestBranch1.vspec index 9c4fc1f4..7d8d8190 100644 --- a/tests/vspec/test_structs/TestBranch1.vspec +++ b/tests/vspec/test_structs/TestBranch1.vspec @@ -20,6 +20,15 @@ NestedStruct.z: datatype: double default: 1 +NestedStruct.mode: + type: property + description: "Mode of the nested struct" + datatype: string + allowed: + - ACTIVE + - INACTIVE + - ERROR + ParentStruct: type: struct description: "A struct that is going to contain properties that are structs themselves"