Enforce security quantity balance invariants#253
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 13 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: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughChangesSecurity Balance Validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant UpdateCapTable
participant EventIndexes
participant SecurityBalanceValidators
participant AuthorizedShareCeilings
UpdateCapTable->>EventIndexes: Build stock and non-stock event indexes
UpdateCapTable->>SecurityBalanceValidators: Validate successors and balances
SecurityBalanceValidators-->>UpdateCapTable: Return validated transitions
UpdateCapTable->>AuthorizedShareCeilings: Validate issued-share ceilings
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@copilot review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de4daaf240
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…g-v2' into codex/ocf-stock-security-balance
HardlyDifficult
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@copilot review |
HardlyDifficult
changed the base branch from
codex/ocf-stock-plan-pool-ceiling-v2
to
codex/ocf-schema-constraints-category
Reviewed exact head |
|
@codex review exact head @coderabbitai resume Copilot completed this exact-head review with no findings; CI is green and all review threads are clear. |
|
@cursor review Please review exact head |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6a70c2f. Configure here.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Test/daml/OpenCapTable/TestWarrantTransfer.daml (1)
16-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
createWarrantIssuancehelper across test files.This function is defined identically in
TestWarrantCancellation.daml(lines 15-38) and here. Both copies were edited in lockstep in this PR to add theissuanceDateparameter — a sign this duplication will keep costing maintenance effort as the underlying OCF warrant issuance schema evolves. Consider moving it toTestHelpers.damlalongside the newequityCompensationSuccessorIssuance/successor helpers.🤖 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 `@Test/daml/OpenCapTable/TestWarrantTransfer.daml` around lines 16 - 39, Move the duplicated createWarrantIssuance helper from TestWarrantTransfer.daml and TestWarrantCancellation.daml into TestHelpers.daml alongside the existing successor helpers, preserving its issuanceDate parameter and implementation. Update both test files to reuse the shared helper and remove their local definitions.
🤖 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/codegen/templates/CapTable.daml.template`:
- Around line 605-620: Validate reduction quantities as non-negative before
passing them to stockSecurityReductionEvents or equivalent reduction-event
constructors. Apply this consistently across the transfer, cancellation,
repurchase, conversion, release, and equity-exercise paths, or enforce the same
rule centrally in both balance-event validators. Ensure negative lifecycle
quantities are rejected rather than negated into positive balance increases.
---
Outside diff comments:
In `@Test/daml/OpenCapTable/TestWarrantTransfer.daml`:
- Around line 16-39: Move the duplicated createWarrantIssuance helper from
TestWarrantTransfer.daml and TestWarrantCancellation.daml into TestHelpers.daml
alongside the existing successor helpers, preserving its issuanceDate parameter
and implementation. Update both test files to reuse the shared helper and remove
their local definitions.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 482eed67-554d-4b52-a754-97631376d2b6
⛔ Files ignored due to path filters (1)
dars/dars.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Test/daml/OpenCapTable/TestEquityCompensationCancellation.damlTest/daml/OpenCapTable/TestEquityCompensationTransfer.damlTest/daml/OpenCapTable/TestFinalReferenceValidation.damlTest/daml/OpenCapTable/TestHelpers.damlTest/daml/OpenCapTable/TestNonStockSecurityBalances.damlTest/daml/OpenCapTable/TestStockClassAuthorizedShares.damlTest/daml/OpenCapTable/TestStockConversion.damlTest/daml/OpenCapTable/TestStockSecurityBalances.damlTest/daml/OpenCapTable/TestStockTransfer.damlTest/daml/OpenCapTable/TestWarrantCancellation.damlTest/daml/OpenCapTable/TestWarrantTransfer.damldars/OpenCapTable-v34/0.0.12/OpenCapTable-v34.darscripts/codegen/templates/CapTable.daml.template
|
✅ Action performedReviews resumed. Review finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a70c2f0dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed CodeRabbit’s outside-diff maintainability suggestion in |
|
@copilot review Please review exact head |
Reviewed exact head |
|
@codex review exact head @coderabbitai resume Copilot completed this exact-head review with no findings; CI is green and all review threads are clear. |
|
@cursor review Please review exact head |
|
✅ Action performedReviews resumed. Review finished.
|
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a068d2b. Configure here.
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review exact head Retrying because the prior acknowledged request completed without a submitted review. Please review the central negative-reduction guards, scaled fixed-quantity VestingTerms, and inline-vesting precedence. Copilot, CodeRabbit, and Cursor have all completed this exact revision with no findings; CI is green and review threads are clear. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…egory' into codex/ocf-stock-security-balance
|
Restacked onto the final reviewed head of #269 ( Exact head: Fresh validation on the combined branch:
Keeping this PR in draft until exact-head CI is green. |
|
CI is green and all review threads are resolved on exact head |
Reviewed exact head |
|
@codex review exact head |
|
@coderabbitai review exact head |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c271839. Configure here.
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Preliminary OCF validation-policy auditPolicy: OCF Validation Policy Appears supported
Needs line-by-line evidence or narrowing
Proposed next step |
Summary
Stacked on #269 as the security quantity-balance category.
This enforces non-negative quantity balances for OCF securities that carry share/unit quantities.
UpdateCapTablerejects final cap-table states where a source security is reduced, replaced, or retired for more units than were issued.Review feedback carried forward in this category:
balance_security_idretire the source security where OCF moves the remainder to the balance security.portionconditions may reuse the source terms unchanged.vesting_terms_idare present, matching OCF's documented override behavior.OCF documentation
security_id; StockTransfer.quantity, StockCancellation.quantity, StockRepurchase.quantity, and StockConversion.quantity_converted consume units from that security.vesting_terms_idmay be ignored when inlinevestingsare present, so successor continuity follows the scaled inline schedule in that shape.Validation
npm run buildnpm test(complete suite; 596 test scripts: 594 behavioral + 2 setup)npm run lint(0 errors; 2 pre-existing warnings inscripts/remove-participant-dar.ts)npm run lint:damlnpm run formatnpm run check:schema-gaps -- --ocf-schema-dir /tmp/open-cap-format-ocf-schema-check/schemanpm run verify-darsnpm run verify-packagenpm run check-upgrade-compat(0.0.11 -> 0.0.12)git diff --checkOpenCapTable-v34-0.0.12.dar: SHA-2568bb46a77ab9697b4446a2cfbabda29ba8b40b24bec073c428155e0d62271d5fb, 3,169,059 bytesNote
High Risk
Changes core cap-table validation for quantities, transfers, and authorized-share accounting; regressions would block legitimate updates or allow invalid cap tables. Risk is mitigated by the large new script suite and package version bump with upgrade-compat checks.
Overview
This PR ships OpenCapTable-v34 0.0.12, which rejects
UpdateCapTablestates where stock, warrant, or equity-compensation securities are over-reduced, over-exercised, or retired inconsistently (including balance successors, terminal events, splits, and authorized-share ceiling behavior described in the PR).The visible diff is mostly Daml Script coverage and test refactors: new
TestStockSecurityBalancesandTestNonStockSecurityBalancesmodules, shared helpers (createWarrantIssuance, date-parameterized cancellations/splits,equityCompensationRelease), and existing transfer/cancellation/conversion tests updated so partial flows materialize successor issuances on the event date before the lifecycle transaction. Test now depends on the0.0.12DAR;dars.lockrecords the published artifact.Reviewed by Cursor Bugbot for commit c271839. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests