Skip to content

Commit 147c8ce

Browse files
committed
feat(s2dm): enhance pluralization handling and naming conventions in GraphQL schema generation
Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com>
1 parent 479032e commit 147c8ce

File tree

6 files changed

+195
-15
lines changed

6 files changed

+195
-15
lines changed

docs/s2dm.md

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,34 @@ The exporter handles all `vspec` data types as follows:
8282
- **Strings** → GraphQL String
8383
- **Numbers** → GraphQL Int, Float, or custom scalars (Int8, UInt16, etc.)
8484
- **Booleans** → GraphQL Boolean
85-
- **Arrays** → GraphQL Lists
85+
- **Arrays** → GraphQL Lists (with natural plural field names)
8686
- **Allowed values** → GraphQL Enums
8787

88+
#### List Field Names (Automatic Pluralization)
89+
90+
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:
91+
92+
- `seat` → `seats: [Vehicle_Cabin_Seat]`
93+
- `door` → `doors: [Vehicle_Cabin_Door]`
94+
- `window` → `windows: [Vehicle_Cabin_Window]`
95+
- `battery` → `batteries: [Vehicle_Battery]`
96+
- `mirror` → `mirrors: [Vehicle_Body_Mirror]`
97+
98+
This improves GraphQL schema readability by following common naming conventions instead of using mechanical suffixes like `_s`.
99+
100+
**Example:**
101+
```graphql
102+
type Vehicle_Cabin {
103+
"""Cabin seats for passengers."""
104+
seats: [Vehicle_Cabin_Seat]
105+
106+
"""Cabin doors."""
107+
doors: [Vehicle_Cabin_Door]
108+
}
109+
```
110+
111+
**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.
112+
88113
#### Enum Value Sanitization
89114

