25868: Renames internal paramaters for consistency, adds . to all internally computed features, MAJOR - #920
25868: Renames internal paramaters for consistency, adds . to all internally computed features, MAJOR#920howsoRes wants to merge 6 commits into
Conversation
howso-mario
left a comment
There was a problem hiding this comment.
The broad dataset-parameter renames, 119.0.0 release migration, and dot-prefixed default feature creation pass the focused tests I ran. I’m requesting changes for three concrete correctness issues: details.amlg now looks up dot-prefixed keys in a map that remains keyed by undotted computation names; the saved-state migration does not rename featureMdaMap inside the default parameter map; and react_aggregate introduces an unused, misspelled public parameter. I’m treating the other public renames as intentional MAJOR-version changes rather than requiring compatibility aliases. CI is also currently blocked by the repeated Linux aggregate-goal test failure.
| can_use_computed_contribution_values | ||
| (contains_index (get !computedFeaturesMap "computed_map") "similarity_conviction") | ||
| (!= .null (retrieve_from_entity existing_case_id (get !computedFeaturesMap ["computed_map" "similarity_conviction"]) )) | ||
| (contains_index (get !computedFeaturesMap "computed_map") ".similarity_conviction") |
There was a problem hiding this comment.
!computedFeaturesMap["computed_map"] is keyed by undotted computation names, with dot-prefixed feature names stored as its values. A read-only runtime reproduction after react_into_features produced {similarity_conviction: ".similarity_conviction", residual_contribution: ".residual_contribution"}. Therefore this new lookup for ".similarity_conviction" always misses; the equivalent changes at lines 511 and 550 also miss for residual and distance contribution. The optimized existing-case path consequently recomputes these metrics instead of retrieving their stored feature values, potentially changing results and adding substantial query cost. Please retain the undotted map keys while allowing their mapped feature names to be dot-prefixed, and add a regression test covering details for an existing case after react_into_features.
| (assign_to_entities (assoc | ||
|
|
||
| !dataParametersPaths (get import_metadata_map "!hyperparameterParamPaths") | ||
| !defaultDataParametersMap (get import_metadata_map "!defaultHyperparameters") |
There was a problem hiding this comment.
The migration copies !defaultHyperparameters directly into !defaultDataParametersMap, but only rewrites nested featureMdaMap keys in !hyperparameterMetadataMap. All new default-map consumers read featureProbabilitiesMap, so an upgraded trainee whose default map contains the old key can silently lose those probabilities and fall back to other weights. Please apply the same recursive key migration to the default map and extend ut_h_migration.amlg to assert both migrated maps contain featureProbabilitiesMap and no featureMdaMap.
| ; specified robust or non-robust type. | ||
| #{type "boolean"} | ||
| robust_hyperparameters .null | ||
| robust_dataparamseters .null |
There was a problem hiding this comment.
robust_dataparamseters appears misspelled. Because this is a declared public endpoint parameter, the typo becomes part of validation/generated schemas and callers using the natural spelling will be rejected. It is also not referenced anywhere after declaration, so the accepted parameter currently has no effect. Please choose the canonical spelling—consistent with the other snake-case dataset-parameter names—wire it into the intended robust-selection behavior or remove it, and add a focused endpoint test.
renames:
!hyperparameterParamPaths to !dataParametersPaths
!defaultHyperparameters to !defaultDataParametersMap
!hyperparameterMetadataMap to !hyperparameterMetadataMap
all other internal variables or parameters referencing 'hyperparameters' to 'data parameters'
internal data parameter 'featureMdaMap' is renamed to 'featureProbabilitiesMap'
adds a leading '.' to conviction-related auto-computed features such as '.distance_contribution' and '.similarity_conviction' for flows where user does not manually specify a feature name