[giswqs]

Fixes the macOS failure on main (run 39977755541): publication_surrounds_keep_off_the_data got an unexpected "the legend lists more than fits its corner" warning.

The legend's box is sized to its rows' width plus 2 × padding, then the inside is computed as the box width less 2 × padding, which can land one ulp short of the rows' width. Which widths hit that depends on the font, so with macOS's system font the one-layer legend in that test did, and on Linux it didn't. The overflow checks (rows and colorbar) now compare with a 1e-6 mm tolerance.

Regression test a_publication_legend_sized_to_its_rows_fits_them runs the publication layout for 300 layer names of different widths; without the fix it fails on Linux too ("aRb7").

Summary by CodeRabbit

• Bug Fixes
  • Publication layouts now tolerate a negligible measurement difference when checking whether legends and colorbars fit. This prevents warnings when they exceed available space by no more than 0.000001 mm.
• Tests
  • Added coverage verifying that publication legends fit their rows across layouts with varied layer-name widths.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 05712b9c-9fc8-4246-9d16-c2ef6bab7a56

📥 Commits

Reviewing files that changed from the base of the PR and between 6e4f2a2 and c42e94d.

📒 Files selected for processing: 2

• crates/engine/src/cmd/layout.rs
• crates/engine/src/tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Publication-layout legend and colorbar fit checks now allow up to 1e-6 mm of excess before issuing warnings. A new test generates layouts for 300 polygon datasets with varied layer-name widths and checks for “more than fits” warnings.

Changes: Publication layout fit

Layer / File(s)	Summary
Overflow checks and coverage crates/engine/src/cmd/layout.rs, crates/engine/src/tests.rs	The legend and colorbar overflow checks now require excess greater than 1e-6 mm. A test checks 300 generated publication layouts for the “more than fits” warning.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

• opengeos/RustGIS#682 — This change addresses false legend overflow warnings in publication layouts.

Possibly related PRs

• opengeos/RustGIS#742 — This PR changes publication legend sizing and placement, which feeds into the fit checks updated here.
• opengeos/RustGIS#709 — This PR adds publication legend and colorbar layout behavior covered by the updated fit checks.
• opengeos/RustGIS#706 — This PR changes how publication layouts calculate legend room, which is used by the fit checks.

Suggested reviewers: elpidioassinaturas, jgregoriorincon

Merge Risk: ⚪ Minimal · up to c42e9

No actionable merge-blocking risk remains.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: preventing exact-fit publication legends from being reported as overflowing.
Description check		The description explains the problem, cause, solution, and regression testing. It does not use the required Summary, Testing, and Related issue headings, and it does not provide an issue reference, bu…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the problem, cause, solution, and regression testing. It does not use the required Summary, Testing, and Related issue headings, and it does not provide an issue reference, but the required information is mostly present.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 too large.)

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the legend’s space,
And lets a tiny margin pass.
Three hundred maps with names of width,
Now test the fit and warning’s truth.
The colorbar gets room to spare.}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Code review

I reviewed the 2-file diff, which changes crates/engine/src/cmd/layout.rs and adds a test in crates/engine/src/tests.rs. I did not build or run anything.

Bugs

• None found. The FIT_MM = 1e-6 tolerance is applied consistently to all four overflow comparisons (legend width, legend height, colorbar width, colorbar height). 1e-6 mm is far below any visible size and far above f64 rounding at layout scales (tens to hundreds of mm). Real overflows still warn. Confidence: high.

Security

• None. The change adds no new input handling and no unwrap in non-test code.

Performance

• Test cost (low confidence, nit): a_publication_legend_sized_to_its_rows_fits_them creates 300 Sessions and runs 300 layout.publication calls. Each call writes a GeoJSON file, and the layout presumably measures fonts. This is probably fine on Linux, but it could be slow on Windows or macOS CI. Reducing it to about 100 names would probably still catch the regression, since the PR says "aRb7" is among those that fail. Reusing one temp file with the layer renamed would also work.

Quality

• The tolerance is a function-local const declared mid-block. It reads fine, but if other layout checks use similar epsilons (e.g. the rows_h >= 1.0 check just below), a shared module-level constant would be more discoverable. Confidence: low.
• The tolerance papers over the cause: the inner size is computed as the box width minus 2 × padding. Sizing the box from the same values, or comparing against the stored content width, would remove the rounding error instead of tolerating it. The tolerance is still a reasonable minimal fix. Confidence: low.
• The test depends on the font in use, so which names hit the rounding error varies by machine. The 300-name sweep handles this, but it is probabilistic. It is a good regression guard on Linux, as the PR claims it fails without the fix. Confidence: medium.
• The test cleans up its temp dir only at the end, so a failed assertion leaves the directory behind. The directory name is PID-scoped, so this is harmless.

AGENTS.md

• unwrap() appears only in test code, which is allowed.
• The change fits the repo's guidelines (never crash, wasm-safe, no UI strings added). The new warning text is unchanged, so no tl! or i18n impact.
• cargo xtask ci was not run here, so I could not confirm it passes.

Change size

Compared: the whole PR against main (merge base 2502d90 to PR head 6e4f2a2).

Metric	Value
Files changed	2 (added 0, modified 2, removed 0, renamed 0)
Size before	949.0 KB
Size after	950.5 KB
Total size change	+1.5 KB

File	Before	After	Change
crates/engine/src/tests.rs	830.0 KB	831.2 KB	+1.3 KB
crates/engine/src/cmd/layout.rs	119.0 KB	119.2 KB	+204 B

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-749/
Web app	https://preview.rustgis.app/pr-749/app/
Commit	c42e94d

GitHub Pages takes a minute or two to publish a new commit.

[github-actions]

Code review

I found nothing that needs changing. I reviewed the PR statically: I read the diff and the two changed files in full, with no build or test run.

What changed

• crates/engine/src/cmd/layout.rs: the two "legend overflow" warning checks in publication now compare against a FIT_MM = 1e-6 tolerance. One check is for the rows and the other is for the colorbar.
• crates/engine/src/tests.rs: a new regression test, a_publication_legend_sized_to_its_rows_fits_them. It runs the layout for 300 layer names of different widths.

Bugs

• None found. The tolerance is far below any visible size (1e-6 mm) and is applied the same way to every comparison in both warnings. Real overflows, which are far larger than a rounding error, still warn. (High confidence)
• The rows_h >= 1.0 check just below is unchanged. It is a different kind of test, so it does not need the tolerance. (Medium confidence)

Security

• None. The change only affects the thresholds for a warning.

Performance

• None. The new test creates 300 sessions and 300 small files, which should be quick. It does remove its temp directory at the end.

Quality

• Test depends on the font (low severity, medium confidence). The test only fails if some name happens to land on a rounding error with the font in use. The PR says it reproduces on Linux ("aRb7"), which is reasonable evidence. It could still pass vacuously with other fonts, such as a different fontconfig setup. A direct unit test of the comparison would be sturdier, but it is probably not worth the effort.
• Test cleanup (nit, low severity). If an assertion fails, the temp directory is left behind because the remove_dir_all call is never reached. This matches the neighbouring tests' pattern, so it is acceptable.
• unwrap() in the test is fine under the AGENTS.md rules, which only forbid it in non-test code.

AGENTS.md

• The change follows the "never crash" and architecture rules: no unwrap in non-test code, no new dependencies, no UI strings, and no wasm impact.
• It includes a regression test for the fix, as AGENTS.md asks.
• I did not run cargo xtask ci.

Change size

Compared: the whole PR against main (merge base 694e6f8 to PR head c42e94d)

Metric	Value
Files changed	2 (added 0, modified 2, removed 0, renamed 0)
Size before	1003.1 KB
Size after	1004.6 KB
Total size change	+1.5 KB

File	Before	After	Change
crates/engine/src/tests.rs	848.1 KB	849.4 KB	+1.3 KB
crates/engine/src/cmd/layout.rs	155.0 KB	155.2 KB	+204 B