PersonaPlex Moshi CPU speech parity#109
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_cb0460c0-1af2-45d7-a4ef-c1756f335268) |
There was a problem hiding this comment.
Pull request overview
This PR completes the PersonaPlex Moshi CPU speech parity work by aligning EMEL’s Moshi speech generation (sampling/state orchestration + GGUF metadata contract) and key CPU numeric paths (notably AArch64 Q4_K, softmax expf, and RMS norm) with a GGML CPU-only reference, and updating fixtures/tests to prove the maintained end-to-end path.
Changes:
- Adds a new Moshi speech generator + executor state-machine pipeline with explicit sampling phases and actor-owned deterministic RNG, plus PersonaPlex prompt/voice-prefill flows.
- Extends GGUF conversion/loader support to carry PersonaPlex inference metadata (prompt tokens/silence frames/dep_q) and optional EMEL packed Q4_K tensor metadata.
- Updates kernel and codec implementations/tests to lock down seed-sensitive numeric behavior and allow Mimi decode from a valid codebook prefix.
Reviewed changes
Copilot reviewed 47 out of 49 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/paritychecker/parity_engines.cpp | Adds a parallel matmul lane pool to paritychecker generation harness state and minor formatting tweaks. |
| tools/paritychecker/CMakeLists.txt | Makes the reference logging patcher robust to both model. and model-> call styles. |
| tools/bench/moshi_make_tiny_fixture.py | Updates tiny fixture metadata to include PersonaPlex-active inference knobs and clarifies Mimi fixture purpose. |
| tools/bench/moshi_gguf_convert.py | Adds PersonaPlex inference metadata normalization/validation and optional LM Q4_K tensor packing into EMEL’s packed dtype metadata. |
| tests/speech/codec/mimi_lifecycle_tests.cpp | Updates Mimi lifecycle expectations for active n_q=2 and adds partial-prefix decode coverage. |
| tests/speech/codec/mimi_detail_tests.cpp | Updates Mimi detail tests for active n_q=2 and adds partial-prefix decode validation. |
| tests/models/README.md | Refreshes fixture checksums and documentation for the updated tiny Moshi/Mimi fixtures and active n_q. |
| tests/models/moshi-tiny-lm.gguf | Updates the LFS pointer to the refreshed tiny LM GGUF fixture. |
| tests/models/moshi-tiny-config.json | Adds PersonaPlex inference metadata keys to the tiny Moshi config fixture. |
| tests/models/mimi-tiny.gguf | Updates the LFS pointer to the refreshed tiny Mimi GGUF fixture. |
| tests/models/mimi-tiny-q8.gguf | Updates the LFS pointer to the refreshed tiny Mimi Q8 fixture. |
| tests/models/mimi-tiny-f16.gguf | Updates the LFS pointer to the refreshed tiny Mimi F16 fixture. |
| tests/model/moshi/binding_tests.cpp | Extends Moshi LM KV bindings/tests to include inference metadata + depformer schedule metadata, and updates Mimi fixture expectations. |
| tests/kernel/lifecycle_tests.cpp | Adds bit-level parity tests for ARM64 softmax expf and RMS norm square order; validates kernel::any kind selection. |
| src/emel/speech/generator/moshi/guards.hpp | Introduces Moshi generator guards for contract validation, voice/prompt flows, graph execution, and output gating. |
| src/emel/speech/generator/moshi/actions.hpp | Implements Moshi generator effects for binding, voice/prompt prefill, graph stepping, delay masking, caching, and output publication. |
| src/emel/speech/generator/moshi/context.hpp | Adds Moshi generator actor context (runtime, voice prompt state, graph binding, delay/cache state, hybrid memory actor). |
| src/emel/speech/generator/moshi/events.hpp | Defines public Moshi generator events and per-dispatch ctx structs for init/voice/prompt/step flows. |
| src/emel/speech/generator/moshi/errors.hpp | Defines generator error codes for init/bind/memory/graph/voice/prompt/output paths. |
| src/emel/speech/generator/moshi/detail.hpp | Adds small internal helpers for runtime-event unwrapping and cache addressing. |
| src/emel/speech/generator/moshi/sm.hpp | Adds the Moshi generator SML transition table orchestrating memory, voice/prompt, graph execution, caching, and output production. |
| src/emel/speech/generator/moshi/any.hpp | Provides a stable alias for the Moshi generator state machine type. |
| src/emel/speech/generator/moshi/executor/detail.hpp | Adds executor tensor binding helpers, sampling primitives, and streaming KV view utilities. |
| src/emel/speech/generator/moshi/executor/context.hpp | Defines executor context (model contract binding, sampling config, KV bindings, kernel backend). |
| src/emel/speech/generator/moshi/executor/events.hpp | Defines executor init/step events and a large per-step ctx with scratch buffers for logits/top-k/sampling and intermediate tensors. |
| src/emel/speech/generator/moshi/executor/guards.hpp | Adds executor guards for contract/model/shape checks and capability routing across embedding/projection/attention/sampling phases. |
| src/emel/speech/generator/moshi/executor/actions.hpp | Implements executor effects for binding, KV binding, projection/attention kernels, and explicit sampling phases. |
| src/emel/speech/generator/moshi/executor/errors.hpp | Defines executor error codes for init/model/shape/unsupported graph execution/unexpected events. |
| src/emel/speech/generator/moshi/executor/sm.hpp | Adds executor SML transition table and per-event wrappers for init/graph-step dispatch. |
| src/emel/speech/generator/moshi/executor/any.hpp | Provides binding helpers to connect the generator to the executor via a graph_step dispatch function. |
| src/emel/speech/codec/mimi/quantizer/guards.hpp | Allows decode with a valid codebook-prefix span (bounded by semantic level count and n_q). |
| src/emel/speech/codec/mimi/guards.hpp | Mirrors prefix-valid decode request validation at the facade level. |
| src/emel/speech/codec/mimi/detail.hpp | Updates codec docs/contracts to reflect reference layout scans and prefix decode semantics. |
| src/emel/speech/codec/mimi/detail.cpp | Reworks RVQ encode to use reference squared-distance scanning and updates decode to accept active-prefix n_q. |
| src/emel/model/moshi/detail.cpp | Adds loader support/validation for inference metadata + depformer schedule and updates storage sizing via gguf compute_tensor_data_size. |
| src/emel/model/data.hpp | Extends Moshi LM hparams struct with inference metadata fields + depformer schedule storage. |
| src/emel/kernel/detail.hpp | Aligns packed Q4_K x8 scale packing, updates Q4_K×Q8_K dot accumulation order, adds ARM64 NEON expf parity, and fixes RMS square widening order. |
| src/emel/kernel/any.hpp | Makes kernel::any default-construct to the detected host backend kind. |
| src/emel/kernel/aarch64/sm.hpp | Adds AArch64 dispatch routes for packed Q4_K × f32 RHS and argmax variants; formatting cleanups. |
| src/emel/kernel/aarch64/guards.hpp | Adds/threads through new packed-Q4×f32 guard paths and formats SIMD guard helpers. |
| src/emel/gguf/loader/detail.hpp | Treats EMEL packed Q4_K x8 dtypes (41/42) as valid GGML types and computes their payload sizes. |
| docs/benchmarking.md | Documents that PersonaPlex inference contract is owned by GGUF metadata (prompt/silence/dep_q). |
| CMakeLists.txt | Adds tests/speech/generator/moshi_lifecycle_tests.cpp to the test build. |
| .planning/debug/moshi-personaplex-audio-parity.md | Records the parity investigation and resolution evidence in the planning debug log. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| struct effect_bind_nonzero_sampling_seed { | ||
| void operator()(const event::initialize_run &runtime_ev, | ||
| context &ctx) const noexcept { | ||
| ctx.sampling.random_state = runtime_ev.request.sampling_seed; | ||
| } | ||
| }; |
| bool | ||
| process_event(const emel::speech::generator::moshi::event::graph_step &ev) { | ||
| event::step_ctx ctx{}; | ||
| event::step_run runtime_ev{ev, ctx}; | ||
| const bool accepted = base_type::process_event(runtime_ev); |
![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: bd6f07b626
ℹ️ 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".
| const bool input_tail = ctx.lmgen.needed_tokens > 0 && | ||
| runtime_ev.request.audio_tokens.size() >= | ||
| static_cast<size_t>(ctx.lmgen.needed_tokens) && | ||
| runtime_ev.request.audio_tokens.size() != | ||
| static_cast<size_t>(ctx.lmgen.codebook_count); |
There was a problem hiding this comment.
When a PersonaPlex caller passes an audio_tokens span larger than needed_tokens but smaller than the full codebook count, this guard accepts the request, but effect_write_tail_input only copies the first needed_tokens entries and silently ignores the rest. That lets malformed frames drive generation with truncated conditioning instead of returning request_shape; the tail shape should be accepted only when it exactly matches needed_tokens (and the same condition should be mirrored by guard_has_tail_input).
Useful? React with 👍 / 👎.
| embeddings->dims[3] <= | ||
| static_cast<int64_t>(event::k_max_voice_embedding_dim) && |
There was a problem hiding this comment.
This limit is applied to dims[3] (the prompt frame count), but the fixed embedding_frame buffer is sized for one frame's embedding dimension (dims[0]) in the load-frame actions. A voice GGUF with dims[0] > 8192 and a small frame count can report load_voice success only for every prefill to be rejected later, while a long prompt can be rejected despite fitting the per-frame buffer; this check should bound dims[0] instead.
Useful? React with 👍 / 👎.
| for (int32_t index = 0; index < runtime_ev.ctx.delayed_dep_q; ++index) { | ||
| if (runtime_ev.request.audio_tokens_out[static_cast<size_t>(index)] == | ||
| action::k_token_zero) { | ||
| has_ungenerated = true; |
There was a problem hiding this comment.
effect_begin_step initializes generated audio lanes with k_token_ungenerated (-2), so if an accepted graph dispatch leaves any lane untouched the delayed output can still contain -2. This guard checks k_token_zero (-1) instead, which can mark such a frame as produced and publish an invalid -2 token; compare against the ungenerated sentinel here.
Useful? React with 👍 / 👎.
| , sml::state<state_uninitialized> <= sml::state<state_uninitialized> | ||
| + sml::unexpected_event<sml::_> | ||
| [ guard::guard_unexpected_error_out_present{} ] | ||
| / action::effect_mark_unexpected_and_store{} |
There was a problem hiding this comment.
These are the only unexpected_event routes and they are scoped to state_uninitialized, so after a successful initialization an unsupported external event such as a second initialize is rejected without running effect_mark_unexpected_and_store. In that scenario callers get false while error_out and callbacks remain stale, rather than the explicit unexpected-event error this machine otherwise models.
Useful? React with 👍 / 👎.
| return ctx.sampling.enabled && ctx.sampling.text_temperature > 0.0f && | ||
| !guard_text_sampling_config_valid{}(runtime_ev, ctx); |
There was a problem hiding this comment.
When sampling is enabled and text_temperature is NaN, this invalid guard returns false because NaN > 0.0f is false; the argmax guard also checks <= 0.0f, and the valid sampling guard is false as well. A no-forced-token step can therefore reach the text-logits decision state with no matching transition and fail without storing an error, so non-finite temperatures should be routed as invalid.
Useful? React with 👍 / 👎.
| return runtime_ev.request.forced_text_token >= 0 && | ||
| detail::token_in_embedding_range( | ||
| runtime_ev.request.forced_text_token, | ||
| runtime_ev.request.model.moshi_lm.text_card); |
There was a problem hiding this comment.
A caller can force forced_text_token == text_card because token_in_embedding_range accepts the extra embedding sentinel row, and effect_publish_forced_text_token then publishes that value directly. The logits paths only produce [0, text_card) and downstream public text-token checks expect < text_card, so forced output tokens should reject the sentinel row even though input embeddings may use it.
Useful? React with 👍 / 👎.
| const int32_t frame_count = runtime_ev.request.pre_text_silence_frames + | ||
| runtime_ev.request.text_token_count + | ||
| runtime_ev.request.post_text_silence_frames; |
There was a problem hiding this comment.
These public frame counts are added as int32_t before any of the nonnegative or maximum-size checks run, so a malformed request such as INT32_MAX + 1 can invoke signed overflow in the guard instead of routing deterministically to the prompt error path. Compute the total in a wider type after validating each term, or use checked addition before comparing against k_max_delay_rows.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b68cc077-bf60-4543-8a40-c1962fc8d2b4) |
gabewillen
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e8737de64
ℹ️ 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".
| const int32_t row = detail::cache_position( | ||
| ctx.lmgen, ctx.lmgen.offset + ctx.lmgen.delays[column]); | ||
| detail::cache_at(ctx.lmgen, row, column) = | ||
| runtime_ev.request.audio_tokens[static_cast<size_t>(index)]; |
There was a problem hiding this comment.
When PersonaPlex supplies the public tail-codebook span, this path writes those tokens into the delayed cache but leaves runtime_ev.ctx.provided_input false. The later guard_should_write_generated route then takes effect_write_generated instead of the preserving effect_write_generated_missing path, so for the usual one-step delayed lanes the generated audio overwrites the caller-provided conditioning tokens that were just cached.
Useful? React with 👍 / 👎.
| return ctx.sampling.enabled && ctx.sampling.audio_temperature > 0.0f && | ||
| !guard_depformer_sampling_config_valid{}(runtime_ev, ctx); |
There was a problem hiding this comment.
With sampling enabled and audio_temperature set to NaN, this invalid guard returns false because NaN > 0.0f is false, while the argmax guard below also rejects it via NaN <= 0.0f. A depformer logits step can then reach state_depformer_logits_bind_result_decision with no matching argmax, sampling, invalid, or bind-failed transition, causing process_event to fail without storing the modeled error.
Useful? React with 👍 / 👎.
| , sml::state<state_uninitialized> <= sml::state<state_uninitialized> | ||
| + sml::unexpected_event<sml::_> | ||
| [ guard::guard_unexpected_error_out_present{} ] | ||
| / action::effect_mark_unexpected_and_store{} |
There was a problem hiding this comment.
These are the only unexpected_event routes and they are scoped to state_uninitialized, so after a successful executor initialization an unsupported external event such as a second initialize is rejected by SML without running effect_mark_unexpected_and_store. Callers then get false with stale error_out rather than the explicit unexpected_event error modeled here.
Useful? React with 👍 / 👎.
gabewillen
changed the title
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fd3e6037-c1e0-4c56-85b3-c0092cf9421b) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63e4dd3011
ℹ️ 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".
| struct guard_sampling_seed_nonzero { | ||
| bool operator()(const event::initialize_run &runtime_ev, | ||
| const action::context &) const noexcept { | ||
| return runtime_ev.request.sampling_seed != 0u; |
There was a problem hiding this comment.
When a caller initializes sampling with sampling_seed equal to 2147483647 (or 4294967294), this guard accepts it because the seed is nonzero, but compute_sampling_exponential_argmax uses modulus 2147483647; the first RNG update sets random_state to 0, uniform becomes 0, and the generator then stays in the zero state and repeatedly picks the top candidate instead of sampling. Reject or remap seeds that are congruent to 0 modulo the RNG modulus.
Useful? React with 👍 / 👎.
| request.sampling.text_temperature > 0.0f && | ||
| request.sampling.audio_top_k > 0 && | ||
| request.sampling.text_top_k > 0 && request.sampling.seed != 0u && | ||
| request.max_blocks > 0 && request.block_tokens > 0 && |
There was a problem hiding this comment.
When initialize is called with max_blocks * block_tokens larger than the supplied temporal KV position_capacity, this accepts the session even though the executor later maps memory block ids directly to block_id * snapshot.block_tokens physical cache positions. After enough frames, valid memory allocations can therefore produce physical positions beyond the provided KV cache and every subsequent generation fails as generate_failed; reject such configurations up front or require a cache large enough for the reserved memory geometry.
Useful? React with 👍 / 👎.
| return runtime_ev.request.sampling_text_top_k <= | ||
| runtime_ev.request.model.moshi_lm.text_card; |
There was a problem hiding this comment.
When the executor is initialized directly with sampling enabled and sampling_text_top_k <= 0, this branch treats the value as within range and stores it, so initialization reports success but the first sampled text-token step can only fail later with graph_execution_unsupported; the audio top-k guard below has the same gap. Nonpositive top-k values should be rejected or normalized during initialization, matching the PersonaPlex session guard that already requires positive top-k values.
Useful? React with 👍 / 👎.
| const int32_t weight_index = lm.depformer_weight_schedule[index]; | ||
| if (weight_index < 0 || weight_index >= lm.dep_q) { |
There was a problem hiding this comment.
When a GGUF supplies a per-step schedule that points at a valid dep_q slot whose lm.depformer_in.N.weight tensor is absent, this validation still succeeds because it only bounds the integer against dep_q. The executor later resolves lm.depformer_in.%d.weight from the scheduled index before each depformer step, so such a model loads and initializes but fails generation as graph_execution_unsupported for the affected codebook; validate the scheduled tensor names before accepting the artifact.
Useful? React with 👍 / 👎.
| str(args.audio_top_k), | ||
| str(args.text_top_k), |
There was a problem hiding this comment.
When someone runs personaplex_compare.py with non-default --audio-top-k or --text-top-k, these values are passed only to the EMEL runner, while the reference command below still receives just the temperatures and the reference driver has no top-k options. That makes the comparison change one sampler but not the other, so reported match rates can reflect different sampling policies rather than EMEL/reference parity; either pass equivalent settings to the reference lane or remove the asymmetric CLI knobs.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_cef03099-03c9-4e16-80e1-d1ab5e4115b2) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 744adfa9ff
ℹ️ 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".
| const auto *projection = detail::find_lm_transformer_tensor( | ||
| runtime_ev.request.model, layer, "self_attn.out_projs.0.weight"); |
There was a problem hiding this comment.
This runtime path requires every temporal layer to have self_attn.out_projs.0.weight, but the Moshi LM contract validator only checks norm/input-projection/gating-in for transformer blocks. A GGUF missing this output projection can pass initialization and then every graph step fails here as graph_execution_unsupported; require the same output-projection tensor in the model contract before accepting the artifact.
Useful? React with 👍 / 👎.
| void operator()(const event::advance_voice_run &runtime_ev, | ||
| context &ctx) const noexcept { | ||
| runtime_ev.ctx.child_err = 0; | ||
| moshi::event::begin_personaplex_prompt request{}; |
There was a problem hiding this comment.
When the session reaches prompt prefill, this starts the generator prompt with a default begin_personaplex_prompt, leaving text_token_count at 0. In that state the generator takes the empty-prompt route and the later advance_prompt.text_token value (including the runner's prompt_text_token) is never consumed, so any non-silence PersonaPlex system prompt is silently skipped; pass a nonzero text-token count or expose the count during session initialization before entering state_prompt_prefill.
Useful? React with 👍 / 👎.
| if [[ ! -f "$MODEL_CONFIG" ]]; then | ||
| echo "error: missing pinned PersonaPlex model config: $MODEL_CONFIG" >&2 | ||
| exit 1 |
There was a problem hiding this comment.
In a clean artifact directory, personaplex-config.json is created by scripts/setup_moshi_cpp_reference.sh inside the later non---run-only setup path, but this new check runs before that fetch. Normal scripts/bench_mimi_compare.sh invocations, and setup_moshi_cpp_reference.sh --build-only which delegates here, now exit with “missing pinned PersonaPlex model config” instead of fetching/building; move this read until after setup or skip it for build-only.
Useful? React with 👍 / 👎.
| detail::cache_at(ctx.lmgen, row, index) = | ||
| runtime_ev.request.audio_tokens[static_cast<size_t>(index)]; |
There was a problem hiding this comment.
The full-input route writes caller-provided tokens into the delayed cache before any token-range validation by the graph executor. If a supplied token is outside the model card range, delayed lanes can be cached successfully and only poison a later frame when that row is read, leaving the actor ready with corrupted conditioning; validate public input token values in the step guards before mutating the cache.
Useful? React with 👍 / 👎.
| const auto *linear_in = detail::find_lm_transformer_tensor( | ||
| runtime_ev.request.model, layer, "gating.linear_in.weight"); |
There was a problem hiding this comment.
The LM contract accepts transformer blocks that only provide gating.0.linear_in.weight, but the executor looks only for gating.linear_in.weight. A model using the accepted indexed naming can initialize successfully and then fail every temporal layer as unsupported at generation time; either require the unindexed name in the contract/converter or add the same fallback here.
Useful? React with 👍 / 👎.
|
|
||
| moshi_lm_send2(generator, tokens); | ||
| const int produced = moshi_lm_receive(generator, text_token, tokens); | ||
| if (produced != 0) { |
There was a problem hiding this comment.
When moshi_lm_receive reports no delayed output for a frame, this skips both the reference output log line and the PCM frame. personaplex_compare.py requires len(ref_output) == args.frames, while the EMEL runner logs an EMEL_OUTPUT line and keeps a zero audio frame for every live/flush frame, so any initial no-output delay is reported as a frame-count or audio-length mismatch instead of a comparable silent/no-output frame.
Useful? React with 👍 / 👎.
| const auto *linear_out = detail::find_lm_transformer_tensor( | ||
| runtime_ev.request.model, layer, "gating.linear_out.weight"); |
There was a problem hiding this comment.
The transformer contract accepts a layer once the gating input weight exists, but this runtime step also requires gating.linear_out.weight. An artifact missing that output projection passes load and initialization, then fails generation as unsupported at the feed-forward projection; validate the gating output tensor alongside the input tensor before accepting the model.
Useful? React with 👍 / 👎.
| const auto *projection = detail::find_depformer_codebook_tensor( | ||
| model, layer, "self_attn.out_projs.%d.weight", weight_index); |
There was a problem hiding this comment.
The depformer block contract only checks norms and attention input projection, but each depformer layer later resolves self_attn.out_projs.%d.weight for the scheduled weight index. A GGUF missing these per-codebook output projections can bind and initialize successfully, then every affected depformer step fails as unsupported; include the scheduled depformer output-projection tensors in the model contract.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fcb5376c-ca9f-4cf3-8350-a435cd8c1d8c) |
There was a problem hiding this comment.
💡 Codex Review
emel.cpp/src/emel/model/moshi/detail.cpp
Lines 620 to 622 in 060d96c
Validate every indexed audio embedding
When an LM GGUF is missing an indexed audio embedding such as lm.emb.1.weight (or has it with the wrong shape) but still has at least n_q tensors under the lm.emb. prefix, this contract accepts it because it only counts the family and shape-checks lm.emb.0.weight. The executor later iterates lm.emb.%d.weight for every codebook in guard_token_input_embedding_supported, so the model initializes successfully and then every token-input step fails as graph_execution_unsupported; require each lm.emb.0..n_q-1.weight with the expected shape here.
emel.cpp/src/emel/model/moshi/detail.cpp
Lines 655 to 657 in 060d96c
Validate every depformer logits weight
When a GGUF has at least dep_q tensors under lm.linears. but an indexed logits weight such as lm.linears.1.weight is missing or has the wrong shape, this count check still accepts the artifact. The executor later resolves lm.linears.%d.weight for each depformer codebook in guard_depformer_logits_supported, so initialization succeeds and generation fails as graph_execution_unsupported for the affected codebook; require every indexed lm.linears.0..dep_q-1.weight with the expected {depformer_dim, card} shape here.
ℹ️ 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".
| tensor->dims[0] == ne0 && | ||
| (tensor->n_dims == 1 || tensor->dims[1] == ne1); |
There was a problem hiding this comment.
When a malformed GGUF provides a matrix weight as a 1-D tensor whose dims[0] matches the hidden size, this helper returns true even for callers that pass a required ne1 like hidden_dim * 3. The LM contract only checks existence for many transformer tensors (for example has_lm_transformer_block), so guard_temporal_layer_projection_supported can accept such a tensor and only fail later inside the matmul path as graph_execution_unsupported instead of rejecting the artifact/route at the shape guard; require n_dims >= 2 whenever the expected second extent is not the vector default.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9e2f6591-bba5-40b1-a6ed-45c0564a054a) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f097dbbcf
ℹ️ 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".
| exit 1 | ||
| fi | ||
| done | ||
| for tool in cmake ninja git python3 zig; do |
There was a problem hiding this comment.
When --run-only is passed, the wrapper is documented to use an existing build and artifacts, but this unconditional tool check still exits if build-only tools such as cmake, ninja, git, or zig are absent. That prevents running PersonaPlex comparisons on machines or containers that already have the artifacts and binaries but only the runtime compare dependencies; keep the build-tool requirements under the non-run-only path and leave only the tools actually needed for the compare checked here.
Useful? React with 👍 / 👎.
| matched = 0 | ||
| total = 0 | ||
| for left_frame, right_frame in zip(left, right): | ||
| width = min(streams, len(left_frame), len(right_frame)) |
There was a problem hiding this comment.
If either lane emits fewer than public_n_q codes for a frame, this uses the shorter width as the denominator, so a truncated log can report a perfect match as long as the prefix tokens agree. The later checks only compare frame counts, which lets dropped codebooks pass the PersonaPlex parity thresholds instead of failing the run; validate that every parsed frame contains the requested number of public streams before computing the fraction.
Useful? React with 👍 / 👎.
| const size_t begin = layer_offset + physical_position * dim; | ||
| const size_t end = begin + dim; |
There was a problem hiding this comment.
When a caller supplies a custom KV view with a very large layer_cache_offsets[layer], begin + dim can wrap and this guard can accept because the wrapped end is within the span. effect_write_temporal_layer_kv_cache then indexes from the original huge begin, so malformed cache metadata can write out of bounds; use checked multiply/add and reject offsets past the cache before the write path (the depformer guard below has the same pattern).
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_011367fc-86e9-403a-950e-01c571ed4225) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cf0988476
ℹ️ 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".
| : model->moshi_lm.inference_pre_text_silence_frames + | ||
| model->moshi_lm.inference_post_text_silence_frames; |
There was a problem hiding this comment.
When an empty begin_personaplex_prompt request uses the model defaults, the accepted hparams inference_pre_text_silence_frames and inference_post_text_silence_frames are added as int32_t before any maximum-size check. A malformed but otherwise loadable LM with two large nonnegative default counts can trigger signed overflow here, so the prompt-begin guard can behave nondeterministically instead of routing to the modeled prompt error; validate each term and add in a wider/checked type before comparing to k_max_delay_rows.
Useful? React with 👍 / 👎.
| const bool personaplex = lm.depformer_weights_per_step; | ||
| const int32_t delayed_dep_q = personaplex ? lm.inference_dep_q : lm.dep_q; | ||
| const int32_t needed_tokens = codebook_count - delayed_dep_q - 1; | ||
| const int32_t row_count = max_delay + 2 + static_cast<int32_t>(personaplex); |
There was a problem hiding this comment.
When a GGUF carries a very large nonnegative moshi.lm.delays value, the hparam loader can store it as int32_t and this guard adds max_delay + 2 + personaplex before rejecting rows above k_max_delay_rows. A delay such as INT32_MAX therefore invokes signed overflow in the guard instead of producing a deterministic bind failure; compute the row count in a wider/checked type after bounding each delay.
Useful? React with 👍 / 👎.
|
|
||
| struct effect_reset_session { | ||
| void operator()(const event::reset &, context &ctx) const noexcept { | ||
| ctx.session = {}; |
There was a problem hiding this comment.
When the executor is used directly with bound memory::streaming::sm position actors, event::reset clears only ctx.session, leaving temporal_positions advanced from the previous run. After reinitializing the same executor, the first graph step continues at the old physical/logical position and can attend over stale KV slots from the prior session instead of starting a fresh stream; reset the bound temporal/depformer position actors as part of this reset path.
Useful? React with 👍 / 👎.
Summary
Maintained evidence
d2c7fb0ad75d49353a8ffe4c7558f72839e0add6314fa8d531a9b1d2eff519961821eacc...9aef, reference9cf2da56...7afcbuild/personaplex_compare_organic_rope_guarded_10s/personaplex_compare.jsonVerification
0x3f908001and0x3ea6cd3bKnown repository gate exceptions
sm_scheduler/idle_asyncandsm_scheduler/busy_worker_asyncentriesPacked Q4_K and quantization evidence
--emel-lmand--emel-mimi; reports record model paths and SHA-256 valuesUpdated verification
Note
Low Risk
Documentation-only additions under
.planning/with no production code or runtime path changes.Overview
Adds generated planning architecture specs under
.planning/architecture/for the PersonaPlex/Moshi speech stack, mirroring the SML definitions insrc/.New
memory_streaming.mddocuments the streaming memory actor with an embedded Mermaid diagram and a full transition table (initialize, advance, reset,capture_view, error paths). Matching.mmdsources live under.planning/architecture/mermaid/.The same pattern is added for
speech_generator_moshi,speech_generator_moshi_executor(including explicit temporal RoPE query/key phases),speech_generator_moshi_personaplex_session(init → voice/prompt prefill → live/flush), andspeech_tokenizer_moshi(tokenize/detokenize phases and delay output).These files are documentation-only; they do not change runtime behavior.
Reviewed by Cursor Bugbot for commit 8cf0988. Bugbot is set up for automated code reviews on this repo. Configure here.