Skip to content

test(inkling): enforce cache capacity invariance - #784

Merged
JustVugg merged 1 commit into
JustVugg:devfrom
dyKiU:bugfix/inkling-cache-capacity-regression
Aug 2, 2026
Merged

test(inkling): enforce cache capacity invariance#784
JustVugg merged 1 commit into
JustVugg:devfrom
dyKiU:bugfix/inkling-cache-capacity-regression

Conversation

@dyKiU

@dyKiU dyKiU commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Add regression coverage for the cache-capacity fix at capacities 1, 2, and 8.

Add a runtime expert-ID guard so cache mismatches fail clearly instead of using the wrong expert
weights and producing junk output.

Validation

  • make -C c check
  • CUDA testing not applicable; no CUDA changes
  • Performance validation not applicable; no performance claims

Compatibility

  • The default CPU build remains dependency-free
  • No model files, generated binaries, or benchmark artifacts are included

@JustVugg JustVugg added bug Difetto verificato nel codice quality Qualità del modello / quantizzazione labels Aug 2, 2026
@JustVugg
JustVugg force-pushed the bugfix/inkling-cache-capacity-regression branch from 920bb8f to 2f8957c Compare August 2, 2026 22:18
@JustVugg

JustVugg commented Aug 2, 2026

Copy link
Copy Markdown
Owner

I rebased this for you — the conflict was mine to fix, not yours.

#786 landed an hour ago and added a step to the same inkling-oracle job, right where your cap sweep sits, so a fifteen-line PR from a first contribution ended up CONFLICTING because of a merge I did. Making you resolve that would have been backwards. Your commit is unchanged; only the ci.yml hunk was re-applied, keeping both changes:

for cap in 1 2 8; do
  SNAP=tiny_inkling ./inkling "$cap" 0 tiny_inkling/ref_inkling.json
done
- name: KV prefix reuse is token-identical (serve, two turns)

Also: CI never ran on your branch at all. GitHub holds workflows from a first-time contributor's fork in action_required, and nothing surfaced that to either of us. Approved now.

On the change itself

Both halves are the right instinct.

The capacity sweep. A capacity-1 cache evicts on every routed expert, which is where slot bookkeeping breaks; a capacity-8 run on a tiny fixture may never evict at all and would pass over the bug forever. Testing the boundary rather than the comfortable middle is the harder discipline and the one that catches things.

The expert-id guard is the part I would defend hardest if anyone objects to the cost:

if (e->eid != idx[(int64_t)s*K + kk]) { ... exit(1); }

Serving the wrong expert's weights does not crash — it produces fluent, confident, wrong output, and nothing downstream can tell. That failure class is the same one we spent today on from the other end: a KV prefix reused at the wrong length answers from a different conversation and still reads plausibly. A loud refusal beats junk that looks like text. The per-token cost is O(S·K) integer comparisons against a routed-expert matmul — not measurable.

Green CI and this merges. Thank you — and sorry the first thing your PR met was our own silence and then our own conflict.

@JustVugg
JustVugg merged commit 44d7958 into JustVugg:dev Aug 2, 2026
13 checks passed
@dyKiU
dyKiU deleted the bugfix/inkling-cache-capacity-regression branch August 4, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Difetto verificato nel codice quality Qualità del modello / quantizzazione

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants