[adr] Behavioral contract for the low-level WebDriver BiDi layer by titusfortner · Pull Request #17786 · SeleniumHQ/selenium

titusfortner · 2026-07-15T21:13:54Z

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

🔗 Related

Replacement for [adr] BiDi low-level definitions are generated from a shared spec model without orchestration #17701 which framed generation as the requirement instead of the behaviors
Builds on internal-implementation premise from [adr] BiDi implementation boundaries #17670
The shared, binding-neutral schema referenced below was created in: [js] Add binding-neutral BiDi schema with cddl2ts-gated fidelity #17700
Python's generated adoption of this contract merged in [py] generate the internal BiDi protocol layer from the shared binding-neutral schema #17761 and is referred to here as "Python next"
since it isn't yet being used (the code currently in use is referred to as "Python now"). Java's adoption
of this contract is in progress in [java][bidi] Add BiDi code generator #17777.

📝 Proposal notes

Automatic generation. This ADR requires behaviors, not code generation. Generation matters because the
shared schema makes some behavior cheap to derive, but the contract is still the behavior every binding
exhibits, regardless of how it is implemented.
Scope. This record stops at the serialization layer: turning typed calls into wire messages and wire
messages back into typed objects. Transport behavior is out of scope and should be decided separately if
needed.
How to read the comparison table below. Each row is a point of compliance with a decision in this
ADR, showing where each implementation currently stands. The summaries after the table
differentiate the reasons for divergences (marked for replacement, minimizing maintenance costs, or
intentional design choices).
Two ways to consider decisions from this ADR (with table rows noted for reference as applicable)
1. Per-type precision support. Generating from the CDDL dramatically reduces
  most of the cost versus handwriting: closed-type rejection (6a), null and type checks (7a/7b), typed value
  objects (1). The exception is per-type extensibility in static languages: carrying and retaining undeclared
  fields on extensible types (6b/9b) stays costly even generated, because preserving and re-serializing them
  requires a separate deserialization path (though the webdriverbidi-net project shows it is possible even
  when handwritten, despite the cost). Two valid alternatives to the ADR positions are:
  - Drop per-type behavior entirely for a simpler, lossy common standard (better supports handwritten
    implementations).
  - Drop only per-type extensibility (better supports static languages).
2. Browser/schema drift support. The organizing rule is representability:
  tolerate what can be represented without inventing data; reject what would produce a false typed
  object (a corrupt value or an unknown closed-union variant, 7a/7b/7c). Of the two ways a browser drifts
  from the binding's schema, the ADR tolerates one and rejects the other: extra fields a newer browser adds
  are tolerated (9a/9b, forward compatibility), while a required field an older browser omits is rejected
  (8a) rather than fabricated or tracked as absent, since no static binding can hold an omitted required
  field consistently. A project schema override can relax a specific field when a remote end lags, degrading
  to a missing field rather than a failed session. The remaining way to tighten drift handling is:
  - Reject undeclared extras instead of accepting them (9a/9b), at the cost of forward compatibility
    (the layer breaks against a newer browser).

📊 Implementation comparison

How each implementation (current, proposed, and external) stands on selected comparison points against the ADR

The record states ten decisions across three sections (Representation, Outbound, Inbound). The table groups
its comparison points the same way: a row's number is the decision it tests, with letters marking multiple
points under one decision.

