Skip to content

Unify CPM workflow around spec() and cpm() - #56

Open
psychelzh wants to merge 42 commits into
mainfrom
codex/cpm-unified-workflow
Open

psychelzh wants to merge 42 commits into
mainfrom
codex/cpm-unified-workflow

Conversation

@psychelzh

@psychelzh psychelzh commented Mar 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • replace the legacy threshold/k-fold API and the old fit()/fit_resamples() split with a single staged workflow built around spec() + cpm()
  • standardize on one fold-based cpm result object and simplify summary/tidy output to correlation-focused reporting
  • simplify the runtime/file layout, remove single-fit leftovers, and align docs, tests, pkgdown, and touchstone with the current workflow

API changes

  • spec() and cpm() are now the only public workflow entry points
  • remove cpm_spec(), fit(), fit_resamples(), cpm_resamples(), and resample_metrics()
  • stage helpers are now cpm_selection_cor(), cpm_construction_strength(), and cpm_model_lm()
  • cpm() runtime settings now center on spec, resamples, covariates, return_edges, and na_action
  • summary() / tidy() now report correlation-based CPM results only

Internal cleanup

  • consolidate the main runtime into R/cpm-spec.R, R/cpm.R, R/fit.R, and R/folds.R while keeping selection/construction as explicit CPM stages
  • remove the old single-fit path and related helper layers
  • realign tests/testthat to the current module boundaries
  • update touchstone benchmarks so main and this branch can be compared under the old and new APIs

Validation

  • air format .
  • devtools::document()
  • devtools::build_readme()
  • devtools::test() # 312 passing
  • pkgdown::build_site(preview = FALSE, install = TRUE)
  • covr::package_coverage() # 100%
  • devtools::check() # 0 errors / 0 warnings / 0 notes

@codecov

codecov Bot commented Mar 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (5d7ad83) to head (6a568f3).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##              main       #56    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            4         6     +2     
  Lines          346      1032   +686     
==========================================
+ Hits           346      1032   +686     

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

cpmr/R/cpm.R

Line 214 in 6a4913a

tibble::as_tibble(apply(sum_x$edges, 2, list))

P1 Badge Guard edge tidying when edges are not stored

tidy.cpm(component = "edges") unconditionally calls apply(sum_x$edges, 2, list), but sum_x$edges is NULL for the default cpm() workflow (return_edges = "none"). In that common path this raises a hard error (dim(X) must have a positive length) instead of returning a usable result or a clear user-facing message, so users cannot tidy edge output unless they know to override edge storage up front.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@psychelzh
psychelzh force-pushed the codex/cpm-unified-workflow branch from 0a57d41 to c1f6dfb Compare March 29, 2026 04:16
@psychelzh
psychelzh force-pushed the main branch 2 times, most recently from a729e18 to 7cfe746 Compare March 29, 2026 06:01
@psychelzh
psychelzh force-pushed the codex/cpm-unified-workflow branch 2 times, most recently from d42570c to b1d5be1 Compare March 29, 2026 07:36
@psychelzh

Copy link
Copy Markdown
Owner Author

/benchmark

@github-actions

Copy link
Copy Markdown
Contributor

Touchstone benchmark results for codex/cpm-unified-workflow versus main (with a 95% confidence interval in relative change):

  • 🚀workflow_default: 272ms -> 235ms [-15.2%, -12.53%]
  • 🚀workflow_standardize_edges: 499ms -> 392ms [-22.51%, -20.63%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

psychelzh and others added 19 commits March 29, 2026 20:54
* feat: add cpm spec and fit skeleton

* style: apply air formatting suggestions

* test: cover cpm_spec print branches

* docs: guide phase-two fit migration usage

* docs: rebuild README from README.Rmd

* Add GitHub Actions workflow for documentation generation

* devtools::document()

Signed-off-by: Liang Zhang <psychelzh@outlook.com>

* feat!: remove cpm() wrapper and finalize fit(cpm_spec) migration

- drop legacy cpm() entry-point and related docs
- keep fit(cpm_spec(), ...) as sole API path
- update tests, snapshots, README, and vignette for new API
- apply formatting fixes required by CI

BREAKING CHANGE: cpm() and confounds alias are removed.

* fix: resolve devtools::document warnings blocking CI

* refactor: drop local %||% and rely on base operator

---------

Signed-off-by: Liang Zhang <psychelzh@outlook.com>
Merged via squash. Completes semantics split, argument boundaries, result-object documentation, and accessor verbs. Remaining items (glance, augment, predict design) tracked in #34.
@psychelzh
psychelzh force-pushed the codex/cpm-unified-workflow branch from b1d5be1 to 6e264ce Compare March 29, 2026 13:00
@psychelzh

Copy link
Copy Markdown
Owner Author

/benchmark

@github-actions

Copy link
Copy Markdown
Contributor

Touchstone benchmark results for codex/cpm-unified-workflow versus main (with a 95% confidence interval in relative change):

  • 🚀workflow_default: 246ms -> 191ms [-24.62%, -20.72%]
  • 🚀workflow_standardize_edges: 513ms -> 343ms [-34.44%, -32.06%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant