test(engine): the round trip checks that long digit strings typed as numbers keep their digits by Mavrick-F · Pull Request #1932 · opengeos/RustGIS

Mavrick-F · 2026-11-13T11:51:49Z

Summary

A follow-up to #1904, from review there: the never-silent round trip now makes long digit strings and checks that a reader that types them as numbers keeps every digit.

Generator. Text values include identifiers past i64 (12345678901234567891, 12345678901234567000, -99999999999999999999), 2^53 + 1, and a decimal past 17 significant digits.
Oracle. "Text became a number" is a declared change for CSV, KML and GPX only when the number keeps the text's digits: a whole number exactly, a decimal at the precision of its digits, as the readers type text since fix(data): long digit strings in CSV, KML and GPX stay text instead of being rounded #1924. Before, any text that parsed to the same f64 passed, so a rounded identifier would have been allowed.

Testing

Against the reader before fix(data): long digit strings in CSV, KML and GPX stay text instead of being rounded #1924 (its io.rs put back), seed 1 fails at once: csv: changed without a notice: ["text changed to number (a: Text(\"0.12345678901234567891\") → Float(0.12345678901234568))"].
With the current reader, seeds 1–6 pass at 500 cases.
cargo fmt, and the fork's CI.

Related issue

None.

Summary by CodeRabbit

Tests
- Expanded round-trip coverage for text values containing large integers and decimals, including cases where numeric conversion may alter digits.
- Refined checks to distinguish conversions that preserve digits from those that do not.

…ped number keeps them Text a reader types as a number is a declared change only when the number keeps every digit (a whole number exactly, a decimal at its own precision), as the readers now type text (opengeos#1924). The generator makes identifiers past i64 and decimals past 17 digits. Against the reader before opengeos#1924, seed 1 fails at once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

coderabbitai · 2026-11-13T11:52:15Z

No actionable comments were generated in the recent review.

Walkthrough

The roundtrip property test adds large integer and decimal text cases. It checks whether parsed floating-point values preserve the source digits before classifying text as converted to a number.

Changes: Roundtrip precision checks

Layer / File(s)	Summary
Generate and classify numeric text `crates/engine/tests/never_silent_roundtrip.rs`	The generator adds large integers and decimals. A new `keeps_digits` check evaluates whether numeric conversions retain their digits, and the classification uses that result alongside numeric parsing.

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

Change: Other

Merge Risk: Minimal · up to 71d94

This change only extends a roundtrip test with long numeric-text cases and a digit-preservation check. It does not touch production behavior, and no merge-blocking risk remains.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title uses the conventional test(scope) prefix and names the main change: the round-trip check now verifies that long digit strings typed as numbers keep their digits. It matches the generator and…
Description check		The description follows the template sections: Summary, Testing, and Related issue are filled in. The UI evidence section is missing. The template requires it, but the author did not keep the required…
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: Title check

Explanation

The title uses the conventional test(scope) prefix and names the main change: the round-trip check now verifies that long digit strings typed as numbers keep their digits. It matches the generator and oracle changes in the summary and objectives. The title is long at 94 characters, but it is specific and accurate.

Full details: Description check

Explanation

The description follows the template sections: Summary, Testing, and Related issue are filled in. The UI evidence section is missing. The template requires it, but the author did not keep the required line 'No visible UI change.' Here the line is present, so the section is handled. The Summary explains the generator and oracle changes and why they matter. Testing lists a failing pre-fix run, a passing run at 500 cases across seeds 1-6, and cargo fmt with CI. Related issue is answered as None. The description is complete.

Finishing Touches

Generate unit tests (beta)

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

_{A rabbit checks each digit in flight
Large numbers hop into the test
Decimals keep their places bright
A parsed value faces the rest
The roundtrip lands with digits right
Then carrots crown the careful quest}

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: `fbe09fe6-0c1f-41c6-8c55-f8a386145734`

Commits

Reviewing files that changed from the base of the PR and between `ff056d8` and `71d9413`.

Files selected for processing: 1

`crates/engine/tests/never_silent_roundtrip.rs`

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

github-actions · 2026-11-13T11:53:51Z

        (Value::Text(x), Value::Null) if x.trim().is_empty() => "blank text became null".into(),
        (Value::Text(x), Value::Text(y)) if x.trim() == y.trim() => "text trimmed".into(),
-        (Value::Text(x), _) if reads_as(x, b) => "text became a number".into(),
+        (Value::Text(x), _) if reads_as(x, b) && keeps_digits(x, b) => "text became a number".into(),


Small diagnostic gap, not a correctness problem. When text reads as the same f64 but keeps_digits is false (e.g. 12345678901234567000 read back as Float), the arm falls through and the failure reads as text changed to float, which doesn't say why it failed. Naming that case in its own arm makes the failure actionable for whoever hits it:

Suggested change

(Value::Text(x), _) if reads_as(x, b) && keeps_digits(x, b) => "text became a number".into(),

(Value::Text(x), _) if reads_as(x, b) && keeps_digits(x, b) => "text became a number".into(),

(Value::Text(x), _) if reads_as(x, b) => "text became a number that drops digits".into(),

This is a quality point; it can wait for the release quality pass if you prefer.

Left for the release quality pass: the failure already shows the text and the number (Text("…") → Float(…)), which says what was lost; a more specific label is wording.

github-actions · 2026-11-13T11:54:01Z

Code review

Bugs: None found. I traced keeps_digits against double_holds in crates/data/src/io.rs and against the five new generator values (12345678901234567891, 12345678901234567000, 9007199254740993, 0.12345678901234567891, -99999999999999999999). Each is typed as Text by the reader and rejected by the oracle in the same way, and the trimming and precision logic matches. Confidence: high for the logic. The tests were not run in this review, so the PR's claim that they pass is unverified here.

Security: None found. The change is test-only and adds no input handling outside the test.

Performance: None found. The new strategy arm adds a small fixed sample set.

Quality:

The failure for "reads as the same f64 but drops digits" falls through to text changed to float, which doesn't say why it failed. A dedicated arm would fix that. Posted inline with a suggestion (confidence: high that it's a gap, low impact). It can wait for the release quality pass.
keeps_digits mirrors the reader's double_holds digit rule, so a mistake in that formula would appear in both and pass. The oracle still checks the stored value, so this is a limit rather than a hole (confidence: low).

AGENTS.md: Compliant. No unwrap/expect/panic in the added code, the change is in a test, and the never-silent rule is honored by the oracle now rejecting digit-dropping numbers.

Low-confidence notes (not posted):

want.is_empty() returns true for non-finite text such as inf, so the oracle would accept it as a number without a digit check. The generator never produces such text, and the reader types non-finite values as Text, so this is latent (confidence: low).
keeps_digits treats an empty whole-number string (a bare + or -) as a whole number, while double_holds requires it to be non-empty. reads_as gates the call, so there is no practical effect (confidence: high, no impact).

Already raised: 0. There were no earlier review threads on this PR.

Change size

Compared with the base branch (the whole PR: merge base ff056d8 to PR head 71d9413).

Measure	Value
Files changed	1 (added 0, modified 1, removed 0, renamed 0)
Size before	23.8 KB
Size after	25.0 KB
Total size change	+1.2 KB

Path	Before	After	Change
`crates/engine/tests/never_silent_roundtrip.rs`	23.8 KB	25.0 KB	+1.2 KB

github-actions · 2026-11-13T12:02:41Z

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-1932/
Web app	https://preview.rustgis.app/pr-1932/app/
Commit	`71d9413`

GitHub Pages takes a minute or two to publish a new commit. Newer commits from the fork aren't published until a maintainer adds the preview label (once per commit).

github-actions Bot reviewed Nov 13, 2026

View reviewed changes

coderabbitai Bot approved these changes Nov 13, 2026

View reviewed changes

Mavrick-F merged commit 9e6cf81 into opengeos:main Nov 13, 2026
18 checks passed

Navigation Menu

Repository navigation

test(engine): the round trip checks that long digit strings typed as numbers keep their digits - #1932

test(engine): the round trip checks that long digit strings typed as numbers keep their digits#1932
Mavrick-F merged 1 commit into
opengeos:mainfrom
Mavrick-F:test/roundtrip-long-digits

Mavrick-F commented Nov 13, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Nov 13, 2026 •

edited

Loading

Uh oh!

github-actions Bot Nov 13, 2026

Uh oh!

Mavrick-F Nov 13, 2026

Uh oh!

github-actions Bot commented Nov 13, 2026

Uh oh!

github-actions Bot commented Nov 13, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	(Value::Text(x), _) if reads_as(x, b) && keeps_digits(x, b) => "text became a number".into(),
	(Value::Text(x), _) if reads_as(x, b) && keeps_digits(x, b) => "text became a number".into(),
	(Value::Text(x), _) if reads_as(x, b) => "text became a number that drops digits".into(),

Conversation

Mavrick-F commented Nov 13, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Related issue

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Nov 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

Mavrick-F Nov 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 13, 2026

Code review

Change size

Uh oh!

github-actions Bot commented Nov 13, 2026

📖 Docs preview

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Mavrick-F commented Nov 13, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Nov 13, 2026 •

edited

Loading