[py]: add patching rules for CDP code generator

[nxs7]

User description

💥 What does this PR do?

As far as I'm concerned, there are five $refs in the CDP Documentation that incorrectly refer to their own domain. Specifically, one pertains to a method, and four pertain to events:

• Method
  • DOM.resolveNode
    • backendNodeId
• Events
  • Debugger.scriptFailedToParse
    • scriptLanguage
  • Debugger.scriptParsed
    • scriptLanguage
    • debugSymbols
  • DOM.scrollableFlagUpdated
    • nodeId

Currently, the generate.py script correctly patches the method (DOM.resolveNode), but fails to patch the events. This causes issues when subscribing to these events. For example:

from selenium import webdriver
driver = webdriver.Chrome()
driver.get("https://www.google.com/")
devtools, connection = driver.start_devtools()
connection.on(devtools.debugger.ScriptParsed, lambda x: print(x))
# Force the browser to resend all scripts
connection.execute(devtools.debugger.disable())
connection.execute(devtools.debugger.enable()) # Raises a lot of "name not defined" error
driver.quit()

This PR adds rules to properly patch these events.

🔧 Implementation Notes

New rules have been added to patch the mentioned events, adhering to the original style.

💡 Additional Considerations

I'm unsure whether the order of event parameters will change in the future. If it does, we may need to update the code to match parameters by name rather than by index.

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Fix CDP code generator patching for erroneous $refs in events

• Add patches for Debugger.scriptFailedToParse and scriptParsed events

• Add patch for DOM.scrollableFlagUpdated event

• Update comments to reflect all patched domains

---

Diagram Walkthrough

flowchart LR
  A["CDP JSON Files"] --> B["generate.py Parser"]
  B --> C["Patching Rules"]
  C --> D["Fixed Event Parameters"]
  D --> E["Generated Python CDP Modules"]

Loading

File Walkthrough

	Relevant files
Bug fix	generate.py Add CDP event patching rules                                                          py/generate.py Add patching rules for Debugger.scriptFailedToParse and scriptParsed events Add patching rule for DOM.scrollableFlagUpdated event Update comments to document all three patched domains Reorganize existing DOM command patching logic +20/-5

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible IndexError Event parameter indexing for Debugger patches assumes fixed positions; if schema order changes or parameters are optional, direct indices (16, 17, 18) may be out of range at runtime. if event.name == 'scriptFailedToParse': # Patch 1.1 event.parameters[16].ref = 'ScriptLanguage' elif event.name == 'scriptParsed': # Patch 1.2 event.parameters[17].ref = 'ScriptLanguage' # Patch 1.3 event.parameters[18].items.ref = 'DebugSymbols' Fragile Logic Patching by hard-coded parameter indices is brittle; consider matching parameters by name to avoid silent mispatching when the protocol evolves. event.parameters[16].ref = 'ScriptLanguage' elif event.name == 'scriptParsed': # Patch 1.2 event.parameters[17].ref = 'ScriptLanguage' # Patch 1.3 event.parameters[18].items.ref = 'DebugSymbols' elif domain.domain == 'DOM': for cmd in domain.commands: if cmd.name == 'resolveNode': # Patch 2.1 cmd.parameters[1].ref = 'BackendNodeId' for event in domain.events: if event.name == 'scrollableFlagUpdated': # Patch 2.2 event.parameters[0].ref = 'NodeId' elif domain.domain == 'Page': None Guarding Accessing nested fields like event.parameters[18].items.ref assumes both parameters and items exist; add defensive checks to prevent AttributeError when 'items' or 'parameters' is missing. event.parameters[17].ref = 'ScriptLanguage' # Patch 1.3 event.parameters[18].items.ref = 'DebugSymbols'

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add defensive parameter validation Guard against missing or reordered parameters to avoid IndexError or incorrect patching when the protocol schema changes. Validate the parameter list length and names before applying positional patches. If the expected parameter is not found, log a warning and skip the patch instead of crashing. py/generate.py [1012-1021] if domain.domain == 'Debugger': for event in domain.events: if event.name == 'scriptFailedToParse': # Patch 1.1 - event.parameters[16].ref = 'ScriptLanguage' + if len(event.parameters) > 16 and event.parameters[16].name == 'scriptLanguage': + event.parameters[16].ref = 'ScriptLanguage' + else: + logger.warning("Debugger.scriptFailedToParse: expected parameter 'scriptLanguage' at index 16 not found; skipping patch") elif event.name == 'scriptParsed': # Patch 1.2 - event.parameters[17].ref = 'ScriptLanguage' + if len(event.parameters) > 17 and event.parameters[17].name == 'scriptLanguage': + event.parameters[17].ref = 'ScriptLanguage' + else: + logger.warning("Debugger.scriptParsed: expected parameter 'scriptLanguage' at index 17 not found; skipping patch") # Patch 1.3 - event.parameters[18].items.ref = 'DebugSymbols' + if len(event.parameters) > 18 and event.parameters[18].name == 'debugSymbols' and event.parameters[18].items: + event.parameters[18].items.ref = 'DebugSymbols' + else: + logger.warning("Debugger.scriptParsed: expected parameter 'debugSymbols' at index 18 not found; skipping patch") Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that using hard-coded indices like [16] is brittle and adds crucial validation to prevent IndexError or silent mis-patching, significantly improving the script's robustness.	Medium
	Validate indices and names for DOM Avoid hard-coded indices for DOM patches to prevent IndexError and mispatching if parameter order changes. Check parameter list bounds and confirm parameter names match expected targets before applying patches, logging a warning if not found. py/generate.py [1022-1030] elif domain.domain == 'DOM': for cmd in domain.commands: if cmd.name == 'resolveNode': # Patch 2.1 - cmd.parameters[1].ref = 'BackendNodeId' + if len(cmd.parameters) > 1 and cmd.parameters[1].name == 'backendNodeId': + cmd.parameters[1].ref = 'BackendNodeId' + else: + logger.warning("DOM.resolveNode: expected parameter 'backendNodeId' at index 1 not found; skipping patch") for event in domain.events: if event.name == 'scrollableFlagUpdated': # Patch 2.2 - event.parameters[0].ref = 'NodeId' + if len(event.parameters) > 0 and event.parameters[0].name == 'nodeId': + event.parameters[0].ref = 'NodeId' + else: + logger.warning("DOM.scrollableFlagUpdated: expected parameter 'nodeId' at index 0 not found; skipping patch") Apply / Chat Suggestion importance[1-10]: 8 __ Why: Similar to the first suggestion, this correctly points out the risk of using hard-coded indices ([1] and [0]) and proposes adding validation, which makes the patching logic much more resilient to future protocol changes.	Medium
	Guard against None descriptions Prevent AttributeError when event.description is None by checking before calling replace. This avoids crashes if the schema omits the description or it was already removed. py/generate.py [1031-1035] elif domain.domain == 'Page': for event in domain.events: if event.name == 'screencastVisibilityChanged': # Patch 3 - event.description = event.description.replace('`', '') + if event.description: + event.description = event.description.replace('`', '') + else: + logger.warning("Page.screencastVisibilityChanged: missing description; skipping backtick cleanup") Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential AttributeError if event.description is None and adds a necessary check, which improves the script's robustness against variations in the input data.	Medium
Update

[cgoldberg]

The generator code originates from:
https://github.com/HyperionGray/python-chrome-devtools-protocol/blob/master/generator/generate.py

I was going to suggest proposing this fix upstream to that repo instead... but I just looked, and our version has diverged somewhat from the original, so it probably makes sense to fix this here. (but you might want to open a PR there also).

I also noticed there is a test suite in the upstream repo: https://github.com/HyperionGray/python-chrome-devtools-protocol/blob/master/generator/test_generate.py

It might be worth pulling that into our repo's unit tests and update it to reflect changes like this.

[nxs7]

I was going to suggest proposing this fix upstream to that repo instead... but I just looked, and our version has diverged somewhat from the original

That's literally what I did... and thought :D

but you might want to open a PR there also

Sure, I'll do it when I get a chance.