Don't trust a converted checkpoint's config for the stop set

The engine armed its stop tokens from config.json's eos_token_id and nothing
else. That trusts metadata written by third-party conversion tooling, which is
a thing we already know goes wrong: the README documents a mirror shipping
int4 MTP heads that silently give 0% draft acceptance. GLM-5.2 declares THREE
eos ids (<|endoftext|>, <|user|>, <|observation|>); a converter that rewrites
config.json with a reduced list leaves the engine stopping on fewer tokens
than the model emits, and the missed ones get detokenized and printed into the
chat as literal text while generation runs past the end of the turn.

Two independent defenses:

  - eos_token_id is now unioned with generation_config.json, which is
    HuggingFace's authority for generation (config.json often carries a
    partial legacy copy). An extra stop is harmless; a missing one is not.

  - every added-token the TOKENIZER marks "special":true is armed as a stop,
    whatever the configs say. Those are control tokens (<|user|>, <|assistant|>,
    <sop>, [gMASK], the image/video/audio markers) and are never legitimate
    content in a reply -- GLM itself lists three of them as official eos.
    <think>/<tool_call>/<arg_key> are "special":false and are deliberately NOT
    swept up: they are real output. tok.h was parsing added_tokens but throwing
    the "special" flag away, so the distinction wasn't available to anyone.