✅ compliant / ❌ diverges; known bugs are not counted as intentional divergence
Cost — maintenance burden: 🔴 high / 🟡 med / 🟢 low; → compares handwritten cost with generated-from-schema cost ([js] Add binding-neutral BiDi schema with cddl2ts-gated fidelity #17700)
Value — ●●● high / ●●○ med / ●○○ low.

Decision	Cost: Man→Gen	Value	wdio	bidi-net	.NET Trunk	Ruby Trunk	Python now	Python next	Java Trunk	Java PR
Generated			yes	no	no	yes	yes	yes	no	yes
1 · typed value objects	🟡① → 🟢	●●●	❌	✅	✅	✅	❌	✅	❌	✅
2 · surface names mirror spec	🟢	●●●	✅	✅	✅	✅	✅	✅	✅	✅
3 · preserve numeric range and precision	🟢	●●○	✅	✅	❌③	✅	✅	✅	❌③	✅
5a · reject an undefined enum value	🟢	●●○	✅	✅	❌	✅	❌	✅	✅	✅
5b · nullable constant: literal or null only	🟢	●○○	✅	✅	❌	✅	✅	✅	✅	✅
5c · required-field presence	🟢	●●●	✅	✅	✅	✅	✅	✅	❌	✅
6a · closed type rejects extras	🔴 → 🟢	●●○	✅	✅	❌	✅	✅	✅	❌	✅
6b · extensible types carry vendor extras to the wire	🔴② → 🟡	●●●	✅	✅	❌	✅	✅	✅	✅	❌
7a · reject a null in a non-nullable field	🔴 → 🟢	●●●	❌	✅	❌	✅	❌	✅	❌	✅
7b · reject a malformed value	🔴① → 🟢	●●●	❌	✅	✅	✅	❌	✅	❌	❌④
7c · reject an unresolvable closed-union variant	🔴 → 🟢	●●●	❌	✅	✅	✅	❌⑤	✅	✅	✅
8a · reject a missing required field	🟢	●●●	❌	✅	❌⑥	✅	❌⑥	✅	✅	✅
9a · accept an undeclared field	🟢	●●○	✅	✅	✅	✅	✅	✅	✅	✅
9b · retain an inbound extra on an extensible type	🔴② → 🟡	●●●	✅	✅	❌	✅	❌	✅	❌	❌
9c · non-extensible types drop extras	🔴 → 🟢	●○○	❌	✅⑦	❌	✅	✅⑦	✅	✅⑦	✅⑦
10 · preserve received values faithfully	🟢	●●○	✅	✅	✅	✅	✅	✅	✅	✅

Per-implementation through-lines

wdio — Generated objects, but no generated serialization layer: outbound is validated at compile
time, while inbound is whatever falls out of casting the payload onto the object, with no runtime checks,
so 7a/7b/7c and 8a are all ❌ (nothing rejects a malformed or missing value).
bidi-net — Handwritten and strict by default, it deliberately pays the cost for per-type behavior:
closed types reject extras, extensible types carry vendor extras, and both malformed and missing inbound
values error. It is now fully aligned with the inbound decisions; the manual, temporary relaxations it
ships when a browser lags a required field are exactly the project schema overrides this ADR sanctions.
.NET trunk — Handwritten with broad shared mechanisms: its only extension point is a single
bag on the top-level payload (command params, result, event args), not on the nested types, so a per-type
extra has nowhere to attach, and missing fields get defaults instead of being tracked as absent. Those
choices avoid expensive per-type work, but they are lossy, which is why it diverges on things
that are costly by hand and cheap to generate.
Ruby trunk — The prototype, kept current with this draft (missing-required rejection landed in [rb] always reject a missing required inbound BiDi field #17936,
the inbound scalar-arm check in [rb] reject an inbound BiDi scalar outside its union's declared arms #17947).
Python now — The generated layer currently shipping, not built from the shared schema; it predates
this contract and diverges on most inbound checks, and is superseded by Python next.
Python next — Generated from the shared schema ([py] generate the internal BiDi protocol layer from the shared binding-neutral schema #17761) and now aligned with the record on every
point; [py] update new BiDi layer generation to conform to latest proposed ADR #17942 replaced the tolerated-absence path with rejecting a missing required field.
Java trunk — Handwritten, being updated by the referenced PR.
Java PR — Generated adoption, in progress, with several known gaps: inbound values aren't held to
their declared type (7b — a string or fractional number, or an off-case enum, is coerced instead of
rejected), per-type extras aren't carried or retained (6b/9b), nested types don't warn on an undeclared
field (9c), and scalar-arm unions aren't generated from the schema's scalarValues signal (input.Origin).

Notes

① Cheaper for static implementations: the type system gives it with little extra code, even
un-generated.

② Cheaper for dynamic implementations: a plain runtime value does the job, with no static wrapper or
typed map.

③ Narrows a small, bounded js-uint field (a status code, a line or column number) to a 32-bit int; the
genuinely wide fields (byte sizes, counts) stay long, so the miss is limited to these small slots.

④ Selenium's shared NumberCoercer is too lenient to enforce this (it truncates fractional integers and
accepts float- or string-encoded numbers); the BiDi layer likely needs a stricter, BiDi-scoped coercer.

⑤ Performs no runtime union dispatch, so an undeclared closed-union variant is not rejected.

⑥ Fills a missing required field with a default (0/null/false) a caller can't distinguish from a real
value, so downstream code acts on fabricated data with no signal the field never arrived.

⑦ Compliant, but does not log the warning the ADR requires here.

🗣 Discussion

Discussed at the TLC meetings below; see the minutes for the full discussion and attribution.

2026-08-06 — compared the ADR point by point with the webdriverbidi-net implementation; the two agreed on desired behavior except on tolerating a missing required inbound field, and pujagani took an action to check whether it was feasible in Java.
2026-08-13 — pujagani reported that allowing a missing required field would be challenging to implement properly in Java, so the point was dropped, resolving the divergence and leaving titusfortner, pujagani, and jimevans in agreement.
2026-09-03 — clarified that validation is done against a generated schema (published specs plus supported vendor extensions), where issues like a missing required field can be managed with temporary overrides during generation; AutomatedTester questioned the growing number of CDDL files and checking the schema into the repo, and was satisfied by the versioning rationale.

📌 Tracking

Tracking issue: (linked on acceptance)

qodo-code-review · 2026-07-15T21:14:56Z

PR Summary by Qodo

Add ADR defining behavioral contract for low-level WebDriver BiDi layer

📝 Documentation 🕐 10-20 Minutes

AI Description

• Add an ADR that standardizes wire-observable behaviors for the BiDi low-level layer.
• Distinguish spec-mandated compliance rules from binding-owned design decisions.
• Document rationale, rejected alternatives, and consequences to align all bindings.

Diagram

graph TD
  E["High-level API"] --> B["Low-level BiDi layer"]
  A["BiDi Spec (CDDL)"] --> B["Low-level BiDi layer"] --> C["Transport"] --> D["Remote end"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Mandate code generation as the contract mechanism

➕ Can reduce per-binding implementation divergence by centralizing production
➕ Potentially lowers ongoing maintenance for typed models
➖ Does not guarantee runtime behaviors (validation/strictness/immutability) without additional rules
➖ Imposes tooling and workflow constraints that may not fit every binding

2. Define a conformance test suite as the primary artifact

➕ Makes the contract executable and objectively verifiable across bindings
➕ Encodes edge cases precisely (e.g., nullability, union resolution, strictness relaxations)
➖ Harder to review and discuss design intent without a written rationale
➖ Still needs a normative written source for why tests exist and what’s intentional vs. incidental

3. Publish the contract as binding-specific guidelines instead of one ADR

➕ Lets each binding tailor wording and examples to its language/runtime
➕ May reduce friction for teams with different idioms
➖ Reintroduces drift risk and repeated re-litigation of decisions across bindings
➖ Makes cross-binding review and enforcement significantly harder

Recommendation: Keep the ADR as the single binding-neutral normative source of behavioral requirements (as this PR does). Consider following up with a shared conformance suite that directly exercises the ADR’s decision points (strict inbound validation, extensibility handling, extras preservation scope) to make adherence measurable without making generation or tooling a requirement.

Files changed (1) +188 / -0

Documentation (1) +188 / -0

nnnnn-bidi-low-level-behavioral-contract.mdAdd ADR specifying low-level BiDi behavioral contract +188/-0

Add ADR specifying low-level BiDi behavioral contract

• Introduces a proposed architecture decision record defining a binding-neutral behavioral contract for the low-level WebDriver BiDi layer. The document enumerates compliance vs. binding-owned decisions across outbound, inbound, extensibility, and surface design, and records considered alternatives and consequences.

docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md

qodo-code-review · 2026-07-15T21:15:27Z

Code Review by Qodo

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

1. ~~Placeholder ADR number~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new ADR is added with placeholder ID "NNNNN" in both the filename and document heading and uses
"Discussion: _PR pending_" instead of a PR link, which violates the documented ADR numbering/linking
rules and harms traceability for future citations.

Code

docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md[R1-5]
+# NNNNN. Behavioral contract for the low-level WebDriver BiDi layer
+
+- Status: Proposed
+- Discussion: _PR pending_
+

Evidence
The ADR process documentation in this repo requires decision records to be numbered by the proposing
PR number and renamed before merge, and the template indicates the number should be the PR number
and the discussion field should link to the PR. The newly added ADR still contains placeholders
instead.
docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md[1-5]
docs/decisions/README.md[3-5]
docs/decisions/README.md[42-45]
docs/decisions/README.md[90-91]
docs/decisions/0000-template.md[1-8]

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 record is still using placeholders (`nnnnn-...` filename, `# NNNNN.` header, and `Discussion: _PR pending_`). The `docs/decisions` README defines ADR numbering as the proposing PR number and requires renaming before merge, and the template expects the discussion field to link to the PR.

## Issue Context
If merged with placeholders, this record will be harder to cite consistently and will not follow the project’s documented decision-record process.

## Fix Focus Areas
- docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md[1-5]
- docs/decisions/README.md[42-45]

## Proposed fix
1. Rename `docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md` to `<PR_NUMBER>-bidi-low-level-behavioral-contract.md`.
2. Update the first heading to `# <PR_NUMBER>. Behavioral contract for the low-level WebDriver BiDi layer`.
3. Replace `Discussion: _PR pending_` with a link to this PR (and optionally additional relevant threads under Context, per the template).

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

2. ~~Ambiguous const/null example~~ ✓ Resolved 🐞 Bug ⚙ Maintainability ⭐ New

Description

In item 3, the parenthetical example “a true/null field rejects false” is mildly ambiguous
shorthand and can read like a generic boolean-validation example rather than explicitly “allowed set
is {literal, null}” for nullable constants. Tightening this wording would make the intended rule (as
defined in item 2) unambiguous for adopters implementing outbound validation.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R63-64]
+   as does a value neither the literal nor `null` for a nullable constant (a `true`/`null` field rejects
+   `false`; see item 2). A static

Evidence
Item 3’s example uses the true/null shorthand while item 2 is where the contract precisely
defines nullable-constant behavior (“literal or null”). Making item 3’s example explicitly reflect
that membership rule removes the ambiguity.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[59-65]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[50-57]

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 example in item 3 uses shorthand (“`true`/`null` field rejects `false`”) that can be read as a general boolean-type validation example, instead of explicitly conveying the nullable-constant membership rule.

### Issue Context
Item 2 already defines the rule for nullable constants (“send the literal or `null`”), and item 3 is illustrating that outbound validation should reject anything outside that allowed set.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[59-64]

### Suggested edit
Replace the parenthetical with wording that explicitly states the allowed set, e.g.
- "… (e.g., for a nullable constant whose literal is `true`, the only valid values are `true` and `null`; `false` must be rejected; see item 2)."

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

3. ~~Mis-cited extras rules~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The ADR cites item 7 when describing closed types “accepts-but-ignores” unknown inbound properties
and when saying other types “tolerates and drops”, but item 7 only requires that unknown inbound
properties must not error while the preserve/drop rules are defined by item 8. This can mislead
adopters into treating “drop/ignore” as part of Compliance (item 7) rather than a Decision scoped by
item 8, increasing the risk of divergent extras handling across bindings.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R74-75]

+   while a closed type rejects unknown properties outbound (item 3) and accepts-but-ignores them inbound
+   (item 7). The one prohibition is never injecting arbitrary properties into a *closed* type, which would

Evidence
Item 7 defines only that unknown inbound properties must not cause an error, while item 8 defines
when extras are preserved vs dropped; the changed lines cite item 7 in contexts that describe
ignore/drop behavior.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[74-75]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[96-98]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[100-103]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[113-113]

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 cross-references **item 7** (“Unknown inbound properties never cause an error”) in places where the text is actually describing **whether unknown properties are ignored/dropped vs preserved**, which is governed by **item 8** (“Preserving extras is scoped…”).

### Issue Context
- Item 7 is the *tolerance/no-error* rule.
- Item 8 is the *preserve vs drop* scoping rule.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[69-78]
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[109-114]

### Suggested edit direction
- In item 4, change the parenthetical so it distinguishes tolerance (item 7) from preservation/drop behavior (item 8), e.g. “...tolerates unknown inbound properties (item 7) and drops them unless scoped for preservation (item 8).”
- In item 8, change “tolerates and drops (item 7)” to reference item 8 (or “item 7 for tolerance; item 8 for drop/preserve scope”).

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

4. ~~Ambiguous section reference~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The Decision intro says “the next section marks which is which,” but the Compliance/Decision labels
are applied inline on each numbered behavior and the next section primarily explains the tagging
scheme. This is a minor ambiguity that can be fixed by rephrasing the sentence to refer to the items
below being tagged (or the next section explaining the tags).

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R18-20]

