tidymodels-review-patterns

What this skill does

# Tidymodels Code Review Patterns

## Overview

Anti-pattern detection and best practices for tidymodels workflows based on "Tidy Modeling with R" (TMwR) principles. This skill enables systematic code review for data leakage, resampling violations, workflow issues, evaluation problems, and reproducibility concerns.

## Data Leakage Patterns (CRITICAL)

### DL-001: Recipe Fitted on Test Data

**Severity**: CRITICAL

**Anti-Pattern**:
```r
# WRONG: Fitting recipe on test data
rec <- recipe(outcome ~ ., data = test_data) |>
 prep()

# WRONG: prep() using test data
rec <- recipe(outcome ~ ., data = train_data) |>
  prep(training = test_data)
```

**Correct Pattern**:
```r
# CORRECT: Recipe always prepped on training data only
rec <- recipe(outcome ~ ., data = train_data) |>
  prep(training = train_data)

# BEST: Use workflow (handles automatically)
wf <- workflow() |>
  add_recipe(rec) |>
  add_model(model_spec)

fit <- fit(wf, data = train_data)
```

**Detection**: Look for `prep()` calls with test data or recipes defined on test sets.

---

### DL-002: Preprocessing Before Split

**Severity**: CRITICAL

**Anti-Pattern**:
```r
# WRONG: Normalizing before splitting
df_normalized <- df |>
  mutate(across(where(is.numeric), scale))

split <- initial_split(df_normalized)

# WRONG: Feature selection before split
important_vars <- df |>
  select(where(~cor(.x, df$outcome) > 0.3))

split <- initial_split(important_vars)
```

**Correct Pattern**:
```r
# CORRECT: Split first, then preprocess in recipe
split <- initial_split(df, strata = outcome)
train_data <- training(split)

rec <- recipe(outcome ~ ., data = train_data) |>
  step_normalize(all_numeric_predictors()) |>
  step_corr(all_numeric_predictors(), threshold = 0.9)
```

**Detection**: Any transformations (scale, normalize, mutate) applied before `initial_split()`.

---

### DL-003: Target Encoding Without Cross-Validation

**Severity**: CRITICAL

**Anti-Pattern**:
```r
# WRONG: Target encoding using full dataset statistics
rec <- recipe(outcome ~ ., data = train_data) |>
  step_lencode_mixed(all_nominal_predictors(), outcome = vars(outcome))

# Then prep without proper CV
prepped <- prep(rec)
```

**Correct Pattern**:
```r
# CORRECT: Target encoding within workflow with resampling
rec <- recipe(outcome ~ ., data = train_data) |>
  step_lencode_mixed(all_nominal_predictors(), outcome = vars(outcome))

wf <- workflow() |>
  add_recipe(rec) |>
  add_model(model_spec)

# Encoding computed fresh for each fold
cv_results <- fit_resamples(wf, resamples = vfold_cv(train_data))
```

**Detection**: `step_lencode_*` or `step_embed` used outside workflow with resampling.

---

### DL-004: Feature Selection Using Test Data

**Severity**: CRITICAL

**Anti-Pattern**:
```r
# WRONG: Selecting features based on test correlations
test_cors <- cor(test_data[, -1], test_data$outcome)
selected_vars <- names(test_cors[abs(test_cors) > 0.3])

# WRONG: Using test data for variable importance
importance <- varImp(model, data = test_data)
```

**Correct Pattern**:
```r
# CORRECT: Feature selection in recipe (computed on training only)
rec <- recipe(outcome ~ ., data = train_data) |>
  step_select_vip(all_predictors(), outcome = "outcome", threshold = 0.8)

# CORRECT: Or use recursive feature elimination with CV
rfe_results <- rfe_fit(
  wf,
  resamples = vfold_cv(train_data),
  sizes = c(5, 10, 15, 20)
)
```

**Detection**: Variable selection operations referencing test data.

---

### DL-005: prep() Called Before initial_split()

**Severity**: CRITICAL

**Anti-Pattern**:
```r
# WRONG: Prepping recipe on full data before split
full_rec <- recipe(outcome ~ ., data = full_data) |>
  step_normalize(all_numeric_predictors()) |>
  prep()

# Then splitting
split <- initial_split(full_data)
```

**Correct Pattern**:
```r
# CORRECT: Always split first
split <- initial_split(full_data, strata = outcome)
train_data <- training(split)

rec <- recipe(outcome ~ ., data = train_data) |>
  step_normalize(all_numeric_predictors())
# Don't prep manually - let workflow handle it
```

