[Mavrick-F]

Summary

Reading a service layer page by page (OGC API Features, or ArcGIS past its record limit) used several times more memory than its data. Reading 100,000 Brentwood, TN parcels (155 MB of GeoJSON) through service.add peaked at 1.27 GB. With this change it peaks at 576 MB.

Where the memory went. I profiled each step separately: the 100,000 parcels replayed offline as 50 ArcGIS pages of 2,000, each step in its own process, measured as the step's peak resident memory.

Step	Brentwood (155 MB)	Fairfax (88 MB)
ogc::read_pages, before	+1,248 MB	+667 MB
ogc::read_pages, after	+323 MB	+183 MB
io::read_bytes (the joined GeoJSON into a dataset), unchanged	+263 MB	+109 MB
layer.addDownload, unchanged	+417 MB	+199 MB

The peak was in Pages, which held every feature as a serde_json::Value tree: about 8 bytes in memory for each byte of text.

The change. Pages now keeps each feature as the JSON text the server sent (Box<RawValue>; the workspace already enables raw_value). Only a page's other members (links, numberMatched, properties, …) are read into values. finish writes those members, then splices the features' text in as it came. read_pages is now about 2× the data, and 4× faster (2.4 s → 0.5 s for Brentwood).

• The joined collection keeps everything the old one kept. The paging members are still removed. Feature text now comes through exactly as the server wrote it: spacing, member order, 1.50, é escapes, and integers too large for a u64, which the old round trip turned into floats. The offline replay's output is JSON-equal to main's for both layers.
• ArcGIS's "first features again" check still compares the first feature as a value, so a server that sends its first page again with different spacing is still caught. Only that one feature per page is parsed.
• Kept as text, a feature isn't subject to serde_json's nesting limit, so a page nested deeper than values allow would get spliced in, and the whole joined document would then fail to load. A small scan (too_deep) rejects such a page, and the read stops there with a note, as it did before.
• Messages are unchanged: a page that isn't JSON "couldn't be read (…)"; JSON that isn't a feature collection "isn't a feature collection".

Not in this PR (both are separate decisions in #830):

• Building the dataset as pages arrive (step 3). The peak is now in layer.addDownload and the GeoJSON read, not the paging. Step 3 would remove the joined text, about 1× the data, from that peak.
• Raising MAX_PAGED_FEATURES (step 4).

No new dependency, and no change to commands, the data model or the project format.

Testing

• cargo xtask ci: passed (1,996 tests, layers ok)

• cargo build --locked --target wasm32-unknown-unknown -p rustgis-web: passed

• New ogc::tests::pages_joined_as_written covers:

  • features that come through byte for byte across two pages, with the paging members gone;
  • a re-sent ArcGIS first page with different spacing, which is still caught;
  • a page nested 130 deep that stops the read, while 120 deep passes and brackets inside strings don't count.

  It fails on main (the text is re-serialized). Disabling the depth guard fails it too. It checks that the features are held as text. It doesn't measure memory: the workspace forbids unsafe, so a test can't install a counting allocator. The numbers above come from a separate harness.

• Live: service.add of Brentwood's county parcels (ArcGIS Enterprise 11.3, 100,000 of 105,117 features, with the cap note) went from 32.7 s and 1,270 MB peak on main to 28.9 s and 576 MB, in the same release build.

Related issue

Part of #830 (steps 1 and 2).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Combined feature responses now preserve the original formatting and numeric representations of features.
  • Duplicate pages are recognized consistently even when JSON whitespace differs.
  • Malformed or excessively nested feature pages are handled using the existing stop behavior.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

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: 5b6d9e44-2a67-4dad-802d-7b2b7d066735

Commits

Reviewing files that changed from the base of the PR and between 6e8a94c and 11c63fe.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

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: 823c2edf-2598-4942-a8ad-865c154c5377

Commits

Reviewing files that changed from the base of the PR and between 9f30da4 and 6e8a94c.

Files selected for processing: 1

• crates/data/src/ogc.rs

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

Walkthrough

Paged feature processing now retains feature objects as raw JSON text. It separates collection metadata from features, compares repeated ArcGIS pages by parsed feature value, and appends retained feature text when joining responses.

Changes: Paged feature text

Layer / File(s)	Summary
Split pages and retain feature text crates/data/src/ogc.rs	Page splitting separates collection metadata from raw feature text. Paging state stores raw features and initializes from the split result.
Compare and join paged features crates/data/src/ogc.rs	ArcGIS repeated-page detection compares first features as parsed values. Joined responses append retained feature text. Tests cover formatting, numeric forms, escapes, large integers, and nesting limits.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Possibly related issues

• [Feature]: Read a service's pages in memory close to the data's size #830: This PR implements the raw-feature-text step described in the issue; it does not include the issue’s other proposed paging changes.
• [Feature]: Read every feature of an ArcGIS FeatureServer or MapServer layer, past its record limit #764: This PR changes the ArcGIS paging path mentioned in the issue, but does not change its maxRecordCount behavior.

Possibly related PRs

• opengeos/RustGIS#823: This PR introduced the read_pages and Pages flow that this change updates.

Suggested reviewers: giswqs

Merge Risk: Minimal · up to 6e8a9

No actionable merge-blocking issue is identified; merge after the normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: retaining paged service features as JSON text to improve memory use.
Description check		The description includes complete Summary, Testing, and Related issue sections. It explains the motivation, implementation, measured results, test coverage, limitations, and scope.
Docstring Coverage		Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 1 files.
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.

Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 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

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit keeps each feature’s text,
Its commas, digits, braces next.
It checks the pages by meaning clear,
Then joins their words without a smear.
Hop, raw JSON, preserved right here!}

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

[github-actions]

Code review

I read the whole diff (one file, crates/data/src/ogc.rs) and the surrounding Pages code. I found no bugs, security problems or guideline violations. I could not run the build or the tests.

Bugs

Nothing found. What I checked:

• Empty collection. If the first page's other members are all removed, head is {}. open.len() > 1 picks "features":[ without a leading comma, so the output stays valid.
• First page. split_page removes features from the members, so the output has no stale empty features array. The old code left one behind and overwrote it.
• Duplicate-first-page detection. first_value re-parses the first feature into a Value on both sides of the comparison. A repeated first page with different spacing is still caught, and pages_joined_as_written covers this.
• Depth. too_deep ignores brackets inside strings, handles escapes, and uses a limit of 127 to match serde_json's Value limit. Reading features as text would otherwise skip that limit and produce a collection that can't be read as a whole. The test covers depths 120 and 130 and a bracket-heavy string.
• Truncation. finish_at_max calls truncate on a Vec<Box<RawValue>>, which is fine.
• Dependency. The workspace serde_json already enables raw_value, so no Cargo.toml or licence change is needed.

Security

Nothing found. The code is still free of unwrap and panic!, and the splice uses strip_suffix with a fallback rather than indexing. The depth guard is kept, so hostile deeply nested pages still stop with an error. RawValue copies are validated JSON, so the spliced output cannot be malformed or injected.

Performance

• Improvement. Features are held as their JSON text, which fixes the memory blow-up from [Feature]: Read a service's pages in memory close to the data's size #830. The output buffer is pre-sized.
• Nit (low confidence). split_page makes several passes over each page: the too_deep scan, the map parse, a re-parse of the features array into Vec<Box<RawValue>>, and a parse of the other members into Value. These passes are linear in page size and cheap next to the memory saved. They could be reduced to one by deserializing into a struct with features: Option<Vec<Box<RawValue>>> and #[serde(flatten)] rest: Map. Flatten with RawValue has caveats, so this is optional.

Quality

• Nit (low confidence). In split_page, a body whose features member is not an array returns Ok(None), the same as "not a feature collection". That matches the old behaviour, so I'm only noting it.
• The test pages_joined_as_written is thorough. It covers spacing, member order, a trailing zero, an escaped character, an integer past u64, and the depth boundary. It uses unwrap in test code, which is allowed.

AGENTS.md

No violations. There is no panic path, no UI text, no new dependency, and nothing that affects wasm. Please still run cargo xtask ci before merging.

Change size

Compared: the whole PR against main (merge base e1e70a9 to PR head 6e8a94c)

Metric	Value
Files changed	1 (added 0, modified 1, removed 0, renamed 0)
Size before	166.6 KB
Size after	173.0 KB
Total size change	+6.4 KB

File	Before	After	Change
crates/data/src/ogc.rs	166.6 KB	173.0 KB	+6.4 KB

[github-actions]

Code review

I read the full diff of crates/data/src/ogc.rs and the surrounding Pages::land, finish and finish_at_max code. I found no bugs. I did not build the code or run the tests.

Bugs

Nothing found. What I checked:

• Depth limit. MAX_DEPTH = 127 matches serde_json's recursion limit, which allows 127 levels of nesting. too_deep skips brackets inside strings and handles \" escapes. The test cases (120 passes, 130 stops, brackets inside strings ignored) are consistent with that.
• Joined output. finish strips the trailing } from the serialized head. It adds a comma before "features" only when other members remain, so {} gives {"features":[…]}. Features are comma-joined, and split_page removes features from the head, so the key can't appear twice. finish_at_max truncates before splicing, so the cap still holds.
• Offsets check. first_value(&more) == *start behaves like the old more.first() == start.as_ref(). The start.is_some() guard means an unparseable or missing first feature can't match by accident.
• Behaviour kept. The paging members are still removed. Error and "isn't a feature collection" messages are unchanged. Non-object JSON, and a features member that isn't an array, still give Ok(None).
• Unavailable-feature edge cases. A null or non-object feature is carried as text, which is how the old code treated it as a value. Duplicate top-level keys collapse in the BTreeMap, as they did when read into values.

Security

No concerns. Hostile input is still bounded by the depth guard, the max feature cap and the existing seen loop check. The new code has no unwrap, expect or unsafe, so it follows the never-crash rule.

Performance

• Peak in finish (low confidence, nit). The output Vec is allocated up front while every Box<RawValue> is still alive, so the feature text briefly exists twice (about 2× the data). That is still far below the old value-tree cost. If more headroom were wanted, finish could free each box as it is copied, but the up-front capacity prevents that, so this is not worth doing here.
• Cost of too_deep (informational). It is one byte pass per page, so it is cheap next to the JSON parse.

Quality

• Double parse (nit). For a non-object JSON page, split_page parses the bytes a second time, with IgnoredAny, to tell "not a collection" from "unreadable". This only happens on an error path and is fine.
• Test coverage. pages_joined_as_written covers byte-for-byte splicing, the re-sent first page with different spacing, and the depth boundary on both sides. It doesn't check memory, as the PR says.

AGENTS.md

• The change is data-layer only and has no new dependency (raw_value was already enabled). It has no UI text, so nothing needs tl!, and it has no wasm or layering impact.
• The PR says cargo xtask ci and the wasm build passed.

Change size

Compared: the whole PR against main (merge base 302288d to PR head 11c63fe)

Metric	Value
Files changed	1 (added 0, modified 1, removed 0, renamed 0)
Size before	166.6 KB
Size after	173.0 KB
Total size change	+6.4 KB

Path	Before	After	Change
crates/data/src/ogc.rs	166.6 KB	173.0 KB	+6.4 KB

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-841/
Web app	https://preview.rustgis.app/pr-841/app/
Commit	11c63fe

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