+**Any implementation of this layer must exhibit the behaviors below.** Some follow from conforming to the
+WebDriver BiDi specification; the rest are choices this record standardizes so bindings don't diverge —
+the next section marks which is which.

Evidence
Lines 18–20 claim the “next section marks which is which,” while the subsequent “How to read”
section defines the tiers and the actual tier markers appear inline on the numbered behaviors (e.g.,
item 1 includes “*(Compliance.)*”).
docs/decisions/17786-bidi-low-level-behavioral-contract.md[18-20]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[28-36]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[46-48]

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 intro refers to “the next section” as doing the marking, but the Compliance/Decision markers are attached inline to each numbered item; the following section mainly explains what those tags mean.

### Issue Context
This is a minor clarity/readability fix to align the intro text with the structure readers see.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[18-20]

ⓘ 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 turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 6f34324

Results up to commit 613cd02

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

1. ~~Placeholder ADR number~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new ADR is added with placeholder ID "NNNNN" in both the filename and document heading and uses
"Discussion: _PR pending_" instead of a PR link, which violates the documented ADR numbering/linking
rules and harms traceability for future citations.

Code

docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md[R1-5]
+# NNNNN. Behavioral contract for the low-level WebDriver BiDi layer
+
+- Status: Proposed
+- Discussion: _PR pending_
+

