[choupeanut]

Summary

Chunked GeoTIFF reads now report progress and return DataError::Cancelled when their caller cancels. Previously the serial decimation loop did neither, and parallel decoders could not see the caller's thread-local progress probe.

• Check cancellation before allocating the serial output and between chunks on both paths.
• Count all chunks examined, including chunks skipped while making an overview, so successful reads reach their total.
• Capture the caller's probe and install it on each parallel worker, restoring the previous probe when work ends. Worker cancellation uses the existing shared failure path, so partial raster data is not returned as success.
• Keep the decoding, placement, thread-count and memory-budget policies unchanged. An individual chunk's decoding and placement still runs to completion; cancellation is observed between chunks.

No dependencies, public command parameters or plugin interfaces changed. This is independent of the LERC implementation in #1984. No other project's code was followed or copied.

Testing

• Regression test chunked_reads_count_progress_and_stop_before_or_during_serial_and_parallel_work: all 12 combinations passed (serial/parallel, full-resolution/overview, successful/pre-cancelled/cancelled after progress).
• The parallel cases require a worker to report the first progress item before the caller can decode, proving that the thread-local probe reaches workers. Successful reads check exact pixel output, sample type, rounding counts and completed progress totals.
• cargo xtask ci: passed (fmt, Clippy with -D warnings, workspace tests: 6,396 passed / 23 ignored, layers).
• pre-commit run --all-files: all hooks passed.
• CARGO_INCREMENTAL=0 cargo build --locked --target wasm32-unknown-unknown -p rustgis-web: passed.

UI evidence

No visible UI change.

Related issue

Closes #2015.

Summary by CodeRabbit

• Improvements
  • Chunked GeoTIFF reads now provide progress updates that account for all examined chunks, including those skipped during decimation.
  • Cancellation is checked during both serial and parallel reads, so processing can stop more responsively.
  • Progress completion and cancellation behavior is covered across both reading modes.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Chunked GeoTIFF reads now initialize progress, count examined chunks, and check for cancellation in serial and parallel processing. Parallel workers install the caller’s progress probe. Tests cover completion counts and cancellation.

Changes: Chunked GeoTIFF Reads

Layer / File(s)	Summary
Serial chunk progress crates/data/src/raster/geotiff.rs	Serial decimation initializes progress with the total chunk count, checks for cancellation before and after processing, and advances progress for every examined chunk, including skipped chunks. The documentation describes these progress and cancellation boundaries.
Parallel probe handoff and validation crates/data/src/progress.rs, crates/data/src/raster/geotiff.rs	The new current() accessor clones the caller’s installed probe. Parallel decimation installs that probe in workers, counts skipped and processed chunks, and checks for cancellation during setup, processing, and before return. Progress-step failures follow the worker failure path. Tests cover serial and parallel counts and cancellation.

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: mavrick-f, giswqs

Merge Risk: Minimal · up to 008b0

Chunked GeoTIFF reads now report progress and stop on cancellation in both serial and parallel modes. No concrete merge-blocking risk was identified, and the change is easy to revert.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: chunked GeoTIFF reads now support cancellation and progress reporting. The fix(data) prefix matches the scope. The changeset adds cancellation checks and progress coun…
Linked Issues check		Directly linked issue #2015 has coding requirements: (1) serial decimate must call progress::begin and progress::step per chunk and stop on cancel; (2) decimate_parallel must hand the caller's…
Out of Scope Changes check		All changes reported for the whole PR (merge base 6f68c9c to head 008b0d8) are in crates/data/src/progress.rs and crates/data/src/raster/geotiff.rs. The progress.rs change adds one crate-priva…
Description check		The description follows the repository template. It has Summary, Testing, UI evidence, and Related issue sections. The Summary states what changed and why: chunked GeoTIFF reads now report progress an…

Full details: Title check

Explanation

The title names the main change: chunked GeoTIFF reads now support cancellation and progress reporting. The fix(data) prefix matches the scope. The changeset adds cancellation checks and progress counting in the serial and parallel chunked read paths, and passes the progress probe to workers. The title is 63 characters and 9 words, which is concise and specific.

Full details: Linked Issues check

Explanation

Directly linked issue #2015 has coding requirements: (1) serial decimate must call progress::begin and progress::step per chunk and stop on cancel; (2) decimate_parallel must hand the caller's probe to workers, so workers count and cancel; (3) cancellation must surface as DataError::Cancelled on both paths; (4) a regression test must cover both paths. The reviewed head's crates/data/src/progress.rs adds pub(crate) fn current() (non-wasm), which returns the caller's probe for worker install. The PR summary and the change summary for crates/data/src/raster/geotiff.rs state the serial begin/step and pre-allocation check, the between-chunk checks, the overview skip counting, worker probe install with restore, worker cancellation through the existing failure path, and a regression test covering serial and parallel, full-resolution and overview, successful and cancelled cases. The test name and its 12-case run are reported. Limitation: the text diff for geotiff.rs could not be read (changed blobs unavailable), so the geotiff behavior rests on the summaries and PR description, not on read source. No wasm-target parallel behavior was verified.

