Add ds4-doctor self-diagnostic binary#556
Open
AnandSundar wants to merge 6 commits into
Open
Conversation
…ispatch ds4-doctor is a new read-only self-diagnostic command. U1 introduces the binary skeleton, the DS4_HELP_DOCTOR tool entry, and Makefile build rules across all four build paths (Darwin GPU/Linux CUDA, Darwin CPU/Linux CPU). Help options: --json, --color, -m/--model, --kv-disk-dir, --timeout, -h/--help --json already emits an empty checks[] payload so downstream U5 only needs to fill it in; --help dispatches through ds4_help_print() exactly like ds4-bench/ds4-eval/ds4-agent. The binary uses only ds4_doctor.o + ds4_help.o and links via $(CC) because it intentionally avoids any backend lib. This keeps it buildable across every backend variant and CI-friendly without GPU hardware. U2..U5 will add the actual checks (backend detection, GGUF header walk, env sanity) and the human + JSON renderers.
ds4-doctor U2 introduces the canonical check primitives (KTD-2): - doctor_status: ok / skip / warn / fail - doctor_check: id, title, status, duration_ms, message - now_ms(): CLOCK_MONOTONIC millisecond timer doctor_check_backend() uses the same #ifdef ladder as default_backend() in ds4_bench.c to identify which backend the binary was compiled with (DS4_NO_GPU / __APPLE__ Metal / DS4_ROCM_BUILD ROCm / else CUDA). This is a compile-time probe only: it does not load any model, open any device, or touch the filesystem, so it remains safe in CI without GPU hardware. main() now builds a one-element checks array, times the probe, and prints via print_human(). The JSON output is unchanged for now and will be filled in by U5. U3 (model file integrity), U4 (port / memory / kv-disk), and U5 (JSON renderer + exit-code policy) are still pending.
doctor_check_model() walks the 24-byte GGUF v3 fixed header:
magic u32 0x46554747 ("GGUF", little-endian)
version u32 must equal 3 (matches ds4.c:1985 enforcement)
n_tensors u64 reported in the message
n_kv u64 reported in the message
The probe opens the file read-only, fstat()s for total size, reads the
24 header bytes, then closes. No metadata-kv walk, no tensor walk, no
mmap — strictly less than ds4.c does at load time.
Status mapping:
- no path -> SKIP ("use -m FILE or DS4_GGUF")
- open/stat fail -> FAIL ("cannot open <path>")
- short magic read -> FAIL ("header too short")
- bad magic -> FAIL ("bad magic 0x...")
- wrong version -> FAIL ("unsupported GGUF version")
- short header -> FAIL ("header truncated at <field>")
- all good -> OK with size in GiB, version, tensor + kv counts
The check is forward-compatible with the timeout-bounded read that U5
will add. The struct field is already a fixed-size char[256] message
buffer (changed from const char *) so the doctor_check_model family can
build dynamic messages without dynamic allocation.
Forward declarations for doctor_check_port/memory/kv_disk already exist
from U2; U4 fills them in.
Three environment sanity checks, all read-only and CI-safe:
port
Bind a SOCK_STREAM to 127.0.0.1:8000 with SO_REUSEADDR (the same
default ds4-server uses; constant DOCTOR_DEFAULT_PORT mirrors
ds4_server.c). On EADDRINUSE return WARN ("port may be held by a
running ds4-server"); on EACCES return WARN (port needs elevated
privileges); on success return OK with "127.0.0.1:8000 bindable".
SO_REUSEADDR prevents stale TIME_WAIT sockets from causing false
positives.
memory
sysconf(_SC_PHYS_PAGES) * sysconf(_SC_PAGESIZE) -> total RAM in
GiB. Threshold ladder from the plan:
< 4 GiB -> FAIL ("below minimum")
< 8 GiB -> WARN ("below recommended")
else -> OK
sysconf failure -> FAIL.
kv-disk
If --kv-disk-dir is unset -> SKIP.
stat() the path. ENOENT/EACCES -> WARN with the underlying error
("server will create it on demand" is only valid for ENOENT-style
missing directories). Not-a-directory -> FAIL.
statvfs() reports free space; if statvfs fails we still report the
directory as accessible but with 0.00 GiB free, leaving the OK label
honest about what was measurable.
main() now runs all 5 checks and times each one. JSON output still
prints the U1 empty-array payload and exits 0; U5 wires the real
JSON renderer and exit-code policy.
…-code policy (U5)
U5 wires the final user-visible surface of ds4-doctor.
JSON renderer (--json)
One-line output of:
{
"status": "ok"|"warn"|"fail",
"summary": "<N> checks: <ok> ok, <warn> warn, <fail> fail, <skip> skip",
"checks": [
{ "id", "title", "status", "duration_ms", "message" }, ...
]
}
Every string field is JSON-escaped via print_json_escaped(). Numbers
(duration_ms, summary counts) are emitted unquoted. Worst status
wins: any FAIL -> fail, else any WARN -> warn, else ok.
Human renderer (default)
Tabular layout: ID / STATE / TIME / MESSAGE, plus a one-line
"Status: ..." footer that mirrors the JSON summary. Auto-detects
TTY for color when --color is unset; explicit --color forces ANSI
escapes regardless of stdout. STATUS column uses green/yellow/red/
grey per KTD-4.
Exit-code policy (R4)
0 no FAIL, no WARN
1 at least one WARN, no FAIL
2 at least one FAIL
Implementation reads doctor_summary_status(), the same function the
JSON renderer uses, so the JSON "status" field and the process
exit code can never disagree.
Helpers added:
doctor_summary counts + worst-status tracker
doctor_summary_update accumulates ok/warn/fail/skip + tracks max
doctor_summary_status fold -> DOCTOR_OK / WARN / FAIL
doctor_summary_name string form of the fold
print_json_escaped minimal JSON string escaper
print_json one-line JSON emitter
status_color ANSI escape per status, empty when !color
color_off "\x1b[0m" when color, empty otherwise
print_human tabular + footer
The skeleton placeholders are gone; main() now runs all five checks,
prints either JSON or human output, and returns 0/1/2 per R4.
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.
Add
ds4-doctorself-diagnostic binaryA small read-only pre-flight check for local installations. Runs five checks (backend compile, GGUF v3 header walk, default server port bindability, total system RAM, optional KV-disk directory) and exits 0/1/2 based on the worst severity.
Why
The repo already has five binary tools (
ds4,ds4-server,ds4-bench,ds4-eval,ds4-agent). New users hit the same first-day questions:download_model.shleave a partial file?"bind()fail with EACCES?"--kv-disk-diractually be writable?"Today the answer is grep the source.
ds4-doctoranswers those in one command.What it does
--jsonemits one line for scripts:{"status":"ok","summary":"5 checks: 3 ok, 0 warn, 0 fail, 2 skip","checks":[ {"id":"backend","title":"Backend compile","status":"ok","duration_ms":0,"message":"..."}, ... ]}Exit codes:
0= ok/skip only,1= at least one warn,2= at least one fail.Design notes
--doctorflag onds4_cli.ds4_cli.cis an interactive REPL, not a dispatcher. The existing pattern is one binary per subcommand (ds4-bench,ds4-eval,ds4-agent);ds4-doctorfollows the same pattern.ds4.c/ds4_cuda.cu/ds4_metal.mis linked. Safe to compile and run on a CI box with no Metal/CUDA hardware.__APPLE__/DS4_ROCM_BUILD/DS4_NO_GPU/ else CUDA), mirroring the pattern inds4_bench.c.magic u32 = 0x46554747,version u32 = 3,n_tensors u64,n_kv u64). Constant mirrorsDS4_GGUF_MAGIC/DS4_GGUF_VERSIONinds4.c.127.0.0.1:8000). ReportsWARNforEADDRINUSE/EACCES,FAILfor anything else.sysconf(_SC_PHYS_PAGES/_SC_PAGESIZE): <4 GiB fails, <8 GiB warns.stat+statvfsonly; does not create the directory. Skips cleanly when--kv-disk-diris omitted.--colorforces it on for CI logs.Build
Added to all four existing build paths (
make,make cpu,make cuda-spark,make cuda-generic,make cuda CUDA_ARCH=...,make strix-halo). The link line is intentionally short —$(CC) $(CFLAGS) -o $@ ds4_doctor.o ds4_help.o $(LDLIBS)— because the doctor has no GPU deps and no core library code.Test
tests/ds4_test.cadds adoctortest entry that runs three sub-tests against the freshly-built./ds4-doctor:--jsonoutput is parseable and contains the expected top-level fields.modelcheck withstatus:"fail".modelcheck withstatus:"ok".make testnow depends onds4-doctorso the binary is built before./ds4_testruns.Commits
7c18cfdskeleton + options parsing + help dispatch346f1b6backend compile check9831bf5model file integrity checkf5c0e09port, memory, kv-disk checksbcfc3b2JSON renderer + tabular human renderer + exit-code policydbd0e8ftest runner entry + README documentationTesting done
I cannot compile here (this fork is being prepared on a Windows shell without
nvcc/Metaltoolchains); I followed the same patch-by-patch review path contributors usually take, against the patterns already inds4_bench.c,ds4_eval.c,ds4_help.c,ds4.c, andds4_server.c. A maintainer runningmake cpu && ./ds4-doctor --jsonon Linux will validate the build paths and the human renderer;./ds4_test --doctorwill run the three new sub-tests.