[adr] BiDi low-level definitions are generated from a shared spec model without orchestration

[titusfortner]

📄 The decision, its rationale, considered options, and consequences are in the record file
this PR adds; read it there. The sections below are proposal notes and review logistics.

View ADR Directly: https://github.com/titusfortner/selenium/blob/adr-bidi-cddl/docs/decisions/17701-bidi-generated-protocol-layer.md

🔗 Related

• Builds on [adr] BiDi is an implementation mechanism, not a public API #17670 (BiDi is an internal implementation mechanism); assumes it is accepted.
• Alternative to [adr] Java CDDL generator proposal #17692 (Java BiDi generator proposal).

📝 Proposal notes

• One record because the three decisions are facets of one design for the generated definitions layer — one context, one rationale.
• Out of scope here: the high-level API, the transport substrate, and orchestration internals (left to each binding).

🗣 TLC discussion

Pending — not yet discussed.

📌 Tracking

Tracking issue: (linked on acceptance)

[qodo-code-review]

PR Summary by Qodo

Add ADR for generating BiDi low-level protocol definitions from shared spec model
📝 Documentation 🕐 Less than 10 minutes

Description

• Document BiDi layering and responsibilities from transport through high-level API
• Decide to generate read-only low-level definitions from a single shared spec model
• Specify orchestration imports definitions rather than injecting logic into generated code

Diagram

graph TD
  Spec{{"WebDriver BiDi spec (CDDL)"}} --> Shared["Shared spec model"] --> Gen["Generated definitions"] --> Orch["Orchestration layer"] --> API["High-level API"]
  Transport["Transport/session substrate"] --> Orch
  Transport --> Gen

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Split into multiple ADRs (shared model, immutability, no injection)

• ➕ Each decision can be discussed/accepted independently
• ➕ Smaller docs can be easier to scan and reference later
• ➖ Repeats the same context and background across multiple records
• ➖ Harder to see the decisions as a single coherent design for the generated definitions layer

2. Add as a BiDi design doc instead of an ADR

• ➕ More room for examples and binding-specific guidance
• ➕ Can evolve without the formality of ADR lifecycle
• ➖ Less explicit about the decision and rationale
• ➖ Harder to track acceptance status and considered options over time

Recommendation: Keep the single ADR as proposed: the three points are tightly coupled facets of one generated-definitions design, and the record format captures rationale and rejected alternatives cleanly. Consider follow-up design docs only if bindings need implementation-level guidance/examples beyond the decision itself.

Files changed (1) +101 / -0

Documentation (1) +101 / -0

nnnnn-bidi-generated-protocol-layer.mdNew ADR defining BiDi generated low-level protocol layer strategy +101/-0

New ADR defining BiDi generated low-level protocol layer strategy

• Adds a proposed ADR describing BiDi’s four-layer architecture and standardizing the low-level definitions layer. Specifies generation from a single shared spec model, immutability/read-only semantics for generated objects, and a strict separation where orchestration imports (not injects into) generated definitions.

docs/decisions/nnnnn-bidi-generated-protocol-layer.md

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 11 rules

1. No binding-status section 🐞 Bug ⚙ Maintainability

Description

The new ADR omits the standard “## Binding status” section/table, so there’s no in-record place to
track per-binding convergence as the decision-process documentation expects. This makes
implementation tracking and later updates to the record less consistent with the repository’s ADR
workflow.

Code

docs/decisions/17701-bidi-generated-protocol-layer.md[R91-101]

+## Consequences
+
+- The supported API depends only on the wrapper, not on the generated definitions'
+  shape, so regenerating from a changed spec does not change what users program against.
+- The generated definitions can live in their own namespace that the higher layers migrate onto,
+  and the existing implementation is retired.
+- Orchestration and the high-level API are checked-in source, navigable and reviewable, and
+  regenerating the definitions never touches them.
+- A binding that today combines the layers in one class (with injected orchestration and
+  enhancements) splits them: the generated definitions move to their own namespace, which the
+  orchestration and high-level API import.

Evidence

The ADR template defines a “## Binding status” section and table for tracking convergence, and the
decisions README explicitly calls out updating that binding-status table as bindings implement the
decision; the new ADR currently ends after “## Consequences” and contains no such section.

docs/decisions/0000-template.md[39-51]
docs/decisions/README.md[43-44]
docs/decisions/17701-bidi-generated-protocol-layer.md[91-101]

Agent prompt

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

### Issue description
The ADR ends after **Consequences** and does not include the repository’s standard **Binding status** section/table, which the ADR template and process docs use to track per-language implementation progress.

### Issue Context
This repository’s ADR template includes a dedicated section for tracking binding convergence, and the decisions README references updating that table as bindings implement the decision.

### Fix Focus Areas
- docs/decisions/17701-bidi-generated-protocol-layer.md[91-101]
- docs/decisions/0000-template.md[39-51]
- docs/decisions/README.md[43-44]

### Suggested change
Append a `## Binding status` section at the end of the ADR, including the standard table (Java/Python/Ruby/.NET/JavaScript) with initial statuses (e.g., `pending`) and optional tracking links/notes.

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

---

2. Missing ADR metadata ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new decision record deviates from the repository’s ADR template by omitting the Date: metadata
field and by leaving the Discussion: field as “_PR pending_”, which reduces traceability for when
and where the decision was discussed.

Code

docs/decisions/nnnnn-bidi-generated-protocol-layer.md[R3-5]

+- Status: Proposed
+- Discussion: _PR pending_
+

Evidence

The template explicitly includes a Date field and expects Discussion to be a link; the new record’s
header omits Date and uses a placeholder for Discussion.

docs/decisions/nnnnn-bidi-generated-protocol-layer.md[3-5]
docs/decisions/0000-template.md[6-9]

Agent prompt

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

### Issue description
The ADR header is missing the `Date:` field and does not link to the PR in the `Discussion:` field, deviating from the project’s decision record template.

### Issue Context
The provided template includes `Date: YYYY-MM-DD` and a `Discussion:` link to the record’s PR to preserve traceability.

### Fix Focus Areas
- docs/decisions/nnnnn-bidi-generated-protocol-layer.md[3-5]
- docs/decisions/0000-template.md[6-9]

### Suggested change
1. Add a `- Date: YYYY-MM-DD` line (date the status last changed).
2. Replace `_PR pending_` with a link to this PR (or the final discussion reference) before merge.

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

---

3. Placeholder ADR number ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new decision record uses the placeholder ID “NNNNN/nnnnn” in both the filename and H1 header,
which conflicts with the documented requirement to use the next unused numeric decision ID and makes
the record hard to cite reliably.

Code

docs/decisions/nnnnn-bidi-generated-protocol-layer.md[1]

+# NNNNN. BiDi's low-level definitions are generated from a shared spec model and imported, not injected

Evidence

The new record’s title line shows a placeholder ID, while the decisions README specifies using the
next unused numeric ID and notes that numbers are stable once merged.

docs/decisions/nnnnn-bidi-generated-protocol-layer.md[1-1]
docs/decisions/README.md[30-32]
docs/decisions/README.md[68-69]

Agent prompt

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

### Issue description
The decision record is using a placeholder decision number (`NNNNN` / `nnnnn`) instead of a real numeric decision ID.

### Issue Context
The decisions README documents that each record should be numbered using the next unused number and that numbers become stable references once merged.

### Fix Focus Areas
- docs/decisions/nnnnn-bidi-generated-protocol-layer.md[1-1]
- docs/decisions/README.md[30-32]
- docs/decisions/README.md[68-69]

### Suggested change
1. Rename the file to the next available numeric prefix (e.g., `0001-bidi-generated-protocol-layer.md` if `0001` is next).
2. Update the H1 header to match the same numeric ID.

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

---

Previous review results

Review updated until commit 67b42c9

Results up to commit 6d22642

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Missing ADR metadata ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new decision record deviates from the repository’s ADR template by omitting the Date: metadata
field and by leaving the Discussion: field as “_PR pending_”, which reduces traceability for when
and where the decision was discussed.

Code

docs/decisions/nnnnn-bidi-generated-protocol-layer.md[R3-5]

+- Status: Proposed
+- Discussion: _PR pending_
+

Evidence

The template explicitly includes a Date field and expects Discussion to be a link; the new record’s
header omits Date and uses a placeholder for Discussion.

docs/decisions/nnnnn-bidi-generated-protocol-layer.md[3-5]
docs/decisions/0000-template.md[6-9]

Agent prompt

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

### Issue description
The ADR header is missing the `Date:` field and does not link to the PR in the `Discussion:` field, deviating from the project’s decision record template.

### Issue Context
The provided template includes `Date: YYYY-MM-DD` and a `Discussion:` link to the record’s PR to preserve traceability.

