Skip to content

Commit b39b78b

Browse files
AdeshDeshmukhamisevsk
authored andcommitted
fix: merge Prompts from referenced modelkits during Kitfile resolution
The mergeKitfiles function merges Code, DataSets, and Docs when resolving modelkit references, but silently drops Prompts. This means any prompts attached to referenced ModelKits are lost. Add the missing Prompts merge to match the existing pattern used for other slice fields (Code, DataSets, Docs). Signed-off-by: Adesh Deshmukh <adeshkd123@gmail.com>
1 parent d029d9f commit b39b78b

2 files changed

Lines changed: 125 additions & 0 deletions

File tree

pkg/lib/kitfile/resolve.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ func mergeKitfiles(into, from *artifact.KitFile) *artifact.KitFile {
133133
result.DataSets = append(result.DataSets, from.DataSets...)
134134
result.Docs = into.Docs
135135
result.Docs = append(result.Docs, from.Docs...)
136+
result.Prompts = into.Prompts
137+
result.Prompts = append(result.Prompts, from.Prompts...)
136138
return result
137139
}
138140

pkg/lib/kitfile/resolve_test.go

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// Copyright 2024 The KitOps Authors.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
//
15+
// SPDX-License-Identifier: Apache-2.0
16+
17+
package kitfile
18+
19+
import (
20+
"testing"
21+
22+
"github.com/kitops-ml/kitops/pkg/artifact"
23+
"github.com/stretchr/testify/assert"
24+
)
25+
26+
func TestMergeKitfiles_Prompts(t *testing.T) {
27+
prompt1 := artifact.Prompt{Name: "prompt1", Path: "prompts/p1.txt", Description: "first prompt"}
28+
prompt2 := artifact.Prompt{Name: "prompt2", Path: "prompts/p2.txt", Description: "second prompt"}
29+
prompt3 := artifact.Prompt{Name: "prompt3", Path: "prompts/p3.txt", Description: "third prompt"}
30+
31+
tests := []struct {
32+
name string
33+
into *artifact.KitFile
34+
from *artifact.KitFile
35+
expected []artifact.Prompt
36+
}{
37+
{
38+
name: "both nil prompts",
39+
into: &artifact.KitFile{},
40+
from: &artifact.KitFile{},
41+
expected: nil,
42+
},
43+
{
44+
name: "into has prompts, from has none",
45+
into: &artifact.KitFile{Prompts: []artifact.Prompt{prompt1, prompt2}},
46+
from: &artifact.KitFile{},
47+
expected: []artifact.Prompt{prompt1, prompt2},
48+
},
49+
{
50+
name: "from has prompts, into has none",
51+
into: &artifact.KitFile{},
52+
from: &artifact.KitFile{Prompts: []artifact.Prompt{prompt2, prompt3}},
53+
expected: []artifact.Prompt{prompt2, prompt3},
54+
},
55+
{
56+
name: "both have prompts — merged in order",
57+
into: &artifact.KitFile{Prompts: []artifact.Prompt{prompt1}},
58+
from: &artifact.KitFile{Prompts: []artifact.Prompt{prompt2, prompt3}},
59+
expected: []artifact.Prompt{prompt1, prompt2, prompt3},
60+
},
61+
{
62+
name: "overlapping prompts — all preserved",
63+
into: &artifact.KitFile{Prompts: []artifact.Prompt{prompt1, prompt2}},
64+
from: &artifact.KitFile{Prompts: []artifact.Prompt{prompt2, prompt3}},
65+
expected: []artifact.Prompt{prompt1, prompt2, prompt2, prompt3},
66+
},
67+
}
68+
69+
for _, tt := range tests {
70+
t.Run(tt.name, func(t *testing.T) {
71+
result := mergeKitfiles(tt.into, tt.from)
72+
assert.Equal(t, tt.expected, result.Prompts, "Prompts mismatch")
73+
})
74+
}
75+
}
76+
77+
func TestMergeKitfiles_PreservesOtherFields(t *testing.T) {
78+
into := &artifact.KitFile{
79+
ManifestVersion: "1.0.0",
80+
Package: artifact.Package{
81+
Name: "into-pkg",
82+
},
83+
Code: []artifact.Code{{Path: "into/code.py"}},
84+
DataSets: []artifact.DataSet{{Path: "into/data.csv"}},
85+
Docs: []artifact.Docs{{Path: "into/doc.md"}},
86+
Prompts: []artifact.Prompt{{Name: "into-prompt", Path: "into/prompt.txt"}},
87+
}
88+
from := &artifact.KitFile{
89+
ManifestVersion: "1.0.0",
90+
Package: artifact.Package{
91+
Name: "from-pkg",
92+
},
93+
Code: []artifact.Code{{Path: "from/code.py"}},
94+
DataSets: []artifact.DataSet{{Path: "from/data.csv"}},
95+
Docs: []artifact.Docs{{Path: "from/doc.md"}},
96+
Prompts: []artifact.Prompt{{Name: "from-prompt", Path: "from/prompt.txt"}},
97+
}
98+
99+
result := mergeKitfiles(into, from)
100+
101+
assert.Len(t, result.Prompts, 2, "should have prompts from both")
102+
assert.Equal(t, "into-prompt", result.Prompts[0].Name, "into prompts should come first")
103+
assert.Equal(t, "from-prompt", result.Prompts[1].Name, "from prompts should come second")
104+
105+
assert.Len(t, result.Code, 2, "code should still merge")
106+
assert.Len(t, result.DataSets, 2, "datasets should still merge")
107+
assert.Len(t, result.Docs, 2, "docs should still merge")
108+
assert.Equal(t, "into-pkg", result.Package.Name, "package name should use firstNonEmpty")
109+
}
110+
111+
func TestMergeKitfiles_DoesNotMutateInput(t *testing.T) {
112+
intoPrompts := []artifact.Prompt{{Name: "p1"}, {Name: "p2"}}
113+
fromPrompts := []artifact.Prompt{{Name: "p3"}}
114+
115+
into := &artifact.KitFile{Prompts: intoPrompts}
116+
from := &artifact.KitFile{Prompts: fromPrompts}
117+
118+
result := mergeKitfiles(into, from)
119+
120+
assert.Len(t, result.Prompts, 3, "result should have 3 prompts")
121+
assert.Len(t, into.Prompts, 2, "input 'into' should be unmodified")
122+
assert.Len(t, from.Prompts, 1, "input 'from' should be unmodified")
123+
}

0 commit comments

Comments
 (0)