Skip to content

coli: the banner said GLM-5.2 over every model - #832

Merged
JustVugg merged 1 commit into
devfrom
feat/banner-model
Aug 4, 2026
Merged

coli: the banner said GLM-5.2 over every model#832
JustVugg merged 1 commit into
devfrom
feat/banner-model

Conversation

@JustVugg

@JustVugg JustVugg commented Aug 4, 2026

Copy link
Copy Markdown
Owner

coli chat, run, info, plan and tune all printed the same third banner line whatever you loaded:

GLM-5.2 · 744B MoE · int4 · streaming CPU

Start a chat with Inkling, Kimi K3, OLMoE or the new DeepSeek V4 engine and the banner still said GLM-5.2 · 744B. With four engines in the tree and a fifth arriving, that is no longer a cosmetic detail — it is the first thing the TUI tells you, and it was false four times out of five.

Where the wrongness actually lives

Not in banner(). In what banner() would have had to ask:

def model_arch(model):
    ...
    return "glm"          # every unrecognised model_type lands here

That default is right for its job. colibri.c is the general engine, so an unknown checkpoint should be offered to it rather than refused. It is simply the wrong answer to a different question — what did the user load — and the banner needs that one.

So model_banner_line() reads the raw model_type and leaves the dispatch in model_arch() untouched. No engine-selection behaviour changes.

What it prints now

GLM-5.2 · 744B MoE · 372 GB on disk
Inkling · 975B MoE · 469 GB on disk
Kimi K3 · 2.8T MoE · 1.1 TB on disk
DeepSeek V4 Flash · 284B MoE · 167 GB on disk
OLMoE · 7B MoE · 4.2 GB on disk
qwen3_moe · 48L x 128E MoE · 61 GB on disk        ← not in the roster

Three decisions worth stating, because each one is a place where it would have been easy to lie:

Parameter counts come from the README roster. Same numbers, one source; they cannot drift apart without someone noticing.

An unrecognised model_type is not forced into a name. It prints its own type and the geometry measured from its own config. Guessing "GLM-5.2" for a Qwen checkpoint is exactly the bug being fixed, and inventing a nicer-looking wrong answer is not an improvement over the old one.

A model with no config.json, or an unreadable path, keeps the original tagline. coli info banners before it validates the model directory, so this had to be safe on a bad path — there is a test for it.

Cost

Size is measured by stat-ing the shards. No safetensors headers are parsed: a banner runs before every command and may not cost a scan of a 400 GB checkpoint. The whole line is one config.json read plus one stat per shard.

For the other PRs that touch banner()

#825 and #670 both change banner("run") into banner("run", <cuda flag>), positionally. model= here is therefore keyword-only, so a second positional argument raises TypeError rather than being silently read as a path. test_model_is_keyword_only pins that.

Whichever of us lands second gets a trivial one-line conflict instead of a banner that tries to open True as a directory.

Tests

Eight new cases in tests/test_cli_output.py, run with the existing suite (16/16 green):

  • every engine names itself
  • the regression itself — a deepseek_v4 config must not produce a line containing GLM or 744B
  • an unknown model_type states its own type and its measured geometry
  • a config with no model_type says unknown model rather than inventing one
  • no model, and an unreadable path, both keep the generic tagline without raising
  • sizes do not round to 0 GB on a small checkpoint
  • model= is keyword-only

`coli chat`, `run`, `info`, `plan` and `tune` all printed

    GLM-5.2 · 744B MoE · int4 · streaming CPU

regardless of what was loaded. Load Inkling, Kimi K3, OLMoE or the new
DeepSeek V4 engine and the third banner line still named GLM.

## Why it was wrong, and where the wrongness lives

Not in the banner. In what the banner would have had to ask:

    def model_arch(model):
        ...
        return "glm"          # <- every unrecognised model_type

That default is correct for its job -- colibri.c is the general engine, so
an unknown checkpoint should be offered to it. It is the wrong answer to a
different question, "what did the user load", and the banner needs that
one. So `model_banner_line()` reads the raw `model_type` instead, and the
dispatch in `model_arch()` is untouched.

## What it prints

    GLM-5.2 · 744B MoE · 372 GB on disk
    DeepSeek V4 Flash · 284B MoE · 167 GB on disk
    OLMoE · 7B MoE · 4.2 GB on disk
    qwen3_moe · 48L x 128E MoE · 61 GB on disk      <- not in the roster

Parameter counts come from the README roster, so the two cannot drift apart
without someone noticing. A model_type that is not in the table is NOT
forced into a name: it prints its own type and the geometry measured from
its config, which is honest and still useful. No config.json at all, or an
unreadable path, keeps the original tagline -- `coli info` banners before it
validates the model directory, so this has to be safe on a bad path.

Size is measured by stat-ing the shards; no safetensors headers are parsed.
A banner runs before every command and may not cost a scan of a 400 GB
checkpoint.

## Note for the open PRs that also touch banner()

#825 and #670 both change `banner("run")` to pass a CUDA flag positionally.
`model=` is therefore **keyword-only**, so a second positional argument
raises TypeError instead of being silently read as a path. There is a test
for exactly that.

Eight tests in tests/test_cli_output.py, including the regression itself: a
`deepseek_v4` config must not produce a line containing "GLM" or "744B".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JustVugg
JustVugg merged commit cc65879 into dev Aug 4, 2026
15 of 16 checks passed
JustVugg added a commit that referenced this pull request Aug 4, 2026
`check` has been red on dev since #832:

    ERROR: test_size_is_reported_without_rounding_to_zero
    OSError: [Errno 28] No space left on device
    Ran 394 tests in 549.596s
    FAILED (errors=1, skipped=51)

My fault, and the comment on the line was the mistake:

    shard.truncate(shard_bytes)   # sparse: costs no disk

True on ext4 and APFS. NOT true on NTFS, where ftruncate allocates. The
test asked a Windows runner to materialise 372 GB and filled its disk.
Linux and macOS passed, so it only ever failed on one of three platforms --
and only after landing, since #832's own CI ran before the merge.

The size is now supplied by patching os.path.getsize, and the shard on disk
is an empty file that exists only so listdir() finds it. Same assertions,
no allocation anywhere: measured, the whole class now costs 4 KB instead of
372 GB.

Verified it still fails when the formatting is wrong -- changing the
sub-10 GB branch from .1f to .0f makes it red -- so this is not the kind of
repair that quietly turns a test into a no-op.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant