Skip to content

[build] derive per-type inbound/outbound directionality in the shared schema - #17864

Merged
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:bidi-schema-directionality
Aug 3, 2026
Merged

[build] derive per-type inbound/outbound directionality in the shared schema#17864
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:bidi-schema-directionality

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

Builds on the shared BiDi schema projector (#17700).

💥 What does this PR do?

  • The shared BiDi schema now records, per structured type, whether a caller sends it (outbound), receives it (inbound), or both.
  • Every language binding can now read one authoritative signal to tell send-side types from receive-only ones, instead of each re-deriving it.

🔧 Implementation Notes

  • inbound and outbound are independent booleans, so all four combinations are meaningful — a type used both ways (network.Cookie) is (true, true); a flattened base or message envelope that is part of no message is (false, false).
  • Additive and backward compatible

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: the directionality pass in the schema projector and its unit tests
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • Follow-on PR for Ruby to implement outbound-scoped domain accessors

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

@selenium-ci selenium-ci added the C-nodejs JavaScript Bindings label Aug 3, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Derive per-type inbound/outbound reachability flags in shared BiDi schema

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Compute per-structured-type inbound/outbound flags from command/event message reachability.
• Reuse a single ref-walk implementation for both integrity checks and directionality.
• Add unit tests covering all flag combinations and validator behavior.
Diagram

graph TD
  M["Model (commands/events)"] --> P["projectSchema()"] --> R["reachableTypes()"] --> T[("schema.types")]
  T --> CS["checkSchema()"] --> OK["Validators pass"]
  T --> CC["checkCompleteness()"] --> OK
  TEST["project_bidi_schema_test.mjs"] --> P

  subgraph Legend
    direction LR
    _p["Process"] ~~~ _d[("Data")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Heuristic directionality (name-based or domain-based)
  • ➕ Lower implementation effort; no graph traversal needed
  • ➕ May be sufficient for simple schemas with strong naming conventions
  • ➖ Incorrect when structurally identical types differ by use-site (the PR explicitly tests this)
  • ➖ Brittle to schema evolution; encourages binding-specific divergence
2. Derive directionality during codegen per language
  • ➕ Keeps schema simpler; avoids adding new fields to shared schema
  • ➕ Allows generator-specific nuance in defining “send-side” vs “receive-side”
  • ➖ Reintroduces duplicated logic across bindings (the problem this PR solves)
  • ➖ Harder to keep consistent across languages and releases
3. Manually annotate directionality in the source CDDL/model
  • ➕ Explicit and potentially easier to reason about for humans
  • ➕ Avoids depending on reachability semantics if they ever change
  • ➖ Ongoing maintenance burden; easy to forget to update
  • ➖ More error-prone than a computed property; needs additional validation anyway

Recommendation: Keep the PR’s reachability-based computation in the shared projector. It is binding-neutral, robust to structural look-alikes, and reuses the same reference edges as schema validation, minimizing drift between “what the schema means” and “what the validators enforce.”

Files changed (2) +182 / -18

Enhancement (1) +80 / -18
project_bidi_schema.mjsAdd reachability-based inbound/outbound flags to projected structured types +80/-18

Add reachability-based inbound/outbound flags to projected structured types

• Introduces shared helpers to extract referenced type names and compute transitive reachability over projected type nodes. Uses command params refs as outbound roots and command result + event params refs as inbound roots, then annotates every record/union with boolean inbound/outbound flags. Updates schema validation to reuse the shared ref extraction, and extends completeness checks to fail if a structured type is missing either flag.

javascript/selenium-webdriver/project_bidi_schema.mjs

Tests (1) +102 / -0
project_bidi_schema_test.mjsAdd unit tests for directionality flags and completeness enforcement +102/-0

Add unit tests for directionality flags and completeness enforcement

• Adds a new test suite that constructs a minimal AST/model to verify all four directionality combinations, including types reachable from neither side. Ensures structurally similar types are split purely by reachability (not name), validates that checkSchema/checkCompleteness pass for correct schemas, and asserts completeness fails closed if a flag is missing. Also confirms enums/aliases are not required to carry directionality flags.

javascript/selenium-webdriver/project_bidi_schema_test.mjs

@titusfortner
titusfortner force-pushed the bidi-schema-directionality branch from 825f545 to 8f5df6c Compare August 3, 2026 02:58
@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 [bidi] derive per-type inbound/outbound directionality in the shared schema [build] derive per-type inbound/outbound directionality in the shared schema Aug 3, 2026
@titusfortner
titusfortner merged commit 094f26f into SeleniumHQ:trunk Aug 3, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-nodejs JavaScript Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants