Port the coli CLI launcher to Go (dependency-free) — first step for #310#454
Port the coli CLI launcher to Go (dependency-free) — first step for #310#454Magnet-js wants to merge 6 commits into
Conversation
Port the `coli` launcher to a native Go binary as the first step toward a Python-free runtime path (issue JustVugg#310). The CLI process management moves to Go; the OpenAI-compatible gateway and offline tooling stay in Python and are invoked as subprocesses, so the Python code remains authoritative and its test suite is untouched (63/63 still pass). - build/info/run/chat: native Go (Python-free), incl. the engine byte protocol (READY/END sentinels, STAT lines, RESET/MORE control writes, two-stage Ctrl-C) and the streaming-markdown chat REPL. - serve/web: spawn openai_server.py (gateway stays Python); relay SIGTERM for a clean shutdown and isolate the child process group. - plan/doctor: drive resource_plan.py / doctor.py via `python3 -` so no .py file needs a new entrypoint. - bench/convert: run the Python tools unchanged. New c/go/ module: standard library only, zero third-party requires, to preserve Colibri's dependency-free default path. Added an optional `make coli-go` target (not part of all/check) and .gitignore entries. Verified byte-for-byte parity with the Python coli: help/info/error strings, env_for across flag combinations (incl. --auto-tier), plan/doctor JSON, and chat output (incl. multibyte UTF-8 split across engine chunks). One documented cosmetic delta: the TTY input box is not cursor-redrawn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the first automated tests for the Go `coli` port and gate them in `make check`, so the parity claims stop being manual. - env_test.go: envFor golden oracle — spawns the real Python `c/coli` env_for as reference and diffs the child-env map byte-for-byte across 13 flag combinations (ram/ngen/topp/topk/temp/repin/ctx/policy/gpu=none + a combo). This is the net that will let us delete c/coli safely. - args_test.go: parseArgs table (flags before/after subcommand, intermixed positionals, serve/convert flags) plus the error paths (invalid --policy, unknown command, unexpected positional). - mdstream_test.go: utf8Decoder rune-split-across-chunks and MDStream bold/inline-code/heading/bullet/fence rendering, including markers split across chunk boundaries. - engine coverage: ftoa (Python str(float) parity) and parseStat. Makefile: new `test-go` target (gofmt + go vet + go test), wired into `test`/`check`. Guarded on the Go toolchain being present so the dependency-free Python path still runs `make check` without Go installed. Also in this change: - diskfree (Windows): implement diskFree via GetDiskFreeSpaceExW (syscall only, no third-party dep), matching Python's shutil.disk_usage which works on Windows — `info` now reports real free space there. - comments: drop the dead docs/tuning-9950x3d-5090.md reference in engine.go; correct the termWidth comment, which overstated $COLUMNS availability and parity with Python's shutil.get_terminal_size. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
) Declare c/coli frozen: new commands, flags, output, and behavior changes go to the Go CLI (c/go/), not to this file. Only security/correctness fixes are allowed here, and must land in the Go CLI in the same change. The gateway (openai_server.py) and offline tools are not frozen. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address the PR review properly (not with documentation workarounds): - terminal width (JustVugg#24): real tty query — TIOCGWINSZ on Unix, GetConsoleScreenBufferInfo on Windows — with $COLUMNS precedence matching Python's shutil.get_terminal_size, instead of reading only $COLUMNS (which is rarely exported to children, so the box used to render at 80 cols). - Windows signals (JustVugg#12/JustVugg#19): SIGINT/SIGTERM cannot be delivered to another process on Windows, so the old Process.Signal calls were silent no-ops. Create the child in a new process group and relay CTRL_BREAK_EVENT via GenerateConsoleCtrlEvent; abstracted as interruptChild/terminateChild (per-OS). Applies to the chat engine and the gateway. - Windows OMP (JustVugg#14): physicalCPUCount now counts physical cores via GetLogicalProcessorInformationEx (matching Python physical_cpu_count), so OMP_NUM_THREADS is no longer doubled on a hyperthreaded box. - Windows disk-free stays real (GetDiskFreeSpaceExW, prior commit). - cmd_chat (JustVugg#15/JustVugg#17): check StdinPipe/StdoutPipe/StderrPipe errors; close the errlog temp file before removing it (fd leak + Windows open-file delete). - main (JustVugg#9): drop the unreachable handler branch that silently exited 0; fail loudly if the command/handler maps ever drift. - engine (JustVugg#11): use stdlib bytes.HasSuffix instead of the hand-rolled helper. Tests: - args_test.go: pin the exact CLI error strings (Go-native contract). - drivers_test.go: golden error-paths driving the embedded Python against the real resource_plan.py / doctor.py, catching API drift via the exit code. README: correct the parity/known-differences sections — OMP and terminal width are no longer differences; document the Windows Ctrl-Break relay and the Go-native (non-argparse) error strings; point at the Go tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…to go-cli-port
|
Thank you for building this — it's clean work, and the motivation in #310 is a real problem: the runtime path shouldn't need Python when the engine is dependency-free C. I'm going to be straight with you rather than let it sit: I don't want to add Go to this project. Not because of the code, which is fine, but because of what it commits us to. colibri is a C project whose whole promise is one compiler and zero dependencies; adding a second toolchain means every contributor and packager needs it, both launchers have to be kept in step forever, and every future flag lands twice or drifts. That's a permanent cost paid by everyone, for a problem that only affects people who don't have Python. What I do want is the outcome you're after — no interpreter on the runtime path. If we're going to spend a rewrite, the natural answer for this project is C: the launcher's job is argument parsing, environment setup and spawning a process, all of which the engine already does natively. Then There's a smaller, useful piece hiding in your PR that I'd take today: the process-management and environment-setup logic, which you clearly got right. If you wanted to bring that over as C, or to write up what the Go version handles that the current Python launcher doesn't (signal handling, exit codes, Windows quirks), that would go straight in. I realize this isn't the answer you hoped for after the work you put in, and I'm sorry to give it after the PR has been open a while. That's on me for not saying it sooner. The reasoning is about the project's shape, not the quality of what you wrote. |
|
Hi @JustVugg That said, I understand your point about C. My main reason for choosing Go was that it's easy to write a simple, readable HTTP server in it. As far as I know, that's not really the case for C (change my mind). But overall, I agree with you on the CLI. I'd be happy to keep working on it in C as well, though I'm not very familiar with C, my background is more Java/Kotlin and Go. Maybe next time we should discuss this directly in the related issue. |
2 participants
Addresses #310 (make the runtime path Python-free by porting the CLI/gateway to Go).
Scope — CLI process management only (incremental first step)
Ports the
colilauncher (c/coli, 761 lines) to a native Go binary. The gateway and offline tooling stay in Python and are invoked as subprocesses, so the Python code remains authoritative and its test suite is untouched.build,info,run,chatserve,webopenai_server.py(gateway stays Python)plan,doctorresource_plan.py/doctor.pyviapython3 -bench,convertDesign
c/go/module, standard library only — zero third-party requires. Colibri's runtime path is dependency-free by design (CONTRIBUTING.md) and the gateway is stdlib-only for the same reason; the Go binary keeps that property (single static binary). Cobra/Viper were considered and rejected on these grounds.c/*.pyfile is modified.make checkand the Python tests (63/63) still pass.plan/doctor/--auto-tierfeed a small driver topython3 -(stdin) that imports the existing modules, so no support file needs a new entrypoint.\x01\x01READY\x01\x01\n/\x01\x01END\x01\x01\nsentinels,STAT tok tps hit rsslines,\x02RESET/\x02MOREcontrol writes, two-stage Ctrl-C inchat.Build
The binary finds the engine and Python support files exactly like
./coli;COLI_ENGINEoverrides the engine path.Verified parity vs the Python
coli--help/info/ error strings — matchestests/test_cli_output.pycontractenv_forchild environment — byte-identical across--ram/--temp/--topp/--topk/--ngen/--ctx/--repin/--gpu/--vram/--policy/--auto-tierplan/doctorJSON + text — identical (sameresource_plan.py/doctor.py)chat— identical output in both stderr scenarios; multibyte UTF-8 split across engine chunks renders identicallyserve— spawns the gateway,/health+/v1/modelsanswer, SIGINT/SIGTERM shut down cleanly with no orphanbench/convert— argv construction matches (two-pass convert, dataset-skip logic)gofmt/go vetclean; cross-compiles for linux and windowsKnown deltas (documented in
c/go/README.md)OMP_NUM_THREADSdefaults to the logical CPU count (runtime.NumCPU()) rather than physical cores — an overridablesetdefault.chatTTY input box is drawn once per prompt rather than cursor-redrawn; behaviour is otherwise identical.