Full details: Out of Scope Changes check

Explanation

All changes reported for the whole PR (merge base 6f68c9c to head 008b0d8) are in crates/data/src/progress.rs and crates/data/src/raster/geotiff.rs. The progress.rs change adds one crate-private accessor used only to pass the probe to workers. The geotiff.rs change adds progress, cancellation, probe handover and a regression test within the chunked read path. The PR reports no changes to decoding, placement, thread-count or memory-budget policy, dependencies, public parameters or plugin interfaces. The summaries show no unrelated change. Limitation: the geotiff.rs diff could not be read, so this verdict relies on the summaries.

Full details: Description check

Explanation

The description follows the repository template. It has Summary, Testing, UI evidence, and Related issue sections. The Summary states what changed and why: chunked GeoTIFF reads now report progress and return DataError::Cancelled, and parallel workers receive the caller's probe. The Testing section names a regression test, reports 12 passing combinations, and lists CI, pre-commit, and a wasm build. The UI section correctly says there is no visible UI change. The Related issue section links #2015. One gap: the wasm build is reported as passed in the description, but the PR objectives say that build was still running, so the result is not final. This is a minor inconsistency and does not block the check.

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 hops where chunk counts grow,
Through tiled fields, the counters flow.
If Cancel calls, the workers heed,
Each skipped chunk is counted, indeed.
The rabbit curls beside the stream.}

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: ee25c85c-6d4b-4198-a6d1-f0aace36aee8

Commits

Reviewing files that changed from the base of the PR and between bce5a7c and 008b0d8.

Files selected for processing: 2

• crates/data/src/progress.rs
• crates/data/src/raster/geotiff.rs

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

[github-actions]

The second comment anchored on a line outside the diff, so GitHub rejected it. I'll fold that point into the summary instead.

Code review

Verdict: No confirmed bugs. The cancel and progress handover looks correct on the paths I traced. Two low-severity design notes, both raised inline or below, and one process gap.

Bugs

• None confirmed. I traced the serial and parallel decimate paths. The probe handover to workers, the failed flag, the join and panic handling, and the final check() all look right. Cancelled reads return DataError::Cancelled and never return partial data. Each chunk gets exactly one step(1), including skipped ones, so successful reads reach the total of 16 in the test. Confidence: high.

Security

• None found. The change only reads and reports progress.

Performance

• None found. The added check() and step() calls run once per chunk, which is negligible next to chunk decode.

Quality

• Nested begin can reset an outer step's count (low confidence, about 30%). Inline on crates/data/src/raster/geotiff.rs:1193. Reads never touched progress before this PR. I found no current production caller that reads a GeoTIFF inside a counted loop, so this is latent.
• Doc comment paragraph (nit). The new "Progress counts chunks…" sentence runs straight on from the first doc paragraph with no blank /// line, so rustdoc merges them. Left for the release quality pass.

AGENTS.md

• Whole-image read has no Cancel check (confidence high, severity low). This is not on a diff line, so it is not an inline comment. When decimate isn't used, read_image_to_buffer decodes the whole image in one call with no progress::check, at crates/data/src/raster/geotiff.rs:1563. That covers every in-memory read with no decimation and no extra samples, and file reads under 1M pixels. The work is bounded by max_samples(). AGENTS.md asks that a long step without a check be said in its doc comment, so note this on read_limited_with_nodata or confirm it's acceptable for chunked reads only, as the PR title says.
• wasm build not yet confirmed. The PR body says the local wasm build is still running. By reading, progress::current and decimate_parallel are both cfg(not(target_arch = "wasm32")), and check/step/begin exist on all targets. AGENTS.md requires that build to pass, so the author should post its result.

Already raised

• Nothing to dedupe: pr-context/prior-reviews.json has no threads. Earlier concerns left alone: 0.

Change size

Compared with the whole PR against main (merge base 6f68c9c to PR head 008b0d8).

Measure	Value
Files changed	2 (added 0, modified 2, removed 0, renamed 0)
Size before	209.9 KB
Size after	215.2 KB
Total size change	+5.5 KB

File	Before	After	Change
crates/data/src/raster/geotiff.rs	204.5 KB	209.7 KB	+5.3 KB
crates/data/src/progress.rs	5.3 KB	5.5 KB	+207 B

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-2023/
Web app	https://preview.rustgis.app/pr-2023/app/
Commit	008b0d8

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