[elpidioassinaturas]

Summary

Session::run reports the keys a command doesn't read. The list of keys a command reads comes from the top level of its parameter description, through param_keys. In cube.trend's description, the keys of a STAC trend (url, asset, bbox, maxCloud, cellSize and resampling) sat inside the "collection" entry's note, where param_keys doesn't look.

So every STAC trend from the CLI, a script, Python or the MCP server got a false notice of the form cube.trend ignores 'url', 'asset', … (it reads …), even though the run uses all of these keys.

The notice is there so that a script or an agent can correct itself. Here it points the wrong way: an agent that drops the "ignored" keys breaks the run.

• The six keys move to the top level of the description, each with the note it had. The "collection" entry keeps a short note that points to them.
• No behaviour changes. trend_cmd and stac_cube read the same keys as before.

Testing

• New test a_stac_trends_keys_are_read:
  • param_keys for cube.trend holds collection, url, asset, bbox, maxCloud, cellSize, resampling, start and end;
  • a STAC trend against an API on a closed local port, with all of these keys given, raises no "ignores" notice.
• On main's description, the test fails at the first check: url is missing from the keys.
• param_keys_reads_descriptions and unknown_params_are_said pass unchanged.
• cargo fmt --all --check and cargo clippy -p rustgis-engine --all-targets -- -D warnings are clean, and the rustgis-engine tests pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Documentation
  • Clarified that STAC collection processing is available on desktop, reads data in place across an area, and processes large areas in pieces.
  • Listed the STAC collection options as top-level parameters: URL, asset, bounding box, cloud limit, cell size, and resampling method. These options describe how collection data is located and processed.

[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: e19f8823-9060-4629-9b4e-7d2594f22898

📥 Commits

Reviewing files that changed from the base of the PR and between 5bdb187 and 18b38fb.

📒 Files selected for processing: 1

• crates/engine/src/cmd/cube.rs

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

📝 Walkthrough

The cube.trend description now lists STAC collection options as top-level parameters and describes collection processing. A test checks the declared parameter keys and verifies that cellSize: 0 returns an error without an “ignores” notice.

Changes: STAC trend parameters

Layer / File(s)	Summary
Parameter description and validation crates/engine/src/cmd/cube.rs, crates/engine/src/tests.rs	The description lists STAC options as top-level parameters and describes collection processing as desktop-only, in place over an area, and split into pieces when large. The test checks the parameter keys and validates the error for cellSize: 0 without an “ignores” notice.

Priority: ⬇️ Low

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

Change: Bug fix

Possibly related issues

• [Feature]: Autocomplete commands, parameter keys and values in the Script pane #662: The test covers STAC trend parameter keys in the command description. This may support parameter-key autocomplete when that feature reads command parameter descriptions.

Suggested reviewers: giswqs

Merge Risk: ⚪ Minimal · up to 18b38

The parameter declaration and validation test introduce no established merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	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 describes the STAC trend parameter-key issue addressed by the pull request. It is specific and related to the main change, although it is somewhat long.
Description check		The description includes complete Summary and Testing sections with the problem, solution, verification steps, and test results. The Related issue section from the template is missing, but this is non…
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: Description check

Explanation

The description includes complete Summary and Testing sections with the problem, solution, verification steps, and test results. The Related issue section from the template is missing, but this is non-critical.

✨ 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 reads the trend keys clear,
STAC options gather near.
The cell size check reports a flaw,
No ignored notice gives the clue.
The fields sit at the top in view,
And pieces form when areas grow.}

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

[github-actions]

Code review

I reviewed the whole PR (2 files). It looks correct and I found nothing blocking. I did not run the tests; there is no Bash tool here.

What it does: The cube.trend parameter description had its STAC keys (url, asset, bbox, maxCloud, cellSize, resampling) nested inside the parenthesised "collection" note. param_keys only reads top-level entries, so passing those keys produced a false "cube.trend ignores …" notice. The PR moves them to top-level entries, and I traced the new string through param_list in crates/engine/src/lib.rs:

• Each new key opens a top-level entry ("url"?:, "asset"?: and so on).
• "start" and "end" appear once inside the collection's parentheses, where they are not read as keys, and once at top level. That keeps the "each command names a key once" check in param_keys_reads_descriptions passing.
• Commas inside parentheses and brackets are not split points, so the entries parse correctly.

Bugs

None found.

Security

None. The test connects only to a loopback address.

Performance

None.

Quality

• Test depends on a refused local connection (low confidence, minor). a_stac_trends_keys_are_read asserts that s.run("cube.trend", …) errors by connecting to 127.0.0.1:9. This holds as long as nothing listens on port 9 and the connection is refused quickly. A sandbox that drops packets instead of refusing them could make it slow, and a missing desktop feature would still pass. That is probably acceptable, but if the command validates the STAC params before any network call, you could assert on that error instead.
• Description is long (nit). The cube.trend string is now a very long single-line literal. That matches how the file already did it, so I wouldn't act on this.

AGENTS.md

• There are no unwrap() calls in non-test code. The .unwrap() in the new test is allowed in test code.
• No UI text, dependency or asset changes, and the wasm build is unaffected.

