[bidi] Correct float/enum type fidelity in the shared schema and validate primitives outbound in Ruby

[titusfortner]

🔗 Related Issues

• [adr] Behavioral contract for the low-level WebDriver BiDi layer #17786
• fix(cddl): preserve float literals so (0.0..1.0) is distinct from (0..1) webdriverio/cddl#77

💥 What does this PR do?

• Fixes two type-fidelity bugs in the shared WebDriver BiDi schema:
  • Float parameters were typed as integers.
  • A nullable inline enum was degraded to a free string
• Adds outbound primitive validation in Ruby code to validate schema changes
• Updates cddl npm version to support the schema fixes

🔧 Implementation Notes

• The float fix depends on the cddl bump. Previously (0.0..1.0) and (0..1) both parse to integer-valued bounds, so the parser needed to define IsFloat to distinguish them
• The enum fix needs no new Ruby code: the existing enum-validation path picks up the hoisted type.

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s): Claude Code
  • What was generated: the IsFloat schema-projection fix, the nullable-inline-enum hoisting fix, the Ruby outbound primitive validation, their tests, and the regenerated protocol files.
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

• Behavior change on the internal (@api private) BiDi layer: scrollbar_type now takes a Symbol (:classic/:overlay) instead of a String — the intended closed-vocabulary form.
• Slightly stricter outbound: a wrong-typed primitive that previously reached the wire is now rejected locally. Values that already worked are unaffected.

🔄 Types of changes

• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)

[qodo-code-review]

PR Summary by Qodo

Fix BiDi schema number/enum fidelity and add Ruby outbound primitive validation

🐞 Bug fix ✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Fix schema projection so float ranges map to number, not integer.
• Hoist nullable inline literal choices into named enums to preserve closed vocabularies.
• Add Ruby outbound primitive-type validation and update generated protocol types/tests.

Diagram

graph TD
  A["cddl (npm)"] --> B["normalize_bidi_ast.mjs"] --> C["project_bidi_schema.mjs"] --> D["Projected BiDi schema"] --> E["Ruby BiDi protocol files"] --> F["Serialization::Record validation"]
  G["Ruby BiDi domains"] --> E
  G --> F

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Validate primitives only during serialization (as_json / wire_value)

• ➕ Keeps object construction permissive; errors surface only when actually sending over the wire
• ➕ Reduces risk of breaking callers that build objects but never transmit them
• ➖ Bugs are detected later and are harder to attribute to the call site
• ➖ Invalid objects can exist in memory and fail nondeterministically depending on code paths

2. Generate per-field Ruby type checks in each record initializer

• ➕ Validation is fully localized per class/field; potentially clearer errors
• ➕ Avoids adding more logic to the shared Serialization::Record base
• ➖ Codegen and maintenance cost increases significantly
• ➖ Harder to keep consistent across all protocol types and future schema changes

Recommendation: Keep the PR’s approach: schema fidelity fixes belong in the shared JS normalization/projection pipeline, and outbound primitive validation in Serialization::Record provides consistent, early, and low-maintenance enforcement across all generated Ruby protocol types. The alternatives either delay failures until send-time or increase generator complexity and drift risk.

Files changed (15) +168 / -55

Enhancement (1) +26 / -6

record.rbAdd outbound primitive type validation to Record construction +26/-6

Add outbound primitive type validation to Record construction

• Refactors outbound validation to a 'validate_present' helper and adds 'check_outbound_primitive' for scalar primitive enforcement. This rejects wrong-typed primitives locally during record initialization while keeping list handling consistent with inbound behavior.

rb/lib/selenium/webdriver/bidi/serialization/record.rb

Bug fix (8) +66 / -31

normalize_bidi_ast.mjsHoist nullable inline enums while preserving nullability on fields +23/-8

Hoist nullable inline enums while preserving nullability on fields

• Extends inline-enum hoisting to accept choices of >=2 literals plus a null arm. The synthetic enum definition now contains only literal values, while the referencing field retains the null alternative so the enum remains nullable without degrading to a free string.

javascript/selenium-webdriver/normalize_bidi_ast.mjs

project_bidi_schema.mjsProject float-bounded ranges as number using IsFloat markers +6/-2

Project float-bounded ranges as number using IsFloat markers

• Adjusts range primitive inference to consult the parser’s 'IsFloat' marker on bounds. This prevents '(0.0..1.0)'-style ranges from being incorrectly inferred as 'integer' due to integral bound values.

javascript/selenium-webdriver/project_bidi_schema.mjs

browsing_context.rbCorrect ImageFormat.quality primitive from integer to number +1/-1

Correct ImageFormat.quality primitive from integer to number

• Updates the generated BiDi BrowsingContext ImageFormat record so 'quality' is typed as 'number' (float-capable) rather than 'integer', matching the schema fix.

rb/lib/selenium/webdriver/bidi/protocol/browsing_context.rb

emulation.rbFix geolocation primitives and validate scrollbarType via hoisted enum +18/-4

Fix geolocation primitives and validate scrollbarType via hoisted enum

• Changes geolocation coordinate fields (latitude/longitude/heading) from integer to number primitives. Introduces a named enum constant for scrollbar type override and validates outbound values against it before record construction.

rb/lib/selenium/webdriver/bidi/protocol/emulation.rb

