fix: prevent rect mutation during tag bin probing

[gminor7]

Fixes issue #66

[greptile-apps]

Greptile Summary

This PR prevents the original rect object from being mutated (specifically rot, x, and y) while the non-exclusive-tag bin-probing loop calls testBin.add(). It does so by passing a shallow spread clone { ...rect, rot:false, data: {...rect.data} } to the test bin instead of the original rect.

• The spread clone is only safe for plain objects implementing IRectangle. When rect is a Rectangle class instance, { ...rect } copies only the private backing fields (_width, _height, …) — the prototype getters (width, height, etc.) are not own enumerable properties and are not copied. testRect.width is therefore undefined, findNode(NaN, ...) never finds a placement, and testBin.add(testRect) always returns undefined, effectively disabling bin-sharing for all such callers.
• Lines 170–171 contain literal + diff-marker characters that were accidentally left in the committed source code.

Confidence Score: 4/5

The fix is safe for plain-object callers (the documented and tested pattern) but introduces a regression for Rectangle class instance callers in the non-exclusive-tag path.

One P1 finding: the spread clone omits prototype getters, silently breaking bin-sharing for Rectangle class instance users. A minor P2 comment-formatting artifact is also present. The core mutation bug is correctly fixed for the predominant plain-object usage pattern, so the risk is bounded to a specific combination (Rectangle instances + non-exclusive tags), but it still needs to be addressed before merging.

src/maxrects-packer.ts — the testRect clone construction at line 167

Important Files Changed

Filename	Overview
src/maxrects-packer.ts	Introduces a shallow-spread clone to avoid mutating rot/x/y during bin probing; works for plain-object rects but breaks Rectangle class instance users because spread omits prototype getters, causing testRect.width to be undefined and the placement test to always fail.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["addArray(rects) — non-exclusiveTag path"] --> B["testBin = bin.clone()"]
    B --> C["For each rect in same-tag group"]
    C --> D{"rect is plain object?"}
    D -- Yes --> E["testRect = { ...rect, rot:false, data:{...rect.data} }\ncopies width, height, x, y ✓"]
    D -- No: Rectangle instance --> F["testRect = { ...rect, rot:false, data:{...} }\ncopies _width, _height ... NOT width/height ✗"]
    E --> G["testBin.add(testRect)\nfindNode(testRect.width + pad, ...) → valid"]
    F --> H["testBin.add(testRect)\nfindNode(undefined + pad = NaN, ...) → always undefined"]
    G -- "undefined (no fit)" --> I["return false — try next bin"]
    G -- "non-undefined (fits)" --> J["continue loop"]
    H --> I
    I --> K["targetBin never found → new bin created for every group"]
    J --> L["All rects fit → add to current bin ✓"]

Loading

_{Reviews (1): Last reviewed commit: "fix: prevent rect mutation during tag bi..." | Re-trigger Greptile}

[greptile-apps]

Spread breaks placement test for Rectangle class instances

{ ...rect } on a Rectangle instance only copies the private backing fields set directly in the constructor (_width, _height, _x, _y, _rot, etc.) — not the prototype getters (width, height, x, y). As a result, testRect.width is undefined, and place() calls findNode(undefined + padding, ...) which returns undefined every time. testBin.add(testRect) therefore always returns undefined, causing every existing bin to be rejected during probing and all rects to be placed in new bins instead of being packed together.

The fix works correctly when callers pass plain objects implementing IRectangle (the pattern shown in all existing tests), but silently breaks bin-sharing for users who pass Rectangle class instances. To cover both cases, extract the getter values explicitly:

const testRect: IRectangle = {
    width: rect.width,
    height: rect.height,
    x: rect.x,
    y: rect.y,
    rot: false,
    data: rect.data ? { ...rect.data } : rect.data,
    tag: (rect as any).tag,
};

[greptile-apps]

Git diff markers accidentally committed into source

Lines 170–171 contain a literal + character (a git diff marker) in the middle of the comment text, e.g. + // Use a cloned rect.... These should be plain comment lines.

Suggested change

	+ // Use a cloned rect for test placement to avoid mutating
	+ // the original rect (especially `rot`) during bin probing
	// Use a cloned rect for test placement to avoid mutating
	// the original rect (especially `rot`) during bin probing