From f796f3645fb1f70eccfd1b8dd255eac7e8c7695e Mon Sep 17 00:00:00 2001 From: dylanhuff-at-coder Date: Thu, 2 Apr 2026 10:07:01 -0700 Subject: [PATCH] fix(coderd): fix isContextLimitKey false positive on max_context_version (#23950) `isContextLimitKey` had a fallback heuristic that matched any key starting with `"max"` containing `"context"`, causing false positives on keys like `"max_context_version"`. A provider returning such metadata would have the value parsed as a context limit. Replace substring matching on the separator-stripped key with word-level matching. A new `metadataKeyWords` function tokenizes keys by splitting on separators and camelCase boundaries, then the fallback requires `"context"` paired with a limit-related word (`"limit"`, `"window"` + qualifier, `"length"` + qualifier, or `"tokens"` + qualifier). Known exact forms like `"context_window"` remain in the fast-path switch. Closes https://github.com/coder/coder/issues/23332 --- coderd/x/chatd/chatloop/chatloop.go | 59 +++++++++++++++++-- .../chatloop/contextlimit_internal_test.go | 58 ++++++++++++++---- 2 files changed, 101 insertions(+), 16 deletions(-) diff --git a/coderd/x/chatd/chatloop/chatloop.go b/coderd/x/chatd/chatloop/chatloop.go index 011c34ca61..6453c60f23 100644 --- a/coderd/x/chatd/chatloop/chatloop.go +++ b/coderd/x/chatd/chatloop/chatloop.go @@ -10,6 +10,7 @@ import ( "strings" "sync" "time" + "unicode" "charm.land/fantasy" fantasyanthropic "charm.land/fantasy/providers/anthropic" @@ -1317,11 +1318,25 @@ func isContextLimitKey(key string) bool { return true } - return strings.Contains(normalized, "context") && - (strings.Contains(normalized, "limit") || - strings.Contains(normalized, "window") || - strings.Contains(normalized, "length") || - strings.HasPrefix(normalized, "max")) + words := metadataKeyWords(key) + if !slices.Contains(words, "context") { + return false + } + + if slices.Contains(words, "limit") { + return true + } + + if slices.Contains(words, "window") { + return slices.Contains(words, "size") || slices.Contains(words, "max") + } + + if slices.Contains(words, "length") { + return slices.Contains(words, "max") + } + + return (slices.Contains(words, "token") || slices.Contains(words, "tokens")) && + (slices.Contains(words, "max") || slices.Contains(words, "limit")) } func normalizeMetadataKey(key string) string { @@ -1342,6 +1357,40 @@ func normalizeMetadataKey(key string) string { return b.String() } +func metadataKeyWords(key string) []string { + words := make([]string, 0, 4) + var current strings.Builder + + flush := func() { + if current.Len() == 0 { + return + } + words = append(words, current.String()) + current.Reset() + } + + var prev rune + var hasPrev bool + for _, r := range key { + if !unicode.IsLetter(r) { + flush() + hasPrev = false + continue + } + + if hasPrev && unicode.IsUpper(r) && unicode.IsLower(prev) { + flush() + } + + _, _ = current.WriteRune(unicode.ToLower(r)) + prev = r + hasPrev = true + } + + flush() + return words +} + func numericContextLimitValue(value any) (int64, bool) { switch typed := value.(type) { case int64: diff --git a/coderd/x/chatd/chatloop/contextlimit_internal_test.go b/coderd/x/chatd/chatloop/contextlimit_internal_test.go index 85e490f425..f70fad09de 100644 --- a/coderd/x/chatd/chatloop/contextlimit_internal_test.go +++ b/coderd/x/chatd/chatloop/contextlimit_internal_test.go @@ -53,6 +53,29 @@ func TestNormalizeMetadataKey(t *testing.T) { } } +func TestMetadataKeyWords(t *testing.T) { + t.Parallel() + + tests := []struct { + key string + want []string + }{ + {"max_context_tokens", []string{"max", "context", "tokens"}}, + {"maxContextTokens", []string{"max", "context", "tokens"}}, + {"MAX_CONTEXT", []string{"max", "context"}}, + {"ContextWindow", []string{"context", "window"}}, + {"context2limit", []string{"context", "limit"}}, + {"", []string{}}, + } + for _, tt := range tests { + t.Run(tt.key, func(t *testing.T) { + t.Parallel() + got := metadataKeyWords(tt.key) + require.Equal(t, tt.want, got) + }) + } +} + func TestIsContextLimitKey(t *testing.T) { t.Parallel() @@ -60,7 +83,6 @@ func TestIsContextLimitKey(t *testing.T) { name string key string want bool - skip bool }{ // Exact matches after normalization. {name: "context_limit", key: "context_limit", want: true}, {name: "context_window", key: "context_window", want: true}, @@ -75,18 +97,22 @@ func TestIsContextLimitKey(t *testing.T) { {name: "Context-Window mixed case", key: "Context-Window", want: true}, {name: "MAX_CONTEXT_TOKENS screaming", key: "MAX_CONTEXT_TOKENS", want: true}, {name: "contextLimit camelCase", key: "contextLimit", want: true}, + {name: "modelContextLimit camelCase", key: "modelContextLimit", want: true}, - // Fallback heuristic: contains "context" + limit/window/length. + // Fallback heuristic: tokenized "context" + limit/window/length. {name: "model_context_limit", key: "model_context_limit", want: true}, {name: "context_window_size", key: "context_window_size", want: true}, {name: "context_length_max", key: "context_length_max", want: true}, - // Fallback heuristic: starts with "max" + contains "context". - // BUG(isContextLimitKey): "max_context_version" matches - // because it contains "context" and starts with "max", - // but a version field is not a context limit. - // TODO: Fix the heuristic and remove this skip. - {name: "max_context_version false positive", key: "max_context_version", want: false, skip: true}, // Non-matching keys. + // Exact matches remain valid after separator stripping. + {name: "max_context_", key: "max_context_", want: true}, + {name: "max_context_limit", key: "max_context_limit", want: true}, + + // Non-matching keys should not be treated as context limits. + {name: "max_context_version false positive", key: "max_context_version", want: false}, + {name: "context_tokens_used false positive", key: "context_tokens_used", want: false}, + {name: "context_length_used false positive", key: "context_length_used", want: false}, + {name: "context_window_used false positive", key: "context_window_used", want: false}, {name: "context_id no limit keyword", key: "context_id", want: false}, {name: "empty string", key: "", want: false}, {name: "unrelated key", key: "model_name", want: false}, @@ -97,9 +123,6 @@ func TestIsContextLimitKey(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - if tt.skip { - t.Skip("known bug: isContextLimitKey false positive") - } got := isContextLimitKey(tt.key) require.Equal(t, tt.want, got) }) @@ -385,6 +408,19 @@ func TestExtractContextLimit(t *testing.T) { assert.False(t, result.Valid) }) + t.Run("ContextUsageCountersIgnored", func(t *testing.T) { + t.Parallel() + metadata := fantasy.ProviderMetadata{ + "openai": &testProviderData{ + data: map[string]any{ + "context_tokens_used": float64(64000), + }, + }, + } + result := extractContextLimit(metadata) + assert.False(t, result.Valid) + }) + t.Run("NilMetadata", func(t *testing.T) { t.Parallel() result := extractContextLimit(nil)