fix(llama): preserve full eos_token_id list through to the runtime

[roma5087]

Background

HF config.json/generation_config.json may declare eos_token_id as either
a single id or a list of stop-token ids (all Llama 3.1+ checkpoints, and
openbmb/MiniCPM5-2B, whose config has eos_token_id: [1, 130073]). The
families/llama build path only ever kept a single scalar
ModelConfig.eos_token_id, so a multi-id checkpoint was silently truncated to
its first id when writing runtime.json, and the C++ runtime loader
(families/llama/runtime/plugin.cpp) would reject a list value outright,
crashing bundle load with llama runtime.json has invalid 'eos_token_id'.
Truncating to the first id (an earlier version of this fix did that) is also
semantically wrong for the flagship case: Llama 3.1-Instruct's real per-turn
stop token (<|eot_id|>) is the last id in its list, not the first, so that
approach would silently break generation stopping instead of crashing.

No originating issue is linked — per CONTRIBUTING.md, an issue is expected
before investing in a large/cross-cutting/design-unclear change; this is
scoped to one family with a clear, already-validated approach, so no issue was
opened first.

Exit Criteria

• A checkpoint with a list-valued eos_token_id builds and its bundle loads
  and runs without crashing.
• All ids in the list are honored as valid stop tokens at generation time, not
  just the first.
• Ordinary single-scalar eos_token_id checkpoints continue to build and run
  unchanged (aside from the wire-format wrapping described below).
• Non-goals: this PR does not fix the parallel bos_token_id/pad_token_id
  missing-vs-zero bug (see Notes), and does not fix an unrelated,
  pre-existing TinyLlama generation bug discovered during regression testing
  (see Notes) — both are intentionally out of scope.

Implementation

Preserve the full eos_token_id list end-to-end instead of collapsing it to
one id:

• families/llama/config.py: add ModelConfig.eos_token_ids: tuple[int, ...]
  and a shared _as_id_list() helper that normalizes a missing value, a
  scalar, or a list into a tuple, checking for None explicitly (not
  truthiness), so a legitimate id of 0 survives.
• families/llama/model.py: runtime.json's "eos_token_id" is now always
  emitted as a JSON array, for both the config.json-derived path and the
  generation_config.json override path (the override path previously had no
  empty-list fallback at all; it now falls back to [-1]).
• families/llama/runtime/plugin.cpp: RuntimeConfig::eos_token_id (scalar)
  becomes eos_token_ids (vector<int32_t>), parsed as a JSON array with an
  explicit empty-vector guard, and wired into
  LlamaTextGenConfig::id_eos_ids — the pipeline's existing multi-stop-token
  mechanism (pipeline.cpp's normalize_eos_token_ids, sampler.cpp's
  is_stop_token/build_sampling_params), which already existed and was
  already unit-tested, but was never being fed more than one id.

Affected: families/llama only (build path + native runtime plugin). No
other family is touched.

Bundle/artifact format change: runtime.json's eos_token_id field
changes from sometimes-scalar to always-array. This is a breaking change to
the bundle wire format (see Notes for details) — no public API or ABI change.

Change categories

• Model or runtime behavior
• Public API
• ABI
• Bundle or artifact format
• Dependencies
• Documentation only
• CI or developer tooling

Validation

Commands and Results

pytest families/llama/tests/test_config.py            # 4 passed
pytest families/llama/tests/test_runtime_config.py     # 3 passed (new)
pytest families/llama/tests/ --ignore=families/llama/tests/test_e2e.py
                                                        # 16 passed (incl. real-TensorRT
                                                        #  test_builder_variants.py, test_rope_scaling.py)
pytest families/llama/tests/test_e2e.py --e2e-model=minicpm5-2b -s
                                                        # 1 passed, 7 skipped
                                                        # (native output compared against a real
                                                        #  transformers.generate() reference via the
                                                        #  family's normalized-edit-distance oracle)

Regression check (not an automated test, manual verification): rebuilt
tinyllama-1.1b (plain scalar eos_token_id: 2) with this fix, confirmed
runtime.json correctly contains "eos_token_id":[2] and the native runtime
loads and runs it without error (see Notes for a separate, pre-existing,
unrelated generation-quality issue found while doing this check).

Hardware, Environment, and Revisions

• Tested head: this branch, rebased onto upstream/main (fast-forward, zero
  divergent commits at time of testing)
• GPU: NVIDIA A40, 48GB VRAM, driver 580.159.03 (CUDA forward-compatibility
  mode, CUDA 13.3 userspace)
• Container: nvcr.io/nvidia/tensorrt:26.07-py3
• TensorRT 11.1.0.106, CUDA 13.3, torch==2.12.0+cu130 (matches
  Dockerfile.dev.x86-gpu, the community GPU CI environment)
