Renovate runNodeImmediately interface, add summarize to EmailNode, and template fix #422
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Check all task statuses (active, completed, failed, canceled, executing) instead of just active - Add failure_reason, checked_statuses, and last_error to orphaned job logs - Differentiate between badger.ErrKeyNotFound and other storage errors - Improve diagnostics for orphaned job detection
- Add detailed error logging with error_message, error_slug, full_error, and call_data_preview - Change simulation_type from quick to full for more accurate state - Improve error visibility in aggregator logs for debugging
…ite responses - Return values from ABI outputs now take priority over event data - Event data is used only if no return value exists - Fallback to boolean true only if neither return value nor events exist - Change priority logs to Debug level to avoid production noise - Ensures exactInputSingle returns amountOut instead of true
… tests for SendGrid/Telegram
- Remove nodeConfig parameter from output handler interface and implementations - Update CreateEmptyOutput signature to use nodeType string only - Update all output handler method signatures (SetError, SetSuccess, GetError, GetSuccess, GetData) - Remove unused nodeConfig fields from protobuf Node message - Fix TestRunNodeImmediately_ContractWrite_TupleWithTemplates to use derived runner address - Add ethclient connection in tuple test to avoid nil pointer panic
Node configuration is now passed through inputVariables with a special 'config' key. The backend extracts this to create a properly configured TaskNode via CreateNodeFromType. This design ensures nodes are self-contained and consistent across all three code paths: - runNodeWithInputs (testing single nodes) - simulateTask (testing workflows) - deployed workflows (production execution) Supports both 'config' and 'nodeConfig' keys for backward compatibility.
…ture - Changed RunNodeWithInputsReq protobuf to accept TaskNode instead of separate nodeType/nodeConfig fields - Backend now uses ExtractNodeConfiguration() to extract config from TaskNode, sharing code with SimulateTask/CreateTask - Updated RunNodeImmediatelyRPC to use node.Type and extract config from the TaskNode - Updated aggregator RPC server logging to use node.Type from TaskNode - Added comprehensive test cases demonstrating the new API pattern - Created MIGRATION_RUN_NODE_WITH_INPUTS.md guide with before/after examples Benefits: - Type safety: All node config is now properly typed in protobuf - Consistency: Matches SimulateTask and CreateTask API patterns - Completeness: All fields (value, gasLimit, etc.) are properly exposed in schema - Validation: Protobuf schema validation at API layer - Code reuse: Shares ExtractNodeConfiguration() logic across all APIs
- Initialize MacroSecrets automatically in Engine.New() from config - Remove redundant SetMacroSecrets() calls from tests and Aggregator - Include MacroSecrets in testutil.GetAggregatorConfig() from loaded config - Fix QuoterV2 ABI to include all 4 return values (amountOut, sqrtPriceX96After, initializedTicksCrossed, gasEstimate) - Fix QuoterV2 parameter order: (tokenIn, tokenOut, amountIn, fee, sqrtPriceLimitX96) - Fix quote result extraction to use correct data structure - Fix Tenderly simulation test nil pointer by providing ethclient instance This improves encapsulation and ensures tests work without manual secret setup.
The Engine.New() now initializes macroSecrets from config, so we need to clear them AFTER creating the engine, not before.
Same fix as before - aa.GetSenderAddress() requires a valid ethclient, not nil.
The test was using a hardcoded runner address that didn't match the owner's derived address, causing validation to fail. Now uses the actual derived address.
The PayMaster contract on-chain was updated, which changed the hash returned by GetHash(). Updated the test to match the current contract behavior.
- Add TestSchedulerExecutesAllNodesInSequence to verify linear chains execute completely - Add TestSchedulerExecutesNodeAfterBranch to verify nodes after branch paths execute - These tests verify the fix to runKahnScheduler termination condition that was causing nodes at the end of execution chains (like email_report_success) to be skipped
The scheduler incorrectly marked nodes as 'branch targets' if they received ANY branch condition edge, even if they also had regular edges. This caused nodes to remain gated when the branch condition wasn't selected, even though all their regular predecessors had completed. The bug manifested in production where the email node had: - A regular edge from run_swap - A branch condition edge from branch.1 (else path) When branch.0 (if path) was selected, run_swap executed but the email node remained gated by the unselected branch.1 edge and never executed. Fix: Only mark a node as a pure 'branch target' if it has NO regular predecessors. Nodes with mixed edges now execute when all regular predecessors complete, regardless of branch selections. Tests added: - TestSchedulerExecutesAllNodesInSequence: linear execution - TestSchedulerExecutesNodeAfterBranch: branch with continuation - TestSchedulerNodeWithMixedEdges: regression test for the specific bug
…stency - Change RunTriggerReq to accept complete TaskTrigger instead of separate trigger_type and trigger_config fields - Makes RunTrigger consistent with RunNodeWithInputs (uses TaskNode) and SimulateTask (uses TaskTrigger) - Update RPC handler and engine implementation to extract config from TaskTrigger using ExtractTriggerConfigData - Update event_trigger_test.go to construct proper TaskTrigger objects - Improve type safety and maintainability by using structured types instead of raw maps
Implements correct priority system for ContractWrite response data: 1. PRIORITY 1: Function return values from ABI outputs 2. PRIORITY 2: Event data (when found, overrides return values) 3. PRIORITY 3: Boolean true fallback (no return value and no events) Changes: - Extracted priority logic into helper functions for clarity: - addReturnValuesToDecodedData() - handles PRIORITY 1 - handleEventDataForMethod() - handles PRIORITY 2 & 3 - Simplified main execution flow by replacing inline logic with function calls - When events are found in transaction receipt, they now override return values (provides more useful data than simple booleans like approve() returning true) - When no events found but return value exists, keeps the return value - When neither events nor return value exist, uses boolean true as fallback Testing: - Added comprehensive test suite: vm_runner_contract_write_data_priority_test.go - 4 test cases covering all priority levels: * Priority2_EventData_OverridesReturnValue ✅ * Prio * Prio * Prio * Prio * Prio * Prio * Prio * Prio * Prio urnNoEvents ✅ * Priority1_ComplexReturnValue_KeepsDespiteEvents ✅ All tests pass, confirming correct behavior for: - ERC20 approve() with Approval event → returns event fields - ERC20 decimals() without events → returns uint8 value - Functions with no- Functions with no- Functions with no- Functions withith complex returns → keeps return value despite events i- Functions with no- Functions with no- Functions with no- Functions withith val- Functions with no- Functions with no- Functions with no- Functions withith ean returns, providing more context about what happened in the transaction.
… ContractRead node
Closed
chrisli30
added a commit
that referenced
this pull request
Oct 31, 2025
…d template fix (#422) * fix: enhance orphaned job cleanup with detailed logging - Check all task statuses (active, completed, failed, canceled, executing) instead of just active - Add failure_reason, checked_statuses, and last_error to orphaned job logs - Differentiate between badger.ErrKeyNotFound and other storage errors - Improve diagnostics for orphaned job detection * chore: enhance Tenderly simulation error logging - Add detailed error logging with error_message, error_slug, full_error, and call_data_preview - Change simulation_type from quick to full for more accurate state - Improve error visibility in aggregator logs for debugging * fix: prioritize function return values over event data in contract write responses - Return values from ABI outputs now take priority over event data - Event data is used only if no return value exists - Fallback to boolean true only if neither return value nor events exist - Change priority logs to Debug level to avoid production noise - Ensures exactInputSingle returns amountOut instead of true * fix: add RestAPINode.Config.options support and summarizer injection; tests for SendGrid/Telegram * feat: added ai summarizer * Refactor node output handlers and fix tuple test - Remove nodeConfig parameter from output handler interface and implementations - Update CreateEmptyOutput signature to use nodeType string only - Update all output handler method signatures (SetError, SetSuccess, GetError, GetSuccess, GetData) - Remove unused nodeConfig fields from protobuf Node message - Fix TestRunNodeImmediately_ContractWrite_TupleWithTemplates to use derived runner address - Add ethclient connection in tuple test to avoid nil pointer panic * Remove node_config field from RunNodeWithInputsReq in avs.proto * Fix runNodeWithInputs to extract node config from inputVariables Node configuration is now passed through inputVariables with a special 'config' key. The backend extracts this to create a properly configured TaskNode via CreateNodeFromType. This design ensures nodes are self-contained and consistent across all three code paths: - runNodeWithInputs (testing single nodes) - simulateTask (testing workflows) - deployed workflows (production execution) Supports both 'config' and 'nodeConfig' keys for backward compatibility. * refactor: Update RunNodeWithInputs RPC to use complete TaskNode structure - Changed RunNodeWithInputsReq protobuf to accept TaskNode instead of separate nodeType/nodeConfig fields - Backend now uses ExtractNodeConfiguration() to extract config from TaskNode, sharing code with SimulateTask/CreateTask - Updated RunNodeImmediatelyRPC to use node.Type and extract config from the TaskNode - Updated aggregator RPC server logging to use node.Type from TaskNode - Added comprehensive test cases demonstrating the new API pattern - Created MIGRATION_RUN_NODE_WITH_INPUTS.md guide with before/after examples Benefits: - Type safety: All node config is now properly typed in protobuf - Consistency: Matches SimulateTask and CreateTask API patterns - Completeness: All fields (value, gasLimit, etc.) are properly exposed in schema - Validation: Protobuf schema validation at API layer - Code reuse: Shares ExtractNodeConfiguration() logic across all APIs * Fix macro secrets initialization and QuoterV2 ABI issues - Initialize MacroSecrets automatically in Engine.New() from config - Remove redundant SetMacroSecrets() calls from tests and Aggregator - Include MacroSecrets in testutil.GetAggregatorConfig() from loaded config - Fix QuoterV2 ABI to include all 4 return values (amountOut, sqrtPriceX96After, initializedTicksCrossed, gasEstimate) - Fix QuoterV2 parameter order: (tokenIn, tokenOut, amountIn, fee, sqrtPriceLimitX96) - Fix quote result extraction to use correct data structure - Fix Tenderly simulation test nil pointer by providing ethclient instance This improves encapsulation and ensures tests work without manual secret setup. * Fix TestLoadSecretForTask by clearing secrets after engine creation The Engine.New() now initializes macroSecrets from config, so we need to clear them AFTER creating the engine, not before. * Fix nil ethclient in TestContractWriteTenderlySimulation Same fix as before - aa.GetSenderAddress() requires a valid ethclient, not nil. * Fix ExactClientRequest test to use derived runner address The test was using a hardcoded runner address that didn't match the owner's derived address, causing validation to fail. Now uses the actual derived address. * Update TestGetHash expected hash to match current PayMaster contract The PayMaster contract on-chain was updated, which changed the hash returned by GetHash(). Updated the test to match the current contract behavior. * Add regression tests for scheduler premature termination fix - Add TestSchedulerExecutesAllNodesInSequence to verify linear chains execute completely - Add TestSchedulerExecutesNodeAfterBranch to verify nodes after branch paths execute - These tests verify the fix to runKahnScheduler termination condition that was causing nodes at the end of execution chains (like email_report_success) to be skipped * Fix Kahn scheduler bug with nodes having mixed regular and branch edges The scheduler incorrectly marked nodes as 'branch targets' if they received ANY branch condition edge, even if they also had regular edges. This caused nodes to remain gated when the branch condition wasn't selected, even though all their regular predecessors had completed. The bug manifested in production where the email node had: - A regular edge from run_swap - A branch condition edge from branch.1 (else path) When branch.0 (if path) was selected, run_swap executed but the email node remained gated by the unselected branch.1 edge and never executed. Fix: Only mark a node as a pure 'branch target' if it has NO regular predecessors. Nodes with mixed edges now execute when all regular predecessors complete, regardless of branch selections. Tests added: - TestSchedulerExecutesAllNodesInSequence: linear execution - TestSchedulerExecutesNodeAfterBranch: branch with continuation - TestSchedulerNodeWithMixedEdges: regression test for the specific bug * refactor: Update RunTriggerReq to use TaskTrigger structure for consistency - Change RunTriggerReq to accept complete TaskTrigger instead of separate trigger_type and trigger_config fields - Makes RunTrigger consistent with RunNodeWithInputs (uses TaskNode) and SimulateTask (uses TaskTrigger) - Update RPC handler and engine implementation to extract config from TaskTrigger using ExtractTriggerConfigData - Update event_trigger_test.go to construct proper TaskTrigger objects - Improve type safety and maintainability by using structured types instead of raw maps * fix: add more testing around the ai summary function * Refactor ContractWrite data priority system with event handling Implements correct priority system for ContractWrite response data: 1. PRIORITY 1: Function return values from ABI outputs 2. PRIORITY 2: Event data (when found, overrides return values) 3. PRIORITY 3: Boolean true fallback (no return value and no events) Changes: - Extracted priority logic into helper functions for clarity: - addReturnValuesToDecodedData() - handles PRIORITY 1 - handleEventDataForMethod() - handles PRIORITY 2 & 3 - Simplified main execution flow by replacing inline logic with function calls - When events are found in transaction receipt, they now override return values (provides more useful data than simple booleans like approve() returning true) - When no events found but return value exists, keeps the return value - When neither events nor return value exist, uses boolean true as fallback Testing: - Added comprehensive test suite: vm_runner_contract_write_data_priority_test.go - 4 test cases covering all priority levels: * Priority2_EventData_OverridesReturnValue ✅ * Prio * Prio * Prio * Prio * Prio * Prio * Prio * Prio * Prio urnNoEvents ✅ * Priority1_ComplexReturnValue_KeepsDespiteEvents ✅ All tests pass, confirming correct behavior for: - ERC20 approve() with Approval event → returns event fields - ERC20 decimals() without events → returns uint8 value - Functions with no- Functions with no- Functions with no- Functions withith complex returns → keeps return value despite events i- Functions with no- Functions with no- Functions with no- Functions withith val- Functions with no- Functions with no- Functions with no- Functions withith ean returns, providing more context about what happened in the transaction. * chore: remove emoji from Execution Step Log * fix: resolve template variables before validating contract address in ContractRead node * Update core/config/config.go Co-authored-by: Copilot <[email protected]> * Update core/taskengine/summarizer_ai.go Co-authored-by: Copilot <[email protected]> * Improve core/taskengine/summarizer_ai.go * refactor: consolidate task status storage key logic into shared package - Create storage/schema package with centralized TaskStatusToStorageKey logic - Remove duplicate code from core/apqueue/cleanup.go - Update core/taskengine to use shared storage/schema package - Improve comment clarity for branch target gating in vm.go scheduler Fixes code duplication identified by Copilot review * fix: address Copilot code review suggestions - Use exact match for 'undefined' check instead of substring match to avoid false positives - Compile regex pattern once at package level for better performance - Remove unused methodName and contractAddress parameters from extractTransactionValue * Export subject and body in restApi’s response data * Integration tests use salt=2 → smart wallet * fix: add styled HTML email support for AI summary notifications * test: replace GetTestConfig() with direct helper calls in tests Replace all uses of testutil.GetTestConfig() that directly access fields with calls to dedicated helper functions (GetTestTenderlyAccount, GetTestTenderlyProject, GetTestTenderlyAccessKey, GetAggregatorConfig). This modernizes test config access and ensures tests use the proper helper functions that panic with clear error messages if config is missing. Files updated: - core/taskengine/tenderly_client_test.go - core/taskengine/vm_runner_contract_write_data_priority_test.go - core/taskengine/vm_runner_contract_write_simulation_test.go * Change the github actions rpc endpoint value to CHAIN_ENDPOINT * fix summarizer tests: Added nil check for error before accessing err.Error() * Claned up deprecated docs * Relax the AI output verification from summarizer tests --------- Co-authored-by: Wei Lin <[email protected]> Co-authored-by: Will Zimmerman <[email protected]> Co-authored-by: Copilot <[email protected]>
chrisli30
added a commit
that referenced
this pull request
Oct 31, 2025
…d template fix (#422) * fix: enhance orphaned job cleanup with detailed logging - Check all task statuses (active, completed, failed, canceled, executing) instead of just active - Add failure_reason, checked_statuses, and last_error to orphaned job logs - Differentiate between badger.ErrKeyNotFound and other storage errors - Improve diagnostics for orphaned job detection * chore: enhance Tenderly simulation error logging - Add detailed error logging with error_message, error_slug, full_error, and call_data_preview - Change simulation_type from quick to full for more accurate state - Improve error visibility in aggregator logs for debugging * fix: prioritize function return values over event data in contract write responses - Return values from ABI outputs now take priority over event data - Event data is used only if no return value exists - Fallback to boolean true only if neither return value nor events exist - Change priority logs to Debug level to avoid production noise - Ensures exactInputSingle returns amountOut instead of true * fix: add RestAPINode.Config.options support and summarizer injection; tests for SendGrid/Telegram * feat: added ai summarizer * Refactor node output handlers and fix tuple test - Remove nodeConfig parameter from output handler interface and implementations - Update CreateEmptyOutput signature to use nodeType string only - Update all output handler method signatures (SetError, SetSuccess, GetError, GetSuccess, GetData) - Remove unused nodeConfig fields from protobuf Node message - Fix TestRunNodeImmediately_ContractWrite_TupleWithTemplates to use derived runner address - Add ethclient connection in tuple test to avoid nil pointer panic * Remove node_config field from RunNodeWithInputsReq in avs.proto * Fix runNodeWithInputs to extract node config from inputVariables Node configuration is now passed through inputVariables with a special 'config' key. The backend extracts this to create a properly configured TaskNode via CreateNodeFromType. This design ensures nodes are self-contained and consistent across all three code paths: - runNodeWithInputs (testing single nodes) - simulateTask (testing workflows) - deployed workflows (production execution) Supports both 'config' and 'nodeConfig' keys for backward compatibility. * refactor: Update RunNodeWithInputs RPC to use complete TaskNode structure - Changed RunNodeWithInputsReq protobuf to accept TaskNode instead of separate nodeType/nodeConfig fields - Backend now uses ExtractNodeConfiguration() to extract config from TaskNode, sharing code with SimulateTask/CreateTask - Updated RunNodeImmediatelyRPC to use node.Type and extract config from the TaskNode - Updated aggregator RPC server logging to use node.Type from TaskNode - Added comprehensive test cases demonstrating the new API pattern - Created MIGRATION_RUN_NODE_WITH_INPUTS.md guide with before/after examples Benefits: - Type safety: All node config is now properly typed in protobuf - Consistency: Matches SimulateTask and CreateTask API patterns - Completeness: All fields (value, gasLimit, etc.) are properly exposed in schema - Validation: Protobuf schema validation at API layer - Code reuse: Shares ExtractNodeConfiguration() logic across all APIs * Fix macro secrets initialization and QuoterV2 ABI issues - Initialize MacroSecrets automatically in Engine.New() from config - Remove redundant SetMacroSecrets() calls from tests and Aggregator - Include MacroSecrets in testutil.GetAggregatorConfig() from loaded config - Fix QuoterV2 ABI to include all 4 return values (amountOut, sqrtPriceX96After, initializedTicksCrossed, gasEstimate) - Fix QuoterV2 parameter order: (tokenIn, tokenOut, amountIn, fee, sqrtPriceLimitX96) - Fix quote result extraction to use correct data structure - Fix Tenderly simulation test nil pointer by providing ethclient instance This improves encapsulation and ensures tests work without manual secret setup. * Fix TestLoadSecretForTask by clearing secrets after engine creation The Engine.New() now initializes macroSecrets from config, so we need to clear them AFTER creating the engine, not before. * Fix nil ethclient in TestContractWriteTenderlySimulation Same fix as before - aa.GetSenderAddress() requires a valid ethclient, not nil. * Fix ExactClientRequest test to use derived runner address The test was using a hardcoded runner address that didn't match the owner's derived address, causing validation to fail. Now uses the actual derived address. * Update TestGetHash expected hash to match current PayMaster contract The PayMaster contract on-chain was updated, which changed the hash returned by GetHash(). Updated the test to match the current contract behavior. * Add regression tests for scheduler premature termination fix - Add TestSchedulerExecutesAllNodesInSequence to verify linear chains execute completely - Add TestSchedulerExecutesNodeAfterBranch to verify nodes after branch paths execute - These tests verify the fix to runKahnScheduler termination condition that was causing nodes at the end of execution chains (like email_report_success) to be skipped * Fix Kahn scheduler bug with nodes having mixed regular and branch edges The scheduler incorrectly marked nodes as 'branch targets' if they received ANY branch condition edge, even if they also had regular edges. This caused nodes to remain gated when the branch condition wasn't selected, even though all their regular predecessors had completed. The bug manifested in production where the email node had: - A regular edge from run_swap - A branch condition edge from branch.1 (else path) When branch.0 (if path) was selected, run_swap executed but the email node remained gated by the unselected branch.1 edge and never executed. Fix: Only mark a node as a pure 'branch target' if it has NO regular predecessors. Nodes with mixed edges now execute when all regular predecessors complete, regardless of branch selections. Tests added: - TestSchedulerExecutesAllNodesInSequence: linear execution - TestSchedulerExecutesNodeAfterBranch: branch with continuation - TestSchedulerNodeWithMixedEdges: regression test for the specific bug * refactor: Update RunTriggerReq to use TaskTrigger structure for consistency - Change RunTriggerReq to accept complete TaskTrigger instead of separate trigger_type and trigger_config fields - Makes RunTrigger consistent with RunNodeWithInputs (uses TaskNode) and SimulateTask (uses TaskTrigger) - Update RPC handler and engine implementation to extract config from TaskTrigger using ExtractTriggerConfigData - Update event_trigger_test.go to construct proper TaskTrigger objects - Improve type safety and maintainability by using structured types instead of raw maps * fix: add more testing around the ai summary function * Refactor ContractWrite data priority system with event handling Implements correct priority system for ContractWrite response data: 1. PRIORITY 1: Function return values from ABI outputs 2. PRIORITY 2: Event data (when found, overrides return values) 3. PRIORITY 3: Boolean true fallback (no return value and no events) Changes: - Extracted priority logic into helper functions for clarity: - addReturnValuesToDecodedData() - handles PRIORITY 1 - handleEventDataForMethod() - handles PRIORITY 2 & 3 - Simplified main execution flow by replacing inline logic with function calls - When events are found in transaction receipt, they now override return values (provides more useful data than simple booleans like approve() returning true) - When no events found but return value exists, keeps the return value - When neither events nor return value exist, uses boolean true as fallback Testing: - Added comprehensive test suite: vm_runner_contract_write_data_priority_test.go - 4 test cases covering all priority levels: * Priority2_EventData_OverridesReturnValue ✅ * Prio * Prio * Prio * Prio * Prio * Prio * Prio * Prio * Prio urnNoEvents ✅ * Priority1_ComplexReturnValue_KeepsDespiteEvents ✅ All tests pass, confirming correct behavior for: - ERC20 approve() with Approval event → returns event fields - ERC20 decimals() without events → returns uint8 value - Functions with no- Functions with no- Functions with no- Functions withith complex returns → keeps return value despite events i- Functions with no- Functions with no- Functions with no- Functions withith val- Functions with no- Functions with no- Functions with no- Functions withith ean returns, providing more context about what happened in the transaction. * chore: remove emoji from Execution Step Log * fix: resolve template variables before validating contract address in ContractRead node * Update core/config/config.go Co-authored-by: Copilot <[email protected]> * Update core/taskengine/summarizer_ai.go Co-authored-by: Copilot <[email protected]> * Improve core/taskengine/summarizer_ai.go * refactor: consolidate task status storage key logic into shared package - Create storage/schema package with centralized TaskStatusToStorageKey logic - Remove duplicate code from core/apqueue/cleanup.go - Update core/taskengine to use shared storage/schema package - Improve comment clarity for branch target gating in vm.go scheduler Fixes code duplication identified by Copilot review * fix: address Copilot code review suggestions - Use exact match for 'undefined' check instead of substring match to avoid false positives - Compile regex pattern once at package level for better performance - Remove unused methodName and contractAddress parameters from extractTransactionValue * Export subject and body in restApi’s response data * Integration tests use salt=2 → smart wallet * fix: add styled HTML email support for AI summary notifications * test: replace GetTestConfig() with direct helper calls in tests Replace all uses of testutil.GetTestConfig() that directly access fields with calls to dedicated helper functions (GetTestTenderlyAccount, GetTestTenderlyProject, GetTestTenderlyAccessKey, GetAggregatorConfig). This modernizes test config access and ensures tests use the proper helper functions that panic with clear error messages if config is missing. Files updated: - core/taskengine/tenderly_client_test.go - core/taskengine/vm_runner_contract_write_data_priority_test.go - core/taskengine/vm_runner_contract_write_simulation_test.go * Change the github actions rpc endpoint value to CHAIN_ENDPOINT * fix summarizer tests: Added nil check for error before accessing err.Error() * Claned up deprecated docs * Relax the AI output verification from summarizer tests --------- Co-authored-by: Wei Lin <[email protected]> Co-authored-by: Will Zimmerman <[email protected]> Co-authored-by: Copilot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements and refactorings across the codebase, focusing on API modernization, logging simplification, improved orphaned job cleanup, and enhanced configuration handling. The most significant changes include updating the
RunNodeWithInputsReqAPI to use a fully typedTaskNodestructure, simplifying logging and request handling in the aggregator, making the orphaned job cleanup more robust, and adding support for optional AI notification summarization in configuration.API & Protobuf Modernization:
RunNodeWithInputsReqprotobuf message to require a completeTaskNodestructure instead of partial config fields, improving type safety, validation, and consistency with other APIs. Migration examples and a detailed guide were added inMIGRATION_RUN_NODE_WITH_INPUTS.md.RunTriggerReqand related test usage to use a fullTaskTriggerstructure, aligning with the new API pattern. [1] [2]Aggregator Logging & Initialization:
RunNodeWithInputsandRunTriggermethods to use the new typed node/trigger structures and removed unnecessary debug helpers. [1] [2] [3]Job Cleanup Robustness:
core/apqueue/cleanup.goto check for tasks in all possible statuses (active, completed, failed, canceled, executing) before deciding a job is orphaned, and improved logging for cleanup actions. [1] [2]Configuration Enhancements:
NotificationsSummaryConfigstruct and associated YAML config options for AI-based notification summarization, with support for provider, model, token limits, temperature, and budget. [1] [2] [3] [4]Developer Experience:
These changes collectively improve type safety, maintainability, and developer experience, while also enabling new features like AI summarization and more robust job management.