Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an offline ChangesCompare Heapsnapshots Feature
Daemon Connection and Kill Handling
Formatting Reflows
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new offline command, compare-heapsnapshots, which allows users to compare two .heapsnapshot files and view per-class differences in added/deleted nodes and size deltas without requiring a Chrome connection. It implements the necessary parsing, aggregation, diffing, and formatting logic, along with comprehensive unit tests. The feedback highlights a critical issue where sorting the diffs is non-deterministic for classes with identical size deltas due to the randomized iteration order of HashSet. This can cause the index used to query detailed class views to change across CLI invocations. To resolve this, a deterministic tie-breaker sorting by class name is recommended.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib.rs (1)
251-256: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer auto-derived
shortfor consistency.Elsewhere in this enum (e.g.
InspectHeapSnapshotNode,outputfields) short flags are auto-derived via#[arg(long, short)]rather than explicitly pinned. Since'b'/'c'are exactly the auto-derived values here, using the same convention would be more consistent.♻️ Proposed consistency tweak
/// Path to the base (earlier) .heapsnapshot file - #[arg(long, short = 'b')] + #[arg(long, short)] base: String, /// Path to the current (later) .heapsnapshot file - #[arg(long, short = 'c')] + #[arg(long, short)] current: String,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib.rs` around lines 251 - 256, The `base` and `current` fields in this enum use explicit short flags even though they match clap’s auto-derived values. Update the `#[arg(...)]` attributes on these fields to use the same `#[arg(long, short)]` convention used by `InspectHeapSnapshotNode` and the `output` fields so the enum stays consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/memory.rs`:
- Line 399: The `diffs.sort_by` ordering in `memory.rs` is only comparing
`size_delta`, so ties can reorder differently across runs because
`all_names.into_iter()` comes from a randomized `HashSet`. Update the sorting in
the summary path to use a deterministic secondary key from the class identity
(for example the class name or another stable field from the `diff` item) so the
row order is reproducible across separate invocations and `--class-index`
matches the displayed summary index.
---
Nitpick comments:
In `@src/lib.rs`:
- Around line 251-256: The `base` and `current` fields in this enum use explicit
short flags even though they match clap’s auto-derived values. Update the
`#[arg(...)]` attributes on these fields to use the same `#[arg(long, short)]`
convention used by `InspectHeapSnapshotNode` and the `output` fields so the enum
stays consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b6d31b44-cc7b-4a10-a3ce-ddb0812f72d6
📒 Files selected for processing (3)
src/commands/executor.rssrc/commands/memory.rssrc/lib.rs
|
/gemini review |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces a new offline command, compare-heapsnapshots, which compares two .heapsnapshot files to report per-class added and deleted nodes without requiring a Chrome connection. It includes helper functions for parsing, aggregating, diffing, and formatting the snapshot data, along with comprehensive unit tests. The reviewer suggested two performance optimizations: first, avoiding redundant string cloning in build_class_aggregates by only cloning when inserting a new class name; second, optimizing diff_snapshots to eliminate an intermediate HashSet allocation and reduce map lookups by iterating over the maps directly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/commands/memory.rs (2)
406-415: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
csv_escapeinformat_node_detailsto avoid divergent escaping.This is a verbatim copy of the escaping logic already inlined in
format_node_details(Lines 226-230). Having two copies invites future drift. Route the existing call through this helper.♻️ Proposed fix
- let escaped_name = if name.contains(',') || name.contains('"') || name.contains('\n') || name.contains('\r') { - format!("\"{}\"", name.replace('"', "\"\"")) - } else { - name.to_string() - }; + let escaped_name = csv_escape(name);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/memory.rs` around lines 406 - 415, `format_node_details` currently duplicates the CSV escaping logic that `csv_escape` already provides, which can lead to drift between the two paths. Update `format_node_details` to call `csv_escape` for class/node name escaping instead of keeping its own inline implementation, and leave `csv_escape` as the single shared helper for this behavior.
355-397: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDetail node ordering is non-deterministic across runs.
added_ids/deleted_idsare populated by iteratingcur.nodes/base_a.nodes, which areHashMaps seeded with a per-process random state. The summary sort is now stable (Line 399), but for a class with multiple added/removed nodes, the--class-indexdetail view rows (format_class_diff_detail) will appear in a different order on each separate invocation. Add a deterministic key so detail output is reproducible.🔧 Proposed fix: sort node lists by id
let added_count = added_ids.len(); let removed_count = deleted_ids.len(); if added_count == 0 && removed_count == 0 { return None; } + // Sort by id so the per-class detail view is reproducible across + // separate `--class-index` invocations (HashMap order is randomized). + let mut added: Vec<(u64, u64)> = added_ids.into_iter().zip(added_self_sizes).collect(); + added.sort_unstable_by_key(|&(id, _)| id); + let (added_ids, added_self_sizes): (Vec<u64>, Vec<u64>) = added.into_iter().unzip(); + let mut deleted: Vec<(u64, u64)> = deleted_ids.into_iter().zip(deleted_self_sizes).collect(); + deleted.sort_unstable_by_key(|&(id, _)| id); + let (deleted_ids, deleted_self_sizes): (Vec<u64>, Vec<u64>) = deleted.into_iter().unzip(); let count_delta = added_count as i64 - removed_count as i64;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/memory.rs` around lines 355 - 397, Make the per-class detail rows deterministic in `memory.rs` by ensuring `HeapSnapshotClassDiff` stores `added_ids` and `deleted_ids` in a stable order before `format_class_diff_detail` renders them. The current loops that populate these vectors from `cur_agg` and `base_agg` iterate `HashMap` entries, so sort the collected node ids (and keep `added_self_sizes` / `deleted_self_sizes` aligned with them) using a consistent key such as the node id or a derived deterministic tuple. This should be done in the aggregation logic that builds `HeapSnapshotClassDiff`, not in the formatter, so `--class-index` output is reproducible across runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/commands/memory.rs`:
- Around line 406-415: `format_node_details` currently duplicates the CSV
escaping logic that `csv_escape` already provides, which can lead to drift
between the two paths. Update `format_node_details` to call `csv_escape` for
class/node name escaping instead of keeping its own inline implementation, and
leave `csv_escape` as the single shared helper for this behavior.
- Around line 355-397: Make the per-class detail rows deterministic in
`memory.rs` by ensuring `HeapSnapshotClassDiff` stores `added_ids` and
`deleted_ids` in a stable order before `format_class_diff_detail` renders them.
The current loops that populate these vectors from `cur_agg` and `base_agg`
iterate `HashMap` entries, so sort the collected node ids (and keep
`added_self_sizes` / `deleted_self_sizes` aligned with them) using a consistent
key such as the node id or a derived deterministic tuple. This should be done in
the aggregation logic that builds `HeapSnapshotClassDiff`, not in the formatter,
so `--class-index` output is reproducible across runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b406a0b9-24f9-4db0-ba3b-4a8e6cd7310c
📒 Files selected for processing (3)
src/commands/executor.rssrc/commands/memory.rssrc/lib.rs
- Reformatted argument lists and chained method calls for better clarity in `executor.rs`, `memory.rs`, `read_page.rs`, `screenshot.rs`, `third_party.rs`, and `lib.rs`. - Enhanced error handling messages in `lib.rs` for clearer debugging. - Updated tests in `read_page.rs`, `screenshot.rs`, and `lib.rs` for improved readability and consistency in assertions. - Removed unnecessary line breaks and adjusted formatting in various files to adhere to style guidelines.
|
/gemini review |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces a new offline command, compare-heapsnapshots, which compares two local .heapsnapshot files and reports per-class added or deleted nodes without requiring a Chrome connection. It also includes extensive code formatting and style cleanups across several files. The review feedback highlights two important performance and memory optimizations in src/commands/memory.rs: dropping the raw HeapSnapshot ASTs immediately after aggregation to prevent potential Out-Of-Memory (OOM) errors on large snapshots, and pre-allocating vector capacities during the diffing process to minimize reallocations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/commands/memory.rs (1)
333-339: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider the
EntryAPI to simplify the insert-or-update branch.The
get_mut/else-insert pattern duplicates theagg.nodes.insert(id, self_size)logic across both branches.entry(...).or_insert_with(...)collapses this to one line without behavior change.♻️ Proposed refactor
- if let Some(agg) = aggregates.get_mut(name_ref) { - agg.nodes.insert(id, self_size); - } else { - let mut agg = ClassAggregate::new(); - agg.nodes.insert(id, self_size); - aggregates.insert(name_ref.clone(), agg); - } + aggregates + .entry(name_ref.clone()) + .or_insert_with(ClassAggregate::new) + .nodes + .insert(id, self_size);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/memory.rs` around lines 333 - 339, The insert-or-update logic in the aggregate handling duplicates the same node insertion in both branches. Refactor the `aggregates` update in `memory.rs` to use the `Entry` API on `name_ref` so `ClassAggregate::new()` is only created when missing and `agg.nodes.insert(id, self_size)` happens in one place. Keep the existing behavior unchanged while simplifying the branch around the `get_mut`/insert path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/commands/memory.rs`:
- Around line 333-339: The insert-or-update logic in the aggregate handling
duplicates the same node insertion in both branches. Refactor the `aggregates`
update in `memory.rs` to use the `Entry` API on `name_ref` so
`ClassAggregate::new()` is only created when missing and `agg.nodes.insert(id,
self_size)` happens in one place. Keep the existing behavior unchanged while
simplifying the branch around the `get_mut`/insert path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 36e8f73a-3cf5-4170-be03-af9aa790539c
📒 Files selected for processing (7)
src/commands/evaluate.rssrc/commands/executor.rssrc/commands/memory.rssrc/commands/read_page.rssrc/commands/screenshot.rssrc/commands/third_party.rssrc/lib.rs
* preallocate added/deleted vectors using upper-bound capacities to avoid reallocations * compute base length once to size deletion buffers efficiently * scope snapshot parsing to drop raw data immediately after building aggregates, lowering peak memory usage during diff
|
/gemini review |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/commands/memory.rs (1)
316-342: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
entry()to avoid double hashmap lookups in the hot loop.
get_mutfollowed byinsertperforms two lookups per node;entry()does it in one. This loop runs once per node in the entire snapshot, so for large snapshots this adds up.♻️ Proposed refactor
- if let Some(agg) = aggregates.get_mut(name_ref) { - agg.nodes.insert(id, self_size); - } else { - let mut agg = ClassAggregate::new(); - agg.nodes.insert(id, self_size); - aggregates.insert(name_ref.clone(), agg); - } + aggregates + .entry(name_ref.clone()) + .or_insert_with(ClassAggregate::new) + .nodes + .insert(id, self_size);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/memory.rs` around lines 316 - 342, The node aggregation loop in `src/commands/memory.rs` is doing two HashMap lookups per class name via `aggregates.get_mut()` followed by `insert`; switch this hot path to `HashMap::entry()` in the `while current_idx + node_size <= nodes.len()` loop so each `name_ref` is handled with a single lookup. Update the `ClassAggregate` insertion logic to use the entry API while preserving the current `ClassAggregate::new()` initialization and `agg.nodes.insert(id, self_size)` behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/commands/memory.rs`:
- Around line 316-342: The node aggregation loop in `src/commands/memory.rs` is
doing two HashMap lookups per class name via `aggregates.get_mut()` followed by
`insert`; switch this hot path to `HashMap::entry()` in the `while current_idx +
node_size <= nodes.len()` loop so each `name_ref` is handled with a single
lookup. Update the `ClassAggregate` insertion logic to use the entry API while
preserving the current `ClassAggregate::new()` initialization and
`agg.nodes.insert(id, self_size)` behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 90584542-39aa-4394-8831-6c213c013471
📒 Files selected for processing (7)
src/commands/evaluate.rssrc/commands/executor.rssrc/commands/memory.rssrc/commands/read_page.rssrc/commands/screenshot.rssrc/commands/third_party.rssrc/lib.rs
…n termination * add connection timeout for Chrome WebSocket handshake with actionable error messaging * document failure handling and agent guidance in AGENTS.md and SKILL.md * introduce guarded `kill-daemon` command with `--force` flag and TTY-based confirmation/refusal logic * prevent non-interactive agents from killing daemon without explicit force * add unit-testable decision logic for daemon termination behavior * update CLI argument parsing and command handling for new `kill-daemon` behavior
|
/gemini review |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces a new offline command compare-heapsnapshots to diff two .heapsnapshot files and report per-class added/deleted nodes, complete with text/JSON formatting and unit tests. It also implements a connection timeout for the Chrome WebSocket handshake to prevent indefinite hangs, and adds a safety guard to the kill-daemon command to refuse non-interactive execution unless --force is specified. The review feedback suggests a valuable performance optimization in the heap snapshot diffing logic to avoid allocating and sorting detailed node vectors when only the summary view is requested.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/commands/memory.rs (1)
57-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant “what” comment.
Line 57 only restates the next CDP call; either drop it or replace it with the reason this enable step must precede streaming. As per coding guidelines,
src/**/*.rs: Rust code comments should explain "why", not "what".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/memory.rs` around lines 57 - 59, The comment before the HeapProfiler enable call is redundant and only restates what `client.send_to_target(session_id, "HeapProfiler.enable", json!({}))` already shows. Remove that “what” comment or replace it with a brief “why” explanation for `send_to_target` in `memory.rs`, focusing on why HeapProfiler must be enabled before starting streaming.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/memory.rs`:
- Around line 652-658: The out-of-range classIndex validation in the memory
command is currently using a generic anyhow error, which will surface as an
unspecified CLI code. Update the invalid index path in the class selection logic
to return a typed CliError with ErrorCode::InvalidInput instead of anyhow!, so
the error boundary preserves the stable user-input error code; use the existing
classIndex/diffs lookup in memory command to locate the change.
- Around line 316-343: The flat node parsing in the memory snapshot aggregation
loop currently stops at the last complete record and silently ignores any
trailing partial data. Update the logic around the aggregates-building loop in
the memory command to validate that nodes.len() is evenly divisible by node_size
before iterating, and return an error for malformed snapshots instead of relying
on the current_idx + node_size <= nodes.len() check. Keep the fix localized to
the class aggregation path that uses ClassAggregate and the node offsets so
truncated arrays are rejected early.
In `@src/lib.rs`:
- Around line 874-876: The match on kill-daemon handling is partially moving cli
by destructuring cli.command, which breaks later uses of cli.command and
build_request(&cli) in the same flow. Update the Commands::KillDaemon branch to
borrow cli.command instead of taking ownership, and dereference the force field
inside the pattern so the existing cli value remains available for the later
matches and request-building logic.
---
Nitpick comments:
In `@src/commands/memory.rs`:
- Around line 57-59: The comment before the HeapProfiler enable call is
redundant and only restates what `client.send_to_target(session_id,
"HeapProfiler.enable", json!({}))` already shows. Remove that “what” comment or
replace it with a brief “why” explanation for `send_to_target` in `memory.rs`,
focusing on why HeapProfiler must be enabled before starting streaming.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4a715b7-0eb2-443f-bd39-be280522faec
📒 Files selected for processing (10)
AGENTS.mdskill/chrome-devtools/SKILL.mdsrc/cdp.rssrc/commands/evaluate.rssrc/commands/executor.rssrc/commands/memory.rssrc/commands/read_page.rssrc/commands/screenshot.rssrc/commands/third_party.rssrc/lib.rs
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new offline compare-heapsnapshots command to compare two heap snapshot files and report per-class added or deleted nodes. It also adds safety guards to the kill-daemon command to prevent non-interactive agents from dropping approved Chrome connections, introduces a connection timeout for Chrome WebSocket handshakes to avoid indefinite hangs, and updates documentation. Feedback suggests extracting duplicate node field offset parsing logic in src/commands/memory.rs into a reusable helper struct to improve maintainability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…rity and reuse in snapshot parsing
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfb3705f19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… grouping for non-object nodes
|
/agentic_review |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
add compare-heapsnapshots command and documentation for memory leak debugging workflow refactor diffing to separate summary vs per-node detail computation (lazy detail resolution) remove per-node vectors from summary structs to reduce memory usage add compute_class_node_detail for on-demand node-level diff output detect and warn on snapshots with zero shared node IDs (likely different sessions) validate snapshot node array alignment to catch truncated/malformed files early fix node iteration bounds to ensure full and consistent traversal improve CSV/text formatting with consistent newline handling make connect timeout parsing pure, validated, and unit-testable add extensive unit tests for timeout parsing, snapshot validation, and diff correctness update CLI and docs with constraints on same-session snapshot comparison harden kill-daemon handling by avoiding partial moves in match logic
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new offline command compare-heapsnapshots to compare two .heapsnapshot files and report per-class added/deleted nodes. It also updates the kill-daemon command with safety guards and confirmation prompts, refusing to run non-interactively without the --force flag, and adds a timeout to the Chrome WebSocket connection handshake to prevent indefinite hangs. The review feedback identifies a critical compilation error in src/commands/memory.rs due to the use of the non-standard is_multiple_of method on usize, and suggests performance optimizations for heap snapshot diffing, such as avoiding redundant Option checks inside loops and pre-allocating capacity for HashSet collections.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…etection refactor compute_class_node_detail to avoid repeated Option checks inside loops handle class-new and class-deleted cases with direct branches for efficiency reduce per-iteration overhead with simplified HashMap lookups preallocate HashSet capacity in snapshots_share_no_node_ids to avoid reallocation during ID collection
|
@coderabbitai full review |
✅ Action performedFull review finished. |
replace explicit error check with ? operator for cleaner propagation
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@codex review |
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 9ae27ee |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new offline command compare-heapsnapshots to diff two V8 heap snapshots and group V8 heap objects by class or type name, accompanied by extensive documentation and tests. It also adds a connection timeout for Chrome WebSocket handshakes to prevent indefinite hangs, and updates the kill-daemon command to require a --force flag when run non-interactively. The review feedback correctly identifies two critical compilation errors in the new heap snapshot diffing code: the use of the non-standard is_multiple_of method on usize, and a type mismatch when attempting to add a reference &u64 to a u64 variable without dereferencing.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 0c9120b |
1 participant
Summary by CodeRabbit
New Features
.heapsnapshotfiles, showing a class-diff summary by default and optional per-class detail via--class-index(deterministic ordering).Bug Fixes
--force, require interactive confirmation, and prevent non-interactive use unless--forceis set.Documentation
Tests
--class-indexvalidation.