diff --git a/docs/tech-specs/flow-configurable-parameters.md b/docs/tech-specs/flow-configurable-parameters.md index 546d66b8..b3b0ee5a 100644 --- a/docs/tech-specs/flow-configurable-parameters.md +++ b/docs/tech-specs/flow-configurable-parameters.md @@ -263,22 +263,35 @@ The system will: #### Parameter Resolution Process -1. **Flow Class Loading**: Load flow class and extract parameter metadata -2. **Metadata Extraction**: Extract `type`, `description`, `order`, `advanced`, and `controlled-by` for each parameter -3. **Type Definition Lookup**: Retrieve parameter type definitions from schema/config store using `type` field -4. **UI Form Generation**: - - Use `description` and `order` fields to create ordered parameter forms - - Parameters with `advanced: true` are hidden in basic mode or grouped in an "Advanced" section - - Parameters with `controlled-by` may be hidden in simple mode if they inherit from their controller -5. **Parameter Inheritance Resolution**: +When a flow is started, the system performs the following parameter resolution steps: + +1. **Flow Class Loading**: Load flow class definition and extract parameter metadata +2. **Metadata Extraction**: Extract `type`, `description`, `order`, `advanced`, and `controlled-by` for each parameter defined in the flow class's `parameters` section +3. **Type Definition Lookup**: For each parameter in the flow class: + - Retrieve the parameter type definition from schema/config store using the `type` field + - The type definitions are stored with type "parameter-types" in the config system + - Each type definition contains the parameter's schema, default value, and validation rules +4. **Default Value Resolution**: + - For each parameter defined in the flow class: + - Check if the user provided a value for this parameter + - If no user value provided, use the `default` value from the parameter type definition + - Build a complete parameter map containing both user-provided and default values +5. **Parameter Inheritance Resolution** (controlled-by relationships): - For parameters with `controlled-by` field, check if a value was explicitly provided - If no explicit value provided, inherit the value from the controlling parameter - If the controlling parameter also has no value, use the default from the type definition -6. **Validation**: Validate user-provided and inherited parameters against type definitions -7. **Default Application**: Apply default values for missing parameters from type definitions + - Validate that no circular dependencies exist in `controlled-by` relationships +6. **Validation**: Validate the complete parameter set (user-provided, defaults, and inherited) against type definitions +7. **Storage**: Store the complete resolved parameter set with the flow instance for auditability 8. **Template Substitution**: Replace parameter placeholders in processor parameters with resolved values 9. **Processor Instantiation**: Create processors with substituted parameters +**Important Implementation Notes:** +- The flow service MUST merge user-provided parameters with defaults from parameter type definitions +- The complete parameter set (including applied defaults) MUST be stored with the flow for traceability +- Parameter resolution happens at flow start time, not at processor instantiation time +- Missing required parameters without defaults MUST cause flow start to fail with a clear error message + #### Parameter Inheritance with controlled-by The `controlled-by` field enables parameter value inheritance, particularly useful for simplifying user interfaces while maintaining flexibility: @@ -337,6 +350,43 @@ The `controlled-by` field enables parameter value inheritance, particularly usef } ``` +#### Flow Service Implementation + +The flow configuration service (`trustgraph-flow/trustgraph/config/service/flow.py`) requires the following enhancements: + +1. **Parameter Resolution Function** + ```python + async def resolve_parameters(self, flow_class, user_params): + """ + Resolve parameters by merging user-provided values with defaults. + + Args: + flow_class: The flow class definition dict + user_params: User-provided parameters dict + + Returns: + Complete parameter dict with user values and defaults merged + """ + ``` + + This function should: + - Extract parameter metadata from the flow class's `parameters` section + - For each parameter, fetch its type definition from config store + - Apply defaults for any parameters not provided by the user + - Handle `controlled-by` inheritance relationships + - Return the complete parameter set + +2. **Modified `handle_start_flow` Method** + - Call `resolve_parameters` after loading the flow class + - Use the complete resolved parameter set for template substitution + - Store the complete parameter set (not just user-provided) with the flow + - Validate that all required parameters have values + +3. **Parameter Type Fetching** + - Parameter type definitions are stored in config with type "parameter-types" + - Each type definition contains schema, default value, and validation rules + - Cache frequently-used parameter types to reduce config lookups + #### Config System Integration 3. **Flow Object Storage** @@ -377,6 +427,10 @@ The `controlled-by` field enables parameter value inheritance, particularly usef - Substitution occurs alongside `{id}` and `{class}` replacement - Invalid parameter references result in launch-time errors - Type validation happens based on the centrally-stored parameter definition +- **IMPORTANT**: All parameter values are stored and transmitted as strings + - Numbers are converted to strings (e.g., `0.7` becomes `"0.7"`) + - Booleans are converted to lowercase strings (e.g., `true` becomes `"true"`) + - This is required by the Pulsar schema which defines `parameters = Map(String())` Example resolution: ``` @@ -384,6 +438,11 @@ Flow parameter mapping: "model": "llm-model" Processor parameter: "model": "{model}" User provides: "model": "gemma3:8b" Final parameter: "model": "gemma3:8b" + +Example with type conversion: +Parameter type default: 0.7 (number) +Stored in flow: "0.7" (string) +Substituted in processor: "0.7" (string) ``` ## Testing Strategy diff --git a/trustgraph-cli/trustgraph/cli/start_flow.py b/trustgraph-cli/trustgraph/cli/start_flow.py index 317276fc..fa9ce6a8 100644 --- a/trustgraph-cli/trustgraph/cli/start_flow.py +++ b/trustgraph-cli/trustgraph/cli/start_flow.py @@ -5,6 +5,9 @@ Parameters can be provided in three ways: 1. As key=value pairs: --param model=gpt-4 --param temp=0.7 2. As JSON string: -p '{"model": "gpt-4", "temp": 0.7}' 3. As JSON file: --parameters-file params.json + +Note: All parameter values are stored as strings internally, regardless of their +input format. Numbers and booleans will be converted to string representation. """ import argparse @@ -81,9 +84,13 @@ def main(): if args.parameters_file: with open(args.parameters_file, 'r') as f: - parameters = json.load(f) + params_data = json.load(f) + # Convert all values to strings + parameters = {k: str(v) for k, v in params_data.items()} elif args.parameters: - parameters = json.loads(args.parameters) + params_data = json.loads(args.parameters) + # Convert all values to strings + parameters = {k: str(v) for k, v in params_data.items()} elif args.param: # Parse key=value pairs parameters = {} @@ -95,23 +102,9 @@ def main(): key = key.strip() value = value.strip() - # Try to parse value as JSON first (for numbers, booleans, etc.) - try: - # Handle common cases where we want to preserve the string - if value.lower() in ['true', 'false']: - parameters[key] = value.lower() == 'true' - elif value.replace('.', '').replace('-', '').isdigit(): - # Check if it's a number - if '.' in value: - parameters[key] = float(value) - else: - parameters[key] = int(value) - else: - # Keep as string - parameters[key] = value - except ValueError: - # If JSON parsing fails, treat as string - parameters[key] = value + # All parameter values must be strings for Pulsar + # Just store everything as a string + parameters[key] = value start_flow( url = args.api_url, diff --git a/trustgraph-flow/trustgraph/config/service/flow.py b/trustgraph-flow/trustgraph/config/service/flow.py index 6b55c36f..b99b7d0a 100644 --- a/trustgraph-flow/trustgraph/config/service/flow.py +++ b/trustgraph-flow/trustgraph/config/service/flow.py @@ -10,6 +10,95 @@ class FlowConfig: def __init__(self, config): self.config = config + # Cache for parameter type definitions to avoid repeated lookups + self.param_type_cache = {} + + async def resolve_parameters(self, flow_class, user_params): + """ + Resolve parameters by merging user-provided values with defaults. + + Args: + flow_class: The flow class definition dict + user_params: User-provided parameters dict (may be None or empty) + + Returns: + Complete parameter dict with user values and defaults merged (all values as strings) + """ + # If the flow class has no parameters section, return user params as-is (stringified) + if "parameters" not in flow_class: + if not user_params: + return {} + # Ensure all values are strings + return {k: str(v) for k, v in user_params.items()} + + resolved = {} + flow_params = flow_class["parameters"] + user_params = user_params if user_params else {} + + # First pass: resolve parameters with explicit values or defaults + for param_name, param_meta in flow_params.items(): + # Check if user provided a value + if param_name in user_params: + # Store as string + resolved[param_name] = str(user_params[param_name]) + else: + # Look up the parameter type definition + param_type = param_meta.get("type") + if param_type: + # Check cache first + if param_type not in self.param_type_cache: + try: + # Fetch parameter type definition from config store + type_def = await self.config.get("parameter-types").get(param_type) + if type_def: + self.param_type_cache[param_type] = json.loads(type_def) + else: + logger.warning(f"Parameter type '{param_type}' not found in config") + self.param_type_cache[param_type] = {} + except Exception as e: + logger.error(f"Error fetching parameter type '{param_type}': {e}") + self.param_type_cache[param_type] = {} + + # Apply default from type definition (as string) + type_def = self.param_type_cache[param_type] + if "default" in type_def: + default_value = type_def["default"] + # Convert to string based on type + if isinstance(default_value, bool): + resolved[param_name] = "true" if default_value else "false" + else: + resolved[param_name] = str(default_value) + elif type_def.get("required", False): + # Required parameter with no default and no user value + raise RuntimeError(f"Required parameter '{param_name}' not provided and has no default") + + # Second pass: handle controlled-by relationships + for param_name, param_meta in flow_params.items(): + if param_name not in resolved and "controlled-by" in param_meta: + controller = param_meta["controlled-by"] + if controller in resolved: + # Inherit value from controlling parameter (already a string) + resolved[param_name] = resolved[controller] + else: + # Controller has no value, try to get default from type definition + param_type = param_meta.get("type") + if param_type and param_type in self.param_type_cache: + type_def = self.param_type_cache[param_type] + if "default" in type_def: + default_value = type_def["default"] + # Convert to string based on type + if isinstance(default_value, bool): + resolved[param_name] = "true" if default_value else "false" + else: + resolved[param_name] = str(default_value) + + # Include any extra parameters from user that weren't in flow class definition + # This allows for forward compatibility (ensure they're strings) + for key, value in user_params.items(): + if key not in resolved: + resolved[key] = str(value) + + return resolved async def handle_list_classes(self, msg): @@ -99,8 +188,13 @@ class FlowConfig: await self.config.get("flow-classes").get(msg.class_name) ) - # Get parameters from message (default to empty dict if not provided) - parameters = msg.parameters if msg.parameters else {} + # Resolve parameters by merging user-provided values with defaults + user_params = msg.parameters if msg.parameters else {} + parameters = await self.resolve_parameters(cls, user_params) + + # Log the resolved parameters for debugging + logger.debug(f"User provided parameters: {user_params}") + logger.debug(f"Resolved parameters (with defaults): {parameters}") # Apply parameter substitution to template replacement function def repl_template_with_params(tmp):