Skip to content

fix(PM_cycle): cycle object for cycles = 0 runs - #399

Open
mhovd wants to merge 1 commit into
mainfrom
claude/pmetrics-pmcore-pharmsol-fixes-qdno6d-cycle-posterior
Open

mhovd wants to merge 1 commit into
mainfrom
claude/pmetrics-pmcore-pharmsol-fixes-qdno6d-cycle-posterior

Conversation

@mhovd

@mhovd mhovd commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Bug. After a cycles = 0 run, creating the cycle object fails with `cycle` must be size 1, not 2, and $cycle$status is empty. The code used rep(1:n_cyc, ...), and with n_cyc = 0 that is rep(1:0, ...), which gives two values.

Fix. Build the gamlam cycle index from the cycle column itself.

Test. A real cycles = 0 fit. The test fails on main.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GTyUgJEd5uCEmv9qYrv8dC


Generated by Claude Code

A cycles = 0 run writes a single row with cycle 0, and rep(1:0, ...) gave
two values, so creating the cycle object failed and $cycle$status was
empty.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GTyUgJEd5uCEmv9qYrv8dC
Copilot AI balanced review requested due to automatic review settings October 11, 2026 10:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation fixes the reported failure with low risk; the noted test assertion weakness is non-blocking.

1 open finding
What changed in this PR

Fixes cycle-object creation for cycles = 0 runs by preserving cycle identifiers from generated output.

Changes:

  • Derives gamma/lambda cycle indices from raw$cycle.
  • Adds a regression test for zero-cycle prior runs.
File Description
R/​PM_cycle.R Corrects zero-cycle gamma/lambda indexing.
tests/​testthat/​test-pm-cycle-posterior.R Tests zero-cycle object creation.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


expect_length(run2$cycle$status, 1)
expect_equal(run2$cycle$data$objective$cycle, 0)
expect_true(all(run2$cycle$data$gamlam$cycle == 0))
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.84%. Comparing base (ef474a1) to head (de56917).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #399      +/-   ##
==========================================
+ Coverage   64.77%   64.84%   +0.06%     
==========================================
  Files          52       52              
  Lines       22964    22965       +1     
==========================================
+ Hits        14876    14891      +15     
+ Misses       8088     8074      -14     
Files with missing lines Coverage Δ
R/PM_cycle.R 78.90% <100.00%> (+0.47%) ⬆️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

4 participants