Skip to content

[build] BiDi schema generation cleanup - #17837

Merged
titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:js-bidi-cddl-cleanup
Jul 28, 2026
Merged

[build] BiDi schema generation cleanup#17837
titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:js-bidi-cddl-cleanup

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

💥 What does this PR do?

Two independent cleanups to the JavaScript BiDi CDDL→schema generation pipeline; the generated schema/TS output is unchanged.

  • Schema generation accepts multiple --cddl files and concatenates their parsed definitions
  • Aligns schema projection's handling of extensible records with what cddl2ts already does

🔧 Implementation Notes

  • Should never have needed to access //py/private:merge_cddl from JS generation code
  • Top-level CDDL productions are position-independent (refs resolve by name), so parsing each spec separately and concatenating equals parsing one merged file — no merge tool required.
  • js_run_binary rejects external/cross-package srcs, so each spec is staged into the package with copy_file first (same pattern the dfns/spec_html steps already use).
  • The cddl parser emits an unbounded bound as Infinity; only the AST's JSON round-trip coerces it to null. The old m === null check silently relied on that coercion — the fix accepts either, covered by a regression test that projects an Infinity AST directly.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: the build-pipeline refactor and the projector fix + its regression test
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • Groundwork for a planned follow-up PR adding support for vendor-specific CDDL extensions

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Bug fix (backwards compatible)

@selenium-ci selenium-ci added C-nodejs JavaScript Bindings B-build Includes scripting, bazel and CI integrations labels Jul 28, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Parse multiple BiDi CDDL specs and fix schema projection

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Parses multiple BiDi CDDL specifications directly, removing the Python merge dependency.
• Stages external specifications for Bazel-compatible JavaScript generation.
• Handles raw Infinity bounds during extensible-record projection, with regression coverage.
Diagram

graph TD
  A["CDDL Specs"] --> B["Staged Copies"] --> C["CDDL Parser"] --> D["Combined AST"] --> E["Model Builder"] --> H["TypeScript Output"]
  D --> F["Schema Projector"] --> G["Schema Output"]
  E --> F
  D --> H
Loading
High-Level Assessment

The current approach is appropriate because top-level CDDL references resolve by name, making independently parsed and concatenated definitions equivalent to merged source text. Retaining a separate Python merge action would preserve unnecessary cross-language and private-package coupling, while direct external inputs are incompatible with js_run_binary; package-local staging is therefore the simplest compatible design.

Files changed (5) +50 / -70

Enhancement (1) +8 / -4
generate_bidi.mjsAccept and combine multiple CDDL inputs +8/-4

Accept and combine multiple CDDL inputs

• Allows repeated --cddl arguments and concatenates definitions from independently parsed specifications into one AST. Updates validation and usage text for the multi-input interface.

javascript/selenium-webdriver/generate_bidi.mjs

Bug fix (1) +14 / -9
project_bidi_schema.mjsRecognize Infinity as an unbounded occurrence +14/-9

Recognize Infinity as an unbounded occurrence

• Adds a shared unbounded-occurrence check accepting both parser-native Infinity and JSON-coerced null. Extensible maps and structural spreads no longer depend on an AST JSON round trip for correct projection.

javascript/selenium-webdriver/project_bidi_schema.mjs

Refactor (1) +16 / -55
generate_bidi.bzlReplace CDDL merge action with staged parser inputs +16/-55

Replace CDDL merge action with staged parser inputs

• Removes the custom CDDL merge rule and Python merge-tool dependency. Copies each specification into the package and passes repeated --cddl arguments to the JavaScript generator.

javascript/selenium-webdriver/private/generate_bidi.bzl

Tests (1) +10 / -0
project_bidi_schema_test.mjsCover direct projection of Infinity-bounded records +10/-0

Cover direct projection of Infinity-bounded records

• Adds a regression test proving a raw parser AST with an Infinity upper bound produces an extensible record without a phantom text field.

javascript/selenium-webdriver/project_bidi_schema_test.mjs

Documentation (1) +2 / -2
BUILD.bazelDocument direct parsing of adjacent CDDL specs +2/-2

Document direct parsing of adjacent CDDL specs

• Updates the generation target comment to describe adjacent specifications as separately parsed inputs rather than merged source files.

javascript/selenium-webdriver/BUILD.bazel

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

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

Qodo Logo

@titusfortner titusfortner changed the title [js] BiDi CDDL parse + schema-projection cleanup [build] BiDi schema generation cleanup Jul 28, 2026
@titusfortner
titusfortner merged commit 9fd65b5 into SeleniumHQ:trunk Jul 28, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-nodejs JavaScript Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants