Claude Code Review Report

Recently, I ran Claude Code across several projects(like image-rs, lofty of symphonia) to look for potential issues.

In this projects, a significant portion of reported findings turned out to be incorrect or minor false positives, but a non-trivial subset was valid and included real issues ranging from incorrect comments to actual logic bugs. As a result, the findings generally require manual validation to separate noise from actionable problems.

Full Reports (findings-interactive.html - interactive report view for manual inspection, findings-short.md - compact list ready to copy into GitHub, findings-table.html - compact tabular report version):

findings-interactive.html
findings-short.md
findings-table.html

Example findings (this is only a subset of the findings, specifically those most likely to be actual bugs; for the full list, see the reports above):

=================================

CPY_1 HIGH

Description: In transform_light_source for SpotLight, the y-coordinate is being computed by subtracting region.x() instead of region.y(). This applies to both light.y and light.points_at_y. The result is that spot light coordinates are translated incorrectly along the y axis, producing wrong lighting whenever the filter region does not start at x==y, leading to mis-rendered specular/diffuse spotlight effects.

Locations:

crates/resvg/src/filter/mod.rs:1078-1085
 1078 |             light.x = point.x - region.x() as f32;
 1079 |             light.y = point.y - region.x() as f32;
 1080 |             light.z *= sz;
 1081 | 
 1082 |             let mut point = tiny_skia::Point::from_xy(light.points_at_x, light.points_at_y);
 1083 |             ts.map_point(&mut point);
 1084 |             light.points_at_x = point.x - region.x() as f32;
 1085 |             light.points_at_y = point.y - region.x() as f32;

Fix: Replace region.x() with region.y() on both line 1079 (light.y = point.y - region.y() as f32;) and line 1085 (light.points_at_y = point.y - region.y() as f32;). Compare with the PointLight branch (line 1070) which uses region.y() correctly.

CPY_3 HIGH

Description: In default_fallback_selector, when picking a display family name for the fallback face, the unwrap_or fallback uses &base_face.families[0] instead of &face.families[0]. This means the warn message can report the base face's family name as the new fallback family. Classic copy-paste error from the block immediately above (lines 171-175).

Locations:

crates/usvg/src/text/mod.rs:177-181
  177 |                 let new_family = face
  178 |                     .families
  179 |                     .iter()
  180 |                     .find(|f| f.1 == fontdb::Language::English_UnitedStates)
  181 |                     .unwrap_or(&base_face.families[0]);

Fix: Change .unwrap_or(&base_face.families[0]) in the new_family block to .unwrap_or(&face.families[0]).

LOGIC_2 HIGH

Description: In morphology::apply, the kernel size is computed as min(rx.ceil()*2, src.width) instead of min(rx.ceil()*2 + 1, src.width). The SVG spec requires the morphology kernel to be (2*radiusX+1) x (2*radiusY+1) (a symmetric kernel centered on the pixel). For example, with rx=1 the kernel should be 3 pixels wide, but the current code only iterates 2 columns, dropping the right-most neighbor and producing asymmetric, biased dilation/erosion.

Locations:

crates/resvg/src/filter/morphology.rs:17-18
   17 |     let columns = std::cmp::min(rx.ceil() as u32 * 2, src.width);
   18 |     let rows = std::cmp::min(ry.ceil() as u32 * 2, src.height);

Fix: Change to let columns = std::cmp::min(rx.ceil() as u32 * 2 + 1, src.width); and let rows = std::cmp::min(ry.ceil() as u32 * 2 + 1, src.height); so the kernel is symmetric per the SVG spec.

PANIC_2 HIGH

Description: collect_normals indexes path.points()[0] to get the initial move-to coordinates. If the input path has zero points (e.g., a malformed or degenerate textPath target), this panics. The function is reachable from resolve_clusters_positions_path for any text-on-path.

Locations:

crates/usvg/src/text/layout.rs:756-757
  756 |     let mut prev_mx = path.points()[0].x;
  757 |     let mut prev_my = path.points()[0].y;

Fix: Replace with let first = path.points().first().copied(); if first.is_none() { return vec![None; clusters.len()]; } let first = first.unwrap(); (or guard with an early return).

PERF_5 MEDIUM

Description: Inside the fallback loop, fontdb.face(base_font_id)? is called on every iteration even though the result is invariant for the duration of the loop. For a large fontdb.faces() set this performs O(n) redundant lookups.

Locations:

crates/usvg/src/text/mod.rs:158-159
  158 |                 // Check that the new face has the same style.
  159 |                 let base_face = fontdb.face(base_font_id)?;

Fix: Hoist the lookup out of the loop: compute let base_face = fontdb.face(base_font_id)?; once before iterating.

Start by inspecting the reported locations in crates/resvg/src/filter/mod.rs and morphology.rs, then crates/usvg/src/text/mod.rs and text/layout.rs. Compare each finding with the surrounding branches and the cited SVG behavior before changing anything. Done means the five reported bugs and the fallback-loop inefficiency are validated, addressed where confirmed, and project validation passes.