Evidence
The ADR process documentation in this repo requires decision records to be numbered by the proposing
PR number and renamed before merge, and the template indicates the number should be the PR number
and the discussion field should link to the PR. The newly added ADR still contains placeholders
instead.
docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md[1-5]
docs/decisions/README.md[3-5]
docs/decisions/README.md[42-45]
docs/decisions/README.md[90-91]
docs/decisions/0000-template.md[1-8]

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 record is still using placeholders (`nnnnn-...` filename, `# NNNNN.` header, and `Discussion: _PR pending_`). The `docs/decisions` README defines ADR numbering as the proposing PR number and requires renaming before merge, and the template expects the discussion field to link to the PR.

## Issue Context
If merged with placeholders, this record will be harder to cite consistently and will not follow the project’s documented decision-record process.

## Fix Focus Areas
- docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md[1-5]
- docs/decisions/README.md[42-45]

## Proposed fix
1. Rename `docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md` to `<PR_NUMBER>-bidi-low-level-behavioral-contract.md`.
2. Update the first heading to `# <PR_NUMBER>. Behavioral contract for the low-level WebDriver BiDi layer`.
3. Replace `Discussion: _PR pending_` with a link to this PR (and optionally additional relevant threads under Context, per the template).

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

Results up to commit dfe1c07

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

1. ~~Ambiguous section reference~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The Decision intro says “the next section marks which is which,” but the Compliance/Decision labels
are applied inline on each numbered behavior and the next section primarily explains the tagging
scheme. This is a minor ambiguity that can be fixed by rephrasing the sentence to refer to the items
below being tagged (or the next section explaining the tags).

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R18-20]

+**Any implementation of this layer must exhibit the behaviors below.** Some follow from conforming to the
+WebDriver BiDi specification; the rest are choices this record standardizes so bindings don't diverge —
+the next section marks which is which.

Evidence
Lines 18–20 claim the “next section marks which is which,” while the subsequent “How to read”
section defines the tiers and the actual tier markers appear inline on the numbered behaviors (e.g.,
item 1 includes “*(Compliance.)*”).
docs/decisions/17786-bidi-low-level-behavioral-contract.md[18-20]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[28-36]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[46-48]

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 intro refers to “the next section” as doing the marking, but the Compliance/Decision markers are attached inline to each numbered item; the following section mainly explains what those tags mean.

### Issue Context
This is a minor clarity/readability fix to align the intro text with the structure readers see.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[18-20]

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

Results up to commit 5b77f31

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

1. ~~Mis-cited extras rules~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The ADR cites item 7 when describing closed types “accepts-but-ignores” unknown inbound properties
and when saying other types “tolerates and drops”, but item 7 only requires that unknown inbound
properties must not error while the preserve/drop rules are defined by item 8. This can mislead
adopters into treating “drop/ignore” as part of Compliance (item 7) rather than a Decision scoped by
item 8, increasing the risk of divergent extras handling across bindings.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R74-75]

+   while a closed type rejects unknown properties outbound (item 3) and accepts-but-ignores them inbound
+   (item 7). The one prohibition is never injecting arbitrary properties into a *closed* type, which would

Evidence
Item 7 defines only that unknown inbound properties must not cause an error, while item 8 defines
when extras are preserved vs dropped; the changed lines cite item 7 in contexts that describe
ignore/drop behavior.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[74-75]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[96-98]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[100-103]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[113-113]

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 cross-references **item 7** (“Unknown inbound properties never cause an error”) in places where the text is actually describing **whether unknown properties are ignored/dropped vs preserved**, which is governed by **item 8** (“Preserving extras is scoped…”).

