refactor(statmodel): Finalize namespaces for comparisons components

[tonywu1999]

Summary by CodeRabbit

• UI Changes

  • Comparison name input for custom comparisons is now a static text field instead of a dynamically rendered element.

• Refactor

  • Internal restructuring of input handling and validation logic to improve code organization and maintainability.

• Tests

  • Updated test suite to align with restructured input handling.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Refactors server module, UI components, and tests to use NAMESPACE_STATMODEL constants for input references instead of hard-coded names, changing how contrast UI elements, validation logic, and matrix builders access namespaced inputs. Replaces a dynamic UI output for comparison naming with a static text input.

Changes

Cohort / File(s)	Summary
Server & UI module refactoring R/module-statmodel-server.R, R/statmodel-ui-comparisons.R	Replaced hard-coded input names (group1, group2, group3, comp_name) with NAMESPACE_STATMODEL constants in contrast validation, matrix building, and UI rendering logic. Converted dynamic uiOutput for comparison name to static textInput with label "Comparison Name". Updated validate_contrast_inputs, build_custom_pairwise_contrast, and build_all_against_one_contrast to use input[[...]] syntax with namespaced keys.
Server module tests tests/testthat/test-module-statmodel-server.R	Updated all test cases to populate mock inputs using namespaced keys (e.g., inputs[[NAMESPACE_STATMODEL$comparisons_custom_pairwise_choice1]]) instead of direct list assignments (e.g., group1, group2). Applied consistently across custom pairwise, all-vs-one, and custom non-pairwise comparison tests and validation checks.
Utility function tests tests/testthat/test-utils-statmodel-server.R	Refactored test input construction to use NAMESPACE_STATMODEL keys for populating input lists (e.g., comparisons_custom_pairwise_choice1/2, comparisons_custom_nonpairwise_weights) instead of pre-filled named elements, maintaining identical test logic and expected outcomes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring extra attention:
  • Verify all hard-coded input name replacements are correctly mapped to their corresponding NAMESPACE_STATMODEL constant keys across server module logic and tests.
  • Confirm the static textInput replacement for uiOutput in the UI does not break downstream reactive dependencies or validation workflows.
  • Cross-check that namespaced input keys in tests align with those defined in the module's namespace implementation.

Possibly related PRs

• refactor(statmodel): Set up a constants file for all namespace IDs #130: Introduces NAMESPACE_STATMODEL constants file and refactors the same module/UI/tests to use namespaced input IDs—directly foundational to these changes.
• refactor(statsmodel): Refactor statsmodel server into sub-functions #119: Updates contrast UI, validation, and build functions with namespaced input references; modifies the same build_* functions and input access patterns in this PR.
• tests(stat-model): Add unit tests to stat model server #118: Refactors server module structure with moduleServer and session$ns; aligns with the namespacing refactor applied across server and test layers.

Poem

🐰 Namespaces bloom where chaos once grew,
Constants guide inputs, crisp and true,
From hard-coded names to namespaced keys,
Modular contrasts dance with ease!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: replacing hard-coded input names with namespaced references throughout the statmodel comparisons components.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor-statmodel-comparisons

---

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

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

[github-actions]

Failed to generate code suggestions for PR

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

R/module-statmodel-server.R (1)

77-82: Input validation correctly switched to namespaced IDs; consider also validating name

The same-group and weight-sum checks now use the namespaced pairwise and non-pairwise weight inputs, so they track ID changes via NAMESPACE_STATMODEL. You might additionally validate that the custom non-pairwise name is non-empty now that it’s a free textInput.

-  } else if (contrast_mode == CONSTANTS_STATMODEL$comparison_mode_custom_nonpairwise) {
-    wt_sum = sum(sapply(1:length(condition_list), function(i) {
-      input[[paste0(NAMESPACE_STATMODEL$comparisons_custom_nonpairwise_weights, i)]]
-    }))
-    
-    validate(
-      need(wt_sum == 0, "The contrast weights should sum up to 0")
-    )
-  }
+  } else if (contrast_mode == CONSTANTS_STATMODEL$comparison_mode_custom_nonpairwise) {
+    validate(
+      need(
+        nzchar(input[[NAMESPACE_STATMODEL$comparisons_custom_nonpairwise_name]]),
+        "Please provide a comparison name"
+      )
+    )
+
+    wt_sum = sum(sapply(1:length(condition_list), function(i) {
+      input[[paste0(NAMESPACE_STATMODEL$comparisons_custom_nonpairwise_weights, i)]]
+    }))
+    
+    validate(
+      need(wt_sum == 0, "The contrast weights should sum up to 0")
+    )
+  }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9289268 and ca08aa1.

📒 Files selected for processing (4)

