Skip to content

feat(ui): offer a gp.run tool's parameter values by kind in the Script pane - #827

Merged
giswqs merged 2 commits into
opengeos:mainfrom
jgregoriorincon:feat/script-tool-values
Oct 24, 2026
Merged

giswqs merged 2 commits into
opengeos:mainfrom
jgregoriorincon:feat/script-tool-values

Conversation

@jgregoriorincon

@jgregoriorincon jgregoriorincon commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

In a gp.run line's "params", the Script pane now offers the values the tool's parameter takes, by its kind, as the tool's form offers them:

  • Layers: a polygon, point, raster or vector parameter offers the layers of that kind that Run finds by their name; one that takes any layer offers every layer, then the standalone tables no layer's name hides (with the table icon and Table · N rows beside them).
  • Fields: a field, numeric field or coordinate field parameter offers the fields it takes of the layer or table the tool's first layer parameter names, written before or after it.
  • Choices: a choice parameter offers its choices in the tool's order ("units": " → Kilometers, Meters, Miles, Feet).

Nothing is offered for a number, a bool, a path, a text, several layers in one string, or in a task.addStep line's "params".

Two commits:

  1. refactor(ui): one rule for the layers and fields a tool's parameter takes (takes_layer, takes_field, first_layer, input_dataset, input_table in panes.rs), used by the tool's form and the Model Builder's source_layer. The form behaves as before.
  2. feat(ui): tool_value in scriptcomplete.rs reads the value being typed from the "params" object's {, so the scanner is unchanged; Of gains ToolFields(input) and Choices. The user guide's Script section says so.

Known limits, as for a "field" value today: no fields are offered when the input's name holds escaped quotes; a coordinate field's list reads up to 1000 rows per field each time the offer changes (the form keeps it cached).

Testing

  • cargo xtask ci. Everything passes except rustgis-render's a_curved_labels_halos_go_under_all_its_letters, which fails on main too with thin system fonts (test: fix main CI on macOS and Windows (thin-font halo test, sign-in cancel race) #820).
  • New tests: a_tools_values_are_offered_by_kind, a_tools_values_have_help_and_icons, a_tools_choice_is_accepted (kittest, with the accessibility check on).
  • Manual (World Atlas, desktop): polygon layers for Dissolve's input; every layer then a table for Buffer's input; numeric fields for Summary Statistics' field and every field for its case, with the input before or after; "units": "mi → Miles, accepted with Enter; nothing for "distance": "; the Buffer and Summary Statistics forms and a Model Builder Dissolve node unchanged.

Related issue

Closes #788

Summary by CodeRabbit

  • New Features
    • Script completion now suggests values for tool parameters in gp.run calls, including compatible layers, tables, fields, and predefined choices.
    • Field suggestions are filtered to match the parameter’s requirements, and suggestions include relevant descriptions and icons.
  • Bug Fixes
    • Layer and field selections are matched more consistently to the selected tool parameter, helping prevent incompatible choices from appearing.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: 840e6770-e2df-442c-b2d9-2e08f6726212
Commits

Reviewing files that changed from the base of the PR and between d908c53 and 9f642a2.

Files selected for processing: 4
  • crates/ui-egui/src/modelbuilder.rs
  • crates/ui-egui/src/panes.rs
  • crates/ui-egui/src/scriptcomplete.rs
  • docs/user-guide/analysis.md

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 panes now share helpers for matching layers and fields to parameter kinds. In the Script pane, quoted values in a gp.run parameter can offer matching layers, tables, input fields, or parameter choices.

Changes: Tool parameter completion

Layer / File(s) Summary
Shared parameter-kind filtering
crates/ui-egui/src/panes.rs, crates/ui-egui/src/modelbuilder.rs
Shared helpers filter layers and fields by parameter kind and resolve input datasets and tables. Geoprocessing pane choices and Model Builder use the shared layer selection.
Script parameter value completion
crates/ui-egui/src/scriptcomplete.rs, docs/user-guide/analysis.md
The Script pane offers supported values for gp.run parameters, with matching help and icons. Tests cover filtering, unsupported contexts, and accepting choices. The user guide describes the completion options.

Priority: Normal

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

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ScriptPane
  participant ToolSpec
  participant PanesHelpers
  participant Session
  ScriptPane->>ToolSpec: Resolve the named tool parameter
  ScriptPane->>PanesHelpers: Match the parameter kind
  PanesHelpers->>Session: Resolve matching layers, fields, or input dataset
  Session-->>ScriptPane: Return matching input values
  ScriptPane-->>ScriptPane: Offer matching values or parameter choices
Loading

Suggested reviewers: giswqs

Merge Risk: Minimal · up to 9f642

No additional merge-blocking issue is established. The previously reported coordinate-field completion cost remains worth monitoring on large datasets.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: offering gp.run tool parameter values by kind in the Script pane.
Description check Passed The description includes complete Summary, Testing, and Related issue sections. It explains the behavior, scope, known limits, tests, manual checks, and linked issue.
Linked Issues check Passed Issue [#788] is an active direct target. The diff adds tool_value handling for quoted values in a gp.run "params" object. It filters layers with shared takes_layer rules, offers compatible fie…
Out of Scope Changes check Passed The changes stay within issue [#788]. The panes.rs helpers centralize the layer and field rules required by the issue. The Model Builder update consumes that shared layer-parameter rule. The script …
Docstring Coverage Passed Docstring coverage is 95.24% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 3 files. (1 skipped: 1 …
Full details: Linked Issues check

Explanation

Issue [#788] is an active direct target. The diff adds tool_value handling for quoted values in a gp.run "params" object. It filters layers with shared takes_layer rules, offers compatible fields from the first layer input with shared takes_field rules, and preserves choice order. value_ids performs case-insensitive prefix matching. Of::ToolFields and Of::Choices use the existing value insertion path, and layer, table, and field help and icons are implemented. The diff excludes unsupported kinds and task.addStep values. The refactor updates the tool form and Model Builder to use the shared rules. The added tests cover layer, table, field, choice, unsupported-value, ordering, help, icon, and acceptance behavior.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue [#788]. The panes.rs helpers centralize the layer and field rules required by the issue. The Model Builder update consumes that shared layer-parameter rule. The script completion tests and user-guide update document and verify the requested behavior. No unrelated feature or unrelated file change is shown in the reviewed diff.

Full details: Docstring Coverage

Explanation

Docstring coverage is 95.24% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 3 files. (1 skipped: 1 unsupported.)

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 taps a script with care,
Matching fields and layers there.
Choices hop into the line,
Tables join the list in time.
The closing quote falls into place.

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

@github-actions

Copy link
Copy Markdown

Code review

I read the whole diff and found nothing that needs fixing before merge. I couldn't build or run the tests here, so this is a read-through only.

Bugs

  • None found (confidence: medium-high).
    • The refactor into takes_layer, takes_field, first_layer, input_dataset and input_table keeps the old behaviour of the tool form. The Layer, Polygon, Point, Raster and Vector filters match the closures they replace. RasterLayers and VectorLayers still use the same raster test.
    • modelbuilder::source_layer now calls the shared first_layer, with the same kind set as before.
    • In tool_value, the tool's input is read by joining the text before the cursor with the rest of the line. The test with "field" before "input" covers that join.
    • The table-versus-layer name shadowing in the offered list follows the same rule layer_by_name and find_table use when Run resolves a name.

Security

  • None. The change only adds editor completions and reads project data. No file or network access is added.

Performance

  • Low confidence, minor. tool_value calls layer_by_name once per layer and find_table once per table, so building the list is quadratic. Project sizes make this unlikely to matter. If completion is computed every frame, it is cheap to skip when the cursor isn't in a gp.run value.

Quality

  • tool_value handles Layer, PolygonLayer, PointLayer, RasterLayer and VectorLayer but not RasterLayers or VectorLayers. The doc comment says these are skipped on purpose (several layers in one string). That reads as intended.
  • takes_layer and takes_field return false for unrelated kinds. The _ => false arms hide any future layer-like ParamKind that gets added. This is a small maintainability risk (confidence: low).
  • The tests are thorough. They cover kinds, the input after the field, tables, case folding, other commands' params, the help and icon paths, and accepting a choice.

AGENTS.md

  • No unwrap, expect, panic! or unsafe in the changed non-test code. It uses ? and get() throughout.
  • User-visible text goes through tl! and i18n::fmt ("Table · {n} rows"). The new strings don't need catalog entries.
  • It reuses the existing ui-egui helpers, so no layering or wasm concerns.
  • The docs are updated in docs/user-guide/analysis.md.

Change size

Compared: the whole PR against main (merge base 45b28ec to PR head 9f642a2)

Metric Value
Files changed 4 (added 0, modified 4, removed 0, renamed 0)
Size before 523.6 KB
Size after 534.6 KB
Total size change +11.0 KB
File Before After Change
crates/ui-egui/src/scriptcomplete.rs 127.0 KB 136.7 KB +9.7 KB
crates/ui-egui/src/panes.rs 281.2 KB 282.3 KB +1.1 KB
docs/user-guide/analysis.md 39.7 KB 40.0 KB +334 B
crates/ui-egui/src/modelbuilder.rs 75.6 KB 75.5 KB -134 B

@jgregoriorincon
jgregoriorincon marked this pull request as ready for review October 24, 2026 15:27
Comment thread crates/ui-egui/src/scriptcomplete.rs
@github-actions

Copy link
Copy Markdown

Code review

The change is a clean refactor plus a useful feature. It moves the form's layer and field filters into shared helpers (takes_layer, takes_field, first_layer, input_dataset, input_table). It then reuses them in the Script pane to complete values inside a gp.run line's "params". I traced these paths and found no correctness bugs:

  • the form's layer filtering, which is unchanged;
  • raster and vector kinds;
  • the first_layer lookup;
  • a value being typed versus a key being typed;
  • fall-through to key_at;
  • the Of::Layers fallback to tables in help and icons.

The new tests cover most of the behaviour.

Bugs

  • None found.

Security

  • None found. All lookups use get() or ?, so there is no panic path.

Performance

  • scriptcomplete.rs:535 (confidence: medium): offer_at runs every frame while the cursor is in a value. For a CoordinateField parameter it calls holds_coordinates on every field each frame. The form in panes.rs caches this because it reads rows. A large layer or table could stutter. I posted an inline comment with options.

Quality

  • None worth raising. The extraction removes duplicated logic between the form and the Script pane.
  • The docs/user-guide/analysis.md edit also re-wraps an existing long line. That is harmless.

AGENTS.md

  • Compliant: no unwrap, expect or panic; new UI text uses tl! or i18n::fmt; the user guide is updated; the layering rule is respected.
  • I did not run cargo xtask ci. I have no Bash tool in this review.

Change size

Compared: the whole PR against main (merge base 45b28ec to PR head 9f642a2)

Metric Value
Files changed 4 (added 0, modified 4, removed 0, renamed 0)
Size before 523.6 KB
Size after 534.6 KB
Total size change +11.0 KB
File Before After Change
crates/ui-egui/src/scriptcomplete.rs 127.0 KB 136.7 KB +9.7 KB
crates/ui-egui/src/panes.rs 281.2 KB 282.3 KB +1.1 KB
docs/user-guide/analysis.md 39.7 KB 40.0 KB +334 B
crates/ui-egui/src/modelbuilder.rs 75.6 KB 75.5 KB -134 B

@github-actions

Copy link
Copy Markdown

📖 Docs preview

Item Value
Preview https://preview.rustgis.app/pr-827/
Web app https://preview.rustgis.app/pr-827/app/
Commit 9f642a2

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Offer a gp.run tool's parameter values by kind in the Script pane

2 participants