Skip to content

Model exact conversion trigger variants#421

Closed
HardlyDifficult wants to merge 46 commits into
codex/canonical-conversion-mechanismsfrom
codex/exact-conversion-triggers
Closed

Model exact conversion trigger variants#421
HardlyDifficult wants to merge 46 commits into
codex/canonical-conversion-mechanismsfrom
codex/exact-conversion-triggers

Conversation

@HardlyDifficult

@HardlyDifficult HardlyDifficult commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • model convertible and warrant conversion triggers as exact discriminator-driven variants instead of partial records
  • preserve schema-faithful raw OCF parsing while typed parsing rejects duplicate trigger IDs and reversed elective date ranges
  • register the intentional conversion-trigger semantic refinement with live source, built, and package witnesses
  • remove duplicated forbidden-field checks and unreachable fallback behavior
  • add warrant writer and reader round-trip coverage for ConvertibleConversionRight plus exact nested invalid-path diagnostics
  • preserve the canonical-versus-persisted warrant valuation split and any-resistant type contracts inherited from Model canonical conversion mechanisms #420
  • reconcile the final exact-optional factory runtime regression with compile-time rejection coverage

Validation

  • npm run lint
  • npm run format
  • npm run build
  • npm run typecheck
  • npm run test:declarations
  • full suite: 85 suites / 5,203 tests
  • git diff --check

Stack

Depends on #420 at exact head c97ddc3. This PR remains draft until the parent and its own fresh CI, QuickStart, and exact-head reviewer gates complete.

# Conflicts:
#	src/functions/OpenCapTable/convertibleIssuance/createConvertibleIssuance.ts
#	src/functions/OpenCapTable/convertibleIssuance/getConvertibleIssuanceAsOcf.ts
#	src/functions/OpenCapTable/warrantIssuance/createWarrantIssuance.ts
#	src/functions/OpenCapTable/warrantIssuance/getWarrantIssuanceAsOcf.ts
#	src/types/native.ts
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0281a278-560f-408f-a188-0e2b66201f21

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

Reviews paused

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/exact-conversion-triggers

Comment @coderabbitai help to get the list of available commands.

@HardlyDifficult
HardlyDifficult marked this pull request as ready for review July 10, 2026 04:53
@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

CI is green on the latest head, including the full OCP QuickStart integration suite.

@copilot review
@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

@HardlyDifficult Thanks for the update — I'll review the latest changes now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

Pull request overview

Models OCF conversion triggers as six exact discriminated-union variants and centralizes validation so convertible/warrant issuance read/write boundaries reject malformed or cross-variant trigger payloads instead of silently normalizing them.

Changes:

  • Introduces ConversionTriggerFor<TRight> exact union types and applies them to ConvertibleConversionTrigger / WarrantExerciseTrigger.
  • Adds parseConversionTriggerFields and wires it into convertible + warrant issuance DAML↔OCF converters to enforce variant-specific fields.
  • Adds compile-time declaration contracts plus runtime round-trip / rejection tests and pinned-schema alignment coverage for all six trigger variants.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/types/conversionTriggers.types.ts Source-level TS compile contracts for the six exact trigger variants (including negative @ts-expect-error cases).
test/declarations/conversionTriggers.types.ts Built-declaration compile contracts to ensure emitted dist types preserve the exact trigger unions.
test/schemaAlignment/conversionTriggerShapes.test.ts Schema-alignment tests asserting pinned OCF trigger shapes are accepted/rejected correctly via parseOcfObject.
test/converters/conversionTriggerVariants.test.ts Converter round-trip and boundary-rejection tests covering all six variants across convertible + warrant issuance paths.
test/createOcf/falsyFieldRoundtrip.test.ts Extends falsy/field-preservation tests to include trigger_date on DAML trigger payloads.
src/types/native.ts Adds exact trigger union types (ConversionTriggerFor) and replaces prior loose trigger interfaces for convertibles/warrants.
src/utils/conversionTriggers.ts New centralized trigger validator/builder (parseConversionTriggerFields) enforcing discriminator-specific timing fields and null-handling.
src/functions/OpenCapTable/convertibleIssuance/createConvertibleIssuance.ts Validates and writes convertible triggers using parseConversionTriggerFields, emitting only the allowed timing fields per variant.
src/functions/OpenCapTable/convertibleIssuance/getConvertibleIssuanceAsOcf.ts Reads/validates convertible triggers via parseConversionTriggerFields (with nullIsAbsent for ledger nulls).
src/functions/OpenCapTable/warrantIssuance/createWarrantIssuance.ts Validates and writes warrant triggers using parseConversionTriggerFields, ensuring canonical timing field emission.
src/functions/OpenCapTable/warrantIssuance/getWarrantIssuanceAsOcf.ts Reads/validates warrant triggers via parseConversionTriggerFields (with nullIsAbsent for ledger nulls).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/utils/conversionTriggers.ts
Comment thread src/functions/OpenCapTable/convertibleIssuance/createConvertibleIssuance.ts Outdated
@HardlyDifficult
HardlyDifficult marked this pull request as draft July 10, 2026 16:19
@HardlyDifficult
HardlyDifficult marked this pull request as ready for review July 10, 2026 16:39
@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

Current head 5231202 is green and merge-clean. Empty required trigger strings are rejected, both issuance writers share one strict timing serializer, and all prior review threads are resolved.

@copilot-pull-request-reviewer please review the latest head.
@coderabbitai review

@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

cursor review

@cursor cursor Bot 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.

✅ 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 5231202. Configure here.

@HardlyDifficult
HardlyDifficult marked this pull request as draft July 10, 2026 16:42
…nisms' into codex/exact-conversion-triggers

# Conflicts:
#	src/functions/OpenCapTable/convertibleIssuance/createConvertibleIssuance.ts
…nisms' into codex/exact-conversion-triggers

# Conflicts:
#	src/functions/OpenCapTable/warrantIssuance/createWarrantIssuance.ts
@HardlyDifficult
HardlyDifficult marked this pull request as ready for review July 10, 2026 18:51
@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

cursor review

Copilot AI commented Jul 11, 2026

Copy link
Copy Markdown

@copilot review

Reviewed exact head eed32ce5205972d11b2a67bdcc52462700a75b1a and found no new actionable issues.

@HardlyDifficult

HardlyDifficult commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Copilot completed a fresh no-findings review of exact head eed32ce: #421 (comment). Exact CI and QuickStart are green, GitHub reports CLEAN/MERGEABLE, and all prior threads are resolved. Please perform a formal review of this exact head against a42473a, focusing on exact conversion-trigger variants and stock-class storage invariants.

@cursor cursor Bot 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.

✅ 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 eed32ce. Configure here.

@HardlyDifficult
HardlyDifficult marked this pull request as draft July 11, 2026 22:35
@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

Material parent propagation complete at exact head 4debb8a, incorporating #420 at 0f61496. Source typecheck passed on canton-node-sdk 0.0.227, preserving this PR's exact conversion-trigger architecture plus exhaustive relationship and symmetric vesting-graph validation. The PR remains draft and unassigned while CI runs.

@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

Fresh semantic audit hardening is pushed at exact head 5ec313a90f54832f15a212e7077c761f0c105f72.

Two runtime invariants that declarations/tests previously missed are now enforced symmetrically:

  • trigger_id must be unique within each convertible or warrant parent list; duplicates across the same or different trigger variants fail with precise indexed context { triggerId, firstIndex, duplicateIndex }.
  • ELECTIVE_IN_RANGE requires canonical dates with start_date <= end_date; equal dates remain valid and reversed ranges report the exact end_date path.

The shared validators run on writer and ledger-reader paths for both parent families. Regressions cover duplicate variants, reversed reads/writes, valid/equal round trips, source+built declaration gates, and post-build runtime behavior. Accessor/proxy inputs remain trap-safe and right/mechanism discriminant mismatches remain rejected.

Validation on this exact revision: focused 2 suites / 100 tests; full 83 suites / 4,454 tests; build; typecheck; declarations; exact public-config checks; lint; format; diff check; and six post-build duplicate/reversed rejection probes with structured SDK errors. The PR remains draft and unassigned; no parent merge or reviews were requested.

…act-conversion-triggers

# Conflicts:
#	test/client/OcpClient.test.ts
#	test/config/environment.test.ts
@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

Propagated corrected PR #420 head 0c4e50550d9313429d7d9f170d3ca8ebb4cf232f with a normal merge commit. The combined head is 3562f6986784b09c6df280d7cc419227193babe5.

