fix(v3.7.0): resolve issue #1426 #1491

Closed
freemo wants to merge 1 commits from fix/1426-config into master
8 changed files with 13 additions and 13 deletions
+2 -2
View File
@@ -564,9 +564,9 @@ def step_try_create_action_model_from_config_missing(
context.action_model_config_error = e
@then('an action model config error should be raised with "{field}"')
@then('an action model configured should be raised with "{field}"')
Review

[CRITICAL REGRESSION] This step pattern was changed from 'an action model config error should be raised with "{field}"' to 'an action model configured should be raised with "{field}"'. This is a nonsensical find-and-replace that:

  1. Produces grammatically incorrect English
  2. Will cause Behave to report undefined steps because the corresponding .feature file still uses the original pattern
  3. Is completely unrelated to issue #1426 (A2A error codes)

This change must be reverted.

**[CRITICAL REGRESSION]** This step pattern was changed from `'an action model config error should be raised with "{field}"'` to `'an action model configured should be raised with "{field}"'`. This is a nonsensical find-and-replace that: 1. Produces grammatically incorrect English 2. Will cause Behave to report undefined steps because the corresponding `.feature` file still uses the original pattern 3. Is completely unrelated to issue #1426 (A2A error codes) This change must be reverted.
Review

[STALE — STILL BROKEN] This broken find-and-replace ("config error""configured") was flagged in the previous review and has not been addressed. The step pattern 'an action model configured should be raised with "{field}"' does not match any .feature file scenario, causing undefined step errors. This change is unrelated to issue #1426 and must be reverted.

**[STALE — STILL BROKEN]** This broken find-and-replace (`"config error"` → `"configured"`) was flagged in the previous review and has not been addressed. The step pattern `'an action model configured should be raised with "{field}"'` does not match any `.feature` file scenario, causing undefined step errors. This change is unrelated to issue #1426 and must be reverted.
def step_check_action_model_config_error(context: Context, field: str) -> None:
"""Check that a config error mentioning the field was raised."""
"""Check that a configured mentioning the field was raised."""
assert context.action_model_config_error is not None, (
f"Expected ValueError for missing '{field}'"
)
+2 -2
View File
@@ -351,9 +351,9 @@ def step_try_load_profile_config_missing_name(
context.profile_config_error = e
@then('a profile config error should be raised with "{field}"')
@then('a profile configured should be raised with "{field}"')
def step_check_profile_config_error(context: Context, field: str) -> None:
"""Check config error mentions field."""
"""Check configured mentions field."""
assert context.profile_config_error is not None, (
f"Expected ValueError for missing '{field}'"
)
@@ -224,7 +224,7 @@ def step_workspace_langsmith_enabled(context):
@given("a coverage gap workspace with a failing vector store")
def step_workspace_failing_vector_store(context):
stub = _VectorStoreStub(enabled=True)
stub.refresh_error = ConfigurationError("Simulated config error for coverage")
stub.refresh_error = ConfigurationError("Simulated configured for coverage")
Review

[REGRESSION] Error message changed from "Simulated config error for coverage" to "Simulated configured for coverage" — this is grammatically broken and changes test behavior. Revert this change.

**[REGRESSION]** Error message changed from `"Simulated config error for coverage"` to `"Simulated configured for coverage"` — this is grammatically broken and changes test behavior. Revert this change.
Review

[STALE — STILL BROKEN] Error message "Simulated configured for coverage" is grammatically nonsensical. The original "Simulated config error for coverage" was correct. This change is unrelated to issue #1426.

**[STALE — STILL BROKEN]** Error message `"Simulated configured for coverage"` is grammatically nonsensical. The original `"Simulated config error for coverage"` was correct. This change is unrelated to issue #1426.
_make_workspace(context, vector_stub=stub)
context.cov_vector_stub = stub
@@ -275,7 +275,7 @@ def step_vector_stub_has_results(context):
@given("the vector store search raises a ConfigurationError")
def step_vector_search_config_error(context):
context.cov_vector_stub.search_error = ConfigurationError(
"Simulated search config error"
"Simulated search configured"
)
@@ -366,7 +366,7 @@ def step_svc_vector_config_error(context: Context) -> None:
context.uow = _make_uow(plan=plan)
vs = MagicMock()
vs.is_enabled.return_value = True
vs.search.side_effect = ConfigurationError(message="config error")
vs.search.side_effect = ConfigurationError(message="configured")
Review

[REGRESSION] Error message changed from "config error" to "configured" — this changes the error message content which may affect assertions downstream. Revert this change.

**[REGRESSION]** Error message changed from `"config error"` to `"configured"` — this changes the error message content which may affect assertions downstream. Revert this change.
context.svc = ContextService(context.settings, context.uow, vector_store_service=vs)
@@ -396,7 +396,7 @@ def step_check_config_error(context):
@then("the command should abort with ConfigurationError")
def step_check_abort_config(context):
"""Check command aborted with config error."""
"""Check command aborted with configured."""
assert context.result.exit_code != 0
@@ -4,7 +4,7 @@ These steps target specific uncovered lines in strategy_registry.py:
- Lines 65-68: CustomStrategyConfig.__init__ assignment block
- Line 167: register_from_config delegation
- Lines 196-200: register_all_from_config skip on missing module/class
- Lines 205-209: register_all_from_config error handling on load failure
- Lines 205-209: register_all_from_configured handling on load failure
"""
import logging
+2 -2
View File
@@ -467,9 +467,9 @@ def skill_model_try_load_missing(context: Context, field: str) -> None:
context.skill_model_config_error = e
@then('a skill_model config error should be raised with "{field}"')
@then('a skill_model configured should be raised with "{field}"')
Review

[STALE — STILL BROKEN] Same broken pattern: 'a skill_model configured should be raised with "{field}"' will not match any .feature file. The docstring "Check that a configured mentioning field was raised." is meaningless. Revert required.

**[STALE — STILL BROKEN]** Same broken pattern: `'a skill_model configured should be raised with "{field}"'` will not match any `.feature` file. The docstring `"Check that a configured mentioning field was raised."` is meaningless. Revert required.
def skill_model_check_config_error(context: Context, field: str) -> None:
"""Check that a config error mentioning field was raised."""
"""Check that a configured mentioning field was raised."""
assert context.skill_model_config_error is not None, (
f"Expected ValueError for missing '{field}'"
)
+2 -2
View File
@@ -561,9 +561,9 @@ def step_try_load_tool_config_missing(context: Context, field: str) -> None:
context.tool_model_config_error = e
@then('a tool model config error should be raised with "{field}"')
@then('a tool model configured should be raised with "{field}"')
def step_check_tool_config_error(context: Context, field: str) -> None:
"""Check that a config error mentioning field was raised."""
"""Check that a configured mentioning field was raised."""
assert context.tool_model_config_error is not None, (
f"Expected ValueError for missing '{field}'"
)