Skip to content

Align tiling scale math with MIN_MODEL_PX invariant and fix mismatched test intent - #9

Closed
matteius with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-code-for-review-comments
Closed

Align tiling scale math with MIN_MODEL_PX invariant and fix mismatched test intent#9
matteius with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-code-for-review-comments

Conversation

Copilot AI commented Aug 3, 2026

Copy link
Copy Markdown

This PR addresses all review-thread comments on PR #8 by tightening tiling scale rounding behavior, removing dead imports, and correcting a test scenario that did not exercise the path it claimed to validate.

  • Tiling region size rounding

    • Updated plan_tiles() to compute region_w / region_h using floor-based rounding where required-scale math is applied, preventing crops from becoming larger than intended due to ceil(...).
    • This preserves the module invariant that a min_object_px target does not drop below MIN_MODEL_PX after resize/letterbox edge rounding.
  • Test correction for overflow/truncation behavior

    • Fixed the test case in tests/test_tiling.py whose input contradicted its own intent (“very small min_object_px explodes the grid”).
    • Adjusted parameters so the test actually drives the high-tile-count / truncation path rather than collapsing to a full-frame tile.
  • Code hygiene

    • Removed unused dataclasses.replace import from utils/tiling.py (including duplicate occurrence), eliminating lint noise.
# before (can overshoot region size)
region_w = math.ceil(model_w / required_scale)

# after (does not exceed required crop size)
region_w = math.floor(model_w / required_scale)

Copilot AI changed the title [WIP] Fix code based on review comments Align tiling scale math with MIN_MODEL_PX invariant and fix mismatched test intent Aug 3, 2026
Copilot AI requested a review from matteius August 3, 2026 19:50
@matteius matteius closed this Aug 3, 2026
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.

2 participants