Add AGENTS.md agent instructions

[roninsightrx]

Renames CLAUDE.md to AGENTS.md and updates its contents.

Why AGENTS.md

AGENTS.md is the cross-harness convention for agent instructions, so the same file is picked up by other AI coding agent tooling rather than only by Claude Code. The wording is now harness-agnostic ("agents" / "AI coding agents") instead of referring to one specific assistant.

What the file contains

• Package overview and the PKPDsim dependency (GitHub-only, via Remotes:).
• Common commands — devtools::load_all(), devtools::test(), single-file test filtering, devtools::document(), devtools::check("--no-manual --as-cran").
• Architecture — the estimation pipeline from get_map_estimates() through the parse_* functions, mle_wrapper(), calc_residuals() and get_varcov_matrix(); a key-function table; parameter conventions (ETA, omega, IOV, fixed).
• Test organization and the CI workflow.

Corrections to the previous content

While verifying the existing text against the source, a few things turned out to be inaccurate:

• It said get_map_estimates() "supports five estimation methods via the method argument". In fact method is the optim() method (default "BFGS"); the estimation approach is chosen by type, which check_inputs() / parse_weight_prior() accept as "map", "pls" or "ls". The two are now documented separately.
• It listed methods (np, np_hybrid, get_np_estimates()) that do not exist on master.
• weight_prior — the actual control over prior strength, given on the SD scale and squared into weight_prior_var — was undocumented.
• Noted that README.md still advertises map_flat_prior while the code accepts pls.

Newly documented behavior that requires reading several files to discover:

• Models without a cpp attribute silently fall back to ll_func_generic().
• With residuals = TRUE the numDeriv::hessian() call is skipped and the FOCE Jacobian vcov is preferred over fit$vcov — so that path drives both vcov and CWRES.
• Mixture-model fitting, the shape of the returned map_estimates object, and the fact that optimizer failures return the error object instead of throwing.
• tests/testthat/nm/ NONMEM fixtures use hard-coded reference values because NONMEM is licensed and not run in CI.

Verification

Claims about commands and layout were checked against the repo: devtools::load_all() succeeds, the argument defaults (type = "map", method = "BFGS") match, and both devtools::test() and devtools::test(filter = "get_map_estimates") run with no failures (pre-existing warnings only). No CLAUDE.md remains in the repo.

🤖 Generated with Claude Code

[roninsightrx]

Changes requested: AGENTS.md contains factual instructions that would mislead coding agents.

• AGENTS.md:32-33 — devtools::check("--no-manual --as-cran") passes that string as the positional pkg argument rather than as check flags, so it attempts to check a package path with that name. Replace it with devtools::check(args = c("--no-manual", "--as-cran")), matching .github/workflows/R-CMD-check.yaml:51-55.

• AGENTS.md:35-36 — PKPDsim is a public repository, so remotes::install_github("InsightRX/PKPDsim") does not require PAT_TOKEN; moreover, remotes recognizes GITHUB_PAT, not a variable named PAT_TOKEN. Say that a token is optional for rate limits, or explain separately that CI maps secrets.PAT_TOKEN to GITHUB_PAT.

• AGENTS.md:62-66 — weight_prior_var == 0 does not produce a usable least-squares fallback. Although get_map_estimates() selects calc_ofv_ls, it subsequently evaluates omega$est / weight_prior_var and solve(omega$est / weight_prior_var) while building the optimizer data, so zero causes non-finite values/an error before fitting. Tell agents to use type = "ls"; do not document zero prior weight as supported unless the implementation is fixed.

• AGENTS.md:129-131 — parse_omega_matrix() does not accept CV% as an input format. It accepts a full covariance matrix or a lower-triangle vector of variances/covariances. create_block_from_cv() can create a diagonal lower-triangle block from a CV fraction, but that is a separate conversion. Correct the listed accepted formats.

• AGENTS.md:138-145 — The return and error behavior is stated too broadly. Residual/g.o.f. fields are added only when residuals = TRUE (and mahalanobis ends up NULL when they are skipped). Also, only the non-mixture mle_wrapper() call is inside the tryCatch that returns an error object; mixture fits and errors elsewhere propagate normally. Qualify both statements so callers do not rely on fields or error handling that are not guaranteed.

Reviewed by Codex

[roninsightrx]

Addressed the Codex review — all five findings verified against source and fixed in e5e36e8.