• Model revisions: openbmb/MiniCPM5-2B (main), TinyLlama/TinyLlama-1.1B-Chat-v1.0
  (main) — both unpinned/latest at time of testing

Not Run / Remaining Gaps

• Not validated against a real Llama 3.1+ checkpoint directly (gated on HF,
  requires license acceptance). Validated the mechanism generically (full
  id-list preservation, confirmed against a public mirror's config.json/
  generation_config.json shape for Llama-3.1-8B-Instruct) and validated
  end-to-end against MiniCPM5-2B's real multi-id config instead.
• Other existing llama e2e manifests (falcon3-1b, minitron-4b-*,
  nemotron-nano-4b) were not re-run against real checkpoints in this
  session; their code path is unchanged in kind (just array-wrapped), so
  risk is believed low but this is unverified.
• Tensor-parallel (tensor_parallel_size > 1) and dual-profile/dynamic-KV
  engine layouts were not separately exercised for the multi-id case (only
  the default "split" layout was tested); eos_token_id parsing happens once
  in parse_runtime_config before any layout-specific logic, so this is
  believed layout-independent but not explicitly run.

Contributor Self-Review

• I have completed a self-review of this change.

Additionally, two independent rounds of focused review (correctness, test
coverage, contribution readiness) were performed; all findings from the
first round were addressed and re-verified in the second.

Notes For Future Readers

• Breaking change: runtime.json's eos_token_id field changes from
  sometimes-scalar to always-array. A bundle built with the pre-change llama
  plugin cannot be loaded by the post-change runtime, and vice versa (strict
  field parsing rejects the wrong shape). No migration path or version field
  is added: llama bundles are build-and-consume artifacts produced and
  immediately consumed by matching family code, not persisted or distributed
  release artifacts, and the repository has no existing convention for
  versioning a family section's internal field shapes. Anyone holding an
  existing llama bundle must rebuild it against this commit.
• Out of scope, tracked separately: bos_token_id (and pad_token_id)
  have the same missing-vs-zero bug the pre-fix eos_token_id had
  (d.get(field, -1) or -1 maps a legitimate 0 to -1). Confirmed against
  MiniCPM5-2B's real config (bos_token_id: 0). Left untouched here to keep
  this PR single-purpose.
• Found, NOT fixed here: while regression-testing the ordinary
  single-EOS case, found that the existing tinyllama-1.1b.json e2e case
  fails against unmodified upstream/main — the native engine's first decode
  step predicts EOS immediately for a simple prompt, producing empty output,
  versus a correct HF reference ("Capital: Paris"). Reproduced identically
  against the unmodified upstream config.py/model.py/plugin.cpp,
  confirming this pre-dates this branch and is unrelated to eos_token_id
  handling (this PR's diff never touches chat-template detection/rendering
  code). Suspected root cause: chat_templates.cpp's format-detection
  heuristic substring-matches on <|user|>/<|assistant|> tags and
  misclassifies TinyLlama's template as "Phi" format, injecting the wrong
  turn-separator token. Filing as a separate issue.
• Suggested review order: config.py → model.py → plugin.cpp → tests.

Risk level

• Low
• Medium
• High

Risk rationale: the bundle wire-format change is breaking but confined to an
internal build-and-consume artifact, not a persisted/distributed one; the
stop-token matching behavior change affects every llama-family model (not
just multi-EOS ones, since the field is now always array-wrapped), validated
broadly (16 unit/GPU tests + 1 real e2e correctness-oracle run) but not
against every existing llama manifest.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ab7b6041-7c0d-400a-9353-eaf54efbabb0

📥 Commits

Reviewing files that changed from the base of the PR and between 22472c0 and f1eacc2.

📒 Files selected for processing (1)

• apps/benchmark/performance/release.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

📝 Summary

Summary

Preserve multiple Hugging Face eos_token_id values across Llama bundle generation and runtime loading.

• Normalize missing, scalar, list, tuple, and zero-valued IDs.
• Write runtime.json with an EOS ID array.
• Pass all EOS IDs to native multi-stop-token sampling.
• Keep scalar EOS checkpoints functional.
• Add configuration and runtime tests.
• Add MiniCPM5-2B validation.
• Exclude MiniCPM5-2B from release performance profiles until a workload and receipt exist.

Architecture impact

• Family-owned files: Llama configuration, model loading, native runtime plugin, tests, and MiniCPM5-2B manifest.
• Shared surface: The Llama bundle runtime.json format changes from scalar to array EOS values.
• Dependency direction: Python configuration emits the array format. The native Llama plugin consumes it. Existing sampling logic receives the complete ID list.
• Affected consumers: Llama runtime loading and existing Llama bundles. Existing bundles require rebuilding because the internal format is incompatible.
• Unresolved blast-radius questions: Confirm all Llama bundle producers and runtime consumers support the array format. Confirm downstream tooling does not assume a scalar eos_token_id. Confirm release-performance coverage before removing the MiniCPM5-2B exclusion.
• Validation status: Community CI is reported as failing. The failure details and test results are not available here.

