Add plot_serocurve() for population-level serodynamic curve visualization

[Copilot]

• Fix | (OR) assertions in test-internal-popparams.R:106-107 to use all() ensuring both subjects and parameters are present
• Fix non-standard dplyr::rename(c("new" = "old", ...)) to standard rename(new = "old", ...) form in Run_Mod.R
• Fix non-idiomatic dplyr::select(!c(...)) to select(-all_of(...)) in Run_Mod.R

[sschildhauer]

@copilot Please create a new branch for this PR entitlted plot_serocurve. Also, this PR should use the code from branch mon_popparams, which as already established an attribute that holds mu.par. Please do not modify any other existing functions or tests in order to create the new plot_serocurve function. Do not modify run_mod or its outputs from the mon_popparams branch. The visualizations look good as is. Keep all edits in this thread. Do not open another pull request.

[Copilot]

@sschildhauer I've opened a new pull request, #194, to work on those changes. Once the pull request is ready, I'll request review from you.

[sschildhauer]

@copilot Please create a new branch for this PR entitlted plot_serocurve. Also, this PR should use the code from branch mon_popparams, which as already established an attribute that holds mu.par. Please do not modify any other existing functions or tests in order to create the new plot_serocurve function. Do not modify run_mod or its outputs from the mon_popparams branch. The visualizations look good as is. Keep all edits and recommendations in this thread. Do not open another pull request.

[Copilot]

@copilot Please create a new branch for this PR entitlted plot_serocurve. Also, this PR should use the code from branch mon_popparams, which as already established an attribute that holds mu.par. Plea...

Done (commit 7c8e12b). I've reverted all changes to run_mod() and its tests, restoring the full mon_popparams branch implementation (including unpack_jags(), prep_popparams(), ex_popparams(), param_recode() helpers and their tests).

plot_serocurve() has been updated to work with the mon_popparams population_params format — it filters to Population_Parameter == "mu.par", pivots wider, and transforms the log-scale values to the original parameter scale before computing curves. nepal_sees_jags_output has been regenerated with with_pop_params = TRUE so the examples and tests work out of the box.

@copilot Please create a new branch for this PR entitlted plot_serocurve. Also, this PR should use the code from branch mon_popparams, which as already established an attribute that holds mu.par. Plea...

Done (commit 7c8e12b). This PR is on branch copilot/create-plot-serocurve-function and uses the mon_popparams branch's existing run_mod() implementation — no modifications were made to run_mod() or its tests. The only additions are the plot_serocurve() function and its examples/tests.

[sschildhauer]

@copilot please address all feedback and comments given in this review. Please keep all changes in the same thread for this pull request.

[Copilot]

@copilot please address all feedback and comments given in this review. Please keep all changes in the same thread for this pull request.

All review feedback addressed in commit a7974e0:

• show_all_curves and alpha_samples parameters removed
• "newperson" option description updated to reference the "predictive distribution"

[sschildhauer]

Looks good

[sschildhauer]

@claude, Please fix the warning in the plot_serocurve test set.

[claude]

Claude finished @sschildhauer's task in 3m 0s —— View job

---

Fix warning in plot_serocurve test set

• Inspect the plot_serocurve code and tests
• Identify the warning in the test set
• Apply the fix
• Verify locally (all 7 tests pass with 0 warnings)
• Commit and push (54d5fe7)

---

What was wrong

CI surfaced this warning in R-CMD-check, which _R_CHECK_FORCE_SUGGESTS / Tests generated warnings. escalates to a failure:

── Warning ('test-plot_serocurve.R:46:5'): plot_serocurve() works with population param_source (default) ──
No shared levels found between `names(values)` of the manual scale and the data's fill values.

The triggering case was show_ci = FALSE (serocurve-population-no-ci). In R/plot_serocurve.R, the scale_fill_manual(values = c("ci" = "red")) block was inside if (!multi_strat) but not gated on show_ci, so when no ribbon was drawn, ggplot2 found no fill aesthetic for the manual scale to match.

The fix

Wrapped scale_fill_manual() in if (show_ci) so it only gets added when a CI ribbon actually exists. scale_color_manual() is still applied (the median line always needs its legend entry).

if (!multi_strat) {
  p <- p + ggplot2::scale_color_manual(...)
  if (show_ci) {
    p <- p + ggplot2::scale_fill_manual(...)
  }
}

Verification

Ran testthat::test_file("tests/testthat/test-plot_serocurve.R") locally on R 4.6.0:

[ FAIL 0 | WARN 0 | SKIP 0 | PASS 7 ]

All 7 tests pass with zero warnings. The existing vdiffr snapshot for serocurve-population-no-ci still matches because the removed scale was a no-op (no fill aesthetic was on the plot).

Branch ·

[sschildhauer]

@claude, please review PR.

[claude]

Claude finished @sschildhauer's task in 2m 26s —— View job

---

PR Review

• Gather context: read the PR diff vs origin/main
• Read R/plot_serocurve.R and its tests
• Verify previously-flagged issues were addressed
• Check pkgdown YAML, NEWS.md, DESCRIPTION, WORDLIST, CI status
• Post comprehensive review

---

Summary

This PR has been substantially scoped down from earlier iterations: it now adds only plot_serocurve() and its associated example, docs, vdiffr snapshots, NEWS entry, WORDLIST entries, and pkgdown reference entry. R/as_case_data.R and all as_case_data snapshots are now identical to origin/main (the revert request was honored). R/Run_Mod.R differs from main by exactly one whitespace character, and R/use_att_names.R / R/nepal_sees_jags_output.R only have docstring tweaks. The version is correctly bumped (0.0.0.9056) above main (0.0.0.9055).

CI status: all substantive checks pass — R-CMD-check, test-coverage, spellcheck, version check, pkgdown, lint, NEWS check, docs check, README check. (The one "Claude Code Review" failure is the bot run, not a test failure.) The previously failing as_case_data::results_are_consistent_with_simulated_data test is now green after the r45 snapshot sync.

---

Findings — R/plot_serocurve.R

1. Silent empty plot when param_source = "newperson" and no newperson subject exists (moderate — unchanged from prior review)

R/plot_serocurve.R:111-130 — if no rows match Subject == "newperson", the filter returns an empty tibble and the downstream reframe() / summarise() silently produce an empty plot. This is the same finding flagged in the May 30 review; it has not been addressed in this iteration. A user who fits without with_pop_params = TRUE and tries param_source = "newperson" could get a blank plot with no actionable error.

