Repository navigation
model: add Beam (Reflection) - #281
Open
danielhanchen wants to merge 23 commits into
Open
danielhanchen wants to merge 23 commits into
danielhanchen wants to merge 23 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfb9f2cecb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Harmony templates whose tool calls carry <|constrain|>xml bodies with rfl:parameter arguments were detected as gpt-oss, whose parser expects JSON arguments behind to=functions.NAME. Every tool call failed to parse and the lazy grammar pushed the model towards the wrong call format. Add a dedicated handler, detected ahead of the gpt-oss check: - reasoning and final channels parse as before - one or more calls (<|end|> between parallel calls), arguments typed from the tool schema, a missing closing tag tolerated - lazy grammar triggered on the call header, eager for tool_choice=required - end-of-generation tokens are preserved so the parser sees where the last call ends; output cut off mid-call keeps the call open Tests cover the reference parser corpus plus streaming, typed arguments, parallel calls and structured output.
When rope_parameters is absent, take the sliding-attention rope params from the values TextModel already normalized instead of replacing them with empty dicts, so rope.freq_base is still written.
Without them the generic defaults silently skip the routed scaling and the top-k renormalization; the converter always writes both keys.
The beam graph always builds the iSWA cache, which asserts is_swa_any() at context creation; fail at load time and in the converter instead.
The attn temp input asserts a nonzero scale on the first decode, so a file that sets it to zero now fails to load instead of aborting later.
danielhanchen
added a commit
that referenced
this pull request
Oct 16, 2026
ggml-org#30670 is the upstream PR for the same change (one squashed commit, identical changed lines).
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds support for Reflection's
Beam-1.0-501B-A23B(HFBeamForCausalLM): converter, model graph and a chat parser for its tool-call format.Architecture
build_moe_ffn; correction bias used for selection only, routed scale and normalised weights required in the GGUFllm_graph_input_attn_temp, off by defaultsrc/models/beam.cpp<|channel|>+<|constrain|>xml+rfl:parameter)harmony-xmlchat parser, detected only when all three markers are presentNo new ggml op.
Correctness
test-llama-archs -a beam: OK on CPU, CUDA and Meta.test-chat,test-jinjapass.apply_chat_template; multi-turn tool-call sessions (parallel calls, typed and nested arguments, long logs) pass end to end throughllama-server.Effect on other architectures
llama-graph.cpp: the power-law temperature is a separate branch; existing users take the old path.llm_graph_input_attn_tempgains a shape-onlycan_reuse, sinceset_inputrecomputes the scale every ubatch.llama-vocab.cpp: Beam's</s>stays a normal token (it is plain text in its tokenizer), using the existing gemma4/plamo exception keyed on a Beam-only token. Token ids, EOG sets and attributes are unchanged on 19 upstream test vocabs and 224 local GGUFs.common/jinja: thelistfilter accepts objects (returns the keys).common/chat.cpp: the new parser is only selected for templates containing all three markers.AI Usage
Used Claude and Local Models for testing, iteration and code design - manual verification of model / PR usage