From 4d24d8a77d1bf46da2d44f26a2929ee8f6d3e4df Mon Sep 17 00:00:00 2001 From: Michael Yang Date: Tue, 18 Nov 2025 14:07:58 -0800 Subject: [PATCH] gocritic --- .golangci.yaml | 6 ++++++ app/dialog/dlgs.go | 1 + app/server/server.go | 6 ++---- app/tools/browser.go | 24 ++++++++--------------- cmd/cmd_test.go | 12 ++++-------- convert/convert_gptoss.go | 2 +- fs/ggml/gguf.go | 2 +- harmony/harmonyparser.go | 4 +--- llm/server.go | 12 +++++------- middleware/openai_encoding_format_test.go | 6 ++---- ml/device.go | 6 +++--- model/bytepairencoding.go | 8 ++++---- model/model.go | 2 +- parser/parser_test.go | 12 +++--------- runner/llamarunner/image.go | 2 +- runner/llamarunner/runner.go | 1 - runner/ollamarunner/runner.go | 1 - sample/transforms.go | 2 +- server/routes.go | 16 ++++++--------- server/routes_debug_test.go | 12 ++++-------- server/sched.go | 6 ++---- thinking/template.go | 6 ++---- 22 files changed, 58 insertions(+), 91 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 2aefd3fb7..7db2b858b 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -36,6 +36,12 @@ linters: errcheck: exclude-functions: - fmt.Fprintf + gocritic: + disabled-checks: + # Detects suspicious duplicated sub-expressions. + # Prone to false positives when used on cgo code + # https://github.com/go-critic/go-critic/issues/897#issuecomment-568892104 + - dupSubExpr perfsprint: strconcat: false concat-loop: false diff --git a/app/dialog/dlgs.go b/app/dialog/dlgs.go index fa99795e3..9a5448d35 100644 --- a/app/dialog/dlgs.go +++ b/app/dialog/dlgs.go @@ -22,6 +22,7 @@ import ( var ErrCancelled = errors.New("Cancelled") // Cancelled refers to ErrCancelled. +// // Deprecated: Use ErrCancelled instead. var Cancelled = ErrCancelled diff --git a/app/server/server.go b/app/server/server.go index 3791908af..2109b942a 100644 --- a/app/server/server.go +++ b/app/server/server.go @@ -345,11 +345,9 @@ func GetInferenceComputer(ctx context.Context) ([]InferenceCompute, error) { slog.Info("Matched", "inference compute", ic) inference = append(inference, ic) - } else { + } else if len(inference) > 0 { // Break out on first non matching line after we start matching - if len(inference) > 0 { - return inference, nil - } + return inference, nil } } time.Sleep(100 * time.Millisecond) diff --git a/app/tools/browser.go b/app/tools/browser.go index b85c05334..cec7993e2 100644 --- a/app/tools/browser.go +++ b/app/tools/browser.go @@ -384,15 +384,9 @@ func wrapLines(text string, width int) []string { wrapped = append(wrapped, "") } else if len(line) <= width { wrapped = append(wrapped, line) + } else if words := strings.Fields(line); len(words) == 0 { + wrapped = append(wrapped, line) } else { - // Word wrapping while preserving whitespace structure - words := strings.Fields(line) - if len(words) == 0 { - // Line with only whitespace - wrapped = append(wrapped, line) - continue - } - currentLine := "" for _, word := range words { // Check if adding this word would exceed width @@ -537,15 +531,13 @@ func (b *BrowserOpen) Execute(ctx context.Context, args map[string]any) (any, st if err != nil { return nil, "", fmt.Errorf("page not found for cursor %d: %w", cursor, err) } - } else { + } else if len(b.state.Data.PageStack) != 0 { // get last page - if len(b.state.Data.PageStack) != 0 { - pageURL := b.state.Data.PageStack[len(b.state.Data.PageStack)-1] - var err error - page, err = b.getPageFromStack(pageURL) - if err != nil { - return nil, "", fmt.Errorf("page not found for cursor %d: %w", cursor, err) - } + pageURL := b.state.Data.PageStack[len(b.state.Data.PageStack)-1] + var err error + page, err = b.getPageFromStack(pageURL) + if err != nil { + return nil, "", fmt.Errorf("page not found for cursor %d: %w", cursor, err) } } diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go index b604d8ce6..60cd938b6 100644 --- a/cmd/cmd_test.go +++ b/cmd/cmd_test.go @@ -924,10 +924,8 @@ func TestPushHandler(t *testing.T) { t.Errorf("expected output %q, got %q", tt.expectedOutput, got) } } - } else { - if err == nil || !strings.Contains(err.Error(), tt.expectedError) { - t.Errorf("expected error containing %q, got %v", tt.expectedError, err) - } + } else if err == nil || !strings.Contains(err.Error(), tt.expectedError) { + t.Errorf("expected error containing %q, got %v", tt.expectedError, err) } }) } @@ -1014,10 +1012,8 @@ func TestListHandler(t *testing.T) { if got := string(output); got != tt.expectedOutput { t.Errorf("expected output:\n%s\ngot:\n%s", tt.expectedOutput, got) } - } else { - if err == nil || !strings.Contains(err.Error(), tt.expectedError) { - t.Errorf("expected error containing %q, got %v", tt.expectedError, err) - } + } else if err == nil || !strings.Contains(err.Error(), tt.expectedError) { + t.Errorf("expected error containing %q, got %v", tt.expectedError, err) } }) } diff --git a/convert/convert_gptoss.go b/convert/convert_gptoss.go index d7bfb361d..297c3cf8e 100644 --- a/convert/convert_gptoss.go +++ b/convert/convert_gptoss.go @@ -111,7 +111,7 @@ func (m *gptossModel) Tensors(ts []Tensor) []*ggml.Tensor { for name, mxfp4 := range mxfp4s { dims := mxfp4.blocks.Shape() if !strings.HasSuffix(name, ".weight") { - name = name + ".weight" + name += ".weight" } if strings.Contains(name, "ffn_down_exps") { out = append(out, &ggml.Tensor{ diff --git a/fs/ggml/gguf.go b/fs/ggml/gguf.go index 11c57b0aa..81f00af11 100644 --- a/fs/ggml/gguf.go +++ b/fs/ggml/gguf.go @@ -226,7 +226,7 @@ func (llm *gguf) Decode(rs io.ReadSeeker) error { Name: name, Kind: kind, Offset: offset, - Shape: shape[:], + Shape: shape, } llm.tensors = append(llm.tensors, &tensor) diff --git a/harmony/harmonyparser.go b/harmony/harmonyparser.go index 902729b14..d2b7832cb 100644 --- a/harmony/harmonyparser.go +++ b/harmony/harmonyparser.go @@ -200,9 +200,7 @@ func (s *HarmonyParser) parseHeader(raw string) HarmonyHeader { before := raw[:channelIndex] after := raw[channelIndex+len("<|channel|>"):] // the channel name is `after` all the way up to the first (if any) whitespace character - idx := strings.IndexFunc(after, func(r rune) bool { - return unicode.IsSpace(r) - }) + idx := strings.IndexFunc(after, unicode.IsSpace) if idx == -1 { idx = len(after) } diff --git a/llm/server.go b/llm/server.go index 79b69df01..0d5fc994b 100644 --- a/llm/server.go +++ b/llm/server.go @@ -982,13 +982,11 @@ nextLayer: slog.Warn("model request too large for system", "requested", format.HumanBytes2(cpuSize), "available", format.HumanBytes2(available), "total", format.HumanBytes2(systemInfo.TotalMemory), "free", format.HumanBytes2(systemInfo.FreeMemory), "swap", format.HumanBytes2(systemInfo.FreeSwap)) return fmt.Errorf("model requires more system memory (%s) than is available (%s)", format.HumanBytes2(cpuSize), format.HumanBytes2(available)) } - } else { - if vramSize > systemInfo.TotalMemory { - // disable partial offloading when model is greater than total system memory as this - // can lead to locking up the system - s.options.NumGPU = 0 - gpuLayers = ml.GPULayersList{} - } + } else if vramSize > systemInfo.TotalMemory { + // disable partial offloading when model is greater than total system memory as this + // can lead to locking up the system + s.options.NumGPU = 0 + gpuLayers = ml.GPULayersList{} } if gpuLayers.Sum() == 0 { diff --git a/middleware/openai_encoding_format_test.go b/middleware/openai_encoding_format_test.go index d3797e1c1..80981b42b 100644 --- a/middleware/openai_encoding_format_test.go +++ b/middleware/openai_encoding_format_test.go @@ -210,10 +210,8 @@ func TestEmbeddingsMiddleware_InvalidEncodingFormat(t *testing.T) { if !strings.Contains(errResp.Error.Message, "encoding_format") { t.Errorf("expected error message to mention encoding_format, got %q", errResp.Error.Message) } - } else { - if resp.Code != http.StatusOK { - t.Errorf("expected status 200, got %d: %s", resp.Code, resp.Body.String()) - } + } else if resp.Code != http.StatusOK { + t.Errorf("expected status 200, got %d: %s", resp.Code, resp.Body.String()) } }) } diff --git a/ml/device.go b/ml/device.go index 7b3398b8d..1d9a6f4de 100644 --- a/ml/device.go +++ b/ml/device.go @@ -543,12 +543,12 @@ func (d DeviceInfo) updateVisibleDevicesEnv(env map[string]string) { } v, existing := env[envVar] if existing { - v = v + "," + v += "," } if d.FilterID != "" { - v = v + d.FilterID + v += d.FilterID } else { - v = v + d.ID + v += d.ID } env[envVar] = v } diff --git a/model/bytepairencoding.go b/model/bytepairencoding.go index acb58743b..9c9ba7f63 100644 --- a/model/bytepairencoding.go +++ b/model/bytepairencoding.go @@ -143,9 +143,9 @@ func (bpe BytePairEncoding) Encode(s string, addSpecial bool) ([]int32, error) { case r == 0x00ad: r = 0x0143 case r <= 0x0020: - r = r + 0x0100 + r += 0x0100 case r >= 0x007f && r <= 0x00a0: - r = r + 0x00a2 + r += 0x00a2 } sb.WriteRune(r) @@ -264,9 +264,9 @@ func (bpe BytePairEncoding) Decode(ids []int32) (string, error) { case r == 0x0143: r = 0x00ad case r > 0x0100 && r <= 0x0120: - r = r - 0x0100 + r -= 0x0100 case r > 0x0120 && r <= 0x0142: - r = r - 0x00a2 + r -= 0x00a2 } // NOTE: not using WriteRune here because it writes the UTF-8 diff --git a/model/model.go b/model/model.go index 1e966b456..3c69986b4 100644 --- a/model/model.go +++ b/model/model.go @@ -146,7 +146,7 @@ func NewTextProcessor(s string) (TextProcessor, error) { func modelForArch(c fs.Config) (Model, error) { arch := c.Architecture() if pooling.Type(c.Uint("pooling_type")) != pooling.TypeNone { - arch = arch + "_embed" + arch += "_embed" } f, ok := models[arch] diff --git a/parser/parser_test.go b/parser/parser_test.go index 3300aad3e..478e56aae 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -326,17 +326,11 @@ MESSAGE system`, return } - switch tt.err.(type) { - case *ParserError: - var pErr *ParserError - if errors.As(err, &pErr) { - // got the correct type of error - return - } - } - if errors.Is(err, tt.err) { return + } else if pErr := (*ParserError)(nil); errors.As(err, &pErr) { + // got the correct type of error + return } t.Fatalf("unexpected error: expected: %v, actual: %v", tt.err, err) diff --git a/runner/llamarunner/image.go b/runner/llamarunner/image.go index 9fc970811..2143fbe4a 100644 --- a/runner/llamarunner/image.go +++ b/runner/llamarunner/image.go @@ -61,7 +61,7 @@ func (c *ImageContext) MultimodalTokenize(llamaContext *llama.Context, data []by return nil, nil } - if len(data) <= 0 { + if len(data) == 0 { return nil, errors.New("received zero length image") } diff --git a/runner/llamarunner/runner.go b/runner/llamarunner/runner.go index c42da9b53..e4c8528ff 100644 --- a/runner/llamarunner/runner.go +++ b/runner/llamarunner/runner.go @@ -995,7 +995,6 @@ func Execute(args []string) error { log.Println("Server listening on", addr) if err := httpServer.Serve(listener); err != nil { - log.Fatal("server error:", err) return err } diff --git a/runner/ollamarunner/runner.go b/runner/ollamarunner/runner.go index 45804427e..e2a9c5487 100644 --- a/runner/ollamarunner/runner.go +++ b/runner/ollamarunner/runner.go @@ -1429,7 +1429,6 @@ func Execute(args []string) error { log.Println("Server listening on", addr) if err := httpServer.Serve(listener); err != nil { - log.Fatal("server error:", err) return err } diff --git a/sample/transforms.go b/sample/transforms.go index 3f677553f..c7e9d7388 100644 --- a/sample/transforms.go +++ b/sample/transforms.go @@ -30,7 +30,7 @@ func temperature(ts []token, temp float32) { // Ensure temperature clipping near 0 to avoid numerical instability temp = max(temp, 1e-7) for i := range ts { - ts[i].value = ts[i].value / temp + ts[i].value /= temp } } diff --git a/server/routes.go b/server/routes.go index c6ebddee2..cee18cf6f 100644 --- a/server/routes.go +++ b/server/routes.go @@ -362,11 +362,9 @@ func (s *Server) GenerateHandler(c *gin.Context) { if req.Think == nil { req.Think = &api.ThinkValue{Value: true} } - } else { - if req.Think != nil && req.Think.Bool() { - c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("%q does not support thinking", req.Model)}) - return - } + } else if req.Think != nil && req.Think.Bool() { + c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("%q does not support thinking", req.Model)}) + return } r, m, opts, err := s.scheduleRunner(c.Request.Context(), name.String(), caps, req.Options, req.KeepAlive) @@ -1996,11 +1994,9 @@ func (s *Server) ChatHandler(c *gin.Context) { if req.Think == nil { req.Think = &api.ThinkValue{Value: true} } - } else { - if req.Think != nil && req.Think.Bool() { - c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("%q does not support thinking", req.Model)}) - return - } + } else if req.Think != nil && req.Think.Bool() { + c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("%q does not support thinking", req.Model)}) + return } r, m, opts, err := s.scheduleRunner(c.Request.Context(), name.String(), caps, req.Options, req.KeepAlive) diff --git a/server/routes_debug_test.go b/server/routes_debug_test.go index 6f9104c39..a618b6ada 100644 --- a/server/routes_debug_test.go +++ b/server/routes_debug_test.go @@ -196,11 +196,9 @@ func TestGenerateDebugRenderOnly(t *testing.T) { if tt.expectNumImages > 0 && response.DebugInfo.ImageCount != tt.expectNumImages { t.Errorf("expected image count %d, got %d", tt.expectNumImages, response.DebugInfo.ImageCount) } - } else { + } else if w.Code != http.StatusOK { // When debug is disabled, it should attempt normal processing - if w.Code != http.StatusOK { - t.Errorf("expected status %d, got %d", http.StatusOK, w.Code) - } + t.Errorf("expected status %d, got %d", http.StatusOK, w.Code) } }) } @@ -401,11 +399,9 @@ func TestChatDebugRenderOnly(t *testing.T) { if tt.expectNumImages > 0 && response.DebugInfo.ImageCount != tt.expectNumImages { t.Errorf("expected image count %d, got %d", tt.expectNumImages, response.DebugInfo.ImageCount) } - } else { + } else if w.Code != http.StatusOK { // When debug is disabled, it should attempt normal processing - if w.Code != http.StatusOK { - t.Errorf("expected status %d, got %d", http.StatusOK, w.Code) - } + t.Errorf("expected status %d, got %d", http.StatusOK, w.Code) } }) } diff --git a/server/sched.go b/server/sched.go index 84ca18dd5..68eef162f 100644 --- a/server/sched.go +++ b/server/sched.go @@ -429,10 +429,8 @@ func (s *Scheduler) load(req *LlmRequest, f *ggml.GGML, systemInfo ml.SystemInfo } s.activeLoading = llama - } else { - if s.activeLoading.ModelPath() != req.model.ModelPath { - panic(fmt.Errorf("attempting to load different model after eviction (original %v new %v)", s.activeLoading.ModelPath(), req.model.ModelPath)) - } + } else if s.activeLoading.ModelPath() != req.model.ModelPath { + panic(fmt.Errorf("attempting to load different model after eviction (original %v new %v)", s.activeLoading.ModelPath(), req.model.ModelPath)) } s.loadedMu.Unlock() diff --git a/thinking/template.go b/thinking/template.go index 798fa34c7..5f9d328d5 100644 --- a/thinking/template.go +++ b/thinking/template.go @@ -68,8 +68,7 @@ func InferTags(t *template.Template) (string, string) { enterFn := func(n parse.Node) bool { ancestors = append(ancestors, n) - switch x := n.(type) { - case *parse.FieldNode: + if x, ok := n.(*parse.FieldNode); ok { if len(x.Ident) > 0 && x.Ident[0] == "Thinking" { var mostRecentRange *parse.RangeNode for i := len(ancestors) - 1; i >= 0; i-- { @@ -121,8 +120,7 @@ func InferTags(t *template.Template) (string, string) { func rangeUsesField(rangeNode *parse.RangeNode, field string) bool { found := false enterFn := func(n parse.Node) bool { - switch x := n.(type) { - case *parse.FieldNode: + if x, ok := n.(*parse.FieldNode); ok { if x.Ident[0] == field { found = true }