diff --git a/docs/DIAGNOSE.md b/docs/DIAGNOSE.md index 7b437d5..79dc5a8 100644 --- a/docs/DIAGNOSE.md +++ b/docs/DIAGNOSE.md @@ -213,3 +213,5 @@ Fix blocking findings and review indeterminate findings before rollout. Check unsupported mappings against actual instrumentation and the intended released collector before a separate shadow trial. The [fixture demo](../examples/diagnose/README.md) exercises the local path. + +`topk_keys` entries require a `field` (`prompt_key`, `user_key`, or `session_key`). Omitting `weight` is allowed; the connector applies its default. A missing `field` yields `unsupported_mapping` on the entry path and line. diff --git a/internal/diagnose/connector.go b/internal/diagnose/connector.go index 83cfb1d..93925a1 100644 --- a/internal/diagnose/connector.go +++ b/internal/diagnose/connector.go @@ -298,6 +298,12 @@ func (c *checker) topKeys(n *yaml.Node) { } for _, item := range items { m := c.object(item, "field", "weight") + // field is required; weight may be omitted (connector default). Point the + // finding at the entry when the field key is absent so path/line are useful. + if m["field"] == nil { + c.add("unsupported_mapping", "unsupported", item) + continue + } field := c.string(m["field"]) c.enum(m["field"], "prompt_key", "user_key", "session_key") c.enum(m["weight"], "tokens", "requests") diff --git a/internal/diagnose/mappings_test.go b/internal/diagnose/mappings_test.go index fddec35..3e14d65 100644 --- a/internal/diagnose/mappings_test.go +++ b/internal/diagnose/mappings_test.go @@ -4,6 +4,7 @@ package diagnose import ( + "fmt" "strings" "testing" ) @@ -81,3 +82,75 @@ func TestBackendPreservedAndSecretsNotCopied(t *testing.T) { t.Fatal("copied backend configuration") } } + + +func TestTopKKeysRequireField(t *testing.T) { + safe := fixture(t, "safe") + for _, tc := range []struct { + name, replacement string + code int + finding string + wantPathPrefix string + wantAttrEmpty bool + }{ + { + name: "missing-field", replacement: " topk_keys:\n - weight: tokens\n slices:", + code: 4, finding: "unsupported_mapping", wantPathPrefix: "connectors.genaisketch.topk_keys[0]", wantAttrEmpty: true, + }, + { + name: "empty-field", replacement: " topk_keys:\n - field: \"\"\n weight: tokens\n slices:", + code: 4, finding: "unsupported_mapping", wantPathPrefix: "connectors.genaisketch.topk_keys[0].field", wantAttrEmpty: true, + }, + { + name: "unknown-field", replacement: " topk_keys:\n - field: not_a_key\n weight: tokens\n slices:", + code: 4, finding: "unsupported_mapping", wantPathPrefix: "connectors.genaisketch.topk_keys[0].field", wantAttrEmpty: true, + }, + { + name: "valid-user-with-weight", replacement: " topk_keys:\n - field: user_key\n weight: tokens\n slices:", + code: 0, finding: "", + }, + { + name: "valid-prompt-without-weight", replacement: " topk_keys:\n - field: prompt_key\n slices:", + code: 0, finding: "", + }, + { + name: "valid-session-with-requests", replacement: " topk_keys:\n - field: session_key\n weight: requests\n slices:", + code: 0, finding: "", + }, + } { + t.Run(tc.name, func(t *testing.T) { + r := check(t, strings.Replace(safe, " slices:", tc.replacement, 1)) + if r.ExitCode() != tc.code { + t.Fatalf("exit %d, want %d; report=%+v", r.ExitCode(), tc.code, r) + } + if tc.finding == "" { + if r.Status != "supported_safe" || len(r.Findings) != 0 { + t.Fatalf("want supported_safe with no findings, got %+v", r) + } + return + } + if !hasFinding(r, tc.finding) { + t.Fatalf("missing %s in %+v", tc.finding, r) + } + if r.Status == "supported_safe" { + t.Fatal("missing field must not be supported_safe") + } + var hit Finding + for _, f := range r.Findings { + if f.ID == tc.finding { + hit = f + break + } + } + if tc.wantPathPrefix != "" && (hit.Path == "" || hit.Line == 0 || !strings.HasPrefix(hit.Path, tc.wantPathPrefix)) { + t.Fatalf("path/line = %q:%d, want prefix %q with line", hit.Path, hit.Line, tc.wantPathPrefix) + } + if tc.wantAttrEmpty && hit.Attribute != "" { + t.Fatalf("attribute leaked into report: %q", hit.Attribute) + } + if strings.Contains(fmt.Sprintf("%+v", r), "not_a_key") { + t.Fatal("arbitrary field value leaked into report") + } + }) + } +}