Validate complex issuance reader payloads#435
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…oder-backed-issuance-readers
|
@coderabbitai pause |
✅ Action performedReviews paused. |
…coder-backed-issuance-readers # Conflicts: # src/functions/OpenCapTable/convertibleIssuance/getConvertibleIssuanceAsOcf.ts # src/functions/OpenCapTable/equityCompensationIssuance/getEquityCompensationIssuanceAsOcf.ts # test/validation/damlToOcfValidation.test.ts
|
Independent release review found and fixed the final semantic-traversal blocker in The pre-AJV convertible-discount scan is now iterative and identity-memoized, with linked diagnostic paths materialized only on an actual issue. New regressions prove that all three writer surfaces visit a shared DAG once per unique container, return a bounded SDK validation error for a 20,000-level unknown-root chain instead of Validation is green locally: 87 suites / 4,874 tests, build, declarations, typecheck, lint, format, and diff check; an independent rerun of the DAG/path and 20,000-level matrices also passes. Keeping this PR draft/unassigned while exact GitHub CI and QuickStart rerun. Reviewer requests remain deferred until its parent chain is replayed and stable. |
|
Exact head
The PR remains draft/unassigned because #434 and the upstream parent chain must be replayed onto their newly hardened heads before exact reviewer evidence would be durable. |
…coder-backed-issuance-readers
…coder-backed-issuance-readers # Conflicts: # src/functions/OpenCapTable/capTable/ocfToDaml.ts # src/functions/OpenCapTable/shared/singleContractRead.ts
|
Updated exact head 425a2dd after reconciling the final #434 base. Validation on this exact SHA:
Keeping the PR draft and unassigned while remote CI and mergeability initialize. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR hardens the SDK’s cap-table issuance read/write boundaries by routing convertible, equity-compensation, stock, and warrant issuance payloads through generated DAML codecs and stricter canonical parsing before OCF conversion. It replaces prior “structural cast” behavior with decoder-path-based schema mismatch errors and tighter numeric/integer handling.
Changes:
- Added dedicated complex-issuance decoding/dispatch via
decodeDamlEntityData/extractAndDecodeDamlEntityData, and standardized issuance reader results to{ event, contractId }. - Introduced strict fixed-point Numeric(10) parsing (no exponent/scientific notation), safer integer parsing, and updated writer validation to require canonical
object_typeand plain JSON inputs. - Migrated/expanded tests and type-level contracts (source + dist) to assert exact boundary types, decoder-path error shapes, and empty-text preservation where DAML allows it.
Reviewed changes
Copilot reviewed 47 out of 48 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/validation/damlToOcfValidation.test.ts | Updates issuance reader validation expectations to generated-decoder OcpParseError shapes and {event} results. |
| test/utils/vesting.test.ts | Aligns vesting write-boundary behavior with “no filtering” and bracket-style diagnostic paths. |
| test/utils/triggerFields.test.ts | Updates trigger_condition expectations to allow empty/blank Text values. |
| test/types/stockIssuanceReaders.types.ts | Adds compile-time contracts for stock issuance reader/writer/public API type exactness. |
| test/types/complexIssuanceReaders.types.ts | Adds compile-time contracts for convertible/equity-comp/warrant issuance boundaries and pricing-branch typing. |
| test/schemaAlignment/dateBoundaryInvariants.test.ts | Updates AST-based invariants for vesting boundary delegation/usage and removes filtering expectations. |
| test/functions/stockIssuanceBoundaries.test.ts | Adds end-to-end stock issuance boundary tests across direct/dispatcher/operation/ledger/public surfaces. |
| test/functions/generatedDamlNumericReaders.test.ts | Reframes numeric/monetary boundary failures as generated-decoder schema mismatches where applicable. |
| test/declarations/stockIssuanceReaders.types.ts | Adds built (dist) declaration contracts mirroring stock issuance source type contracts. |
| test/declarations/complexIssuanceReaders.types.ts | Adds built (dist) declaration contracts for complex issuance reader/writer/public API exactness. |
| test/createOcf/stockIssuanceReadConversions.test.ts | Updates stock issuance DAML→OCF conversion tests to generated-decoder parse-error shapes and empty Text preservation. |
| test/createOcf/falsyFieldRoundtrip.test.ts | Extends falsy/null preservation fixtures for convertible issuance decoding. |
| test/converters/valuationVestingConverters.test.ts | Tightens vesting-period integer rejection codes and expands unsafe/overlong cases. |
| test/converters/dateBoundaryValidation.test.ts | Aligns read/write date boundary tests with generated-decoder error sourcing and bracket paths; rejects explicit nulls on write where required. |
| test/converters/conversionWriterBoundaries.test.ts | Updates bracket-style array paths in expected validation errors. |
| test/converters/conversionTriggerVariants.test.ts | Adjusts trigger variant tests for blank Text preservation and generated-decoder parse errors on schema divergence. |
| test/converters/conversionSemanticBoundaries.test.ts | Updates semantic boundary tests to expect generated-decoder schema mismatch for generated-wire numeric misuse. |
| test/converters/conversionMechanismMatrix.test.ts | Updates mechanism matrix tests for bracket paths, null-root typing, valuation enum encoding, and blank description preservation. |
| test/converters/conversionDescriptorBoundaries.test.ts | Splits “generated issuance” reader boundary expectations into decoder-path-based parse errors vs validation errors elsewhere. |
| src/utils/conversionTriggers.ts | Changes conversion-trigger parsing to preserve empty/blank strings and updates duplicate-id diagnostics to bracket paths. |
| src/functions/OpenCapTable/warrantIssuance/getWarrantIssuanceAsOcf.ts | Moves warrant issuance decoding to generated codec boundary, enforces template id, standardizes {event} result, and uses strict Numeric(10) parsing for vestings. |
| src/functions/OpenCapTable/warrantIssuance/createWarrantIssuance.ts | Requires canonical object_type, plain writer input, stricter text/date handling, and post-conversion canonical validation. |
| src/functions/OpenCapTable/vestingTerms/vestingPeriodInteger.ts | Uses safe-integer encoding helper and separates out-of-range vs format/type errors. |
| src/functions/OpenCapTable/stockIssuance/getStockIssuanceAsOcf.ts | Routes stock issuance decoding through generated codec boundary and standardizes {event} result; relaxes empty Text handling. |
| src/functions/OpenCapTable/stockIssuance/createStockIssuance.ts | Rewrites stock issuance writer to produce exact generated payload with strict Numeric(10)/Monetary parsing and canonical validation. |
| src/functions/OpenCapTable/shared/vesting.ts | Removes vesting-row filtering and enforces strict Numeric(10) parsing with bracket diagnostic paths. |
| src/functions/OpenCapTable/shared/triggerFields.ts | Removes non-blank enforcement for trigger_condition (Text preservation). |
| src/functions/OpenCapTable/shared/ocfWriterValidation.ts | Introduces validateCanonicalObjectType and improves contextual error-path rewriting; preserves empty strings for Text. |
| src/functions/OpenCapTable/shared/ocfValues.ts | Tightens Monetary shape validation (exact fields), adjusts required-vs-type handling for null/undefined, and bracketizes string array paths. |
| src/functions/OpenCapTable/shared/generatedDamlValues.ts | Disallows exponent-form Numeric(10) parsing and changes required handling to treat only undefined as “missing”. |
| src/functions/OpenCapTable/shared/damlNumerics.ts | Adds strict DAML Numeric(10) parsing and Monetary encoding/decoding helpers (no exponent; range/scale checks). |
| src/functions/OpenCapTable/shared/damlIntegers.ts | Adds safe integer encoding and strict integer-like string parsing (no scientific notation; bounded length; safe-range checks). |
| src/functions/OpenCapTable/shared/conversionMechanisms.ts | Aligns conversion mechanisms with strict optional text/date handling, valuation enum mapping, and null-root invalid-type classification. |
| src/functions/OpenCapTable/equityCompensationIssuance/getEquityCompensationIssuanceAsOcf.ts | Routes equity-comp issuance through generated codec boundary, uses strict numeric/integer parsing, and validates pricing from raw DAML shapes. |
| src/functions/OpenCapTable/equityCompensationIssuance/equityCompensationPricing.ts | Tightens Monetary shape errors, disallows exponent-form amounts, enforces nonnegative amounts, and adds DAML-specific pricing validator. |
| src/functions/OpenCapTable/equityCompensationIssuance/createEquityCompensationIssuance.ts | Rewrites writer to exact generated payload with canonical object_type enforcement, strict Numeric(10)/Int parsing, and canonical validation. |
| src/functions/OpenCapTable/convertibleIssuance/getConvertibleIssuanceAsOcf.ts | Routes convertible issuance through generated codec boundary, enforces template id, and preserves empty Text where allowed. |
| src/functions/OpenCapTable/convertibleIssuance/createConvertibleIssuance.ts | Requires canonical object_type/plain input and validates final canonical OCF shape after conversion. |
| src/functions/OpenCapTable/capTable/ocfToDaml.ts | Routes issuance writers through dedicated exact paths instead of generic schema parsing flow. |
| src/functions/OpenCapTable/capTable/damlToOcf.ts | Dispatches issuance types through correlated generated-decoder preflight before generic conversions. |
| src/functions/OpenCapTable/capTable/damlEntityData.ts | Integrates complex issuance decode/extract/validation into shared entity decoder and create-argument extraction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Review follow-up pushed in f67ac18: removed the two unused numeric helpers and consolidated the duplicate string validators. Local validation is green: build, declaration/public API checks, 109/109 test suites (7,236 tests), typecheck, lint, Prettier, and diff-check. An independent exact-SHA audit also reported CLEAN. Keeping this PR draft while CI reruns. |
…coder-backed-issuance-readers
|
Integrated corrected parent #434 head 9cf5593 via a normal merge at f075f38. The merge was conflict-free and an independent exact-SHA audit verified both ancestries, preservation of all upstream review fixes, an empty remerge diff, and no issuance-source drift. Validation is green: focused 6/6 suites (1,011 tests), full 109/109 suites (7,237 tests), build, declarations/public API/cardinality/package-consumer checks, typecheck, lint, Prettier, and diff-check. Keeping this PR draft and unassigned while fresh CI runs. |
|
@cursor review Please review exact current head |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f075f38. Configure here.
|
Restacked onto #434 at exact head |
Summary
Why
These readers previously trusted ledger payloads through structural casts. That allowed malformed or cross-template data to reach converters and made the public type contract look safer than the runtime boundary actually was. Strict decoding is intentionally backwards-incompatible for invalid and historical payload shapes.
Validation
npm run formatnpm run lintnpm run cleannpm run buildnpm run test:declarationsnpm run test:ci— 76 suites, 2,882 testsStack
Note
High Risk
Touches core cap-table issuance read/write paths and financial fields (quantities, prices, triggers); stricter decoding will reject wrong templates and malformed data that previously slipped through casts.
Overview
Complex issuance reads no longer trust cast ledger JSON. A new
issuanceContractDatapath decodes convertible, equity compensation, stock, and warrant issuances through their full contract wrapper codecs, with plain-data preflight, required own-properties, and lossless encode/decode checks.decodeDamlEntityData/extractAndDecodeDamlEntityDataanddamlToOcfdispatch these kinds before the generic decoder so direct helpers, the dispatcher, andget*AsOcfshare the same parse-error family.Per-entity writers and readers are aligned to that boundary: issuance
*DataToDamlflows use canonical OCF writer validation (requiredobject_type, contextualparseOcfEntityInputerrors); converters calldecodeDamlEntityDataup front; contract reads useextractAndDecodeDamlEntityDataplus expected template IDs instead of hand-parsingissuance_data.Shared validation changes (also affect conversion mechanisms and vesting):
parseDamlNumeric10/nativeMonetaryToDamlNumeric10, stricterparseDamlSafeInteger(no scientific notation / unsafe range), DAML Numeric without exponents, optional text allows empty strings, and equity compensationvalidateEquityCompensationPricingFromDamlso compensation-type rules win over malformed forbidden price fields. Stock issuance stops dropping zero vestings / 0–0 share ranges on write; severalget*AsOcfresults exposeeventinstead of type-specific property names.Intentionally backwards-incompatible for historically permissive or malformed ledger and test payloads.
Reviewed by Cursor Bugbot for commit f075f38. Bugbot is set up for automated code reviews on this repo. Configure here.