90115
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 {
113138

114139
This ensures complete traceability between the VSS source and the generated GraphQL schema.
115140

141+
### GraphQL Naming Convention Warnings
142+
143+
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.
144+
145+
**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.
146+
147+
**Warning Output:** When plural type names are detected, two files are generated in `vspec_reference/`:
148+
149+
1. **`plural_type_warnings.yaml`** - Lists all detected plural type names:
150+
```yaml
151+
# WARNING: These elements in the reference model seem to have a plural name...
152+
153+
Vehicle.Cabin.Lights:
154+
singular: Light
155+
currentNameInGraphQLModel: Vehicle_Cabin_Lights
156+
157+
Vehicle.Body.Mirrors:
158+
singular: Mirror
159+
currentNameInGraphQLModel: Vehicle_Body_Mirrors
160+
161+
# Whitelisted words (excluded from plural detection):
162+
whitelisted_non_plurals:
163+
- ADAS
164+
- ABS
165+
- Status
166+
- Chassis
167+
# ... etc
168+
```
169+
170+
2. **Console warnings:** During export, warnings are logged for immediate visibility:
171+
```
172+
WARNING: Type 'Vehicle_Cabin_Lights' uses potential plural name 'Lights'.
173+
Suggested singular: 'Light' (VSS FQN: Vehicle.Cabin.Lights)
174+
```
175+
176+
**Review Process:** These warnings help VSS maintainers identify:
177+
- **True plurals** - VSS branches that should be renamed to singular form
178+
- **False positives** - Words that end in 's' but aren't actually plural (add to whitelist)
179+
- **Acceptable exceptions** - Cases where plural names are intentional
180+
116181
### VSS Instances Become GraphQL Structures
117182
When your `vspec` has instances (like multiple seats), the exporter creates proper GraphQL types:
118183

@@ -186,7 +251,9 @@ myOutput/
186251
├── README.md # Documentation and provenance info
187252
├── vspec_lookup_spec.yaml # Complete VSS tree (fully expanded)
188253
├── vspec_units.yaml # Units used (if provided via -u or implicit)
189-
└── vspec_quantities.yaml # Quantities used (if provided via -q or implicit)
254+
├── vspec_quantities.yaml # Quantities used (if provided via -q or implicit)
255+
├── plural_type_warnings.yaml # Plural type names detected (if any)
256+
└── pluralized_field_names.yaml # Fields changed to plural form (if any instances)
190257
```
191258

192259
### VSS Reference Files
@@ -197,11 +264,14 @@ The `vspec_reference/` directory provides complete traceability:
197264
- **vspec_lookup_spec.yaml** - Complete VSS specification tree (fully processed and expanded) in YAML format
198265
- **vspec_units.yaml** - Unit definitions used during generation (included if units were provided via `-u` flag or implicitly loaded)
199266
- **vspec_quantities.yaml** - Quantity definitions used during generation (included if quantities were provided via `-q` flag or implicitly loaded)
267+
- **plural_type_warnings.yaml** - VSS branches with plural type names that may violate GraphQL naming conventions (generated if any detected)
268+
- **pluralized_field_names.yaml** - Fields whose names were changed to plural form for list fields (generated if any instances exist)
200269

201270
These files allow you to:
202271
1. Trace GraphQL elements back to their VSS source using the FQN in `@vspec` directives
203272
2. Reproduce the exact GraphQL schema by re-running the exporter
204273
3. Understand which input files were used for generation
274+
4. Review naming convention warnings and pluralization changes
205275

206276

207277

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ dependencies = [
1717
"graphene>=3.4.3",
1818
"pandas>=2.2.3",
1919
"case-converter>=1.2.0",
20+
"inflect>=7.4.0",
2021
]
2122
authors = [
2223
{name="COVESA VSS", email="covesa-dev@covesa.global"}

src/vss_tools/exporters/s2dm/__init__.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,29 +109,31 @@ def cli(
109109
log.info("Generating S2DM GraphQL schema...")
110110

111111
# Generate the schema
112-
schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments = generate_s2dm_schema(
112+
schema, unit_enums_metadata, allowed_enums_metadata, mapping_metadata = generate_s2dm_schema(
113113
tree, data_type_tree, extended_attributes=extended_attributes
114114
)
115115

116116
if modular:
117117
# Write modular files
118118
write_modular_schema(
119-
schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments, output_dir, flat_domains
119+
schema, unit_enums_metadata, allowed_enums_metadata, mapping_metadata, output_dir, flat_domains
120120
)
121121
log.info(f"Modular GraphQL schema written to {output_dir}/")
122122
else:
123123
# Single file export: write to outputDir/outputDir.graphql
124124
graphql_file = output_dir / f"{output_dir.name}.graphql"
125125
full_schema_str = print_schema_with_vspec_directives(
126-
schema, unit_enums_metadata, allowed_enums_metadata, vspec_comments
126+
schema, unit_enums_metadata, allowed_enums_metadata, mapping_metadata
127127
)
128128
with open(graphql_file, "w") as outfile:
129129
outfile.write(full_schema_str)
130130

131131
log.info(f"GraphQL schema written to {graphql_file}")
132132

133133
# Generate VSS reference files (will check for implicit files)
134-
generate_vspec_reference(tree, data_type_tree, output_dir, extended_attributes, vspec, units, quantities)
134+
generate_vspec_reference(
135+
tree, data_type_tree, output_dir, extended_attributes, vspec, units, quantities, mapping_metadata
136+
)
135137

136138
except S2DMExporterException as e:
137139
log.error(e)

src/vss_tools/exporters/s2dm/reference_generator.py

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ def generate_vspec_reference(
3434
vspec_file: Path,
3535
units_files: tuple[Path, ...],
3636
quantities_files: tuple[Path, ...],
37+
mapping_metadata: dict[str, dict] | None = None,
3738
) -> None:
3839
"""
3940
Generate VSS reference files alongside GraphQL output.
@@ -49,6 +50,7 @@ def generate_vspec_reference(
4950
vspec_file: Path to vspec file (to find implicit units/quantities)
5051
units_files: Unit files from CLI
5152
quantities_files: Quantity files from CLI
53+
mapping_metadata: Optional metadata including plural type warnings
5254
5355
Raises:
5456
S2DMExporterException: If reference file generation fails
@@ -152,8 +154,58 @@ def generate_vspec_reference(
152154
except (PermissionError, OSError) as e:
153155
raise S2DMExporterException(f"Failed to write quantities file {quantities_output}: {e}") from e
154156

157+
# Write plural type warnings if any were collected
158+
if mapping_metadata and mapping_metadata.get("plural_type_warnings"):
159+
warnings_output = reference_dir / "plural_type_warnings.yaml"
160+
try:
161+
with open(warnings_output, "w") as f:
162+
# Write header comment
163+
f.write(
164+
"# WARNING: These elements in the reference model seem to have a plural name, "
165+
"while GraphQL best practices suggest the use of the singular form for type names.\n"
166+
)
167+
f.write("# Consider replacing plural forms and whitelisting false positives.\n\n")
168+
169+
# Write each warning as FQN with nested fields
170+
for warning in mapping_metadata["plural_type_warnings"]:
171+
f.write(f"{warning['fqn']}:\n")
172+
f.write(f" suggested_singular: {warning['suggested_singular']}\n")
173+
f.write(f" current_name_in_graphql_model: {warning['type_name']}\n")
174+
f.write("\n")
175+
176+
warning_count = len(mapping_metadata["plural_type_warnings"])
177+
log.info(f" - Plural warnings: {warnings_output.name} ({warning_count} warning(s))")
178+
except (PermissionError, OSError) as e:
179+
raise S2DMExporterException(f"Failed to write plural warnings file {warnings_output}: {e}") from e
180+
181+
# Write pluralized field names if any were collected
182+
if mapping_metadata and mapping_metadata.get("pluralized_field_names"):
183+
pluralized_output = reference_dir / "pluralized_field_names.yaml"
184+
try:
185+
with open(pluralized_output, "w") as f:
186+
# Write header comment
187+
f.write(
188+
"# Following names were changed to their plural form as they resolve to an output type "
189+
"with a List type modifier.\n\n"
190+
)
191+
192+
# Write each pluralized field as FQN with nested fields
193+
for entry in mapping_metadata["pluralized_field_names"]:
194+
f.write(f"{entry['fqn']}:\n")
195+
f.write(f" plural_field_name: {entry['plural_field_name']}\n")
196+
f.write(f" path_in_graphql_model: {entry['path_in_graphql_model']}\n")
197+
f.write("\n")
198+
199+
log.info(
200+
f" - Pluralized fields: {pluralized_output.name} "
201+
f"({len(mapping_metadata['pluralized_field_names'])} field(s))"
202+
)
203+
except (PermissionError, OSError) as e:
204+
raise S2DMExporterException(f"Failed to write pluralized fields file {pluralized_output}: {e}") from e
205+
155206
# Generate README.md for provenance documentation
156-
generate_reference_readme(reference_dir, vspec_file, actual_units, actual_quantities)
207+
has_plural_warnings = mapping_metadata and bool(mapping_metadata.get("plural_type_warnings"))
208+
generate_reference_readme(reference_dir, vspec_file, actual_units, actual_quantities, has_plural_warnings)
157209

158210
except S2DMExporterException:
159211
# Re-raise our custom exceptions
@@ -168,6 +220,7 @@ def generate_reference_readme(
168220
vspec_file: Path,
169221
units_files: tuple[Path, ...] | None,
170222
quantities_files: tuple[Path, ...] | None,
223+
has_plural_warnings: bool = False,
171224
) -> None:
172225
"""
173226
Generate README.md documenting the provenance of reference files.
@@ -177,6 +230,7 @@ def generate_reference_readme(
177230
vspec_file: Original vspec input file
178231
units_files: Units files used (explicit or implicit)
179232
quantities_files: Quantities files used (explicit or implicit)
233+
has_plural_warnings: Whether plural type warnings were generated
180234
181235
Raises:
182236
S2DMExporterException: If README generation fails
@@ -211,6 +265,10 @@ def generate_reference_readme(
211265
readme_content += """
212266
* **vspec_quantities.yaml** - Quantity definitions categorizing measurements."""
213267

268+
if has_plural_warnings:
269+
readme_content += """
270+
* **plural_type_warnings.txt** - VSS branches with plural type names (GraphQL prefers singular)."""
271+
214272
readme_content += """
215273
216274
## Documentation

src/vss_tools/exporters/s2dm/type_builders.py

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from typing import Any
2323

2424
import caseconverter
25+
import inflect
2526
import pandas as pd
2627
from graphql import (
2728
GraphQLArgument,
@@ -64,6 +65,42 @@ def _extract_extended_attributes(row: pd.Series, extended_attributes: tuple[str,
6465
return extracted
6566

6667

68+
# Initialize inflect engine for pluralization (singleton)
69+
_inflect_engine = inflect.engine()
70+
71+
72+
def _check_and_collect_plural_type_name(
73+
converted_type_name: str, fqn: str, original_name: str, plural_name_warnings: dict[str, dict[str, Any]]
74+
) -> None:
75+
"""
76+
Check if a name appears to be plural and collect for reporting.
77+
78+
Uses inflect library to detect potential plural forms. Reports all cases where
79+
a singular form is detected, without filtering. This allows downstream tools
80+
to decide which cases are true issues vs. acceptable exceptions.
81+
82+
Args:
83+
converted_type_name: The converted type name (for schema)
84+
fqn: The fully qualified VSS name for context
85+
original_name: The original VSS branch/struct name (before conversion)
86+
plural_name_warnings: Dictionary to store metadata (adds to "plural_type_warnings" key)
87+
"""
88+
# Check if inflect detects a singular form
89+
# Returns False if already singular, or the singular form if plural
90+
singular = _inflect_engine.singular_noun(original_name)
91+
92+
if singular:
93+
plural_name_warnings.setdefault("plural_type_warnings", []).append(
94+
{"type_name": converted_type_name, "fqn": fqn, "plural_word": original_name, "suggested_singular": singular}
95+
)
96+
97+
# Also log to console for immediate visibility
98+
log.warning(
99+
f"Type '{converted_type_name}' uses potential plural name '{original_name}'. "
100+
f"Suggested singular: '{singular}' (VSS FQN: {fqn})"
101+
)
102+
103+
67104
def create_unit_enums() -> tuple[dict[str, GraphQLEnumType], dict[str, dict[str, dict[str, str]]]]:
68105
"""
69106
Create GraphQL enum types for VSS units grouped by quantity.
@@ -415,8 +452,13 @@ def create_object_type(
415452
GraphQL object type for the branch
416453
"""
417454
branch_row = branches_df.loc[fqn]
455+
original_name = branch_row["name"]
418456
type_name = convert_name_for_graphql_schema(fqn, GraphQLElementType.TYPE, S2DM_CONVERSIONS)
419457

458+
# Check if branch name is plural and collect for reporting
459+
# Pass original VSS branch name (before any conversion)
460+
_check_and_collect_plural_type_name(type_name, fqn, original_name, vspec_comments)
461+
420462
def get_fields() -> dict[str, GraphQLField]:
421463
fields = {}
422464

@@ -478,7 +520,15 @@ def get_fields() -> dict[str, GraphQLField]:
478520
fields.update(hoisted_fields)
479521

480522
if child_row.get("instances"):
481-
fields[f"{field_name}_s"] = GraphQLField(GraphQLList(child_type))
523+
# Use natural plural form for list fields (using inflect directly)
524+
plural_field_name = _inflect_engine.plural(field_name)
525+
fields[plural_field_name] = GraphQLField(GraphQLList(child_type))
526+
527+
# Collect pluralized field name for reporting
528+
field_path = build_field_path(type_name, plural_field_name)
529+
vspec_comments.setdefault("pluralized_field_names", []).append(
530+
{"fqn": child_fqn, "plural_field_name": plural_field_name, "path_in_graphql_model": field_path}
531+
)
482532
else:
483533
fields[field_name] = GraphQLField(child_type)
484534

tests/test_s2dm_exporter.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
get_metadata_df,
1717
print_schema_with_vspec_directives,
1818
)
19+
from vss_tools.exporters.s2dm.type_builders import _sanitize_enum_value_for_graphql
1920
from vss_tools.main import get_trees
2021
from vss_tools.utils.graphql_utils import GraphQLElementType, convert_name_for_graphql_schema
2122

@@ -308,8 +309,8 @@ def test_instance_tag_support(self):
308309
assert "id: ID!" in sdl
309310

310311
# Verify the complete structure matches the reference pattern
311-
# The seat should be a list field (seat_s) because it has instances
312-
assert "seat_s: [Vehicle_Cabin_Seat]" in sdl
312+
# The seat should be a list field (seats) with natural plural because it has instances
313+
assert "seats: [Vehicle_Cabin_Seat]" in sdl
313314

314315
def test_allowed_value_enums_generation(self):
315316
"""Test that allowed value enums are generated correctly."""
@@ -536,13 +537,11 @@ def test_non_instantiated_property_hoisting(self, tmp_path: Path):
536537
assert "someSignal" in cabin_type_content
537538
# Verify it has the instantiate=false metadata
538539
assert 'metadata: [{key: "instantiate", value: "false"}]' in cabin_type_content
539-
# And door_s array field should also be there
540-
assert "door_s" in cabin_type_content
540+
# And doors array field should also be there (natural plural)
541+
assert "doors" in cabin_type_content
541542

542543
def test_enum_value_sanitization_with_spaces(self):
543-
"""Test that enum values with spaces are properly sanitized and annotated."""
544-
from vss_tools.exporters.s2dm.type_builders import _sanitize_enum_value_for_graphql
545-
544+
"""Test that enum values with spaces are sanitized correctly."""
546545
# Test the sanitization function
547546
assert _sanitize_enum_value_for_graphql("some value") == ("SOME_VALUE", True)
548547
assert _sanitize_enum_value_for_graphql("SOME VALUE") == ("SOME_VALUE", True)

0 commit comments

Comments
 (0)