Skip to content

Conversation

@titusfortner
Copy link
Member

@titusfortner titusfortner commented Oct 20, 2025

User description

We all know anyone can do whatever they want with Ruby objects, but the source code should indicate which things we should be trying to use.


PR Type

Documentation


Description

  • Mark low-level BiDi implementation classes as private API

  • Add @api private YARD documentation tags to internal modules

  • Clarify recommended public API alternatives in class documentation

  • Improve code documentation with module descriptions


Diagram Walkthrough

flowchart LR
  BiDi["BiDi Module Classes"]
  BiDi -->|"Add @api private tags"| Docs["YARD Documentation"]
  BiDi -->|"Add usage guidance"| Comments["Class Comments"]
  Comments -->|"Reference public APIs"| Public["driver.manager, driver.navigate, driver.script"]
Loading

File Walkthrough

Relevant files
Documentation
13 files
browser.rb
Add private API documentation to Browser class                     
+7/-0     
browsing_context.rb
Add private API tag and usage guidance to BrowsingContext
+2/-1     
log_handler.rb
Add private API documentation to LogHandler class               
+5/-0     
network.rb
Add private API documentation to Network class                     
+6/-0     
cookies.rb
Add private API tag to Cookies class                                         
+4/-0     
credentials.rb
Add private API tag to Credentials class                                 
+4/-0     
headers.rb
Add private API tag to Headers class                                         
+4/-0     
intercepted_auth.rb
Add private API tag to InterceptedAuth class                         
+4/-0     
intercepted_item.rb
Add private API tag to InterceptedItem class                         
+4/-0     
intercepted_request.rb
Add private API tag to InterceptedRequest class                   
+4/-0     
intercepted_response.rb
Add private API tag to InterceptedResponse class                 
+4/-0     
url_pattern.rb
Add private API tag to UrlPattern module                                 
+4/-0     
session.rb
Add private API documentation to Session class                     
+4/-0     

@titusfortner titusfortner requested a review from aguspe October 20, 2025 23:17
@selenium-ci selenium-ci added C-rb Ruby Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Oct 20, 2025
@qodo-merge-pro
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unsafe script usage guidance

Description: The comment suggests accessing logging via driver.script, which may encourage executing
arbitrary scripts for logging and could lead to security risks if user input is passed to
script execution; verify documentation does not promote unsafe script injection patterns.
log_handler.rb [23-27]

Referred Code
# Implements the Log of the WebDriver-BiDi specification
# This functionality should be accessed through `driver.script` method
#
# @api private
#
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix incorrect hash mutation during iteration

In as_json, avoid mutating the Cookies hash during iteration by creating a new
hash for each cookie within the map block.

rb/lib/selenium/webdriver/bidi/network/cookies.rb [28-35]

 def as_json
   map do |name, val|
-    self[:name] = name.to_s
-    self[:value] = {type: 'string', value: val.to_s}
-
-    [compact]
+    {
+      name: name.to_s,
+      value: {type: 'string', value: val.to_s}
+    }
   end
 end
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical bug where the as_json method mutates the hash it is iterating over, leading to incorrect behavior and data corruption. The proposed fix is accurate and resolves the issue.

High
Learned
best practice
Fix inaccurate module comment

Update the comment to accurately reflect that this class implements the Network
module, not Navigation, to avoid misleading documentation.

rb/lib/selenium/webdriver/bidi/network.rb [24-25]

-# Implements the Navigation Module of the WebDriver-BiDi specification
-# Continue to use functionality from existing `driver.navigate` method
+# Implements the Network Module of the WebDriver-BiDi specification
+# Continue to use functionality from existing `driver.network`-related methods
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Keep API and documentation accurate and consistent by aligning comments/annotations with actual functionality.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-devtools Includes everything BiDi or Chrome DevTools related C-rb Ruby Bindings Review effort 1/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants