mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 20:17:20 +00:00
fix(channel-memory): retry truncated extraction JSON (#1364)
Co-authored-by: Collective Developer <man@collective.dev>
This commit is contained in:
ナムandCollective Developer authored and GitHub committed
1 parent
374defe04e
commit
af80890edf
2 files changed
+181
-13
No files matched your search
@@ -12,6 +12,13 @@ import (
|
||||
usagecaps "github.com/nextlevelbuilder/goclaw/internal/usage/caps"
|
||||
)
|
||||
|
||||
const (
|
||||
extractionMaxInputChars = 12000
|
||||
extractionRetryMaxInputChars = 6000
|
||||
extractionMaxOutputTokens = 4096
|
||||
extractionRetryMaxOutputTokens = 4096
|
||||
)
|
||||
|
||||
type ExtractedItem struct {
|
||||
Type string `json:"type"`
|
||||
Summary string `json:"summary"`
|
||||
@@ -24,6 +31,44 @@ func Extract(ctx context.Context, provider providers.Provider, model string, cap
|
||||
if provider == nil {
|
||||
return nil, fmt.Errorf("background provider unavailable")
|
||||
}
|
||||
resp, err := callExtractionProvider(ctx, provider, model, caps, messages, allowed, extractionMaxInputChars, extractionMaxOutputTokens, "channel-memory-extraction")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if resp == nil {
|
||||
return nil, fmt.Errorf("background provider returned nil response")
|
||||
}
|
||||
if resp.FinishReason != "length" {
|
||||
items, err := parseExtractionResponse(resp.Content)
|
||||
if !shouldRetryExtractionParse(resp, err) {
|
||||
return items, err
|
||||
}
|
||||
}
|
||||
|
||||
resp, err = callExtractionProvider(ctx, provider, model, caps, messages, allowed, extractionRetryMaxInputChars, extractionRetryMaxOutputTokens, "channel-memory-extraction-retry")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if resp == nil {
|
||||
return nil, fmt.Errorf("background provider returned nil response")
|
||||
}
|
||||
if resp.FinishReason == "length" {
|
||||
return nil, fmt.Errorf("extraction response truncated after retry")
|
||||
}
|
||||
return parseExtractionResponse(resp.Content)
|
||||
}
|
||||
|
||||
func callExtractionProvider(
|
||||
ctx context.Context,
|
||||
provider providers.Provider,
|
||||
model string,
|
||||
caps *usagecaps.Service,
|
||||
messages []store.PendingMessage,
|
||||
allowed []string,
|
||||
maxInputChars int,
|
||||
maxOutputTokens int,
|
||||
purpose string,
|
||||
) (*providers.ChatResponse, error) {
|
||||
var sb strings.Builder
|
||||
for _, msg := range messages {
|
||||
sb.WriteString(msg.CreatedAt.Format(time.RFC3339))
|
||||
@@ -40,7 +85,7 @@ func Extract(ctx context.Context, provider providers.Provider, model string, cap
|
||||
}
|
||||
sb.WriteString(body)
|
||||
sb.WriteByte('\n')
|
||||
if sb.Len() > 12000 {
|
||||
if sb.Len() > maxInputChars {
|
||||
sb.WriteString("...(truncated)\n")
|
||||
break
|
||||
}
|
||||
@@ -51,34 +96,36 @@ func Extract(ctx context.Context, provider providers.Provider, model string, cap
|
||||
{Role: "system", Content: extractionPrompt(allowed)},
|
||||
{Role: "user", Content: sb.String()},
|
||||
},
|
||||
Options: map[string]any{"max_tokens": 1200, "temperature": 0.1},
|
||||
Options: map[string]any{"max_tokens": maxOutputTokens, "temperature": 0.1},
|
||||
}
|
||||
var resp *providers.ChatResponse
|
||||
var err error
|
||||
if caps != nil {
|
||||
resp, err = caps.Chat(ctx, provider, req, usagecaps.ChatOptions{
|
||||
return caps.Chat(ctx, provider, req, usagecaps.ChatOptions{
|
||||
ModelID: model,
|
||||
Purpose: "channel-memory-extraction",
|
||||
MaxOutputTokens: 1200,
|
||||
Purpose: purpose,
|
||||
MaxOutputTokens: maxOutputTokens,
|
||||
})
|
||||
} else {
|
||||
resp, err = provider.Chat(ctx, req)
|
||||
}
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return parseExtractionResponse(resp.Content)
|
||||
return provider.Chat(ctx, req)
|
||||
}
|
||||
|
||||
func extractionPrompt(allowed []string) string {
|
||||
return `Extract only durable, reusable work context from channel messages.
|
||||
Allowed item types: ` + strings.Join(allowed, ", ") + `.
|
||||
Never include secrets, credentials, tokens, payment data, private addresses, phone numbers, health/legal/financial sensitive details, casual chatter, jokes, or low-confidence guesses.
|
||||
Return at most 20 items, highest-confidence first.
|
||||
Return strict JSON array only. Each item:
|
||||
{"type":"people|projects|decisions|todos|preferences|events","summary":"one concise redacted fact","topics":["..."],"entities":["..."],"confidence":0.0-1.0}
|
||||
If nothing durable remains, return [].`
|
||||
}
|
||||
|
||||
func shouldRetryExtractionParse(resp *providers.ChatResponse, err error) bool {
|
||||
if err == nil || resp == nil {
|
||||
return false
|
||||
}
|
||||
raw := strings.TrimSpace(resp.Content)
|
||||
return raw == "" || strings.Contains(err.Error(), "unexpected end of JSON input")
|
||||
}
|
||||
|
||||
func parseExtractionResponse(content string) ([]ExtractedItem, error) {
|
||||
raw := strings.TrimSpace(content)
|
||||
raw = strings.TrimPrefix(raw, "```json")
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
package channelmemory
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/providers"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
)
|
||||
|
||||
type fakeExtractionProvider struct {
|
||||
responses []providers.ChatResponse
|
||||
requests []providers.ChatRequest
|
||||
}
|
||||
|
||||
func (f *fakeExtractionProvider) Chat(_ context.Context, req providers.ChatRequest) (*providers.ChatResponse, error) {
|
||||
f.requests = append(f.requests, req)
|
||||
if len(f.responses) == 0 {
|
||||
return &providers.ChatResponse{Content: "[]", FinishReason: "stop"}, nil
|
||||
}
|
||||
resp := f.responses[0]
|
||||
f.responses = f.responses[1:]
|
||||
return &resp, nil
|
||||
}
|
||||
|
||||
func (f *fakeExtractionProvider) ChatStream(_ context.Context, req providers.ChatRequest, _ func(providers.StreamChunk)) (*providers.ChatResponse, error) {
|
||||
return f.Chat(context.Background(), req)
|
||||
}
|
||||
|
||||
func (f *fakeExtractionProvider) DefaultModel() string { return "fake-model" }
|
||||
func (f *fakeExtractionProvider) Name() string { return "fake" }
|
||||
|
||||
func TestExtractRetriesAfterLengthFinish(t *testing.T) {
|
||||
provider := &fakeExtractionProvider{responses: []providers.ChatResponse{
|
||||
{Content: `[{"type":"todos","summary":"Finish`, FinishReason: "length"},
|
||||
{Content: `[{"type":"todos","summary":"Finish release notes","topics":["release"],"entities":["GoClaw"],"confidence":0.9}]`, FinishReason: "stop"},
|
||||
}}
|
||||
|
||||
items, err := Extract(context.Background(), provider, "fake-model", nil, extractionTestMessages(12), DefaultAllowedTypes)
|
||||
if err != nil {
|
||||
t.Fatalf("Extract() error = %v", err)
|
||||
}
|
||||
if len(items) != 1 {
|
||||
t.Fatalf("len(items) = %d, want 1", len(items))
|
||||
}
|
||||
if items[0].Summary != "Finish release notes" {
|
||||
t.Fatalf("summary = %q", items[0].Summary)
|
||||
}
|
||||
if len(provider.requests) != 2 {
|
||||
t.Fatalf("provider calls = %d, want 2", len(provider.requests))
|
||||
}
|
||||
if got := provider.requests[0].Options[providers.OptMaxTokens]; got != extractionMaxOutputTokens {
|
||||
t.Fatalf("first max_tokens = %v, want %d", got, extractionMaxOutputTokens)
|
||||
}
|
||||
if got := provider.requests[1].Options[providers.OptMaxTokens]; got != extractionRetryMaxOutputTokens {
|
||||
t.Fatalf("retry max_tokens = %v, want %d", got, extractionRetryMaxOutputTokens)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractRetriesAfterEmptyResponse(t *testing.T) {
|
||||
provider := &fakeExtractionProvider{responses: []providers.ChatResponse{
|
||||
{Content: ``, FinishReason: "stop"},
|
||||
{Content: `[]`, FinishReason: "stop"},
|
||||
}}
|
||||
|
||||
items, err := Extract(context.Background(), provider, "fake-model", nil, extractionTestMessages(5), DefaultAllowedTypes)
|
||||
if err != nil {
|
||||
t.Fatalf("Extract() error = %v", err)
|
||||
}
|
||||
if len(items) != 0 {
|
||||
t.Fatalf("len(items) = %d, want 0", len(items))
|
||||
}
|
||||
if len(provider.requests) != 2 {
|
||||
t.Fatalf("provider calls = %d, want 2", len(provider.requests))
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractRetriesAfterUnexpectedJSONEnd(t *testing.T) {
|
||||
provider := &fakeExtractionProvider{responses: []providers.ChatResponse{
|
||||
{Content: `[{"type":"projects","summary":"`, FinishReason: "stop"},
|
||||
{Content: `[{"type":"projects","summary":"Gateway dashboard rollout","topics":["dashboard"],"entities":["Gateway"],"confidence":0.86}]`, FinishReason: "stop"},
|
||||
}}
|
||||
|
||||
items, err := Extract(context.Background(), provider, "fake-model", nil, extractionTestMessages(5), DefaultAllowedTypes)
|
||||
if err != nil {
|
||||
t.Fatalf("Extract() error = %v", err)
|
||||
}
|
||||
if len(items) != 1 {
|
||||
t.Fatalf("len(items) = %d, want 1", len(items))
|
||||
}
|
||||
if items[0].Type != "projects" {
|
||||
t.Fatalf("type = %q, want projects", items[0].Type)
|
||||
}
|
||||
if len(provider.requests) != 2 {
|
||||
t.Fatalf("provider calls = %d, want 2", len(provider.requests))
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseExtractionResponseStripsCodeFence(t *testing.T) {
|
||||
items, err := parseExtractionResponse("```json\n[]\n```")
|
||||
if err != nil {
|
||||
t.Fatalf("parseExtractionResponse() error = %v", err)
|
||||
}
|
||||
if len(items) != 0 {
|
||||
t.Fatalf("len(items) = %d, want 0", len(items))
|
||||
}
|
||||
}
|
||||
|
||||
func extractionTestMessages(n int) []store.PendingMessage {
|
||||
messages := make([]store.PendingMessage, 0, n)
|
||||
base := time.Date(2026, 7, 5, 9, 0, 0, 0, time.UTC)
|
||||
for i := range n {
|
||||
messages = append(messages, store.PendingMessage{
|
||||
Sender: "tester",
|
||||
Body: "Durable project context that may be extracted into channel memory.",
|
||||
CreatedAt: base.Add(time.Duration(i) * time.Minute),
|
||||
})
|
||||
}
|
||||
return messages
|
||||
}
|
||||
Reference in new issue
Block a user