fix(truncate): pick a stable record path when arrays tie (#1770)

## Bug

When a tool response is over the truncation limit, `findArraysRecursive`
(`internal/truncate/truncator.go`) collects candidate record arrays by
ranging over each JSON object, and `chooseBestArray` picks one. On a
tie, `chooseBestArray` keeps whichever candidate came first. Go
randomizes map iteration, so identical payloads with equally good
sibling arrays get a different `RecordPath` on different calls. The
`read_cache` handle minted for that response then pages a different
array each time.

## Trigger

An upstream tool returns, for example, `{"zeta":[…20 rows], "alpha":[…20
rows], "mid":[…20 rows]}` and the response is over the limit.
`content_forward.go` calls `Truncate` on every such response. Repeated
identical calls return `record_path` values of `zeta`, `alpha`, or `mid`
depending on map order.

## Fix

Object keys are now traversed in sorted order, so the tie-break is
stable: the first key in sorted order wins. Non-tied selection is
unchanged.

## Verification

- `go test -count=1 -run TestTruncate_TiedArraysPickStableRecordPath
./internal/truncate/` passes. The test runs 50 calls on the payload
above and expects `alpha` every time.
- With `truncator.go` reverted (`git stash push
internal/truncate/truncator.go`), the same test fails: `call 0: record
path "zeta", want "alpha"`.
- `go test -count=1 ./internal/truncate/` passes.
- `go vet ./internal/truncate/` is clean.
- `gofmt -l internal/truncate/` is clean.
- I also ran `go test -count=1 ./internal/server/` locally, but it hit
the default 10-minute test timeout, which isn't a result either way. A
narrower rerun failed at setup because of a local build-cache error. I'm
relying on CI for that package.

Commands were run with `GOFLAGS=-mod=readonly CGO_ENABLED=0`.

Author: rudycelekli

internal/truncate/record_path_order_test.go

@@ -0,0 +1,31 @@
+package truncate
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+)
+
+// Two equally good record arrays must resolve to the same record path on
+// every call, or identical upstream responses get cache handles paging
+// different arrays.
+func TestTruncate_TiedArraysPickStableRecordPath(t *testing.T) {
+	row := func(i int) string { return fmt.Sprintf(`{"id":%d,"name":"%s"}`, i, strings.Repeat("x", 40)) }
+	rows := make([]string, 20)
+	for i := range rows {
+		rows[i] = row(i)
+	}
+	arr := "[" + strings.Join(rows, ",") + "]"
+	content := fmt.Sprintf(`{"zeta":%s,"alpha":%s,"mid":%s}`, arr, arr, arr)
+
+	tr := NewTruncator(500)
+	for i := 0; i < 50; i++ {
+		res := tr.Truncate(content, "tool", nil)
+		if !res.CacheAvailable {
+			t.Fatalf("expected a cache handle, got simple truncation")
+		}
+		if res.RecordPath != "alpha" {
+			t.Fatalf("call %d: record path %q, want %q", i, res.RecordPath, "alpha")
+		}
+	}
+}

internal/truncate/truncator.go

@@ -3,6 +3,7 @@ package truncate
 import (
 	"encoding/json"
 	"fmt"
+	"sort"
 	"strings"
 	"unicode/utf8"
 
@@ -137,8 +138,16 @@ func (t *Truncator) findArraysRecursive(data interface{}, currentPath string, de
 		}
 
 	case map[string]interface{}:
-		// Traverse object fields
-		for key, val := range v {
+		// Traverse object fields in sorted order: chooseBestArray keeps the
+		// first of equally good arrays, so map order would otherwise pick a
+		// different record path for the same payload on each call.
+		keys := make([]string, 0, len(v))
+		for key := range v {
+			keys = append(keys, key)
+		}
+		sort.Strings(keys)
+		for _, key := range keys {
+			val := v[key]
 			newPath := key
 			if currentPath != "" {
 				newPath = currentPath + "." + key