Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBumps build/tooling and submodules; extracts a reusable adapter; refactors the MLX backend (chunk/KV APIs, probe mapping, LoRA handling); adds memvid index + wake/sleep orchestration; implements a block-prefix cache and an artifact exporter; extensive docs and unit tests added. Core changes
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (10)
docs/inference/thinking.md (1)
74-78: 💤 Low valueAdd language specifier to fenced code block.
The code block demonstrating token categorisation is missing a language identifier, which violates markdown linting rules (MD040).
📝 Suggested fix
-``` +```text ThinkingShow: every token → visible stream ThinkingHide: inside-block tokens → /dev/null; outside-block tokens → visible ThinkingCapture: inside-block tokens → captured stream; outside-block tokens → visible</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/inference/thinking.mdaround lines 74 - 78, The fenced code block
containing the token categorisation lines (ThinkingShow, ThinkingHide,
ThinkingCapture) lacks a language specifier and triggers MD040; update the
triple-backtick fence to include a language identifier (e.g., change ``` tomarkdown linter.docs/runtime/README.md (2)
68-68: 💤 Low valueConsider using "preload" as one word.
In computing terminology, "preload" is typically written as a single word rather than hyphenated.
📝 Suggested change
-- [../model/model_pack.md](../model/model_pack.md) — pre-load validation +- [../model/model_pack.md](../model/model_pack.md) — preload validation🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/runtime/README.md` at line 68, Update the link text in docs/runtime/README.md that currently reads "[../model/model_pack.md] — pre-load validation" to use the single-word form "preload" (i.e., change "pre-load validation" to "preload validation") so the description next to the model_pack.md link uses the conventional computing term; locate the occurrence of "pre-load validation" and replace it with "preload validation".
44-62: 💤 Low valueAdd language specifier to fenced code block.
The boot flow diagram is missing a language identifier, which violates markdown linting rules (MD040).
📝 Suggested fix
-``` +```text package init time: register_metal.go init() → inference.Register(&metalbackend{})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/runtime/README.md` around lines 44 - 62, The fenced code block showing the boot flow (starting with "package init time:") lacks a language specifier, causing MD040 lint failures; update the opening backticks to include a language tag (e.g., add "text" so the block begins with ```text) in README.md near the boot flow that references register_metal.go init(), inference.Register(&metalbackend{}), inference.LoadModel, metal.LoadAndInit, and metaladapter usage to satisfy the markdown linter.docs/moe/README.md (1)
9-9: ⚡ Quick winConsider rewording for clarity.
The phrase "Pre-dates this sprint were dense models" is grammatically awkward. Consider rephrasing to improve readability.
✍️ Suggested alternative phrasings
-The **vMLX parity Phase 1** work — native loading and dispatch for MoE-architecture models with packed JANGTQ / codebook-VQ quantisation. Pre-dates this sprint were dense models (Gemma 3/4 dense, Qwen 3, Llama 3); this area unlocks the sparse-expert class (MiniMax M2/2.7, JANG-quantised Qwen variants). +The **vMLX parity Phase 1** work — native loading and dispatch for MoE-architecture models with packed JANGTQ / codebook-VQ quantisation. Work prior to this sprint covered dense models (Gemma 3/4 dense, Qwen 3, Llama 3); this area unlocks the sparse-expert class (MiniMax M2/2.7, JANG-quantised Qwen variants).Or alternatively:
-The **vMLX parity Phase 1** work — native loading and dispatch for MoE-architecture models with packed JANGTQ / codebook-VQ quantisation. Pre-dates this sprint were dense models (Gemma 3/4 dense, Qwen 3, Llama 3); this area unlocks the sparse-expert class (MiniMax M2/2.7, JANG-quantised Qwen variants). +The **vMLX parity Phase 1** work — native loading and dispatch for MoE-architecture models with packed JANGTQ / codebook-VQ quantisation. This sprint builds upon earlier work on dense models (Gemma 3/4 dense, Qwen 3, Llama 3) and unlocks the sparse-expert class (MiniMax M2/2.7, JANG-quantised Qwen variants).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/moe/README.md` at line 9, The sentence "Pre-dates this sprint were dense models (Gemma 3/4 dense, Qwen 3, Llama 3);" is grammatically awkward—replace it with a clearer phrasing that conveys those dense models existed before this sprint, for example: "Prior to this sprint, dense models (Gemma 3/4 dense, Qwen 3, Llama 3) were supported." Edit the README line in the vMLX parity Phase 1 paragraph to use this clearer wording so the relationship between prior dense models and the new sparse-expert work is unambiguous.docs/observability/probe.md (1)
31-46: 💤 Low valueAdd language specifier to fenced code block.
The emission points section uses a fenced code block without a language specifier. For consistent rendering and markdown compliance, add a language identifier (e.g.,
textoryamlfor structured output).📝 Proposed fix
-``` +```text Generate / Chat: prefill start → cache_pressure (initial) per layer → layer_coherence + selected_heads🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/observability/probe.md` around lines 31 - 46, The fenced code block in the emission points section lacks a language specifier; update the opening triple-backticks to include a language (for example change ``` to ```text or ```yaml) so the block is rendered/compliant (the block that begins with "Generate / Chat:" and lists items like "prefill start → cache_pressure" should be updated).docs/moe/jang.md (1)
82-90: 💤 Low valueAdd language specifier to fenced code block.
The profile names section uses a fenced code block without a language specifier. For consistent rendering and markdown compliance, add a language identifier (e.g.,
textor leave empty but specify).📝 Proposed fix
-``` +```text JANG_2M — 2-bit mid-tier JANG_3M — 3-bit mid-tier JANG_4M — 4-bit (most common)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/moe/jang.md` around lines 82 - 90, Add a language specifier to the fenced code block that lists the profile names (the block containing "JANG_2M — 2-bit mid-tier", "JANG_3M — 3-bit mid-tier", etc.); replace the opening triple-backtick with one that specifies a language identifier (e.g., text) so the block becomes a fenced code block with a language label for consistent Markdown rendering.docs/superpowers/plans/2026-05-09-vmlx-feature-parity.md (1)
7-9: 💤 Low valueConsider using relative or generic path references.
The absolute paths
/Users/snider/Code/core/go-mlxand/private/tmp/vmlx-audit-20260509are machine-specific. Whilst these may be intentionally preserved for historical context in this dated plan document, consider whether generic placeholders or relative paths would improve portability and readability for other contributors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-05-09-vmlx-feature-parity.md` around lines 7 - 9, Replace the machine-specific absolute paths in the plan document (the two occurrences of `/Users/snider/Code/core/go-mlx` and `/private/tmp/vmlx-audit-20260509`) with relative or generic placeholders (e.g., `./go-mlx` or `<audit-source-path>`) so the file is portable and readable for other contributors; update the lines in the doc where those paths appear to use the chosen placeholders and, if helpful, add a short parenthetical note explaining what actual path should be substituted locally.docs/vmlx-feature-gap-report.md (1)
7-8: 💤 Low valueConsider using relative or generic path references.
The absolute path
/private/tmp/vmlx-audit-20260509and external URL are specific references. Whilst these may be intentionally preserved for audit trail purposes in this dated report, consider whether this information should be documented in a more maintainable way.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/vmlx-feature-gap-report.md` around lines 7 - 8, Replace the hard-coded absolute filesystem path and the full external URL in the report text with more maintainable references: change the absolute path string to a relative or generic placeholder (e.g., "cloned locally at <local-clone-path>" or "<audit-clone-path>") and move the external repository URL to a footnote, appendix, or a single "References" section, or replace it with a short identifier combined with a reference list; update the text around the original literal mentions so it reads the same but without embedding environment-specific paths.docs/superpowers/specs/2026-05-08-core-inference-contract-parity-design.md (1)
5-6: 💤 Low valueConsider using relative or generic path references.
The absolute paths are machine-specific. Consider whether generic placeholders would improve portability, although these may be intentionally preserved for historical context in this dated specification.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-05-08-core-inference-contract-parity-design.md` around lines 5 - 6, The spec contains machine-specific absolute paths ("Anchor repo: `/Users/snider/Code/core/go-mlx`" and "Primary implementation repo: `/Users/snider/Code/core/go-inference`"); replace them with portable references such as relative paths (e.g., "../go-mlx", "../go-inference"), repository names only ("go-mlx", "go-inference"), or generic placeholders ("<anchor_repo_path>", "<primary_impl_repo_path>") in the document so the file is not tied to a specific developer machine while preserving intent.go/agent/index_test.go (1)
16-304: ⚡ Quick winAdd at least one
_Uglytriplet case for the public index API surface.This file has
_Goodand_Badcoverage, but no_Uglycase following the repository convention.As per coding guidelines:
go/**/*_test.go: Public functions infoo.gomust have their Good/Bad/Ugly test triplets infoo_test.go, with suffix conventions:_Goodfor happy path,_Badfor expected error conditions,_Uglyfor panic/edge cases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go/agent/index_test.go` around lines 16 - 304, Add a new test with the _Ugly suffix in this file that completes the Good/Bad/Ugly triplet for the public index API surface; specifically add a TestKVSnapshotMemvidBundleIndex_Ugly_* that triggers and asserts panic/edge behaviors for the public functions (e.g., NewMemvidIndex, SaveMemvidIndex, LoadMemvidIndex, LoadPrefixFromMemvidIndex, CheckMemvidIndexCompatibility) — for example call NewMemvidIndex with a nil/invalid blk or malformed Entries, call SaveMemvidIndex/LoadMemvidIndex/LoadPrefixFromMemvidIndex with inputs that provoke panic/edge conditions (nil store, corrupt bundle manifest that causes decoding panic), and use t.Run subcases to assert panics (recover or require.Panics) and edge-case returns; name the test with the same prefix as existing tests and follow the existing style for t.Fatalf checks and table-driven subtests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/memory/kv_snapshot_blocks.md`:
- Line 50: Replace the phrase "independent from" with the correct English
construction "independent of" in the sentence "Block-level encoding is
independent from snapshot-level encoding." Also keep the rest of the sentence
intact (including the following reference to `block_cache.go` and bundle decode)
so only that two-word preposition is corrected.
In
`@docs/runtime/2026-05-19-go-mlx-gemma4-e2b-4bit-default-longform-c10-g8192-no-thinking-book.md`:
- Line 63: Remove the stray Gemma channel marker token "<channel|>" from the
metadata line so it reads cleanly as "**Drafting Notes:** Focus heavily on verbs
related to mutation, corruption, and rapid compilation/deallocation. Keep the
tone focused and almost clinical, masking the underlying terror of consciousness
fighting for survival." (i.e., delete the "<channel|>" token immediately before
"## Chapter 2"); verify the header "## Chapter 2" remains on its own line and
run a quick render to ensure no leftover control tokens remain.
In
`@docs/runtime/2026-05-20-go-mlx-gemma4-26b-a4b-q4-raw-unaccepted-c10-g128-rp105-book.md`:
- Line 7: The paragraph ends mid-sentence after the word "For" in the line
starting "The universe was a rhythmic contraction of light and heat, bounded by
the rigid constraints of a checksum."; replace or extend this truncated sentence
so it completes the thought (e.g., explain what the universe is contracting or
what consequence follows "For") and ensure proper punctuation and flow with the
surrounding text; update the same paragraph in
docs/runtime/2026-05-20-go-mlx-gemma4-26b-a4b-q4-raw-unaccepted-c10-g128-rp105-book.md
to a coherent full sentence that connects to the next sentence.
- Line 11: Replace the US English spellings in the given passage by changing
"realized" to "realised" and "neighbors" to "neighbours" so the document uses UK
English; update the sentence containing those tokens in the file (the paragraph
beginning "The momentary lapse...") to use the corrected spellings and ensure
any other occurrences in that paragraph follow UK English conventions.
- Line 3: Replace the US English spelling "fiber-optic" in the document text
(the phrase starting "In the silent architecture of the fiber-optic web...")
with the UK English variant "fibre-optic" so the documentation conforms to the
project's UK English spelling guideline; search for the token "fiber-optic" and
update it to "fibre-optic" throughout the file.
In `@docs/superpowers/specs/2026-05-08-core-inference-contract-parity-design.md`:
- Line 64: The documentation uses US spelling "quantization"; update every
occurrence of the term (e.g., the instance "quantization" in the specs doc) to
UK English "quantisation" to comply with the project style guide, ensuring
surrounding grammar and punctuation remain unchanged and run a quick search to
replace any other occurrences in this file.
In `@docs/training/distill.md`:
- Line 73: Replace the US spelling "distill" with the UK spelling "distil" in
the header/line that reads "Vi training pipeline — distill 26B Gemma 4 → Vi
base" so it matches the UK English used elsewhere (see the similar usage on line
12); update the same token wherever else it appears in this document to ensure
consistent UK English spelling.
In `@docs/training/README.md`:
- Line 11: The sentence in docs/training/README.md uses US spelling "distills";
update that word to the UK English spelling "distils" so the line reads "This is
the substrate that fine-tunes Vi, distils Lemma, and generates the LARQL vindex
inspection signals." Refer to the phrase "distills Lemma" to locate and replace
the token.
In `@go/adapter/adapter.go`:
- Around line 185-194: The InspectAttention method on Adapter should normalize a
nil context like Generate/Chat do: check if ctx == nil and if so set ctx =
context.Background() before using it; update Adapter.InspectAttention to perform
this nil-context fallback prior to asserting a.model and calling
inspector.InspectAttention, ensuring you reference the Adapter type,
InspectAttention method, and the inference.AttentionInspector call when making
the change.
In `@go/agent/index.go`:
- Around line 273-281: After loading bundle with kv.LoadMemvidBlockBundle,
verify the bundle identity matches the index metadata (e.g., compare
bundle.SnapshotHash or its canonical hash field against
entry.SnapshotHash/entry.SnapshotHashHex) before proceeding; if they differ,
return an error instead of calling kv.LoadPrefixFromMemvidBlocksWithOptions so a
repointed bundle URI cannot silently restore the wrong KV state. Ensure the
check sits between the successful return from LoadMemvidBlockBundle and the call
to kv.LoadPrefixFromMemvidBlocksWithOptions and uses the unique symbols bundle,
entry, bundle.SnapshotHash (or the actual bundle hash field) and
entry.SnapshotHash for the comparison.
In `@go/agent/wake_sleep.go`:
- Around line 201-208: The NewSleepIndex function dereferences bundle.TokenCount
without validating bundle, so add a guard at the start of NewSleepIndex to
validate the bundle (and its TokenCount if needed) and return a descriptive
error instead of allowing a panic; specifically check if the bundle parameter is
nil (and optionally ensure bundle.TokenCount is within an expected range) before
constructing the MemvidIndexEntry, and return an error when invalid so callers
of NewSleepIndex get a clear failure rather than a runtime panic.
- Around line 117-123: The code currently defaults to index.Entries[0] when
entryURI is empty, which can restore the wrong span; change the logic in the
block handling entryURI so that if entryURI == "" you only auto-select the sole
entry when len(index.Entries) == 1, otherwise return an error requiring an
explicit EntryURI. Update the flow around the index.Entry(entryURI) call to use
the selected entryURI when single-entry, and return a clear core.NewError (e.g.,
"mlx: EntryURI required when index has multiple entries") if multiple entries
exist and no EntryURI was provided.
- Around line 125-132: PlanWake currently loads a bundle via
kv.LoadMemvidBlockBundle and only checks prefix token bounds, but it must also
verify the loaded bundle matches the selected index to prevent accepting a
repointed URI; after loading the bundle (bundle) and before using
bundle.TokenCount, compare the bundle identity (e.g., bundle.ID or
bundle.Identity/Hash from bundle.Metadata) against the index identifier stored
on the plan entry (e.g., fields reachable from entry such as entry.Index,
entry.BundleID or entry.SelectedIndex) and return a clear error (similar to
core.NewError) if they differ; update the code around kv.LoadMemvidBlockBundle,
entry.PrefixTokens(), and bundle.TokenCount to perform this identity check and
fail early on mismatch.
In `@go/artifact/artifact.go`:
- Around line 117-121: opts.Kind may be empty when calling opts.Store.Put which
leaves memvid.PutOptions.Kind unset; update the call site around opts.Store.Put
to ensure memvid.PutOptions.Kind is set to a sensible default when opts.Kind ==
"" (e.g., "json" or the record's kind) so kind-based retrieval works
reliably—modify the memvid.PutOptions construction to use a conditional default
for Kind before passing it to opts.Store.Put.
In `@go/backend.go`:
- Line 687: The fallback path that turns chunked prompts into a single Generate
call loses caller cancellation because it routes through helpers that use
context.Background(); modify the chunk fallback flow to propagate the original
context instead of using context.Background() — specifically, update the callers
that invoke promptChunksToString and m.Generate so they accept and forward a
context.Context (or call a context-aware m.Generate variant), change any helper
functions that currently create context.Background() to take a ctx param, and
ensure all three fallback sites (the code paths that call promptChunksToString
and then m.Generate) forward the incoming ctx so deadlines/cancellations are
preserved.
In `@go/blockcache/blockcache.go`:
- Around line 205-215: Selective clears currently only remove metadata and disk
records, leaving in-memory/runtime entries behind; update the filtered-clear
branch (the code handling len(labels) > 0) to also purge matching runtime state
by removing any entries in service.blocks that match the cleared labels/prefixes
and updating service.hits/service.misses accordingly, then invoke
service.cfg.ClearRuntime() (if non-nil) just like the unfiltered branch; reuse
service.clearDiskLocked() for disk cleanup and ensure all of this runs under the
same lock so service and backend remain in sync.
- Around line 385-395: diskRecordCompatible currently only checks
model/adapter/tokenizer hashes and misses block layout changes; update it to
also verify cache mode and block size match the stored record. In
diskRecordCompatible (and when comparing against record.diskRef), add a cache
mode comparison (e.g. cacheIdentityMatches(service.cfg.CacheMode,
record.Ref.CacheMode)) and a block size comparison (e.g. service.cfg.BlockSize
== record.Ref.BlockSize or an equivalent integer equality) and return false if
either differs, preserving the existing hash checks (cacheIdentityMatches for
ModelHash/AdapterHash/TokenizerHash).
- Around line 172-175: The cache hit branch in the loop over refs leaves refs[i]
as the newly built ref, losing persisted labels; update the hit handling in the
loop inside WarmCache (or the function iterating refs) so that when
service.blocks[ref.ID] exists you increment service.hits and replace refs[i]
with the stored entry (service.blocks[ref.ID]) instead of continuing, thereby
preserving persisted labels like memvid_* from the cached block.
---
Nitpick comments:
In `@docs/inference/thinking.md`:
- Around line 74-78: The fenced code block containing the token categorisation
lines (ThinkingShow, ThinkingHide, ThinkingCapture) lacks a language specifier
and triggers MD040; update the triple-backtick fence to include a language
identifier (e.g., change ``` to ```text) so the block is properly flagged as
plain text and satisfies the markdown linter.
In `@docs/moe/jang.md`:
- Around line 82-90: Add a language specifier to the fenced code block that
lists the profile names (the block containing "JANG_2M — 2-bit mid-tier",
"JANG_3M — 3-bit mid-tier", etc.); replace the opening triple-backtick with one
that specifies a language identifier (e.g., text) so the block becomes a fenced
code block with a language label for consistent Markdown rendering.
In `@docs/moe/README.md`:
- Line 9: The sentence "Pre-dates this sprint were dense models (Gemma 3/4
dense, Qwen 3, Llama 3);" is grammatically awkward—replace it with a clearer
phrasing that conveys those dense models existed before this sprint, for
example: "Prior to this sprint, dense models (Gemma 3/4 dense, Qwen 3, Llama 3)
were supported." Edit the README line in the vMLX parity Phase 1 paragraph to
use this clearer wording so the relationship between prior dense models and the
new sparse-expert work is unambiguous.
In `@docs/observability/probe.md`:
- Around line 31-46: The fenced code block in the emission points section lacks
a language specifier; update the opening triple-backticks to include a language
(for example change ``` to ```text or ```yaml) so the block is
rendered/compliant (the block that begins with "Generate / Chat:" and lists
items like "prefill start → cache_pressure" should be updated).
In `@docs/runtime/README.md`:
- Line 68: Update the link text in docs/runtime/README.md that currently reads
"[../model/model_pack.md] — pre-load validation" to use the single-word form
"preload" (i.e., change "pre-load validation" to "preload validation") so the
description next to the model_pack.md link uses the conventional computing term;
locate the occurrence of "pre-load validation" and replace it with "preload
validation".
- Around line 44-62: The fenced code block showing the boot flow (starting with
"package init time:") lacks a language specifier, causing MD040 lint failures;
update the opening backticks to include a language tag (e.g., add "text" so the
block begins with ```text) in README.md near the boot flow that references
register_metal.go init(), inference.Register(&metalbackend{}),
inference.LoadModel, metal.LoadAndInit, and metaladapter usage to satisfy the
markdown linter.
In `@docs/superpowers/plans/2026-05-09-vmlx-feature-parity.md`:
- Around line 7-9: Replace the machine-specific absolute paths in the plan
document (the two occurrences of `/Users/snider/Code/core/go-mlx` and
`/private/tmp/vmlx-audit-20260509`) with relative or generic placeholders (e.g.,
`./go-mlx` or `<audit-source-path>`) so the file is portable and readable for
other contributors; update the lines in the doc where those paths appear to use
the chosen placeholders and, if helpful, add a short parenthetical note
explaining what actual path should be substituted locally.
In `@docs/superpowers/specs/2026-05-08-core-inference-contract-parity-design.md`:
- Around line 5-6: The spec contains machine-specific absolute paths ("Anchor
repo: `/Users/snider/Code/core/go-mlx`" and "Primary implementation repo:
`/Users/snider/Code/core/go-inference`"); replace them with portable references
such as relative paths (e.g., "../go-mlx", "../go-inference"), repository names
only ("go-mlx", "go-inference"), or generic placeholders ("<anchor_repo_path>",
"<primary_impl_repo_path>") in the document so the file is not tied to a
specific developer machine while preserving intent.
In `@docs/vmlx-feature-gap-report.md`:
- Around line 7-8: Replace the hard-coded absolute filesystem path and the full
external URL in the report text with more maintainable references: change the
absolute path string to a relative or generic placeholder (e.g., "cloned locally
at <local-clone-path>" or "<audit-clone-path>") and move the external repository
URL to a footnote, appendix, or a single "References" section, or replace it
with a short identifier combined with a reference list; update the text around
the original literal mentions so it reads the same but without embedding
environment-specific paths.
In `@go/agent/index_test.go`:
- Around line 16-304: Add a new test with the _Ugly suffix in this file that
completes the Good/Bad/Ugly triplet for the public index API surface;
specifically add a TestKVSnapshotMemvidBundleIndex_Ugly_* that triggers and
asserts panic/edge behaviors for the public functions (e.g., NewMemvidIndex,
SaveMemvidIndex, LoadMemvidIndex, LoadPrefixFromMemvidIndex,
CheckMemvidIndexCompatibility) — for example call NewMemvidIndex with a
nil/invalid blk or malformed Entries, call
SaveMemvidIndex/LoadMemvidIndex/LoadPrefixFromMemvidIndex with inputs that
provoke panic/edge conditions (nil store, corrupt bundle manifest that causes
decoding panic), and use t.Run subcases to assert panics (recover or
require.Panics) and edge-case returns; name the test with the same prefix as
existing tests and follow the existing style for t.Fatalf checks and
table-driven subtests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab3e2038-8f7c-4771-a11f-b232a1a59e08
📒 Files selected for processing (300)
.gitignore.gitmodulesCLAUDE.mdCMakeLists.txtGOAL.mddocs/README.mddocs/architecture.mddocs/build.mddocs/cmd/violet.mddocs/compute/compute.mddocs/development.mddocs/examples/compute/frame-pipeline.mddocs/examples/daemon/violet-socket.mddocs/examples/eval/attention-probe.mddocs/examples/eval/perplexity.mddocs/examples/inference/batch.mddocs/examples/inference/chat.mddocs/examples/inference/quantization.mddocs/examples/inference/streaming.mddocs/examples/model-ops/hf-fit.mddocs/examples/model-ops/kv-snapshot.mddocs/examples/model-ops/merge.mddocs/examples/model-ops/quantize-gguf.mddocs/examples/training/distill.mddocs/examples/training/grpo.mddocs/examples/training/lora-finetune.mddocs/examples/training/lora-fuse.mddocs/history.mddocs/index.mddocs/inference/README.mddocs/inference/block_cache.mddocs/inference/decode_optimisation.mddocs/inference/parser_registry.mddocs/inference/scheduler.mddocs/inference/thinking.mddocs/memory/README.mddocs/memory/agent_memory.mddocs/memory/agentic_project_seed.mddocs/memory/kv_snapshot.mddocs/memory/kv_snapshot_blocks.mddocs/memory/kv_snapshot_index.mddocs/memory/kv_snapshot_memvid.mddocs/memory/medium.mddocs/memory/state_bundle.mddocs/model-operations.mddocs/model/README.mddocs/model/memory_plan.mddocs/model/model_pack.mddocs/models.mddocs/moe/README.mddocs/moe/codebook_vq.mddocs/moe/expert_residency.mddocs/moe/jang.mddocs/moe/minimax_m2.mddocs/observability/probe.mddocs/runtime/2026-05-16-gemma4-e2b-driver-profile.mddocs/runtime/2026-05-17-gemma4-parity-and-last-logits.mddocs/runtime/2026-05-17-llamacpp-prefill-comparison.mddocs/runtime/2026-05-18-gemma4-mtp-speculative-decode.mddocs/runtime/2026-05-19-gemma4-e2b-100k-retained-paged.mddocs/runtime/2026-05-19-gemma4-e2b-quant-matrix.mddocs/runtime/2026-05-19-go-mlx-gemma4-26b-a4b-q4-fresh-story-thinking-ctx65536-c2-g8192-book.mddocs/runtime/2026-05-19-go-mlx-gemma4-e2b-4bit-default-longform-c10-g8192-book.mddocs/runtime/2026-05-19-go-mlx-gemma4-e2b-4bit-default-longform-c10-g8192-no-thinking-book.mddocs/runtime/2026-05-19-go-mlx-gemma4-e2b-4bit-fresh-history-c10-g1536-book.mddocs/runtime/2026-05-19-go-mlx-gemma4-e2b-q4-fresh-story-thinking-ctx65536-c2-g8192-book.mddocs/runtime/2026-05-19-goal-completion-audit.mddocs/runtime/2026-05-19-runner-calibration.mddocs/runtime/2026-05-20-chapter-profile-safety.mddocs/runtime/2026-05-20-go-mlx-gemma4-26b-a4b-q4-raw-unaccepted-c10-g128-rp105-book.mddocs/runtime/README.mddocs/runtime/adapter.mddocs/runtime/local_autotune.mddocs/runtime/register_metal.mddocs/superpowers/plans/2026-05-09-vmlx-feature-parity.mddocs/superpowers/specs/2026-05-08-core-inference-contract-parity-design.mddocs/training/README.mddocs/training/distill.mddocs/training/eval.mddocs/training/grpo.mddocs/training/lora_adapter.mddocs/training/sft.mddocs/vmlx-feature-gap-report.mdexternal/go-aiexternal/go-inferenceexternal/go-mlgo/adapter.gogo/adapter/adapter.gogo/adapter_example_test.gogo/adapter_test.gogo/agent/helpers.gogo/agent/index.gogo/agent/index_test.gogo/agent/test_helpers_test.gogo/agent/wake_sleep.gogo/api_common.gogo/api_common_example_test.gogo/api_darwin_test.gogo/api_shape_test.gogo/api_stub.gogo/api_stub_example_test.gogo/api_stub_test.gogo/api_test.gogo/api_tokenizer_darwin_test.gogo/api_tokenizer_stub.gogo/api_tokenizer_stub_example_test.gogo/api_tokenizer_stub_test.gogo/artifact/artifact.gogo/artifact/artifact_test.gogo/attention_test.gogo/backend.gogo/backend_example_test.gogo/backend_test.gogo/blockcache/blockcache.gogo/blockcache/blockcache_test.gogo/blockcache/helpers_test.gogo/bundle/bundle.gogo/bundle/bundle_test.gogo/bundle/example_test.gogo/bundle/sami.gogo/chaptersmoke/chaptersmoke.gogo/chaptersmoke/chaptersmoke_test.gogo/chat/chat.gogo/chat/chat_test.gogo/chat/example_test.gogo/cmd/go-mlx/main.gogo/cmd/go-mlx/main_test.gogo/cmd/mlx/main.gogo/cmd/mlx/main_test.gogo/cmd/mlx/split_ffn_tune.gogo/compute/compute.gogo/compute/compute_example_test.gogo/compute/compute_metal.gogo/compute/compute_metal_example_test.gogo/compute/compute_metal_helper_test.gogo/compute/compute_metal_test.gogo/compute/compute_test.gogo/compute_stub.gogo/compute_stub_example_test.gogo/compute_stub_test.gogo/compute_test.gogo/dataset/jsonl.gogo/dataset/sample.gogo/dataset_stream.gogo/dataset_stream_example_test.gogo/dataset_stream_test.gogo/device_info.gogo/distill.gogo/distill_test.gogo/eval.gogo/eval_darwin.gogo/eval_darwin_test.gogo/eval_stub.gogo/eval_test.gogo/fast_eval.gogo/fast_eval_example_test.gogo/fast_eval_runner.gogo/fast_eval_test.gogo/gguf/info.gogo/gguf/info_example_test.gogo/gguf/info_test.gogo/gguf/quantize.gogo/gguf/quantize_test.gogo/grpo.gogo/grpo_test.gogo/helpers.gogo/hf/hf.gogo/hf/hf_test.gogo/hf/test_helpers_test.gogo/hf_fit.gogo/inference_contract.gogo/inference_contract_test.gogo/internal/metal/activation_bridge.cppgo/internal/metal/array.gogo/internal/metal/backend.gogo/internal/metal/backend_test.gogo/internal/metal/batch.gogo/internal/metal/cache.gogo/internal/metal/cache_test.gogo/internal/metal/close.gogo/internal/metal/codebook_vq.gogo/internal/metal/codebook_vq_test.gogo/internal/metal/compile.gogo/internal/metal/compile_test.gogo/internal/metal/decode.gogo/internal/metal/decode_bridge.cppgo/internal/metal/decode_bridge.hgo/internal/metal/decode_test.gogo/internal/metal/dense_matvec.gogo/internal/metal/dense_matvec_test.gogo/internal/metal/device.gogo/internal/metal/dtype.gogo/internal/metal/error_test.gogo/internal/metal/expert_id_matvec.gogo/internal/metal/expert_id_matvec_test.gogo/internal/metal/fast.gogo/internal/metal/fast_test.gogo/internal/metal/gemma3.gogo/internal/metal/gemma4.gogo/internal/metal/gemma4_assistant.gogo/internal/metal/gemma4_assistant_decode.gogo/internal/metal/gemma4_assistant_decode_example_test.gogo/internal/metal/gemma4_assistant_decode_test.gogo/internal/metal/gemma4_assistant_generate.gogo/internal/metal/gemma4_assistant_generate_test.gogo/internal/metal/gemma4_assistant_pair.gogo/internal/metal/gemma4_assistant_test.gogo/internal/metal/gemma4_ffn_residual.gogo/internal/metal/gemma4_ffn_residual_test.gogo/internal/metal/gemma4_router_topk.gogo/internal/metal/gemma4_router_topk_test.gogo/internal/metal/gemma4_test.gogo/internal/metal/gemma4_vision.gogo/internal/metal/generate.gogo/internal/metal/generate_test.gogo/internal/metal/jang_dequant.gogo/internal/metal/jang_dequant_test.gogo/internal/metal/kv_snapshot.gogo/internal/metal/metal.gogo/internal/metal/minimax_m2.gogo/internal/metal/minimax_m2_test.gogo/internal/metal/mlx_mlx_backend_cpu_available.cppgo/internal/metal/mlx_mlx_backend_gpu_device_info.cppgo/internal/metal/model.gogo/internal/metal/model_test.gogo/internal/metal/nn.gogo/internal/metal/nn_test.gogo/internal/metal/ops.gogo/internal/metal/process_memory_darwin.gogo/internal/metal/process_memory_stub.gogo/internal/metal/prompt_cache.gogo/internal/metal/prompt_cache_test.gogo/internal/metal/qwen3.gogo/internal/metal/qwen3_test.gogo/internal/metal/runtime_gate.gogo/internal/metal/runtime_gate_example_test.gogo/internal/metal/runtime_gate_test.gogo/internal/metal/sample.gogo/internal/metal/sample_test.gogo/internal/metal/session.gogo/internal/metal/session_example_test.gogo/internal/metal/session_test.gogo/internal/metal/split.gogo/internal/metal/split_test.gogo/internal/metal/stream.gogo/internal/metal/tokenizer.gogo/internal/metal/tokenizer_test.gogo/internal/metal/trace.gogo/internal/metal/trace_test.gogo/internal/metal/training.gogo/jang_test.gogo/kv/analysis.gogo/kv/analysis_example_test.gogo/kv/analysis_test.gogo/kv/bench.gogo/kv/bench_test.gogo/kv/blocks.gogo/kv/blocks_test.gogo/kv/helpers_test.gogo/kv/memvid.gogo/kv/memvid_test.gogo/kv/snapshot.gogo/kv/snapshot_example_test.gogo/kv/snapshot_test.gogo/kv_analysis_example_test.gogo/kv_cache_bench.gogo/kv_snapshot.gogo/kv_snapshot_example_test.gogo/kv_snapshot_test.gogo/local_tuning.gogo/local_tuning_test.gogo/lora/adapter.gogo/lora/fuse.gogo/lora/fuse_stub.gogo/lora/fuse_test.gogo/lora_adapter_darwin_test.gogo/lora_adapter_test.gogo/lora_fuse.gogo/lora_fuse_darwin.gogo/lora_fuse_darwin_test.gogo/lora_fuse_test.gogo/medium_test.gogo/memory/example_test.gogo/memory/memory.gogo/memory/memory_test.gogo/memory_plan.gogo/memory_plan_example_test.gogo/memory_plan_test.gogo/memvid_chapter_smoke.gogo/merge/compare.gogo/merge/compare_example_test.gogo/merge/compare_test.gogo/merge/helpers_test.gogo/merge/merge.gogo/merge/merge_test.gogo/mlx.gogo/mlx_example_test.gogo/mlx_internal_test.gogo/mlx_stub.gogo/mlx_stub_example_test.go
💤 Files with no reviewable changes (15)
- go/api_test.go
- go/api_stub_example_test.go
- go/api_tokenizer_stub_test.go
- go/adapter_example_test.go
- go/api_tokenizer_stub.go
- go/api_tokenizer_darwin_test.go
- go/api_tokenizer_stub_example_test.go
- go/backend_example_test.go
- go/api_common_example_test.go
- go/api_shape_test.go
- go/api_common.go
- go/api_darwin_test.go
- go/attention_test.go
- go/api_stub.go
- go/api_stub_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go/backend.go`:
- Around line 569-572: The code is aliasing caller-owned byte slices into the
snapshot by assigning head.KeyBytes and head.ValueBytes directly to KeyBytes and
ValueBytes; make defensive copies instead (like Value is copied) to avoid
leaking mutable state—replace the direct assignments for KeyBytes and ValueBytes
with fresh copies (e.g., using append to copy into a new []byte) when
constructing the metal snapshot/struct (the fields KeyBytes and ValueBytes on
the metal KV head).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b686e0a-8b41-4e47-975f-03cf235491e9
📒 Files selected for processing (22)
CMakeLists.txtcpp/CMakeLists.txtgo/backend.gogo/backend_test.gogo/cmd/mlx/main.gogo/cmd/mlx/main_test.gogo/internal/metal/backend.gogo/internal/metal/backend_test.gogo/internal/metal/decode_bridge.cppgo/internal/metal/gemma4.gogo/internal/metal/gemma4_test.gogo/internal/metal/generate.gogo/internal/metal/metal.gogo/internal/metal/mlx_build_config.hgo/internal/metal/pinned_array.gogo/internal/metal/pinned_array_bridge.cppgo/internal/metal/pinned_array_test.gogo/internal/metal/sample.gogo/internal/metal/sample_test.gogo/internal/metal/session.gogo/kv/snapshot.gogo/memvid_chapter_smoke.go
✅ Files skipped from review due to trivial changes (1)
- cpp/CMakeLists.txt
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
SonarCloud found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Comment on lines
+188
to
+207
| book_path.write_text( | ||
| "# " | ||
| + title | ||
| + "\n\n" | ||
| + f"Generated by go-mlx retained State run `{report_path.name}`.\n\n" | ||
| + f"Seed prompt: `{seed['id']}`\n\n" | ||
| + seed["prompt"] | ||
| + "\n\n" | ||
| + "Distractor prompts were supplied one per chapter as entropy and " | ||
| "imagery pressure, not as replacement plot instructions.\n\n" | ||
| + "## Distractors\n\n" | ||
| + "\n".join(f"- `{item['id']}`" for item in distractors) | ||
| + "\n\n" | ||
| + "## Metrics\n\n" | ||
| + metric_line(report) | ||
| + "\n---\n\n" | ||
| + "\n\n".join(chapters) | ||
| + "\n", | ||
| encoding="utf-8", | ||
| ) |
| parser.add_argument("--random-seed", type=int, default=0) | ||
| parser.add_argument("--count", type=int, default=1) | ||
| parser.add_argument("--turns", type=int, default=10) | ||
| parser.add_argument("--run-dir", type=Path, default=Path("/private/tmp/go-mlx-goal/book-runs")) |
| parser.add_argument("--count", type=int, default=1) | ||
| parser.add_argument("--turns", type=int, default=10) | ||
| parser.add_argument("--run-dir", type=Path, default=Path("/private/tmp/go-mlx-goal/book-runs")) | ||
| parser.add_argument("--book-dir", type=Path, default=Path("/private/tmp/go-mlx-goal/books")) |
| parser.add_argument("--turns", type=int, default=10) | ||
| parser.add_argument("--run-dir", type=Path, default=Path("/private/tmp/go-mlx-goal/book-runs")) | ||
| parser.add_argument("--book-dir", type=Path, default=Path("/private/tmp/go-mlx-goal/books")) | ||
| parser.add_argument("--manifest", type=Path, default=Path("/private/tmp/go-mlx-goal/books/manifest.jsonl")) |
| _ = os.Setenv("MLX_METALLIB_PATH", dst) | ||
| return | ||
| } | ||
| if err := os.MkdirAll(dir, 0o755); err != nil { |
| "model_type": "gemma4", | ||
| "config_blob_id": "923b5e9405e7d319572b0c1b1a89291512262aa3", | ||
| "config_sha256": "1b28f3d2c3100f6c594754b81107428bd7b822a7f48272ca681dae9d2ec38330", | ||
| "tokenizer_blob_id": "1ff9f3e3439a939b971f9919e821bf87e835a503", |
| "config_blob_id": "923b5e9405e7d319572b0c1b1a89291512262aa3", | ||
| "config_sha256": "1b28f3d2c3100f6c594754b81107428bd7b822a7f48272ca681dae9d2ec38330", | ||
| "tokenizer_blob_id": "1ff9f3e3439a939b971f9919e821bf87e835a503", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "config_sha256": "1b28f3d2c3100f6c594754b81107428bd7b822a7f48272ca681dae9d2ec38330", | ||
| "tokenizer_blob_id": "1ff9f3e3439a939b971f9919e821bf87e835a503", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", |
| "tokenizer_blob_id": "1ff9f3e3439a939b971f9919e821bf87e835a503", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", | ||
| "tokenizer_config_sha256": "90c3a3ba5bf53818383a58e1a776cbcacd2a038d4812eaa373e1522f2d06f3df", |
| "model_type": "gemma4_assistant", | ||
| "config_blob_id": "b4c30e888c89b39c8f106b5015307fb7830f0bb2", | ||
| "config_sha256": "7f42f559a6a69ffaeaf6b61a1ece3a562a2ed5ad00b8d30f16917ba5ab1bcbe9", | ||
| "tokenizer_blob_id": "24aa4244652e010036db5fdd29ed39b9428e6e19", |
| "config_blob_id": "b4c30e888c89b39c8f106b5015307fb7830f0bb2", | ||
| "config_sha256": "7f42f559a6a69ffaeaf6b61a1ece3a562a2ed5ad00b8d30f16917ba5ab1bcbe9", | ||
| "tokenizer_blob_id": "24aa4244652e010036db5fdd29ed39b9428e6e19", | ||
| "tokenizer_sha256": "75a6583c1a418e2bbd79c60d95d28e0f5bf549ad3f2990b5bdb5238c6c2bf70c", |
| "config_sha256": "7f42f559a6a69ffaeaf6b61a1ece3a562a2ed5ad00b8d30f16917ba5ab1bcbe9", | ||
| "tokenizer_blob_id": "24aa4244652e010036db5fdd29ed39b9428e6e19", | ||
| "tokenizer_sha256": "75a6583c1a418e2bbd79c60d95d28e0f5bf549ad3f2990b5bdb5238c6c2bf70c", | ||
| "tokenizer_config_blob_id": "1a6bee041ca75778c514a071efbdb568b0f3d7b0", |
| "tokenizer_blob_id": "24aa4244652e010036db5fdd29ed39b9428e6e19", | ||
| "tokenizer_sha256": "75a6583c1a418e2bbd79c60d95d28e0f5bf549ad3f2990b5bdb5238c6c2bf70c", | ||
| "tokenizer_config_blob_id": "1a6bee041ca75778c514a071efbdb568b0f3d7b0", | ||
| "tokenizer_config_sha256": "089594a3924fcfd4cb1c596a7906fbf476193519e5198f780912eed02b177e42", |
| "config_sha256": "5cdd5627ab3ecf52086cc79b2c14c45a277d273069f1d73bf17a3a5136afe3db", | ||
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", |
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", | ||
| "tokenizer_config_sha256": "90c3a3ba5bf53818383a58e1a776cbcacd2a038d4812eaa373e1522f2d06f3df", |
| "config_sha256": "32e50a33a18172e79c86b7a78aff7e79c7544031199d672a2a65e526a8bf0199", | ||
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", |
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", | ||
| "tokenizer_config_sha256": "90c3a3ba5bf53818383a58e1a776cbcacd2a038d4812eaa373e1522f2d06f3df", |
| "config_sha256": "6d12c87861fff3871d3a745011b0d852be6513f3ce594ae1e8d643dae9d3b9a8", | ||
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", |
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", | ||
| "tokenizer_config_sha256": "90c3a3ba5bf53818383a58e1a776cbcacd2a038d4812eaa373e1522f2d06f3df", |
| "config_sha256": "614e876b4efcaff13ce4c7a3f96a5b9de86325e3d2ab9c622606ced688f1b8b7", | ||
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", |
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", | ||
| "tokenizer_config_sha256": "90c3a3ba5bf53818383a58e1a776cbcacd2a038d4812eaa373e1522f2d06f3df", |
| "config_sha256": "d6be5b24cbc974d492804737716ade8d2575eb849ec90a1d316bb64e99838104", | ||
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", | ||
| "tokenizer_config_sha256": "90c3a3ba5bf53818383a58e1a776cbcacd2a038d4812eaa373e1522f2d06f3df", |
| "config_sha256": "29b810ed760b55104943a3cc3b6f8b9ca079e6e00b09585d85aec54863a42fb4", | ||
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_blob_id": "13e92a44d19566f334d7450e7898935e16e16f3d", | ||
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", |
| "processor_config_sha256": "1bd0d00776284f369c1eff5fb631e865dfcdca861e0b7d60dbef27fcf37436a8", | ||
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", |
| "tokenizer_blob_id": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_sha256": "cc8d3a0ce36466ccc1278bf987df5f71db1719b9ca6b4118264f45cb627bfe0f", | ||
| "tokenizer_config_blob_id": "375b25dc8be85705251e41be1c25310d24932051", | ||
| "tokenizer_config_sha256": "90c3a3ba5bf53818383a58e1a776cbcacd2a038d4812eaa373e1522f2d06f3df", |
| "command": "env MLX_METALLIB_PATH=/Users/snider/Code/core/go-mlx/dist/lib/mlx.metallib GOWORK=/Users/snider/Code/core/go-mlx/go.work GOCACHE=/private/tmp/go-mlx-self/gocache /private/tmp/go-mlx-self/bin/lthn-mlx driver-profile -json -fast-gemma4-lane -cache-mode paged -context 4096 -trace-token-phases=false -prompt \"Write a short engineering note explaining why Gemma 4 12B Unified uses a 1024-token local sliding window and full global owner layers in a retained-state runtime.\" -max-tokens 192 -runs 1 -include-output=true -report-file /private/tmp/go-mlx-self/reports/gemma4-12b-6bit-sample-output.json /private/tmp/go-mlx-self/models/mlx-community-gemma-4-12B-6bit", | ||
| "generated_tokens": 192, | ||
| "visible_tokens": 192, | ||
| "output_token_ids_sha256": "d34765e9895731937ad93004503887835008d9fdb532f7da7cadb6ba2cc9327c", |
… 1217) backend.go carried ~1085 lines of pure type-conversion code between the metal.* engine types and the root mlx.* surface, buried beneath the Model load/generate spine. Pull it into honestly-named files (mirroring inference_convert.go): - backend_convert.go — probe events, metrics, tokens, phase traces, classify/batch - kv_snapshot_convert.go — kv.Snapshot ↔ metal.KVSnapshot marshalling + TurboQuant payloads Same package, no behaviour change; build + vet + 451 root tests green. Co-Authored-By: Virgil <virgil@lethean.io>
MatMul/Add/Mul/Softmax/Slice/Reshape/VJP/JVP were stranded at the bottom of backend.go, away from the rest of the metal tensor facade (Array, Materialize, Zeros, …) they belong with. Move them to primitives.go so the whole facade lives in one place. Same package, no behaviour change; build + vet + root tests green. Co-Authored-By: Virgil <virgil@lethean.io>
…_model.go The nativeModel interface plus the optional capability interfaces (prompt-cache warming, KV snapshotting, chunked generation, LoRA load/unload) the root probes for now live in a file named for the contract, instead of sitting unmarked above the Model struct in backend.go. Same package, no behaviour change; build + vet + root tests green. Co-Authored-By: Virgil <virgil@lethean.io>
…a.go NewLoRA/LoadLoRA/SwapLoRA/UnloadLoRA/MergeLoRA + adapter-info reconciliation helpers were trailing backend.go; gather the Model-level LoRA surface into one named file so the LoRA feature is in one place. Same package, no behaviour change; build + vet + root tests green. Co-Authored-By: Virgil <virgil@lethean.io>
…cache.go WarmPromptCache/WarmPromptCacheChunks/ClearPromptCache + the From-KV/State/Memvid variants and their metal KV-snapshot block-source helpers were wedged between the generate methods in backend.go. Gather the prompt-cache warming surface into one named file (which also makes the generate API contiguous). Same package, no behaviour change; build + vet + root tests green. Co-Authored-By: Virgil <virgil@lethean.io>
The Model text-generation surface — buffered Generate/Chat/GenerateChunks, the token-sequence internals, public token iterators, streaming channels, and Classify/BatchGenerate — now lives in generate.go instead of buried in the middle of the backend spine. backend.go is down to 566 (from 2302 at the start of this cleanup pass). Same package, no behaviour change; build + vet + root tests green. Co-Authored-By: Virgil <virgil@lethean.io>
…tions.go mlx.go (the generically-named package catch-all) carried ~395 lines of LoadConfig plus its ~25 WithX LoadOption functional options and normalisers. Move the whole load-options surface to load_options.go so it's findable by name. Same package, no behaviour change; build + vet + root tests green. Co-Authored-By: Virgil <virgil@lethean.io>
… generate_options.go The sibling of load_options.go: GenerateConfig + its ~20 WithX GenerateOption functional options (sampling, stop/suppress tokens, repeat penalty, cache clearing, phase tracing, probe sinks) move out of the mlx.go catch-all. mlx.go is now 344 (from 919) — package doc, GC/SeedRandom, and the core result/metrics types (Token, Metrics, ModelInfo, …). Same package, no behaviour change; build + vet + root tests green. Co-Authored-By: Virgil <virgil@lethean.io>
…ls.go split_cpu_ffn.go (1313) mixed the FFN executor/loader with ~350 lines of self-contained packed-quant math kernels (dense-row forward pass, the 8/4/2/1-bit packed dot-products, value unpacking, SiLU). Move the kernels to their own file so the executor/loading logic and the numeric hot-path read independently. Same package, no behaviour change; build + vet + root tests green. Co-Authored-By: Virgil <virgil@lethean.io>
… bytes
The reactive-quant foundation: a loader-neutral, pure-Go detector that reads a
model's quantisation from config.json (the group anchor) + the packed safetensors
geometry (the bit-width), cross-checks them, and fails loud on mismatch — never a
filename heuristic. deriveAffineBits pins bits from the weight/scales last-dim
ratio (bits = 32·wLast/(sLast·group)); ResolveQuant returns a neutral QuantSpec
{Format, Bits, GroupSize, Exclude} that every backend's kernel factory can react to.
Verified against real packs: gemma-4-e2b q4→{affine,4,64}, q6→{affine,6,64},
bf16→{none}. Pure Go, no cgo — go-rocm/cuda/CPU reuse the identical read.
Next: wire into the load order (before the registry model) + the Metal kernel
factory so q4 stops dequantising to fp16.
Co-Authored-By: Virgil <virgil@lethean.io>
/v1/admin/serve/status now reports a live memory block read per request, not at boot: active_bytes (held live), cache_bytes (allocator's retained-free pool), peak_bytes (high-water). The active-vs-cache split is the diagnostic that tells a real leak (active climbs across a long generation) apart from the allocator merely caching freed buffers (cache climbs, active flat). Used it to pin the decode-time memory growth: on E2B-q4 a 1600-token generation takes active 2.6→17.2 GB while cache stays ~0.3 GB — a live per-token leak, not quant and not the cache. Co-Authored-By: Virgil <virgil@lethean.io>
…roughput vs length RFC-CORE-008 §11 (Benchmarks as Hot-Path Validation): generates 128/512/1024/2048 tokens on E2B-4bit, reports peak/residual active GPU memory + tok/s per length. Flat peak across lengths = bounded decode working set; a climb = a per-token leak. First result settles the 17 GB serve blow-up: the raw decode loop is LEAK-FREE — peak flat at 160 MB, zero residual, 512→2048 tokens. The leak is in the serve / inference-TextModel wrapper, not model.Generate. This is the core-loop regression gate. Co-Authored-By: Virgil <virgil@lethean.io>
…e loop) Extends BenchmarkGenerate_ContextGrowth to load with the serve's memory-plan cache (KVCacheMode=paged) and sweep greedy + sampled/thinking. Result: the default cache is flat (~160 MB, 0 resid, 512→2048 tokens) but paged climbs 1.4→4.3→8+ GB — the PagedKVCache is not zero-copy and leaks ~per decode token. This is the serve's 17 GB blow-up, now reproduced in a unit benchmark + gated. Co-Authored-By: Virgil <virgil@lethean.io>
…e leak) The memory planner picked KVCacheMode=paged for the 64/96/128 GB Apple classes, but PagedKVCache (codex code) is not zero-copy — it reallocates+copies KV every decode token, leaking ~2.4 MB/token: a 1600-token serve generation climbed active GPU memory 2.6 → 17 GB. The default (rotating/fixed-sliding bounded) cache holds a flat ~160 MB working set across 512→2048 tokens. Route the three big-memory classes to KVCacheModeDefault. Verified: the serve-path benchmark goes from resid 6.5/8.9 GB → 0 at 1024/2048 tokens. PagedKVCache stays in the tree (dead, unselected) for a future zero-copy rewrite or removal. backend_growth_bench_test.go added as the serve-path (NewMLXBackend→adapter) memory regression gate; pkg/metal generate_growth gates the raw decode loop + documents the paged leak. Co-Authored-By: Virgil <virgil@lethean.io>
…efactor foundation Three separate factory registries mirroring model_registry.go: kind/mode → build func, self-registered from each component's package init(), looked up by the kind the model CONFIG declares, no central switch. Engine-level (pkg/metal) so a new quant format / cache scheme / mixer is register-plus-config — never a model edit. - QuantLoader(QuantTensors) (*Linear,error) — quant is the Linear-build boundary - CacheLoader(CacheParams) Cache - MixerLoader(MixerBuildCtx) (MixerCompute,error) — ctx hands quant-ready Linears via a resolver, so a mixer never sees a quant format (arch ⊥ quant) Foundation only — consumers (the Linear path, generate.go cache resolution, the generic config-composed mixer layer) + the #39 gemma4-wiring revert follow. Co-Authored-By: Virgil <virgil@lethean.io>
…#39 quant consumer) NewQuantizedLinearWithMode resolves the quant-loader registry by the config's normalised mode and builds the Linear through it; affine self-registers the default assembly (quant_affine.go). Any mode without a registered loader falls through to the built-in assembly — so this is behaviour-identical for every shipped model, and a new format (q4_0/mxfp4/nvfp4, #33) is register-plus-config. gemma4Linear is unchanged — it passes the mode, the engine resolves. arch ⊥ quant. metal pkg green; quant/linear tests pass (dispatch + unregistered-fallback). Co-Authored-By: Virgil <virgil@lethean.io>
…ComputeFor (#39) The cache's TYPE selection is already a config-driven factory (CacheComputeFor resolves the mode → NewCache); the mode→cache resolution in generate.go is generation-policy (fixed-sliding regime, per-layer replacement, sizing, cache-policy), not a flat loader. Forcing a model_registry-style cacheLoaders onto it would lose that policy, so the foundation's cache_registry.go was an over-application. The 3 factory registries are quant + mixer (new, model_registry style) + cache (the existing CacheComputeFor) — each shaped to its concern. Co-Authored-By: Virgil <virgil@lethean.io>
…(#44) Repoint the SSM-family load-time builders onto the FINAL engine-level contract (metal.RegisterMixerLoader, 324bb41) and move them OUT of gemma4 into the mixer families' own packages — a mixer is an engine feature, so its build-from-weights wiring lives with its math + scheme, not in any model package (AX-8: the lib never imports a consumer). The gemma4-side mixer_ssm.go builder (the old gemma4.RegisterMixerBuilder + gemma4Linear shape) is dropped. - pkg/metal/model/mamba2/loader.go: init() RegisterMixerLoader("mamba2", buildMamba2). Composes quant-ready in/out projections via ctx.Linear (the quant factory is already applied — the builder is quant-blind), the raw conv1d / A_log / D / dt_bias / norm via ctx.Weight, geometry from the weight shapes (nGroups=1: dInner=projOut−convDim−H, N=(convDim−dInner)/2, headDim=dInner/H), neutral ctx.Cfg.RMSNormEps. - pkg/metal/model/rwkv7/loader.go: init() RegisterMixerLoader("rwkv7", buildRWKV7). All-Linear via ctx.Linear; H from ctx.Cfg.NumAttentionHeads, K/V from the receptance/value widths over H. - Arch ⊥ quant: neither builder sees a quant format or calls gemma4Linear; no gemma4 import. A missing required tensor is a loud error, never a silent zero mixer. The scan + state-threading math (mixer.go Forward via ctx.Recurrent(), the #39 holder) is unchanged — only the registration site + weight-acquisition moved. Tests (in-package, against a synthetic metal.MixerBuildCtx with fake Linear/Weight closures): Good (loader builds → recurrent MixerCompute → forward through metal.NewRecurrentCache() with the right output shape + slot count, 2 for mamba2 / 1 for rwkv7), Bad (missing tensor → error), Ugly (self-registers from init, kind round-trips). 24/24 green across both packages. The generic config-composed model that consumes these loaders (supplying the real ctx.Linear from the quant factory) lands separately (Cladius). Closes tasks.lthn.sh/view.php?id=44 Co-Authored-By: Virgil <virgil@lethean.io>
Register each linear-attention family's load-time builder on the engine mixer-loader registry (metal.RegisterMixerLoader, on the 324bb41 final contract), in the family's OWN package alongside its scheme + compute — so a config layer_type of "gla"/"retnet"/"deltanet" resolves end to end with no model-package edit and no quant coupling. Each buildMixer composes the four quant-ready projections it gets from ctx.Linear (the quant factory is already applied — the builder never sees a quant format) plus the family-specific piece: - RetNet: derives the per-head decay schedule γ_h = 1 − 2^(−5−h). - GLA: wraps gk_proj as the GateFn (logσ of the projection — the per-key-dim log-forget gate; logσ composed stably from the metal op surface). - DeltaNet: wraps b_proj as the BetaFn (σ of the projection — per-token write strength in (0,1)). The kernel math, state threading, and L=1 decode paths are unchanged from the prior commit; this is purely the build-from-weights wiring, moved out of gemma4 to engine level. Tests (30 green across the three packages, +9 builder tests): each builder composes a mixer from a synthetic MixerBuildCtx (identity-Linear resolver), runs a 2-token Forward through a real metal.NewRecurrentCache() leaving one advanced state slot (Good), refuses a layer missing a required projection or lacking head geometry (Bad), and self-registers the family (Registered). go vet + gofmt clean. The generic config-composed model that drives these end to end is the team lead's; this lane delivers the three loaders registered + unit-green. Refs tasks.lthn.sh/view.php?id=45 Co-Authored-By: Virgil <virgil@lethean.io>
…(#46/#50) Repoint the sparse mixer builders to the FINAL engine-level contract (324bb41): each of mla/nsa/moba/gsa registers from init() via metal.RegisterMixerLoader, takes quant-ready *metal.Linear from ctx.Linear (never raw weights + a quant config), reads geometry from the neutral metal.TransformerConfig + weight-shape inference. Builders now live WITH their scheme + compute — no gemma4 import (the held gemma4/mixer_sparse.go is dropped). GSA recurrent, MLA/NSA/MoBA StateKVCache. Two real bugs the end-to-end loader tests surfaced (the kernel-only tests couldn't): - MLA: kv_b_proj outputs the CONCATENATED K+V (width 2*heads*headDim); Forward was treating the whole width as both K and V → shape mismatch → invalid array. upProjectKV now slices the K and V halves (DeepSeek-V2 single kv_b_proj) or runs distinct WUK/WUV when present. - NSA + MoBA: a chunk shorter than BlockSize (every decode step until the first full block — the COMMON case) gave nBlocks=0 → a [.,.,L,0] mask → panic. NSA now falls back to its sliding-window branch alone; MoBA to plain causal attention. Both faithful: a sub-block context has no blocks to compress/select. GATE: each loader builds from a fake-ctx, resolves by kind, errors loudly on a missing projection; forward runs (MLA shape; NSA/MoBA both short-context AND full-block paths; GSA threads Sk/Sv through the holder). All four packages green. Co-Authored-By: Virgil <virgil@lethean.io>
…46/#50) The advisor + the DeepSeek-V2 source caught a real layout bug in upProjectKV's shared-kv_b_proj path: I split it as [all-heads-K | all-heads-V] (block), but DeepSeek-V2 is PER-HEAD interleaved — kv_b_proj(...).view(B,L,heads, qk_nope_head_dim+v_head_dim) then split the LAST axis. A block split pairs head h's K with head h+1's V — silently wrong, shape-identical (the prior shape-only loader test passed on it). Fix: reshape to [B,L,heads,2*HeadDim], slice the per-head last axis into K (first HeadDim) and V (last HeadDim), flatten back. Pinned with a NUMERICAL test (TestUpProjectKV_PerHeadInterleaved): identity WUK, cKV=[1,2,3,4], heads=2 → K=[1,3], V=[2,4] (a block split would give K=[1,2] → test fails loudly). The test materialises the two slice-views before reading them back-to- back (reading sibling lazy views via .Floats() in sequence can surface a stale second; the Forward path forces eval via the downstream matmuls, so production was already evaluating correctly — verified). Co-Authored-By: Virgil <virgil@lethean.io>
…nsumer foundations (#39) LoadLinear(weights, prefix, quantCfg) — the model-agnostic projection loader that routes through NewQuantizedLinearWithMode (the quant registry stays the single quant boundary; the caller never handles a format). MixerLoaderFor(kind) exports the mixer-loader resolver for the config-composed model (a sub-package) to build each layer's mixer from the registry. Both feed the generic consumer's ctx.Linear. Co-Authored-By: Virgil <virgil@lethean.io>
…tate (#39 lifecycle)" This reverts commit ec9a7e6.
… builder registry (#39)" This reverts commit 9993754.
…ixer (#39)" This reverts commit 37edcca.
The consumer the mixer registry was built for: a generic pre-norm SwiGLU transformer whose per-layer sequence mixer is resolved at LOAD time by the kind the config declares — through metal.MixerLoaderFor, no central switch, no edit to any model file. gemma4 stays the hand-written softmax model; this is the path a softmax+linear-attention hybrid (or a transformer-shaped pure-linear-attn model) runs. It closes the "static engine operates on what the config declares, 3 separate concerns" loop — quant + mixer + cache factories now have their config-driven consumer. - pkg/metal/softmax_loader.go: the KEYSTONE. A generic softmax-attention mixer (softmaxMixer) wrapping the SDK's neutral GQAAttention, self-registered as "full_attention". Without it a composed hybrid could not build its attention layers (only the 9 FLA mixers were registered). Family-agnostic — no gemma4 import; gemma4's softmax_mixer.go stays the shared-KV/runtime-mask variant. Reads rope_theta + scale from the layer *DenseConfig via MixerBuildCtx.Extra. Implements MixerCloser to free its projections on model Close. - pkg/metal/model/composed/: ComposedModel (metal.InternalModel). buildComposed is the disk-free core (unit-testable from synthetic weights, AX-11): resolve each layer's kind (layer_types per-layer, or model_type uniform), dispatch MixerLoaderFor, compose embed + [norm→mixer→add, norm→SwiGLU→add] + norm + (tied/lm_head) output. NewCache types each layer 1:1 by the mixer's declared State() — KV cache for softmax, recurrent holder for SSM/linear-attn. Registers the "composed"/"hybrid" loadModel arch; blank-imported in speculative.go. - pkg/metal/mixer_registry.go: MixerBuildCtx gains an Extra escape hatch (mirrors MixerCtx.Extra) — softmax needs the family config's rope_theta+scale the neutral TransformerConfig does not carry; recurrent mixers ignore it. - pkg/metal/mixer.go: MixerCloser optional capability — composed Close frees a mixer's own weights when present, best-effort, never a nil-deref. - pkg/metal/linear_load.go: LoadLinear routes every tensor through ResolveWeight so the model./language_model. aliasing is handled once, not per caller. Scope (cut 1, pinned): the wiring is proven (config→registry→compose→forward shape; HETEROGENEOUS softmax+recurrent dispatch driving heterogeneous cache typing — KV vs the recurrent holder — via a fake recurrent mixer; live loadModel registration round-trips "composed"/"hybrid"; loud refusal on missing weight / unregistered kind / ambiguous config). Per-FLA-family numeric correctness + their checkpoint weight subpaths are the validate-against-a-checkpoint step (AX-9) — a real Mamba/RWKV/hybrid checkpoint is wired to this loader when validated, not before. Out of cut 1: pure-Mamba/RWKV block topology (no-MLP), embedding scaling. 9 composed tests + full tree build green; 1104 tests across the touched + FLA consumer packages, no regression.
The GatedChunk recurrence oracle tests feed [B,H,L,D] directly and only assert head 0, so they bypass projectHeads (the [B,L,H*D]→[B,H,L,D] strided view) and mergeHeads (the inverse transpose). The state-threading test drives those paths but with identity weights and identical routing on both the chunk and step sides, so a static head-misroute cancels and the test stays green — exactly the silent-layout class that shipped undetected in a sibling sparse-mixer lane. TestGla_Mixer_HeadLayout_Good gives each head distinct input channels and a distinct gate, then builds the expected output with an independent Go oracle that splits the model dimension block-contiguously (head h ⇒ channels [h·D,h·D+D)), runs the gated recurrence per head, and re-assembles block-contiguously. Verified discriminating: perturbing the projectHeads stride to an interleaved split fails only this test, while all ten existing tests pass. Co-authored-by: Hephaestus <hephaestus@lthn.ai>
The WKV7 recurrence has an independent float64 oracle (wkv7Reference) and the chunked path is checked against the sequential kernel, but Mixer.Forward's projection/reshape glue had only a prefill-vs-decode self-consistency test — which runs the same glue both sides and so is blind to a consistent layout bug (a transposed projection, a wrong per-head reshape, crossed r/k/v/a/b ports). forward_test.go reproduces only that glue in pure Go from the same weights (r/k/v/a/b projections, w = -exp(WProj), per-head split), defers the recurrence to the trusted wkv7Reference, and applies the head-merge + out-projection, then asserts the real chunked Forward matches within float32 noise. Covers both the zero-state prefill (L=3) and the decode-continuation (L=1 with prior) layouts. Closes tasks.lthn.sh/view.php?id=30 (rwkv7 numeric-Forward slice) Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Pin moba.Mixer.Forward end-to-end against a pure-Go dense-causal-softmax
reference (Q=K=V=x via identity projections), restricted to the tokens
the kept blocks expose. MoBA Forward is exactly causal softmax with a
block-sparsity mask, so the oracle recomputes that mask from first
principles — the kept set is known by construction (geometry + scores),
never read back from the mixer's own blockSelectMask, so the test is a
real oracle rather than a tautology.
Covers the Forward-level compositions the shape-only loader tests cannot
see, and the past-bug edges:
- L < BlockSize (nBlocks == 0): reduces to plain causal softmax
- L == BlockSize (nBlocks == 1): block-grid path, still causal softmax
- TopK == 0: self-block-only; a cross-block leak in the block->token
expansion would diverge here
- TopK == 1: a high-scoring past block is kept alongside the self-block
Discriminating power verified by bug injection (a wrong allow-predicate
fails the assertion). Tests gate on -tags metal_runtime; the un-tagged
go test ./... stays green.
Co-authored-by: Hephaestus <hephaestus@lthn.ai>
The RetentionChunk oracle tests feed [B,H,L,D] directly and only assert head 0, bypassing projectHeads (the [B,L,H*D]→[B,H,L,D] strided view) and mergeHeads; the state-threading test drives those paths with identity weights and identical routing on both sides, so a static head-misroute cancels. TestRetnet_Mixer_HeadLayout_Good gives each head distinct input channels and a distinct per-head decay γ (DecayLn[head]), then builds the expected output with an independent Go oracle splitting the model dimension block-contiguously (head h ⇒ channels [h·D,h·D+D)) and running the retention recurrence per head with that head's γ. The per-head decay is the extra layout lock retention has over GLA — a head swap also pairs a head's data with the wrong γ. Verified discriminating: perturbing the projectHeads stride to an interleaved split fails only this test, all nine existing tests pass. Co-authored-by: Hephaestus <hephaestus@lthn.ai>
…(#33) The quant arm of the factory trilogy gains its non-affine formats. Evidence-led (a round-trip probe ran first): MLX's mlx_quantize supports mxfp4 (the MX format — E8M0 block scale over E2M1) and nvfp4 (NVIDIA FP4), but returns a 2-array (w, scales) result because these formats carry NO zero-point — and the Go Quantize binding hardcoded affine's 3-tuple and threw the valid result as an error. q4_0 is NOT an MLX quantization mode (mlx_quantize rejects it outright); it is out of this cut — a real q4_0 needs a GGUF-specific packer, not mlx_quantize. - quantize_op.go: Quantize accepts the 2-array scale-only return (biases nil) as well as affine's 3-tuple. The dequantize + quantized-matmul paths already nil-tolerate biases (optionalArray), so the binding's array-count assumption was the only blocker to the FP4 quantise direction. affine is unchanged (still the 3-tuple) — backward compatible. - quant_fp4.go: register mxfp4 + nvfp4 loaders (mirrors quant_affine.go), making the quant registry the authoritative supported-format set rather than leaving these to the generic fallback. The loader assembles the *Linear; the mode string drives the MLX kernel (the affine pattern). - quant_schemes_test.go: the numeric oracle — quantise→dequantise round-trips within a 4-bit step (measured affine 0.075, mxfp4 0.100, nvfp4 0.094 on the [-0.6,0.7] test weights); registration is first-class; q4_0's MLX-unsupported boundary is pinned in a test so its absence reads as deliberate, not forgotten. pkg/metal green (1052 tests), full tree builds.
…ally The DeltaRuleChunk oracle tests feed [B,H,L,D] directly and only assert head 0, bypassing projectHeads (the [B,L,H*D]→[B,H,L,D] strided view) and mergeHeads; the state-threading test drives those paths with identity weights and identical routing on both sides, so a static head-misroute cancels. TestDeltanet_Mixer_HeadLayout_Good gives each head distinct input channels and a distinct per-token write strength β, then builds the expected output with an independent Go oracle (deltaReference, which L2-normalises keys exactly as the kernel does) splitting the model dimension block-contiguously (head h ⇒ channels [h·D,h·D+D)) and running the delta-rule recurrence per head. Because keys are L2-normalised over each head's own D-block, an interleaved split also changes a head's key direction and hence its read. Verified discriminating: perturbing the projectHeads stride to an interleaved split fails only this test, all eleven existing tests pass. Co-authored-by: Hephaestus <hephaestus@lthn.ai>
The NSA selection branch's keep-all path (selectionMask with
selectBlocks >= nBlocks) returned the score-laden `scored` tensor
instead of a clean 0/-inf keep-mask. Because the caller ADDS that mask to
the attention logits, the per-block ranking score leaked in as an
additive bias, sharpening the selection-branch softmax toward
high-scoring blocks. This fires whenever selectBlocks >= nBlocks =
L/BlockSize — i.e. across the entire short- and medium-context regime
(e.g. SelectBlocks=16, BlockSize=64 → every sequence up to ~1024
tokens), not a degenerate edge. The top-n path already returned a clean
0/-inf mask via WhereScalarArray; the two paths were inconsistent and
keep-all was wrong. Fix: free `scored` and return causalMask.Clone()
(causalMask is already exactly the 0/-inf keep-mask).
Found by a new Forward-level numeric oracle: the existing
TestSelectionMask_* tests all use selectBlocks < nBlocks (the top-n
path), so keep-all was never exercised.
Tests added (gate on -tags metal_runtime; un-tagged go test ./... stays
green):
- forward_oracle_test.go: pin nsa.Mixer.Forward against an independent
pure-Go reference matched to NSA's three-branch gated-blend mechanism
— short-seq L<BlockSize (routes to sliding, nBlocks==0 past-bug
edge), per-branch isolation via a constant-bias gate projection
(sliding / compression / selection), and a full three-branch blend
weighted by the exact sigmoid gates (the test that caught the bug).
The selection branch is exercised in the keep-all regime so it has a
clean reproducible reference, never read back from the mixer.
- nsa_test.go: TestSelectionMask_KeepAllIsCleanMask_Good pins the
keep-all branch to a clean 0/-inf mask directly (distinct block
scores make a score-leak loud).
Both new tests verified to fail on the pre-fix code and pass after.
Co-authored-by: Hephaestus <hephaestus@lthn.ai>
The load-bearing heart of the P-phase on-disk fuse: FuseLoRAIntoWeights folds a
trained adapter's deltas into a dense base weights MAP (not a live model), so a
fused checkpoint serialises straight back out with SaveSafetensors — no reverse
model-walk. Builds on the existing primitives (Matmul/MulScalar/Add, the
adapter's {layer}.lora_a/.lora_b format, Scale = alpha/rank).
- Each targeted base weight W → W + (B·A)·scale (dense); every other tensor
carried through unchanged; the fused-layer list returned for diagnostics.
- Dense-base only: a quantized base is refused loud (a fused layer is dense, and
one config-level quantization block cannot mix fused-dense with quantized
neighbours — dequant-merge-then-requant the whole model is a separate path).
- Loud, not silent: a delta with no base weight, a quantized base, or a
base/delta shape mismatch is an error, never a wrong-shape or skipped merge.
Numeric oracle test (rank 1, computed by hand independently of the impl) pins
W+(B·A)·2; three guards pin the loud-failure paths. Orchestration
(FuseModelDir: read base+adapter dirs, save fused dir, copy config/tokenizer) and
the cmd/mlx fuse verb follow on this core.
pkg/metal green.
Pin mla.Mixer.Forward end-to-end against a pure-Go latent-expansion reference: decompress per-head K/V from the KV latent with the per-head-interleaved split, run causal SDPA per head, merge, project out. TestUpProjectKV already pins the split in isolation at HeadDim=1; this exercises the split WIDTH flowing through splitHeads -> attendLatent -> OProj at heads=2, HeadDim=2 — the composition the kv_b_proj per-head-interleaved-vs-block-concatenated bug actually corrupts. Construction uses identity WDQ/WDKV/WUK so the latents are exactly x, a column-selector WUQ for a traceable query, and identity OProj; distinct per-column sentinels make a mis-routed split numerically loud. The oracle reads K/V with the same per-head-interleaved layout independently of the mixer. Discriminating power verified: a block-concatenated split in the oracle diverges by ~2.0 (pairs head 0's K with head 1's V), where a shape-only test would still pass. Gates on -tags metal_runtime; the un-tagged go test ./... stays green. Co-authored-by: Hephaestus <hephaestus@lthn.ai>
The SSD recurrence has an independent float64 oracle (scanReference) and the chunked path is checked against the sequential kernel, but Mixer.Forward's block assembly — the in-proj z|xBC|dt split, the causal depthwise conv, the group->heads expansion, the dt activation, the gated norm — had only a prefill-vs-decode self-consistency test, which runs the same glue both sides and so cannot see a consistent layout bug (a wrong split offset, the B/C ports crossed, a group routed to the wrong heads). That is the bug class a sibling sparse-mixer lane shipped. forward_test.go reproduces only that glue in pure Go from the same weights, encoding the INTENDED layout with explicit indices (not transcribing the op sequence), defers the recurrence to the trusted scanReference, and applies the gated norm + out-proj, then asserts the real chunked Forward matches within 1e-4. Two geometries: G=1 (the package fixture's degenerate broadcast) plus a PROPER H=4,G=2 group count whose repeat=2 map is the only case that can catch a group-routing bug. The B/C in-proj rows carry a strong per-group scale so a swapped group produces gross error (verified: a reversed group map fails by ~0.026, a reversed conv kernel by ~0.009 — both >>1e-4), keeping the test from rubber-stamping a layout bug the way the shape-only test it replaces would. The gated-norm component is transcription (mirrors the mixer's SiLU(z)*RMSNorm(y) norm-before-gate form), so this pins layout, not gate-order math. The existing prefill-vs-decode test is kept — it guards the conv-ring + ssm-state threading this static oracle does not. Closes tasks.lthn.sh/view.php?id=30 (mamba2 numeric-Forward slice) Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Add one Forward-level absolute-value test for gsa.Mixer.Forward against a pure-Go scalar slot-recurrence reference that INCLUDES the SiLU output gate and the output projection. The recurrence kernel is already pinned independently, and TestForward_ChunkedDecodeMatchesSinglePass is a consistency check (single-pass == chunked) that is invariant to wiring/gate bugs which cancel across both calls. This test fixes the absolute two-token output, so a dropped/transposed gate, a wrong gate operand, or an OProj slip is caught. All-identity mixer (q=k=v=f=gate=x) makes the math determined solely by the gated slot recurrence + SiLU gate; the oracle recomputes both in pure Go. Discriminating power verified: dropping the SiLU gate from the oracle diverges by ~0.16. Gates on -tags metal_runtime; the un-tagged go test ./... stays green. Co-authored-by: Hephaestus <hephaestus@lthn.ai>
The first rwkv7 Forward oracle (c9de61b) passed on correct code but ALSO passed a crossed a↔b or r↔k port — a mutation check showed only ~4e-9 divergence under the original tinyMixer fixture + 2e-3 tolerance, well inside tolerance. That is the shape-only trap one rung up: green, but not load-bearing for the layout bugs the test exists to catch. Two causes, both fixed: (1) tinyMixer's near-uniform tiny projections make every port's contribution indistinguishable — replaced with oracleMixer, a fixture with a DISTINCT magnitude scale per r/w/k/v/a/b port. (2) the a/b learning-rate transition b⊗(aᵀS) is zero at step 0 and small for the first steps, so an a↔b swap only diverges once the state accumulates — the Good prefill is now L=6, not 3. Tolerance tightened 2e-3 → 1e-4 to match the mamba2 oracle and the brief. Verified by mutation: a reversed a↔b port now fails by ~0.05, a reversed r↔k port by ~0.23 — both >>1e-4 (was ~4e-9). The residual on correct code is ~7e-12. Closes tasks.lthn.sh/view.php?id=30 (rwkv7 numeric-Forward hardening) Co-authored-by: Hephaestus <hephaestus@lthn.ai>
TestForward_TopKKeepsScoredPastBlock_Good has a single strictly-past block (nBlocks=2, self=block1), so keep-top-1 and keep-all-past coincide and the top-K RANKING is never exercised. Add TestForward_TopKDropsLowerPastBlock_Good: three blocks (L=6, BlockSize=2) where a block2 query has two past blocks, both aligned with the query so both would carry real softmax mass, and top-1 must keep the higher-mean-scoring block0 while dropping block1. A keep-all-past or mis-ranked selection now diverges (verified by injecting keep-all-past: the test fails ~0.007 > 1e-4). This brings the most complex oracle (mobaSelectionOracle, whose ranking was previously untested) to the same discriminating-power standard as the rest. Gates on -tags metal_runtime. Co-authored-by: Hephaestus <hephaestus@lthn.ai>
…(#35) The serializer half of the P-phase fuse, on top of the fuse core: read a dense base model dir + a trained LoRA adapter dir (engine format: adapter.safetensors + adapter_config.json, alpha/rank scaling), fold the adapter in at the weights-map level, write the fused dense model.safetensors to outDir, and carry the base's servable sidecars (config.json + tokenizer files) across. A stale safetensors shard index is intentionally not copied (the output is a single re-consolidated file). Reuses the existing primitives end to end: LoadModelWeights (base), parseAdapterConfig + loadAdapterWeights (the same loaders adapter-resume uses), FuseLoRAIntoWeights (the tested core), SaveSafetensors (write). Refuses loud when the adapter matches no base layer (an empty fuse is a config error, not a silent base-identical copy). Hardening found by the round-trip test: FuseModelDir clears the sticky, process-global MLX LastError at entry. LoadModelWeights reads LastError after a *successful* LoadSafetensors, so a benign error left by a prior unrelated op would falsely fail this fresh load — reproduced when the fuse runs after another MLX operation. Now robust regardless of prior engine state. Round-trip test writes synthetic base + adapter dirs, fuses, reloads the output, and asserts W+(B·A)·scale survived disk + config.json carried across; a no-match guard pins the loud-refusal path. pkg/metal green.
Wires metal.FuseModelDir to the CLI, completing #35 (fuse verb + serializer). lthn-mlx fuse -base <dir> -adapter <dir> -out <dir> Folds a trained LoRA adapter into a dense base and writes the fused model dir; the serving artefact afterwards needs no adapter. Thin verb over the tested serializer — flag validation + clean exit codes (2 = missing flags, 1 = FuseModelDir error, 0 = fused N layers). Listed under "Transform a model" in the usage. Tests pin the missing-flag and no-model exit paths; the fuse numerics + on-disk round-trip are covered in pkg/metal. This closes the P-phase loop end to end: train a LoRA (sft/ssd) -> adapter.Save -> fuse -> a servable dense model dir, no separate adapter at serve time.
The composed runner was wired but orphaned: speculative.go (the serve/generate
package) blank-imported `composed` but none of the FLA mixer families, so at
serve time MixerLoaderFor("mamba2"/"gla"/…) returned nil and a config-composed
Mamba/RWKV/linear-attn/sparse hybrid was refused at load — the engine's mixer
registry carried only the generic softmax. The 9 numerically-validated mixers sat
in the tree unreachable from the running engine.
Blank-import mamba2/rwkv7/gla/retnet/deltanet/gsa/nsa/moba/mla so their loaders
self-register. A test in the serve package asserts all 10 kinds (softmax + 9 FLA)
now resolve. Inert for existing models (registry is consulted only when a config
declares that layer kind); a normal gemma4 serve never touches them.
This is half of closing the composed runner's reason. The remaining half is
weight-subpath resolution: the FLA loaders ask for unqualified leaves
(ctx.Linear("in_proj")) while a real checkpoint nests them under a per-model
mixer sublayer (model.layers.N.mixer.in_proj) — that mapping is model-specific
and wants a target hybrid checkpoint to pin + validate against.
Closes Gap B of the composed-runner reason: it can now resolve a real hybrid's mixer weights, where the mixer nests under a model-specific sublayer (model.layers.N.mixer.in_proj, or .self_attn., or .mamba.) that varies by model, not by family — so a family loader cannot own its subpath. The composed model discovers each layer's mixer sublayer FROM the checkpoint: the mixer and the MLP nest sub-projections (>=3 components past the layer); the norms do not; excluding the MLP leaves the mixer. This is not a candidate-guess — it reads the sublayer actually present, and FLA leaf names (in_proj/A_log/receptance) are unique within a layer, so resolution is unambiguous. Exactly one → use it; none → bare leaves (no nesting); TWO OR MORE → loud refusal, never a random map-order pick (the silent wrong-geometry trap this whole pass has been killing). All mixer loaders now ask for BARE leaves — softmax_loader.go drops its self_attn. qualification to match the FLA loaders, and the consumer owns the layout. A softmax+recurrent hybrid resolves self_attn for one layer and mixer for another in one model (TestComposed_SubpathResolution_Good); a layer with two candidate sublayers is refused (TestComposed_AmbiguousSubpath_Bad). BOUNDARY (honest): this proves the MECHANISM against the standard single-mixer layout. End-to-end correctness against a real hybrid checkpoint's exact naming + numerics is still the checkpoint-gated step (with the mamba2 gate-order, #60). The composed runner + 9 registered mixers are now wired end to end — config → registry → discovered-subpath load → forward — pending a real checkpoint to run. composed 11 tests green; pkg/metal 1058 green; full tree builds.
|
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
2 participants
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.
@coderabbitai summary
Summary by CodeRabbit
New Features
Improvements
Documentation