### Issue Context
- Item 7 is the *tolerance/no-error* rule.
- Item 8 is the *preserve vs drop* scoping rule.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[69-78]
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[109-114]

### Suggested edit direction
- In item 4, change the parenthetical so it distinguishes tolerance (item 7) from preservation/drop behavior (item 8), e.g. “...tolerates unknown inbound properties (item 7) and drops them unless scoped for preservation (item 8).”
- In item 8, change “tolerates and drops (item 7)” to reference item 8 (or “item 7 for tolerance; item 8 for drop/preserve scope”).

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

qodo-code-review · 2026-07-15T21:22:55Z

Code review by qodo was updated up to the latest commit 98e1858

Copilot

Pull request overview

Adds a new ADR to the cross-binding decision log that defines a behavioral contract for Selenium’s internal, low-level WebDriver BiDi protocol layer (validation, extensibility, parsing strictness, and surface shape), intended to keep binding implementations aligned on wire-observable behavior.

Changes:

Introduces a decision record enumerating required outbound/inbound behaviors for the BiDi low-level layer.
Defines a two-tier taxonomy (Compliance vs Decision) to separate spec-forced requirements from Selenium-standardized choices.
Records rationale, alternatives, and consequences to support consistent adoption across bindings.

qodo-code-review · 2026-07-15T22:22:30Z

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

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

qodo-code-review · 2026-07-15T23:05:45Z

Code review by qodo was updated up to the latest commit 5b77f31

qodo-code-review · 2026-07-15T23:49:02Z

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

qodo-code-review · 2026-07-16T00:37:27Z

Code review by qodo was updated up to the latest commit 1b0e91a

qodo-code-review · 2026-07-16T13:47:34Z

Code Review by Qodo

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

1. ~~Undeclared-field warning underspecified~~ ✗ Dismissed 🐞 Bug ⛨ Security ⭐ New

Description

Decision 9 requires implementations to "log a warning" when dropping undeclared fields on
non-extensible types, but it does not specify what must/must-not be logged, which invites
cross-binding divergence and can lead to sensitive data being written to logs. Because undeclared
fields are arbitrary by definition, a naive "log the field" implementation can leak payload values.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R81-83]

+9. **Tolerate an undeclared field.** If the type is declared extensible, the layer must preserve the
+   field in the type's map (decision 1). If it is not, the layer must log a warning that an undeclared field
+   was received, and drop it.

Evidence
The ADR explicitly mandates warning logs for undeclared fields on non-extensible types, but does not
constrain content; combined with the ADR’s allowance that undeclared fields exist and are arbitrary,
this creates a realistic risk of inconsistent implementations and potential sensitive-data logging.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[81-83]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[39-41]

Agent prompt

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

## Issue description
Decision 9 mandates logging a warning for undeclared inbound fields on non-extensible types, but it does not constrain the warning’s content (e.g., whether values are included) or define a minimum observable/consistent behavior across bindings. Since undeclared fields can contain arbitrary data, logging values is a realistic security footgun.

## Issue Context
The ADR is meant to define wire-observable behavior that different language bindings can implement consistently. “Log a warning” is not a sufficiently precise behavioral contract unless the required semantics are stated (what information is included, redaction rules, and/or an allowed structured-diagnostics alternative).

## Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[39-41]
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[81-83]

## What to change
Update Decision 9 to explicitly define the minimum warning semantics, for example:
- The warning MUST include: the type name and the unexpected field name(s).
- The warning MUST NOT include: field values (or MUST redact/truncate values) to avoid leaking sensitive data.
- Optionally: allow a structured diagnostics hook/event as an equivalent to logging, as long as it’s caller-observable and testable.

Keep the drop behavior the same; just make the mandated warning safe and deterministic across bindings.

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

2. ~~Extras retention rule undefined~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

Decision 3 retains unknown properties only when “the received type or its command-parameter
counterpart is extensible”, but it doesn’t define what a “command-parameter counterpart” is or how
to determine this mapping for results and events. This makes the retain-vs-drop behavior
non-deterministic across bindings even though it’s presented as part of the contract.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R57-61]

+   - **An undeclared property warns, and is tolerated** (forward-compatibility). Where the type is
+     round-trippable — the received type or its command-parameter counterpart is extensible — the unknown
+     property is kept on the object, so a caller can reproduce it on the wire: a vendor attribute received on
+     a `network.Cookie` can be set again through `storage.setCookie` unchanged. Other undeclared properties
+     are tolerated but not retained.

Evidence
The retention condition references “command-parameter counterpart” as a normative criterion but
provides no definition or general mapping rule.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[57-61]

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’s rule for when to retain unknown inbound properties depends on an undefined concept (“command-parameter counterpart”), which will lead to inconsistent implementations.

### Issue Context
This affects forward-compatibility behavior and round-tripping of vendor/future fields.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[57-61]

### Suggested change
Define the rule in a schema-addressable way, for example:
- explicitly define what “counterpart” means (e.g., a type pair linked by an explicit schema annotation or a named relationship), and how it applies to command results vs events.
- or simplify the retention condition to depend only on properties of the *received* type (e.g., “retain extras only on types marked extensible AND designated round-trippable by the shared schema”).

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

3. ~~Ambiguous “verbatim” strings~~ ✗ Dismissed 🐞 Bug ⚙ Maintainability

Description

The phrase “spec strings reach the wire verbatim” is undefined and can be read as requiring
byte/escape-form preservation, while item 10 explicitly defines fidelity as value-level (“not its
byte-form”). This ambiguity can lead to inconsistent binding behavior and conformance tests around
string serialization/normalization.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R24-25]

+baseline: spec strings reach the wire verbatim, an *omitted* field stays distinct from an explicit `null`,
+and a union resolves by the rule the spec declares. What it leaves open,

Evidence
The baseline sentence introduces “verbatim” language for strings, but later the ADR explicitly says
fidelity is value-level and not byte-form, making the earlier wording easy to misinterpret as
requiring lexical/escaping preservation.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[22-25]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[83-87]

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 states “spec strings reach the wire verbatim,” which is ambiguous (byte/lexeme vs semantic string value) and appears to conflict with the later rule that fidelity is of the value, not its byte-form.

### Issue Context
This document is defining a cross-binding behavioral contract; ambiguous requirements can result in different bindings (or generators) implementing incompatible behaviors.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[22-25]

### Suggested direction
Reword “spec strings reach the wire verbatim” to explicitly state what must be preserved (e.g., spec-defined tokens like enum/discriminator values and wire keys must not be recased/normalized) and avoid implying byte/escape-form preservation unless that is truly intended.

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

4. ~~Omission/null mapping ambiguous~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The Consequences section says missing required inbound fields can be represented by setting the
field to null (“null marks omitted”), but the contract also requires rejecting an explicitly
present null for non-nullable fields. Without an explicit requirement to check wire-level field
presence before mapping omission to null, implementations can incorrectly accept explicit null
as “omitted.”

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R154-156]

+  field non-null yet leave it *omitted* when missing. A nullable slot suffices for most fields (real data is
+  never `null` there, so `null` marks *omitted*); a required *nullable* field (network `context`/`navigation`,
+  response sizes, log `text`, ~30 in all) instead needs *omitted* kept distinct from `null`. The trigger is

Evidence
Decision 4 defines null in a non-nullable field as structurally invalid, and Decision 7 requires
rejecting present invalid values; the Consequences text then recommends using null to mark
omission, which is ambiguous unless presence is checked before the mapping.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[49-52]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[74-78]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[153-156]

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’s Consequences suggests using `null` as an internal marker for an *omitted* (missing) required inbound field. However, the ADR also defines `null` in a non-nullable field as structurally invalid and requires rejecting present-but-invalid values. Without a clear “check presence first” rule, a binding can collapse both cases to `null` and accidentally treat an explicit inbound `null` as omission.

### Issue Context
This is about inbound decoding/validation semantics: bindings must be able to reject explicit `null` values for non-nullable fields (invalid) while still tolerating missing required fields (omitted).

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[153-156]

### Suggested change
Adjust the Consequences bullet to explicitly require presence-aware validation at the wire/deserialization boundary, e.g.:
- Explicit `null` for a non-nullable field MUST be rejected (decision 7), and
- Only after confirming the field is absent may a binding map omission to an internal `null`/`None`/`Optional.empty` representation for convenience.

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

5. ~~Integer token handling unclear~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Decision 1 requires integer to reject whole-valued 5.0 as “float-encoded”, which depends on the
JSON number’s lexical form (not just its numeric value) and is not enforceable once a typical JSON
parser has normalized the token. Without explicitly requiring lexeme-preserving parsing (or
weakening the rule), bindings will either be unable to conform or will diverge in what they
accept/reject.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R31-33]

+     declared. A primitive matches by JSON kind, not language representation: `number` admits any JSON
+     number, while `integer` rejects a fractional or float-encoded one (a whole-valued `5.0` included);
+     neither direction coerces across the boundary silently.

Evidence

The ADR explicitly calls out rejecting “float-encoded” integers including 5.0, which implies
lexical inspection rather than only checking integral numeric value.

docs/decisions/17786-bidi-low-level-behavioral-contract.md[31-33]

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 requires rejecting an `integer` value that was encoded as `5.0`, which implies inspecting the JSON numeric lexeme. Many JSON stacks do not preserve that distinction after parsing, so the behavior is underspecified and likely to diverge across bindings.

### Issue Context
This is in the normative validation rule for primitives.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[31-33]

### Suggested change
Amend the text to either:
1) explicitly require lexeme-preserving number parsing (or an equivalent exact-number/token API) for inbound validation when enforcing `5` vs `5.0`, **or**
2) redefine the rule in terms of numeric value only (e.g., “reject values with a fractional component”), removing the lexical-form requirement.

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

View medium (4)

6. ~~Error surfacing boundary unclear~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

Decision 3 says remote errors must surface even if their payload fails validation, but it doesn’t
specify the minimum classification boundary for “this is an error response” versus “this is a
malformed/unclassifiable message”. Without that clarification, implementations can still
legitimately throw local serialization errors for malformed error envelopes, contradicting the
stated requirement to not let validation override remote errors.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R47-51]

+3. **Inbound, those same two are tolerated.** Process error responses first: a remote error surfaces as an
+   error even if its payload fails validation — an unrecognized error code included — and the checks here
+   must not turn it into a local serialization failure; the error's contents are otherwise out of scope.
+   Otherwise these rules govern every typed payload the layer parses — a command's result and a
+   server-initiated event's parameters alike, with response correlation and event routing out of scope:

Evidence
The ADR requires error responses to bypass normal validation failures, but does not define the
classification boundary needed to apply that rule consistently.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[47-51]

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 promises that remote errors are surfaced even when the error payload fails validation, but it doesn’t define what must be successfully parsed to classify a message as an error response, nor what happens when those identifying fields are missing/invalid.

