[py] update new BiDi layer generation to conform to latest proposed ADR

[titusfortner]

🔗 Related Issues

Builds on #17761, which landed the generated _bidi layer
Aligns with the low-level contract proposed in #17786
Ruby made the same two behavioral changes in #17936 and #17939

💥 What does this PR do?

• A required field missing from an inbound payload now errors, instead of being left unset with a warning (decision 8).
• An integer field accepts a whole number the remote spelled as 5.0, and rejects only a fractional one (decision 4).
• An outbound bare scalar on a union is checked against the arms the schema pins, so a value like Origin("banana") fails locally instead of being sent (decisions 4 and 5).

🔧 Implementation Notes

• Missing required fields. This deletes the tolerance path — strict_inbound() and its context variable are gone — rather than adding a stricter mode alongside it, matching what [rb] always reject a missing required inbound BiDi field #17936 did in Ruby. One error names every field that was missing, since they were already being collected. Inbound handling of undeclared properties is unchanged: warn and drop on a closed type, keep silently on an extensible one.
• Whole-valued floats. A browser is free to send 5 or 5.0 for an integer, since JS has no int/float split, so the check matches by JSON kind rather than Python type. Inbound normalizes to int so the field still holds its declared type; outbound accepts either and sends what the caller set.
• Bare-scalar arms. The generator emits the schema's scalarValues for a non-object-only union and fails at generation time if one declares none, so the runtime can never quietly fall back to accepting any scalar. input.Origin is the only such union today.
• ADR cross-references removed from generator, runtime, and test comments — [adr] Behavioral contract for the low-level WebDriver BiDi layer #17786 is still a proposal and has renumbered its decisions twice, so two of the references had already drifted onto the wrong decision.

🤖 AI assistance

• AI assisted (complete below)
  • Tool(s): Claude Code (Opus 5)
  • What was generated: the conformance audit against the ADR, the implementation, the tests, and this description
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

Typed BiDi exceptions and the schema's Firefox moz: install options are Ruby parity rather than contract conformance, so they follow in a separate PR.

🔄 Types of changes

• Breaking change (inbound tolerance is removed, though _bidi is internal and not yet consumed by the public API)

[qodo-code-review]

PR Summary by Qodo

[py] Align generated BiDi serialization with low-level contract semantics

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Error on inbound payloads missing required fields; keep undeclared-property behavior unchanged.
• Accept whole-valued floats for integer primitives and normalize inbound values to int.
• Validate outbound bare-scalars against union scalar arms; fail generation without scalarValues.

Diagram

graph TD
  A["generate_bidi_protocol.py"] -->|emits _SCALAR_VALUES| B["Generated Union classes"] -->|used by| C["_bidi/serialization.py"]
  D["Client code"] -->|as_json / build| C -->|outbound JSON| E{{"BiDi wire"}}
  E -->|inbound JSON| C -->|from_json| D

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep strict_inbound as an opt-in mode

• ➕ Avoids breaking existing consumers that relied on tolerant inbound behavior
• ➕ Allows gradual rollout of strictness
• ➖ Diverges from the low-level contract this layer aims to implement
• ➖ Creates two behavioral modes to support and test indefinitely
• ➖ Encourages silent partial objects (UNSET required fields) to leak into caller logic

2. Derive allowed bare-scalar values from union arms at runtime

• ➕ Avoids adding scalarValues plumbing to the generator
• ➕ Potentially more flexible if schema evolves
• ➖ Runtime would need to inspect schema/IR not available in generated code
• ➖ Harder to guarantee correctness; failures happen later (caller runtime)
• ➖ More overhead and complexity in the hot serialization path

3. Permit any scalar for non-object-only unions (status quo)

• ➕ Maximally tolerant of unexpected scalar payloads
• ➖ Sends invalid values over the wire (delayed failure)
• ➖ Contradicts schema-pinned arms and undermines local validation guarantees
• ➖ Harder to debug because errors occur remotely

Recommendation: Prefer the PR’s approach: make inbound required-field absence a hard error, accept whole-valued floats for integer primitives to match JSON/JS reality, and validate outbound bare-scalars against schema-pinned union arms. Failing generation when scalarValues are missing is the right tradeoff because it prevents silent broadening of accepted scalars and moves the failure to build-time rather than user runtime.

Files changed (5) +98 / -124

Bug fix (2) +63 / -67

generate_bidi_protocol.pyPlumb union scalarValues into IR and emit _SCALAR_VALUES +15/-7

Plumb union scalarValues into IR and emit _SCALAR_VALUES

• Extends UnionIR with scalar_values, populates it from schema scalarValues for alias-unions, and emits a _SCALAR_VALUES frozenset for generated Union subclasses. Adds a generation-time guard to fail non-object-only unions that would otherwise accept any scalar at runtime. Removes unstable ADR decision references from comments.

py/generate_bidi_protocol.py

serialization.pyTighten inbound required-field handling and validate union bare-scalars +48/-60

Tighten inbound required-field handling and validate union bare-scalars

• Removes strict_inbound tolerance mode and makes missing required inbound fields raise BiDiSerializationError with a bounded list of missing keys. Updates primitive checks so integer accepts whole-valued floats (e.g., 5.0) and normalizes inbound floats to int, while rejecting fractional floats and bools. Adds union outbound validation against generated _SCALAR_VALUES, and cleans up ADR cross-references in docs/comments.

py/selenium/webdriver/common/_bidi/serialization.py

Tests (3) +35 / -57

protocol_tests.pyUpdate end-to-end test docstring wording +2/-2

Update end-to-end test docstring wording

• Adjusts commentary describing event routing scope to remove ADR references and keep the explanation accurate.

py/test/selenium/webdriver/common/bidi/protocol_tests.py

