chore: trim hook test suite to core coverage and condense hooks docs
Per review: keep fall-through, claim, first-valid-wins, skipped-result warning, malformed-result isolation, sanitization, and the end-to-end synthetic-plugin test; drop the auxiliary variants. Compress the hooks.md section to prose with a minimal return example.
This commit is contained in:
Teknium
parent
c7c687aa4b
commit
09993ea41a
|
|
@ -79,45 +79,6 @@ def test_plugin_classification_wins(monkeypatch):
|
|||
assert result.provider == "acmecloud"
|
||||
assert result.status_code is None
|
||||
|
||||
|
||||
def test_plugin_overrides_builtin_classification(monkeypatch):
|
||||
# A 429 classifies as rate_limit built-in; a plugin can reclassify it.
|
||||
monkeypatch.setattr(
|
||||
plugins_mod, "invoke_hook",
|
||||
lambda name, **kw: [{"reason": "overloaded"}],
|
||||
)
|
||||
|
||||
result = classify_api_error(
|
||||
_FakeAPIError("too many requests", status_code=429),
|
||||
provider="zai",
|
||||
)
|
||||
assert result.reason == FailoverReason.overloaded
|
||||
|
||||
|
||||
def test_enum_reason_accepted(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
plugins_mod, "invoke_hook",
|
||||
lambda name, **kw: [{"reason": FailoverReason.billing}],
|
||||
)
|
||||
|
||||
result = _classify_unclaimed_error()
|
||||
assert result.reason == FailoverReason.billing
|
||||
|
||||
|
||||
def test_reason_only_dict_uses_dataclass_defaults(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
plugins_mod, "invoke_hook",
|
||||
lambda name, **kw: [{"reason": "server_error"}],
|
||||
)
|
||||
|
||||
result = _classify_unclaimed_error()
|
||||
assert result.reason == FailoverReason.server_error
|
||||
assert result.retryable is True
|
||||
assert result.should_compress is False
|
||||
assert result.should_rotate_credential is False
|
||||
assert result.should_fallback is False
|
||||
|
||||
|
||||
# ── Invalid returns are ignored, first valid wins ───────────────────────
|
||||
|
||||
|
||||
|
|
@ -130,17 +91,6 @@ def test_invalid_reason_falls_through_to_builtin(monkeypatch):
|
|||
result = _classify_unclaimed_error()
|
||||
assert result.reason == FailoverReason.unknown
|
||||
|
||||
|
||||
def test_non_dict_results_ignored(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
plugins_mod, "invoke_hook",
|
||||
lambda name, **kw: ["model_not_found", 123, ["nope"], None],
|
||||
)
|
||||
|
||||
result = _classify_unclaimed_error()
|
||||
assert result.reason == FailoverReason.unknown
|
||||
|
||||
|
||||
def test_first_valid_result_wins(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
plugins_mod, "invoke_hook",
|
||||
|
|
@ -200,30 +150,6 @@ def test_helper_exception_never_breaks_classification(monkeypatch):
|
|||
|
||||
# ── Hook kwargs contract ────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_hook_receives_parsed_error_context(monkeypatch):
|
||||
seen = {}
|
||||
|
||||
def _capture(name, **kw):
|
||||
seen.update(kw, hook_name=name)
|
||||
return []
|
||||
|
||||
monkeypatch.setattr(plugins_mod, "invoke_hook", _capture)
|
||||
|
||||
_classify_unclaimed_error(approx_tokens=1234, num_messages=7)
|
||||
|
||||
assert seen["hook_name"] == "transform_api_error_classification"
|
||||
assert seen["provider"] == "acmecloud"
|
||||
assert seen["model"] == "acme/large-1"
|
||||
assert seen["status_code"] is None
|
||||
assert seen["error_type"] == "_FakeAPIError"
|
||||
assert "flux capacitor drift" in seen["error_message"]
|
||||
assert seen["approx_tokens"] == 1234
|
||||
assert seen["num_messages"] == 7
|
||||
assert isinstance(seen["error_body"], dict)
|
||||
assert isinstance(seen["error"], _FakeAPIError)
|
||||
|
||||
|
||||
def test_message_override_and_error_context_sanitized(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
plugins_mod, "invoke_hook",
|
||||
|
|
@ -264,23 +190,6 @@ def _load_synthetic_plugin(tmp_path):
|
|||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
def test_synthetic_plugin_self_scopes(tmp_path):
|
||||
demo = _load_synthetic_plugin(tmp_path)
|
||||
# Different provider: pass.
|
||||
assert demo.classify(
|
||||
provider="anthropic", error_message=_UNCLAIMED_MESSAGE,
|
||||
) is None
|
||||
# Different message: pass.
|
||||
assert demo.classify(
|
||||
provider="acmecloud", error_message="model not found",
|
||||
) is None
|
||||
# Provider and unambiguous phrase: claim.
|
||||
assert demo.classify(
|
||||
provider="acmecloud", error_message=_UNCLAIMED_MESSAGE,
|
||||
) is not None
|
||||
|
||||
|
||||
def test_synthetic_plugin_end_to_end(tmp_path, monkeypatch):
|
||||
"""register() + real invoke_hook + classify_api_error, no mocks."""
|
||||
demo = _load_synthetic_plugin(tmp_path)
|
||||
|
|
|
|||
|
|
@ -849,51 +849,18 @@ For standing guidance that should shape the built-in missing-evidence nudge, use
|
|||
|
||||
### `transform_api_error_classification`
|
||||
|
||||
Fires **once per failed API call**, at the top of `agent/error_classifier.classify_api_error()` — BEFORE the built-in classification pipeline. Cold path: it never fires on a successful call. Provider plugins use it to own their provider's error quirks (a vendor-specific 404 that should fast-fallback, a misleading status code) without core patches.
|
||||
Fires once per failed API call, at the top of `agent/error_classifier.classify_api_error()`, before the built-in pipeline. Provider plugins use it to own their provider's error quirks without core patches. It is behavior-changing (transform family): the returned classification drives retry, compression, credential rotation, and fallback routing.
|
||||
|
||||
This hook is **behavior-changing** (transform family): the returned classification drives retry, compression, credential-rotation, and fallback routing for the failed call.
|
||||
|
||||
**Callback signature:**
|
||||
Callbacks receive the parsed error context as kwargs — `provider` (self-scope on this), `model`, `status_code`, `error_type`, `error_code`, `error_message`, `error_body`, `error`, `approx_tokens`, `context_length`, `num_messages`. Return `None` to decline, or a dict to claim the error:
|
||||
|
||||
```python
|
||||
def my_callback(provider: str, model: str, status_code, error_type: str,
|
||||
error_code, error_message: str, error_body, error,
|
||||
approx_tokens, context_length, num_messages, **kwargs):
|
||||
return {"reason": "model_not_found", # required: a FailoverReason name
|
||||
"retryable": False, "should_fallback": True} # optional recovery-hint overrides
|
||||
```
|
||||
|
||||
| Parameter | Type | Description |
|
||||
|-----------|------|-------------|
|
||||
| `provider` | `str` | The provider whose call failed — **self-scope on this** |
|
||||
| `model` | `str` | The model identifier for the failed call |
|
||||
| `status_code` | `int \| None` | HTTP status, when the error carried one |
|
||||
| `error_type` | `str` | The exception class name |
|
||||
| `error_code` | `str \| None` | Provider error code, when present |
|
||||
| `error_message` | `str` | Lower-cased error message text |
|
||||
| `error_body` | `dict` | Parsed provider error body, when present |
|
||||
| `error` | `Exception` | The original exception object |
|
||||
| `approx_tokens` | `int \| None` | Approximate prompt tokens of the failed request |
|
||||
| `context_length` | `int \| None` | Model context length, when known |
|
||||
| `num_messages` | `int \| None` | Message count of the failed request |
|
||||
Dispatch is run-all-then-pick-first: every callback runs, failures are isolated, and the first valid result in registration order wins (valid-but-losing results log a runtime warning). Invalid dicts and unknown reasons are skipped, so a broken plugin can never break classification.
|
||||
|
||||
**Return value — claim the error:**
|
||||
|
||||
```python
|
||||
return {
|
||||
"reason": "model_not_found", # required: a FailoverReason name
|
||||
"retryable": False, # optional recovery-hint overrides
|
||||
"should_fallback": True,
|
||||
"should_compress": False,
|
||||
"should_rotate_credential": False,
|
||||
"message": "...", # optional user-facing guidance
|
||||
"error_context": {...}, # optional extra context
|
||||
}
|
||||
```
|
||||
|
||||
Return `None` (or nothing) to decline and defer to the built-in pipeline. Dispatch is **run-all-then-pick-first**: every registered callback runs on each failed call (an earlier answer never stops later callbacks), each callback's failure is isolated, and the first valid result **in registration order** wins — if two plugins can both answer, the first-registered one is the tie-break, and every valid-but-losing result is reported with a runtime warning so a shadowed provider plugin is visible in logs. Invalid dicts and unknown reasons are skipped, so a broken plugin can never break error classification.
|
||||
|
||||
**Privacy:** `error_message` and `error_body` may carry an unredacted provider error dump. Do not log or forward them from a callback without redaction.
|
||||
|
||||
**Python plugins only.** Shell hooks cannot register for this event: the shell response parser has no channel for the classification directive, so a shell registration is refused at config parse with a warning rather than being silently ignored.
|
||||
**Privacy:** `error_message` and `error_body` may carry unredacted provider data. **Python plugins only** — shell registrations are refused at config parse with a warning.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue