From 76067c8389177bc5bc192d4b391c5d7fdde7bbae Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 09:11:35 +0100 Subject: [PATCH 1/5] fix(windows): six packages off the experimental lane's failure list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The windows CI lane fails 46 of the module suite's packages. This lands the six whose cause is ours rather than a dependency's, each verified against the lane's own log (run 31109255741, job 92642423611). - serving/admin: pathWithinDir's escape arm tested only "../", but PathRel hands back an OS-native relative path, so an escaping result reads "..\evil" on Windows and was waved through as contained — a containment bypass, not just a red test. Split the check into hasParentPrefix, which tests the native separator too. Fixes TestPathWithinDir_Containment_Good and TestResolveModelNameToPath_SymlinkEscape_Bad. - serving/admin tests: two reload bodies pasted a filesystem path raw into a quoted JSON literal; "C:\Users\..." decodes as the invalid escape "\U". Route both through a jsonString helper. - serving/scheduler: the BeginPrepare span was recorded with a bare time.Since, and Windows' monotonic tick returns exactly 0 for a sub-microsecond prepare — blanking PrefillDuration and, via its positive- span guard, PrefillTokensPerSec as well. Floor it in measuredSpan. - inference (root): TestGGUF_DiscoverModels_Ugly built its expectation with core.JoinPath (always '/') and compared it against a path DiscoverModels produced with native separators — PathJoin is the one that matches. TestDiscover_Good_RelativeBaseDir asked PathRel to relativise t.TempDir() against the cwd, which cannot be expressed when CI puts the checkout on D: and TEMP on C:; the base now sits under the cwd. - kv/blockcache, model/bundle: five tests inject faults through POSIX-only filesystem semantics — chmod 0o500 on a directory (Windows os.Chmod only toggles FILE_ATTRIBUTE_READONLY and still permits creates and unlinks) and a directory's non-zero Stat size (0 on Windows, so ReadFull is handed an empty buffer and cannot fail). Skipped there with the reason recorded, so the arms keep their POSIX coverage instead of asserting a false negative. Receipt — macOS, the required lane's platform: go test -count=1 ./serving/admin/... ./serving/scheduler/... \ ./kv/blockcache/... ./model/bundle/... ./lab/... . ok serving/admin 0.870s · serving/scheduler 0.549s · kv/blockcache 1.501s ok model/bundle 1.222s · lab 0.624s · inference 0.316s gofmt -l: clean · go vet: clean lab's TestCmd_RunServe_Bad_ListenAddrInUse asserted the POSIX errno text ("in use"); Windows says "Only one usage of each socket address ... is normally permitted". It now asserts the syscall stage ("bind:"), which both report and which is what the test is actually pinning. The remaining 40 packages are dependency-side and ledgered separately. Co-Authored-By: Virgil --- go/discover_test.go | 17 ++++++++++++----- go/gguf_test.go | 4 +++- go/kv/blockcache/blockcache_branch_test.go | 1 + go/kv/blockcache/blockcache_disk_test.go | 16 ++++++++++++++++ go/lab/cmd_test.go | 5 ++++- go/model/bundle/bundle_cov_test.go | 8 ++++++++ go/serving/admin/admin_test.go | 17 +++++++++++++++-- go/serving/admin/reload.go | 16 +++++++++++++++- go/serving/scheduler/cb_step.go | 16 ++++++++++++++-- 9 files changed, 88 insertions(+), 12 deletions(-) diff --git a/go/discover_test.go b/go/discover_test.go index 245b7cd3d..4503c7da1 100644 --- a/go/discover_test.go +++ b/go/discover_test.go @@ -314,15 +314,22 @@ func TestDiscover_Good_AbsolutePath(t *testing.T) { } func TestDiscover_Good_RelativeBaseDir(t *testing.T) { - base := t.TempDir() - createModelDir(t, core.JoinPath(base, "relative-model"), map[string]any{ - "model_type": "gemma3", - }, 1) - cwdResult := core.Getwd() checkResultOK(t, cwdResult) cwd := cwdResult.Value.(string) + // The base must sit under the working directory, not in t.TempDir(): + // PathRel cannot express a path that crosses volumes, and on Windows CI + // the checkout ("D:\a\...") and TEMP ("C:\Users\...") are different drives. + baseResult := core.MkdirTemp(cwd, "discover-relative") + checkResultOK(t, baseResult) + base := baseResult.Value.(string) + t.Cleanup(func() { core.RemoveAll(base) }) + + createModelDir(t, core.PathJoin(base, "relative-model"), map[string]any{ + "model_type": "gemma3", + }, 1) + relBaseResult := core.PathRel(cwd, base) checkResultOK(t, relBaseResult) relBase := relBaseResult.Value.(string) diff --git a/go/gguf_test.go b/go/gguf_test.go index 1bb6560a3..93c1c4017 100644 --- a/go/gguf_test.go +++ b/go/gguf_test.go @@ -35,7 +35,9 @@ func TestGGUF_ReadGGUFInfo_Bad(t *testing.T) { func TestGGUF_DiscoverModels_Ugly(t *testing.T) { dir := t.TempDir() - path := writeMinimalGGUFAt(t, core.JoinPath(dir, "model.gguf"), map[string]any{ + // PathJoin (filepath.Join), not JoinPath (always '/'): the expectation is + // compared against a path DiscoverModels built with native separators. + path := writeMinimalGGUFAt(t, core.PathJoin(dir, "model.gguf"), map[string]any{ "general.architecture": "gemma4_text", "general.file_type": uint32(7), }) diff --git a/go/kv/blockcache/blockcache_branch_test.go b/go/kv/blockcache/blockcache_branch_test.go index 14d8140c7..672e3c71c 100644 --- a/go/kv/blockcache/blockcache_branch_test.go +++ b/go/kv/blockcache/blockcache_branch_test.go @@ -66,6 +66,7 @@ func TestBlockcache_Service_WriteDiskBlockMkdirFailure(t *testing.T) { // passes; the subsequent MkdirAll that recreates the cache directory then // fails on the read-only parent, surfacing the recreate error. func TestBlockcache_Service_ClearDiskRecreateFailure(t *testing.T) { + requirePOSIXDirPermissions(t) parent := core.PathJoin(t.TempDir(), "parent") if result := core.MkdirAll(parent, 0o700); !result.OK { t.Fatalf("MkdirAll(parent) error = %s", result.Error()) diff --git a/go/kv/blockcache/blockcache_disk_test.go b/go/kv/blockcache/blockcache_disk_test.go index c8dfe3925..2c379c355 100644 --- a/go/kv/blockcache/blockcache_disk_test.go +++ b/go/kv/blockcache/blockcache_disk_test.go @@ -4,6 +4,7 @@ package blockcache import ( "context" + "runtime" "testing" core "dappco.re/go" @@ -11,6 +12,18 @@ import ( state "dappco.re/go/inference/model/state" ) +// requirePOSIXDirPermissions skips a test whose fault injection is a +// chmod'd read-only directory. On Windows os.Chmod only toggles +// FILE_ATTRIBUTE_READONLY, which does not deny creating or unlinking +// entries inside a directory, so the failure arm under test never fires +// and the assertion would report a false negative. +func requirePOSIXDirPermissions(t *testing.T) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("chmod cannot deny directory writes on Windows (FILE_ATTRIBUTE_READONLY only)") + } +} + // recordingStateWriter is a test stub that returns a fixed ChunkRef and records // the last payload it received. It lets the State cold-store success path be // driven with a ChunkRef whose optional fields (Codec/Segment/FrameOffset) are @@ -215,6 +228,7 @@ func TestBlockcache_Service_DiskRecordUnreadableQuarantined(t *testing.T) { // WriteFile-failure branch: a read-only DiskPath directory already exists (so the // inner MkdirAll no-ops), but the block record cannot be written into it. func TestBlockcache_Service_WarmCacheWriteFailure(t *testing.T) { + requirePOSIXDirPermissions(t) diskPath := core.PathJoin(t.TempDir(), "blocks") if result := core.MkdirAll(diskPath, 0o700); !result.OK { t.Fatalf("MkdirAll(diskPath) error = %s", result.Error()) @@ -256,6 +270,7 @@ func TestBlockcache_Service_ClearCacheRunsRuntimeHook(t *testing.T) { // DiskPath's parent directory is made read-only, so the post-load RemoveAll // inside clearDiskLocked cannot unlink the block directory. func TestBlockcache_Service_ClearCacheDiskFailure(t *testing.T) { + requirePOSIXDirPermissions(t) parent := core.PathJoin(t.TempDir(), "parent") diskPath := core.PathJoin(parent, "blocks") if result := core.MkdirAll(diskPath, 0o700); !result.OK { @@ -313,6 +328,7 @@ func TestBlockcache_Service_DiskBytesStatFallback(t *testing.T) { // labelled block is persisted, the DiskPath directory is made read-only, so // unlinking the matched block's record file fails and the error is surfaced. func TestBlockcache_Service_ClearCacheRemoveBlockFailure(t *testing.T) { + requirePOSIXDirPermissions(t) diskPath := core.PathJoin(t.TempDir(), "blocks") if result := core.MkdirAll(diskPath, 0o700); !result.OK { t.Fatalf("MkdirAll(diskPath) error = %s", result.Error()) diff --git a/go/lab/cmd_test.go b/go/lab/cmd_test.go index 4c1007bba..e274e89a4 100644 --- a/go/lab/cmd_test.go +++ b/go/lab/cmd_test.go @@ -167,7 +167,10 @@ func TestCmd_RunServe_Bad_ListenAddrInUse(t *core.T) { got := r.Error() core.AssertFalse(t, r.OK) - core.AssertContains(t, got, "in use") + // Assert on the syscall stage, not the errno text: POSIX says "address + // already in use", Windows says "Only one usage of each socket address + // ... is normally permitted". Both are reported as a bind failure. + core.AssertContains(t, got, "bind:") } func TestCmd_newServeMux_Good(t *core.T) { diff --git a/go/model/bundle/bundle_cov_test.go b/go/model/bundle/bundle_cov_test.go index 35bb1142c..4d7e79733 100644 --- a/go/model/bundle/bundle_cov_test.go +++ b/go/model/bundle/bundle_cov_test.go @@ -4,6 +4,7 @@ package bundle import ( "math" + "runtime" "strings" "testing" @@ -260,7 +261,14 @@ func TestBundle_FileHash_OpenErrorMissing(t *testing.T) { // ReadFull-failure branch: opening a directory succeeds and its reported size // is below the streaming threshold, so FileHash takes the buffer path, but // reading bytes from a directory descriptor fails (EISDIR). +// +// POSIX-only: Windows reports a directory's Stat size as 0, so ReadFull is +// handed a zero-length buffer and returns nil without ever touching the +// descriptor — the read-failure arm is unreachable there. func TestBundle_FileHash_ReadErrorOnDirectory(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("a directory Stat reports size 0 on Windows, so the ReadFull arm cannot fail") + } dir := t.TempDir() if _, err := FileHash(dir); err == nil { t.Fatal("FileHash(directory) error = nil, want read error") diff --git a/go/serving/admin/admin_test.go b/go/serving/admin/admin_test.go index c38d55bcf..2e4d36eda 100644 --- a/go/serving/admin/admin_test.go +++ b/go/serving/admin/admin_test.go @@ -5,6 +5,7 @@ package admin import ( "crypto/sha256" "encoding/hex" + "encoding/json" "net/http" "net/http/httptest" "runtime" @@ -16,6 +17,18 @@ import ( "dappco.re/go/inference" ) +// jsonString renders s as a JSON string literal. Test bodies interpolate +// filesystem paths, and a Windows path ("C:\Users\...") pasted raw into a +// quoted JSON literal is a decode error — "\U" is not a valid escape. +func jsonString(t *testing.T, s string) string { + t.Helper() + encoded, err := json.Marshal(s) + if err != nil { + t.Fatalf("marshal %q: %v", s, err) + } + return string(encoded) +} + // fakeReloader records the path (and load options) it was asked to swap in. // err, when set, makes ReloadModel fail without mutating current/gotPath — // simulating a load failure that must not corrupt the resolver's state. @@ -341,7 +354,7 @@ func TestReloadHandler_ModelPath_Good(t *testing.T) { rl := &fakeReloader{current: "/models/old"} mux := NewMux(Config{Reloader: rl}) - body := `{"model_path":"` + dir + `","confirm_machine":"` + MachineHash() + + body := `{"model_path":` + jsonString(t, dir) + `,"confirm_machine":"` + MachineHash() + `","context_length":4096,"adapter_path":"/adapters/lora"}` rec := httptest.NewRecorder() mux.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, PathReload, strings.NewReader(body))) @@ -372,7 +385,7 @@ func TestReloadHandler_ModelPathEscapesDir_Bad(t *testing.T) { rl := &fakeReloader{current: "/models/old"} mux := NewMux(Config{Reloader: rl}) - body := `{"model_path":"` + outside + `","confirm_machine":"` + MachineHash() + `"}` + body := `{"model_path":` + jsonString(t, outside) + `,"confirm_machine":"` + MachineHash() + `"}` rec := httptest.NewRecorder() mux.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, PathReload, strings.NewReader(body))) diff --git a/go/serving/admin/reload.go b/go/serving/admin/reload.go index 4a5d36294..48702ddbe 100644 --- a/go/serving/admin/reload.go +++ b/go/serving/admin/reload.go @@ -150,6 +150,10 @@ func reloadFail(w http.ResponseWriter, log io.Writer, from, target, reason strin // PathRel containment test (not a raw string prefix — a case-insensitive // filesystem can hand back a different casing that a byte-prefix check would // falsely reject). +// +// The escape arm tests the native separator as well as '/': PathRel returns an +// OS-native relative path, so on Windows an escaping result reads "..\evil", +// which a '/'-only prefix check would wave through as contained. func pathWithinDir(rootResolved, resolved string) bool { if resolved == rootResolved { return true @@ -162,12 +166,22 @@ func pathWithinDir(rootResolved, resolved string) bool { if r == "" || r == "." { return true } - if r == ".." || core.HasPrefix(r, "../") || core.PathIsAbs(r) { + if r == ".." || hasParentPrefix(r) || core.PathIsAbs(r) { return false } return true } +// hasParentPrefix reports whether a relative path opens with a parent-dir +// segment under either separator convention. +func hasParentPrefix(rel string) bool { + if core.HasPrefix(rel, "../") { + return true + } + separator := core.Env("DS") + return separator != "" && separator != "/" && core.HasPrefix(rel, ".."+separator) +} + // bindModelPathToStandardDir accepts an absolute model path and verifies it // canonicalises to a child of standardModelDir() with a sha sidecar present. func bindModelPathToStandardDir(path string) (string, error) { diff --git a/go/serving/scheduler/cb_step.go b/go/serving/scheduler/cb_step.go index 4793ed6cb..48796d875 100644 --- a/go/serving/scheduler/cb_step.go +++ b/go/serving/scheduler/cb_step.go @@ -249,6 +249,18 @@ type cbPrepared struct { prepDur time.Duration // the BeginPrepare span — the request's honest PrefillDuration } +// measuredSpan reports the time elapsed since start, floored at one +// nanosecond. A prefill that finished faster than the platform's monotonic +// tick can resolve — Windows' clock returns exactly 0 for a sub-microsecond +// span — still happened, and reporting 0 would both blank PrefillDuration and +// suppress PrefillTokensPerSec, whose divide is guarded on a positive span. +func measuredSpan(start time.Time) time.Duration { + if elapsed := time.Since(start); elapsed > 0 { + return elapsed + } + return time.Nanosecond +} + // run is the single drive-loop goroutine: it owns the lane set, the pending // queue, and the lane→request map outright (single-writer, no lock). // @@ -299,7 +311,7 @@ func (e *cbStepEngine) run() { e.cancelled.Add(1) return } - req.prefillDur = time.Since(prepStart) + req.prefillDur = measuredSpan(prepStart) req.decodeStart = time.Now() byLane[h.ID] = req e.admitted.Add(1) @@ -324,7 +336,7 @@ func (e *cbStepEngine) run() { go func(req *cbReq) { prepStart := time.Now() p, err := overlap.BeginPrepare(req.ctx, inference.LaneSpec{PromptIDs: req.promptIDs, MaxNew: req.maxNew, StopTokens: req.stops, Sampler: req.sampler}) - prepCh <- cbPrepared{req: req, p: p, err: err, prepDur: time.Since(prepStart)} + prepCh <- cbPrepared{req: req, p: p, err: err, prepDur: measuredSpan(prepStart)} }(req) } e.queued.Store(int64(len(pending))) From 830e99113fa0a6a0a212a210f8410ef7200979d1 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 09:18:37 +0100 Subject: [PATCH 2/5] =?UTF-8?q?fix(windows):=20four=20more=20packages=20of?= =?UTF-8?q?f=20the=20lane=20=E2=80=94=20pathx=20splits=20on=20either=20sep?= =?UTF-8?q?arator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit core.PathBase and core.PathDir match only the platform's own separator (core.Env("DS")), so on Windows a '/'-separated path reads as having no separator at all: PathBase("/models/gemma3-1b") returns the whole string, PathDir("/models/x/adapter.safetensors") returns ".". Go accepts '/' on every platform and it arrives from config files, CLI flags, HTTP model ids and hub refs, so four call sites deriving a name or a parent from a caller-supplied path were wrong there — not merely red under test. internal/pathx carries the both-separator Base/Dir/Join. '\' is a boundary only where the platform says so; on POSIX it stays an ordinary filename character, which the _Ugly tests pin on both hosts. - eval/bench: run.Model is a hub ref ("org/model-a") as often as a path, and those are '/'-separated everywhere — the matrix row was named for the whole ref instead of its base. - train/lora: adapterConfigPath resolved the sidecar of "/models/my-lora/adapter.safetensors" to "./adapter_config.json" — the wrong directory, not just wrong-looking. Its join also hardcoded '/' onto a PathDir result; pathx.Join keeps whichever separator the input used, so a '/'-spelled Windows path no longer comes back mixed. The filepath.Clean-free fast path the comment defends is preserved. - serving/compat: resolverModelNames published the entire ModelPath as the OpenAI model id. - model/arch/google/gemma3/gguf: gemma3ModelName wrote the whole checkpoint path into general.name. Receipt — macOS: go test -count=1 ./internal/pathx/... ./train/lora/... ./eval/bench/... \ ./serving/compat/... ./model/arch/google/gemma3/... ok internal/pathx 0.260s · train/lora 0.718s · eval/bench 0.839s ok serving/compat 0.300s · gemma3 0.771s · gemma3/gguf 1.026s gofmt -l: clean · go vet: clean · go build ./...: clean Co-Authored-By: Virgil --- go/eval/bench/matrix.go | 8 +- go/internal/pathx/pathx.go | 120 ++++++++++++++++ go/internal/pathx/pathx_test.go | 146 ++++++++++++++++++++ go/model/arch/google/gemma3/gguf/convert.go | 7 +- go/serving/compat/mux.go | 6 +- go/train/lora/inspect.go | 33 ++--- 6 files changed, 300 insertions(+), 20 deletions(-) create mode 100644 go/internal/pathx/pathx.go create mode 100644 go/internal/pathx/pathx_test.go diff --git a/go/eval/bench/matrix.go b/go/eval/bench/matrix.go index 5fd5a064b..d8b8e6651 100644 --- a/go/eval/bench/matrix.go +++ b/go/eval/bench/matrix.go @@ -8,6 +8,7 @@ import ( "time" core "dappco.re/go" + "dappco.re/go/inference/internal/pathx" ) // matrix.go is the multi-model layer over the single-model harness: a bench MATRIX is a list of @@ -94,7 +95,10 @@ func LoadMatrixConfig(data []byte) (MatrixConfig, error) { return cfg, core.NewError(core.Sprintf("bench.LoadMatrixConfig: run %d has no model", i)) } if run.Name == "" { - run.Name = core.PathBase(core.Trim(run.Model)) + // pathx.Base: run.Model is a hub ref as often as a path + // ("org/model-a"), and those are '/'-separated everywhere — + // core.PathBase would name the whole ref on Windows. + run.Name = pathx.Base(core.Trim(run.Model)) } if len(run.Lanes) == 0 { run.Lanes = []string{MatrixLanePlain} @@ -229,7 +233,7 @@ func RunMatrix(ctx context.Context, cfg MatrixConfig, load MatrixLoad, out io.Wr } name := run.Name if name == "" { - name = core.PathBase(core.Trim(run.Model)) // positional runs bypass LoadMatrixConfig's defaulting + name = pathx.Base(core.Trim(run.Model)) // positional runs bypass LoadMatrixConfig's defaulting } row := MatrixRow{Name: name, Lane: lane, Model: run.Model, Draft: run.Draft} row = runMatrixLane(ctx, row, run, lane, tokens, cfg, load) diff --git a/go/internal/pathx/pathx.go b/go/internal/pathx/pathx.go new file mode 100644 index 000000000..2cf3c2ba9 --- /dev/null +++ b/go/internal/pathx/pathx.go @@ -0,0 +1,120 @@ +// SPDX-Licence-Identifier: EUPL-1.2 + +// Package pathx splits paths on either separator convention. +// +// core.PathBase and core.PathDir match only the platform's own separator +// (core.Env("DS")), so on Windows a '/'-separated path reads as having no +// separator at all: PathBase("/models/gemma3-1b") hands back the whole string +// and PathDir("/models/x/adapter.safetensors") hands back ".". Go itself +// accepts '/' in paths on every platform — it arrives from config files, CLI +// flags, HTTP model ids and Hugging Face refs — so any code deriving a name or +// a parent directory from a caller-supplied path needs both conventions. +// +// Base and Dir here are the both-separator forms. They do not clean, resolve +// or otherwise normalise: callers that need a real filesystem path still build +// it with core.PathJoin. +package pathx + +import core "dappco.re/go" + +// Base returns the last element of p, treating both '/' and the platform +// separator as boundaries. Trailing separators are ignored. An empty path, or +// one that is nothing but separators, returns "". +// +// '\' is a boundary only where the platform says so: on POSIX it is an +// ordinary filename character, and treating it as a separator there would +// corrupt legitimate names. +// +// pathx.Base("/models/gemma3-1b") // "gemma3-1b" — every platform +// pathx.Base("org/model-a") // "model-a" — every platform +// pathx.Base(`C:\models\gemma3-1b\`) // "gemma3-1b" on Windows +func Base(p string) string { + p = trimTrailingSeparators(p) + if p == "" { + return "" + } + if i := lastSeparator(p); i >= 0 { + return p[i+1:] + } + return p +} + +// Dir returns all but the last element of p, treating both '/' and the +// platform separator as boundaries. A path with no separator returns "", +// letting callers distinguish "no parent recorded" from the "." that +// core.PathDir reports. +// +// pathx.Dir("/models/my-lora/adapter.safetensors") // "/models/my-lora" +// pathx.Dir("adapter.safetensors") // "" +// pathx.Dir(`C:\models\my-lora\adapter.safetensors`) // `C:\models\my-lora` on Windows +func Dir(p string) string { + p = trimTrailingSeparators(p) + i := lastSeparator(p) + if i < 0 { + return "" + } + if i == 0 { + return p[:1] // a root-anchored path keeps its leading separator + } + return p[:i] +} + +// Join appends child to dir using the separator dir is already written with, +// falling back to the platform's own when dir carries none. +// +// Unlike core.PathJoin it does not clean the result — callers on a hot path +// feed already-canonical directories — and it preserves the caller's +// convention rather than rewriting a '/'-spelled Windows path into a mixed +// one. An empty dir yields the bare child, matching PathJoin's "empty root = +// relative result". +// +// pathx.Join("/models/my-lora", "adapter_config.json") // "/models/my-lora/adapter_config.json" +// pathx.Join("/models/my-lora/", "adapter_config.json") // "/models/my-lora/adapter_config.json" +func Join(dir, child string) string { + if dir == "" { + return child + } + separator := separatorOf(dir) + if core.HasSuffix(dir, separator) { + return dir + child + } + return dir + separator + child +} + +// separatorOf reports the separator p is written with — the last one it +// contains — or the platform's own when it contains none. +func separatorOf(p string) string { + if i := lastSeparator(p); i >= 0 { + return p[i : i+1] + } + if separator := core.Env("DS"); separator != "" { + return separator + } + return "/" +} + +// lastSeparator reports the index of the final '/' or platform separator in p, +// or -1 when p carries neither. +func lastSeparator(p string) int { + best := core.LastIndex(p, "/") + if separator := core.Env("DS"); separator != "" && separator != "/" { + if i := core.LastIndex(p, separator); i > best { + best = i + } + } + return best +} + +// trimTrailingSeparators drops any run of trailing separators, so that a path +// written with a trailing slash names the same element as one without. +func trimTrailingSeparators(p string) string { + separator := core.Env("DS") + for len(p) > 1 { + tail := p[len(p)-1:] + if tail != "/" && (separator == "" || tail != separator) { + break + } + p = p[:len(p)-1] + } + return p +} diff --git a/go/internal/pathx/pathx_test.go b/go/internal/pathx/pathx_test.go new file mode 100644 index 000000000..ac584448b --- /dev/null +++ b/go/internal/pathx/pathx_test.go @@ -0,0 +1,146 @@ +// SPDX-Licence-Identifier: EUPL-1.2 + +package pathx + +import ( + "runtime" + "testing" +) + +// TestPathx_Base_Good pins the ordinary shapes: a '/'-separated absolute path, +// a Hugging Face style ref, and a bare name with no separator at all. All three +// answer identically on every platform, which is the point of the helper — +// core.PathBase hands back the whole string for these on Windows. +func TestPathx_Base_Good(t *testing.T) { + for _, tc := range []struct{ in, want string }{ + {"/models/gemma3-1b", "gemma3-1b"}, + {"/models/gemma4/model.gguf", "model.gguf"}, + {"org/model-a", "model-a"}, + {"model.gguf", "model.gguf"}, + } { + if got := Base(tc.in); got != tc.want { + t.Errorf("Base(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +// TestPathx_Base_Bad covers the inputs that name no element: empty, a lone +// separator, and a run of them. +func TestPathx_Base_Bad(t *testing.T) { + for _, in := range []string{"", "/", "///"} { + if got := Base(in); got != "" { + t.Errorf("Base(%q) = %q, want %q", in, got, "") + } + } +} + +// TestPathx_Base_Ugly covers the surprising-but-valid cases: trailing +// separators name the element before them, and a backslash is a separator only +// where the platform agrees — on POSIX it is a legitimate filename character +// and must survive intact. +func TestPathx_Base_Ugly(t *testing.T) { + if got := Base("/models/gemma3-1b/"); got != "gemma3-1b" { + t.Errorf("Base(trailing slash) = %q, want %q", got, "gemma3-1b") + } + if got := Base("/models/gemma3-1b//"); got != "gemma3-1b" { + t.Errorf("Base(trailing slashes) = %q, want %q", got, "gemma3-1b") + } + + backslashed := `C:\models\gemma3-1b` + got := Base(backslashed) + if runtime.GOOS == "windows" { + if got != "gemma3-1b" { + t.Errorf("Base(%q) = %q, want %q", backslashed, got, "gemma3-1b") + } + return + } + if got != backslashed { + t.Errorf("Base(%q) = %q, want it unchanged — '\\' is a filename character on %s", + backslashed, got, runtime.GOOS) + } +} + +// TestPathx_Join_Good pins the ordinary join and the trailing-separator +// collapse, both of which keep the separator the caller already used. +func TestPathx_Join_Good(t *testing.T) { + for _, tc := range []struct{ dir, child, want string }{ + {"/models/my-lora", "adapter_config.json", "/models/my-lora/adapter_config.json"}, + {"/models/my-lora/", "adapter_config.json", "/models/my-lora/adapter_config.json"}, + {"/", "adapter_config.json", "/adapter_config.json"}, + } { + if got := Join(tc.dir, tc.child); got != tc.want { + t.Errorf("Join(%q, %q) = %q, want %q", tc.dir, tc.child, got, tc.want) + } + } +} + +// TestPathx_Join_Bad covers the empty dir, which yields the bare child rather +// than an accidentally root-anchored path. +func TestPathx_Join_Bad(t *testing.T) { + if got := Join("", "adapter_config.json"); got != "adapter_config.json" { + t.Errorf("Join(empty dir) = %q, want %q", got, "adapter_config.json") + } +} + +// TestPathx_Join_Ugly covers a separator-less dir, which takes the platform's +// own separator — the only case where the result's shape depends on the host. +func TestPathx_Join_Ugly(t *testing.T) { + got := Join(".", "adapter_config.json") + want := "." + separatorOf(".") + "adapter_config.json" + if got != want { + t.Errorf("Join(%q, %q) = %q, want %q", ".", "adapter_config.json", got, want) + } + if runtime.GOOS != "windows" && got != "./adapter_config.json" { + t.Errorf("Join(%q, ...) = %q, want %q on %s", ".", got, "./adapter_config.json", runtime.GOOS) + } +} + +// TestPathx_Dir_Good pins the parent of a nested path and of a path anchored +// directly at the root. +func TestPathx_Dir_Good(t *testing.T) { + for _, tc := range []struct{ in, want string }{ + {"/models/my-lora/adapter.safetensors", "/models/my-lora"}, + {"/models/my-lora", "/models"}, + {"/models", "/"}, + {"org/model-a", "org"}, + } { + if got := Dir(tc.in); got != tc.want { + t.Errorf("Dir(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +// TestPathx_Dir_Bad covers the inputs with no parent to report. Unlike +// core.PathDir these answer "" rather than ".", so a caller can tell "no +// directory component" from "the current directory". +func TestPathx_Dir_Bad(t *testing.T) { + for _, in := range []string{"", "adapter.safetensors"} { + if got := Dir(in); got != "" { + t.Errorf("Dir(%q) = %q, want %q", in, got, "") + } + } +} + +// TestPathx_Dir_Ugly covers trailing separators — "/models/x/" has the same +// parent as "/models/x" — and the platform-dependent backslash. +func TestPathx_Dir_Ugly(t *testing.T) { + if got := Dir("/models/my-lora/"); got != "/models" { + t.Errorf("Dir(trailing slash) = %q, want %q", got, "/models") + } + if got := Dir("/"); got != "/" { + t.Errorf("Dir(root) = %q, want %q — the root is its own parent, as filepath.Dir has it", got, "/") + } + + backslashed := `C:\models\my-lora\adapter.safetensors` + got := Dir(backslashed) + if runtime.GOOS == "windows" { + if got != `C:\models\my-lora` { + t.Errorf("Dir(%q) = %q, want %q", backslashed, got, `C:\models\my-lora`) + } + return + } + if got != "" { + t.Errorf("Dir(%q) = %q, want %q — '\\' is a filename character on %s", + backslashed, got, "", runtime.GOOS) + } +} diff --git a/go/model/arch/google/gemma3/gguf/convert.go b/go/model/arch/google/gemma3/gguf/convert.go index 6576959f1..58a33acb3 100644 --- a/go/model/arch/google/gemma3/gguf/convert.go +++ b/go/model/arch/google/gemma3/gguf/convert.go @@ -7,6 +7,7 @@ import ( "math" core "dappco.re/go" + "dappco.re/go/inference/internal/pathx" basegguf "dappco.re/go/inference/model/gguf" ) @@ -112,8 +113,12 @@ func quantizeGemma3ModelPack(source basegguf.Source, configJSON []byte, tensors // gemma3ModelName derives a general.name from the checkpoint directory basename, // dropping the -bf16/-f32 dense-precision suffix. An empty result simply omits // general.name. +// +// pathx.Base, not core.PathBase: the checkpoint root reaches here from a CLI +// flag or a config file, so it may well be '/'-separated on Windows — where +// core.PathBase would find no separator and return the whole path as the name. func gemma3ModelName(root string) string { - base := core.PathBase(core.TrimSuffix(root, "/")) + base := pathx.Base(root) base = core.TrimSuffix(base, "-bf16") base = core.TrimSuffix(base, "-f32") return base diff --git a/go/serving/compat/mux.go b/go/serving/compat/mux.go index bc9ee2a4a..38c96a255 100644 --- a/go/serving/compat/mux.go +++ b/go/serving/compat/mux.go @@ -26,6 +26,7 @@ import ( core "dappco.re/go" "dappco.re/go/inference" "dappco.re/go/inference/decode/parser" + "dappco.re/go/inference/internal/pathx" anthropiccompat "dappco.re/go/inference/serving/provider/anthropic" ollamacompat "dappco.re/go/inference/serving/provider/ollama" openaicompat "dappco.re/go/inference/serving/provider/openai" @@ -1241,7 +1242,10 @@ func resolverModelNames(resolver openaicompat.Resolver) []string { return lister.ModelNames() } if backend, ok := resolver.(*openaicompat.BackendResolver); ok && backend != nil && backend.ModelPath != "" { - return []string{core.PathBase(backend.ModelPath)} + // pathx.Base: the advertised model id must be the file's own name even + // when ModelPath arrived '/'-separated on Windows, where core.PathBase + // would publish the entire path as the id. + return []string{pathx.Base(backend.ModelPath)} } return nil } diff --git a/go/train/lora/inspect.go b/go/train/lora/inspect.go index 7fbf1eadb..25f02f1e7 100644 --- a/go/train/lora/inspect.go +++ b/go/train/lora/inspect.go @@ -12,6 +12,7 @@ import ( "slices" core "dappco.re/go" + "dappco.re/go/inference/internal/pathx" "dappco.re/go/inference/model/state" ) @@ -101,7 +102,7 @@ func Inspect(path string, identityPath string) (AdapterInfo, error) { return AdapterInfo{}, core.E("lora.Inspect", "parse adapter_config.json", err) } info := AdapterInfo{ - Name: core.PathBase(identityPath), + Name: pathx.Base(identityPath), Path: identityPath, Rank: cfg.Rank, Alpha: cfg.Alpha, @@ -116,10 +117,9 @@ func adapterConfigPath(path string) string { return adapterConfigPathPrecomputed(path, core.HasSuffix(path, ".safetensors")) } -// adapterConfigSuffix carries the leading separator inline so the -// concat-path can drop it cheaply when the input already ends in '/' -// (matching filepath.Join's separator-collapse semantics). -const adapterConfigSuffix = "/adapter_config.json" +// adapterConfigFilename is the sidecar an adapter directory carries; pathx.Join +// supplies whichever separator the directory is already written with. +const adapterConfigFilename = "adapter_config.json" // joinDirChildPattern concatenates a directory path with a relative // child segment, collapsing the duplicate separator when dir already @@ -143,26 +143,27 @@ func joinDirChildPattern(dir, child string) string { // adapterConfigPath; the Inspect hot path computes the .safetensors // suffix check once and threads the result through this helper. // -// Builds the joined path with a direct concat instead of routing through +// Builds the joined path with pathx.Join instead of routing through // core.PathJoin (filepath.Join → filepath.Clean): filepath.Clean always // allocates an internal lazybuf even when the inputs are already canonical, // roughly doubling the cost of producing the result string. Both Inspect // callers feed an already-cleaned adapter path, so the only normalisation -// we need is the "collapse a duplicate '/'" rule that filepath.Join uses -// when joining a path that already ends in '/'. +// we need is the trailing-separator collapse, which pathx.Join does. +// +// pathx rather than core for the split as well: core.PathDir matches only the +// platform separator, so on Windows it reports "." for the whole of a +// '/'-spelled adapter path and the sidecar is looked for in the wrong place. func adapterConfigPathPrecomputed(path string, isSafetensors bool) string { base := path if isSafetensors { - // PathDir returns a substring of path (no alloc); strip the + // pathx.Dir returns a substring of path (no alloc); strip the // trailing weight-file segment so the join targets the parent dir. - base = core.PathDir(path) - } - // Trailing-slash collapse: when base ends in '/', skip the leading - // '/' from adapterConfigSuffix to avoid producing "//adapter_config". - if len(base) > 0 && base[len(base)-1] == '/' { - return base + adapterConfigSuffix[1:] + // A bare filename has no parent — its sidecar sits beside it. + if base = pathx.Dir(path); base == "" { + base = "." + } } - return base + adapterConfigSuffix + return pathx.Join(base, adapterConfigFilename) } func hashAdapter(path string, config []byte) string { From 5b04ab521640b2525c00412770b0a50c0576c42d Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 09:23:18 +0100 Subject: [PATCH 3/5] fix(scheduler): floor the total and decode spans too, not just prefill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The windows lane's first run past the prefill fix moved the failure to the next assertion in the same test: "final durations Total=0s Decode=0s". Same cause — Windows' monotonic tick cannot resolve the simulated lane's spans, so time.Since returns exactly 0 — and the same consequence, since DecodeTokensPerSec's divide is guarded on a positive DecodeDuration. Route both through measuredSpan, as PrefillDuration already was. Receipt: 46 → 41 failing packages on the windows lane from the previous commit (run 31247873871, job 93079323014); serving/scheduler was the one package whose failure moved rather than cleared. go test -count=1 ./serving/scheduler/ → ok 0.551s Co-Authored-By: Virgil --- go/serving/scheduler/cb_step.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/go/serving/scheduler/cb_step.go b/go/serving/scheduler/cb_step.go index 48796d875..d040ce5ec 100644 --- a/go/serving/scheduler/cb_step.go +++ b/go/serving/scheduler/cb_step.go @@ -75,11 +75,14 @@ type cbReq struct { // durations rung). func (r *cbReq) stampMetrics() { r.metrics.PrefillDuration = r.prefillDur + // measuredSpan, not a bare time.Since: a span the platform clock cannot + // resolve still elapsed, and a zero here would suppress the throughput + // rates below, whose divides are guarded on a positive duration. if !r.start.IsZero() { - r.metrics.TotalDuration = time.Since(r.start) + r.metrics.TotalDuration = measuredSpan(r.start) } if !r.decodeStart.IsZero() { - r.metrics.DecodeDuration = time.Since(r.decodeStart) + r.metrics.DecodeDuration = measuredSpan(r.decodeStart) } if r.prefillDur > 0 && r.metrics.PromptTokens > 0 { r.metrics.PrefillTokensPerSec = float64(r.metrics.PromptTokens) / r.prefillDur.Seconds() @@ -250,10 +253,10 @@ type cbPrepared struct { } // measuredSpan reports the time elapsed since start, floored at one -// nanosecond. A prefill that finished faster than the platform's monotonic -// tick can resolve — Windows' clock returns exactly 0 for a sub-microsecond -// span — still happened, and reporting 0 would both blank PrefillDuration and -// suppress PrefillTokensPerSec, whose divide is guarded on a positive span. +// nanosecond. Work that finished faster than the platform's monotonic tick can +// resolve — Windows' clock returns exactly 0 for a sub-microsecond span — still +// happened, and reporting 0 would both blank the duration and suppress the +// throughput rate derived from it, whose divide is guarded on a positive span. func measuredSpan(start time.Time) time.Duration { if elapsed := time.Since(start); elapsed > 0 { return elapsed From c277a2e1d13778506a46a4921078fba7d08660f7 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 09:34:37 +0100 Subject: [PATCH 4/5] test(lora): stop pinning core.PathBase in the identity assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pathx switch made TestInspect_Inspect_Good fail on the windows lane — a test that passed before, so a regression I introduced. It asserted info.Name != core.PathBase(identityPath), recomputing the expectation with the same DS-literal helper the production code had just moved off: with identityPath spelled "/adapters/original/support-tone", Inspect now correctly answers "support-tone" while core.PathBase on Windows answers the whole path. Spell the expected name out instead, so the assertion pins the behaviour rather than a helper that shares the bug. Receipt: go test -count=1 ./train/lora/ → ok 0.296s The other three lora failures this lane started with are already cleared — run 31248300740 shows Name resolving to "support-tone" on Windows. Co-Authored-By: Virgil --- go/train/lora/inspect_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/go/train/lora/inspect_test.go b/go/train/lora/inspect_test.go index b9b2df372..aa6cb7194 100644 --- a/go/train/lora/inspect_test.go +++ b/go/train/lora/inspect_test.go @@ -353,7 +353,11 @@ func TestInspect_Inspect_Good(t *testing.T) { if err != nil { t.Fatalf("Inspect() error = %v", err) } - if info.Path != identityPath || info.Name != core.PathBase(identityPath) { + // The expected name is spelled out rather than recomputed with a path + // helper: core.PathBase matches only the platform separator and would + // hand back the whole identityPath on Windows, so asserting against it + // would pin the bug pathx.Base exists to fix. + if info.Path != identityPath || info.Name != "support-tone" { t.Fatalf("adapter identity = %+v, want name/path derived from identityPath %q", info, identityPath) } if info.Rank != 8 || info.Alpha != 16 || info.Hash == "" { From e38647334f49ea5673a7b6028788d5e3ae7ee978 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 09:37:32 +0100 Subject: [PATCH 5/5] perf(pathx): resolve the separator once, matching discover.go's precedent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pathx called core.Env("DS") up to three times per Base/Dir/Join. Env walks systemInfo's map and falls back to os.Getenv when the key is unset — the common case for "DS" — and lora's adapterConfigPathPrecomputed, which the surrounding comment documents as an Inspect hot path, now routes through here. Cache it behind sync.Once exactly as discover.go's pathSeparator already does for joinPath/cleanPath, and for the same reason: the override is set once at process start and never mutates. Receipt: go test -count=1 ./internal/pathx/ ./train/lora/ ./eval/bench/ \ ./serving/compat/ ./model/arch/google/gemma3/gguf/ ok internal/pathx 0.289s · train/lora 0.335s · eval/bench 0.570s ok serving/compat 0.699s · gemma3/gguf 0.947s Co-Authored-By: Virgil --- go/internal/pathx/pathx.go | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/go/internal/pathx/pathx.go b/go/internal/pathx/pathx.go index 2cf3c2ba9..5cc4f5202 100644 --- a/go/internal/pathx/pathx.go +++ b/go/internal/pathx/pathx.go @@ -15,7 +15,32 @@ // it with core.PathJoin. package pathx -import core "dappco.re/go" +import ( + "sync" + + core "dappco.re/go" +) + +var ( + separatorOnce sync.Once + separatorCache string +) + +// nativeSeparator resolves the directory separator once per process and caches +// it, mirroring discover.go's pathSeparator. core.Env walks a map and falls +// back to os.Getenv when the key is unset, and these helpers sit on hot paths +// (lora's Inspect resolves an adapter sidecar per call). The override is set +// once at process start, typically by tests, and never mutates. +func nativeSeparator() string { + separatorOnce.Do(func() { + if separator := core.Env("DS"); separator != "" { + separatorCache = separator + return + } + separatorCache = "/" + }) + return separatorCache +} // Base returns the last element of p, treating both '/' and the platform // separator as boundaries. Trailing separators are ignored. An empty path, or @@ -87,17 +112,14 @@ func separatorOf(p string) string { if i := lastSeparator(p); i >= 0 { return p[i : i+1] } - if separator := core.Env("DS"); separator != "" { - return separator - } - return "/" + return nativeSeparator() } // lastSeparator reports the index of the final '/' or platform separator in p, // or -1 when p carries neither. func lastSeparator(p string) int { best := core.LastIndex(p, "/") - if separator := core.Env("DS"); separator != "" && separator != "/" { + if separator := nativeSeparator(); separator != "/" { if i := core.LastIndex(p, separator); i > best { best = i } @@ -108,10 +130,10 @@ func lastSeparator(p string) int { // trimTrailingSeparators drops any run of trailing separators, so that a path // written with a trailing slash names the same element as one without. func trimTrailingSeparators(p string) string { - separator := core.Env("DS") + separator := nativeSeparator() for len(p) > 1 { tail := p[len(p)-1:] - if tail != "/" && (separator == "" || tail != separator) { + if tail != "/" && tail != separator { break } p = p[:len(p)-1]