Suggested guard, parallel to the existing population_params check at lines 61-70:

} else {
  param_samples <- model |>
    dplyr::filter(
      .data$Subject == "newperson",
      .data$Iso_type %in% .env$antigen_iso,
      .data$Stratification %in% .env$strat
    ) |>
    ...

  if (nrow(param_samples) == 0) {
    cli::cli_abort(
      c("No {.val newperson} subject found in {.arg model} for the requested {.arg antigen_iso}/{.arg strat}.",
        "i" = "Ensure the model was fit with a `newperson` subject included.")
    )
  }

Fix this →

2. Hardcoded time grid (minor — unchanged from prior review)

R/plot_serocurve.R:135 — tx <- seq(0, 1200, by = 5) is hardcoded. If xlim = c(0, 100) is passed, 240 points are still computed; if a study runs beyond 1200 days, the curve is silently truncated regardless of xlim. Consider clamping tx to xlim when supplied, or exposing t_max / by arguments. Not a blocker for merge.

3. antigen_iso_col set identically in both branches (trivial — unchanged from prior review)

R/plot_serocurve.R:107 and R/plot_serocurve.R:131 — both branches assign antigen_iso_col <- "Iso_type". Lift the assignment above the if/else to remove the duplication. Trivial.

4. Implicit coupling between model and population_params for default antigen_iso / strat (minor)

R/plot_serocurve.R:44-45 — defaults are taken from unique(model$Iso_type) and unique(model$Stratification), but in the population branch the actual data filtered is attr(model, "population_params"). In practice these align, but the implicit assumption is worth a roxygen note or, ideally, defaulting from the populated source per branch.

---

Findings — pkgdown/_pkgdown.yml

5. Orphaned single-function reference section (minor — unchanged from prior review)

pkgdown/_pkgdown.yml:39-41 — the new "Visualize model results" section contains only plot_serocurve and sits directly after the existing plot-function group (plot_jags_trace, plot_jags_effect, plot_jags_dens, plot_predicted_curve) which serve the same purpose. Either fold plot_serocurve into the existing group or rename the existing group to encompass both. Cosmetic.

---

What looks good

• as_case_data fully reverted to origin/main — no diff in R/as_case_data.R or any as_case_data snapshot. Revert request from the latest review is honored, and the test should now pass on R 4.5.
• run_mod left alone — the only diff in R/Run_Mod.R is one whitespace character at line 222; behavior unchanged. Matches the "do not modify run_mod" directive.
• Correct log-scale inversion for mu.par (R/plot_serocurve.R:93-99): y0 = exp(log_y0), y1 = y0 + exp(log_y1), shape = exp(log_shape) + 1 correctly invert the JAGS reparameterization. The asymmetry vs. the newperson branch (no transforms) is intentional and correct.
• Earlier review feedback addressed: the show_ci = FALSE warning is fixed (R/plot_serocurve.R:224-248), show_all_curves / alpha_samples were removed, and the "newperson" doc text now references the "predictive distribution".
• Robust tidyeval — consistent .data$ / .env$ pronouns throughout; no R CMD check notes expected.
• Helpful error when population_params is missing (R/plot_serocurve.R:60-70) — uses cli::cli_abort with actionable guidance.
• vdiffr snapshot coverage — 6 scenarios (single-strat, multi-strat coloured, facet-by-strat, facet-by-antigen, no-CI, newperson). Solid.
• Version bumped to 0.0.0.9056, WORDLIST adds Serodynamic/serodynamic, NEWS entry is accurate (references mu.par and newperson correctly, no longer claims run_mod() stores values on the original scale).

---

Action items before merge

Priority	Item
Moderate	Add empty-result guard for the newperson path (finding 1)
Minor	Hardcoded tx grid (finding 2) — defer if desired
Minor	Lift duplicated antigen_iso_col assignment (finding 3)
Minor	Document or fix antigen_iso/strat default-source asymmetry (finding 4)
Cosmetic	Merge plot_serocurve into the existing plot reference group (finding 5)

None of these block merge. Finding 1 is the only one I'd flag as worth doing before shipping — the rest are polish.
· Branch

[sschildhauer]

@claude, please make the suggested changes in this thread.

[claude]

Claude finished @sschildhauer's task in 3m 28s —— View job

---

Applied suggested changes from this thread

• Read current state of R/plot_serocurve.R and pkgdown/_pkgdown.yml
• Moderate: Added empty-result guard for the newperson path
• Minor: Clamped hardcoded tx grid to xlim when provided
• Minor: Lifted duplicated antigen_iso_col assignment
• Minor: Documented antigen_iso/strat default-source asymmetry
• Cosmetic: Merged plot_serocurve into existing plot pkgdown reference section
• Ran tests locally ([ FAIL 0 | WARN 0 | SKIP 0 | PASS 7 ])
• Committed & pushed as f543317

---

What changed

R/plot_serocurve.R

1. Empty-result guard (newperson path) — after filtering for Subject == "newperson", errors with cli::cli_abort when no rows match, directing the user to ensure the model was fit with a newperson subject. Previously, an empty filter produced a silent empty plot.

   if (nrow(newperson_rows) == 0) {
     cli::cli_abort(
       c("No {.val newperson} subject found in {.arg model} for the requested {.arg antigen_iso}/{.arg strat}.",
         "i" = "Ensure the model was fit with a {.val newperson} subject included.")
     )
   }

2. tx grid clamps to xlim — when xlim is supplied, tx <- seq(xlim[1], xlim[2], by = 5); otherwise the previous default seq(0, 1200, by = 5). Avoids computing 240 points across 0-1200 when only a narrow window is shown.

3. Lifted antigen_iso_col — set once before the if/else rather than duplicated in both branches.

4. Documented default-source asymmetry — roxygen for antigen_iso / strat now notes the defaults come from the subject-level draws (model$Iso_type, model$Stratification) while the population branch reads from attr(model, "population_params"), and that in normal usage these match.

pkgdown/_pkgdown.yml

5. Moved plot_predicted_curve from "Model diagnostics" into the "Visualize model results" section alongside plot_serocurve, so the new section is no longer a single-function orphan and the diagnostics section is purely diagnostic.

Verification

> testthat::test_file("tests/testthat/test-plot_serocurve.R")
[ FAIL 0 | WARN 0 | SKIP 0 | PASS 7 ]

Also manually verified:

• The new cli::cli_abort fires with the right message when the newperson subject is filtered out of the input.
• xlim = c(0, 100) now produces 21 grid points (seq(0, 100, by = 5)) instead of 241.
  · Branch

[sschildhauer]

@Kwan-Jenny, This PR is ready for review

[sschildhauer]

Looks good

[Kwan-Jenny]

Looks good. The function is well documented and the examples/snapshots cover the main cases.

One small thing:

• nepal_sees_jags_output.rda grew from ~226KB to ~731KB (from the new population_params). Makes sense, just flagging it since it ships with the package.

[sschildhauer]

Looks good