Split PMC mutation surface and stabilize Compose ports - #1176
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 4 minutes and 21 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, 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 trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe PR automats local development by replacing fixed Docker Compose port bindings with dynamic allocation via ChangesLocal Infrastructure & Development Workflow
PMC Chart Calculation Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR spans infrastructure automation and a substantial computation-layer refactoring. The infrastructure changes (dynamic port binding, env script, npm integration) are straightforward. The PMC refactoring is dense: three new calculator classes with multiple interdependencies, removal of ~100 lines of prior repository logic, and test suite restructuring. Review requires understanding the prior single-class computation flow, the new multi-class delegation pattern, heart-rate/power-TSS priority logic, regression modeling, EWMA window calculations, and test assertion updates. Multiple files interact (calculator orchestration, repository delegation, test fixtures); changes are heterogeneous across implementation, types, and test patterns. Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Storybook previews for This comment updates automatically on each PR push. |
|
Review app is ready: This environment runs on a dedicated Hetzner server for PR #1176 and updates on each push. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/compose-env.ts`:
- Around line 117-118: The DATABASE_URL and CLICKHOUSE_URL are built using
dotenvEscape(postgresPassword) and dotenvEscape(clickHousePassword), but
dotenvEscape is not URL-safe so passwords containing @ : / ? # will break URL
parsing; replace the dotenvEscape usage for the password components with proper
URL-encoding (e.g. use encodeURIComponent or an equivalent URL-encoding helper)
when constructing DATABASE_URL and CLICKHOUSE_URL so the credentials are
percent-encoded and the resulting URLs remain valid.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 220eeef5-1b9c-45b1-ba0a-8672deeb7320
📒 Files selected for processing (14)
.env.exampleAGENTS.mdREADME.mddocker-compose.ymlpackage.jsonpackages/server/src/repositories/pmc-chart-calculator.test.tspackages/server/src/repositories/pmc-chart-calculator.tspackages/server/src/repositories/pmc-ewma-calculator.test.tspackages/server/src/repositories/pmc-ewma-calculator.tspackages/server/src/repositories/pmc-repository.test.tspackages/server/src/repositories/pmc-repository.tspackages/server/src/repositories/pmc-training-load-calculator.test.tspackages/server/src/repositories/pmc-training-load-calculator.tsscripts/compose-env.ts
Automated Checks (advisory, non-blocking)✅ All checks passed. Surmado Code Review — Free tier limit reachedYou've used all 10 free reviews this month. Deterministic checks (secrets, model strings) still ran above. Upgrade to the Paid plan for 100 reviews/month + $15 per additional 100: https://app.surmado.com/checkout?plan=pr_review_starter Or wait until your next monthly window for 10 more free reviews. Surmado Code Review (v1.2-mt) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 15 files
Confidence score: 3/5
- There is a concrete user-impacting risk in
scripts/compose-env.ts:dotenvEscapedoes not escape$, so values can be shell-expanded when.env.localis sourced, potentially corrupting secrets like passwords. - Given the issue’s medium-high severity (6/10) and solid confidence (7/10), this looks like a real regression risk rather than a cosmetic concern.
- Pay close attention to
scripts/compose-env.tsandwith-env.sh- ensure env values containing$are preserved literally when writing and sourcing.env.local.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Fix all with cubic | Re-trigger cubic
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: This PR refactors core PMC chart computation logic into separate classes, which is a structural change with potential for subtle bugs despite added tests; such refactors require human review to ensure correctness and preserve existing behavior.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: This PR performs a significant refactor of the PMC chart computation, splitting monolithic repository logic into multiple new calculator classes with corresponding tests, while also adding a new Docker Compose port resolution script and modifying infrastructure configuration; given the 2600+...
Re-trigger cubic
bc6ef7d to
2fe903b
Compare
2fe903b to
2a3cd32
Compare
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: This PR includes a significant code refactor that splits PMC chart computation into new classes and modifies docker-compose port bindings, both of which carry moderate risk of subtle issues even with the new tests, so human review is warranted.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: The PR refactors core performance calculation logic (PMC chart, training load, EWMA) into new classes, which, despite preserved test coverage and no detected issues, introduces risk of subtle bugs in a critical business path that computes training stress, threshold power, and chart data—such...
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 6 files (changes from recent commits).
Requires human review: This PR refactors core PMC training-load computation into new calculator classes and adds an infrastructure script for Docker compose port management, both of which are high-impact changes that require human review to ensure correctness and safety.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: This PR involves substantial refactoring of core business logic (PMC chart calculation) and changes to Docker infrastructure and scripts, which carry moderate to high risk and require human review despite the AI finding no issues.
Re-trigger cubic
Split PMC chart computation out of PmcRepository into focused calculator classes with 1:1 colocated tests, reducing the slow Stryker shard surface.
Add a stable auto-port Compose flow (
pnpm compose:up) that writes.env.localand starts DB/ClickHouse/Redis through that env file so local services avoid collisions without changing ports after restarts.Record the source/test 1:1 guideline in
AGENTS.mdand update README/.env.example for local setup.Validation:
pnpm lint, root/server/webtsc --noEmit, focused PMC vitest, earlier targeted PMC Stryker 100% in 7m45s; fulltest:changedand a later Stryker dry run were blocked locally by unrelated ClickHouse/MSW integration instability.Summary by CodeRabbit
New Features
pnpm compose:up) replacing manual Docker commands.Documentation
Tests