fix(api): persist tag_kwd on Go AddChunk (#20138) (#20146)

This commit is contained in:
Harsh Kashyap
2026-10-09 13:18:21 +08:00
committed by GitHub
parent e8f7f97237
commit d513bc2afa
5 changed files with 220 additions and 0 deletions
+9
View File
@@ -725,6 +725,9 @@ func addChunkStringListField(rawBody map[string]json.RawMessage, field, listMess
if !ok {
return nil, nil
}
if string(raw) == "null" {
return nil, errors.New(listMessage)
}
var values []interface{}
if err := json.Unmarshal(raw, &values); err != nil {
return nil, errors.New(listMessage)
@@ -779,6 +782,11 @@ func (h *ChunkHandler) AddChunk(c *gin.Context) {
common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error())
return
}
tagKwd, err := addChunkStringListField(rawBody, "tag_kwd", "`tag_kwd` is required to be a list", "`tag_kwd` must be a list of strings")
if err != nil {
common.ResponseWithCodeData(c, common.CodeDataError, nil, err.Error())
return
}
imageBase64, err := addChunkStringPtrField(rawBody, "image_base64")
if err != nil {
common.ResponseWithCodeData(c, common.CodeArgumentError, nil, err.Error())
@@ -798,6 +806,7 @@ func (h *ChunkHandler) AddChunk(c *gin.Context) {
Content: content,
ImportantKeywords: importantKeywords,
Questions: questions,
TagKwd: tagKwd,
TagFeas: tagFeas,
ImageBase64: imageBase64,
}
+84
View File
@@ -856,6 +856,21 @@ func TestChunkHandlerAddChunkValidatesListFields(t *testing.T) {
body: `{"content":"chunk body","important_keywords":{}}`,
wantMsg: "`important_keywords` is required to be a list",
},
{
name: "tag_kwd null",
body: `{"content":"chunk body","tag_kwd":null}`,
wantMsg: "`tag_kwd` is required to be a list",
},
{
name: "tag_kwd type",
body: `{"content":"chunk body","tag_kwd":{}}`,
wantMsg: "`tag_kwd` is required to be a list",
},
{
name: "tag_kwd element type",
body: `{"content":"chunk body","tag_kwd":["ok",42]}`,
wantMsg: "`tag_kwd` must be a list of strings",
},
}
for _, tt := range tests {
@@ -890,6 +905,75 @@ func TestChunkHandlerAddChunkValidatesListFields(t *testing.T) {
}
}
// TestChunkHandlerAddChunkForwardsTagKwd pins the Go/Python parity contract
// that callers can attach a tag list at write time and the field survives on
// the service-side AddChunkRequest. Issue #20138 documented that the Go path
// was silently dropping the field while the Python chunk API accepted it.
func TestChunkHandlerAddChunkForwardsTagKwd(t *testing.T) {
cases := []struct {
name string
body string
wantTag []string
wantNil bool
}{
{
name: "tag_kwd omitted",
body: `{"content":"chunk body"}`,
wantNil: true,
},
{
name: "tag_kwd explicit empty list",
body: `{"content":"chunk body","tag_kwd":[]}`,
wantTag: []string{},
},
{
name: "tag_kwd with values",
body: `{"content":"chunk body","tag_kwd":["alpha","beta"]}`,
wantTag: []string{"alpha", "beta"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
var got *service.AddChunkRequest
mock := &mockChunkSvc{
addChunkFn: func(ctx context.Context, req *service.AddChunkRequest, userID string) (*service.AddChunkResponse, error) {
got = req
return &service.AddChunkResponse{Chunk: map[string]interface{}{"id": "chunk-1"}}, nil
},
}
gin.SetMode(gin.TestMode)
r := gin.New()
r.Use(func(c *gin.Context) {
c.Set("user", &entity.User{ID: "user1"})
})
h := &ChunkHandler{chunkService: mock}
r.POST("/api/v1/datasets/:dataset_id/documents/:document_id/chunks", h.AddChunk)
w := httptest.NewRecorder()
req, _ := http.NewRequest("POST", "/api/v1/datasets/kb1/documents/doc1/chunks", strings.NewReader(tc.body))
req.Header.Set("Content-Type", "application/json")
r.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String())
}
if got == nil {
t.Fatal("service was not called")
}
if tc.wantNil {
if got.TagKwd != nil {
t.Fatalf("TagKwd = %v, want nil", got.TagKwd)
}
return
}
if !reflect.DeepEqual(got.TagKwd, tc.wantTag) {
t.Fatalf("TagKwd = %v, want %v", got.TagKwd, tc.wantTag)
}
})
}
}
func TestChunkHandlerAddChunkHidesServerErrorDetails(t *testing.T) {
mock := &mockChunkSvc{
addChunkFn: func(ctx context.Context, req *service.AddChunkRequest, userID string) (*service.AddChunkResponse, error) {
+10
View File
@@ -1418,6 +1418,13 @@ func (s *ChunkService) AddChunk(ctx context.Context, req *service.AddChunkReques
"docnm_kwd": docName,
"doc_id": req.DocumentID,
}
if req.TagKwd != nil {
// Mirror Python chunk_api.py: `tag_kwd` is only persisted when the
// caller actually supplied it. Omitting the field when absent avoids
// a Go/Python behavioural divergence on chunks that were not tagged
// at write time. See issue #20138.
chunkData["tag_kwd"] = req.TagKwd
}
if tagFeas != nil {
chunkData["tag_feas"] = tagFeas
}
@@ -1485,6 +1492,9 @@ func (s *ChunkService) AddChunk(ctx context.Context, req *service.AddChunkReques
"create_timestamp": chunkData["create_timestamp_flt"],
"create_time": chunkData["create_time"],
}
if tagKwd, ok := chunkData["tag_kwd"]; ok {
renamedChunk["tag_kwd"] = tagKwd
}
if imgID, ok := chunkData["img_id"]; ok {
renamedChunk["image_id"] = imgID
}
+116
View File
@@ -1092,6 +1092,122 @@ func TestAddChunkSuccess(t *testing.T) {
}
}
// TestAddChunkPersistsTagKwdOnlyWhenProvided pins the Go/Python parity
// contract that mirrors api/apps/restful_apis/chunk_api.py: the `tag_kwd`
// field is only persisted on the inserted chunk when the caller actually
// supplied it, and is echoed back in the response when present. Issue
// #20138 documented that the Go path was silently dropping the field.
func TestAddChunkPersistsTagKwdOnlyWhenProvided(t *testing.T) {
ctx := t.Context()
db := setupChunkTestDB(t)
pushChunkTestDB(t, db)
userID, datasetID, documentID := "user-1", "kb-1", "doc-1"
insertChunkTestKB(t, datasetID, userID)
insertChunkTestDoc(t, documentID, datasetID)
makeService := func() (*ChunkService, *addChunkTestEngine) {
engine := &addChunkTestEngine{}
svc := &ChunkService{
docEngine: engine,
kbDAO: dao.NewKnowledgebaseDAO(),
documentDAO: dao.NewDocumentDAO(),
accessibleFunc: func(datasetIDArg, userIDArg string) bool {
return datasetIDArg == datasetID && userIDArg == userID
},
getKnowledgebaseByIDFunc: func(id string) (*entity.Knowledgebase, error) {
return &entity.Knowledgebase{ID: id, TenantID: userID, EmbdID: "embed-1"}, nil
},
getEmbeddingModelFunc: func(string, string) (*models.EmbeddingModel, error) {
driver := &stubEmbeddingDriver{
embeddings: []models.EmbeddingData{
{Embedding: []float64{1, 2}},
{Embedding: []float64{3, 4}},
},
}
modelName := "embed-1"
return models.NewEmbeddingModel(driver, &modelName, &models.APIConfig{}, 0), nil
},
incrementChunkStatsFunc: func(_, _ string, _, _ int64, _ float64) error { return nil },
tokenizeFunc: func(text string) (string, error) { return text, nil },
fineGrainedTokenizeFunc: func(text string) (string, error) { return text + "_fg", nil },
numTokensFunc: func(text string) int { return len(text) },
}
return svc, engine
}
t.Run("omitted by caller", func(t *testing.T) {
svc, engine := makeService()
resp, err := svc.AddChunk(ctx, &service.AddChunkRequest{
DatasetID: datasetID,
DocumentID: documentID,
Content: "chunk body",
}, userID)
if err != nil {
t.Fatalf("AddChunk() error = %v", err)
}
if len(engine.insertedChunks) != 1 {
t.Fatalf("inserted chunks = %d, want 1", len(engine.insertedChunks))
}
if _, ok := engine.insertedChunks[0]["tag_kwd"]; ok {
t.Fatalf("expected tag_kwd to be absent when caller omits it; got %#v", engine.insertedChunks[0]["tag_kwd"])
}
if _, ok := resp.Chunk["tag_kwd"]; ok {
t.Fatalf("response should not include tag_kwd when caller omits it; got %#v", resp.Chunk["tag_kwd"])
}
})
t.Run("explicit empty list persists empty slice", func(t *testing.T) {
svc, engine := makeService()
resp, err := svc.AddChunk(ctx, &service.AddChunkRequest{
DatasetID: datasetID,
DocumentID: documentID,
Content: "chunk body",
TagKwd: []string{},
}, userID)
if err != nil {
t.Fatalf("AddChunk() error = %v", err)
}
got, ok := engine.insertedChunks[0]["tag_kwd"].([]string)
if !ok {
t.Fatalf("expected []string in tag_kwd, got %T", engine.insertedChunks[0]["tag_kwd"])
}
if len(got) != 0 {
t.Fatalf("expected empty tag_kwd, got %v", got)
}
if echoed, _ := resp.Chunk["tag_kwd"].([]string); len(echoed) != 0 {
t.Fatalf("response tag_kwd = %v, want empty", resp.Chunk["tag_kwd"])
}
})
t.Run("values are persisted and echoed", func(t *testing.T) {
svc, engine := makeService()
want := []string{"alpha", "beta"}
resp, err := svc.AddChunk(ctx, &service.AddChunkRequest{
DatasetID: datasetID,
DocumentID: documentID,
Content: "chunk body",
TagKwd: want,
}, userID)
if err != nil {
t.Fatalf("AddChunk() error = %v", err)
}
got, ok := engine.insertedChunks[0]["tag_kwd"].([]string)
if !ok {
t.Fatalf("expected []string in tag_kwd, got %T", engine.insertedChunks[0]["tag_kwd"])
}
if !reflect.DeepEqual(got, want) {
t.Fatalf("inserted tag_kwd = %v, want %v", got, want)
}
echoed, ok := resp.Chunk["tag_kwd"].([]string)
if !ok {
t.Fatalf("response tag_kwd type = %T, want []string", resp.Chunk["tag_kwd"])
}
if !reflect.DeepEqual(echoed, want) {
t.Fatalf("response tag_kwd = %v, want %v", echoed, want)
}
})
}
func TestAddChunkValidationErrors(t *testing.T) {
ctx := t.Context()
db := setupChunkTestDB(t)
+1
View File
@@ -76,6 +76,7 @@ type AddChunkRequest struct {
Content string `json:"content"`
ImportantKeywords []string `json:"important_keywords,omitempty"`
Questions []string `json:"questions,omitempty"`
TagKwd []string `json:"tag_kwd,omitempty"`
TagFeas interface{} `json:"tag_feas,omitempty"`
ImageBase64 *string `json:"image_base64,omitempty"`
}