Added predicted-probability mapping

[anna-abelman]

Summary

Adds predicted-probability mapping for fitted FishSET models, including interactive Leaflet maps and exportable static ggplot maps.

What changed

• Adds map_predicted_probs() to calculate mean or observation-level predicted probabilities by zone, join them to spatial data, and save the resulting table and plot.
• Supports both interactive Leaflet maps and static ggplot maps, with validation for missing fits, probability matrices, spatial data, and zone-ID mismatches.
• Extends the Model Fit Shiny module with:
  • An option to save the full probability matrix when fitting a model.
  • Controls to select a saved fit and generate an interactive predicted-probability map.
  • A static-map export flow with an in-app preview.
  • User-facing notifications when required mapping inputs or saved probability data are unavailable.
• Adds unit tests covering input validation, probability calculations, table output, static and dynamic map creation, and combined outputs.

Notes

Mapping requires models to be fitted with Save Full Probability Matrix enabled. This can increase memory usage for large datasets.

[Copilot]

🟡 Changes recommended

Unresolved mapping, namespace, CRS, validation, security, and error-handling issues block approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds predicted-probability mapping for FishSET models, including static and interactive maps integrated into the Model Fit Shiny module.

Changes:

• Adds probability aggregation, spatial joining, map generation, and export logic.
• Adds probability-matrix saving and Shiny mapping controls.
• Adds unit tests for validation, calculations, and map outputs.

File summaries

File	Description
tests/testthat/test-map_predicted_probs.R	Tests mapping validation, calculations, and outputs.
R/map_predicted_probs.R	Implements predicted-probability mapping and rendering.
inst/ShinyFiles/MainApp/modules/model_fit_module.R	Adds probability saving and Shiny mapping workflows.

Review details

Suppressed comments (7)

R/map_predicted_probs.R:127

