Skip to content

Commit d2a8b66

Browse files
kasarolzzwCoda-bot
andcommitted
refactor: [Coda] stop merging snippet variable definitions
(LogID: 202510241537520100911040168388BC8) Co-Authored-By: Coda <[email protected]>
1 parent 2ce546a commit d2a8b66

File tree

2 files changed

+22
-94
lines changed

2 files changed

+22
-94
lines changed

backend/modules/prompt/domain/service/manage.go

Lines changed: 11 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -483,9 +483,8 @@ func (p *PromptServiceImpl) doExpandSnippets(ctx context.Context, promptDO *enti
483483
}
484484
}
485485

486-
// Build maps for quick lookup of snippet content and variable definitions
486+
// Build map for quick lookup of snippet content
487487
snippetContentMap := make(map[string]string)
488-
snippetVariableMap := make(map[string][]*entity.VariableDef)
489488
for _, snippet := range promptDetail.PromptTemplate.Snippets {
490489
if snippet == nil || snippet.PromptBasic == nil {
491490
continue
@@ -503,111 +502,68 @@ func (p *PromptServiceImpl) doExpandSnippets(ctx context.Context, promptDO *enti
503502
snippetContent := ptr.From(snippetDetail.PromptTemplate.Messages[0].Content)
504503
snippetContentMap[key] = snippetContent
505504
}
506-
507-
// Collect variable definitions from snippet
508-
if len(snippetDetail.PromptTemplate.VariableDefs) > 0 {
509-
snippetVariableMap[key] = snippetDetail.PromptTemplate.VariableDefs
510-
}
511-
}
512-
513-
// Now expand all snippet references in messages using the pre-built content map and merge variable definitions
514-
var allMergedVariableDefs []*entity.VariableDef
515-
existingVarKeys := make(map[string]bool) // Track existing variable keys to avoid duplicates
516-
517-
// First, collect existing variable definitions from the main prompt
518-
for _, varDef := range promptDetail.PromptTemplate.VariableDefs {
519-
if varDef != nil {
520-
allMergedVariableDefs = append(allMergedVariableDefs, varDef)
521-
existingVarKeys[varDef.Key] = true
522-
}
523505
}
524506

507+
// Expand all snippet references in messages using the pre-built content map
525508
for _, message := range promptDetail.PromptTemplate.Messages {
526509
if message == nil {
527510
continue
528511
}
529512

530513
// Expand content if it exists
531514
if message.Content != nil && *message.Content != "" {
532-
expandedContent, mergedVars, err := p.expandWithSnippetMap(ctx, *message.Content, snippetContentMap, snippetVariableMap)
515+
expandedContent, err := p.expandWithSnippetMap(ctx, *message.Content, snippetContentMap)
533516
if err != nil {
534517
return err
535518
}
536519
message.Content = &expandedContent
537-
// Add merged variable definitions (avoid duplicates)
538-
for _, varDef := range mergedVars {
539-
if varDef != nil && !existingVarKeys[varDef.Key] {
540-
allMergedVariableDefs = append(allMergedVariableDefs, varDef)
541-
existingVarKeys[varDef.Key] = true
542-
}
543-
}
544520
}
545521

546522
// Expand text in parts
547523
for _, part := range message.Parts {
548524
if part == nil || part.Text == nil || *part.Text == "" {
549525
continue
550526
}
551-
expandedText, mergedVars, err := p.expandWithSnippetMap(ctx, *part.Text, snippetContentMap, snippetVariableMap)
527+
expandedText, err := p.expandWithSnippetMap(ctx, *part.Text, snippetContentMap)
552528
if err != nil {
553529
return err
554530
}
555531
part.Text = &expandedText
556-
// Add merged variable definitions (avoid duplicates)
557-
for _, varDef := range mergedVars {
558-
if varDef != nil && !existingVarKeys[varDef.Key] {
559-
allMergedVariableDefs = append(allMergedVariableDefs, varDef)
560-
existingVarKeys[varDef.Key] = true
561-
}
562-
}
563532
}
564533
}
565534

566-
// Update the prompt template with merged variable definitions
567-
promptDetail.PromptTemplate.VariableDefs = allMergedVariableDefs
568-
569535
return nil
570536
}
571537

572-
// expandWithSnippetMap expands snippet references using a pre-built content map and returns expanded content with merged variable definitions
573-
func (p *PromptServiceImpl) expandWithSnippetMap(ctx context.Context, content string, snippetContentMap map[string]string, snippetVariableMap map[string][]*entity.VariableDef) (string, []*entity.VariableDef, error) {
538+
// expandWithSnippetMap expands snippet references using a pre-built content map and returns expanded content
539+
func (p *PromptServiceImpl) expandWithSnippetMap(ctx context.Context, content string, snippetContentMap map[string]string) (string, error) {
574540
// Parse snippet references from content
575541
snippetRefs, err := p.snippetParser.ParseReferences(content)
576542
if err != nil {
577-
return "", nil, err
543+
return "", err
578544
}
579545

580546
// If no references found, return original content and empty variable definitions
581547
if len(snippetRefs) == 0 {
582-
return content, nil, nil
548+
return content, nil
583549
}
584550

585-
// Replace each reference with expanded content from the map and collect variable definitions
551+
// Replace each reference with expanded content from the map
586552
expandedContent := content
587-
var mergedVariableDefs []*entity.VariableDef
588-
processedSnippets := make(map[string]bool) // Track processed snippets to avoid duplicates
589553

590554
for _, ref := range snippetRefs {
591555
// Build lookup key: "promptID_version"
592556
key := fmt.Sprintf("%d_%s", ref.PromptID, ref.CommitVersion)
593557
expandedSnippetContent, exists := snippetContentMap[key]
594558
if !exists {
595-
return "", nil, errorx.NewByCode(prompterr.ResourceNotFoundCode,
559+
return "", errorx.NewByCode(prompterr.ResourceNotFoundCode,
596560
errorx.WithExtraMsg(fmt.Sprintf("snippet content for prompt %d with version %s not found in cache", ref.PromptID, ref.CommitVersion)))
597561
}
598562

599-
// Collect variable definitions from this snippet if not already processed
600-
if !processedSnippets[key] {
601-
if variableDefs, varExists := snippetVariableMap[key]; varExists && len(variableDefs) > 0 {
602-
mergedVariableDefs = append(mergedVariableDefs, variableDefs...)
603-
}
604-
processedSnippets[key] = true
605-
}
606-
607563
// Replace the reference with expanded content
608564
refString := p.snippetParser.SerializeReference(ref)
609565
expandedContent = strings.ReplaceAll(expandedContent, refString, expandedSnippetContent)
610566
}
611567

612-
return expandedContent, mergedVariableDefs, nil
568+
return expandedContent, nil
613569
}

backend/modules/prompt/domain/service/manage_test.go

Lines changed: 11 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -925,15 +925,10 @@ func TestPromptServiceImpl_ExpandSnippets(t *testing.T) {
925925
}
926926
as.NotEmpty(detail.PromptTemplate.Snippets)
927927
as.Equal("hello snippet body", ptr.From(detail.PromptTemplate.Messages[0].Content))
928-
as.Len(detail.PromptTemplate.VariableDefs, 2)
929-
keys := map[string]bool{}
930-
for _, def := range detail.PromptTemplate.VariableDefs {
931-
if def != nil {
932-
keys[def.Key] = true
933-
}
928+
as.Len(detail.PromptTemplate.VariableDefs, 1)
929+
if len(detail.PromptTemplate.VariableDefs) > 0 {
930+
as.Equal("main_var", detail.PromptTemplate.VariableDefs[0].Key)
934931
}
935-
as.True(keys["main_var"])
936-
as.True(keys["snippet_var"])
937932
},
938933
},
939934
}
@@ -966,16 +961,14 @@ func TestPromptServiceImpl_expandWithSnippetMap(t *testing.T) {
966961
snippetParser SnippetParser
967962
}
968963
type args struct {
969-
content string
970-
snippetContentMap map[string]string
971-
snippetVariableMap map[string][]*entity.VariableDef
964+
content string
965+
snippetContentMap map[string]string
972966
}
973967
tests := []struct {
974968
name string
975969
fields fields
976970
args args
977971
wantContent string
978-
wantVars []*entity.VariableDef
979972
wantErr error
980973
}{
981974
{
@@ -988,9 +981,8 @@ func TestPromptServiceImpl_expandWithSnippetMap(t *testing.T) {
988981
},
989982
},
990983
args: args{
991-
content: "test",
992-
snippetContentMap: map[string]string{},
993-
snippetVariableMap: map[string][]*entity.VariableDef{},
984+
content: "test",
985+
snippetContentMap: map[string]string{},
994986
},
995987
wantErr: errors.New("parse fail"),
996988
},
@@ -1004,14 +996,13 @@ func TestPromptServiceImpl_expandWithSnippetMap(t *testing.T) {
1004996
},
1005997
},
1006998
args: args{
1007-
content: "<cozeloop_snippet>id=2&version=v1</cozeloop_snippet>",
1008-
snippetContentMap: map[string]string{},
1009-
snippetVariableMap: map[string][]*entity.VariableDef{},
999+
content: "<cozeloop_snippet>id=2&version=v1</cozeloop_snippet>",
1000+
snippetContentMap: map[string]string{},
10101001
},
10111002
wantErr: errorx.NewByCode(prompterr.ResourceNotFoundCode, errorx.WithExtraMsg("snippet content for prompt 2 with version v1 not found in cache")),
10121003
},
10131004
{
1014-
name: "success merges duplicated variables",
1005+
name: "success expands duplicated snippets",
10151006
fields: fields{
10161007
snippetParser: NewCozeLoopSnippetParser(),
10171008
},
@@ -1020,14 +1011,8 @@ func TestPromptServiceImpl_expandWithSnippetMap(t *testing.T) {
10201011
snippetContentMap: map[string]string{
10211012
"2_v1": "snippet",
10221013
},
1023-
snippetVariableMap: map[string][]*entity.VariableDef{
1024-
"2_v1": {
1025-
{Key: "snippet_var"},
1026-
},
1027-
},
10281014
},
10291015
wantContent: "hello snippet and again snippet",
1030-
wantVars: []*entity.VariableDef{{Key: "snippet_var"}},
10311016
},
10321017
}
10331018