### Issue Context
Transport behavior is said to be out of scope, but this section still imposes a behavioral requirement on error handling that depends on a clear classification boundary.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[47-51]

### Suggested change
Add a short normative clarification such as:
- which minimal envelope fields must be read to classify an inbound message as an error response (before full validation), and
- if those are missing/invalid, whether the message is treated as a transport/protocol error (out of scope here) versus a local serialization error.
This keeps the “errors must surface” promise implementable and consistent across bindings.

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

7. ~~Missing selector behavior undefined~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

Decision 2 only defines missing-field handling after union resolution, but a missing discriminator
or insufficient structural selector fields can prevent a variant from resolving at all. The contract
therefore does not specify whether bindings should error, warn, or return another representation for
these inbound payloads.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R45-49]

+   Otherwise, once the spec's union rule resolves the payload's type (if applicable), each mismatch is
+   handled by its kind:
+   1. **Error if corrupted** — the value cannot fit the resolved type: a null in a non-nullable field, a
+      wrong primitive type, a cardinality mismatch, a non-object where an object is expected.
+   2. **Warn if a required field is missing** — the field must be left *omitted* (not an explicit *null*,

Evidence
The ADR handles missing required fields only after the union's type has resolved, while also
requiring each union variant to be represented by a distinct type. The shared schema's union
selectors resolve discriminated unions from payload[by] and structural unions from required-key
presence, so absent or insufficient selector fields can make that prerequisite fail without any
contract-defined outcome.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[41-70]
javascript/selenium-webdriver/project_bidi_schema.mjs[275-317]
javascript/selenium-webdriver/project_bidi_schema_test.mjs[206-239]

Agent prompt

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

## Issue description
Define the required behavior when an inbound union cannot be resolved because its discriminator is absent or its structural selector is ambiguous. This case currently occurs before the documented missing-field policy can apply.

## Issue Context
Union selectors may depend on a discriminator value or the presence of required fields. The ADR should explicitly choose a uniform resolution-failure policy, such as rejecting unresolved closed unions, rather than leaving each binding to decide.

## Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[41-65]

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

8. Relaxation reporting undefined 🐞 Bug ◔ Observability

Description

Item 7 requires tolerated absences to be “still reported” but does not define the minimum
caller-observable semantics of that reporting, so bindings can diverge (e.g., logging only vs
structured diagnostics) while still claiming conformance.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R112-114]

+   - **The tolerated absence is still reported**, never silently absorbed. A binding *may* additionally
+     let a caller admit an uncatalogued absence at runtime, so a user is not blocked until the next
+     release; that is a per-binding convenience, not required here, and it too reports.

Evidence

The ADR imposes a normative requirement (“still reported”) but provides no definition of how
reporting must be exposed, leaving implementation latitude that undermines uniformity.

docs/decisions/17786-bidi-low-level-behavioral-contract.md[112-114]

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 mandates that tolerated absences are “still reported,” but leaves “reported” undefined. This makes conformance hard to test and encourages cross-binding divergence in what users/maintainers can observe.

## Issue Context
This ADR’s stated goal is uniform wire-observable behavior across bindings. Relaxation reporting affects how drift is detected, tracked, and eventually retired.

## Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[112-114]

## Suggested change
Add a minimum requirement for reporting that is binding-idiomatic but caller-observable, e.g.:
- Require an explicit diagnostic surface (exception subtype, warning callback/hook, or returned structured warning object) that includes at least: the schema/type, the missing field name, and the command/event context.
- Clarify whether logging alone is sufficient or explicitly state it is not.

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

9. ~~Undefined error-code surfacing~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

Item 9 requires unknown ErrorResponse enum tokens to be “surfaced, not thrown” but does not define
the minimum observable semantics (e.g., raw-token preservation and how callers access it), which can
lead to divergent binding behavior despite the ADR’s stated goal of uniform wire-observable
behavior.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R127-130]

+   applied to every binding at once — never one alone, and never by starting lenient. One exception is
+   sanctioned from the start: an enum that reports an error (an unrecognized error code in an
+   `ErrorResponse`), where raising would swallow the error being reported; there, an unknown token is
+   surfaced, not thrown.

Evidence
The ADR states its purpose is to standardize wire-observable behavior across bindings; leaving
“surfaced” undefined for unknown error codes reintroduces the same cross-binding drift risk the ADR
is meant to eliminate.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[13-15]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[18-20]
docs/decisions/17786-bidi-low-level-behavioral-contract.md[122-130]

Agent prompt

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

### Issue description
Item 9 says an unknown `ErrorResponse` error-code token must be “surfaced, not thrown”, but that leaves implementers room to diverge on what is required (whether the raw token is preserved verbatim, and what shape it takes when exposed to callers).

### Issue Context
This ADR is explicitly intended to prevent cross-binding drift on wire-observable behavior; underspecified requirements defeat that goal.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[122-130]

### Suggested change (one possible wording)
Amend item 9 to define the binding-neutral minimum:
- parsing must not throw on unknown `ErrorResponse` code tokens
- the exact raw wire token must be preserved verbatim
- callers must be able to retrieve that raw token (via an idiomatic “unknown/other” representation)

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

10. ~~Absence not a key~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

Item 6 says “every key in the payload” falls into three cases, but one case is “Absence” (a missing
required field), which is not a payload key; this makes the validation taxonomy internally
inconsistent and easier to misread in isolation.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R90-99]