Change size

Compared: the whole PR against main (merge base d9b2de0 to PR head 1aad9ac)

Metric	Value
Files changed	2 (added 0, modified 2, removed 0, renamed 0)
Size before	894.4 KB
Size after	895.6 KB
Total size change	+1.2 KB

File	Before	After	Change
crates/engine/src/tests.rs	849.5 KB	850.5 KB	+1.0 KB
crates/engine/src/cmd/cube.rs	44.9 KB	45.1 KB	+174 B

[github-actions]

Code review

I reviewed the cube.trend parameter description change in crates/engine/src/cmd/cube.rs and the new test a_stac_trends_keys_are_read in crates/engine/src/tests.rs. I found nothing that needs fixing, so I posted no inline comments. I could only read the code, so I did not run the tests.

Bugs

• None found. The six keys (url, asset, bbox, maxCloud, cellSize, resampling) now sit at the top level of the description, where param_keys and param_list read them. Only top-level entries count as keys, so the quoted words in the "collection" note, which sits inside parentheses, are not picked up. Confidence: high.
• param_keys_reads_descriptions requires each command's keys to be unique. Moving url to the top level could clash with "url" in the "layer" entry's parenthetical (or "path" | "url", …). That text is nested in parentheses, so it is not read as a key, and I found no clash. Confidence: high.
• The test relies on stac_cube rejecting cellSize: 0 before any network call. I checked stac.rs at line 552: it validates cellSize first, with the comment "The parameters read here, before the search", and the error text contains "cellSize". trend_cmd calls stac_cube through the collection branch at cube.rs line 291. Confidence: high.

Security

• None. The change only edits a description string and adds a test that stays on the local machine. http://127.0.0.1:9 is never contacted.

Performance

• None.

Quality

• Nit (low): The "collection" note says "with the keys below", which is correct but leaves the user to find which keys it means. The wording is fine, and url, asset and bbox are now listed as their own entries.
• Nit (low): The test comment says "A cell size of 0: refused before any search". The assertion only shows that cellSize is mentioned in the error and that no "ignores" notice appears. If stac_cube is ever refactored to check the other parameters first, the test would still pass, but its stated reason would no longer be accurate. This is minor.

AGENTS.md

• Test code uses unwrap() and unwrap_err(), which is allowed in tests. Non-test code has no new panics or unsafe.
• The change adds no user-visible UI literals, no new dependencies and no wasm-incompatible code. The stac_cube path is already gated with cfg(not(target_arch = "wasm32")).
• The fix comes with a regression test, as the guidelines ask. Confidence: high.

Change size

Compared: the whole PR against main (merge base d9b2de0 to PR head 5bdb187)

Metric	Value
Files changed	2 (added 0, modified 2, removed 0, renamed 0)
Size before	894.4 KB
Size after	895.6 KB
Total size change	+1.2 KB

File	Before	After	Change
crates/engine/src/tests.rs	849.5 KB	850.6 KB	+1.1 KB
crates/engine/src/cmd/cube.rs	44.9 KB	45.1 KB	+174 B

[github-actions]

Code review

I'm still on the review, and it's nearly finished. The PR only rewrites the cube.trend parameter description and adds one test. I checked both.

Bugs: none found.

• param_keys takes top-level quoted words that open an entry, or follow | and are followed by ? or :. The rewrite moves url, asset, bbox, maxCloud, cellSize and resampling out of the "collection" note and into entries of their own, so they now count as keys. I walked through the new description against that rule. "view", "nearest" and "average" are not followed by ? or :, so they stay values. The "start" and "end" inside the note's parentheses sit at depth 1, so they aren't read as keys, and the later top-level entries cover them. Confidence: high.
• Each key now appears once, so the param_keys_reads_descriptions uniqueness check should still pass. I couldn't run tests here. Confidence: high.
• cellSize: 0 is rejected in crates/engine/src/cmd/stac.rs:552-555, with an error that contains cellSize, so the new test's assertion fits. I did not trace that this check runs before any network call in the cube.trend path. The test uses 127.0.0.1:9, so at worst it would fail with a connection error rather than reach a real host. Confidence: medium.

Security: nothing to raise. The test's URL is localhost and the cell size is refused before any search.

Performance: nothing to raise.

Quality (nit, low confidence): the new description is very long, and its "collection" note repeats the keys that now have their own entries (url, asset, bbox, maxCloud, cellSize and resampling below). Trimming that sentence would make the help text easier to read.

AGENTS.md: no violations.

• The test uses unwrap() only in test code, which is allowed.
• No new dependencies, assets, UI strings or layering changes.

Change size

Compared: the whole PR against main (merge base d9b2de0 to PR head 18b38fb)

Metric	Value
Files changed	2 (added 0, modified 2, removed 0, renamed 0)
Size before	894.4 KB
Size after	895.7 KB
Total size change	+1.3 KB

File	Before	After	Change
crates/engine/src/tests.rs	849.5 KB	850.5 KB	+1.1 KB
crates/engine/src/cmd/cube.rs	44.9 KB	45.1 KB	+218 B

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-766/
Web app	https://preview.rustgis.app/pr-766/app/
Commit	18b38fb

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