bidi_protocol_command_tests.pyRefresh test commentary to remove ADR references +2/-2

Refresh test commentary to remove ADR references

• Updates inline comments about extensible extras behavior without changing test assertions or runtime behavior.

py/test/unit/selenium/webdriver/common/bidi_protocol_command_tests.py

bidi_serialization_tests.pyRewrite serialization tests for strict required fields, whole ints, and scalar arms +31/-53

Rewrite serialization tests for strict required fields, whole ints, and scalar arms

• Removes strict_inbound tests and replaces them with assertions that missing required inbound fields always error and enumerate all missing keys once. Adds coverage for rejecting fractional outbound ints, accepting inbound whole-valued float ints (normalized to int), and enforcing pinned union bare-scalar values via _SCALAR_VALUES. Updates section headings/comments to remove ADR cross-references while preserving intent.

py/test/unit/selenium/webdriver/common/bidi_serialization_tests.py

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Scalar arms sort crashes ✓ Resolved 🐞 Bug ≡ Correctness

Description

Union.validate_outbound sorts _SCALAR_VALUES to format the error message; if the schema’s
scalarValues contain mixed primitive types (e.g., str and int), Python raises TypeError during
sorting, masking the real validation error. This turns a caller mistake into an unexpected crash
path.

Code

py/selenium/webdriver/common/_bidi/serialization.py[R580-582]

+        if value not in cls._SCALAR_VALUES:
+            expected = ", ".join(repr(v) for v in sorted(cls._SCALAR_VALUES))
+            raise BiDiSerializationError(f"{owner}.{name}: {value!r} is not one of {cls.__name__}'s arms ({expected})")

Evidence

The PR introduces sorting of _SCALAR_VALUES when formatting the outbound validation error. The
schema tooling explicitly allows literal sets with mixed primitives (returns undefined for a
shared primitive when mixed) but still emits scalarValues, so _SCALAR_VALUES can legally contain
heterogeneous types; sorting such a set is a TypeError in Python 3.

py/selenium/webdriver/common/_bidi/serialization.py[561-583]
javascript/selenium-webdriver/project_bidi_schema.mjs[122-166]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Union.validate_outbound()` builds an error message by calling `sorted(cls._SCALAR_VALUES)`. If `_SCALAR_VALUES` contains mixed, non-orderable types (e.g. `{"a", 1}`), this raises `TypeError: '<' not supported...` while trying to raise `BiDiSerializationError`.

### Issue Context
The schema projector can emit `scalarValues` for `{ const }` arms regardless of whether the literals share a primitive type (it explicitly supports mixed literal types).

### Fix Focus Areas
- py/selenium/webdriver/common/_bidi/serialization.py[561-583]

### Suggested change
Replace `sorted(cls._SCALAR_VALUES)` with a sort over `repr(v)` (or use `key=repr`) so ordering is always defined:

- `expected = ", ".join(sorted((repr(v) for v in cls._SCALAR_VALUES)))`

This keeps output stable and avoids TypeError for mixed literal types.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Inbound scalar not validated ✓ Resolved 🐞 Bug ≡ Correctness

Description

Union.from_json returns any non-dict payload unchanged for non-object-only unions, even when the
generator now emits pinned _SCALAR_VALUES for the union’s scalar arm. This allows inbound values
outside the schema’s declared scalar literals and contradicts the new validate_outbound docstring
claim that inbound errors on the same values.

Code

py/selenium/webdriver/common/_bidi/serialization.py[R565-567]

+        A variant instance passes. A bare scalar passes only for a union that has a scalar arm, and
+        only as one of the literals that arm declares, so a stray string is a caller error rather
+        than a wire round-trip. This mirrors inbound dispatch, which errors on the same values.

Evidence

The generator now has enough information to pin allowed scalar literals (_SCALAR_VALUES), and
outbound validation uses it. However inbound union parsing still returns any scalar payload
unchanged without checking _SCALAR_VALUES, despite the docstring stating inbound errors on the
same values.

py/selenium/webdriver/common/_bidi/serialization.py[561-600]
py/generate_bidi_protocol.py[390-406]
py/generate_bidi_protocol.py[871-897]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Union.from_json()` accepts any non-dict payload for non-object-only unions by returning it unchanged. With this PR, the generator emits `_SCALAR_VALUES` for unions whose scalar arm is pinned to specific literals, and outbound validation rejects values outside that set.

This creates an inbound/outbound mismatch and allows inbound values that violate the schema’s pinned scalar literals.

### Issue Context
- Generator now emits `_SCALAR_VALUES` for unions with `scalarValues`.
- `Union.validate_outbound()` enforces membership in `_SCALAR_VALUES`.
- `Union.from_json()` does not check `_SCALAR_VALUES` at all for scalar payloads.

### Fix Focus Areas
- py/selenium/webdriver/common/_bidi/serialization.py[562-600]
- py/generate_bidi_protocol.py[390-406]
- py/generate_bidi_protocol.py[871-897]

### Suggested change
In `Union.from_json()`, before returning a non-dict payload for a non-object-only union, enforce pinned scalar membership when `_SCALAR_VALUES` is non-empty:

```py
if not isinstance(payload, dict):
   if cls._OBJECT_ONLY: ...
   if cls._SCALAR_VALUES and payload not in cls._SCALAR_VALUES:
       expected = ", ".join(sorted((repr(v) for v in cls._SCALAR_VALUES)))
       raise BiDiSerializationError(
           f"{cls.__name__}: {payload!r} is not one of {cls.__name__}'s arms ({expected})"
       )
   return payload
```

Also update the validate_outbound docstring sentence about inbound behavior to match the implemented behavior (or keep it and make inbound actually error as above).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: ⚖️ Balanced

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-code-review]

Code review by qodo was updated up to the latest commit 955708c