+6. **Inbound payloads are validated against the resolved type.** *(Decision.)* Once item 5 has resolved
+   the type, every key in the payload falls into one of three cases, and a declared field is checked
+   rather than populated silently:
+   - **Corruption — always raises.** A null in a non-nullable field, a value of the wrong primitive
+     type, a list/scalar cardinality mismatch, a non-object where a field expects an object. The wire
+     asserted something untrue, and absorbing it is what misrepresents protocol state.
+   - **Absence — raises.** A required field is missing. Nothing untrue was asserted and every field that
+     did arrive is still correct, so absence is the one case a relaxation may reach (item 7).
+   - **Undeclared key — never raises.** Every type tolerates a property it does not define, open or
+     closed; most parsers do this by default, and a reject-unmapped setting would violate it. The spec

Evidence

The text asserts the three cases apply to “every key in the payload,” but immediately defines
“Absence” as a required field being missing (i.e., not a key), making the classification statement
inaccurate.

docs/decisions/17786-bidi-low-level-behavioral-contract.md[90-99]

Agent prompt

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

## Issue description
Item 6 frames validation as applying to “every key in the payload,” but then includes “Absence” (missing required field) as one of the cases. This is internally inconsistent and can confuse readers about what is being classified.

## Issue Context
This section is defining a cross-binding behavioral contract; the validation taxonomy should be precise and unambiguous.

## Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[90-99]

## Suggested change
Rewrite the lead-in to separate declared-field validation from payload-key handling, e.g.:
- “Once item 5 has resolved the type, validation covers (a) declared fields (including required-field presence) and (b) any undeclared keys present in the payload.”
Or change “every key” to “every field/key case” and explicitly define what ‘absence’ refers to.

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

11. ~~Broken Consequences sentence~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The final Consequences bullet ends with an ungrammatical fragment (“the items that signal feeds”),
which reduces clarity for future readers quoting or implementing the summary.

Code

docs/decisions/17786-bidi-low-level-behavioral-contract.md[R189-190]

+- The per-type signals items 5/6/8 need are all derivable from the spec; a binding that discards one, or
+  parses into a lenient runtime, falls out of conformance on exactly the items that signal feeds.

Evidence
The typo is present in the final bullet of the ADR’s Consequences section.
docs/decisions/17786-bidi-low-level-behavioral-contract.md[185-190]

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 last sentence in Consequences is grammatically incorrect (“the items that signal feeds”), making the summary unclear.

### Issue Context
This is the concluding summary statement and is likely to be quoted in reviews.

### Fix Focus Areas
- docs/decisions/17786-bidi-low-level-behavioral-contract.md[189-190]

### Suggested change
Rewrite to something like:
- “...falls out of conformance on exactly the items those signals feed.”
 or
- “...falls out of conformance on exactly items 5, 6, or 8.”

ⓘ 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 turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

qodo-code-review · 2026-07-24T17:15:08Z

Code review by qodo was updated up to the latest commit 541031b

qodo-code-review · 2026-07-29T02:48:01Z

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

qodo-code-review Bot reviewed Jul 15, 2026

View reviewed changes

Comment thread docs/decisions/nnnnn-bidi-low-level-behavioral-contract.md Outdated

titusfortner requested a review from Copilot July 15, 2026 21:33

titusfortner added the A-needs decision TLC needs to discuss and agree label Jul 15, 2026

Copilot started reviewing on behalf of titusfortner July 15, 2026 21:34

View session

titusfortner mentioned this pull request Jul 15, 2026

[java][bidi] Add BiDi code generator #17777

Merged

2 of 3 tasks

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

qodo-code-review Bot reviewed Jul 15, 2026

View reviewed changes

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

titusfortner requested a review from Copilot July 15, 2026 22:57

Copilot started reviewing on behalf of titusfortner July 15, 2026 22:57

View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

qodo-code-review Bot reviewed Jul 15, 2026

View reviewed changes

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

titusfortner mentioned this pull request Jul 15, 2026

[rb] add objectOnly/preserveExtras/scalar-primitive BiDi schema signals #17784

Merged

2 of 3 tasks

qodo-code-review Bot reviewed Jul 15, 2026

View reviewed changes

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

titusfortner mentioned this pull request Jul 16, 2026

[py] generate the internal BiDi protocol layer from the shared binding-neutral schema #17761

Merged

2 of 3 tasks

qodo-code-review Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

titusfortner mentioned this pull request Jul 16, 2026

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

Closed

nvborisenko reviewed Jul 23, 2026

View reviewed changes

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

qodo-code-review Bot reviewed Jul 24, 2026

View reviewed changes

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

titusfortner mentioned this pull request Jul 24, 2026

[rb] validate nullable-constant BiDi params outbound #17818

Merged

2 of 3 tasks

qodo-code-review Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread docs/decisions/17786-bidi-low-level-behavioral-contract.md Outdated

titusfortner mentioned this pull request Oct 16, 2026

[rb] type vendor-declared BiDi fields instead of making extended types extensible #18190

Open

2 of 3 tasks

Conversation

titusfortner commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Related

📝 Proposal notes

📊 Implementation comparison

🗣 Discussion

📌 Tracking

Uh oh!

qodo-code-review Bot commented Jul 15, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 15, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 15, 2026

Uh oh!

qodo-code-review Bot commented Jul 16, 2026

Uh oh!

qodo-code-review Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 24, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

titusfortner commented Jul 15, 2026 •

edited

Loading

qodo-code-review Bot commented Jul 15, 2026 •

edited

Loading

qodo-code-review Bot commented Jul 16, 2026 •

edited

Loading