fix(validation): reject duplicate mapping keys

[iamrajatrana]

Summary

Reject duplicate keys in YAML and JSON mappings before schema validation can run against silently overwritten data. The loader remains derived from yaml.SafeLoader, adds no dependency, preserves aliases and legal merge-key overrides, and reports both mapping and duplicate-key source locations.

This PR also adds Validation CI so the validator tests and canonical example run on Python 3.11 through 3.14 whenever validation, core specification, or canonical example files change.

Before

source: staging.orders
source: production.orders

$ uv run validation/validate.py duplicate.yaml
Validation PASSED: duplicate.yaml
$ echo $?
0

After

$ uv run validation/validate.py duplicate.yaml
Error: Invalid YAML: while constructing a mapping
...
found duplicate key 'source'
...
$ echo $?
1

Edge cases covered

• Top-level and nested duplicate keys
• Quoted and explicitly tagged equivalent keys
• Duplicate JSON object keys
• Duplicate scalar keys whose values are collections
• A valid duplicate hiding an earlier invalid value
• Explicit duplicates alongside YAML merge keys
• Repeated merge keys
• Same key in separate mappings remains valid
• YAML aliases remain valid
• Legal merge-key overrides remain valid
• Merge keys remain distinct from a quoted literal "<<" key
• Error marks identify the original mapping and duplicate location
• End-to-end non-zero exit for duplicates and successful validation for valid input

Merge-key behavior

A mapping may contain one YAML merge key and explicitly override values inherited through that merge; this remains supported. Repeating the special << merge key in the same mapping is rejected because YAML provides the sequence form (<<: [*first, *second]) for merging multiple mappings and mapping keys are expected to be unique.

Scope notes

• This change is intentionally limited to validation/validate.py. Several converters load their own vendor-specific YAML with yaml.safe_load; applying a shared strict-loading policy across those independent packages requires separate compatibility evaluation.
• Empty documents and top-level sequences can still reach the validator's pre-existing data.get(...) assumption and raise AttributeError. That behavior is unrelated to duplicate-key detection and is not changed here.
• Tests are invoked as uv run validation/test_validate.py, matching their PEP 723 dependency metadata and script-local import path.

Related Issues

Closes #336

Tests

• uv run validation/test_validate.py (16 tests)
• uv run validation/validate.py examples/tpcds_semantic_model.yaml
• Duplicate-key integration matrix across YAML and JSON inputs
• uv run --with ruff ruff check --ignore EXE001 validation/validate.py validation/test_validate.py
• Validation CI matrix: Python 3.11, 3.12, 3.13, and 3.14

Checklist

• New behavior is covered by unit and integration tests
• Validation tests are enforced in GitHub Actions
• Existing canonical semantic model validation still passes
• ASF license headers are present on new files
• No third-party dependency was added

[flyrain]

Thanks for the fix, @iamrajatrana ! Left some comments. Both comments are related to the merge feature, which is removed in YAML 1.2 spec. I'm OK if we decide to not support older versions of YAML.

[flyrain]

PyYAML's flatten_mapping recursively processes a mapping used directly as the value of << without invoking this construct_mapping override, so duplicate keys inside that merge source bypass this loop. For example, <<: {name: orders, name: customers} is reduced to name: customers; in a schema-valid model the validator exits 0 with Validation PASSED. Can we recursively check merge-source mapping nodes before they are flattened, and add a regression test for a duplicate inside an embedded merge mapping?

Alternative, we can deliberately reject << and remove the claims that merge keys are supported. But that seems a bigger discussion. AFAIK, Ossie doesn't enforce the YAML spec version.

[iamrajatrana]

Confirmed and fixed in 97b4a2e. flatten_mapping traverses merge-source mappings without routing them through construct_mapping, so their duplicates were never checked. Rather than recursing into merge sources from inside the override, the check now runs as a construct_document pre-pass over the composed node graph, so a mapping is inspected in its authored form regardless of how it's reached. Regression tests added for inline and anchored merge sources. Details in the summary comment below.

[flyrain]

This loader can reject a valid merge override when the same mapping is later reused through an alias.

For example:

first:
  <<: &defaults
    <<: &base
      source: staging.orders
    source: production.orders

second: *defaults

source: production.orders is a legal override of the value inherited from base. Both first and second should therefore contain source: production.orders, and the standard SafeLoader accepts the document.

However, PyYAML modifies the merged mapping while constructing first. When second later references that mapping, the new duplicate-key check scans the modified representation and reports source as a duplicate.

Could we ensure that each original mapping is checked before merge expansion and is not checked again after PyYAML has modified it? A regression test using the example above would help protect this behavior.

[iamrajatrana]

Confirmed and fixed in 97b4a2e - same root cause as your other comment. Because the pre-pass runs before any flattening, the aliased mapping is never re-inspected in its mutated form, and your example now loads with both first and second resolving to production.orders. Details in the summary comment below.

[flyrain]

cc @RussellSpitzer @jbonofre

[iamrajatrana]

Thanks for the review @flyrain - both comments were valid, and they share one root cause. Fixed in 97b4a2e.

Root cause. Duplicate checking was tied to construct_mapping, but PyYAML's flatten_mapping can traverse and mutate merge-source mapping nodes directly without invoking that override:

• First comment (false negative): an inline merge source such as <<: {name: orders, name: customers} was flattened directly. Its duplicate was never checked, so it became {'name': 'customers'} and the validator exited 0.

• Second comment (false positive): while constructing first, flatten_mapping mutated the aliased mapping's node.value. When that node was later constructed through second, the duplicate check inspected the expanded representation and treated a legal inherited-key override as an authored duplicate. Confirmed against stock SafeLoader, which accepts that document.

Fix. construct_document now performs a pre-pass over the composed node graph before mapping construction or merge flattening. Each mapping is therefore checked in its authored form, including mappings used directly as merge sources. Nodes are tracked by identity, so aliases and recursive anchors are visited once. The pre-pass constructs scalar keys for semantic comparison but rejects collection-valued keys directly, avoiding mutation of the graph being inspected.

Existing behavior is preserved for legal merge overrides, sequence-form merges, repeated explicit << rejection, merge keys versus a quoted "<<" key, and duplicate-error messages and source marks.

Tests: 16 → 34. Coverage now includes both reported cases, anchored merge sources, construction-order independence, sequence-form merges, recursive mapping and sequence aliases, unhashable keys and marks, scalar-key equivalence (1/01, yes/true, "1" versus 1), and multi-document loading. Against the pre-fix loader, the new suite fails only the two reported regressions. The order-independence case already passed before the fix and is intentionally retained as a forward guard.

uv run validation/test_validate.py                                → 34 passed
uv run validation/validate.py examples/tpcds_semantic_model.yaml  → PASSED
ruff check, git diff --check                                      → clean

YAML 1.1 versus 1.2. This PR retains the merge behavior currently accepted by PyYAML. Rejecting << would be a separate compatibility decision that should be made consistently across Ossie's YAML entry points, including converters that call yaml.safe_load directly. I can open a follow-up issue to track that policy decision if you'd prefer.