### Fix Focus Areas
- docs/decisions/nnnnn-bidi-generated-protocol-layer.md[3-5]
- docs/decisions/0000-template.md[6-9]

### Suggested change
1. Add a `- Date: YYYY-MM-DD` line (date the status last changed).
2. Replace `_PR pending_` with a link to this PR (or the final discussion reference) before merge.

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

---

2. Placeholder ADR number ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new decision record uses the placeholder ID “NNNNN/nnnnn” in both the filename and H1 header,
which conflicts with the documented requirement to use the next unused numeric decision ID and makes
the record hard to cite reliably.

Code

docs/decisions/nnnnn-bidi-generated-protocol-layer.md[1]

+# NNNNN. BiDi's low-level definitions are generated from a shared spec model and imported, not injected

Evidence

The new record’s title line shows a placeholder ID, while the decisions README specifies using the
next unused numeric ID and notes that numbers are stable once merged.

docs/decisions/nnnnn-bidi-generated-protocol-layer.md[1-1]
docs/decisions/README.md[30-32]
docs/decisions/README.md[68-69]

Agent prompt

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

### Issue description
The decision record is using a placeholder decision number (`NNNNN` / `nnnnn`) instead of a real numeric decision ID.

### Issue Context
The decisions README documents that each record should be numbered using the next unused number and that numbers become stable references once merged.

### Fix Focus Areas
- docs/decisions/nnnnn-bidi-generated-protocol-layer.md[1-1]
- docs/decisions/README.md[30-32]
- docs/decisions/README.md[68-69]

### Suggested change
1. Rename the file to the next available numeric prefix (e.g., `0001-bidi-generated-protocol-layer.md` if `0001` is next).
2. Update the H1 header to match the same numeric ID.

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

---

Results up to commit 4b3f61d

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. No binding-status section 🐞 Bug ⚙ Maintainability

Description

The new ADR omits the standard “## Binding status” section/table, so there’s no in-record place to
track per-binding convergence as the decision-process documentation expects. This makes
implementation tracking and later updates to the record less consistent with the repository’s ADR
workflow.

Code

docs/decisions/17701-bidi-generated-protocol-layer.md[R91-101]

+## Consequences
+
+- The supported API depends only on the wrapper, not on the generated definitions'
+  shape, so regenerating from a changed spec does not change what users program against.
+- The generated definitions can live in their own namespace that the higher layers migrate onto,
+  and the existing implementation is retired.
+- Orchestration and the high-level API are checked-in source, navigable and reviewable, and
+  regenerating the definitions never touches them.
+- A binding that today combines the layers in one class (with injected orchestration and
+  enhancements) splits them: the generated definitions move to their own namespace, which the
+  orchestration and high-level API import.

Evidence

The ADR template defines a “## Binding status” section and table for tracking convergence, and the
decisions README explicitly calls out updating that binding-status table as bindings implement the
decision; the new ADR currently ends after “## Consequences” and contains no such section.

docs/decisions/0000-template.md[39-51]
docs/decisions/README.md[43-44]
docs/decisions/17701-bidi-generated-protocol-layer.md[91-101]

Agent prompt

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

### Issue description
The ADR ends after **Consequences** and does not include the repository’s standard **Binding status** section/table, which the ADR template and process docs use to track per-language implementation progress.

### Issue Context
This repository’s ADR template includes a dedicated section for tracking binding convergence, and the decisions README references updating that table as bindings implement the decision.

### Fix Focus Areas
- docs/decisions/17701-bidi-generated-protocol-layer.md[91-101]
- docs/decisions/0000-template.md[39-51]
- docs/decisions/README.md[43-44]

### Suggested change
Append a `## Binding status` section at the end of the ADR, including the standard table (Java/Python/Ruby/.NET/JavaScript) with initial statuses (e.g., `pending`) and optional tracking links/notes.

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

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 4b3f61d

[qodo-code-review]

Code review by qodo was updated up to the latest commit a49e5ab

[qodo-code-review]

Code review by qodo was updated up to the latest commit 67b42c9

[diemol]

I was not aware of the consequences of each binding doing their own thing. I believe It makes sense to unify this.

[diemol]

Where is this "shared model"? What form does it have? Who owns it?

[diemol]

What does a thin, stateless convenience mean? Should we be more explicit here so that PRs implementing this understand what belongs in the low-level definition and what goes into a different layer?