Analyte-neutral wording for the QC and statmodel pages - #225
Conversation
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR replaces protein-specific UI terminology with analyte- and feature-level terminology across QC, modeling, visualization, and heatmap interfaces. Related heatmap tests now expect the updated labels. Internal option values and processing behavior remain unchanged. ChangesAnalyte terminology updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/testthat/test-statmodel-ui-options-visualization.R (1)
23-25: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the preserved clustering values in this test.
The implementation must continue emitting
protein,comparison, andbothas option values. The updated test checks only display labels, so it does not protect the contract used byR/plotting_funtions.R.Proposed test additions
expect_true(grepl("analyte dendrogram", ui_html)) expect_true(grepl("comparison dendrogram", ui_html)) expect_true(grepl("analyte and comparison dendrograms", ui_html)) + expect_true(grepl('value="protein"', ui_html)) + expect_true(grepl('value="comparison"', ui_html)) + expect_true(grepl('value="both"', ui_html))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/testthat/test-statmodel-ui-options-visualization.R` around lines 23 - 25, Update the UI options test around the existing dendrogram assertions to also verify that the generated HTML preserves the option values “protein”, “comparison”, and “both”. Keep the current display-label checks and assert each value is emitted, protecting the contract consumed by the plotting code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@R/module-qc-ui.R`:
- Around line 240-241: Update the tooltip text in the abundance summary UI near
h4("Download summary of abundance") to correct the grammar by changing “each
biological samples” to “each biological sample” while preserving the rest of the
message.
In `@R/statmodel-ui-options-visualization.R`:
- Line 96: Update the tooltip text in the visualization options UI to exactly
describe ordering by using analyte means, comparison means, or both, with the
requested capitalization and a terminating period. Keep the existing div
structure and class unchanged.
- Line 77: Update the remaining protein-specific user-facing text in the
statmodel visualization page, including the strings near the heatmap note and
the references to “protein abundances” and “one comparison/protein.” Replace
them with consistent analyte-focused terminology, preserving wording that is
intentionally protein-specific.
---
Nitpick comments:
In `@tests/testthat/test-statmodel-ui-options-visualization.R`:
- Around line 23-25: Update the UI options test around the existing dendrogram
assertions to also verify that the generated HTML preserves the option values
“protein”, “comparison”, and “both”. Keep the current display-label checks and
assert each value is emitted, protecting the contract consumed by the plotting
code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8217c88e-6cc1-465c-8e5f-deb828a4e022
📒 Files selected for processing (6)
R/module-qc-ui.RR/qc-server-plots.RR/statmodel-server-results-table.RR/statmodel-server-visualization.RR/statmodel-ui-options-visualization.Rtests/testthat/test-statmodel-ui-options-visualization.R
| create_heatmap_options <- function(ns) { | ||
| tagList( | ||
| h4("Note: Only one page will be shown in browser. To view all proteins please view this plot as a pdf. Heatmaps require at least two comparisons."), | ||
| h4("Note: Only one page will be shown in browser. To view all analytes please view this plot as a pdf. Heatmaps require at least two comparisons."), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the remaining protein-specific copy from the statmodel visualization page.
This change makes the heatmap controls analyte-focused, but the same page still shows protein abundances at Line 121 and one comparison/protein at Line 147. Users will see mixed terminology. Update those strings unless they are intentionally protein-specific.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@R/statmodel-ui-options-visualization.R` at line 77, Update the remaining
protein-specific user-facing text in the statmodel visualization page, including
the strings near the heatmap note and the references to “protein abundances” and
“one comparison/protein.” Replace them with consistent analyte-focused
terminology, preserving wording that is intentionally protein-specific.
| class = "icon-wrapper", | ||
| icon("question-circle", lib = "font-awesome"), | ||
| div("Determines how to order proteins and comparisons. protein means, comparison means, or both", class = "icon-tooltip") | ||
| div("Determines how to order analytes and comparisons. analyte means, comparison means, or both", class = "icon-tooltip") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the cluster-analysis tooltip a complete sentence.
Use Determines how to order analytes and comparisons using analyte means, comparison means, or both. This also capitalizes Analyte correctly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@R/statmodel-ui-options-visualization.R` at line 96, Update the tooltip text
in the visualization options UI to exactly describe ordering by using analyte
means, comparison means, or both, with the requested capitalization and a
terminating period. Keep the existing div structure and class unchanged.
2 participants
Motivation and Context
The QC and statmodel pages used protein-specific wording. This wording does not fit all analyte types, including metabolomics data. The changes replace protein-specific user-facing text with analyte-neutral terminology without changing processing behavior.
Changes
ALL PROTEINStoALL ANALYTES.Tests
tests/testthat/test-statmodel-ui-options-visualization.R.Coding Guidelines
No coding guideline violations are evident in the reviewed changes.