• This check only catches a mismatch when every spatial row has an NA probability. If at least one zone matches, model zones absent from the spatial data are silently removed by line 130 while the returned table still contains them, so the map and table disagree. Validate missing model zone IDs before filtering.

    if (all(is.na(spat_join[[val_var]]))) {
      stop("All joined probability values are NA. This means the Zone IDs in the model did 
           not match any Zone IDs in the spatial dataset.")
    }

R/map_predicted_probs.R:166

• map_data() expects longitude/latitude degrees, but these limits come from st_bbox(spatdat) in whatever CRS the input uses. For projected sf inputs, the limits are in metres (or another projected unit), so the coastline layer is empty or misaligned with the polygons. Generate the base map in WGS84 and transform/plot it in a consistent CRS.

        ggplot2::map_data(map = map_name,
                          xlim = x_limits,
                          ylim = c(bbox["ymin"], bbox["ymax"]))

R/map_predicted_probs.R:104

• The fallback to 1:ncol() does not actually use spatial row order: the table is later joined by zone-ID value. For an unnamed probability matrix and ordinary IDs such as Zone_A, every join value is unmatched and the function errors; require column names matching the spatial zone IDs or build a validated mapping explicitly.

  zone_names <- colnames(fit$prob_matrix)
  if (is.null(zone_names)) {
    warning("prob_matrix lacks column names. Assuming column index matches zone ID order.")
    zone_names <- as.character(1:ncol(fit$prob_matrix))
  }

R/map_predicted_probs.R:262

• The documented plot_type values are "dynamic" and "static", but any other value silently takes the static branch. A typo or invalid API value therefore produces a different output instead of a validation error; reject unsupported values before dispatching.

    if (plot_type == "dynamic") {
      z_plot <- z_plot_fun_dynamic(spat_join, legend_name = legend_name)
    } else {
      z_plot <- suppressWarnings(z_plot_fun_static(spat_join, legend_name = legend_name))

inst/ShinyFiles/MainApp/modules/model_fit_module.R:465

• The unserialize_table() call runs before the tryCatch that starts at line 473. A missing or corrupt ModelFit table therefore escapes this observer and does not produce the promised user-facing mapping failure notification. Put this lookup and fit check inside the same error-handling path.

      full_fit_list <- unserialize_table(paste0(project_name, "ModelFit"), project_name)
      fit <- full_fit_list[[input$map_fit_input]]
      
      if (is.null(fit$prob_matrix)) {

inst/ShinyFiles/MainApp/modules/model_fit_module.R:544

• The static-map precheck has the same unhandled database-read failure: unserialize_table() is outside the tryCatch below, so a missing or corrupt ModelFit table breaks the observer instead of notifying the user. Move this lookup and fit check into that error-handling path as well.

      full_fit_list <- unserialize_table(paste0(project_name, "ModelFit"), project_name)
      fit <- full_fit_list[[input$map_fit_input]]
      
      if (is.null(fit$prob_matrix)) {

inst/ShinyFiles/MainApp/modules/model_fit_module.R:656

• The tooltip text contains the grammatical typo mapping.:, which is shown directly in the UI. Remove the extra colon so the instruction reads naturally.

                          "Check this to enable spatial mapping.: Doing this for models with 
                          massive datasets can cause memory constraints or slow down 

• Files reviewed: 3/3 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Coordinate handling and validation issues can produce incomplete or misleading maps.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

• Normalize map geometries and limits to one CRS · New

Resolved since last review (5)

• The zone ID is interpolated into raw HTML and then marked safe with htmltools::HTML without…
• var_sym is an rlang symbol, but [[ requires a character or integer subscript. Any dynamic map…
• The static rendering path evaluates unit() inside the package namespace, but grid::unit is…
• When st_crs(spatdat) is missing, the source coordinate system is unknown, so st_transform()…
• The new function is annotated @export, but the committed NAMESPACE has no…

Files not reviewed (1)

• man/map_predicted_probs.Rd: Generated file

Previously missed (6)

In code that hasn't changed since last review

Validate plot_type and output enum arguments

R/​map_predicted_probs.R:40

The documented enum arguments are not validated: an unknown plot_type silently selects the static branch, while an unknown output silently returns a table. Reject unsupported values up front so typos cannot change the requested result.

Validate obs_index as a finite whole-number scalar

R/​map_predicted_probs.R:94

obs_index is documented as an integer, but this bounds check accepts fractional values; base R truncates 1.5 to row 1 while the returned column is labeled prob_obs_1.5. Validate scalar, finite, whole-number input before indexing.

Detect modeled zones missing from spatial data

R/​map_predicted_probs.R:121

This spatial left join starts from spatdat, so a modeled zone that is absent from the spatial data never appears in spat_join. The current check catches only a total mismatch; a partial mismatch silently produces an incomplete map, and table-only output bypasses ID validation despite the documented matching requirement. Compare all modeled IDs with the spatial IDs before branching and report any missing zones.

Handle NA geometry validity before polygon repair

R/​map_predicted_probs.R:138

st_is_valid() can return NA for missing or unreadable geometries. In that case any(!st_is_valid(...)) is NA, so this if fails with “missing value where TRUE/FALSE needed” instead of a useful mapping error. Check the joined geometries for empty/unknown validity explicitly before repairing invalid polygons.

Clear stale map when selected fit changes

inst/​ShinyFiles/​MainApp/​modules/​model_fit_module.R:420

Changing the selected fit leaves the previously generated map visible, so the dropdown can identify fit B while the map still displays fit A (and it remains stale if generation for B fails). Clear the holder whenever map_fit_input changes.

Avoid skipping tests when maps is unavailable

tests/​testthat/​test-map_predicted_probs.R:180

This skip occurs before the dynamic-map and combined-output assertions in the same test_that(), so those cases never run when maps is unavailable. map_predicted_probs() already catches a map_data() failure and builds a zone-only static plot, so remove the skip; alternatively, move only the static case into a separate dependency-gated test.

[Copilot]

Fixed in f249fe6.