Skip to content

Two SOLID gaps: chat-format registries must be edited in four places, and LoRA/multimodal are mandatory on every backend #362

Description

@leehack

Found during a code-health audit of main, confirmed by a follow-up verification pass. No fix applied. Two structural findings that no existing issue covers — #354 is scoped to LlamaCppService and library discovery, #349 to two specific parity items.


1. Open/closed: adding a chat-template family means editing four parallel registries

Place What must be added
lib/src/core/template/chat_format.dart:7 a value in enum ChatFormat — currently 38
lib/src/core/template/chat_format.dart:133 a branch in detectChatFormat, an ordered chain of 34 return ChatFormat.…
lib/src/core/template/chat_template_engine.dart:502 a label in _createHandler's 38-case switch
lib/src/core/template/handlers/ a handler file — currently 33

Nothing ties the four together. Adding an enum value without a _createHandler case is caught by exhaustiveness; adding a handler file without a detectChatFormat branch is not, and the family silently never activates.

The detectChatFormat chain is order-dependent in a way that is load-bearing but only documented in comments — e.g. :148, "Check before Command R7B because both use…". That ordering constraint is invisible to anyone appending a branch at the end.

Suggested direction: give each handler a static descriptor (format id, detection predicate, priority) and build the registry from the handler list, so a new family is one file. If the enum must stay for API reasons, derive the switch from the same table and add a test asserting every ChatFormat value has exactly one handler and one detector.


2. Interface segregation: seven members every backend must implement, most of which throw

abstract class LlamaBackend (lib/src/backends/backend.dart:17) makes these mandatory:

  • LoRA trio — setLoraAdapter (:75), removeLoraAdapter (:78), clearLoraAdapters (:81)
  • Multimodal quartet — multimodalContextCreate (:102), multimodalContextFree (:105), supportsVision (:108), supportsAudio (:111)

What the non-llama.cpp backends actually do with them:

  • litert_lm_backend_web.dart:318, :326, :334, :371, :378, :385, :392 — all seven throw UnsupportedError(
  • webgpu_backend.dart:2524, :2529throw UnsupportedError(_runtimeLoraUnsupportedMessage)

This is the one place the codebase departs from its own established pattern. Fourteen other capabilities are modelled as optional interfaces the engine probes with is: BackendAvailability (:131), BackendGrammarConstraintsSupport (:141), BackendPromptSpeechToTextSupport (:185), BackendTextToSpeech (:349), the embeddings trio (:492, :510, :516), state persistence (:550, :578), and others.

So the fix is not a new idea — it is applying the existing capability-interface pattern to the two groups that were left behind: BackendLoraSupport and BackendMultimodalSupport, probed the same way.

Caveat: LlamaBackend is public API (lib/llamadart.dart), so moving members off the base class is breaking for any external implementer. Worth bundling with the API narrowing in #355 so the break happens once.

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

    enhancementNew feature or requestpriority:P3Watch or strategic work blocked by upstream/runtime/design dependencies

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions