Skip to content

fix: a few regressions from previous PRs - #49

Open
tomerqodo wants to merge 5 commits into
qodo_claude_vs_qodo_base_fix_a_few_regressions_from_previous_prs_pr10from
qodo_claude_vs_qodo_head_fix_a_few_regressions_from_previous_prs_pr10
Open

tomerqodo wants to merge 5 commits into
qodo_claude_vs_qodo_base_fix_a_few_regressions_from_previous_prs_pr10from
qodo_claude_vs_qodo_head_fix_a_few_regressions_from_previous_prs_pr10

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#10

@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Fix regressions and rename window features method

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix cookie deletion using wrong message type
• Rename with_window_features to window_features for consistency
• Fix window features application order and spacing
• Update example code to use new method names
• Remove unused imports and reorganize conditional compilation
Diagram
flowchart LR
  A["Bug Fixes"] --> B["Cookie deletion message"]
  A --> C["Method rename"]
  A --> D["Code organization"]
  B --> E["SetCookie instead of DeleteCookie"]
  C --> F["with_window_features to window_features"]
  D --> G["Remove unused imports"]
  D --> H["Reorganize conditional blocks"]
Loading

Grey Divider

File Changes

1. crates/tauri-runtime-wry/src/lib.rs 🐞 Bug fix +1/-1

Fix cookie deletion message type

• Fix delete_cookie method to use WebviewMessage::SetCookie instead of
 WebviewMessage::DeleteCookie
• Correct the message type for cookie deletion operations

crates/tauri-runtime-wry/src/lib.rs


2. crates/tauri/src/webview/mod.rs 🐞 Bug fix +3/-2

Update Cookie import and re-export

• Remove direct cookie::Cookie import
• Re-export Cookie from tauri_runtime with deprecation note for v3
• Update public API to use runtime's Cookie type

crates/tauri/src/webview/mod.rs


3. crates/tauri/src/webview/webview_window.rs 🐞 Bug fix +6/-6

Rename method and fix feature application order

• Rename method with_window_features to window_features
• Fix window features application order: size before position
• Fix spacing inconsistencies in method calls
• Update documentation example to reflect new method name

crates/tauri/src/webview/webview_window.rs


View more (1)
4. examples/api/src-tauri/src/lib.rs 🐞 Bug fix +23/-25

Reorganize code and update method calls

• Remove unused AtomicUsize import from top level
• Move on_new_window callback into desktop-specific conditional block
• Move app_ and created_window_count initialization into conditional block
• Update method call from with_window_features to window_features
• Change window label format from new-{number} to new-window-{number}

examples/api/src-tauri/src/lib.rs


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 10, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (3) 📎 Requirement gaps (0)

Grey Divider


Action required

1. window_features spacing unformatted 📘 Rule violation ✓ Correctness
Description
The new window_features implementation contains non-rustfmt spacing (extra spaces and missing
spaces after commas), indicating cargo fmt --check would modify the file. This violates the
requirement that Rust code conforms to the project rustfmt configuration.
Code

crates/tauri/src/webview/webview_window.rs[R1316-1322]

+    if let Some(size) = features.size() {
+      self.window_builder = self.window_builder.inner_size(size.width,    size.height);
    }

-    if let Some(size) = features.size() {
-      self.window_builder = self.window_builder.inner_size(size.width, size.height);
+    if let Some(position) = features.position() {
+      self.window_builder = self.window_builder.position(position.x,position.y);
    }
Evidence
PR Compliance ID 3 requires Rust code to be formatted per rustfmt; the added lines show spacing that
rustfmt would normalize (e.g., multiple spaces before size.height and missing spaces after
commas).

AGENTS.md
crates/tauri/src/webview/webview_window.rs[1316-1322]

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 newly added `window_features` code has spacing that is not compliant with rustfmt (extra spaces and missing spaces after commas), so `cargo fmt --check` would likely fail.

## Issue Context
Project compliance requires all Rust code to pass `cargo fmt --all -- --check`.

## Fix Focus Areas
- crates/tauri/src/webview/webview_window.rs[1316-1322]

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


2. Cookie re-export undocumented 📘 Rule violation ✧ Quality
Description
A new public re-export pub use tauri_runtime::Cookie; was added without a /// documentation
comment. This violates the requirement that public API items include documentation comments.
Code

crates/tauri/src/webview/mod.rs[R22-23]

+// Remove this re-export in v3
+pub use tauri_runtime::Cookie;
Evidence
PR Compliance ID 7 requires public API elements to have /// docs; the added `pub use
tauri_runtime::Cookie; is public but only has a non-doc //` comment above it.

AGENTS.md
crates/tauri/src/webview/mod.rs[22-23]

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

## Issue description
A new public re-export (`pub use tauri_runtime::Cookie;`) was introduced without a Rust doc comment (`///`), which violates the requirement that public APIs are documented.

## Issue Context
This is part of the public `tauri::webview` module surface; it should be documented similarly to other public exports.

## Fix Focus Areas
- crates/tauri/src/webview/mod.rs[22-23]

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


3. delete_cookie sends SetCookie 🐞 Bug ✓ Correctness
Description
WryWebviewDispatcher::delete_cookie dispatches WebviewMessage::SetCookie instead of
WebviewMessage::DeleteCookie, so Webview::delete_cookie will not delete cookies and may re-set
them instead.
Code

crates/tauri-runtime-wry/src/lib.rs[R1715-1719]

      Message::Webview(
        *self.window_id.lock().unwrap(),
        self.webview_id,
-        WebviewMessage::DeleteCookie(cookie.clone().into_owned()),
+        WebviewMessage::SetCookie(cookie.into_owned()),
      ),
Evidence
The delete_cookie dispatcher path sends WebviewMessage::SetCookie, while the runtime loop has
separate handlers for SetCookie (calls webview.set_cookie) and DeleteCookie (calls
webview.delete_cookie). This makes delete_cookie invoke the wrong runtime behavior.

crates/tauri-runtime-wry/src/lib.rs[1712-1721]
crates/tauri-runtime-wry/src/lib.rs[3731-3741]

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

### Issue description
`WryWebviewDispatcher::delete_cookie` currently sends `WebviewMessage::SetCookie`, which routes to `webview.set_cookie` instead of `webview.delete_cookie`. This breaks the cookie deletion API.

### Issue Context
The runtime event loop matches on `WebviewMessage::SetCookie` vs `WebviewMessage::DeleteCookie` and performs different operations.

### Fix Focus Areas
- crates/tauri-runtime-wry/src/lib.rs[1712-1721]

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


View more (1)
4. Removed window_features alias 🐞 Bug ✓ Correctness
Description
The public WebviewWindowBuilder::with_window_features method was removed and replaced with
window_features without a deprecated alias, so any downstream code still calling
with_window_features will fail to compile against this v2.7.0 API.
Code

crates/tauri/src/webview/webview_window.rs[R1315-1322]

+  pub fn window_features(mut self, features: NewWindowFeatures) -> Self {
+    if let Some(size) = features.size() {
+      self.window_builder = self.window_builder.inner_size(size.width,    size.height);
    }

-    if let Some(size) = features.size() {
-      self.window_builder = self.window_builder.inner_size(size.width, size.height);
+    if let Some(position) = features.position() {
+      self.window_builder = self.window_builder.position(position.x,position.y);
    }
Evidence
tauri is version 2.7.0 and WebviewWindowBuilder is a public type. The code now exposes
window_features(...) and examples/docs are updated to call it; without keeping
with_window_features as a deprecated wrapper, this is a breaking API change within the v2 line.

crates/tauri/Cargo.toml[1-4]
crates/tauri/src/webview/webview_window.rs[51-55]
crates/tauri/src/webview/webview_window.rs[1304-1322]
crates/tauri/src/webview/mod.rs[519-526]

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

### Issue description
`WebviewWindowBuilder::with_window_features` was renamed to `window_features` without keeping the old method as a deprecated alias. In the v2.x series (currently 2.7.0), removing a public method is a breaking change and will cause downstream compilation failures.

### Issue Context
The builder is part of the public API and examples/docs now call `window_features`, but existing external code may still call `with_window_features`.

### Fix Focus Areas
- crates/tauri/src/webview/webview_window.rs[1304-1351]

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



Remediation recommended

5. on_new_window uses unwrap() 📘 Rule violation ⛯ Reliability
Description
The updated example code uses unwrap() on fallible operations (parse, set_title, and build),
which can panic instead of surfacing errors. This conflicts with the guideline to return/propagate
Result for fallible operations.
Code

examples/api/src-tauri/src/lib.rs[R92-101]

+              tauri::WebviewUrl::External("about:blank".parse().unwrap()),
+            )
+            .window_features(features)
+            .on_document_title_changed(|window, title| {
+              window.set_title(&title).unwrap();
+            })
+            .title(url.as_str());
+
+            let window = builder.build().unwrap();
+            tauri::webview::NewWindowResponse::Create { window }
Evidence
PR Compliance ID 16 discourages panicking or hiding errors for fallible operations; the newly added
code uses unwrap() in multiple places that can fail at runtime.

AGENTS.md
examples/api/src-tauri/src/lib.rs[92-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
Example code added `unwrap()` on fallible operations (`parse`, `set_title`, `build`), which can panic and hides error conditions.

## Issue Context
Compliance requires fallible operations to return/propagate `Result` instead of panicking.

## Fix Focus Areas
- examples/api/src-tauri/src/lib.rs[92-101]

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment on lines +1316 to 1322
if let Some(size) = features.size() {
self.window_builder = self.window_builder.inner_size(size.width, size.height);
}

if let Some(size) = features.size() {
self.window_builder = self.window_builder.inner_size(size.width, size.height);
if let Some(position) = features.position() {
self.window_builder = self.window_builder.position(position.x,position.y);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. window_features spacing unformatted 📘 Rule violation ✓ Correctness

The new window_features implementation contains non-rustfmt spacing (extra spaces and missing
spaces after commas), indicating cargo fmt --check would modify the file. This violates the
requirement that Rust code conforms to the project rustfmt configuration.
Agent Prompt
## Issue description
The newly added `window_features` code has spacing that is not compliant with rustfmt (extra spaces and missing spaces after commas), so `cargo fmt --check` would likely fail.

## Issue Context
Project compliance requires all Rust code to pass `cargo fmt --all -- --check`.

## Fix Focus Areas
- crates/tauri/src/webview/webview_window.rs[1316-1322]

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

Comment on lines +22 to +23
// Remove this re-export in v3
pub use tauri_runtime::Cookie;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. cookie re-export undocumented 📘 Rule violation ✧ Quality

A new public re-export pub use tauri_runtime::Cookie; was added without a /// documentation
comment. This violates the requirement that public API items include documentation comments.
Agent Prompt
## Issue description
A new public re-export (`pub use tauri_runtime::Cookie;`) was introduced without a Rust doc comment (`///`), which violates the requirement that public APIs are documented.

## Issue Context
This is part of the public `tauri::webview` module surface; it should be documented similarly to other public exports.

## Fix Focus Areas
- crates/tauri/src/webview/mod.rs[22-23]

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

Comment on lines +92 to +101
tauri::WebviewUrl::External("about:blank".parse().unwrap()),
)
.window_features(features)
.on_document_title_changed(|window, title| {
window.set_title(&title).unwrap();
})
.title(url.as_str());

let window = builder.build().unwrap();
tauri::webview::NewWindowResponse::Create { window }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. on_new_window uses unwrap() 📘 Rule violation ⛯ Reliability

The updated example code uses unwrap() on fallible operations (parse, set_title, and build),
which can panic instead of surfacing errors. This conflicts with the guideline to return/propagate
Result for fallible operations.
Agent Prompt
## Issue description
Example code added `unwrap()` on fallible operations (`parse`, `set_title`, `build`), which can panic and hides error conditions.

## Issue Context
Compliance requires fallible operations to return/propagate `Result` instead of panicking.

## Fix Focus Areas
- examples/api/src-tauri/src/lib.rs[92-101]

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

Comment on lines 1715 to 1719
Message::Webview(
*self.window_id.lock().unwrap(),
self.webview_id,
WebviewMessage::DeleteCookie(cookie.clone().into_owned()),
WebviewMessage::SetCookie(cookie.into_owned()),
),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

4. Delete_cookie sends setcookie 🐞 Bug ✓ Correctness

WryWebviewDispatcher::delete_cookie dispatches WebviewMessage::SetCookie instead of
WebviewMessage::DeleteCookie, so Webview::delete_cookie will not delete cookies and may re-set
them instead.
Agent Prompt
### Issue description
`WryWebviewDispatcher::delete_cookie` currently sends `WebviewMessage::SetCookie`, which routes to `webview.set_cookie` instead of `webview.delete_cookie`. This breaks the cookie deletion API.

### Issue Context
The runtime event loop matches on `WebviewMessage::SetCookie` vs `WebviewMessage::DeleteCookie` and performs different operations.

### Fix Focus Areas
- crates/tauri-runtime-wry/src/lib.rs[1712-1721]

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

Comment on lines +1315 to 1322
pub fn window_features(mut self, features: NewWindowFeatures) -> Self {
if let Some(size) = features.size() {
self.window_builder = self.window_builder.inner_size(size.width, size.height);
}

if let Some(size) = features.size() {
self.window_builder = self.window_builder.inner_size(size.width, size.height);
if let Some(position) = features.position() {
self.window_builder = self.window_builder.position(position.x,position.y);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

5. Removed window_features alias 🐞 Bug ✓ Correctness

The public WebviewWindowBuilder::with_window_features method was removed and replaced with
window_features without a deprecated alias, so any downstream code still calling
with_window_features will fail to compile against this v2.7.0 API.
Agent Prompt
### Issue description
`WebviewWindowBuilder::with_window_features` was renamed to `window_features` without keeping the old method as a deprecated alias. In the v2.x series (currently 2.7.0), removing a public method is a breaking change and will cause downstream compilation failures.

### Issue Context
The builder is part of the public API and examples/docs now call `window_features`, but existing external code may still call `with_window_features`.

### Fix Focus Areas
- crates/tauri/src/webview/webview_window.rs[1304-1351]

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants