feat: auto-restore session from settings upload by js3110 · Pull Request #1225 · pharmaverse/aNCA

js3110 · 2026-04-14T16:08:22Z

Issue

Closes #722

Description

When a dataset and settings YAML are uploaded together, the app now auto-restores the previous session instead of requiring manual clicks through each step.

What happens on upload with settings:

Auto-applies column mapping from stored settings
Auto-advances through filtering (restores stored filters) and preview
Creates PKNCA data object
Navigates to the tab the user was on when they saved
Auto-runs NCA if it was previously run (nca_ran flag in settings)

Error handling:

Partial mapping match (columns missing in new data): aborts auto-replay, shows warning, stays on Mapping tab
Unresolved duplicates: duplicate modal appears, 15s safety timeout aborts auto-replay
PKNCA data creation failure: dismisses loading popup, shows error
NCA auto-run with incompatible parameters: dismisses popup, shows warning
General pipeline stall: 15s timeout with notification

No regressions: All auto-replay logic is gated behind settings detection. Manual upload workflow is unaffected.

Definition of Done

When dataset + settings are uploaded, the app auto-applies mapping, filters, and data processing
NCA settings (parameters, slope rules, units, ratio table, flags, bioavailability) are restored from settings
App navigates to the tab stored in the settings YAML after processing completes
If the previous session had NCA results, NCA is auto-run with restored settings
Partial mapping matches show a warning and keep the user on the Data tab
Previously resolved duplicates are auto-applied; new duplicates trigger manual resolution
No regressions in manual upload workflow

How to test

Same data, pre-NCA settings: Upload dataset + settings saved before running NCA → app should restore to the saved tab without running NCA
Same data, post-NCA settings: Upload dataset + settings saved after running NCA → app should restore and auto-run NCA
Different data: Upload a different dataset + settings with incompatible mapping → app should abort auto-replay with a warning on the Mapping tab
Different data, NCA tab: Upload different dataset + settings from NCA tab → app should navigate to NCA but show warning about incompatible parameters
Filters: Upload dataset + settings with filters → filters should be restored and auto-applied
Exploration tab: Upload dataset + settings saved from Exploration tab → app should navigate to Exploration, no NCA run

Contributor checklist

Code passes lintr checks
Code passes all unit tests
New logic covered by unit tests
New logic is documented
App or package changes are reflected in NEWS
Package version is incremented
R script works with the new implementation (if applicable)
Settings upload works with the new implementation (if applicable)
If any .scss change was done, run data-raw/compile_css.R

Notes to reviewer

The nca_ran flag is a new boolean field in the settings YAML payload. Old settings files without this field default to FALSE (no auto-run).

When a dataset and settings YAML are uploaded together (or sequentially), the app now auto-advances through the Data tab pipeline (mapping → filtering → preview → PKNCA object creation), navigates to the saved tab, and auto-runs NCA if the previous session was on the NCA tab. - Preserve tab metadata in settings_override flow - Auto-trigger mapping submission when settings are detected - Auto-advance through filtering and preview steps - Navigate to saved tab after data processing completes - Auto-run NCA when target tab is 'nca' - Return skipped mappings from .process_imported_mapping for error detection - Show warning notifications on partial mapping failure - Safety timeouts: 15s for data pipeline, 10s for NCA auto-run - No regressions: all auto-replay logic gated behind settings detection Co-authored-by: Ona <no-reply@ona.com>

Co-authored-by: Ona <no-reply@ona.com>

later::later() callbacks run outside a reactive context, so reading reactiveVals without isolate() causes 'Operation not allowed without an active reactive context' error. Co-authored-by: Ona <no-reply@ona.com>

shinyjs::delay() runs inside a reactive context, eliminating the need for isolate() wrappers. It is already a dependency and purpose-built for deferred execution in Shiny apps. Co-authored-by: Ona <no-reply@ona.com>

…oading popup - Add nca_ran flag to settings export payload (both zip and download) - Set session$userData$nca_ran when NCA results are calculated - Gate auto-run NCA on nca_ran flag (not just tab == 'nca') - Suppress 'Calculating NCA results...' popup during auto-replay - Dismiss 'Restoring session...' popup after auto-replay NCA completes Co-authored-by: Ona <no-reply@ona.com>

…ameters When settings are from a different dataset, the analyte/profile may not match, causing 'Invalid parameters' before tryCatch runs. The early return now resets auto_nca_running and dismisses the popup. Co-authored-by: Ona <no-reply@ona.com>

…dition The previous check in the adnca_mapped observer could fire before .process_imported_mapping had set mapping_skipped. Moving the primary check into the shinyjs::delay callback (after selectize inputs have flushed) ensures mapping_skipped is already populated. The original check remains as a safety net. Co-authored-by: Ona <no-reply@ona.com>

Co-authored-by: Ona <no-reply@ona.com>

- .abort_auto_replay(): shared abort pattern used in 3 places - .restore_duplicate_exclusions(): duplicate key matching logic Co-authored-by: Ona <no-reply@ona.com>

tab_data.R: - .setup_step_navigation(): next/prev/restart button observers - .setup_auto_replay(): all auto-replay observers (steps 1,3,4 + timeout) - .simplify_volume_units(): volume unit simplification from pknca_data data_mapping.R: - .apply_single_mapping(): per-variable validation and selectize update Co-authored-by: Ona <no-reply@ona.com>

…plexity Move step 1 body and filter-pending flag setup into .start_auto_replay(). Replace auto_replay_filter_pending reactiveVal with session$userData to eliminate a separate observer. Co-authored-by: Ona <no-reply@ona.com>

The delay callback already checks auto_replay() before aborting, making the outer guard unnecessary. Co-authored-by: Ona <no-reply@ona.com>

Gero1999 · 2026-04-15T09:14:49Z

LGTM! I tested many cases and seems to work fine. My only concern is with the manual delay:

      # Delay to let the settings cascade settle before triggering NCA
      shinyjs::delay(1500, {
        log_info("Auto-replay: triggering NCA calculation.")
        auto_nca_running(TRUE)
        shinyjs::click("run_nca")
      })

I feel like this can cause trouble if the data is big or there are many settings. But maybe using an observer is not worthy? So I will leave it to your criteria 😄

…igger Use settings %>% debounce(500) instead of shinyjs::delay(1500) to wait for the NCA settings cascade to settle before auto-running NCA. This adapts to dataset size rather than relying on a fixed timer. Co-authored-by: Ona <no-reply@ona.com>

The previous observe() fired immediately when auto_nca_pending became TRUE if processed_pknca_data already existed, before the imported settings cascade had settled. Use a settled flag set by the debounced settings observer, and check from both observeEvent handlers to handle either firing order. Co-authored-by: Ona <no-reply@ona.com>

…rigger Debouncing settings alone was insufficient — processed_pknca_data depends on raw settings (not final_settings) and fires with intermediate values during the cascade. Debouncing the data object directly (1000ms) waits for the entire cascade to settle at the output level. Co-authored-by: Ona <no-reply@ona.com>

Add session$userData$auto_replay_active flag so child modules can check whether auto-replay is in progress. Suppress 'All intervals were filtered' (nca_setup.R) and 'Remember to submit filters' (data_filtering.R) during the settings cascade. Flag is cleared at every auto-replay exit point. Co-authored-by: Ona <no-reply@ona.com>

@param

…variant Add missing @param and @returns docs for tab_nca_server and tab_data_server. Expand the auto_replay_filter_pending comment to document why processed_data fires exactly twice and when the invariant would break. Add log_trace on the skip path. Co-authored-by: Ona <no-reply@ona.com>

…xity Extract .setup_filter_import and .setup_filter_reminder from data_filtering_server to bring cyclomatic complexity under the lintr threshold of 15. Co-authored-by: Ona <no-reply@ona.com>

js3110 · 2026-04-15T12:43:29Z

@Gero1999 thanks, I refactored to take your advice, PTAL

bachapman

Great job, works well for me.

js3110 and others added 12 commits April 14, 2026 14:37

docs: add instruction to pull main before creating branches

58df0d4

Co-authored-by: Ona <no-reply@ona.com>

fix: wrap reactiveVal reads in isolate() inside later callbacks

a5a27ae

later::later() callbacks run outside a reactive context, so reading reactiveVals without isolate() causes 'Operation not allowed without an active reactive context' error. Co-authored-by: Ona <no-reply@ona.com>

refactor: use shinyjs::delay() instead of later::later()

bf6e901

shinyjs::delay() runs inside a reactive context, eliminating the need for isolate() wrappers. It is already a dependency and purpose-built for deferred execution in Shiny apps. Co-authored-by: Ona <no-reply@ona.com>

chore: bump version to .9149, add NEWS entry for #722

e5187f0

Co-authored-by: Ona <no-reply@ona.com>

refactor: extract helpers to reduce cyclomatic complexity

78ad50f

- .abort_auto_replay(): shared abort pattern used in 3 places - .restore_duplicate_exclusions(): duplicate key matching logic Co-authored-by: Ona <no-reply@ona.com>

refactor: remove redundant auto_replay guard in safety timeout

3b559be

The delay callback already checks auto_replay() before aborting, making the outer guard unnecessary. Co-authored-by: Ona <no-reply@ona.com>

js3110 marked this pull request as ready for review April 15, 2026 07:21

js3110 requested review from Gero1999, Shaakon35 and bachapman April 15, 2026 07:22

refactor: refactor roxygen docs mixed up in tab data

ea1d8b0

Gero1999 approved these changes Apr 15, 2026

View reviewed changes

js3110 force-pushed the 722-enhancement/settings-auto-restore branch from 96863c0 to ea1d8b0 Compare April 15, 2026 09:43

js3110 and others added 7 commits April 15, 2026 11:49

refactor: extract filter import and reminder helpers to reduce comple…

0e43020

…xity Extract .setup_filter_import and .setup_filter_reminder from data_filtering_server to bring cyclomatic complexity under the lintr threshold of 15. Co-authored-by: Ona <no-reply@ona.com>

lint

8e7b4da

bachapman approved these changes Apr 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: auto-restore session from settings upload#1225

feat: auto-restore session from settings upload#1225
js3110 wants to merge 20 commits intomainfrom
722-enhancement/settings-auto-restore

js3110 commented Apr 14, 2026 •

edited

Loading

Uh oh!

Gero1999 commented Apr 15, 2026

Uh oh!

js3110 commented Apr 15, 2026

Uh oh!

bachapman left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

js3110 commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description

Definition of Done

How to test

Contributor checklist

Notes to reviewer

Uh oh!

Gero1999 commented Apr 15, 2026

Uh oh!

js3110 commented Apr 15, 2026

Uh oh!

bachapman left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

js3110 commented Apr 14, 2026 •

edited

Loading