1 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
7f29874156
|
fix: address code review findings from PR #1175 (issue #10267)
CI / push-validation (pull_request) Successful in 23s
CI / helm (pull_request) Successful in 37s
CI / build (pull_request) Successful in 3m48s
CI / lint (pull_request) Successful in 3m55s
CI / quality (pull_request) Successful in 4m18s
CI / typecheck (pull_request) Successful in 4m32s
CI / security (pull_request) Successful in 4m35s
CI / integration_tests (pull_request) Successful in 6m47s
CI / e2e_tests (pull_request) Successful in 6m56s
CI / unit_tests (pull_request) Successful in 7m23s
CI / docker (pull_request) Successful in 1m30s
CI / coverage (pull_request) Successful in 14m31s
CI / benchmark-regression (push) Waiting to run
CI / benchmark-publish (push) Waiting to run
CI / status-check (pull_request) Successful in 3s
CI / helm (push) Successful in 27s
CI / push-validation (push) Successful in 26s
CI / build (push) Successful in 3m43s
CI / lint (push) Successful in 3m54s
CI / quality (push) Successful in 4m16s
CI / typecheck (push) Successful in 4m28s
CI / security (push) Successful in 4m43s
CI / integration_tests (push) Successful in 7m3s
CI / e2e_tests (push) Successful in 7m13s
CI / unit_tests (push) Successful in 7m25s
CI / docker (push) Successful in 1m36s
CI / coverage (push) Successful in 13m40s
CI / status-check (push) Successful in 3s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Failing after 14m31s
Implement comprehensive fixes for all P2:should-fix and P3:nit findings from the Strategy Actor code review: Update 5 step functions that were calling the private _execute_with_llm() method directly to instead use the new strategy_tree field from StrategizeResult: - step_execute_and_inspect_tree (line 619) - step_parse_self_dep (line 877) - step_parse_duplicate_step_numbers (line 968) - step_parse_non_sequential_steps (line 1075) - step_parse_non_sequential_steps_and_inspect (line 1755) - step_build_decisions_from_llm_tree (line 1299) - also added execute() call This eliminates the fragile double-execution pattern that produced divergent ULIDs and couples tests to private implementation details. Tests now use the public StrategizeResult.strategy_tree field. P2:should-fix Fixes: - Move json import to module level in plan_executor_coverage_steps.py - Remove redundant Exception clause in plan_executor.py exception handler - Add logging to silent config service fallback in plan.py - Improve structured content block handling in _extract_content() to properly extract text from LangChain MessageContent dicts - Remove duplicated _DEFAULT_ACTOR_NAME constant and import from canonical source (strategy_resolution.py) - Add strategy_tree field to StrategizeResult to expose tree for test inspection without coupling to private _execute_with_llm() method P3:nit Fixes: - Improved docstrings and code clarity Refs: #10267 |