[Mavrick-F]

Summary

Before, an ArcGIS FeatureServer or MapServer layer stopped at the server's maxRecordCount, often 1,000 or 2,000 features, and RustGIS said "the rest aren't read". After, RustGIS reads the rest page by page, up to 100,000 features. That works in the desktop app, in the browser, and through service.add.

Along the way I found that #519's OGC API paging only ran inside the blocking service.add command. When you add a layer from the Add From Service dialog or the Connections tree, the app downloads it with ui.addUrl and layer.addDownload, and that path read only the first page, on the desktop as well as on the web. This PR pages that path too, so OGC API collections added from the app now get every page as well.

How it works

• ogc::read_pages is now a small state machine with no network code of its own. It returns Step::Fetch(Pages) (the next address, plus the page it was found on) or Step::Done(bytes, note). Two drivers run the same logic:
  • service.add and download_features keep their blocking loop, with the byte budget and the one-minute limit. join_pages is a blocking wrapper around the same state machine, so fix: OGC API Features layers read every page of a collection #519's tests still cover it.
  • The app's download poll (serviceui::read_pages and poll_pages) fetches each further page off the frame through cache::download_limited, which works in both builds. Each address is checked against the page it came from, the pages share the download's byte budget, and the layer is added once the read is done. A read whose project was closed meanwhile is dropped with the same notice as an ordinary download.
• OGC API and WFS pages follow their next link, as before.
• ArcGIS: if a query's answer sets exceededTransferLimit, RustGIS reads the layer's description (…/FeatureServer/0?f=json, with the query's token), then reads the layer again from offset 0, in pages of resultOffset and resultRecordCount ordered by object id. It restarts because the first page, requested in no particular order, can hold different features from the ordered first page. On Hawaii's zoning MapServer they shared only 964 of 1,000, and continuing from offset 1,000 read 36 features twice and missed 36. The restart costs one request per layer. If paging fails before an ordered page arrives, RustGIS keeps the original first page and its note.
  • advancedQueryCapabilities.supportsPagination must be true. Otherwise, including servers before 10.3 (which don't report the flag) and descriptions that can't be read, RustGIS keeps the first page and its existing "record limit" note.
  • maxRecordCount is the page size if it's between 1 and 100,000. A hostile value (0, negative, fractional, huge, a string) falls back to the size of the first page.
  • objectIdField, or the field typed esriFieldTypeOID, becomes orderByFields. MapServer layers usually give only the latter: none of the three live MapServers I tested set objectIdField. The name is only used if it's plain, because it goes into the address. An orderByFields already in the query is kept.
• Guards: if a server ignores resultOffset and sends the first ordered page again, that page is dropped with a note. A page that adds nothing would ask for itself again, which the existing loop guard catches. A query that already sets resultOffset or resultRecordCount is read exactly as written.
• The services' notes no longer say the web app reads only one page.

Not in this PR: service group import and refresh (service.import, service.refresh) still read one answer per layer.

No new dependency, and no change to command ids or the project format.

Testing

• cargo xtask ci: passed (1,955 tests, layers ok)
• cargo build --locked --target wasm32-unknown-unknown -p rustgis-web: passed

Live servers. I checked each layer against the server's own returnCountOnly count, and that every object id appears exactly once:

Path	Layer (server, record limit)	Read
service.add (CLI)	Hawaii County zoning (ArcGIS Server 11.5 MapServer, 1,000)	7,721 of 7,721
service.add (CLI)	Eau Claire County, WI zoning (ArcGIS Server 11.5 MapServer, 2,000)	29,768 of 29,768
service.add (CLI)	Brentwood, TN county parcels (ArcGIS Enterprise 11.3 FeatureServer, 2,000)	100,000 of 105,117, with the cap note
service.add (CLI)	Fairfax County parcels (ArcGIS Online, 2,000)	100,000 of 369,542, with the cap note, in 54 s
Desktop app, Add From Service, Add clicked (ui_shot)	Hawaii	7,721 of 7,721
Web app (release Trunk build in Chromium)	Hawaii and Eau Claire at once	7,721 and 29,768, all
Web app	Brentwood	100,000, with the cap note in the status bar

I didn't find a live server without pagination to test the fallback against. That case is covered by the recorded answers below. Fairfax took 54 s of the one-minute limit that blocking commands have, so on a slower connection service.add may stop at that limit first, with its own note. The app path has no such limit.

Tests. The two integration tests use the shared fixture server in tests/support/. Its routes can now match a path together with its query, so each page gets its own answer, and the tests assert the exact sequence of requests. tests/support/ogc.rs gains arcgis_page and arcgis_layer.

• New smoke::service_features_are_read_page_by_page_off_the_frame drives the app's ui.addUrl {"service": true} path. It covers an ArcGIS layer whose first page is unordered, which reads 0–4 in order from offsets 0, 2 and 4, and an OGC API collection in two pages, with no "aren't read" note.
• New tests::arcgis_features_read_past_the_record_limit does the same through service.add, plus a layer that can't page: it keeps its first page and its note, and its description is the only extra request.
• With the restart removed, the data, engine and app tests each fail with [3, 0, 2, 3, 4]. With the app's paging branch removed, the app test reads the first page only.
• New ogc::tests::arcgis_pages_joined_from_offsets uses recorded answers to cover:
  • an unordered first page read again from offset 0 into three ordered pages, with the exact page addresses, the token carried and the paging flag removed;
  • the feature limit reached;
  • hostile maxRecordCount values;
  • a layer without pagination, a pre-10.3 layer and an unreadable description, each returning the first page unchanged with its note;
  • a server that ignores resultOffset; an empty or failing first ordered page, which falls back to the first page; a later failing page, which keeps what was read and adds a note;
  • addresses that aren't ArcGIS queries, and queries that set their own window.
• New ogc::tests::arcgis_page_addresses covers the page and layer-description addresses and how the object-id field is chosen.
• fix: OGC API Features layers read every page of a collection #519's pages_joined_up_to_a_limit and ogc_api_features_follow_next_pages pass unchanged on the state machine.

Related issue

Closes #764

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Service imports now follow OGC API next-page links and can read ArcGIS feature layers page by page, up to 100,000 features.
  • ArcGIS paging is available when the layer supports it; otherwise, imports stop at the server’s record limit.
• Improvements
  • Service imports report when some features remain unread. Commands such as service.add wait up to one minute for page-by-page downloads.

[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: da0714ea-0ba6-41a9-b74b-ea347a555922

Commits

Reviewing files that changed from the base of the PR and between 45b28ec and e0a9c4d.

Files selected for processing: 9

• crates/data/src/ogc.rs
• crates/engine/src/cmd/service.rs
• crates/engine/src/tests.rs
• crates/ui-egui/src/lib.rs
• crates/ui-egui/src/serviceui.rs
• crates/ui-egui/tests/smoke.rs
• docs/user-guide/data-formats.md
• tests/support/ogc.rs
• tests/support/ogc_server.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

The change adds bounded ArcGIS offset pagination to shared page traversal. Command and UI service downloads use the traversal for ArcGIS pages and OGC API next links. Tests and service-format notes cover the updated behavior.

Changes: Service feature pagination

Layer / File(s)	Summary
Shared paging state and behavior crates/data/src/ogc.rs	The shared reader now detects eligible ArcGIS truncation, checks layer pagination support, and reads offset pages within the feature limit. It retains linked-page traversal and adds tests for pagination, limits, and failure cases. Service notes describe the updated page limits.
Command download integration crates/engine/src/cmd/service.rs, crates/engine/src/tests.rs, tests/support/ogc.rs, tests/support/ogc_server.rs	Command downloads now drive the page state machine and validate each page URL against its referrer. Tests cover supported and unsupported ArcGIS pagination, with fixtures that match requests by query string.
Background UI downloads crates/ui-egui/src/lib.rs, crates/ui-egui/src/serviceui.rs, crates/ui-egui/tests/smoke.rs	The UI tracks service page reads, fetches subsequent pages within the remaining byte budget, and adds completed downloads. Smoke tests cover ArcGIS offset pages and OGC API next links.

Priority: Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant UiState
  participant PollPages as serviceui::poll_pages
  participant Pages
  participant GoOn as serviceui::go_on
  participant HTTP
  participant AddLanded as add_landed
  UiState->>PollPages: poll pending page reads
  PollPages->>Pages: land downloaded response
  Pages-->>GoOn: return Fetch or Done
  GoOn->>HTTP: validate and fetch next-page URL
  HTTP-->>PollPages: return page response
  GoOn->>AddLanded: add completed download

Loading

Suggested reviewers: giswqs, dekhtiarjonathan

Merge Risk: Minimal · up to e0a9c

The change makes ArcGIS and OGC API layers read past their per-request record limit, up to 100,000 features, in both commands and the app. Failures fall back to the first page with a note. Tests cover paging, the fallbacks and the app path, and no concrete merge-blocking risk remains.

Pre-merge checks: 4 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 7 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: reading ArcGIS layers beyond their server record limit in the app and browser.
Description check		The description includes the required Summary, Testing, and Related issue sections. It explains the implementation, scope, fallback behavior, test coverage, validation results, and linked issue.
Linked Issues check		Directly linked issue #764 is the only active target. Its coding requirements are met at the reviewed head. 1. ArcGIS paging past the record limit. crates/data/src/ogc.rs read_pages starts ArcGIS…
Out of Scope Changes check		Each changed area connects to #764 or to the paging behavior it requests. - crates/data/src/ogc.rs: the ArcGIS paging, the read_pages/Pages/Step state machine, and join_pages as a blocking …

Full details: Linked Issues check

Explanation

Directly linked issue #764 is the only active target. Its coding requirements are met at the reviewed head.

1. ArcGIS paging past the record limit. crates/data/src/ogc.rs read_pages starts ArcGIS paging only when a /query answer sets exceededTransferLimit. It then requests resultOffset and resultRecordCount pages. service.add (crates/engine/src/cmd/service.rs download_features) drives the same state machine with the existing MAX_PAGED_FEATURES, byte budget (left), and BLOCKING_TIMEOUT. A feature cap or budget overrun keeps the read features and adds the note the first {max} features are read; the rest aren't.
2. Fallback without pagination. arcgis_paging returns None unless advancedQueryCapabilities.supportsPagination is true. Pre-10.3 servers, unreadable descriptions, and non-positive or non-numeric maxRecordCount values fall back to the first page with its existing note. A maxRecordCount outside 1 to MAX_PAGED_FEATURES falls back to the first page size.
3. Recorded-answer tests. The PR reports ogc::tests::arcgis_pages_joined_from_offsets (joined pages, feature cap, hostile maxRecordCount, no-pagination and unreadable-description fallbacks), plus engine and app integration tests. I did not re-run them.

The issue's open question on browser paging is answered. The same state machine runs in the app path (crates/ui-egui/src/serviceui.rs read_pages/poll_pages) through fetch_into_limited. That path keeps the shared byte budget and per-page address checks.

Limits of this assessment. The tail of the ogc.rs diff, which holds finish, stop, and finish_at_max, was truncated. I relied on the diff head, the summary, and the service.rs diff for those behaviors. The app path has no one-minute limit. The PR states this, and the issue's timeout requirement applies to the blocking commands, which keep it.

Full details: Out of Scope Changes check

Explanation

Each changed area connects to #764 or to the paging behavior it requests.

• crates/data/src/ogc.rs: the ArcGIS paging, the read_pages/Pages/Step state machine, and join_pages as a blocking wrapper. These implement #764. The ogc_api note now reads the same on every target. This follows from the browser paging the issue raises.
• crates/engine/src/cmd/service.rs: download_features drives the state machine and keeps its budget and timeout. This is the blocking path the issue names.
• crates/ui-egui/src/lib.rs and serviceui.rs: the app download path pages layers from the Add From Service dialog and the Connections tree. The issue targets ArcGIS layers users add, and the issue's browser question covers this path. The PR states that OGC API collections added from the app are also paged. That is a consequence of the shared state machine and a fix to the #519 gap, not an unrelated feature.
• Tests (crates/engine/src/tests.rs, crates/ui-egui/tests/smoke.rs, tests/support/ogc.rs, tests/support/ogc_server.rs): these support the new behavior. The route matching change lets the fixture return a different answer for each page.
• docs/user-guide/data-formats.md: documents the new limits and the one-minute note.

Service group import and refresh (service.import, service.refresh) are not changed, as the PR states.

No unrelated change was found in the summaries or the inspected diffs.

Full details: Docstring Coverage

Explanation

Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 7 files. (2 skipped: 1 unsupported, 1 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the pages in rows,
Then hops where each next query goes.
The features gather, ordered right,
Until the cap ends their long flight.
The map gets pages, one by one,
And rests when all the reads are done.}

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

[github-actions]

Code review

I read the whole diff and the surrounding source. I found no confirmed bugs. I couldn't run the build or the tests (no Bash tool), so the new tests are unexecuted.

What I checked

• Paging state machine (crates/data/src/ogc.rs): the split of join_pages into read_pages, Pages::land and Step, covering the Describe, Linked and Offsets phases.
  • Looping is bounded by the seen set and the max cap.
  • An empty page that still says exceededTransferLimit re-requests the same offset, and the seen check ends it with a note.
  • A server that ignores resultOffset is caught by comparing against the first feature of the first page.
  • If paging fails before any feature is read, the first page comes back unchanged, so the "record limit" note still works.
• Hostile input: maxRecordCount is range-checked (0, negative, float and string values all fall back), and the orderByFields name is limited to [A-Za-z0-9_] and 128 bytes, so it can't inject query parameters. There is no unwrap, expect or indexing outside tests.
• Download budgets and address checks: the blocking path in crates/engine/src/cmd/service.rs and the new UI path in serviceui.rs both pass the previous page as the referrer to address::check. Both subtract fetched bytes from the download budget.
• Wasm: the web app now follows next links for OGC API too, and the "web app reads the first page" caveat was dropped from the note and the docs. The UI code uses only wasm-safe APIs (fetch_into_limited, the Slot type, Duration).
• Tests: engine, data unit tests and UI smoke tests cover the paged, unpaged, hostile-description, ignored-offset, mid-stream failure and "window requested" cases. The test servers matched the new route logic.

Quality (low confidence, optional)

• Description-fetch errors are silent (≈40%). In Pages::land, any failure of the layer-description request (a network error, a 4xx, bad JSON) is treated the same as "paging unsupported". The user then gets only the generic "record limit" note and can't tell a transient error from an unsupported server. Keeping the error text in the note would help diagnosis.
• Misleading note at exactly the cap (≈30%). When the feature count reaches max exactly on the last page, finish_at_max still says "the rest aren't read" even if nothing remains. That's harmless, but the wording can mislead.
• Overlong doc comment (≈60%). The read_pages doc comment in crates/data/src/ogc.rs has one line well past the file's wrap width ("from the start, since the first page, asked for in no order, …"). rustfmt won't rewrap it, so wrap it by hand.
• Memory held during paging (≈25%). PageRead and Phase::Describe keep the full first page's bytes alive while paging (up to the download cap). It is bounded, so this is not a bug, just something to know.

AGENTS.md

• Nothing violated that I could see:
  • New interface text is wrapped in i18n::fmt.
  • Layering is respected: serviceui uses rustgis_engine::data::ogc.
  • The crate-root deny block still applies.
  • Docs are updated.
• I couldn't check that cargo xtask ci passes.

Bugs

None found.

Security

None found. Paged and described URLs go through address::check with the referrer, and no user-controlled text reaches the URL unvalidated except the already-raw token.

Performance

None found. Reading the layer again from offset 0 costs one extra first-page request, and the code comments explain why that's needed.

Change size

Compared: the whole PR against main (merge base e7dad1e to PR head e0a9c4d)

Metric	Value
Files changed	9 (0 added, 9 modified, 0 removed, 0 renamed)
Size before	1.42 MB
Size after	1.45 MB
Total size change	+31.3 KB

File	Before	After	Change
crates/data/src/ogc.rs	147.2 KB	166.6 KB	+19.4 KB
crates/ui-egui/src/serviceui.rs	10.3 KB	13.8 KB	+3.5 KB
crates/engine/src/tests.rs	888.7 KB	891.8 KB	+3.1 KB
crates/ui-egui/tests/smoke.rs	143.3 KB	146.2 KB	+2.8 KB
tests/support/ogc.rs	2.1 KB	3.2 KB	+1.1 KB
crates/ui-egui/src/lib.rs	187.4 KB	188.0 KB	+603 B
crates/engine/src/cmd/service.rs	25.4 KB	25.8 KB	+402 B
tests/support/ogc_server.rs	6.7 KB	7.0 KB	+285 B
docs/user-guide/data-formats.md	38.9 KB	39.1 KB	+201 B

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-823/
Web app	https://preview.rustgis.app/pr-823/app/
Commit	e0a9c4d

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