feat(rust): differ poc - #542
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
Comment |
0a8b865 to
0366256
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (10)
rust/crates/adc-sdk/src/resource.rs (1)
58-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the relationship between
FieldListTypeandFieldMeta.
FieldListTypeis publicly re-exported, and the corresponding TypeScript API is used bylibs/differ. It is not dead code. Add a doc comment that explains why RustFieldMetarepeats these four strategies.🤖 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 `@rust/crates/adc-sdk/src/resource.rs` around lines 58 - 69, Update the public FieldListType documentation to explain that its four strategies intentionally mirror the corresponding FieldMeta strategies, preserving consistency with the TypeScript API used by libs/differ. Add this relationship to the existing enum-level doc comment without changing the enum variants or behavior.rust/crates/adc-differ/src/differ_v4.rs (4)
294-317: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftConsider borrowing instead of cloning every resource item.
extract_tuplesclones each item at lines 302 and 311.diff_resourcethen clones the matched local item again at line 76 and the remote item inprepare_remote_item. The first clone is redundant, because the tuple values are only read to buildlocal_id_mapand to feed the per-item clone.Changing
ResourceTupleto(String, String, &'a Value)removes one full clone of each config per resource type. This touches the type alias and both loops, so it is reasonable to defer until after the benchmark baseline is recorded.🤖 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 `@rust/crates/adc-differ/src/differ_v4.rs` around lines 294 - 317, The resource extraction path unnecessarily clones each item into ResourceTuple before later per-item processing. Update ResourceTuple and extract_tuples to borrow Value references with an appropriate lifetime, then adjust diff_resource and prepare_remote_item’s loops and ownership handling to consume borrowed items while preserving the existing ID-map and per-item clone behavior. Record the benchmark baseline before making this optimization.
391-421: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift
merge_defaultclones on every call and on every default key.Line 392 deep-clones the whole resource. Line 400 then clones the existing value for every key in
defaults. Line 409 and line 413 recurse, so each level repeats both clones. The clone at line 400 is discarded entirely when the default value is an object or an array and the existing value is absent or null.
merge_defaultruns once per updated resource (line 204) and once per plugin (line 271), so this is on the hot path for the benchmark.Convert the helper to merge in place. The caller clones once, and the recursion mutates without further allocation.
♻️ Sketch of an in-place variant
fn merge_default(resource: &Value, defaults: &Value) -> Value { let mut result = resource.clone(); merge_default_into(&mut result, defaults); result } fn merge_default_into(target: &mut Value, defaults: &Value) { let Value::Object(defaults_map) = defaults else { return }; let Value::Object(target_map) = target else { return }; for (key, value) in defaults_map { if key == "__proto__" || key == "constructor" || key == "prototype" { continue; } match target_map.get_mut(key) { None | Some(Value::Null) => { if !(value.is_object() || value.is_array()) { target_map.insert(key.clone(), value.clone()); } } Some(existing) => { if value.is_object() && existing.is_object() { merge_default_into(existing, value); } else if let (Value::Array(value_arr), Value::Array(existing_arr)) = (value, existing) && let Some(first_default) = value_arr.first() { for item in existing_arr.iter_mut() { merge_default_into(item, first_default); } } } } } }Note:
Some(Value::Null)in aget_mutmatch arm needs a small adjustment, because the borrow is mutable. Check the null case before taking the mutable borrow.🤖 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 `@rust/crates/adc-differ/src/differ_v4.rs` around lines 391 - 421, Refactor merge_default to clone the resource once, then delegate recursive merging to an in-place helper such as merge_default_into. Remove per-key cloned existing values and mutate object and array entries through mutable references; handle null entries without violating Rust’s mutable-borrow rules, while preserving protected-key filtering and existing default-merging behavior.
262-291: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove two avoidable deep clones and the redundant reverse diff.
Three costs in this function are avoidable:
- Line 267 clones the whole local plugin map, but the loop at 269 only reads it while building
merged_local.- Line 273 clones the whole remote plugin map, but
remote_objis only read bychecker.- Line 289 runs
checkerin both directions. The forward call already diffs every shared plugin. The reverse call diffs all of them a second time, and it only needs to detect plugins that exist on the remote and not inmerged_local.
diff_pluginsruns once perObjectMapfield per updated resource, so this is on the hot path.♻️ Proposed refactor
- let local_obj = local.as_object().cloned().unwrap_or_default(); + static EMPTY: std::sync::LazyLock<Map<String, Value>> = std::sync::LazyLock::new(Map::new); + let local_obj = local.as_object().unwrap_or(&EMPTY); let mut merged_local = Map::new(); - for (plugin_name, config) in &local_obj { + for (plugin_name, config) in local_obj { let default = self.default_value.plugins.get(plugin_name).cloned().unwrap_or_else(|| json!({})); merged_local.insert(plugin_name.clone(), merge_default(config, &default)); } - let remote_obj = remote.as_object().cloned().unwrap_or_default(); - - let checker = |left: &Map<String, Value>, right: &Map<String, Value>| -> bool { - for (name, left_plugin) in left { - match right.get(name) { - None => return true, - Some(right_plugin) => { - if diff_value(left_plugin, right_plugin).is_some() { - return true; - } - } - } - } - false - }; - - let changed = checker(&merged_local, &remote_obj) || checker(&remote_obj, &merged_local); + let remote_obj = remote.as_object().unwrap_or(&EMPTY); + + // Any remote-only plugin is a change; the forward walk below covers + // local-only plugins and every shared plugin. + let changed = merged_local.len() != remote_obj.len() + || merged_local.iter().any(|(name, left_plugin)| match remote_obj.get(name) { + None => true, + Some(right_plugin) => diff_value(left_plugin, right_plugin).is_some(), + }); (changed, Value::Object(merged_local))🤖 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 `@rust/crates/adc-differ/src/differ_v4.rs` around lines 262 - 291, Update diff_plugins to iterate over local and remote through borrowed object maps, avoiding cloned whole-map values while preserving the existing empty-value behavior and merged_local construction. Replace the bidirectional checker calls with one forward comparison for shared/local-only plugins plus a lightweight remote-key check that only detects remote plugins absent from merged_local; avoid rerunning diff_value for shared plugins.
21-27: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift
DefaultValueis deep-cloned on every recursivediffcall.Line 27 clones the whole
DefaultValueinto the newDifferV4.diffis recursive:handle_delete(line 110),handle_create(line 132), andhandle_update(line 194) each callDifferV4::diffagain, once per resource item that has nested fields. Each of those calls repeats the deep clone of the core map and the plugin defaults map.For a config with many services that each contain routes, this clone runs per service and dominates useful work. The PR targets a 2-3x speedup, so this is worth fixing before the benchmark numbers are published.
Borrow the defaults instead. Store
&'a DefaultValueinDifferV4, or wrap the owned fallback inCow/Arcso the recursive calls share one instance.🤖 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 `@rust/crates/adc-differ/src/differ_v4.rs` around lines 21 - 27, Avoid deep-cloning DefaultValue on each recursive DifferV4::diff call. Change DifferV4 and its diff construction to borrow or share a single defaults instance, while preserving the existing default fallback when no value is provided; ensure handle_delete, handle_create, and handle_update recursive calls reuse that instance.rust/crates/adc-sdk/src/value_diff.rs (1)
97-113: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftAvoid one
Vecallocation per visited key.
diff_objectcallspath.to_vec()for every key on both sides, anddiff_arraydoes the same per element. Each call allocates and copies the whole current path. The cost grows with depth times node count, which is the hot path for this PR's performance goal.Use a single
&mut DiffPathand push/pop around the recursive call. Clone the path only when a change is recorded.🤖 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 `@rust/crates/adc-sdk/src/value_diff.rs` around lines 97 - 113, Update diff_object and the analogous diff_array traversal to use one mutable DiffPath, pushing each key or index before deep_diff and popping it afterward instead of calling path.to_vec() per child. Adjust deep_diff and change-recording logic so the path is cloned only when constructing a recorded ValueDiff, while preserving traversal order and path contents.rust/Cargo.toml (1)
1-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare and inherit the workspace MSRV.
Set
rust-version = "1.88"in[workspace.package], and addrust-version.workspace = trueto each member’s[package]table. Thelet_chainssyntax requires Rust 1.88. The workspace value alone does not apply to members.🤖 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 `@rust/Cargo.toml` around lines 1 - 7, Update the workspace.package table in Cargo.toml to declare rust-version = "1.88", then add rust-version.workspace = true to the [package] table of every workspace member: adc-sdk, adc-differ, adc-sync-bench, and adc-mock-server.rust/crates/adc-differ/examples/gen_fixtures.rs (1)
91-108: 🚀 Performance & Scalability | 🔵 TrivialVerify whether large generated fixtures should be committed to the repository.
This function writes fixture JSON files for every scale and change-ratio combination, including the "large" scale (10,000 services, roughly 30,000 resources per file). Confirm the intent to commit these generated artifacts, since they add non-trivial size to the repository across small, medium, and large scales.
🤖 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 `@rust/crates/adc-differ/examples/gen_fixtures.rs` around lines 91 - 108, Confirm the repository policy for generated fixture artifacts produced by main, especially the large scale generated by SCALES. If these JSON files are not intended to be committed, update main to generate them only on demand or exclude the output directory from version control; otherwise preserve the generation and explicitly retain the committed fixtures for all scales and change ratios.rust/crates/adc-differ/tests/regression.rs (1)
63-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen the sibling regression assertions.
The test asserts only the event count and the event type. An Update event emitted for an unrelated reason still satisfies these assertions, so the regression guard is weak. Assert the resource type, the resource ID, and the expected diff on
description.♻️ Proposed stronger assertions
let events = DifferV4::diff(&local, &remote, Some(&default_value), None); assert_eq!(events.len(), 1); - assert_eq!(events[0].event_type, adc_sdk::EventType::Update); + let e = &events[0]; + assert_eq!(e.resource_type, ResourceType::Service); + assert_eq!(e.event_type, adc_sdk::EventType::Update); + assert_eq!(e.resource_id, generate_id(service_name)); + assert_eq!( + e.diff, + Some(vec![adc_sdk::ValueDiff::Deleted { + path: vec![adc_sdk::PathSegment::Key("description".into())], + lhs: json!(""), + }]) + );🤖 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 `@rust/crates/adc-differ/tests/regression.rs` around lines 63 - 66, Strengthen the assertions in the DifferV4 regression test by verifying the single Update event’s resource type, resource ID, and diff contents for the description field, while retaining the existing count and event-type checks.rust/crates/adc-differ/tests/basic.rs (1)
10-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
configandevtest helpers into a shared module.Seven integration test files define identical
configandevhelpers. The shared root cause is the absence of a test-support module in the crate. Cargo compiles each file intests/as a separate binary, so addtests/common/mod.rsand declaremod common;in each test file.
rust/crates/adc-differ/tests/basic.rs#L10-L16: replace both helpers withmod common;anduse common::{config, ev};.rust/crates/adc-differ/tests/consumer.rs#L7-L13: replace both helpers with the shared module import.rust/crates/adc-differ/tests/custom_id.rs#L7-L13: replace both helpers with the shared module import.rust/crates/adc-differ/tests/regression.rs#L10-L12: replaceconfigwith the shared module import.rust/crates/adc-differ/tests/service_upstream.rs#L7-L13: replace both helpers with the shared module import.rust/crates/adc-differ/tests/upstream.rs#L7-L9: replaceconfigwith the shared module import.rust/crates/adc-differ/tests/usecase.rs#L8-L14: replace both helpers with the shared module import.♻️ Proposed shared module
Add
rust/crates/adc-differ/tests/common/mod.rs:#![allow(dead_code)] use adc_sdk::{Event, EventType, InternalConfiguration, ResourceType}; use serde_json::Value; pub fn config(v: Value) -> InternalConfiguration { v.as_object().cloned().unwrap_or_default() } pub fn ev(rt: ResourceType, et: EventType, id: &str, name: &str) -> Event { Event::new(rt, et, id, name) }Then in each test file:
-fn config(v: Value) -> InternalConfiguration { - v.as_object().cloned().unwrap_or_default() -} - -fn ev(rt: ResourceType, et: EventType, id: &str, name: &str) -> Event { - Event::new(rt, et, id, name) -} +mod common; +use common::{config, ev};🤖 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 `@rust/crates/adc-differ/tests/basic.rs` around lines 10 - 16, Extract the duplicated config and ev helpers into a shared tests/common/mod.rs module, preserving their current behavior and making them public for integration tests. In rust/crates/adc-differ/tests/basic.rs lines 10-16, consumer.rs lines 7-13, custom_id.rs lines 7-13, regression.rs lines 10-12, service_upstream.rs lines 7-13, upstream.rs lines 7-9, and usecase.rs lines 8-14, remove the local helper definitions, declare mod common;, and import the helpers with use common::{config, ev}; where ev is used; regression.rs and upstream.rs require only config.
🤖 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 `@rust/crates/adc-differ/tests/fixtures_sanity.rs`:
- Around line 22-46: Extend the fixture sanity tests around
small_none_has_no_diff, small_few_has_expected_update_count, and
small_many_has_expected_update_count into table-driven coverage for small,
medium, and large scales. Assert no events for each none fixture, and for few
and many assert both the expected event count and every event’s update type and
resource identity, preserving the existing fixture-loading and DifferV4::diff
setup.
In `@rust/crates/adc-sdk/src/field_meta.rs`:
- Around line 20-23: Update apply_atomic_strips to handle FieldMeta::Atomic by
removing the associated field when strip is true, while preserving existing
behavior for strip false and FieldMeta::Array. Alternatively, remove the
unsupported Atomic variant and its metadata contract if atomic stripping is
intentionally not supported.
In `@rust/crates/adc-sdk/src/value_diff.rs`:
- Around line 84-92: Update the scalar comparison in the value-diff match, near
the existing Value::Array and Value::Object branches, to special-case
Value::Number pairs using as_f64() so numerically equivalent representations
such as 1 and 1.0 are treated as equal. Preserve the existing l != r comparison
for other value types, and add a regression test covering this equivalence to
prevent a spurious DifferV4 update.
In `@rust/crates/adc-sync-bench/src/main.rs`:
- Around line 138-142: Update the median closure in main to handle even-length
sample slices by averaging the two middle sorted values, while retaining the
current middle-element behavior for odd lengths.
- Around line 63-80: Update the request retry loop around the
`client.delete`/`client.put` calls so successful sends are passed through
`error_for_status()` and HTTP 4xx/5xx responses enter the existing retry
handling. In the `Ok(resp)` branch, handle the result of `resp.bytes().await`
rather than discarding it, retrying body-read failures and returning the final
error after the existing attempt limit.
- Around line 107-121: Validate the parsed concurrency, iterations, and
runtime_flavor arguments before reading the fixture or building the runtime:
require concurrency and iterations to be positive, and reject any runtime label
other than “current” or “multi”. Preserve the existing defaults and runtime
selection for valid inputs, while reporting invalid arguments and exiting
cleanly.
- Around line 55-67: Update send_with_retry and the benchmark workload around
request_path so each diff event also sends the corresponding request for every
inline upstream, matching the TypeScript flow; otherwise explicitly label the
reported metrics as per-event overhead rather than total-sync comparisons.
---
Nitpick comments:
In `@rust/Cargo.toml`:
- Around line 1-7: Update the workspace.package table in Cargo.toml to declare
rust-version = "1.88", then add rust-version.workspace = true to the [package]
table of every workspace member: adc-sdk, adc-differ, adc-sync-bench, and
adc-mock-server.
In `@rust/crates/adc-differ/examples/gen_fixtures.rs`:
- Around line 91-108: Confirm the repository policy for generated fixture
artifacts produced by main, especially the large scale generated by SCALES. If
these JSON files are not intended to be committed, update main to generate them
only on demand or exclude the output directory from version control; otherwise
preserve the generation and explicitly retain the committed fixtures for all
scales and change ratios.
In `@rust/crates/adc-differ/src/differ_v4.rs`:
- Around line 294-317: The resource extraction path unnecessarily clones each
item into ResourceTuple before later per-item processing. Update ResourceTuple
and extract_tuples to borrow Value references with an appropriate lifetime, then
adjust diff_resource and prepare_remote_item’s loops and ownership handling to
consume borrowed items while preserving the existing ID-map and per-item clone
behavior. Record the benchmark baseline before making this optimization.
- Around line 391-421: Refactor merge_default to clone the resource once, then
delegate recursive merging to an in-place helper such as merge_default_into.
Remove per-key cloned existing values and mutate object and array entries
through mutable references; handle null entries without violating Rust’s
mutable-borrow rules, while preserving protected-key filtering and existing
default-merging behavior.
- Around line 262-291: Update diff_plugins to iterate over local and remote
through borrowed object maps, avoiding cloned whole-map values while preserving
the existing empty-value behavior and merged_local construction. Replace the
bidirectional checker calls with one forward comparison for shared/local-only
plugins plus a lightweight remote-key check that only detects remote plugins
absent from merged_local; avoid rerunning diff_value for shared plugins.
- Around line 21-27: Avoid deep-cloning DefaultValue on each recursive
DifferV4::diff call. Change DifferV4 and its diff construction to borrow or
share a single defaults instance, while preserving the existing default fallback
when no value is provided; ensure handle_delete, handle_create, and
handle_update recursive calls reuse that instance.
In `@rust/crates/adc-differ/tests/basic.rs`:
- Around line 10-16: Extract the duplicated config and ev helpers into a shared
tests/common/mod.rs module, preserving their current behavior and making them
public for integration tests. In rust/crates/adc-differ/tests/basic.rs lines
10-16, consumer.rs lines 7-13, custom_id.rs lines 7-13, regression.rs lines
10-12, service_upstream.rs lines 7-13, upstream.rs lines 7-9, and usecase.rs
lines 8-14, remove the local helper definitions, declare mod common;, and import
the helpers with use common::{config, ev}; where ev is used; regression.rs and
upstream.rs require only config.
In `@rust/crates/adc-differ/tests/regression.rs`:
- Around line 63-66: Strengthen the assertions in the DifferV4 regression test
by verifying the single Update event’s resource type, resource ID, and diff
contents for the description field, while retaining the existing count and
event-type checks.
In `@rust/crates/adc-sdk/src/resource.rs`:
- Around line 58-69: Update the public FieldListType documentation to explain
that its four strategies intentionally mirror the corresponding FieldMeta
strategies, preserving consistency with the TypeScript API used by libs/differ.
Add this relationship to the existing enum-level doc comment without changing
the enum variants or behavior.
In `@rust/crates/adc-sdk/src/value_diff.rs`:
- Around line 97-113: Update diff_object and the analogous diff_array traversal
to use one mutable DiffPath, pushing each key or index before deep_diff and
popping it afterward instead of calling path.to_vec() per child. Adjust
deep_diff and change-recording logic so the path is cloned only when
constructing a recorded ValueDiff, while preserving traversal order and path
contents.
🪄 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
Run ID: f87851cb-714f-4c8f-a8fa-f8cef8a1bf25
⛔ Files ignored due to path filters (1)
rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.gitignorerust/Cargo.tomlrust/benches/fixtures/large.few.local.jsonrust/benches/fixtures/large.many.local.jsonrust/benches/fixtures/large.none.local.jsonrust/benches/fixtures/large.remote.jsonrust/benches/fixtures/medium.few.local.jsonrust/benches/fixtures/medium.many.local.jsonrust/benches/fixtures/medium.none.local.jsonrust/benches/fixtures/medium.remote.jsonrust/benches/fixtures/small.few.local.jsonrust/benches/fixtures/small.many.local.jsonrust/benches/fixtures/small.none.local.jsonrust/benches/fixtures/small.remote.jsonrust/crates/adc-differ/Cargo.tomlrust/crates/adc-differ/benches/differ_bench.rsrust/crates/adc-differ/examples/gen_fixtures.rsrust/crates/adc-differ/src/differ_v4.rsrust/crates/adc-differ/src/lib.rsrust/crates/adc-differ/tests/basic.rsrust/crates/adc-differ/tests/consumer.rsrust/crates/adc-differ/tests/custom_id.rsrust/crates/adc-differ/tests/fixtures_sanity.rsrust/crates/adc-differ/tests/regression.rsrust/crates/adc-differ/tests/service_upstream.rsrust/crates/adc-differ/tests/upstream.rsrust/crates/adc-differ/tests/usecase.rsrust/crates/adc-mock-server/Cargo.tomlrust/crates/adc-mock-server/src/main.rsrust/crates/adc-sdk/Cargo.tomlrust/crates/adc-sdk/src/differ_meta.rsrust/crates/adc-sdk/src/event.rsrust/crates/adc-sdk/src/field_meta.rsrust/crates/adc-sdk/src/lib.rsrust/crates/adc-sdk/src/resource.rsrust/crates/adc-sdk/src/utils.rsrust/crates/adc-sdk/src/value_diff.rsrust/crates/adc-sync-bench/Cargo.tomlrust/crates/adc-sync-bench/src/main.rs
1 participant
Description
I've introduced a Rust implementation of the differ algorithm, which delivers a performance improvement of approximately 2–3x or more.
It's currently just a proof-of-concept (PoC) implementation; they were write in idiomatic Rust, some traces of TypeScript syntax remain. It hasn't undergone code review or further optimization.
Performance
In-process benchmarks comparing the Rust port (
adc-differ, via criterion) against the existing TS implementation (@api7/adc-differ, via mitata), on identical synthetic fixtures across three scales and three change-density scenarios. Both sides measure the diff algorithm only (warm, in-process — no CLI/Node startup overhead on either side).~2.3x faster on average, scaling linearly with resource count on both sides (no quadratic behavior in either implementation).
Checklist
Summary by CodeRabbit
New Features
Tests
Performance