**Detection**: Sequence analysis - `prep()` appearing before `initial_split()`.

---

## Resampling Violations (MAJOR/CRITICAL)

### RS-001: Missing Stratified Sampling

**Severity**: MAJOR (CRITICAL for imbalanced data)

**Anti-Pattern**:
```r
# WRONG: No stratification for imbalanced outcome
split <- initial_split(df)  # outcome is 95%/5% imbalanced

# WRONG: Unstratified CV
folds <- vfold_cv(train_data, v = 10)
```

**Correct Pattern**:
```r
# CORRECT: Stratify by outcome
split <- initial_split(df, strata = outcome)

# CORRECT: Stratified CV
folds <- vfold_cv(train_data, v = 10, strata = outcome)

# CORRECT: For continuous outcomes, stratify by bins
split <- initial_split(df, strata = outcome, breaks = 4)
```

**Detection**: Missing `strata =` argument with classification outcomes or highly skewed continuous outcomes.

---

### RS-002: Evaluating Model on Training Data

**Severity**: CRITICAL

**Anti-Pattern**:
```r
# WRONG: Predictions on training data for evaluation
fit <- fit(wf, data = train_data)
preds <- predict(fit, train_data)
metrics <- yardstick::metrics(preds, truth = outcome, estimate = .pred)
```

**Correct Pattern**:
```r
# CORRECT: Evaluate on held-out test data
fit <- fit(wf, data = train_data)
preds <- predict(fit, test_data)
metrics <- yardstick::metrics(
  bind_cols(test_data, preds),
  truth = outcome,
  estimate = .pred
)

# BEST: Use resampling for robust estimates
cv_results <- fit_resamples(wf, resamples = folds)
collect_metrics(cv_results)
```

**Detection**: `predict()` and metrics computed on same data used for `fit()`.

---

### RS-003: Tuning Without Nested Cross-Validation

**Severity**: MAJOR

**Anti-Pattern**:
```r
# WRONG: Tune on same folds used for final evaluation
folds <- vfold_cv(train_data)

tune_results <- tune_grid(wf, resamples = folds)
best_params <- select_best(tune_results)

# Using same folds for "final" evaluation
final_wf <- finalize_workflow(wf, best_params)
fit_resamples(final_wf, resamples = folds)  # Overly optimistic!
```

**Correct Pattern**:
```r
# CORRECT: Nested CV or separate test set
# Option 1: Hold out test set for final evaluation
split <- initial_split(df, strata = outcome)
train_data <- training(split)
test_data <- testing(split)

inner_folds <- vfold_cv(train_data, strata = outcome)
tune_results <- tune_grid(wf, resamples = inner_folds)

# Final evaluation on untouched test set
final_fit <- fit(finalize_workflow(wf, select_best(tune_results)), train_data)
predict(final_fit, test_data)

# Option 2: Nested CV
outer_folds <- nested_cv(train_data, outside = vfold_cv(v = 5), inside = vfold_cv(v = 5))
```

**Detection**: Same resampling object used for both tuning and final evaluation.

---

### RS-004: Missing Random Seeds

**Severity**: MAJOR

**Anti-Pattern**:
```r
# WRONG: No seed before random operations
split <- initial_split(df)  # Non-reproducible

folds <- vfold_cv(train_data)  # Different each run

boot <- bootstraps(train_data)  # Non-reproducible
```

**Correct Pattern**:
```r
# CORRECT: Set seed before each random operation
set.seed(123)
split <- initial_split(df, strata = outcome)

set.seed(456)
folds <- vfold_cv(training(split), strata = outcome)

# Or use tidymodels control options
ctrl <- control_resamples(save_pred = TRUE)
```

**Detection**: Random operations (`initial_split`, `vfold_cv`, `bootstraps`, `mc_cv`) without preceding `set.seed()`.

---

### RS-005: Validation Set Reuse

**Severity**: CRITICAL

**Anti-Pattern**:
```r
# WRONG: Using validation set multiple times for decisions
val_split <- validation_split(train_data)

# First use: model selection
results1 <- fit_resamples(wf1, val_split)
results2 <- fit_resamples(wf2, val_split)
# Choose wf1 based on validation

# Second use: hyperparameter tuning
tune_results <- tune_grid(wf1, val_split)
# Choose best params based on same validation set

# Third use: final "evaluation" on same validation
final_results <- fit_resamples(final_w