On the real per-row checkpoint this takes the armed set from 3 to 18:
  [stop] 18 stop tokens: 154820 154827 154829 154821 ... (15 from the
  tokenizer's special set)

Honesty about scope: this is hygiene for a class of bug, NOT a fix for the
trailing-junk report on #298 that prompted it. I hypothesised @woolcoxm's g64
checkpoint had lost eos ids in conversion; he checked, and it hadn't -- his
config arms all three correctly. The emit path is also innocent: is_stop() is
checked BEFORE emit() at every one of the four call sites (4215, 4256, 4908,
4987), so a correctly-armed stop cannot be printed. His trailing junk is still
unexplained and is more likely quantization noise. What this commit buys is
that a checkpoint we don't control cannot leak control tokens into a reply,
which was true before and is not now.

tests/test_stops.c covers both defenses: the union, a missing
generation_config.json, BOTH configs mutilated (the tokenizer still stops all
five control tokens while leaving <think> alone), and T=NULL (the validation
path keeps config-only behaviour).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
JustVugg
2026-07-16 17:59:34 +02:00
parent ac7103fe9c
commit 6de32c55f6
4 changed files with 196 additions and 8 deletions
+4 -1
View File
@@ -166,7 +166,7 @@ else
PYTHON ?= python3
endif
CUDA_OBJ =
TEST_BINS = tests/test_json$(EXE) tests/test_st$(EXE) tests/test_tier$(EXE) tests/test_grammar$(EXE) tests/test_schema_gbnf$(EXE) tests/test_decode_batch$(EXE) tests/test_idot$(EXE) tests/test_i4_grouped$(EXE) tests/test_kv_alloc$(EXE) tests/test_i4_acc512$(EXE) tests/test_compat_direct$(EXE)
TEST_BINS = tests/test_json$(EXE) tests/test_st$(EXE) tests/test_tier$(EXE) tests/test_grammar$(EXE) tests/test_schema_gbnf$(EXE) tests/test_decode_batch$(EXE) tests/test_idot$(EXE) tests/test_i4_grouped$(EXE) tests/test_stops$(EXE) tests/test_kv_alloc$(EXE) tests/test_i4_acc512$(EXE) tests/test_compat_direct$(EXE)
ifneq (,$(LINUX))
TEST_BINS += tests/test_uring$(EXE)
endif
@@ -300,6 +300,9 @@ tests/test_idot$(EXE): tests/test_idot.c glm.c st.h uring.h json.h tok.h tok_uni
tests/test_i4_grouped$(EXE): tests/test_i4_grouped.c glm.c st.h uring.h json.h tok.h tok_unicode.h compat.h grammar.h tier.h
$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
tests/test_stops$(EXE): tests/test_stops.c glm.c st.h uring.h json.h tok.h tok_unicode.h compat.h grammar.h tier.h
$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
tests/test_kv_alloc$(EXE): tests/test_kv_alloc.c glm.c st.h json.h tok.h tok_unicode.h compat.h grammar.h tier.h
$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
+52 -7
View File
@@ -1277,6 +1277,35 @@ static void load_cfg(Cfg *c, const char *snap){
if(eo){ if(eo->t==J_NUM) c->stop_ids[c->n_stop++]=(int)eo->num;
else if(eo->t==J_ARR) for(int i=0;i<eo->len && c->n_stop<8;i++)
c->stop_ids[c->n_stop++]=(int)eo->kids[i]->num; }
/* generation_config.json e' il file AUTOREVOLE per la generazione secondo HuggingFace:
* config.json ne porta spesso una copia legacy o parziale. Un tool di conversione che
* rigenera un config.json ridotto lascia il motore fermo su MENO stop del dovuto, e i
* token di controllo che restano finiscono stampati in chat come testo (woolcoxm, #298:
* "the stop token being printed to chat", verificato sui token id). Unione dei due:
* uno stop in piu' non fa danno, uno in meno si' -- e chi converte i pesi non siamo noi.
* EN: generation_config.json is HF's authority for generation; config.json often carries
* a partial legacy copy. Union both -- an extra stop is harmless, a missing one is not. */
{ char gp[2100]; snprintf(gp,sizeof(gp),"%s/generation_config.json",snap);
FILE *gf=fopen(gp,"rb"); /* assente = nessun problema: e' opzionale */
if(gf){
fseek(gf,0,SEEK_END); long gn=ftell(gf); fseek(gf,0,SEEK_SET);
if(gn>0){
char *gb=malloc(gn+1); size_t gg=fread(gb,1,gn,gf); gb[gg]=0;
char *ga=NULL; jval *gr=json_parse(gb,&ga);
jval *ge=gr?json_get(gr,"eos_token_id"):NULL;
if(ge){
int add[8], na=0;
if(ge->t==J_NUM) add[na++]=(int)ge->num;
else if(ge->t==J_ARR) for(int i=0;i<ge->len && na<8;i++) add[na++]=(int)ge->kids[i]->num;
for(int i=0;i<na && c->n_stop<8;i++){
int dup=0; for(int j=0;j<c->n_stop;j++) if(c->stop_ids[j]==add[i]) dup=1;
if(!dup) c->stop_ids[c->n_stop++]=add[i];
}
}
free(ga); free(gb);
}
fclose(gf);
} }
/* DSA lightning indexer: parametri + tipo per-layer (lista esplicita o formula freq/offset) */
c->index_topk=gi(r,"index_topk"); c->index_nh=gi(r,"index_n_heads"); c->index_hd=gi(r,"index_head_dim");
{ jval *it=json_get(r,"indexer_types");
@@ -4107,18 +4136,34 @@ static int pick_tok(const float *lo, int V, int ban){
/* stop-set attivo (popolato da run_text/run_serve dal config; vuoto in validazione,
* dove si genera un numero fisso di token da confrontare con l'oracolo) */
static int g_stop[9], g_nstop=0;
static int g_stop[64], g_nstop=0; /* config eos + ogni added-token "special" del tokenizer */
static void repin_pass_limit(Model *m,int limit);
static void repin_pass(Model *m){ repin_pass_limit(m,16); }
static inline int is_stop(int t){ for(int i=0;i<g_nstop;i++) if(t==g_stop[i]) return 1; return 0; }
static void stops_arm(const Cfg *c, int tok_eos){
/* T=NULL -> solo gli stop del config (validazione/oracolo, dove il tokenizer non serve). */
static void stops_arm_tok(const Cfg *c, int tok_eos, Tok *T){
g_nstop=0;
for(int i=0;i<c->n_stop;i++) g_stop[g_nstop++]=c->stop_ids[i];
if(tok_eos>=0 && !is_stop(tok_eos)) g_stop[g_nstop++]=tok_eos;
for(int i=0;i<c->n_stop && g_nstop<64;i++) g_stop[g_nstop++]=c->stop_ids[i];
if(tok_eos>=0 && !is_stop(tok_eos) && g_nstop<64) g_stop[g_nstop++]=tok_eos;
int nsp=0;
/* DIFESA IN PROFONDITA' (woolcoxm, #298): il tokenizer marca "special":true i token di
* CONTROLLO -- <|user|>, <|assistant|>, <|observation|>, <sop>, [gMASK], i marker
* image/video/audio. Nessuno di questi e' contenuto legittimo di una risposta: se il
* modello ne emette uno, il turno e' finito (infatti GLM ne elenca tre fra gli eos
* ufficiali). Senza questo, uno di quei token non elencato nel config veniva
* DETOKENIZZATO E STAMPATO IN CHAT come testo, e la generazione proseguiva oltre la
* fine reale -- l'"added stuff on the end" riportato su un checkpoint convertito.
* Fidarsi del config di pesi convertiti da terzi e' precisamente cio' che non
* possiamo controllare; il flag del tokenizer lo possiamo leggere.
* NB: <think>/<tool_call>/<arg_key> hanno "special":false e restano contenuto vero. */
if(T) for(int id=0; id<T->n_ids && g_nstop<64; id++)
if(T->id_special[id] && !is_stop(id)){ g_stop[g_nstop++]=id; nsp++; }
fprintf(stderr,"[stop] %d stop tokens:",g_nstop);
for(int i=0;i<g_nstop;i++) fprintf(stderr," %d",g_stop[i]);
if(nsp) fprintf(stderr," (%d from the tokenizer's special set)",nsp);
fprintf(stderr,"\n");
}
static void stops_arm(const Cfg *c, int tok_eos){ stops_arm_tok(c,tok_eos,NULL); }
/* decode greedy con SELF-SPECULATION n-gram: LOSSLESS (output identico al greedy puro).
* Ogni forward verifica fino a g_draft token proposti dal contesto: i token accettati
@@ -4399,7 +4444,7 @@ static void run_text(Model *m, const char *snap, const char *prompt, int ngen){
Cfg *c=&m->c; char tkp[2048]; snprintf(tkp,sizeof(tkp),"%s/tokenizer.json",snap);
Tok T; tok_load(&T,tkp);
int eos=tok_id_of(&T,"<|endoftext|>");
stops_arm(&m->c, eos);
stops_arm_tok(&m->c, eos, &T);
grammar_setup(&T); /* metodo F: GRAMMAR=file.gbnf (#48) */
if(g_temp<0) g_temp=0.7f; /* auto: 0.7, NON l'1.0 ufficiale — la coda della
* distribuzione int4 e' rumore di quantizzazione */
@@ -4869,7 +4914,7 @@ static int mux_submit(Model *m, Tok *T, ServeCtx *ctx, ServeReq *req, int nctx,
static void run_serve_mux(Model *m, const char *snap){
char tkp[2048]; snprintf(tkp,sizeof(tkp),"%s/tokenizer.json",snap);
Tok T; tok_load(&T,tkp); int eos=tok_id_of(&T,"<|endoftext|>"); stops_arm(&m->c,eos);
Tok T; tok_load(&T,tkp); int eos=tok_id_of(&T,"<|endoftext|>"); stops_arm_tok(&m->c,eos,&T);
g_draft=0; /* one scheduler owns every forward; MTP/speculation is not ragged-safe */
int maxctx=getenv("CTX")?atoi(getenv("CTX")):4096;
int nctx=getenv("KV_SLOTS")?atoi(getenv("KV_SLOTS")):1;
@@ -4970,7 +5015,7 @@ static void run_serve(Model *m, const char *snap){
char tkp[2048]; snprintf(tkp,sizeof(tkp),"%s/tokenizer.json",snap);
Tok T; tok_load(&T,tkp);
int eos=tok_id_of(&T,"<|endoftext|>");
stops_arm(&m->c, eos);
stops_arm_tok(&m->c, eos, &T);
grammar_setup(&T); /* metodo F: GRAMMAR=file.gbnf (#48) */
if(g_temp<0) g_temp=0.7f; /* auto: 0.7, NON l'1.0 ufficiale — la coda della
* distribuzione int4 e' rumore di quantizzazione */
+132
View File
@@ -0,0 +1,132 @@
/* Stop-token arming: the engine must not depend on a converted checkpoint's
* config.json being complete.
*
* Why this test exists (#298, #307): GLM-5.2 declares THREE eos ids
* (<|endoftext|>, <|user|>, <|observation|>). A conversion tool that rewrites
* config.json with a reduced eos_token_id list leaves the engine stopping on
* fewer tokens than the model actually emits -- and the ones it misses get
* DETOKENIZED AND PRINTED INTO THE CHAT as literal text, with generation
* continuing past the real end of the turn. @woolcoxm hit exactly this on a
* g64 checkpoint and confirmed it by comparing token ids.
*
* Two independent defenses, one test each:
* 1. eos_token_id is unioned from generation_config.json (HuggingFace's
* authority for generation) as well as config.json.
* 2. every added-token the TOKENIZER marks "special":true is a stop, whatever
* the configs say. Those are control tokens (<|user|>, <sop>, ...) and are
* never legitimate content. <think>/<tool_call> are "special":false and
* must NOT be swept up -- they are real output.
*
* Defense 2 is what makes this robust against checkpoints we don't control:
* even with BOTH configs mutilated, a control token cannot leak into a reply. */
#define main coli_glm_main_unused
#include "../glm.c"
#undef main
static const char *TOKJSON =
"{\"model\":{\"vocab\":{\"a\":0,\"b\":1,\"c\":2},\"merges\":[[\"a\",\"b\"]]},"
" \"added_tokens\":["
" {\"id\":100,\"content\":\"<|endoftext|>\",\"special\":true},"
" {\"id\":101,\"content\":\"<|user|>\",\"special\":true},"
" {\"id\":102,\"content\":\"<|observation|>\",\"special\":true},"
" {\"id\":103,\"content\":\"<|assistant|>\",\"special\":true},"
" {\"id\":104,\"content\":\"<sop>\",\"special\":true},"
" {\"id\":110,\"content\":\"<think>\",\"special\":false},"
" {\"id\":111,\"content\":\"<tool_call>\",\"special\":false}"
"]}";
/* minimal config.json that survives load_cfg's range validation */
static void write_cfg(const char *dir, const char *fname, const char *eos_json){
char p[512]; snprintf(p,sizeof(p),"%s/%s",dir,fname);
FILE *f=fopen(p,"w"); if(!f){ perror(p); exit(1); }
fprintf(f,"{\"hidden_size\":64,\"num_hidden_layers\":2,\"num_attention_heads\":4,"
"\"n_routed_experts\":8,\"num_experts_per_tok\":2,\"moe_intermediate_size\":32,"
"\"intermediate_size\":64,\"first_k_dense_replace\":1,\"q_lora_rank\":0,"
"\"kv_lora_rank\":16,\"qk_nope_head_dim\":8,\"qk_rope_head_dim\":8,"
"\"v_head_dim\":8,\"n_shared_experts\":1,\"vocab_size\":200,"
"\"n_group\":1,\"topk_group\":1,\"rope_theta\":10000.0");
if(eos_json) fprintf(f,",\"eos_token_id\":%s",eos_json);
fprintf(f,"}\n"); fclose(f);
}
static void write_tok(const char *dir){
char p[512]; snprintf(p,sizeof(p),"%s/tokenizer.json",dir);
FILE *f=fopen(p,"w"); if(!f){ perror(p); exit(1); }
fputs(TOKJSON,f); fclose(f);
}
static void rm_file(const char *dir, const char *fname){
char p[512]; snprintf(p,sizeof(p),"%s/%s",dir,fname); remove(p);
}
static int expect(const char *what, int id, int want){
int got=is_stop(id);
if(got!=want){ fprintf(stderr," FAIL %s: token %d stop=%d, expected %d\n",what,id,got,want); return 1; }
return 0;
}
int main(void){
int fail=0;
char dir[]="/tmp/coli_stops_XXXXXX";
if(!mkdtemp(dir)){ perror("mkdtemp"); return 1; }
write_tok(dir);
char tkp[512]; snprintf(tkp,sizeof(tkp),"%s/tokenizer.json",dir);
Tok T; tok_load(&T,tkp);
printf("test_stops: stop arming vs incomplete checkpoint metadata\n");
/* 1. the tokenizer's "special" flag must survive loading, and must NOT
* swallow <think>/<tool_call>, which are real content. */
if(!T.id_special[101]){ fprintf(stderr," FAIL <|user|> not marked special\n"); fail=1; }
if(!T.id_special[104]){ fprintf(stderr," FAIL <sop> not marked special\n"); fail=1; }
if(T.id_special[110]){ fprintf(stderr," FAIL <think> wrongly marked special\n"); fail=1; }
if(T.id_special[111]){ fprintf(stderr," FAIL <tool_call> wrongly marked special\n"); fail=1; }
if(!fail) printf(" tokenizer: special flag parsed, <think>/<tool_call> excluded ok\n");
/* 2. config.json mutilated to one eos, generation_config.json intact:
* the union must recover all three. This is @woolcoxm's case. */
{ Cfg c; memset(&c,0,sizeof c);
write_cfg(dir,"config.json","[100]");
write_cfg(dir,"generation_config.json","[100,101,102]");
load_cfg(&c,dir);
if(c.n_stop!=3){ fprintf(stderr," FAIL union: expected 3 stops, got %d\n",c.n_stop); fail=1; }
else printf(" config eos=[100] + generation_config=[100,101,102] -> 3 stops ok\n"); }
/* 3. no generation_config.json at all: must not crash, config alone wins */
{ Cfg c; memset(&c,0,sizeof c);
write_cfg(dir,"config.json","[100,101,102]");
rm_file(dir,"generation_config.json");
load_cfg(&c,dir);
if(c.n_stop!=3){ fprintf(stderr," FAIL no-genconfig: expected 3, got %d\n",c.n_stop); fail=1; }
else printf(" generation_config.json absent -> config alone, no crash ok\n"); }
/* 4. BOTH configs mutilated: the tokenizer's special set must still stop the
* turn on every control token, and must still leave <think> alone. */
{ Cfg c; memset(&c,0,sizeof c);
write_cfg(dir,"config.json","[100]");
write_cfg(dir,"generation_config.json","[100]");
load_cfg(&c,dir);
stops_arm_tok(&c,-1,&T);
fail|=expect("endoftext (config)", 100,1);
fail|=expect("<|user|> (tokenizer)", 101,1);
fail|=expect("<|observation|>", 102,1);
fail|=expect("<|assistant|>", 103,1);
fail|=expect("<sop>", 104,1);
fail|=expect("<think> must NOT stop",110,0);
fail|=expect("<tool_call> must NOT stop",111,0);
fail|=expect("plain token must NOT stop",2,0);
if(!fail) printf(" both configs mutilated -> tokenizer still stops all 5 control tokens ok\n"); }
/* 5. T=NULL (validation/oracle path): config stops only, unchanged behaviour */
{ Cfg c; memset(&c,0,sizeof c);
write_cfg(dir,"config.json","[100]");
rm_file(dir,"generation_config.json");
load_cfg(&c,dir);
stops_arm_tok(&c,-1,NULL);
fail|=expect("T=NULL: config stop",100,1);
fail|=expect("T=NULL: no tokenizer sweep",101,0);
if(!fail) printf(" T=NULL -> config stops only (validation path untouched) ok\n"); }
rm_file(dir,"config.json"); rm_file(dir,"generation_config.json"); rm_file(dir,"tokenizer.json");
rmdir(dir);
if(fail){ printf("test_stops: FAIL\n"); return 1; }
printf("test_stops: ok\n");
return 0;
}
+8
View File
@@ -42,6 +42,11 @@ typedef struct {
hmap vocab; /* stringa byte-level -> id */
hmap merges; /* "left\0right" -> rank */
char **id2str; int *id_added; int n_ids; /* id -> stringa; id_added=1 se added-token (output letterale) */
int *id_special; /* 1 = added-token con "special":true nel tokenizer:
* token di CONTROLLO (<|user|>, <|assistant|>, <sop>, ...),
* mai contenuto legittimo di una risposta. Distinto da
* id_added, che copre anche <think>/<tool_call> ("special"
* false), i quali sono testo vero e vanno renderizzati. */
Special *sp; int nsp; /* added tokens, ordinati per lunghezza decrescente */
uint32_t byte2cp[256]; int byte2cp_len[256]; char byte2str[256][3];
int16_t cp2byte[1024];
@@ -106,6 +111,7 @@ static void tok_load(Tok *T, const char *path){
T->n_ids=maxid+1;
T->id2str=calloc(T->n_ids,sizeof(char*));
T->id_added=calloc(T->n_ids,sizeof(int));
T->id_special=calloc(T->n_ids,sizeof(int));
/* vocab: stringa -> id (capacita' potenza di 2, ~2-3x) */
int vc=1; while(vc < vocab->len*2) vc<<=1;
@@ -133,6 +139,8 @@ static void tok_load(Tok *T, const char *path){
char *content=json_get(a,"content")->str; int id=(int)json_get(a,"id")->num;
T->sp[i].str=content; T->sp[i].len=(int)strlen(content); T->sp[i].id=id;
T->id2str[id]=content; T->id_added[id]=1;
jval *sf=json_get(a,"special"); /* "special": true/false */
if(sf && sf->t==J_BOOL && sf->boolean) T->id_special[id]=1;
}
qsort(T->sp,T->nsp,sizeof(Special),cmp_sp_len); /* match piu' lungo per primo */
}