1. devtools::check("--no-manual --as-cran") — correct. args(devtools::check) confirms the first positional is pkg. Changed to devtools::check(args = c("--no-manual", "--as-cran")), matching the workflow. Verified it runs (0 errors; the pre-existing warning/notes are unrelated to this PR, and CI uses error-on: "error").
2. PAT_TOKEN for installing PKPDsim — correct; gh api repos/InsightRX/PKPDsim reports public. Comment now says no token is needed, with GITHUB_PAT as an optional rate-limit workaround. The CI section already documented the secrets.PAT_TOKEN → GITHUB_PAT mapping separately, so that was left as-is.
3. weight_prior_var == 0 — correct. get_map_estimates.R:139 selects calc_ofv_ls, but lines 259-261/321-323 still evaluate omega$est / weight_prior_var and solve() on it. Now documented as unsupported, pointing to type = "ls".
4. omega input formats — correct. parse_omega_matrix() takes a matrix or a lower-triangle vector only; CV% is removed and create_block_from_cv() is described as the separate helper it is.
5. return/error behavior overclaimed — correct. Residual/g.o.f. fields are behind if(residuals) (line 360) and get_mahalanobis() returns NULL without ipred; only the non-mixture mle_wrapper() call is inside the tryCatch (line 302). Both statements qualified.

No findings were rejected. devtools::test() still passes (318 pass, 0 fail). File remains harness-agnostic.

🤖 Generated with Claude Code

[roninsightrx]

The five original findings are resolved, but remaining factual inaccuracies still prevent approval.

• AGENTS.md:94-99 — The Hessian-skipping description is only true for the non-mixture branch. R/get_map_estimates.R:243-249 passes the user-supplied skip_hessian directly for every mixture fit; only the non-mixture branch creates skip_hessian_mle <- skip_hessian || residuals at lines 297-309. Also, the numDeriv Hessian path does not affect CWRES: CWRES comes from calc_cwres(), while obj$fit$vcov is only a variance-covariance source/fallback. Qualify this paragraph as non-mixture behavior and remove the claim that both paths affect CWRES.

• AGENTS.md:47-61 — Neither check_inputs() nor parse_weight_prior() validates or collectively recognizes exactly the three listed values. check_inputs() only special-cases map/pls, parse_weight_prior() only special-cases pls, and an arbitrary string is not rejected and falls through with MAP-like behavior. Reword this as implementation special cases in get_map_estimates()/parse_weight_prior(), and explicitly note that type is currently not validated if agents need to rely on that behavior.

• AGENTS.md:83,114 — ll_func_PKPDsim() computes the optimization objective, not a log-likelihood. calc_ofv_map() returns contributions on the -2 * log(likelihood) scale and ll_func_PKPDsim() sums them. Change both descriptions to “objective function (OFV / -2 log-likelihood)” so agents do not get the sign and scale wrong when modifying likelihood or Hessian code.

• AGENTS.md:123 — check_inputs() is described as comprehensive, but its own source calls it basic and it checks only required inputs for map/pls, censoring type, model class, and parameter names; omega/data/error/weights validation is distributed among later parsers. Change this role to “basic early validation” rather than suggesting all validation is centralized there.

Reviewed by Codex

[jasmineirx]

lgtm except one point of accuracy. plus codex had some outstanding comments i think.

[jasmineirx]

PKPDsim is on CRAN: https://cran.r-project.org/web/packages/PKPDsim/index.html

Should we correct this to avoid confusion?

[roninsightrx]

Good catch — fixed in 40c1ead: the sentence now says PKPDsim is on CRAN, and notes that DESCRIPTION still carries a Remotes: InsightRX/PKPDsim entry so installs pull the GitHub development version.

[roninsightrx]

Addressed both rounds of feedback in 40c1ead. All findings verified against source; none were rejected.

@jasmineirx — PKPDsim CRAN status. Correct. PKPDsim is on CRAN (1.4.1). DESCRIPTION does still have Remotes: InsightRX/PKPDsim, so the sentence now reads: on CRAN, but the Remotes: entry means installs pull the GitHub development version rather than the CRAN release.

Codex follow-up:

1. Hessian skip / CWRES — correct on both counts. The mixture loop passes the user-supplied skip_hessian straight through (get_map_estimates.R:249); only the non-mixture branch computes skip_hessian_mle. And CWRES comes from calc_cwres() via the FOCE Jacobian (calc_residuals.R:123,155), independent of the numDeriv Hessian, which only feeds obj$fit$vcov. Paragraph now scoped to the non-mixture branch, and the CWRES claim is replaced with an explicit note that CWRES is unaffected.
2. type not validated — correct. check_inputs() only special-cases map/pls, parse_weight_prior() only pls; an arbitrary string falls through with MAP-like behavior and no error. Reworded as implementation special cases, with the lack of validation called out explicitly.
3. OFV vs log-likelihood — correct. calc_ofv_map() returns contributions on the -2 * log(likelihood) scale and ll_func_PKPDsim() returns sum(ofv). Both the pipeline diagram and the file table now say objective function (OFV, -2 * log(likelihood)).
4. check_inputs() "comprehensive" — correct; its own roxygen title is "Some basic input checking". Re-described as basic early validation (required args for map/pls, censoring type, model class, parameter names), noting the rest is distributed among the parse_*() functions.

🤖 Generated with Claude Code