@@ -1042,25 +1027,12 @@ func TestPromptServiceImpl_expandWithSnippetMap(t *testing.T) {
10421027
svc.snippetParser = NewCozeLoopSnippetParser()
10431028
}
10441029

1045-
gotContent, gotVars, err := svc.expandWithSnippetMap(context.Background(), ttt.args.content, ttt.args.snippetContentMap, ttt.args.snippetVariableMap)
1030+
gotContent, err := svc.expandWithSnippetMap(context.Background(), ttt.args.content, ttt.args.snippetContentMap)
10461031
unittest.AssertErrorEqual(t, ttt.wantErr, err)
10471032
if ttt.wantErr != nil {
10481033
return
10491034
}
10501035
assert.Equal(t, ttt.wantContent, gotContent)
1051-
if ttt.wantVars != nil {
1052-
assert.Len(t, gotVars, len(ttt.wantVars))
1053-
for _, expected := range ttt.wantVars {
1054-
found := false
1055-
for _, actual := range gotVars {
1056-
if actual != nil && actual.Key == expected.Key {
1057-
found = true
1058-
break
1059-
}
1060-
}
1061-
assert.True(t, found, "expected variable %s not found", expected.Key)
1062-
}
1063-
}
10641036
})
10651037
}
10661038
}

0 commit comments

Comments
 (0)