HUMAN REVIEW REQUIRED: Review the incompatible bundle format, native runtime boundary, downstream consumers, and reported community CI failure.

Walkthrough

Changes

The Llama configuration now normalizes scalar and list EOS values into tuples. The runtime passes all EOS IDs to TensorRT and rejects empty lists. Tests cover fallback and override behavior. A MiniCPM5-2B manifest was added, and its profile was excluded from release performance.

Llama EOS support

Layer / File(s)	Summary
EOS normalization and runtime propagation families/llama/config.py, families/llama/model.py, families/llama/runtime/plugin.cpp	Configuration preserves all normalized EOS IDs. Runtime configuration emits EOS IDs as lists and rejects empty lists.
EOS configuration validation families/llama/tests/test_config.py, families/llama/tests/test_runtime_config.py	Tests cover scalar, multiple, empty, missing, and generation-config EOS values.
MiniCPM5-2B declarations families/llama/tests/manifests/minicpm5-2b.json, apps/benchmark/performance/release.yaml	The manifest defines the MiniCPM5-2B test setup. Release performance excludes the profile without a matching workload or receipt.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ModelConfig
  participant LlamaModel
  participant RuntimePlugin
  participant TensorRT
  ModelConfig->>LlamaModel: normalized eos_token_ids
  LlamaModel->>RuntimePlugin: eos_token_id list
  RuntimePlugin->>TensorRT: id_eos_ids

Loading

Merge Risk: ⚪ Minimal · up to f1eac

No actionable correctness or integration risk remains; the Llama EOS-list changes are ready for normal checks.

🚥 Pre-merge checks | ✅ 7 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 5 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.
Shared Semantic Neutrality	⚠️ Warning	The pull request changes one non-exempt shared file: apps/benchmark/performance/release.yaml. It adds minicpm5-2b to the shared excluded_profiles policy and records model-specific HF qualificati…	Remove the direct minicpm5-2b exclusion from apps/benchmark/performance/release.yaml. Either add a matching release-performance workload and receipt, or implement any required exemption through an existing family-owned contract that kee…

✅ Passed checks (7 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving the full Llama eos_token_id list through runtime processing.
Description check	✅ Passed	The description covers the required background, exit criteria, implementation, change categories, validation results, environment, remaining gaps, self-review, future notes, and risk rationale. It als…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Family Ownership Boundary	✅ Passed	PASS. The changed implementation and tests remain inside families/llama: the new _as_id_list import in families/llama/model.py, the EOS changes in config.py and runtime/plugin.cpp, and the n…
Benchmark Validation Integrity	✅ Passed	No benchmark-integrity failure is introduced. The only performance-suite change adds minicpm5-2b to excluded_profiles; it adds no workload, receipt, timing scope, comparison, aggregation, or metri…
Shared Change Blast Radius	✅ Passed	The shared-surface changes are justified. The description identifies a generic Hugging Face EOS-shape need across multiple Llama checkpoints, the affected producer (families/llama/model.py) and cons…

Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 5 files. (1 skipped: 1 unsupported.)

Full details: Shared Semantic Neutrality

Explanation

The pull request changes one non-exempt shared file: apps/benchmark/performance/release.yaml. It adds minicpm5-2b to the shared excluded_profiles policy and records model-specific HF qualification and missing release-performance evidence. tools/perf_matrix.py consumes this list in _coverage() to exempt the model from the ready-catalog coverage requirement. This is a model-specific validation/performance policy decision in shared benchmark configuration, not model-agnostic behavior supplied through a family-owned narrow contract. The other changed files are within the stated Llama model/runtime/test exclusions.

Resolution

Remove the direct minicpm5-2b exclusion from apps/benchmark/performance/release.yaml. Either add a matching release-performance workload and receipt, or implement any required exemption through an existing family-owned contract that keeps model-specific validation policy out of shared configuration.

---

_{Comment @coderabbitai help to get the list of available commands.}

[yifeif-nv]

Hi @roma5087 thanks for contributing. It looks like the community CI is failing. Please fix the community CI first and I can help you to trigger the internal CI next!

[roma5087]

Closing this in favor of #1288, which landed while I was working through this — same bug (openbmb/MiniCPM5-2B's list-valued eos_token_id). It's additive (eos_token_ids only written when there's more than one id, so single-EOS bundles are untouched) rather than an unconditional breaking wire-format change like this PR's approach, and it adds vocab-bounds validation and explicit boolean rejection that this PR didn't have. Followed qwen3_8/model.py's existing precedent for the normalize pattern too.