fix: slope rules table clearing and data corruption on row removal (#1302)

[wangzhengdna-lang]

Issue

Closes #1302

Description

When all slope rules were removed via the "- Remove selected rows" button, the reactable table failed to clear visually. Successive row removals further corrupted the displayed data: TYPE changed from "Exclusion" to "Selection", RANGE was cleared, and a ghost column (row.names leaked as data) appeared on the left.

Reported by @Gero1999 — identified that req() guards silently abort on NULL, preventing table clearing after PR #1262 refactored manual_slopes to NULL initialization.

Root Cause (4 layers)

Layer	Cause	Fix
1. Table not clearing	validate(need(FALSE)) retains previous output; req() silently aborts	return(NULL) in renderReactable to explicitly clear
2. Data corruption (TYPE→Selection, RANGE cleared, ghost column)	reactable.extras widgets (dropdown_extra, text_extra) send Shiny input events during post-render initialization, overwriting manual_slopes() with widget defaults	Generation counter + 500ms suppression window in edit observer
3. Stray columns (ATPTREF from plot clicks)	rbind/bind_rows merge data frames with different column sets	Column normalization before all combines (add handler, check_slope_rule_overlap, render)
4. Broken updateReactable (silent no-op since #1262)	outputId in slope_selector.R missing one -manual_slopes namespace suffix; fixing it caused double-trigger races with renderReactable	Reverted to no-op; renderReactable + bindEvent(refresh_reactable()) already handles all updates

Definition of Done

• Slope rules table clears when all rows are removed
• Row removal does not corrupt remaining rows (TYPE, RANGE, REASON unchanged)
• No extra/ghost columns appear after successive removals
• Adding rows after clearing works correctly
• Results > Manual Adjustments shows "No rules defined" when rules are empty
• Column normalization prevents stray columns from plot-click paths
• Edit events are suppressed during widget re-initialization

How to test

Requires manual testing (Shiny UI, cannot be covered by unit tests):

1. Table clearing: Add 3 rules → remove all one by one → table should clear after last removal
2. Data integrity: Add rules with different TYPE/RANGE → remove middle row → remaining rows should keep original values
3. Plot-click + button mix: Click plot to add exclusion → use "+ Exclusion/Selection" button → remove rows → no extra columns appear
4. Settings upload: Export settings with slope rules → re-import → verify rules restore correctly
5. Results tab: Remove all rules → check Results > Manual Adjustments shows "No rules defined"

Files changed

• inst/shiny/modules/tab_nca/setup/manual_slopes_table.R — Core fix: return(NULL), column normalization, edit suppression guard
• inst/shiny/modules/tab_nca/setup/slope_selector.R — Reverted broken updateReactable to no-op
• inst/shiny/functions/utils-slope_selector.R — Column normalization in check_slope_rule_overlap
• inst/shiny/modules/tab_nca.R — Results tab clearing observer
• tests/testthat/test-utils-slope_selector.R — Updated NULL→0-row DF assertions + column normalization tests
• inst/shiny/tests/testthat/test-utils-slope-selector.R — Updated NULL→0-row DF assertions + column normalization tests

Contributor checklist

• Code passes lintr checks (only 2 pre-existing cyclocomp warnings, unrelated to this PR)
• Code passes all unit tests (1693 pass, 0 fail, 0 warn, 3 skip)
• New logic covered by unit tests — check_slope_rule_overlap column normalization has 4 new tests
• New logic is documented — CLAUDE.md updated with patterns; inline comments explain suppression guard
• App or package changes are reflected in NEWS
• Package version is incremented (0.1.0.9175 → 0.1.0.9176)
• R script works with the new implementation — N/A (no NCA logic changes)
• Settings upload works with the new implementation — requires manual verification (see How to test)
• If any .scss change was done, run data-raw/compile_css.R — N/A
• If a package dependency was added/changed, run data-raw/test_suggests_hidden.R — N/A

Notes to reviewer

• The edit suppression guard (generation counter + 500ms delay) is a novel pattern in this codebase. It addresses a reactable.extras behavior where widgets send Shiny input events during post-render initialization, which would otherwise overwrite valid data with widget defaults.
• The updateReactable in slope_selector.R was intentionally reverted to a no-op rather than fixed. Fixing the namespace would cause double-trigger races with renderReactable. The renderReactable + bindEvent(refresh_reactable()) pattern already handles all table updates.
• The tab_nca.R observer targets the Results tab's reactable_server("manual_slopes") (output ID <ns>-manual_slopes-table), NOT the Setup tab's manual_slopes_table_server (output ID <ns>-manual_slopes-manual_slopes). These are different Shiny modules.

[Shaakon35]

updateReactable in tab_nca.R targets a non-existent element ID

The new code in tab_nca.R calls:

reactable::updateReactable(ns("manual_slopes-table"), data = ...)

This constructs the ID "nca-manual_slopes-table", but the child module's reactable output is output$manual_slopes (registered via reactableOutput(ns("manual_slopes"))), which produces the actual ID "nca-manual_slopes-manual_slopes".

So "manual_slopes-table" ≠ "manual_slopes-manual_slopes" — the updateReactable call targets a non-existent element and silently does nothing.

Beyond the wrong ID, this pattern breaks module encapsulation. The parent is reaching into the child module's namespace by guessing its internal output name. If the child renames its output, this breaks silently.

Suggested fix: Remove the updateReactable call entirely. The child module already handles empty/NULL data in its renderReactable:

data <- manual_slopes()
if (is.null(data) || nrow(data) == 0) {
  return(NULL)
}

When slope_rules() becomes NULL/empty, the existing reactive chain (slope_rules → manual_slopes → renderReactable) already clears the table. The parent just needs to ensure slope_rules() is set correctly, which it already does.

[wangzhengdna-lang]

@Shaakon35 Thanks for the careful review. However, the namespace analysis here is mixing up two different Shiny modules:

The two modules are different

	Results tab reactable	Setup tab reactable
Module	reactable_server("manual_slopes", ...)	manual_slopes_table_server("manual_slopes", ...)
Output ID pattern	output\$table → <ns>-manual_slopes-table	output\$manual_slopes → <ns>-manual_slopes-manual_slopes
Location	tab_nca.R:118	slope_selector.R:245

The updateReactable in tab_nca.R targets ns("manual_slopes-table") which resolves to tab_nca-manual_slopes-table — this matches the reactable_server module's output ID (<ns>-manual_slopes-table). The namespace IS correct.

Why the observer is still needed

The reactable_server module uses req(data()) in its renderReactable (line 73-78 of reactable.R). When slope_rules() becomes NULL, req() silently aborts — retaining the previous table content instead of clearing it. This is the exact same pattern we fixed in manual_slopes_table.R (changed validate(need(FALSE)) → return(NULL) there).

Since we can't change reactable_server (it's shared by all tables in the app), the observer is the targeted fix: when slope rules are cleared, it explicitly updates this one table to show "No rules defined".