input.rbCorrect pointer pressure primitives from integer to number +6/-6

Correct pointer pressure primitives from integer to number

• Updates generated input action records so 'pressure' and 'tangentialPressure' accept numbers, aligning Ruby protocol typing with the corrected schema.

rb/lib/selenium/webdriver/bidi/protocol/input.rb

browsing_context.rbsUpdate RBS for quality to Numeric +1/-1

Update RBS for quality to Numeric

• Adjusts RBS signatures to reflect 'quality' being numeric rather than integer, matching the regenerated Ruby protocol type.

rb/sig/lib/selenium/webdriver/bidi/protocol/browsing_context.rbs

emulation.rbsUpdate RBS for numeric geolocation and Symbol scrollbarType +8/-6

Update RBS for numeric geolocation and Symbol scrollbarType

• Updates RBS types for geolocation numeric fields and changes scrollbar_type from String? to Symbol?. Adds the enum constant signature used for scrollbar type validation.

rb/sig/lib/selenium/webdriver/bidi/protocol/emulation.rbs

input.rbsUpdate RBS for pointer pressure fields to Numeric +3/-3

Update RBS for pointer pressure fields to Numeric

• Updates constructor signatures so pressure and tangential_pressure parameters are Numeric rather than Integer, aligning with schema-driven protocol changes.

rb/sig/lib/selenium/webdriver/bidi/protocol/input.rbs

Tests (3) +60 / -15

normalize_bidi_ast_test.mjsAdd coverage for nullable inline-enum hoisting behavior +12/-0

Add coverage for nullable inline-enum hoisting behavior

• Adds a test ensuring nullable literal choices are hoisted into a named enum while keeping 'null' on the field reference and out of the enum definition.

javascript/selenium-webdriver/normalize_bidi_ast_test.mjs

project_bidi_schema_test.mjsTest number-vs-integer inference for IsFloat-marked ranges and hoisted enums +16/-8

Test number-vs-integer inference for IsFloat-marked ranges and hoisted enums

• Adds a test case for IsFloat-marked integral bounds projecting to 'number'. Updates the enum signal test to assert nullable inline enums are now hoisted to named enums with nullable references.

javascript/selenium-webdriver/project_bidi_schema_test.mjs

serialization_spec.rbAdd specs for outbound primitive checks and nullable hoisted enum validation +32/-7

Add specs for outbound primitive checks and nullable hoisted enum validation

• Adds tests ensuring wrong-typed primitives are rejected at construction, number fields accept ints/floats, and the hoisted nullable scrollbarType enum validates both outbound symbols and inbound wire tokens (while still allowing null). Removes an obsolete inline-enum primitive test now that the enum is hoisted.

rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb

Other (3) +16 / -3

package.jsonBump cddl devDependency to 0.21.1 +1/-1

Bump cddl devDependency to 0.21.1

• Updates the cddl dependency to pick up parser support needed to distinguish float bounds in ranges.

javascript/selenium-webdriver/package.json

pnpm-lock.yamlLockfile update for cddl 0.21.1 +11/-2

Lockfile update for cddl 0.21.1

• Updates pnpm lock entries to reflect the cddl 0.21.1 bump and its resolved snapshot metadata.

pnpm-lock.yaml

serialization.rbsExpose new outbound validation helpers in RBS +4/-0

Expose new outbound validation helpers in RBS

• Adds signatures for 'validate_present' and 'check_outbound_primitive' to keep the Serialization::Record interface consistent with the new outbound validation logic.

rb/sig/lib/selenium/webdriver/bidi/serialization.rbs

[qodo-code-review]

Code Review by Qodo

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

1. Redundant isLiteral JSDoc 📘 Rule violation ⚙ Maintainability

Description

The new JSDoc comments for isLiteral()/isNullArm() restate what the code does rather than
explaining the rationale for introducing these helpers. This reduces maintainability by adding
narration instead of intent-focused documentation.

Code

javascript/selenium-webdriver/normalize_bidi_ast.mjs[R100-108]

+/** True when `entry` is a string/number/bool literal (`{Type:'literal', Value}`). */
+function isLiteral(entry) {
+  return entry && typeof entry === 'object' && entry.Type === 'literal'
+}
+
+/** True when `entry` is the CDDL null keyword (bare `'null'`) or a `nil`/`null` prelude ref. */
+function isNullArm(entry) {
+  return entry === 'null' || (isGroupRef(entry) && (entry.Value === 'null' || entry.Value === 'nil'))
+}

Evidence

PR Compliance ID 8 requires comments to focus on rationale rather than narrating code behavior. The
added JSDoc in normalize_bidi_ast.mjs describes exactly what the functions do ("True when ..."),
duplicating what is already clear from the predicate implementations.

AGENTS.md: Comments Should Explain Why, Not What
javascript/selenium-webdriver/normalize_bidi_ast.mjs[100-108]

Agent prompt

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

## Issue description
New comments narrate behavior ("True when...") instead of documenting intent/rationale, which conflicts with the project guideline that comments should explain why, not what.

## Issue Context
The added helpers `isLiteral()` and `isNullArm()` are straightforward predicates; the current comments primarily duplicate the function bodies.

## Fix Focus Areas
- javascript/selenium-webdriver/normalize_bidi_ast.mjs[100-108]

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

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}