harden(kv): warn on silent rotation disable + document cache-shift constraint (#70, #71) - #75
Merged
Merged
Conversation
…straint (#70, #71) #70: when KV rotation is requested but a layer's head_dim is not a power of two (<=512), the FWHT guard silently no-ops and you get the unrotated cache with no signal. Warn once at session creation naming the head_dim. (No currently-supported model triggers it; defensive for unusual-head_dim archs.) #71: rotation + packed INT4 store K post-RoPE and rotated, safe only because geist never re-bases cached positions. Document the constraint atop kv_store.c and in ARCHITECTURE.md so a future KV context-shift feature does not silently corrupt the cache (it must un-rotate/unpack before re-RoPE-ing).
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
1 participant
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.
Two small robustness guards on the rotation / packed-INT4 path, from the #65 follow-ups.
#70 — when
GEIST_KV_ROT(or the INT4 default) is active but a layer'shead_dimis not a power of two (or > 512), the FWHT guard silently no-ops: packing/quant still run, so you get the unrotated (worse) cache with no signal. Now warns once at session creation, naming the head_dim. No currently-supported model (Gemma4 128/256/512, BitNet 128) triggers it — this is defensive for future unusual-head_dim archs.ponytail:note marks thegeist_log_callback_tupgrade path (the arch layer has no log routing yet).#71 — the rotation and packed INT4 store K post-RoPE and rotated; this is only correct because geist never re-bases cached positions (sliding window masks, doesn't re-RoPE). A future KV context-shift would silently corrupt the cache unless it un-rotates/unpacks before re-RoPE-ing. Documented atop
kv_store.cand inARCHITECTURE.md(llama.cpp#21038 had to add explicit cache-shift support for the same reason).Closes #70, closes #71. No behavior change on supported models (verified: no spurious warning, generation unchanged).
🤖 Generated with Claude Code