docs: address Copilot review on PR #87 — 6 plan/spec corrections

- design doc: add oob=TRUE to gg_roc new signature (was omitted)
- design doc: replace proposed calc_roc_one_vs_rest() with existing
  .rf_prob_matrix/.rf_one_class_roc/.rf_macro_average_roc helpers;
  document that output uses sens/spec/pct (package contract), not fpr/tpr
- plan #87: add norm.votes=FALSE row-normalization to votes extraction step
- plan #87: tighten patchwork test expectation (iris 4-predictor default
  must return patchwork, not patchwork||ggplot, to catch list regressions)
- plan #88: add pct to per_class column assertions (both occurrences)
- plan #88: correct rfsrc per_class rationale (argument is for API
  discoverability, not to prevent "unused argument" error — rfsrc uses ...)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: ehrlinger

dev/plans/2026-05-21-rf-87-gg-variable-classification-plan.md

@@ -107,7 +107,10 @@ test_that("gg_variable.randomForest classification: plot returns patchwork for a
   rf <- randomForest::randomForest(Species ~ ., data = iris, ntree = 50L)
   gg <- gg_variable(rf)
   p  <- plot(gg)
-  expect_true(inherits(p, "patchwork") || inherits(p, "ggplot"))
+  # iris has 4 predictors, so the default (no xvar) path assembles a multi-panel
+  # patchwork.  Assert patchwork specifically so a regression to a bare list (#80)
+  # would be caught.
+  expect_s3_class(p, "patchwork")
 })
 
 test_that("gg_variable.randomForest classification: layer_data works on single-xvar plot", {
@@ -158,7 +161,14 @@ Find this exact code (near the end of `gg_variable.randomForest`):
   # stored as yhat.<classname> columns — the same shape gg_variable.rfsrc
   # produces.  For regression a single numeric yhat column suffices.
   if (object$type == "classification") {
-    preds           <- object$votes   # n × n_classes matrix of OOB vote fractions
+    preds  <- object$votes   # n × n_classes matrix; OOB vote fractions by default,
+                             # but raw integer counts when forest is fit with
+                             # norm.votes = FALSE.  Row-normalise unconditionally so
+                             # values are always in [0, 1] with rowSums ≈ 1.
+    rs     <- rowSums(preds)
+    if (any(rs > 1 + 1e-8, na.rm = TRUE)) {
+      preds <- preds / rs
+    }
     colnames(preds) <- paste0("yhat.", colnames(preds))
     gg_dta          <- cbind(gg_dta, preds)
     gg_dta$yvar     <- response

dev/plans/2026-05-21-rf-88-multiclass-roc-plan.md

@@ -81,7 +81,7 @@ test_that("gg_roc per_class=TRUE: long format with class column", {
   rf <- randomForest::randomForest(Species ~ ., data = iris, ntree = 100L)
   gg <- gg_roc(rf, per_class = TRUE)
   expect_true("class" %in% names(gg))
-  expect_true(all(c("sens", "spec") %in% names(gg)))
+  expect_true(all(c("sens", "spec", "pct") %in% names(gg)))  # pct = threshold; same 3-col contract as calc_roc
   expect_s3_class(gg$class, "factor")
   expect_equal(nlevels(gg$class), 3L)
 })
@@ -153,7 +153,11 @@ Replace with:
 gg_roc.rfsrc <- function(object, which_outcome, oob = TRUE, per_class = FALSE, ...) {
 ```
 
-Note: the rfsrc per_class path is out of scope for this PR (tracked under issue #72). The argument is added only so `gg_roc(rfsrc_obj, per_class = TRUE)` does not error with "unused argument". Leave the body unchanged.
+Note: the rfsrc per_class path is out of scope for this PR (tracked under issue #72).
+`gg_roc.rfsrc` already accepts `...`, so `per_class = TRUE` would currently be
+silently swallowed rather than erroring. The argument is added explicitly for API
+discoverability and to keep the signature consistent with `gg_roc.randomForest`.
+Leave the body unchanged — no per_class logic is wired up in the rfsrc method.
 
 - [ ] **Step 3: Replace the entire `gg_roc.randomForest` body**
 
@@ -302,7 +306,7 @@ test_that("gg_roc which_outcome='all' still returns macro-average (no class colu
   gg <- gg_roc(rf, which_outcome = "all")
   expect_false("class" %in% names(gg))
   # Macro-average returns a single data frame, not a class-faceted one
-  expect_true(all(c("sens", "spec") %in% names(gg)))
+  expect_true(all(c("sens", "spec", "pct") %in% names(gg)))  # pct = threshold; same 3-col contract as calc_roc
 })
 ```
 

dev/plans/2026-05-21-rf-finishing-work-design.md

@@ -179,14 +179,18 @@ macro-average meaning (backward compatible). No CI computation in this PR.
 **New signature:**
 
 ```r
-gg_roc(object, which_outcome = "all", per_class = FALSE, ...)
+gg_roc(object, which_outcome = "all", per_class = FALSE, oob = TRUE, ...)
 ```
 
+(`oob = TRUE` already exists in both dispatch methods; listed here for completeness.)
+
 **`per_class = TRUE` behaviour:**
 
 - For forests with > 2 classes: compute one-vs-rest ROC for every class *k*
   (scores = `votes[, k]`, positive = `(y == k)`), stack into a long data frame
-  with columns `class` (factor), `fpr`, `tpr`.
+  with columns `class` (factor), `sens`, `spec`, `pct` — the same three-column
+  contract used by the rest of the package (`calc_roc` returns `sens`/`spec`/`pct`;
+  `fpr = 1 − spec` and `tpr = sens` if desired by callers).
 - `attr(gg, "auc")` becomes a named numeric vector: one entry per class,
   ordered by descending AUC. Class factor levels follow the same order.
 - For binary forests: `per_class = TRUE` is a no-op — returns the single-curve
@@ -196,9 +200,15 @@ gg_roc(object, which_outcome = "all", per_class = FALSE, ...)
   integer): `per_class` wins, a `message()` informs the caller that
   `which_outcome` is ignored when `per_class = TRUE`.
 
-**Internal helper `calc_roc_one_vs_rest(scores, y, k)`** — single class OvR
-ROC, returns a data frame `(fpr, tpr)` plus scalar AUC. Extracted so it can be
-reused for both per-class computation and macro-average.
+**Internal helpers** — the implementation reuses existing private functions in
+`R/calc_roc.R` rather than introducing new ones:
+
+- `.rf_prob_matrix(object, oob)` — extracts and normalises the vote matrix
+- `.rf_one_class_roc(probs, y, k)` — single-class OvR ROC, returns
+  `data.frame(sens, spec, pct)` plus scalar AUC
+- `.rf_macro_average_roc(probs, y)` — macro-average across all classes
+
+No new top-level helpers are needed.
 
 ### Plot method (`R/plot.gg_roc.R`)