Preserved every parent schema/conversion guardrail and this PR’s exact conversion-trigger invariants: non-empty unique trigger_id values across variants, exact variant fields, and inclusive start_date/end_date ordering. Because this PR promotes exactOptionalPropertyTypes into the primary compiler config, the propagated parent runtime-rejection tests were updated to use intentional unsafe casts without weakening their runtime assertions.

Validation is green locally: 84 suites / 5,149 tests, build, global exact-optional typecheck, declaration and exact public-config tests, lint, Prettier, and diff checks. This PR remains draft and unassigned while the corrected stack is propagated and fresh exact-head reviews are pending.

@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

Re-propagated corrected PR #420 head f3c5773ba56b110c6d6fe60e7fbf9335336addcd with a normal merge commit. The new exact head is 93cbf41cd8f2fad66c7d717ab4dba1dada6c4ff9.

Preserved global exact optional-property types, unique non-empty trigger IDs, inclusive date-range validation, all parent schema/conversion invariants, and corrected full-payload PlanSecurity/Monetary behavior.

Validation is green locally: 7 focused suites / 517 tests; full coverage run 84 suites / 5,163 tests; clean build; global exact-optional typecheck; declaration and exact public-config tests; lint; Prettier; and diff checks. This PR remains draft and unassigned pending downstream propagation and fresh exact-head reviews.

@HardlyDifficult
HardlyDifficult marked this pull request as ready for review July 12, 2026 07:27
@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

@copilot review

Please review exact current head 93cbf41cd8f2fad66c7d717ab4dba1dada6c4ff9. Pre-review gate: both required checks are successful, GitHub is CLEAN/MERGEABLE, the PR is unassigned, and zero review threads are unresolved. Focus on exact conversion-trigger shapes, optional/list semantics, trap-safe runtime validation, and declaration parity. Report every actionable current-head issue before the other reviewers.

Copilot AI commented Jul 12, 2026

Copy link
Copy Markdown

@copilot review

Please review exact current head 93cbf41cd8f2fad66c7d717ab4dba1dada6c4ff9. Pre-review gate: both required checks are successful, GitHub is CLEAN/MERGEABLE, the PR is unassigned, and...

Reviewed exact head 93cbf41cd8f2fad66c7d717ab4dba1dada6c4ff9 with focus on conversion-trigger shapes, optional/list semantics, trap-safe runtime validation, and declaration parity. I did not find new actionable current-head issues.

@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

@cursor review

Please review exact current head 93cbf41cd8f2fad66c7d717ab4dba1dada6c4ff9. Copilot completed its required exact-head pass with no actionable findings: #421 (comment). Both required checks are green, GitHub is CLEAN/MERGEABLE, and zero review threads are unresolved. Focus on exact conversion-trigger shapes, optional/list semantics, trap-safe validation, and declaration parity.

@cursor cursor Bot 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.

✅ 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 93cbf41. Configure here.

@HardlyDifficult
HardlyDifficult marked this pull request as draft July 12, 2026 08:41
@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

Returned to draft because upstream #417 received a material exact-head corrective revision after full CodeRabbit review. This stacked PR will remain held until the audited #417 head and each intervening parent are propagated normally; fresh CI and Copilot-first reviewer gates are required afterward.

@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

Exact draft head 3b85edbf30dd1b7cb6a11f16c5d2050ca793358a now contains the current #420 parent plus the trigger-semantics remediation and exact-optional stack reconciliation. Local lint, format, build, typecheck, declarations, all 85 suites / 5,203 tests, and diff-check pass. Keeping this draft and unassigned while fresh CI/QuickStart run and the parent advances first.

@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

Fresh GitHub validation is now complete for exact draft head 3b85edbf30dd1b7cb6a11f16c5d2050ca793358a: build-and-test SUCCESS, full QuickStart integration SUCCESS, MERGEABLE, and zero unresolved review threads. The PR remains draft and unassigned until #420 advances first.

@HardlyDifficult

Copy link
Copy Markdown
Collaborator Author

Closing for the contract-owned validation pivot. The runtime trigger validator, duplicate-ID checks, elective-range ordering checks, and related semantic preflight belong in DAML. The small discriminated TypeScript trigger union and thin deterministic codec mapping remain useful and will be absorbed into the future simplified #420 scope rather than preserved as a separate PR. Branch retained for reference.

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.

3 participants