Make diagnostic color matching explicit for neff

[utkarshpawade]

Fixes #469

This PR removes reliance on partial matching when resolving diagnostic names for color/fill scales.

Previously the wrapper accepted neff while internal helpers expected neff_ratio, which worked only because of partial matching. This change makes the accepted names explicit.

Changes

• Accept rhat, neff, and neff_ratio
• Normalize neff to neff_ratio internally
• Add tests for both names

[Copilot]

Pull request overview

Removes reliance on R’s partial argument matching for diagnostic name resolution in color/fill scale helpers, making "neff" vs "neff_ratio" handling explicit (Fixes #469).

Changes:

• Expanded diagnostic name matching to explicitly include "neff" and "neff_ratio", normalizing "neff" to "neff_ratio" internally.
• Added test coverage to ensure "neff" / "neff_ratio" inputs don’t error.
• Documented the behavior change in NEWS.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
tests/testthat/test-mcmc-diagnostics.R	Adds regression test for explicit handling of "neff" vs "neff_ratio"
R/mcmc-diagnostics.R	Makes accepted diagnostic names explicit and normalizes "neff" to "neff_ratio"
NEWS.md	Notes the explicit handling and removal of reliance on partial matching

Comments suppressed due to low confidence (1)

R/mcmc-diagnostics.R:432

• The PR description says the helpers accept rhat, neff, and neff_ratio, but scale_fill_diagnostic() (and likely scale_color_diagnostic() as well) still only declare c("rhat", "neff"), which will reject "neff_ratio" (it won’t partial-match a longer string to "neff"). Consider updating these wrapper defaults/choices to include "neff_ratio" (and rely on diagnostic_color_scale() to normalize "neff" → "neff_ratio"), so the documented API matches actual behavior.

scale_fill_diagnostic <- function(diagnostic = c("rhat", "neff")) {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.66%. Comparing base (bcf5ada) to head (4d335f2).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #470   +/-   ##
=======================================
  Coverage   98.66%   98.66%           
=======================================
  Files          35       35           
  Lines        5860     5860           
=======================================
  Hits         5782     5782           
  Misses         78       78           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jgabry]

The issue is real but this solution seems way too complicated. I think we can avoid all these changes by just switching the diagnostic argument to scale_color_diagnostic from diagnostic = c("rhat", "neff") to diagnostic = c("rhat", "neff_ratio") and then replacing:

scale_color_diagnostic("neff") -> scale_color_diagnostic("neff_ratio")

wherever that's used. Same for scale_fill_diagnostic. Does that make sense?

These are internal functions so we don't need to worry about breaking backwards compatibility, so we shouldn't accept both neff and neff_ratio if we don't need to.

[utkarshpawade]

Yes, that makes sense and it aligns with a minimal change approach; I’ll inspect the current branch state (including any recent edits) and then simplify the implementation exactly as requested.