Skip to content

add claude.md#58

Merged
roninsightrx merged 2 commits into
masterfrom
claude-md
May 20, 2026
Merged

add claude.md#58
roninsightrx merged 2 commits into
masterfrom
claude-md

Conversation

@roninsightrx
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a CLAUDE.md guide intended for Claude Code usage in this repository, documenting PKPDmap’s purpose, key entry points, common dev commands, and high-level architecture.

Changes:

  • Introduces a repository guidance document (CLAUDE.md) with devtools-based workflows for local development.
  • Documents the estimation methods supported by get_map_estimates() and the main estimation pipeline components.
  • Summarizes key functions, parameter conventions, test layout, and CI behavior.

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

Comment thread CLAUDE.md
devtools::document()

# Full R CMD check (as done in CI)
devtools::check("--no-manual --as-cran")
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

devtools::check("--no-manual --as-cran") passes both flags as a single argument string; R CMD check expects them as separate args, and this doesn’t match the CI workflow (which passes --no-manual and --as-cran separately). Update the example to pass a character vector of args so local checks behave like CI.

Suggested change
devtools::check("--no-manual --as-cran")
devtools::check(args = c("--no-manual", "--as-cran"))

Copilot uses AI. Check for mistakes.
Comment thread CLAUDE.md
# Full R CMD check (as done in CI)
devtools::check("--no-manual --as-cran")

# Install PKPDsim dependency from GitHub (requires PAT_TOKEN)
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The comment says installing from GitHub “requires PAT_TOKEN”, but remotes::install_github() uses the GITHUB_PAT environment variable (and in CI this is populated from the PAT_TOKEN secret). Reword this to avoid confusion for local dev (e.g., mention GITHUB_PAT for local installs and that CI maps it from secrets.PAT_TOKEN).

Suggested change
# Install PKPDsim dependency from GitHub (requires PAT_TOKEN)
# Install PKPDsim dependency from GitHub (requires GITHUB_PAT locally; in CI this comes from secrets.PAT_TOKEN)

Copilot uses AI. Check for mistakes.
Comment thread CLAUDE.md
```
get_map_estimates()
├── check_inputs() # validate all inputs upfront
├── parse_*()/ # ~10 parse_ functions normalize inputs
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

In the pipeline diagram, parse_*()/ includes a trailing /, which reads like a typo and is inconsistent with the other entries. Consider removing the slash to keep the diagram clean.

Suggested change
├── parse_*()/ # ~10 parse_ functions normalize inputs
├── parse_*() # ~10 parse_ functions normalize inputs

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@jasmineirx jasmineirx left a comment

Choose a reason for hiding this comment

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

automated codereview brought up a valid point, otherwise lgtm

…2007)

Replace the incorrect ad-hoc CWRES normalization with proper FOCE-based
conditional weighted residuals. The Jacobian F = df/deta is computed via
central finite differences and used to derive both CWRES and the FOCE
variance-covariance matrix, replacing the more expensive numDeriv::hessian
computation when residuals=TRUE.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@roninsightrx roninsightrx merged commit 07556e6 into master May 20, 2026
6 checks passed
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.

3 participants