Skip to content

Consolidate mtp_*/ngram_* speculative APIs onto the generic speculative_* family (ABI-breaking) #51

Description

@leehack

Summary

Three exported API families — llama_dart_speculative_*, llama_dart_mtp_*, and llama_dart_ngram_* — wrap the same upstream common_speculative with the same begin / process_batch / draft / accept shape. The generic family already subsumes the other two.

This removes exported symbols and requires a coordinated llamadart binding update before it can land.

Size

Roughly 490 lines of implementation across the three:

family approx. impl lines
llama_dart_speculative_* ~226
llama_dart_mtp_* ~173
llama_dart_ngram_* ~88

The generic family already covers both

MTP. llama_dart_mtp_init_impl (src/llama_dart_wrapper.cpp:1339) sets params.types = {COMMON_SPECULATIVE_TYPE_DRAFT_MTP}, ctx_type = LLAMA_CONTEXT_TYPE_MTP, n_seq_max = 1, n_rs_seq = 0, n_outputs_max = 1, embeddings = false, ctx_other = ctx_tgt.

llama_dart_speculative_init with type_names = "draft-mtp" reaches the same configuration (:1161-:1169), including falling back to the target model as the draft model when draft_model == nullptr and MTP is the only draft-context type.

ngram-simple. llama_dart_ngram_simple_init (:1530) sets types = {NGRAM_SIMPLE} plus ngram_simple.size_n / size_m. llama_dart_speculative_init with type_names = "ngram-simple" and ngram_size_n / ngram_size_m reaches the same state via llama_dart_apply_ngram_map_params. Only the defaults differ: ngram_simple_init hardcodes 12/48, the generic path inherits upstream defaults.

One capability that must not be lost

llama_dart_mtp_init_impl performs a common_context_can_seq_rm rollback-capability check on both contexts (src/llama_dart_wrapper.cpp:1375-:1388) that the generic path does not. Any consolidation must move that check into the generic path rather than dropping it.

Why it is worth doing

The copies have already drifted — the MTP path is missing the has_last_draft guard, uses a different draft clamp, and validates seq_id differently from the other two. That drift is tracked in #46. Consolidating removes the class of bug rather than fixing this instance of it.

Migration sketch

  1. Move the seq_rm capability check into llama_dart_speculative_init for MTP types.
  2. Confirm which of these symbols llamadart actually binds.
  3. Land the Dart-side change to route MTP and ngram through the generic API.
  4. Remove llama_dart_mtp_* and llama_dart_ngram_* here, drop the 13 corresponding entries from DEFAULT_REQUIRED_SYMBOLS in tools/validate_exports.py, and publish under a new native release tag.

Given the ABI impact, this may be worth deferring until a release where downstream is already being updated.

Related

Triage status: preparation only

This is a coordinated ABI migration, not routine native cleanup. Do not remove exported symbols until a llamadart change is ready and validated against the replacement artifact.

Consolidated findings

Required landing order

  1. Fix the current MTP safety divergence in Speculative wrapper families have drifted: mtp_* missing has_last_draft guard, inconsistent clamp and seq_id checks #46 without removing symbols.
  2. Add the missing MTP rollback-capability behavior to the generic path.
  3. Change llamadart bindings and service logic to use the generic speculative family for MTP and n-gram.
  4. Add old-runtime/new-Dart and new-runtime/old-Dart version-skew tests with actionable failure behavior.
  5. Remove the native exports and required-symbol entries only after downstream no longer requires them.
  6. Publish a new immutable native tag, regenerate Dart bindings, and run representative n-gram, MTP, DSpark/DFlash, state, and output-correctness validation.

Until those gates are satisfied, retain the compatibility symbols.

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:P3Useful cleanup or longer-term work

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions