|
| 1 | +# Development Documentation: Function Support Dev Issues |
| 2 | + |
| 3 | +## Resolved Issues |
| 4 | + |
| 5 | +### 1. Inconsistent Numeric Types (Literals and Constraints) |
| 6 | +Rune `number` and `int` were handled inconsistently, leading to precision loss and `TypeError` when interacting with Python `Decimal` fields in financial models. |
| 7 | + |
| 8 | +* **Resolution**: Strictly mapped Rune `number` to Python `Decimal`. Literals and constraints are now explicitly wrapped in `Decimal('...')` during generation. |
| 9 | +* **Status**: Fixed ([`fe798c1`](https://github.com/finos/rune-python-generator/commit/fe798c1)) |
| 10 | +* **Summary of Impact**: |
| 11 | + * **Precision**: Ensures that calculations involving monetary amounts maintain exact precision, avoiding the pitfalls of floating-point arithmetic. |
| 12 | + * **Type Safety**: Prevents runtime crashes when Pydantic models expect `Decimal` but receive `float` or `int`. |
| 13 | + |
| 14 | +### 2. Redundant Logic Generation |
| 15 | +Expression logic was previously duplicated between different generator components, leading to maintenance overhead and potential diverging implementations of the same DSL logic. |
| 16 | + |
| 17 | +* **Resolution**: Centralized all expression logic within `PythonExpressionGenerator`. Removed `generateIfBlocks` from the higher-level function generator to prevent duplicate emission of conditional statements. |
| 18 | +* **Status**: Fixed ([`2fb276d`](https://github.com/finos/rune-python-generator/commit/2fb276d)) |
| 19 | +* **Summary of Impact**: |
| 20 | + * **Maintainability**: Logic changes (like the Switch fix) only need to be implemented in one place. |
| 21 | + * **Code Quality**: The generated Python is cleaner and follows a predictable pattern for side-effecting blocks. |
| 22 | + |
| 23 | +### 3. Missing Function Dependencies (Recursion & Enums) |
| 24 | +The dependency provider failed to identify imports for types deeply nested in expressions or those referenced via `REnumType`. |
| 25 | + |
| 26 | +* **Resolution**: Implemented recursive tree traversal in `PythonFunctionDependencyProvider` and added explicit handling for Enum types to ensure they are captured in the import list. |
| 27 | +* **Status**: Fixed ([`4878326`](https://github.com/finos/rune-python-generator/commit/4878326)) |
| 28 | +* **Summary of Impact**: |
| 29 | + * **Runtime Stability**: Resolves `NameError` exceptions in generated code where functions used types that were not imported at the top of the module. |
| 30 | + * **Enum Integration**: Functions can now safely use Rosetta-defined enums in conditions and assignments. |
| 31 | + |
| 32 | +### 4. Robust Dependency Management (DAG Population) |
| 33 | +The generator's dependency graph (DAG) previously failed to capture attribute-level dependencies and function-internal dependencies, leading to `NameError` exceptions when a class or function was defined after it was referenced. |
| 34 | + |
| 35 | +* **Resolution**: Implemented recursive dependency extraction in `PythonModelObjectGenerator` and `PythonFunctionGenerator`. The DAG now includes edges for all class attributes, function inputs/outputs, and symbol references, guaranteeing a topologically correct definition order in `_bundle.py` for acyclic dependencies. |
| 36 | +* **Status**: Fixed ([`b813ff0`](https://github.com/finos/rune-python-generator/commit/b813ff0)) |
| 37 | +* **Verification**: `PythonFunctionOrderTest` (Java) confirms strict ordering for linear dependencies. |
| 38 | +* **Summary of Impact**: |
| 39 | + * **Generation Stability**: Eliminates `NameError` causing build failures. |
| 40 | + * **Correctness**: Ensures that the generated Python code adheres to the "define-before-use" principle required by the language. |
| 41 | + |
| 42 | +### 5. Mapping of [metadata id] for Enums |
| 43 | +Enums with metadata constraints failed to support validation and key referencing because they were generated as plain Python `Enum` classes, which cannot carry metadata payloads or validators. |
| 44 | + |
| 45 | +* **Resolution**: Enums with explicit metadata (e.g., `[metadata id]`, `[metadata reference]`) are now wrapped in `Annotated[Enum, serializer(), validator()]`. The `serializer` and `validator` methods are provided by the `EnumWithMetaMixin` runtime class. |
| 46 | +* **Status**: Fixed ([`b813ff0`](https://github.com/finos/rune-python-generator/commit/b813ff0)) |
| 47 | +* **Summary of Impact**: |
| 48 | + * **Feature Parity**: Brings Enums up to par with complex types regarding metadata support. |
| 49 | + * **Validation**: Enables `@key` and `@ref` validation for Enum fields. |
| 50 | + |
| 51 | +### 6. Inconsistent Type Mapping (Centralization) |
| 52 | +Type string generation was scattered across multiple classes, making it impossible to implement global features like string forward-referencing or custom cardinality formatting. |
| 53 | + |
| 54 | +* **Resolution**: Centralized type mapping and formatting in `RuneToPythonMapper`, adding a flexible `formatPythonType` method that handles both legacy and modern typing styles. |
| 55 | +* **Status**: Fixed ([`fe798c1`](https://github.com/finos/rune-python-generator/commit/fe798c1)) |
| 56 | +* **Summary of Impact**: |
| 57 | + * **Extensibility**: Enabled the groundwork for "String Forward References". |
| 58 | + * **Cardinality Control**: Standardized how `list[...]` and `Optional[...]` are generated. |
| 59 | + |
| 60 | +### 7. Switch Expression Support |
| 61 | +`generateSwitchOperation` returned a block of statements instead of a single expression, causing `SyntaxError` during variable assignment. |
| 62 | + |
| 63 | +* **Resolution**: Encapsulated switch logic within a unique helper function closure (`_switch_fn_0`) and returned a call to that function. |
| 64 | +* **Status**: Fixed ([`4d394c5`](https://github.com/finos/rune-python-generator/commit/4d394c5)) |
| 65 | + |
| 66 | +--- |
| 67 | + |
| 68 | +## Unresolved Issues and Proposals |
| 69 | + |
| 70 | +### 1. Constructor-Related Issues |
| 71 | + |
| 72 | +#### Issue: Fragile Object Building (Direct Constructors) |
| 73 | +**Problem**: The generator relies on a magic `_get_rune_object` helper which bypasses IDE checks and is hard to debug. |
| 74 | +* **Recommendation**: Refactor `PythonFunctionGenerator` to use direct Python constructor calls (e.g., `MyClass(attr=val)`). |
| 75 | +* **Status**: **Unresolved**. The codebase currently uses `_get_rune_object`. |
| 76 | + |
| 77 | +#### Issue: Constructor Keyword Arguments SyntaxError |
| 78 | +**Problem**: Python forbids duplicate or invalid keyword arguments. |
| 79 | +* **Recommendation**: Use unique counters for missing/duplicate keys. |
| 80 | +* **Proposed Fix**: The generator should use unique fallback keys (`unknown_0`, `unknown_1`, etc.) when property names are missing or invalid. |
| 81 | +* **Recommended Code Changes**: Use an `AtomicInteger` for unique fallback keys in `PythonExpressionGenerator.java`. |
| 82 | + |
| 83 | +#### Issue: Partial Object Construction (Required Fields) |
| 84 | +**Problem**: Pydantic's default constructor enforces validation immediately, breaking multi-step `set` operations. |
| 85 | +* **Recommendation**: Use `model_construct()`. |
| 86 | +* **Proposed Solution**: Use `model_construct(**kwargs)` for initial object creation to skip validation, allowing the object to be filled via subsequent `set` calls before final consumption. |
| 87 | + |
| 88 | +--- |
| 89 | + |
| 90 | +### 2. Bundle Generation and Dependency Issues |
| 91 | + |
| 92 | +#### Issue: Circular Dependencies / Out-of-Order Definitions (The "Topological Sort" Limitation) |
| 93 | +**Manifestation**: `NameError: name 'cdm_base_datetime_BusinessCenterTime' is not defined` during CDM import. |
| 94 | + |
| 95 | +**Problem**: The generator uses a Directed Acyclic Graph (DAG) to order definitions in `_bundle.py`. However, the current implementation only adds edges for **inheritance (SuperTypes)**. It ignores **Attribute types**, leading to out-of-order definitions. Furthermore, Rosetta allows recursive/circular types (e.g., A has attribute B, B has attribute A), which a DAG cannot resolve by design. |
| 96 | + |
| 97 | +**Reproducing Tests**: |
| 98 | +* **JUnit**: `PythonFunctionOrderTest.testFunctionDependencyOrder` (asserts ClassA defined before ClassB). |
| 99 | +* **Python**: `test_functions_order.py` (triggers NameError during Pydantic decorator execution). |
| 100 | + |
| 101 | +**Proposed Alternatives & Recommendation**: |
| 102 | +Use **String Forward References + `model_rebuild()`** (The official "Pydantic Way" for v2). |
| 103 | +* **The Hybrid DAG Strategy**: We will continue to use the DAG to organize the definition order of classes, but we will limit its scope to **Inheritance only** (`SuperType`). By using String Forward References for attributes, we eliminate the need for the DAG to handle the complex "web" of references, avoiding cycle detection failures while ensuring that parent classes are always defined before their children. |
| 104 | + |
| 105 | +#### Issue: FQN Type Hints for Clean API (Dots vs. Underscores) |
| 106 | +**Problem**: The current generator uses "bundle-mangled" names with underscores (e.g., `cdm_base_datetime_AdjustableDate`) in function and constructor signatures to avoid collisions. |
| 107 | + |
| 108 | +**Proposal**: Use fully qualified, period-delimited names (e.g., `"cdm.base.datetime.AdjustableDate"`) in all signatures. |
| 109 | +* **Mechanism**: Utilize a `_type_registry` mapping at the end of the `_bundle.py` that links Rosetta FQNs to the bundled Python class definitions. |
| 110 | +* **Dependency**: This approach **requires** implementing the **String Forward Reference** solution for circular dependencies. |
| 111 | +* **Benefits**: API Purity (matches CDM/Rosetta names exactly), Consistency, and Encapsulation. |
| 112 | + |
| 113 | +#### Issue: Bundle Loading Performance (Implicit Overhead) |
| 114 | +**Problem**: The current "Bundle" architecture results in significant "load-all-at-once" overhead for large models like CDM. |
| 115 | + |
| 116 | +**Proposal**: Evolve the bundle architecture to support **Partitioned Loading** or **Lazy Rebuilds**. |
| 117 | +* **Status**: **Unresolved**. Prerequisite is the fix for Circular Dependencies via String Forward References. |
| 118 | + |
| 119 | +--- |
| 120 | + |
| 121 | +--- |
| 122 | + |
| 123 | +### 3. Support for External Data Sources |
| 124 | + |
| 125 | +#### Issue: Unmapped External Function Calls (`[codeImplementation]`) |
| 126 | +**Problem**: The Rosetta runtime allows functions to be marked with `[codeImplementation]`, indicating logic provided by the host language (e.g., loading XML codelists in Java). The Python generator does not yet emit the syntax to delegate these calls to Python external implementations. |
| 127 | +* **Manifestation**: Validation functions like `ValidateFpMLCodingSchemeDomain` are either omitted or generate empty/invalid bodies. |
| 128 | +* **Recommendation**: Update the generator to emit calls to a standard Python registry/dispatcher for external functions. |
| 129 | + |
| 130 | +#### Issue: Conditions on Basic Types (Strings) |
| 131 | +**Problem**: The DSL allows attaching validation logic directly to basic types (e.g., `typeAlias BusinessCenter: string condition IsValid...`). This feature, introduced to support data validation against external sources, is not supported in the Python generator. |
| 132 | +* **Gap**: The generator may not correctly wrap simple types to attach validators or trigger validation upon assignment. |
| 133 | +* **Status**: **Unresolved Gap**. |
| 134 | + |
| 135 | +#### Issue: Missing Standard Library for External Data |
| 136 | +**Problem**: The CDM Python implementation lacks the infrastructure to replicate the Java version's ability to load external data (e.g., FpML coding schemes) and validate values against them. |
| 137 | +* **Impact**: Even if the generator called the validation function, the runtime mechanism to perform the check does not exist. |
| 138 | +* **Recommendation**: Implement a Python equivalent of the Java `CodelistLoader` and expose it via the runtime library. |
| 139 | + |
| 140 | +--- |
| 141 | + |
| 142 | +## Backlog |
| 143 | + |
| 144 | +### Enum Wrappers (Global Proposal) |
| 145 | +* **Problem**: While explicit metadata is now supported via `Annotated`, plain Enums do not have a uniform wrapper object at runtime. This leads to inconsistent behavior if code expects to attach metadata dynamically to any enum instance. |
| 146 | +* **Proposal**: Wrap *all* enums in a proxy class (`_EnumWrapper`) that holds the enum value and a metadata dictionary. |
| 147 | +* **Relevant Tests**: |
| 148 | + * Java: `PythonEnumMetadataTest.testEnumWithoutMetadata` (Disabled) |
| 149 | + * Python: `test_enum_metadata.py::test_enum_metadata_behavior` (Skipped) |
| 150 | +* **Usage Note**: This is a proposed architectural change, not a defect fix. |
| 151 | + |
| 152 | +--- |
| 153 | + |
| 154 | +### General Support Suggestions |
| 155 | + |
| 156 | +* **Type Unification Support**: Evaluate if `BaseDataClass` inheriting from a metadata mixin provides a scalable way to handle metadata across various model sizes. |
| 157 | +* **Operator Support**: Consider standardizing helper functions like `rune_with_meta` and `rune_default` to simplify generated code. |
0 commit comments