Skip to content

bug(plugins): ErrorTransformer capability has no caller since the layered refactor #109

Description

@chrisnestrud

Summary

ErrorTransformer is declared, implemented, and tested, but nothing in the
request path calls it. deepseek_v4's TransformError therefore never runs,
and clients see the raw provider message for the thinking-history error.

Root cause

Bridge.errorResponse() in internal/bridge/bridge.go was the only production
caller:

if providerError, ok := anthropic.IsProviderError(err); ok {
    msg := providerError.Error()
    msg = bridge.hooks.TransformError(model, msg)

e436a33 ("restructure into layered architecture") deleted internal/bridge,
and error responses were re-implemented as inline openai.ErrorResponse
payloads in internal/service/server/adapter_dispatch.go. None of those sites
consult the transformer, and Registry.CorePluginHooks()
(internal/extension/plugin/registry.go:511) chains MutateCoreRequest,
FilterContent, RewriteMessages, InjectTools, RememberContent and
DisablePatchProxy but not ErrorTransformer.

Registry.TransformError (internal/extension/plugin/registry.go:356) is now
called only from internal/extension/plugin/registry_test.go. The
TransformError hook field is likewise never invoked in
internal/format/adapter.go:167 or internal/protocol/format/adapter.go:116.

Impact

With the deepseek_v4 extension enabled, a provider error whose message
contains content[].thinking and thinking mode — for example a tool-call
continuation whose thinking block was not replayed — reaches the client as raw
upstream text instead of the actionable message added by d375420:

"Missing required thinking blocks - ensure reasoning items are preserved in
conversation history for tool-call turns."

Observed on c9ae8a8. The message rewrite is the whole intended effect of the
capability, so today the commit that added it has no observable behavior.

Options

(a) Restore the call where provider errors become client payloads, gated on
anthropic.IsProviderError(err) exactly as the deleted errorResponse() did.
In the adapter path those are the eight sites in
internal/service/server/adapter_dispatch.go that format
upstream error: %v, chat upstream error: %v, google upstream error: %v,
upstream stream error: %v, chat stream error: %v, and
google stream error: %v.

(b) Delete the capability if it is not wanted: the interface in
internal/extension/plugin/capabilities.go:99, Registry.TransformError plus
the errorTransformers slice, DSPlugin.TransformError, the two hook fields
and their WithDefaults branches, and the registry test. c764e96 is already
removing dead code and packages, so this may be the intended direction.

Either way I am happy to send a PR — just say which. Related history:
d375420, f2aceed, e436a33, c764e96.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions