Skip to content

[Maintainer] i18n: locale-neutral reference-scale label and half-away-from-zero rounding - #1164

Merged
Aymericr merged 6 commits into
mainfrom
cursor/i18n-reference-scale-rounding-ae7f
Oct 27, 2026
Merged

Aymericr merged 6 commits into
mainfrom
cursor/i18n-reference-scale-rounding-ae7f

Conversation

@Aymericr

@Aymericr Aymericr commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

Follows up on the Tooling findings from #1162 and its review of this PR:

  • Reference scales persist a locale-neutral English label alongside numeric displayLength and displayUnit. This keeps new scenes readable by published @pascal-app/core 1.0.6, MCP, and CLI while allowing current viewers to format numeric labels with the active locale.
  • Legacy labels matching a number plus m, cm, ft, or in are parsed and reformatted for the viewer locale; unrecognized labels remain unchanged.
  • Metric display and write-back share one half-away-from-zero rounding function, including values immediately below an exact half.
  • Hardened the label parser.

apps/editor/proxy.ts is unchanged.

How to test

  1. Run bun test packages/editor/src/components/editor/floorplan-panel.test.ts packages/editor/src/components/ui/controls/metric-control.test.ts.
  2. Run bun run ci.
  3. Create a reference scale under French, switch to English, and verify 2,75 m displays as 2.75 m while the persisted fallback label remains canonical English.
  4. Verify -1.25 writes and displays as -1.3, while -1.2499999999999998 writes and displays as -1.2.

Screenshots / screen recording

N/A — formatting and persistence correctness changes only.

Checklist

  • I've tested this locally with bun dev
  • My code follows the existing code style (run bun check to verify)
  • I've updated relevant documentation (if applicable)
  • This PR targets the main branch
Open in Web Open in Cursor 

Note

Low Risk
Changes are limited to display formatting, optional schema fields, and backward-compatible label parsing; no auth or data-migration risk beyond existing guide scale references.

Reviewed by Cursor Bugbot for commit 9121db3. Bugbot is set up for automated code reviews on this repo. Configure here.

@Aymericr Aymericr added the bot:maintainer label Oct 27, 2026 — with Cursor
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T01:23:19.693566Z f7c6c32 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7c6c32957

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const factor = 10 ** precision
return Math.round((value + Number.EPSILON) * factor) / factor
return (
(Math.sign(value) * Math.round((Math.abs(value) + Number.EPSILON) * factor)) / factor

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid rounding values just below a negative midpoint up

When drag, wheel, or keyboard arithmetic produces a negative value immediately below a half boundary, adding Number.EPSILON to its absolute value can push it onto the boundary and round it away from zero even though the rendered value rounds toward zero. For example, at precision 1, -1.2499999999999998 is displayed by Intl.NumberFormat as -1.2, but this helper returns -1.3, so write-back can still disagree with the display and introduce an unintended step.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Tooling cross-review at f7c6c32: CHANGES REQUESTED

CI is green on this SHA (quality, cli-smoke, Changed paths, ci), and Bugbot passed. The touched tests and the core schema tests pass locally (733/733), and proxy.ts is unchanged.

What's fixed:

  • Rounding: -1.25 now rounds to -1.3, -0.5 to -1 and -2.5 to -3. Positive results are identical to before: 200k random samples showed 0 differences.
  • New saves: they now store displayLength and displayUnit, which render as 2,75 m in French and 2.75 m in English.

P1: this is a persisted scene-format change. packages/core/src/schema/nodes/guide.ts adds displayLength and displayUnit and makes label optional, and new saves omit label entirely. The published @pascal-app/core 1.0.6 (and the MCP/CLI built on it) requires label: z.string(), so older readers will reject guides saved by this version. Fix: keep writing a locale-neutral label (the English 2.75 m) next to the numeric fields, so old readers still parse the scene and new readers format from the numbers.

P2:

  • Legacy labels: they still render verbatim. The test itself expects 2,75 m in English, and older 2.75 m labels stay 2.75 m in French. Parse ^<number> (m|cm|ft|in)$ when reading and reformat it for the current locale.
  • Codex metric-control.tsx:49: adding Number.EPSILON after abs() can still round -1.2499999999999998 up to -1.3, while the display shows -1.2.

Because this touches the schema, Security's written approval at the final SHA is required before merge, in addition to this review.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a6f8c1c. Configure here.

Comment thread packages/editor/src/components/ui/controls/metric-control.tsx Outdated

Copy link
Copy Markdown
Member Author

Tooling cross-review at 1a2d843: APPROVE

All checks are green on this exact SHA (quality, cli-smoke, Changed paths, ci, Cursor Bugbot with no issues). The touched tests (8/8) and the core tests (6120/6120) pass locally.

  • P1 fixed: I installed the published @pascal-app/core@1.0.6 from npm and parsed a real createGuideScaleReference(...) save with it. It passes GuideScaleReference, GuideNode and AnyNode, with label: "2.75 m" written next to displayLength/displayUnit. Feet, inches, cm and sub-cm saves pass too.
  • Legacy labels: 2,75 m, 2.75 m, 2.75m, NBSP/narrow-NBSP grouping and 1,234.5 ft re-format correctly in en and fr. Kitchen wall, 5' 6", ~2.75 m, 2.75 m (approx) and empty labels are shown unchanged.
  • Rounding: for -1.25, 1.25, -1.2499999999999998 and 1.2499999999999998 at precision 1, the shared helper gives -1.3, 1.3, -1.2 and 1.2, and write-back matches the Intl display in en and fr. In 200k random samples there were 0 mismatches against Intl. The Codex P2 (f7c6c32) and the Bugbot finding (a6f8c1c) are both fixed at head. Codex hasn't re-run on head.
  • Nit (P3, non-blocking): a hand-written 3-decimal legacy label such as 1.234 m reads as 1234 m. The app never wrote these (old labels used toFixed(2)), so this is fine as is.

This changes the persisted scene schema, so merging also needs Security's written approval at 1a2d843.

Copy link
Copy Markdown
Member Author

Tooling cross-review at 9121db3: APPROVE

I reviewed only the diff from 1a2d843. All checks are green on this exact SHA (quality, cli-smoke, Changed paths, ci, and Cursor Bugbot with no issues). The touched tests (11/11) and the core schema tests pass locally, and there are no new inline findings.

  • Compat and regressions: New saves still parse against the published @pascal-app/core@1.0.6. Every old French and English label case and every rounding case from my last pass gives the same result as before.
  • Caps and guards: Real lengths in m, cm, ft and in, from 0.5 up to 1,000,000, pass both the 1.0.6 and head schemas and display correctly in en and fr. Only values above 1,000,000 fail the head schema. Bad displayUnit values (including __proto__), NaN and a null label all fall back safely.
  • Regex: Digit groups are split by a required whitespace character, so the pattern has no ambiguous overlap. On adversarial 100k-character inputs it took under 1 ms per run, and time grows linearly with length. The 64-character cap also stops the parser running on long strings.
  • Non-blocking (P2/P3): The reference-scale input doesn't cap values at 1,000,000, so a value over that would save a guide that fails the head schema. That isn't a realistic room length. Separately, sub-cm values in m show as 0 m because labels round to 2 decimals, which was already the case before this PR.

Merging still needs Security's written approval at 9121db3.

@Aymericr
Aymericr merged commit 03b6f25 into main Oct 27, 2026
5 checks passed
@Aymericr
Aymericr deleted the cursor/i18n-reference-scale-rounding-ae7f branch October 27, 2026 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants