Align risperidone evaluation repo with QC-ready report structure#3
Open
SRuedesh wants to merge 5 commits into
Open
Align risperidone evaluation repo with QC-ready report structure#3SRuedesh wants to merge 5 commits into
SRuedesh wants to merge 5 commits into
Conversation
- Add evaluation report input content and updated evaluation plan - Patch risperidone model snapshot for QC findings and observed-data mappings - Split GOF diagnostics by compound and align plotted outputs with observed data - Add local ignored render workflow and justfile helpers for render and cleanup - Update README and references
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds CI/QA workflows and assets, a structured evaluation_plan.json and markdown report content for a risperidone PBPK evaluation, an R function to generate the qualification report, and README/documentation updates. ChangesModel Evaluation and QA Setup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Evaluation/Input/Content/Section2_Methods.md (1)
1-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMethods section file is empty.
This file currently contributes no content, which leaves a missing section in the report structure. Please add at least the section heading/body or remove the file from the report composition until content is ready.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Evaluation/Input/Content/Section2_Methods.md` around lines 1 - 2, The Methods section file Section2_Methods.md is empty; add a minimal Methods heading and content (e.g., "## Methods" plus at least one paragraph describing methodology) or remove Section2_Methods.md from the report composition/import list so the build doesn't include an empty section; update whichever module references Section2_Methods.md (the report assembler or manifest that imports this file) to reflect the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/CrossRefCheckWithAnchors.yml:
- Line 13: The workflow is referencing a reusable workflow with a mutable ref
("uses:
Open-Systems-Pharmacology/Workflows/.github/workflows/MarkdownCrossrefCheck_withAnchors.yml@main");
replace the `@main` ref with a specific commit SHA for that repository so the
action is pinned to an immutable revision (update the line containing
"Open-Systems-Pharmacology/Workflows/.github/workflows/MarkdownCrossrefCheck_withAnchors.yml@main"
to use the chosen commit SHA, e.g. `@<commit-sha>`), and ensure the selected SHA
corresponds to the tested commit in the upstream repo.
In @.github/workflows/MarkdownLinksCheck.yml:
- Line 13: Replace the floating ref to the reusable workflow by pinning the uses
string to a specific commit SHA: update the uses value
"Open-Systems-Pharmacology/Workflows/.github/workflows/MarkdownLinksCheck.yml@main"
in the .github/workflows/MarkdownLinksCheck.yml workflow to the same repository
path but with a concrete commit hash (e.g., @<commit-sha>) so the workflow is
reproducible and stable; ensure you pick the desired commit from the
Open-Systems-Pharmacology/Workflows repo and use that SHA in the uses entry.
In @.github/workflows/SpellChecker.yml:
- Line 11: The workflow uses a mutable ref in the step "uses:
Open-Systems-Pharmacology/Workflows/.github/workflows/SpellChecker.yml@main"
which can change behavior unexpectedly; replace the trailing `@main` with a
specific immutable commit SHA (or a signed tag) from the upstream repository for
the SpellChecker reusable workflow so the action is pinned to a fixed revision
and update the ref string in that uses entry (keep the rest of the uses path the
same).
In @.github/workflows/ValidateEvaluationPlan.yml:
- Line 11: The reusable-workflow reference in the Validate-Evaluation-Plan job
(Validate-Evaluation-Plan) currently uses a mutable ref
"Open-Systems-Pharmacology/Workflows/.github/workflows/ValidateQualificationPlan.yml@main";
replace the "`@main`" with the exact 40-character commit SHA of the target
repository to pin the reusable workflow for immutability and reproducibility so
the `uses:` entry references the immutable commit SHA instead of a branch name.
In `@Evaluation/Input/Content/Concentration_time_profiles.md`:
- Line 1: The markdown link [Section 2.2.2](`#222-clinical-data`) is producing an
invalid fragment (MD051); replace the broken intra-doc anchor by either using
the correct rendered slug for the target heading or by converting the link to
plain text. Locate the link string "[Section 2.2.2](`#222-clinical-data`)" in
Concentration_time_profiles.md and update it to point to the actual heading
anchor (for example the slugified heading such as "`#22-clinical-data`" or
"`#222-clinical-data`" as generated by your markdown renderer) or simply change it
to "Section 2.2.2" without an anchor to eliminate the MD051 lint error.
In `@Evaluation/Input/Content/GOF_diagnostics.md`:
- Line 1: The markdown link fragment `#222-clinical-data` in the sentence
"presented in [Section 2.2.2](`#222-clinical-data`)" is invalid and triggers
MD051; replace that fragment with the correct section anchor (for example
`#2.2.2-clinical-data`) or remove the fragment link entirely so the text reads
"presented in Section 2.2.2" — verify the actual header slug for the target
section and update the link fragment accordingly to match the real heading ID.
In `@Evaluation/Input/Content/Section1_Introduction.md`:
- Line 5: The markdown uses a local fragment link "`#5-references`" that isn't
defined here; update the link in the sentence containing "`#5-references`" to a
file-qualified reference (e.g., References.md#5-references) so the anchor
resolves in standalone viewers, and verify the target anchor name in the
referenced file matches exactly; replace every occurrence of the local fragment
link in this file (the link text with "`#5-references`") with the correct
file-qualified form.
In `@Evaluation/Input/Content/titlepage.md`:
- Around line 6-10: Update the placeholder version metadata in the titlepage
table: replace "x.x-OSPy.y" under "Version", "vx.x" in the "based on *Model
Snapshot* and *Evaluation Plan*" URL, "y.y" under "OSP Version", and "z.z" under
"Qualification Framework Version" with the actual concrete version strings
(e.g., semantic version identifiers or release tags) used for this evaluation so
the document is reproducible and auditable; ensure the URL uses the exact
release tag and that all four fields in the table row are consistently updated.
In `@Evaluation/workflow.R`:
- Around line 10-14: The current fallback to getwd() makes workingDirectory and
thus qualificationPlanFile/snapshotFile nondeterministic; change the resolution
so repoRoot is determined deterministically (e.g. use the repository root via
git or rprojroot) and build workingDirectory and the two file paths relative to
that root. Specifically, replace the getwd() fallback for
thisFile/workingDirectory with a deterministic repo root discovery (for example
using system2("git", "rev-parse --show-toplevel") or
rprojroot::find_root_file(".git")) and then set repoRoot, qualificationPlanFile,
and snapshotFile relative to that discovered root so variables thisFile,
workingDirectory, repoRoot, qualificationPlanFile, and snapshotFile always
resolve the same way.
In `@README.md`:
- Line 29: Update the README to reference the correct evaluation plan filename
used in the repo: replace "evaluation-plan.json" with "evaluation_plan.json"
wherever referenced (specifically the sentence describing the evaluation plan
artifact) so the README matches the actual artifact name evaluation_plan.json.
- Around line 31-33: Replace the ambiguous "found here" link text in the two
README.md sentences with descriptive labels: change the first link text to
something like "latest release in this repository" for the
snapshot/model/evaluation link and change the second link text to "latest OSP
PBPK Model Library release" for the GitHub OSP-PBPK-Model-Library link; update
the Markdown links so the URLs remain the same but the link text is the new
descriptive labels to satisfy MD059 and improve accessibility.
---
Outside diff comments:
In `@Evaluation/Input/Content/Section2_Methods.md`:
- Around line 1-2: The Methods section file Section2_Methods.md is empty; add a
minimal Methods heading and content (e.g., "## Methods" plus at least one
paragraph describing methodology) or remove Section2_Methods.md from the report
composition/import list so the build doesn't include an empty section; update
whichever module references Section2_Methods.md (the report assembler or
manifest that imports this file) to reflect the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 988b4f84-3ae1-4335-9c59-64487de04248
📒 Files selected for processing (27)
.github/workflows/Check_utf8.yml.github/workflows/CrossRefCheckWithAnchors.yml.github/workflows/MarkdownLinksCheck.yml.github/workflows/SpellChecker.yml.github/workflows/ValidateEvaluationPlan.yml.github/workflows/wordlist.txt.gitignoreEvaluation/Input/Content/Concentration_time_profiles.mdEvaluation/Input/Content/Concentration_time_profiles_building.mdEvaluation/Input/Content/Concentration_time_profiles_verification.mdEvaluation/Input/Content/GOF_diagnostics.mdEvaluation/Input/Content/GOF_diagnostics_9hydroxyrisperidone.mdEvaluation/Input/Content/GOF_diagnostics_risperidone.mdEvaluation/Input/Content/Input_table.mdEvaluation/Input/Content/References.mdEvaluation/Input/Content/Section1_Introduction.mdEvaluation/Input/Content/Section2.1_Modeling_Strategy.mdEvaluation/Input/Content/Section2.2_Data.mdEvaluation/Input/Content/Section2.3_Model_Parameters_and_Assumptions.mdEvaluation/Input/Content/Section2_Methods.mdEvaluation/Input/Content/Section3_Results_and_Discussion.mdEvaluation/Input/Content/Section4_Conclusion.mdEvaluation/Input/Content/titlepage.mdEvaluation/Input/evaluation_plan.jsonEvaluation/workflow.RREADME.mdRisperidone-model.json
Comment on lines
+6
to
+10
| | Version | x.x-OSPy.y | | ||
| | ----------------------------------------------- | ------------------------------------------------------------ | | ||
| | based on *Model Snapshot* and *Evaluation Plan* | https://github.com/Open-Systems-Pharmacology/Risperidone-Model/releases/tag/vx.x | | ||
| | OSP Version | y.y | | ||
| | Qualification Framework Version | z.z | |
There was a problem hiding this comment.
Replace placeholder version metadata before merge.
Line 6 to Line 10 still use placeholder values (x.x-OSPy.y, vx.x, y.y, z.z). For a QC-ready report, these should be concrete versions so the report is reproducible and auditable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Evaluation/Input/Content/titlepage.md` around lines 6 - 10, Update the
placeholder version metadata in the titlepage table: replace "x.x-OSPy.y" under
"Version", "vx.x" in the "based on *Model Snapshot* and *Evaluation Plan*" URL,
"y.y" under "OSP Version", and "z.z" under "Qualification Framework Version"
with the actual concrete version strings (e.g., semantic version identifiers or
release tags) used for this evaluation so the document is reproducible and
auditable; ensure the URL uses the exact release tag and that all four fields in
the table row are consistently updated.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/CrossRefCheckWithAnchors.yml (1)
4-6:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRun this check on pull requests too, not only
reviewpushes.Lines 4-6 limit execution to pushes on
review, so this QA check won’t protect typical PR merges tomain. Add apull_requesttrigger (and optionallypushtomain) so failures are caught before merge.Suggested change
-on: - push: - branches: - - review +on: + push: + branches: + - review + - main + pull_request:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/CrossRefCheckWithAnchors.yml around lines 4 - 6, The workflow currently triggers only on push to the review branch (the "push:" -> "branches:" -> "- review" block); update the triggers so the job also runs for pull requests and for pushes to main by adding a "pull_request:" section (e.g., matching the same branches or target branches) and expanding the "push:" branches list to include "main" (or add a separate push rule for main), ensuring the workflow runs on both pull_request events and pushes to main/review..github/workflows/MarkdownLinksCheck.yml (1)
4-6:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis link check should gate PRs, not only
reviewbranch pushes.Line 4-6 currently prevents this workflow from running on normal PR validation paths. Add
pull_request(and optionallypushtomain) so broken links are blocked before merge.Suggested change
-on: - push: - branches: - - review +on: + push: + branches: + - review + - main + pull_request:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/MarkdownLinksCheck.yml around lines 4 - 6, The workflow currently only triggers on push to the review branch (the push: branches: - review block), so it won't run for PR checks; update the workflow triggers to include pull_request (and optionally add push: branches: - main) so the MarkdownLinksCheck.yml runs on PR validation and on pushes to main; locate the triggers section (the push/branches block and the workflow top-level triggers) and add pull_request as a trigger and/or add main to push branches.Evaluation/workflow.R (1)
61-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
maxSimulationsPerCorebefore assigning it.Line 61 accepts any non-null value; invalid values (non-numeric,
NA, or< 1) can break execution later with unclear errors.Suggested change
- if (!is.null(maxSimulationsPerCore)) workflow$simulate$settings$maxSimulationsPerCore <- maxSimulationsPerCore + if (!is.null(maxSimulationsPerCore)) { + if (!is.numeric(maxSimulationsPerCore) || + length(maxSimulationsPerCore) != 1 || + is.na(maxSimulationsPerCore) || + maxSimulationsPerCore < 1) { + stop("maxSimulationsPerCore must be a single numeric value >= 1.", call. = FALSE) + } + workflow$simulate$settings$maxSimulationsPerCore <- as.integer(maxSimulationsPerCore) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Evaluation/workflow.R` at line 61, Validate maxSimulationsPerCore before assigning to workflow$simulate$settings$maxSimulationsPerCore: ensure maxSimulationsPerCore is a non-NA numeric scalar (or integer), has length 1, and is >= 1; if it fails validation, throw a clear error (stop) or coerce safely (e.g. to integer after checking) and only then assign to workflow$simulate$settings$maxSimulationsPerCore to avoid downstream failures.
🧹 Nitpick comments (1)
Evaluation/Input/Content/Section2.2_Data.md (1)
18-18: ⚡ Quick winClarify what "mismatched" means.
The correction note states the Km value "is mismatched" but doesn't specify what it's mismatched between (e.g., publication text vs. supplement, main paper vs. erratum, etc.). For QC documentation, precision is important.
✍️ Example clearer wording
-The 9-hydroxyrisperidone P-gp K<sub>m</sub> reported in the publication and supplement by [Rüdesheim 2022](References.md) is mismatched. The model retains the correct value of 149.6 µmol/L. +The 9-hydroxyrisperidone P-gp K<sub>m</sub> value differs between the main text and supplement in [Rüdesheim 2022](References.md). The model uses the correct value of 149.6 µmol/L from the supplement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Evaluation/Input/Content/Section2.2_Data.md` at line 18, Change the vague phrase "is mismatched" in the sentence about the 9-hydroxyrisperidone P-gp K<sub>m</sub> so it explicitly states what sources disagree (e.g., "publication text vs. supplementary material" or "main paper vs. supplement") and which value each source reports, while retaining the model's value of 149.6 µmol/L; update the sentence referencing "9-hydroxyrisperidone P-gp K<sub>m</sub>" to read something like "The 9-hydroxyrisperidone P-gp K<sub>m</sub> reported in the main text (X µmol/L) differs from the supplement (Y µmol/L); the model retains 149.6 µmol/L" so reviewers can see exactly what is mismatched.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Evaluation/Input/Content/Section2.2_Data.md`:
- Around line 5-17: Add a missing table row for the "9-hydroxyrisperidone P-gp
K_m" parameter (value 149.6 µmol/L, source as in the correction note) into the
existing parameter table so the correction note matches the table; ensure the
row follows the same columns and formatting as other entries (Parameter | Unit |
Value | Source | Description) and include a brief description like "P-gp
affinity parameter for the metabolite" to mirror entries such as "Risperidone
P-gp K_m" and "9-Hydroxyrisperidone f_u".
---
Outside diff comments:
In @.github/workflows/CrossRefCheckWithAnchors.yml:
- Around line 4-6: The workflow currently triggers only on push to the review
branch (the "push:" -> "branches:" -> "- review" block); update the triggers so
the job also runs for pull requests and for pushes to main by adding a
"pull_request:" section (e.g., matching the same branches or target branches)
and expanding the "push:" branches list to include "main" (or add a separate
push rule for main), ensuring the workflow runs on both pull_request events and
pushes to main/review.
In @.github/workflows/MarkdownLinksCheck.yml:
- Around line 4-6: The workflow currently only triggers on push to the review
branch (the push: branches: - review block), so it won't run for PR checks;
update the workflow triggers to include pull_request (and optionally add push:
branches: - main) so the MarkdownLinksCheck.yml runs on PR validation and on
pushes to main; locate the triggers section (the push/branches block and the
workflow top-level triggers) and add pull_request as a trigger and/or add main
to push branches.
In `@Evaluation/workflow.R`:
- Line 61: Validate maxSimulationsPerCore before assigning to
workflow$simulate$settings$maxSimulationsPerCore: ensure maxSimulationsPerCore
is a non-NA numeric scalar (or integer), has length 1, and is >= 1; if it fails
validation, throw a clear error (stop) or coerce safely (e.g. to integer after
checking) and only then assign to
workflow$simulate$settings$maxSimulationsPerCore to avoid downstream failures.
---
Nitpick comments:
In `@Evaluation/Input/Content/Section2.2_Data.md`:
- Line 18: Change the vague phrase "is mismatched" in the sentence about the
9-hydroxyrisperidone P-gp K<sub>m</sub> so it explicitly states what sources
disagree (e.g., "publication text vs. supplementary material" or "main paper vs.
supplement") and which value each source reports, while retaining the model's
value of 149.6 µmol/L; update the sentence referencing "9-hydroxyrisperidone
P-gp K<sub>m</sub>" to read something like "The 9-hydroxyrisperidone P-gp
K<sub>m</sub> reported in the main text (X µmol/L) differs from the supplement
(Y µmol/L); the model retains 149.6 µmol/L" so reviewers can see exactly what is
mismatched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7179c306-2271-4f93-a140-073a5423c1da
📒 Files selected for processing (15)
.github/workflows/Check_utf8.yml.github/workflows/CrossRefCheckWithAnchors.yml.github/workflows/MarkdownLinksCheck.yml.github/workflows/SpellChecker.yml.github/workflows/ValidateEvaluationPlan.yml.github/workflows/wordlist.txtEvaluation/Input/Content/Concentration_time_profiles.mdEvaluation/Input/Content/GOF_diagnostics.mdEvaluation/Input/Content/Section1_Introduction.mdEvaluation/Input/Content/Section2.1_Modeling_Strategy.mdEvaluation/Input/Content/Section2.2_Data.mdEvaluation/Input/Content/Section3_Results_and_Discussion.mdEvaluation/Input/evaluation_plan.jsonEvaluation/workflow.RREADME.md
💤 Files with no reviewable changes (1)
- Evaluation/Input/evaluation_plan.json
✅ Files skipped from review due to trivial changes (5)
- Evaluation/Input/Content/GOF_diagnostics.md
- .github/workflows/wordlist.txt
- Evaluation/Input/Content/Concentration_time_profiles.md
- README.md
- Evaluation/Input/Content/Section3_Results_and_Discussion.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR updates the risperidone evaluation repository to a QC-ready state with revised report content, corrected model and evaluation mappings, and cleaned report structure.
Changes
Added structured evaluation report input content, including model strategy, data, parameter assumptions, results and discussion, conclusion, references, GOF diagnostics, and concentration-time profile sections.
Updated the evaluation plan to align simulation names, observed data mappings, outputs, colors, and report sections.
Patched the risperidone model snapshot for QC findings:
kcatvaluesK_mand documented the source mismatchRemoved redundant simulation outputs that were not used for report evaluation.
Split GOF diagnostics by compound so risperidone and 9-hydroxyrisperidone are reported separately.
Updated README content with a model/report summary, clinical data references, and local references.
Validation
Summary by CodeRabbit
New Features
Documentation
Chores