Skip to content

Feat/m perf 9 engine model identity#3

Merged
ddvnguyen merged 3 commits into
hydra-forkfrom
feat/m-perf-9-engine-model-identity
Jun 19, 2026
Merged

Feat/m perf 9 engine model identity#3
ddvnguyen merged 3 commits into
hydra-forkfrom
feat/m-perf-9-engine-model-identity

Conversation

@ddvnguyen

@ddvnguyen ddvnguyen commented Jun 19, 2026

Copy link
Copy Markdown
Owner

Overview

Additional information

Requirements

ddvnguyen and others added 3 commits June 19, 2026 14:20
@ddvnguyen
feat: [M-Perf.9] engine model identity + cross-model KV safety
1c755a0 
M-Perf.9 (ggml-org#289) closes the loop on cross-model KV safety: a KV cache built
with model A is incompatible with model B, and the Coordinator must never
silently restore a cross-model cache. Three changes:

* llama_model_hash() — public API + impl that returns the SHA-256 of the
  GGUF file the model was loaded from. Computed once in
  llama_model_load_from_file after construction; stored on the
  llama_model struct as hash_hex. New self-contained SHA-256 helper in
  src/llama-hash.cpp (no OpenSSL/libcrypto).

* State META / GET now carry model_alias + model_hash + model_path so
  the Coordinator can record which model built each KV. STATE_META
  (0x32) and STATE_GET (0x30) responses populate these from
  impl->model_name / impl->params_base.model.path / llama_model_hash().

* Engine PREFILL (0x42) now accepts an optional 'model' key in the
  request JSON. The C++ side swaps to the requested alias when the
  preset registry knows it, falls back to the resident model with
  model_fallback: true in the response when the alias is unknown
  (or no preset is configured), and behaves as before when 'model' is
  absent. The engine uses the existing common_preset_context::load_from_ini
  (common/preset.cpp:315-365) — same INI format llama-server router mode
  already uses.

* Engine INFO (0x41) now advertises the new capabilities ('preset',
  'model_hash') and lists preset_aliases.

* Engine PIPELINE_ATTACH (0x46) is now defined as a real opcode
  (previously referenced in WIP that referenced 0x33-0x38 which collides
  with C# OpCode.GetManifest=0x33) and stubbed to return
  HYDRA_STATUS_NOT_IMPLEMENTED (new status 0x06) so the Coordinator can
  distinguish 'not yet built' (issue ggml-org#287) from a real error and fall
  back to solo mode. The dispatch in hydra_handle_connection routes
  0x46 to hydra_handle_pipeline_attach.

* server_task_result_hydra_state and server_task_result_hydra_engine
  gained the model_* fields; to_json() emits them.

Refs: ggml-org#289, advances ggml-org#287
Closes: M-Perf.9
@ddvnguyen @claude
fix: [M-Perf.9 PR#290 review] implement engine PREFILL model swap + p…
8b8198f 
…reset alias registry

Closes the 4 P0 review blockers from PR ggml-org#290.

P0.1 — Add the missing SERVER_TASK_TYPE_HYDRA_ENGINE_PIPELINE_ATTACH entry
        to the task enum. The PR's handler for 0x46 referenced an
        undefined symbol — the build would not have compiled.

P0.2 — Engine PREFILL now parses the optional 'model' key from the
        request body. When the alias is in the preset registry, the
        resident model is swapped via load_model() before the prefill
        runs; when the alias is unknown (or no preset is configured),
        the prefill runs on the resident model and the response carries
        model_fallback:true. The handler also re-derives the slot
        pointer after load_model since it allocates fresh slots.

P0.3 — Engine INFO now returns the actual preset_aliases list from
        the populated preset_alias_to_path map (was hardcoded empty).

P0.4 — Add preset_alias_to_path map to server_context_impl, populated
        at startup from --models-preset PATH (or directory of .ini
        files). Uses common_preset_context::load_from_ini and extracts
        LLAMA_ARG_MODEL from each preset section.

Also renumbered the dispatch cases from the old WIP
SERVER_TASK_TYPE_HYDRA_* names (0x33-0x38) to the new
SERVER_TASK_TYPE_HYDRA_ENGINE_* names (0x40-0x46). The old enum
entries are removed (the wire now exclusively uses 0x40-0x46 — the
0x33-0x38 range was a collision zone with the live C#
OpCode.GetManifest=0x33).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ddvnguyen @claude
fix: use model_tgt (not the non-existent model) in the 3 model_hash l…
5ef3b33 
…ookup sites

The original PR ggml-org#290 used 'model' as the variable name in 3 places
(STATE_GET, STATE_META, and the post-PREFILL response site). The
actual variable in server_context_impl is 'model_tgt' (declared at
server-context.cpp:680). The build failed with 'model was not
declared in this scope' at all 3 sites — caught by a fresh
sm_120 cmake build of llama-server.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 19, 2026
Open
Merged
@ddvnguyen

ddvnguyen commented Jun 19, 2026

Copy link
Copy Markdown
Owner Author

Code review — #3 (feat/m-perf-9-engine-model-identity → hydra-fork)

Cross-linked from ddvnguyen/hydra_vortex#293 (the parent-repo integration tracking issue; closes ddvnguyen/hydra_vortex#287 on the Hydra side once the parent submodule bumps to the merge commit).

Verdict

Ready to merge, but recommend closing this PR in favour of #2's merge because #2 is a strict superset of #3.

Why close instead of merge

#2 feat/m287-combined-expert-mode → hydra-fork is branched from 5ef3b33fc1 (this PR's head) and adds 2 extra commits:

Commit Title
8baa00614f wip: port hydra_expert_mode placement-hook scaffolding (partial)
c357ad25b8 feat: COMBINED expert-split two-engine mode (issue ggml-org#287/ggml-org#260)

#2 contains every commit in this PR plus the COMBINED-half of ggml-org#287 (one-binary role flag, llama_hydra_load_combined_experts, qwen35moe SOLO/COMBINED tensor routing at graph-build time, embedded ggml-RPC worker). Merging #2 makes #3 redundant.

The natural sequence is: merge #2 first (it fast-forwards hydra-fork past #3's head), then close this PR as superseded ("closed via #2's merge").

What was reviewed (in this PR)

  • include/llama.h:614-622 — llama_model_hash() public API: returns cached hash_hex, empty string on failure, stable for model lifetime.
  • src/llama-hash.cpp/h — self-contained SHA-256, 64 KB chunked reads, no OpenSSL dep. Adapted from upstream examples/gguf-hash/gguf-hash.cpp.
  • src/llama-model.{h,cpp} — hash_hex field on llama_model, populated by llama_model_load_from_file_impl after construction.
  • src/CMakeLists.txt:22-23 — llama-hash.cpp/h added to the llama library.
  • src/llama.cpp:405-415 — the actual llama_hash_file_sha256(path_model.c_str(), ...) call after the model is fully loaded.
  • tools/server/server-rpc.h — HYDRA_OP_PIPELINE_ATTACH = 0x46 + HYDRA_STATUS_NOT_IMPLEMENTED = 0x06 + HYDRA_STATUS_BAD_REQUEST = 0x05.
  • tools/server/server-task.h — opcode renumber 0x33-0x38 → 0x40-0x45 with the 0x33-0x3F reserved-collision-zone comment at lines 35-39, plus model_alias / model_hash / model_path fields on server_task_result_hydra_state and server_task_result_hydra_engine + the model_fallback flag on the engine side.
  • tools/server/server-context.cpp — preset alias registry (loading --models-preset INI), model identity threading through STATE_META, STATE_GET, ENGINE_PREFILL responses, the model_tgt fix in commit 5ef3b33fc1 (3 sites that referenced the non-existent model field).

What's good

  • llama_model_hash() API is clean — returns const char * to the cached buffer; no allocation; the doc comment in include/llama.h:614-622 is honest about the empty-on-failure behaviour and the load-time-only computation cost (~1-2s for 25 GB on NVMe, paid once).
  • Hash is computed after the model is fully constructed (src/llama.cpp:405-415), not at GGUF metadata read time. This matters: if the loader hits an error and returns nullptr, no hash is computed. Clean failure mode.
  • server_task_result_hydra_state and server_task_result_hydra_engine are extended in parallel — model identity is reported in both response types, so the Coordinator gets it on every KV-touching RPC.
  • Opcode renumber is documented inline in server-task.h:35-39: "The original 0x33-0x38 (CONFIGURE..SWAP_QUANT) WIP collided with the C# GetManifest opcode and was re-numbered to 0x40-0x46 in M-Perf.9 (Docker “--all-in-one” fails with ModuleNotFoundError: No module named ‘tqdm’ ggml-org/llama.cpp#289)." Future agents reading the file will understand why.
  • 0x33-0x3F is marked as a reserved collision zone — explicit defensive marker, with a comment so future engine opcodes don't accidentally re-collide.
  • PIPELINE_ATTACH (0x46) returns HYDRA_STATUS_NOT_IMPLEMENTED cleanly. The Coordinator already handles that status by falling back to solo. Good defensive design.
  • model_tgt fix in 5ef3b33fc1 is a small but important bug fix — the original implementation referenced a non-existent model field on the slot; this commit corrects to model_tgt at all 3 model_hash lookup sites. Worth a quick git blame check to make sure the third-party code reviews catch this pattern in the future.

Cross-repo coordination gaps (carry-overs from the parent issue)

Same three as in the #2 review:

  1. C# EngineInfo doesn't read the new C++ model-identity fields. The Coordinator relies on WorkerCapabilities (config-side) for KvModelAlias/Hash/Path, but the META/GET/PREFILL response model fields are not parsed. They flow through StateHandler as separate fields. Confirm the round-trip end-to-end.
  2. C# StatusCode enum doesn't list 0x06 (NotImplemented). Works via raw byte read.
  3. specs/rpc-protocol.md is out of date for 0x40-0x46 + 0x33-0x3F zone.

What's NOT in this PR (also in #2, but neither has it)

Recommended action

Close this PR as superseded by #2. The merge of #2 contains every commit here plus the COMBINED work. After #2's merge, the parent submodule bump is a single Hydra PR that lands both ggml-org#290 (model identity, covered by this PR) and ggml-org#286 (multi-engine routing, covered by #2) in one go.

Tracked at ddvnguyen/hydra_vortex#293.

@ddvnguyen ddvnguyen merged commit 0e667e1 into hydra-fork Jun 19, 2026
2 of 23 checks passed
@ddvnguyen ddvnguyen mentioned this pull request Jun 19, 2026
Merged
4 tasks
This was referenced Jul 5, 2026
Merged
Open
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

examples server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ddvnguyen