• R/module-statmodel-server.R (5 hunks)
• R/statmodel-ui-comparisons.R (1 hunks)
• tests/testthat/test-module-statmodel-server.R (7 hunks)
• tests/testthat/test-utils-statmodel-server.R (7 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build

🔇 Additional comments (23)

R/statmodel-ui-comparisons.R (1)

75-76: Non-pairwise name textInput wiring looks correct

Using ns(NAMESPACE_STATMODEL$comparisons_custom_nonpairwise_name) here matches the server and test expectations and keeps the comparison-name input consistent with the new namespacing scheme.

tests/testthat/test-utils-statmodel-server.R (8)

62-64: Pairwise util test now mirrors namespaced inputs

Initializing input with comparisons_custom_pairwise_choice1/2 via NAMESPACE_STATMODEL keeps the build_custom_pairwise_contrast test aligned with the server’s expected input shape.

---

83-85: Multiple-comparison pairwise test uses correct namespaced IDs

The second pairwise test also correctly switches to comparisons_custom_pairwise_choice1/2 keys, so it will keep tracking any future ID changes through NAMESPACE_STATMODEL.

---

105-107: Same-group guard test wired to namespaced pairwise inputs

Using the same namespaced choice keys for the “same groups” case ensures this test exercises the same path as the runtime validation logic.

---

127-131: Non-pairwise contrast creation test correctly sets weights and name

Populating weights via paste0(NAMESPACE_STATMODEL$comparisons_custom_nonpairwise_weights, i) and the name via comparisons_custom_nonpairwise_name matches the server’s expected contract for build_custom_non_pairwise_contrast.

---

149-153: Non-zero-sum rejection test is consistent with new weight/name IDs

The invalid-weights case now also uses the namespaced weight keys and comparison name, so it accurately validates the behavior of build_custom_non_pairwise_contrast with the refactored inputs.

---

173-176: All-vs-one util test uses namespaced choice ID

Setting comparisons_all_vs_one_choice through NAMESPACE_STATMODEL keeps this test aligned with build_all_against_one_contrast and the new UI wiring.

---

195-196: Single-comparison all-vs-one test also updated to namespaced ID

The single-comparison variant correctly uses comparisons_all_vs_one_choice as well, so both all-vs-one utility tests share the same namespaced contract.

---

369-372: get_contrast_panel_ui test now checks for new non-pairwise IDs

Asserting that the rendered HTML contains comparisons_custom_nonpairwise_name and comparisons_custom_nonpairwise_weights ensures the non-pairwise panel exposes the expected namespaced controls.

tests/testthat/test-module-statmodel-server.R (7)

192-193: Pairwise matrix_build test correctly switches to namespaced choices

Using comparisons_custom_pairwise_choice1/2 here matches render_custom_pairwise_inputs and build_custom_pairwise_contrast, so this test will stay in sync with the refactored server logic.

---

283-284: All-vs-one matrix_build test uses the correct choice key

Setting comparisons_all_vs_one_choice via NAMESPACE_STATMODEL is consistent with render_all_against_one_inputs and build_all_against_one_contrast.

---

326-329: Custom non-pairwise matrix_build test matches server expectations

The test now sets the comparison name and each weight using the same namespaced IDs that render_custom_non_pairwise_inputs, validate_contrast_inputs, and build_custom_non_pairwise_contrast read from, so the flow is fully covered.

---

377-379: Same-group validation test aligned with namespaced pairwise IDs

Using comparisons_custom_pairwise_choice1/2 here ensures check_cond() exercises the same validation path as the production UI when identical groups are selected.

---

415-418: Non-zero-sum validation test wired to namespaced non-pairwise IDs

The “BadComparison” case now drives check_cond() through the namespaced non-pairwise name and weight inputs, accurately reflecting the new input schema.

---

455-456: Mode-change reset test uses namespaced pairwise choices

Building the initial matrix with comparisons_custom_pairwise_choice1/2 keeps this reset test consistent with the rest of the contrast-building pipeline.

---

500-501: Duplicate-row guard test uses namespaced pairwise IDs

Both submissions in the duplicate-row test now use the namespaced pairwise choice IDs, so it continues to validate deduplication after the refactor.

R/module-statmodel-server.R (7)

44-45: All-vs-one selectInput now correctly uses namespaced choice ID

selectInput(ns(NAMESPACE_STATMODEL$comparisons_all_vs_one_choice), ...) matches the UI (build_all_vs_one_panel) and server readers, so the all-vs-one target group is consistently namespaced.

---

52-57: Custom pairwise selectInputs are properly namespaced

Both “Group 1” and “Group 2” selectors now use comparisons_custom_pairwise_choice1/2 through ns(), aligning with build_custom_pairwise_contrast and the updated tests.

---

64-67: Non-pairwise weight inputs use consistent namespaced pattern

Generating numeric inputs with IDs paste0(NAMESPACE_STATMODEL$comparisons_custom_nonpairwise_weights, i) matches how validation and matrix-building later read the weights and what tests expect.

---

91-100: Pairwise contrast builder now fully driven by namespaced inputs

The equality guard, index lookups, and label construction all read from comparisons_custom_pairwise_choice1/2, which is consistent with the UI and tests and avoids any stray hard-coded IDs.

---

119-133: Non-pairwise contrast builder uses namespaced weights and name

build_custom_non_pairwise_contrast now computes the weight sum, row entries, and rownames from the same namespaced keys used in the UI and tests, so the whole path is coherent after the refactor.

---

149-156: All-vs-one contrast builder reads choice via namespaced ID

Using input[[NAMESPACE_STATMODEL$comparisons_all_vs_one_choice]] both for index3 and label construction lines this helper up with render_all_against_one_inputs and the tests for all-vs-one contrasts.

---

462-535: Server wiring for contrast mode and actions consistently uses NAMESPACE_STATMODEL IDs

The conditional panel, Rownames, check_cond, matrix_build, and the clear-observer now all depend on the same namespaced comparison mode and submit/clear IDs as the UI and tests. This removes the previous hard-coded input names and centralizes all IDs under NAMESPACE_STATMODEL, which should make future renames much safer.