fix(cpm): cap Late Finish at project duration so SS/FF/SF long-poles stay critical - #381
fix(cpm): cap Late Finish at project duration so SS/FF/SF long-poles stay critical#381seonghobae wants to merge 3 commits into
Conversation
…stay critical
The CPM backward pass seeded the successor `reduce` with `Infinity`, so a node
with successors took its Late Finish purely from successor constraints and was
never bounded by the project duration. Standard CPM (PMBOK float definitions;
Kelley/Walker) initializes every activity's LF to the project finish, then
tightens it with successors: `LF = min(projectDuration, min over successors)`.
Under pure FS networks a successor bound is always <= project duration, so the
missing cap was invisible (every prior test used FS chains). But an SS/FF/SF
successor can impose a looser bound than the project end, letting a predecessor's
LF exceed the project duration and giving a genuinely-critical activity false
total float — and an empty critical path.
Reproduction: `computeCpm([{id:'A',duration:10},{id:'B',duration:2,predecessors:'ASS'}])`
reported A (the sole 10-day long pole) as lf=18 (> project 10), slack=8,
critical=false, criticalPath=[]. A planner would wrongly see 8 days of buffer on
the one activity that drives the finish. After the fix: A lf=10, slack=0,
critical=true, criticalPath=["A"]; B keeps its real 8d float.
Fix: seed the reduce with `projectDurationDays` instead of `Infinity`. Adds a
regression test to tests/unit/dep-types.test.mjs. Verified: full pure-math suite
(13 suites) + property fuzz (14) pass; existing FS and SS-chain assertions
unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HdCssGnNMhKHNu3TXFstWH
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
Comment |
CI status: both red checks are pre-existing/base, not this diffThis PR changes only The two failing required checks are not caused by this diff:
No action fixable from this branch without duplicating #379 or churning on an infra hiccup. Flagging once here rather than re-reacting to each re-run; happy to revisit if either check produces a new, non-empty finding tied to this diff. Generated by Claude Code |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head8b5fa34454a616b0f720681cd4f333b3360f0cb8. -
Head SHA:
8b5fa34454a616b0f720681cd4f333b3360f0cb8 -
Workflow run: 30577484110
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file: analytics.js"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file: analytics.js"]
R1 --> V1["required checks"]
Evidence --> S2["Test: dep-types.test.mjs"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: dep-types.test.mjs"]
R2 --> V2["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test: dep-types.test.mjs"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: dep-types.test.mjs"]
R2 --> V2["targeted test run"]
|
…67-39w9) @hono/node-server <2.0.5 has a moderate path-traversal in `serveStatic` on Windows via an encoded backslash (%5C). The org Security Scan (trivy-fs/OSV) flags it repo-wide. Patched in 2.0.5+; bump to ^2.0.12. Scope of the "breaking" 1.x -> 2.x major is minimal here: server.mjs imports only `serve` (not the vulnerable `serveStatic`), and `serve({ fetch, port }, info => ...)` is unchanged across the major. hono stays ^4.12.27 (deduped; 2.x supports hono 4). No runtime dependency added — the two-dep contract holds. Verified: npm audit → 0 vulnerabilities; test:unit (13/13 files), test:api (smoke + rate-limit) all pass — the real server boots and serves correctly on 2.x. (test:e2e:cloud is browser-revision-blocked locally: the sandbox ships Chromium build 1194 while @playwright/test 1.61.1 wants 1228; CI runs that gate with the matching browser.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HdCssGnNMhKHNu3TXFstWH
The repo carries two lockfiles (CI caches npm/package-lock.json, but a pnpm-lock.yaml also exists). The previous commit updated only package-lock.json, so trivy-fs — which scans every lockfile in the tree, not just the CI one — still flagged @hono/node-server 1.19.14 (GHSA-frvp-7c67-39w9) via pnpm-lock.yaml. Bring pnpm-lock.yaml in sync so both lockfiles pin the patched 2.0.12. Minimal diff: only the @hono/node-server entries change; hono stays 4.12.28, lockfileVersion unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HdCssGnNMhKHNu3TXFstWH
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head5ff195bc54de8418ce56a26f8279f4e6253509b2. -
Head SHA:
5ff195bc54de8418ce56a26f8279f4e6253509b2 -
Workflow run: 30593549226
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Test: dep-types.test.mjs"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: dep-types.test.mjs"]
R2 --> V2["targeted test run"]
2 participants
Problem
The CPM backward pass in
analytics.js(computeCpm) seeded the successorreducewithInfinity:Standard CPM (PMBOK float definitions; Kelley/Walker) initializes every activity's Late Finish to the project finish date, then tightens it with successor constraints:
Seeding with
Infinitydrops theprojectDurationcap, so a node that has successors takes its LF purely from those successors. In a pure FS network the successor bound is always ≤ project duration, so the missing cap is invisible — and every existing unit test uses FS chains (or FS-terminated chains). But an SS/FF/SF successor can impose a looser bound than the project end, letting a predecessor's LF exceed the project duration and giving a genuinely-critical activity false total float (and returning an empty critical path).Reproduction
A (the sole 10-day long pole) drives the schedule end. Before the fix:
A planner would see 8 days of buffer on the one activity that actually determines the finish, and no critical path at all. After the fix:
SS relationships (overlapping/parallel work) are among the most common non-FS links in schedule-control, so this is not an exotic edge case.
Fix
Seed the successor
reducewithprojectDurationDaysinstead ofInfinity— the canonical robust backward-pass initialization (no LF can exceed project completion without extending the project). One-line change plus an explanatory comment.Tests
Adds a regression test to
tests/unit/dep-types.test.mjs(SS long-pole → A critical with zero float,criticalPath === ['A'], B keeps 8d float). The gap that let this ship: no prior test exercised a non-FS link whose successor constraint is looser than the project duration.Verification
All green; existing FS and SS-chain assertions are unchanged (the fix is a no-op for FS networks and for the existing
A(2)→SS B(6)→C(1)case).🤖 Generated with Claude Code
https://claude.ai/code/session_01HdCssGnNMhKHNu3TXFstWH
Generated by Claude Code